Replace blocking wait with non-blocking delay in paxos repair #4434
Conversation
| private static class DelayedRepair | ||
| { | ||
| private final UncommittedPaxosKey uncommitted; | ||
| private final long startAfterMillis; |
There was a problem hiding this comment.
I assumed this was a time delta when I read it (i.e. after millis (have elapsed)). Perhaps atMillis or startAtMillis, or scheduledAtMillis?
But also, this should probably be nanos and we should probably use nanoTime?
| private final boolean autoRepair; | ||
|
|
||
| private final Map<DecoratedKey, AbstractPaxosRepair> inflight = new ConcurrentHashMap<>(); | ||
| private final Queue<DelayedRepair> delayed = new LinkedBlockingQueue<>(); |
There was a problem hiding this comment.
Given usage, can't we just use CLQ?
There was a problem hiding this comment.
You mean concurrent linked queue? delayed is only accessed inside synchronized blocks so I don't think we'd gain anything by using it
There was a problem hiding this comment.
You're right, totally fine with swapping it for ArrayDeque instead (or even LinkedList). Was just trying to downgrade LinkedBlockingQueue, as probably too heavy weight.
| logger.info("Paxos auto repair encountered a potentially in progress ballot, sleeping {}ms to allow the in flight operation to finish", sleepMillis); | ||
|
|
||
| delayed.add(new DelayedRepair(uncommitted, nowMillis + sleepMillis)); | ||
| ScheduledExecutors.scheduledFastTasks.schedule(this::scheduleKeyRepairsOrFinish, sleepMillis, MILLISECONDS); |
There was a problem hiding this comment.
I think we need to synchronise scheduleKeyRepairsOrFinish? Or wrap it in a synchronise call inside the lambda. Currently it inherits its safety from its callers
| return false; | ||
|
|
||
| if (waitForCoordinator) | ||
| maybeWaitForOriginalCoordinator(uncommitted, txnTimeoutMicros); |
There was a problem hiding this comment.
Can we also delete maybeWaitForOriginalCoordinator?
| @Override | ||
| public int compare(DelayedRepair o1, DelayedRepair o2) | ||
| { | ||
| long delta = o1.scheduledAtNanos - o2.scheduledAtNanos; |
2 participants
Thanks for sending a pull request! Here are some tips if you're new here:
Commit messages should follow the following format:
The Cassandra Jira