bitflicker64 commented on code in PR #3204:
URL: https://github.com/apache/hugegraph/pull/3204#discussion_r3990841696
##########
hugegraph-store/hg-store-client/src/main/java/org/apache/hugegraph/store/client/NodeTxExecutor.java:
##########
@@ -373,35 +398,65 @@ 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.
+ throw HgStoreClientException.of(
Review Comment:
🧹 Two of the new interrupt exits drop the `InterruptedException`, so the
server cannot tell them apart from a store failure.
Evidence:
- `:409-410` throws `HgStoreClientException.of("Interrupted before retry " +
i)` with no cause. `:456-458` uses the store failure `t` as the cause and
discards `e`. Neither has an `InterruptedException` as its root cause.
- The FATAL path keeps one. A blocking stub on an interrupted thread fails
with `CANCELLED`, and the `InterruptedException` is its cause: grpc-stub 1.39.0
`ClientCalls.blockingUnaryCall` calls `call.cancel("Thread interrupted", e)`.
- `HugeException.isInterrupted()` only checks the root cause
(`HugeException.java:56-61`). `HugeTask.fail()` relies on it so that a
cancelled task is not recorded as failed (`HugeTask.java:351-355`). Take a task
that is cancelled while its thread is between store calls or in the retry
sleep. It now logs a WARN with a stack trace. If the worker reaches `fail()`
before `cancel()` sets CANCELLED (`:323` interrupts, `:336` sets the status),
the task is stored as FAILED and `cancel()` returns false, so
`DistributedTaskScheduler.cancel()` does not save CANCELLED (`:317-321`).
Before this change the loop swallowed the interrupt and carried on, so this
path did not exist.
Requested change: make `InterruptedException` the root cause on both exits.
For example, use `HgStoreClientException.of("Interrupted before retry " + i,
new InterruptedException())`. In the sleep handler, use
`HgStoreClientException.of("Interrupted while waiting to retry: " +
t.getMessage(), e)` and attach `t` with `addSuppressed`. Then extend
`testInterruptStopsRetrying` and `testInterruptBeforeCallSkipsTheAttempt` to
assert the root cause.
--
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]
