Skip to content
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

Incorrect progress bar when importing compressed snapshots #3005

Closed
lemmih opened this issue Jun 19, 2023 · 8 comments · Fixed by #3022
Closed

Incorrect progress bar when importing compressed snapshots #3005

lemmih opened this issue Jun 19, 2023 · 8 comments · Fixed by #3022
Assignees
Labels
Bug Ready Issue is ready for work and anyone can freely assign it to themselves

Comments

@lemmih
Copy link
Contributor

lemmih commented Jun 19, 2023

Describe the bug

To Reproduce Steps to reproduce the behavior:

  1. Remove any mainnet data: forest-cli db clean
  2. Import a snapshot: forest --encrypt-keystore false --halt-after-import --auto-download-snapshot
  3. Notice how forest appears to freeze after the progress bar hits 100%.

(Note, the same can be observed using a calibnet snapshot, but the effect is less noticeable.)

Expected behaviour

The progress bar is configured such that 100% is the size of the compressed snapshot but, when running, it counts the decompressed bytes. Since our compression factor is roughly 2x, when the progress bar hits 100%, only about half of the compressed snapshot has been consumed.

The progress bar should show how much of the snapshot file we've consumed.

Environment (please complete the following information):

  • OS: Linux
  • Rust version(e.g. rustc --version): rustc 1.70.0 (90c541806 2023-05-31)
  • Branch/commit: main/3e06cd98709ff2ef6ec11d9e943482acc686edf8

Other information and links

@lemmih lemmih added Bug Ready Issue is ready for work and anyone can freely assign it to themselves labels Jun 19, 2023
@aatifsyed
Copy link
Contributor

Probably introduced in #2893

@aatifsyed
Copy link
Contributor

David, am I correct in remembering you wanted to remove progress bars altogether?

@ruseinov ruseinov self-assigned this Jun 20, 2023
@ruseinov
Copy link
Contributor

@lemmih Am I correct to assume that we actually want to decompress it first and then show progress based on decompressed size?

@lemmih
Copy link
Contributor Author

lemmih commented Jun 20, 2023

Nope, that would take up a lot of space. We want to process the data immediately as it is being uncompressed. The progress bar can display how much of the compressed file we've consumed.

@lemmih
Copy link
Contributor Author

lemmih commented Jun 20, 2023

David, am I correct in remembering you wanted to remove progress bars altogether?

They should be changed to use the logging framework instead of printing terminal codes. Let's fix this bug first.

@ruseinov
Copy link
Contributor

2. forest --encrypt-keystore false --halt-after-import --auto-download-snapshot

I did not ask the question I meant to ask, the question here is: how do we come by the uncompressed file size? I'll figure this out.

@ruseinov
Copy link
Contributor

Looks like the best way is to parse zstd frame header in hopes that it stored the original file size, which is optional.

@ruseinov
Copy link
Contributor

Yeah, unfortunately this is not re-exported by async-compression, but we can add a zstd-safe dependency manually as it won't grow the bin size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Ready Issue is ready for work and anyone can freely assign it to themselves
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants