Fix broken WAL delta after stream records abort#7791
Merged
timvisee merged 14 commits intoabort-stream-records-breaks-wal-deltafrom Dec 17, 2025
Merged
Conversation
timvisee
commented
Dec 16, 2025
Comment on lines
15
to
22
| #[serde(from = "ClockMapHelper", into = "ClockMapHelper")] | ||
| pub struct ClockMap { | ||
| clocks: HashMap<Key, Clock>, | ||
| /// Whether this clock map has changed since the last time it was persisted. | ||
| /// Optional snapshot with earlier version of clocks | ||
| snapshot: Option<HashMap<Key, Clock>>, | ||
| /// Whether this clock map has changed since the last time it was persisted | ||
| changed: bool, | ||
| } |
Member
Author
There was a problem hiding this comment.
Here I persist the optional snapshot together with regular clocks.
It took me a few iterations to finally land here. In my opinion this is the best approach because:
- we prevent adding new files
- we prevent conditional flushers
- we now flush actual and snapshot clocks atomically
9 tasks
This greatly simplifies state handling. It also prevent any kind of desynchronization because all newest clocks are always persisted atomically.
97ac0e1 to
b67d7dc
Compare
ffuugoo
approved these changes
Dec 16, 2025
agourlay
reviewed
Dec 17, 2025
generall
approved these changes
Dec 17, 2025
f225e02
into
abort-stream-records-breaks-wal-delta
14 checks passed
timvisee
added a commit
that referenced
this pull request
Dec 17, 2025
* Make set_replica_state async * Add function called when active state of local replica changes * Add snapshot for newest clocks * Bump newest clocks snapshot on replica deactivation * Use newest clocks snapshot during recovery * Add enum for specifying whether to take or clear clocks snapshot * Store clock snapshot inside clock map, removing extra file This greatly simplifies state handling. It also prevent any kind of desynchronization because all newest clocks are always persisted atomically. * Immediately persist clocks after taking snapshot * Always update snapshot, only take if missing * Take clock snapshots through each shard flavor, including proxies * Propagate dedicated functions for taking and clearing clocks snapshot * Only persist clocks immediately if changed on snapshot/clear * Simplify recovery point logic, always take clocks snapshot if exists * Remove unwrap
timvisee
added a commit
that referenced
this pull request
Dec 17, 2025
* Add test to reproduce broken WAL delta after aborting stream records * Add staging env var to slow down stream records transfers for test * Tweak test formatting and utilities a bit * Add comment to test, link to PR describing bug * Update test so it still succeeds with patched behavior * Fix broken WAL delta after stream records abort (#7791) * Make set_replica_state async * Add function called when active state of local replica changes * Add snapshot for newest clocks * Bump newest clocks snapshot on replica deactivation * Use newest clocks snapshot during recovery * Add enum for specifying whether to take or clear clocks snapshot * Store clock snapshot inside clock map, removing extra file This greatly simplifies state handling. It also prevent any kind of desynchronization because all newest clocks are always persisted atomically. * Immediately persist clocks after taking snapshot * Always update snapshot, only take if missing * Take clock snapshots through each shard flavor, including proxies * Propagate dedicated functions for taking and clearing clocks snapshot * Only persist clocks immediately if changed on snapshot/clear * Simplify recovery point logic, always take clocks snapshot if exists * Remove unwrap * Fix typo * Fix doc comment * Transfer driver is async, use Tokio sleep * Reduce visibility
timvisee
added a commit
that referenced
this pull request
Dec 18, 2025
* Add test to reproduce broken WAL delta after aborting stream records * Add staging env var to slow down stream records transfers for test * Tweak test formatting and utilities a bit * Add comment to test, link to PR describing bug * Update test so it still succeeds with patched behavior * Fix broken WAL delta after stream records abort (#7791) * Make set_replica_state async * Add function called when active state of local replica changes * Add snapshot for newest clocks * Bump newest clocks snapshot on replica deactivation * Use newest clocks snapshot during recovery * Add enum for specifying whether to take or clear clocks snapshot * Store clock snapshot inside clock map, removing extra file This greatly simplifies state handling. It also prevent any kind of desynchronization because all newest clocks are always persisted atomically. * Immediately persist clocks after taking snapshot * Always update snapshot, only take if missing * Take clock snapshots through each shard flavor, including proxies * Propagate dedicated functions for taking and clearing clocks snapshot * Only persist clocks immediately if changed on snapshot/clear * Simplify recovery point logic, always take clocks snapshot if exists * Remove unwrap * Fix typo * Fix doc comment * Transfer driver is async, use Tokio sleep * Reduce visibility
Merged
1 task
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
4 participants
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.
Fixes #7787
I suggest to review and merge this into #7787, so that we can merge the fix and test as a whole into
dev. See the mentioned PR for more details on the actual bug.This PR fixes the problem by taking a 'snapshot' of last seen clocks when the replica goes in any non-active state. When doing WAL delta recovery on the replica, we derive the recovery point from the snapshot rather than the actual latest clocks.
When the replica becomes active again we're sure it's in good state. Then the clocks snapshot is cleared.
Relevant test:
All Submissions:
devbranch. Did you create your branch fromdev?New Feature Submissions:
cargo +nightly fmt --allcommand prior to submission?cargo clippy --all --all-featurescommand?Changes to Core Features: