Repository navigation
feat(session): add desired configuration rolling refresh - #668
Conversation
SunSi12138
left a comment
SunSi12138
left a comment
There was a problem hiding this comment.
本轮复审确认上次 5 条 blocker 均已实质修复：shared dial permit、linearizable admission cut、Server-owned rollout、structured publication result、zero-retiring selection→registration race 都已闭环。当前还剩 1 个新的 replacement-publication race blocker；另外 #669 已进入 dev，当前 head 相对最新 dev 仍 behind_by=1，修复后请一并同步再跑 exact-head gate。
SunSi12138
left a comment
SunSi12138
left a comment
There was a problem hiding this comment.
继续复审发现另一个与 #651 bounded/reclaim contract 直接相关的 blocker：planned-refresh redirect 当前是逐代强引用链，连续 refresh + 长生命周期旧调用可以让链和已 dispose 的中间 connection/session 对象持续增长；32-hop cap 最终还会让极端 stale snapshot admission 失败。请把 redirect convergence 做成有界结构，并补 long-lived source + repeated successful refresh 的 deterministic regression。
The blue-green eligibility cut reserved replacement admission but did not freeze the replacement's fatal transition, so a replacement that failed after the reservation and before the source admission closure still let the cut proceed against a dead target. Linearize the cut on a connection-level commit CAS: the admission reservation stays in TryReserveSessionRefreshCommit, the separate TryCommitSessionRefreshRetirement claims the cut, and ObserveFatalFailureForAdmission (including Fail) rejects an unclaimed commit. A failure published first rolls the replacement back, keeps the source selectable, and retries the refresh debt; a failure after the claim follows ordinary post-cut semantics. Fixed, static, and dynamic cut sites all use the same claim. Replace the fixed 32-hop predecessor chain with one shared SessionRefreshRedirect per refresh lineage. Every retired generation points at the same indirection and it holds only the newest Ready replacement, so a pinned long-lived source no longer retains disposed predecessors and stale admission resolves in constant depth regardless of how many rolling refreshes completed. Regressions cover the post-reservation pre-cut replacement failure for fixed/static/dynamic, and more than 32 successful refreshes pinned by a long-lived server-streaming call.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc5e663eaa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b077841238
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efbdcff165
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4dadc3da8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
FutureOnlyvsRollingRefreshintentTryPublishDesiredSessionAsync(...)results for expected lifecycle/implementation rejection while retaining the throwing convenience APISessionRefreshRequestedwith server-instance-scoped generation identity and bounded 24-byte payload validation in the common Protocol v2 frame parserRpcSessionreference identity so a delayed disconnect cannot remove a newer session pin when a transport ID is reused; no ambientAsyncLocalstate reaches the RPC request hot pathMaxRetiringConnections = 0: admitted source work drains while the source remains physically Ready; further refreshes wait until that planned source drainsConnectTransportAsync(...)physical-dial limiter from fix(client): enforce MaxConcurrentClusterConnects on physical dials #666; the temporary replace-before-retire exception applies to connection capacity, not concurrent dial countScope
The initial runtime-published desired Session configuration intentionally includes only
MaxFramePayloadBytes, constrained by the immutable build-time hard ceiling. This PR does not add active-session flow-control renegotiation, compression wire-profile renegotiation, TLS/listener topology replacement, or generic Builder hot reload.Validation
Direct regression coverage includes:
FutureOnly -> same-config RollingRefresh, caller cancellation/retry, and same-generation Rolling arriving during an active scanMaxRetiringConnections = 0selection-to-registration racesMaxConcurrentClusterConnects = 1, proving aggregate physical-dial concurrency remains boundedLocal validation for the final cut protocol: 1802 Release Unit tests pass in UTC, including first-generation and repeated-refresh interleavings, the fixed/static/dynamic rollback and reconnect regressions, and 40-generation bounded redirect tests. Formatting and maintainability gates pass. Negative controls reproduce idle-budget and reused-ID failures, and the latest interaction regressions also fail against their preceding implementation in all three debt-transfer paths, fixed resizing, and fatal one-way admission. Negative controls that remove fatal synchronization or let source admission ignore the published cut both fail their behavioral assertions. See the PR Checks tab for CI on the current commit; preceding PR Quick/Extended runs are historical evidence for their earlier commits.
Closes #651