You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hugegraph.apache.org by "imbajin (via GitHub)" <gi...@apache.org> on 2026/10/12 02:18:39 UTC
[PR] fix(store): recover channels after unavailable scans [hugegraph]
imbajin opened a new pull request, #3313:
URL: https://github.com/apache/hugegraph/pull/3313
## Purpose of the PR
After a Store pod is replaced, an existing Server can continue using stale Store channels for graph scans. Unary RPC failures retire the affected node and channels, while scan failures previously only propagated the iterator error.
Related [Server acceptance report](https://github.com/hugegraph/hugegraph-toolchain/pull/47#issuecomment-6065397279).
## Main Changes
- Reuse the existing expected-node invalidation path when a scan receives `UNAVAILABLE`, allowing later scans to rebuild their channel bindings.
- Preserve the original scan error, healthy nodes after cancellation or invalid requests, and same-address replacement nodes after late failure notifications.
- Add regression coverage for channel retirement and rebinding, error propagation, and replacement preservation.
## Verifying these changes
- [ ] Trivial rework / code cleanup without any test coverage. (No Need)
- [ ] Already covered by existing tests.
- [x] Need tests and can be verified as follows:
- Scanner and shared channel-pool regression tests, Java 17 compilation, formatting and style checks passed.
- In an authenticated single-node deployment, only vertex scans were issued after replacing the Store pod. Subsequent scans recovered on the same Server process; full graph data, schema and index readback matched the baseline. The first scan still timed out.
- Independent code and runtime-evidence reviews completed. The original Server image was restored after validation. PD-only restart, multi-node recovery and other streaming query paths remain outside this validation.
## Does this PR potentially affect the following parts?
- [ ] Dependencies
- [ ] Modify configurations
- [ ] The public API
- [x] Other affects (Store client stream connection recovery)
- [ ] Nope
## Documentation Status
- [ ] `Doc - TODO`: required documentation is pending; complete it before merging.
- [ ] `Doc - Done`: documentation is included here or linked below.
- [x] `Doc - No Need`: internal client connection invalidation.
Documentation files in this PR or paired hugegraph-doc PR:
Same repair branch as [org PR #290](https://github.com/hugegraph/hugegraph/pull/290); upstream submission does not imply merged or upstream-mainline validation.
--
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix(store): recover channels after unavailable scans [hugegraph]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #3313:
URL: https://github.com/apache/hugegraph/pull/3313#issuecomment-6116679821
## [Codecov](https://app.codecov.io/gh/apache/hugegraph/pull/3313?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 48.86%. Comparing base ([`7644566`](https://app.codecov.io/gh/apache/hugegraph/commit/7644566e58f0a3f9acabfef7ab448714b7afe1f1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`ad0d63d`](https://app.codecov.io/gh/apache/hugegraph/commit/ad0d63d0f7beca6ff8e48e02772f952536c50d94?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #3313 +/- ##
============================================
- Coverage 52.29% 48.86% -3.43%
+ Complexity 6052 820 -5232
============================================
Files 762 66 -696
Lines 67399 6137 -61262
Branches 9138 658 -8480
============================================
- Hits 35244 2999 -32245
+ Misses 28601 2891 -25710
+ Partials 3554 247 -3307
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/hugegraph/pull/3313?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix(store): recover channels after unavailable scans [hugegraph]
Posted by "imbajin (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix(store): recover channels after unavailable scans [hugegraph]
Posted by "imbajin (via GitHub)" <gi...@apache.org>.
imbajin commented on code in PR #3313:
URL: https://github.com/apache/hugegraph/pull/3313#discussion_r4245000640
##########
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:
Rechecked at 6c9a272561b15aa578d7f2ccf33e7bd537400ea9: this specific P1 is addressed in the current source.
serverScan() now stops outgoing requests without closing the receive buffer, leaving terminal publication to the response callback after retirement and error storage. I independently reran the bounded Java extraction model against both control flows: the earlier version returned false EOF during blocked retirement, while the updated version kept the consumer waiting and then delivered the original UNAVAILABLE after retirement. The new testReceiverEnteringDuringRetirementDoesNotSeeEof regression covers exactly the missing ordering, including no additional outgoing request and retry with a new channel.
I also checked the final-page completion-exception guard and its regression; it prevents a request-side completion exception from dropping the last page. This is independent source/model verification, not a fresh full Maven or deployed gRPC run. Current-head CI, coverage and the original deployment-recovery acceptance remain separate; no earlier green result is inherited. The previous 7/10 assessment and P1 finding apply to the earlier revision, not this repaired head.
--
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix(store): recover channels after unavailable scans [hugegraph]
Posted by "imbajin (via GitHub)" <gi...@apache.org>.
imbajin merged PR #3313:
URL: https://github.com/apache/hugegraph/pull/3313
--
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org