imbajin commented on code in PR #3313:
URL: https://github.com/apache/hugegraph/pull/3313#discussion_r4244455574
##########
hugegraph-store/hg-store-client/src/main/java/org/apache/hugegraph/store/client/grpc/KvPageScanner.java:
##########
@@ -319,6 +321,13 @@ public void onError(Throwable t) {
} catch (Exception e) {
log.warn("failed to invoke requestObserver.onCompleted(),
reason:", e.getMessage());
}
+ // Retire the failed pool before publishing the original iterator
error,
+ // so immediate retries cannot reuse it. Cancellation or protocol
errors
+ // must not evict healthy nodes, and stale notices preserve
replacements.
+ if (Status.fromThrowable(t).getCode() == Status.Code.UNAVAILABLE) {
+ new NotifyingExecutor(session.getGraphName(),
+ HgStoreNodeManager.getInstance(),
session).handleErr(t);
Review Comment:
[P1] Prevent a concurrent consumer from observing clean EOF during retirement
Retiring the failed pool before exposing the error is the right ordering for
retries, but completed is already set to true at line 318. If a consumer enters
hasNext() while handleErr() is still retiring/notifying the node,
HgBufferProxy.receive() can call serverScan(), which sees completed and closes
the proxy. receive() then finds no stored error yet and returns null, so
hasNext() returns false. A normal iterator consumer stops there and may
silently accept an incomplete scan instead of the original UNAVAILABLE.
A bounded Java extraction model using this head's HgBufferProxy body and the
scanner's terminal/control flow, with retirement gated by a latch, reproduced:
hasNext during retirement = false; proxy closed before error publication =
true; only a later hasNext sees the original error. This is not a full-product
deployment test. The completed-before-error window pre-existed, but the new
synchronous retirement materially widens it.
Please coordinate terminal publication so readers cannot observe successful
completion until retirement and error publication are finished, while
preserving retirement-before-error for retries. Add a regression that pauses
retirement and starts a new consumer at that point; the new test currently
starts the consumer before onError and therefore misses this interleaving.
--
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]
