-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat: refactor generation of snapshots from the cli #5464
Conversation
joshieDo
commented
Nov 16, 2023
- Refactors the generation of snapshots from the cli (dedup some code)
- Add parallelism if desired
- Add some stats that get printed at the end of the generation
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.
smol nit,
pending @shekhirin
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, only nits
/// statistics about various aspects of each snapshot, such as filters size, | ||
/// offset index size, offset list size, and loading time. | ||
fn stats(&self, snapshots: Vec<impl AsRef<Path>>) -> eyre::Result<()> { | ||
let mb = 1024.0 * 1024.0; |
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.
nit: const
avg_duration.as_millis() as f64, | ||
avg_duration.as_micros() as f64 |
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.
can we just use the Debug
impl for Duration
here?