bitflicker64 commented on code in PR #3204:
URL: https://github.com/apache/hugegraph/pull/3204#discussion_r4004358083
##########
hugegraph-store/hg-store-client/src/main/java/org/apache/hugegraph/store/client/NodeTxExecutor.java:
##########
@@ -373,35 +391,75 @@ boolean ifAnyTrue(Supplier<Stream<HgPair<HgStoreNode,
NodeTkv>>> nodeStreamSuppl
}
<T> Optional<T> retryingInvoke(Supplier<T> supplier) {
+ boolean[] deadlineRetried = {false};
return IntStream.rangeClosed(0, NODE_MAX_RETRYING_TIMES).boxed()
.map(
i -> {
+ if
(Thread.currentThread().isInterrupted()) {
+ // The caller (e.g. a REST worker
hitting
+ // restserver.request_timeout) gave
up: stop
+ // retrying instead of holding its
thread.
+ // InterruptedException as the root
cause: the
+ // server's task cancel path
recognises it
+ // (HugeException.isInterrupted()).
+ throw HgStoreClientException.of(
+ "Interrupted before retry " +
i,
+ new InterruptedException());
+ }
T buffer = null;
try {
buffer = supplier.get();
} catch (Throwable t) {
- if (i + 1 <= NODE_MAX_RETRYING_TIMES) {
- try {
- int sleepTime;
- // The first three times try
once every second
- if (i < 3) {
- sleepTime = 1;
- } else {
- // Subsequent incremental
- sleepTime = i - 1;
- }
- log.info("Waiting {} seconds "
+
- "for the next try.",
- sleepTime);
- Thread.sleep(sleepTime *
1000L);
- } catch (InterruptedException e) {
- log.error("Failed to sleep",
e);
+ Failure failure = classify(t);
+ if (failure != Failure.DEADLINE) {
+ // a different failure in between
means the
+ // next deadline is not "in a row"
again
+ deadlineRetried[0] = false;
Review Comment:
⚠️ Resetting the budget here lets one call block on up to six full
deadlines, not two.
Evidence:
- The loop makes 11 attempts (`rangeClosed(0, NODE_MAX_RETRYING_TIMES)`).
With D, X, D, X, ... D, where X is any fast retryable failure, every D gets
retried: 6 × `grpc.timeout.seconds` (600 s on defaults) plus about 38 s of
backoff.
- That pattern is reachable. The DEADLINE sends NOT_WORK without the node
(`NotifyingExecutor.java:249-254`), which reloads partition leaders. While raft
is still electing, or PD still lists the stalled store, the next attempt can
fail quickly with NOT_LEADER, a partition fault or UNAVAILABLE. That resets
this flag, and the attempt after it waits on the stalled store again.
- The description gives the bound as 2 × deadline + backoff. That holds only
for back-to-back deadlines. The request-timeout interrupt still frees REST
workers, but callers that are never interrupted (Gremlin jobs without
`evaluationTimeout`, background tasks) can be held for about 10 minutes again.
Could you bound deadlines per `retryingInvoke` call instead, e.g. count
DEADLINE failures and stop at the second one even if something else failed in
between? The javadoc on `Failure` and
`testDeadlineBudgetResetsAfterAnotherFailure` would need updating to match.
--
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]
