Repository navigation
Keep reconnect state out of the next_event future - #152
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #152 +/- ##
==========================================
+ Coverage 96.27% 96.37% +0.10%
==========================================
Files 36 36
Lines 28507 28657 +150
==========================================
+ Hits 27444 27619 +175
+ Misses 1063 1038 -25
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
isdaniel
force-pushed
the
perf/slim-next-event-future
branch
from
October 31, 2026 00:24
2da414e to
b76ac5f
Compare
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the WAL streaming future-size reduction.
No open issues 🎉
What changed in this PR
This PR reduces read-future sizes in the WAL streaming layer by keeping reconnect state and large temporary values out of the retry future, while preserving retry and recovery behavior.
Changes:
- Boxes both cold reconnect awaits and limits the retry loop’s retained state to the error.
- Adds a 1 KB future-size regression guard and tests for retry backoff and cancellation.
- Enables Tokio’s test utilities for paused-time tests.
| File | Description |
|---|---|
src/stream.rs |
Reduces read-future sizes and adds size, backoff, and cancellation regression tests. |
Cargo.toml |
Enables Tokio’s test-util feature for deterministic timer tests. |
Review effort: Balanced
a memcpy of it each time: +7% CPU per event on the rustls backend, +4% on libpq. Box the two recover_connection awaits, since reconnecting is cold. The retry loop also held its whole match scrutinee, a Result<Option<ChangeEvent>>, across the awaits in its error arm, a 1 KB slot in the future; bind only the error out of the match. Keep the next_event_before future small and guard it EventStream::next_event_before matched `self.held.take()` and `resume_parked_poll().await` as nested scrutinees, so both Option<ChangeEvent> temporaries lived across the final read: its future was 2,800 B against 728 B for next_event. It runs only while an aligned snapshot catches up, so the cost is small, but it is the same pattern the previous commit removes from the retry loop, and the guard could not cover it. Take each early read in its own `if let` that returns, so its event ends before the next await. The future is now 720 B. Behaviour is unchanged. Extend read_path_futures_stay_small to both next_event_before futures. Add the two cases no test reached: a parked poll that ended in an error hands it to the next read, and a commit exactly at the boundary is held while one LSN below is delivered (the boundary is a copy's consistent point, which contains everything before it and nothing at or after it). Test the retry loop's permanent-error and live-connection arms Two arms of next_event_with_retry_until had no test: a permanent read error ends the loop at once, and a decode error on a connection that is still up is surfaced, not retried, because the bad frame is already consumed and a retry would silently skip a change. A null test connection never reports alive and never carries a server error, so neither was reachable. Add two test-only seams on the native connection: one in COPY mode whose server end the test holds, so an ErrorResponse written to it comes out of get_copy_data_async through the real worker; and a setter for the liveness flag the worker normally keeps. The permanent case runs on a real worker and socket with a hard timeout; the live-connection case uses the paused clock, which shows that no backoff started. Both run on the rustls backend; the lines they cover are shared.
isdaniel
force-pushed
the
perf/slim-next-event-future
branch
from
October 31, 2026 03:00
b76ac5f to
b372e9f
Compare
LucaCappelletti94
approved these changes
Oct 31, 2026
4 of 10 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
After f96c6d2 (non-blocking startup), the future returned by
EventStream::next_event()grew from 4.5 KB to 8.4 KB on the default rustls backend.recover_connection()was awaited inline, so its whole connect/TLS state machine became part of every read future, even though reconnecting is cold.Two changes, both in
src/stream.rs:recover_connection()awaits (check_connection_health,next_event_with_retry_until). Reconnecting is cold; the cost is one allocation per reconnect.match. The loop awaited inside theErrarm ofmatch self.next_event_until(..).await, which keeps the match scrutinee (Result<Option<ChangeEvent>>, ~1 KB) alive across those awaits, so the future carried aChangeEvent-sized slot. Boxing alone left the future at 1,768 B; with both changes it is 728 B on rustls (4,504 B on v0.9.0). Most of the diff is the resulting de-indent;git diff -wis +35/-6.Also adds
read_path_futures_stay_small, which asserts thenext_event/next_event_with_retry/EventStream::next_eventfutures stay under 2 KB.stream.rsis excluded from the CodSpeed build, so nothing else would notice this growing again.No behavior change: same retry, backoff and recovery logic.
Verification
next_event_with_retry future grew to 8392 bytes) and passes here.Related issues
n/a. Found while reviewing v0.9.0..main.
Type of change
Checklist
make before-git-pushpasses locally (check, build, format, audit, test, doc-check)--features rustls-tlsif connection/streaming code changed)