dcapwell commented on code in PR #3379:
URL: https://github.com/apache/cassandra/pull/3379#discussion_r1645033471
##########
src/java/org/apache/cassandra/service/accord/AccordService.java:
##########
@@ -430,57 +442,76 @@ public long repair(@Nonnull Seekables keysOrRanges, long
epoch, long queryStartN
return barrier(keysOrRanges, epoch, queryStartNanos, timeoutNanos,
barrierType, isForWrite, repairSyncPoint(allNodes));
}
- private static ReadTimeoutException newBarrierTimeout(TxnId txnId, boolean
global)
+ @VisibleForTesting
+ static ReadTimeoutException newBarrierTimeout(TxnId txnId, boolean global)
{
return new ReadTimeoutException(global ? ConsistencyLevel.ANY :
ConsistencyLevel.QUORUM, 0, 0, false, txnId.toString());
}
- private static ReadTimeoutException newBarrierPreempted(TxnId txnId,
boolean global)
+ @VisibleForTesting
+ static ReadTimeoutException newBarrierPreempted(TxnId txnId, boolean
global)
{
return new ReadPreemptedException(global ? ConsistencyLevel.ANY :
ConsistencyLevel.QUORUM, 0, 0, false, txnId.toString());
}
- private long doWithRetries(LongSupplier action, int retryAttempts, long
initialBackoffMillis, long maxBackoffMillis) throws InterruptedException
+ @VisibleForTesting
+ static ReadExhaustedException newBarrierExhausted(TxnId txnId, boolean
global)
+ {
+ //TODO (usability): not being able to show the txn is a bad UX, this
becomes harder to trace back in logs
+ return new ReadExhaustedException(global ? ConsistencyLevel.ANY :
ConsistencyLevel.QUORUM, 0, 0, false, ImmutableMap.of());
+ }
+
+ @VisibleForTesting
+ static boolean isTimeout(Throwable t)
+ {
+ return t instanceof Timeout || t instanceof ReadTimeoutException || t
instanceof Preempted || t instanceof ReadPreemptedException;
+ }
+
+ @VisibleForTesting
+ static long doWithRetries(Blocking blocking, LongSupplier action, int
retryAttempts, long initialBackoffMillis, long maxBackoffMillis) throws
InterruptedException
{
// Since we could end up having the barrier transaction or the
transaction it listens to invalidated
- RuntimeException existingFailures = null;
+ Throwable existingFailures = null;
Long success = null;
- long backoffMillis = 0;
+ long backoffMillis = initialBackoffMillis;
for (int attempt = 0; attempt < retryAttempts; attempt++)
{
- try
- {
- Thread.sleep(backoffMillis);
- }
- catch (InterruptedException e)
- {
- if (existingFailures != null)
- e.addSuppressed(existingFailures);
- throw e;
- }
- backoffMillis = backoffMillis == 0 ? initialBackoffMillis :
Math.min(backoffMillis * 2, maxBackoffMillis);
try
{
success = action.getAsLong();
break;
}
catch (RequestExecutionException | CoordinationFailed newFailures)
{
- existingFailures = Throwables.merge(existingFailures,
newFailures);
+ existingFailures = FailureAccumulator.append(existingFailures,
newFailures, AccordService::isTimeout);
+
+ try
+ {
+ blocking.sleep(backoffMillis);
+ }
+ catch (InterruptedException e)
+ {
+ if (existingFailures != null)
+ e.addSuppressed(existingFailures);
+ throw e;
+ }
+ backoffMillis = Math.min(backoffMillis * 2, maxBackoffMillis);
}
+ // if an unknown/unexpected error happens retry stops right away
}
if (success == null)
{
checkState(existingFailures != null, "Didn't have success, but
also didn't have failures");
- throw existingFailures;
+ Throwables.throwIfUnchecked(existingFailures);
+ throw new RuntimeException(existingFailures);
Review Comment:
this is possible on the JVM, but super tacky to hit this case... you either
do this *outside* of javac or trick javac... In the common case (most likely
100%) we will return from the previous line
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
