-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[qs] batch store bootstrap perf improvements #15491
Conversation
⏱️ 1h 41m total CI duration on this PR
🚨 1 job on the last run was significantly faster/slower than expected
|
3290472
to
ab67fd5
Compare
Self::gc_previous_epoch_batches_from_db(db_clone, epoch); | ||
}); | ||
} else { | ||
Self::populate_cache_and_gc_expired_batches( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -157,7 +157,7 @@ impl BatchStore { | |||
last_certified_time | |||
); | |||
for (digest, value) in db_content { | |||
let expiration = value.expiration(); | |||
let expiration = value.expiration().saturating_sub(expiration_buffer_usecs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ Forge suite
|
✅ Forge suite
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ Forge suite
|
* [qs] Use expiration buffer to cleanup during bootstrap * [qs] async gc old epoch batches from batch store * [qs] monitor! create batch store
* [qs] Use expiration buffer to cleanup during bootstrap * [qs] async gc old epoch batches from batch store * [qs] monitor! create batch store
Description
Fix a bug from [consensus] sync improvements to help slow nodes sync better #15364 where the batches were not cleanup respecting the expiration buffer during bootstrap.
With [consensus] sync improvements to help slow nodes sync better #15364, doubling cache duration in batch store means bootstrap time during epoch changes is now doubled because all the batches now need to be read from storage. To avoid this, this PR introduces a separate path for epoch changes vs restarts. During epoch change, the old batches from storage are deleted asynchronously. At restarts, the batches are still cleaned up synchronously.