Repository navigation
refactor(server): coordinate graph rpc lifecycle #3329
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
04e31cd
fix(server): register late graph cache rpc
imbajin 665ebe7
fix(server): protect RPC service cleanup
imbajin 736d29f
fix(server): preserve RPC graph ownership
imbajin 5f5825d
fix(test): await rpc listener close
imbajin e7fb041
fix(server): retain rebase resolutions
imbajin 11a3cfc
fix(server): preserve rpc lifecycle on failure
imbajin cf26235
fix(server): preserve cleanup errors and retries
imbajin 9c4725d
fix(server): retain graph cleanup retry ownership
imbajin 8fa2d60
chore(server): merge latest master
imbajin 969bd33
Merge remote-tracking branch 'origin/master' into HEAD
imbajin f403170
fix: complete graph cleanup and task deletion
imbajin b074481
chore: merge master into rpc lifecycle
imbajin 4b2ca10
fix(server): consume local graph add signals
imbajin 1549a1f
fix(server): retain peer cleanup priority
imbajin ff5926e
fix(server): clear failed metadata delete token
imbajin f520505
refactor(server): simplify close failure collection
imbajin 92f92f1
fix(server): defer pending adds during reload
imbajin b72c5fa
fix(server): sync graph lifecycle with master
imbajin bb45358
fix(server): integrate local graph binding
imbajin 22ec7bb
fix(server): retain rpc after peer close failure
imbajin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Keep a stable proxy-to-bootstrap identity across failed cleanup retries.
At this head, removeServiceProxy matches the original proxy against bootstrap.getProxyIns(). In SOFA RPC 5.7.6, DefaultConsumerBootstrap.unRefer() catches an Exception from cluster.destroy() and then clears proxyIns. If the explicit cluster.destroy() in cleanup() also fails, this code retains the bootstrap for retry, but the next removeServiceProxy call with the original proxy no longer matches it and returns false. StandardHugeGraph's cleanup Runnable ignores that boolean and is then removed, so graph-scoped cleanup can report completion while the failed bootstrap is still retained until global destruction.
Please retain a stable original-proxy/bootstrap association until cleanup fully succeeds, and add a regression that retries the same removeServiceProxy after unRefer has cleared proxyIns and cluster destruction initially fails. Existing failure tests retry owner.destroy(), which does not exercise this path. This finding is based on current-head source tracing, not an independently executed native failure test.
Upstream behavior: https://github.com/sofastack/sofa-rpc/blob/v5.7.6/bootstrap/bootstrap-api/src/main/java/com/alipay/sofa/rpc/bootstrap/DefaultConsumerBootstrap.java#L228-L256
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. The SOFA 5.7.6 source path is confirmed, but the native destroy-failure sequence has not been reproduced. Existing serviceId/ConsumerConfig references are shared by multiple proxies, so they cannot safely replace a per-registration identity. Following the agreed scope, this conflict refresh will not introduce a new cleanup registry or a null-proxy matching fallback. I have recorded a separate follow-up to reproduce this sequence with a second live proxy of the same service before choosing a narrow fix. Global provider destruction still retains the bootstrap for retry. Leaving this thread open because the reported failure path is not fixed.