Repository navigation
fix(net): let resumed subscriber forks follow route swaps - #6024
Conversation
Co-Authored-By: GPT-6.1-Sol <noreply@openai.com>
Restore the original quest while finite group-only closure remains unresolved. The implementation and prior source validation remain at caca147. Co-Authored-By: GPT-6.1-Sol <noreply@openai.com>
The merged main replaced `Demand.unused()` with `wanted`; wait through `unwanted` instead. Restore the fork comment's reason for not resolving through the broadcast again. The fork quest is complete: the group-only closure gap it inherits is the existing `resume-closed` quest's, shared by every resumed reader. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 5 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: e57cc99762
ℹ️ 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".
A fork made between routes or after its front concluded found no route source and ended with a dropped route, though its parent still held the copy and its retained groups. Seed the fork with a fork of the parent's newest copy at that copy's generation, then follow the route as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 participant
Problem
A resumed JavaScript subscriber fork stayed on the route serving when it was created. If that route failed during an incomplete group, the fork failed instead of reading the remaining frames from its replacement. This also affected IETF FILL reads.
Approach
Open an independent logical reader on the same route, reusing the parent's reader implementation. The fork starts on a fork of the parent's newest copy, at that copy's generation, so it keeps the retained groups between routes and after the front concludes, then follows later routes like any reader. Cover recovery after the parent closes, independent preferences and replay cursors, cleanup, and shared on-demand requests. The fork comment keeps its reason for not resolving through the broadcast again and notes that a routed fork follows route swaps.
Rust FILL already fetches through the logical track, so it needs no change.
Retires
quest/m1/resume-fork.md.Impact
Alternatives
Forking the serving copy cannot follow replacement routes. Reusing the logical reader avoids a second recovery implementation.
Decisions
No maintainer decision was given; the recommended option was taken. The earlier no-merge hold still stands until the maintainer lifts it.
A finite group-only fork's
closedstays pending afterrecvGroup()EOF. The fork now shares the logicalReader, so it has the same gap every resumed reader has today. Its only caller, the IETF FILL path, always closes its fork explicitly and never awaitsclosed, so nothing leaks or hangs in practice./quest/m1/resume-closed.md, whose first bullet is exactly this for every resumed reader, forks included. Retire the fork quest.[M]resume-closed quest and its end-policy questions.Validation
origin/main; adapted the preferences test to feat(net)!: track demand reports the wanted group, with refresh on join #5930'sDemand.wanted(waits throughunwanted).Front.conclude()orpark()replaying its parent's copy fails without the seeding fix.just checkpassed at the new head.Follow-ups
/quest/m1/resume-closed.md: its group-only closure test should also cover a fork, since forks share theReader.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code