Stops ABS in wen_restart when generating a snapshot #4896
+195
−112
Conversation
This file contains 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
9dea61e to
3967cd3
Compare
|
@wen-coding: requesting your review as an expert on the wen_restart code |
wen-coding
reviewed
Feb 10, 2025
| let mut last_snapshot_end_time = None; | ||
|
|
||
| loop { | ||
| if exit.load(Ordering::Relaxed) || stop.load(Ordering::Relaxed) { |
There was a problem hiding this comment.
This looks pretty good to me. My only question is: if we stop in the middle, would this leave anything broken on disk so that we can't really generate snapshot later?
There was a problem hiding this comment.
Nothing is stopped in the middle, so we're safe.
wen-coding
approved these changes
Feb 10, 2025
|
@wen-coding Does this need to be backported to v2.2? |
Nah, I think we can just wait for the normal release cycle. |
HaoranYi
reviewed
Feb 11, 2025
HaoranYi
reviewed
Feb 11, 2025
| /// Query and manage the status of AccountsBackgroundService | ||
| #[derive(Debug, Clone)] | ||
| pub struct AbsStatus { | ||
| /// Flag to query if ABS is running |
There was a problem hiding this comment.
nit: we could optimized this into just one AtomicU8.
0 - running
1 - stopping
2 - stopped.
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.
Problem
When wen_restart generates a snapshot, AccountsBackgroundService is still running. The accounts-db background tasks (flush/clean/shrink) cannot run concurrent with each other, but since ABS is still running (and generating a snapshot calls flush & clean), this invariant is broken. We observe asserts on larger tests indicating as such.
Summary of Changes
Stop ABS before wen-restart generates a snapshot.
Note to reviewers: I recommend ignoring whitespace in the diff view.