feat(sdk): act on s2s reconnect advice in append and read sessions - #703
feat(sdk): act on s2s reconnect advice in append and read sessions#703infiniteregrets wants to merge 13 commits into
Conversation
f667d83 to
54c3568
Compare
Greptile SummaryThe PR propagates S2S reconnect advice into append and read session control flow so sessions can hand over proactively without consuming retry budget.
Confidence Score: 5/5The PR appears safe to merge; no actionable correctness, security, or compatibility failures were identified. Append advice drains the active acknowledgement stream before handoff, while read advice advances the resume position before reconnecting, preserving append ambiguity guarantees and read continuity. Important Files Changed
Sequence DiagramsequenceDiagram
participant Server
participant Decoder
participant Session
participant Replacement as New connection
Server-->>Decoder: Regular S2S frame with reconnect flag
Decoder->>Session: Decoded ack or read batch
Decoder->>Session: Set per-connection advice signal
alt Append session
Session->>Server: Half-close request body
Server-->>Session: Drain remaining acknowledgements and EOF
else Read session
Session->>Session: Advance resume sequence past delivered batch
end
Session->>Replacement: Reconnect without consuming retry budget
Reviews (1): Last reviewed commit: "feat(sdk): act on s2s reconnect advice i..." | Re-trigger Greptile |
A server that is about to terminate sets the reconnect-advised bit on the regular frames it sends. Sessions now finish the current connection cleanly and move to a fresh one, rather than staying attached until the terminal server_draining error arrives. An append session stops admitting inputs, half-closes so the server can acknowledge everything it accepted, drains those acknowledgements, and then reconnects. Because the whole request body is already on the wire before the half-close, a clean end means nothing is left unacknowledged, so this holds even under AppendRetryPolicy::NoSideEffects. A read session finishes the batch the advice arrived on and resumes from the position after it. Neither path is treated as a failure, so no retry budget is consumed, and both pace themselves if advice repeats without progress. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
54c3568 to
c2d50d5
Compare
| .await?; | ||
| Ok(Box::pin(try_stream! { | ||
| loop { | ||
| match timeout(Duration::from_secs(20), batches.next()).await { |
There was a problem hiding this comment.
not a part of this PR I know, but lets move this duration into a const with a comment, so its less magic-y and clearly related to the document s2s heartbeat max https://s2.dev/docs/api/protocol#data-flow (plus some buffer)
There was a problem hiding this comment.
thanks, please check!
| Some(Ok(batch)) => { | ||
| Some(Ok(ReadItem::ReconnectAdvised)) => { | ||
| batches = None; | ||
| if end.count == Some(0) || end.bytes == Some(0) { |
There was a problem hiding this comment.
this is confusing -- maybe lets have a util fn that clarifies that we are checking if the read end condition is already satisfied, like read_limits_exhausted(&end) or something, and maybe a comment stating that this is an optimization, to avoid a useless reconnect for a read that was already satisfied at the moment we received the reconnect signal
2 participants
No description provided.