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

Create a progress bar for the plan validation #217

Merged
merged 3 commits into from
Apr 8, 2022

Conversation

RyuzakiKK
Copy link
Contributor

Validating a plan could take a non trivial amount of time, depending on
the size of a plan and by the I/O speed.

With this commit we add a progress bar that shows the status of the
validation and prefixes the assembling progress bar with "Assembling ".


I added just the validation and not the actual "planning" because, from my testings, the planning is usually done nearly instantaneously and the progress percentage would just go from 0% to 100%.

@RyuzakiKK RyuzakiKK marked this pull request as draft March 30, 2022 15:27
@RyuzakiKK RyuzakiKK marked this pull request as ready for review March 30, 2022 15:37
@RyuzakiKK
Copy link
Contributor Author

RyuzakiKK commented Mar 30, 2022

Example output when extracting an index file:

ValidatingPlan1    0.00%
ValidatingPlan1    4.43% 00m10s
ValidatingPlan1   10.65% 00m08s
ValidatingPlan1   18.52% 00m06s
ValidatingPlan1   23.07% 00m06s
ValidatingPlan1   31.06% 00m05s
ValidatingPlan1   35.51% 00m05s
ValidatingPlan1   42.22% 00m04s
ValidatingPlan1   44.63% 00m04s
ValidatingPlan1   50.05% 00m04s
ValidatingPlan1   53.87% 00m04s
ValidatingPlan1   59.34% 00m03s
ValidatingPlan1   66.79% 00m02s
ValidatingPlan1   70.55% 00m02s
ValidatingPlan1   88.07%
ValidatingPlan1  100.00% 7s
Assembling    0.00%
Assembling    5.22% 00m09s
Assembling   17.73% 00m04s
[...]

EDIT: In the previous commit I used "Validating", but given that this operation could be repeated if we have an invalid seed, I switched to "ValidatingPlan{number}" to include the attempt number.

Copy link
Owner

@folbricht folbricht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you. If you want there's an opportunity to improve my old code here. If NewProgressBar() returned something like NullProgressBar that implements the ProgressBar interface instead of nil in the non-terminal case, you could get rid of all the nil checks for it thoughout the code (the if pb != nil bit)

progressbar.go Outdated Show resolved Hide resolved
@RyuzakiKK
Copy link
Contributor Author

Sure, I should be able to do that tomorrow.

ProgressBar is not a command and should be placed in the project root
directory.

This will allow us to create new progress bars from functions that are
not located in `cmd/desync`.

Signed-off-by: Ludovico de Nittis <ludovico.denittis@collabora.com>
Signed-off-by: Ludovico de Nittis <ludovico.denittis@collabora.com>
Validating a plan could take a non trivial amount of time, depending on
the size of a plan and by the I/O speed.

With this commit we add a progress bar that shows the status of the
validation and prefixes the assembling progress bar with "Assembling ".

Signed-off-by: Ludovico de Nittis <ludovico.denittis@collabora.com>
@RyuzakiKK
Copy link
Contributor Author

With the latest revision I added the suggested NullProgressBar, and the progressbar for when we chunk an invalid seed.

Copy link
Owner

@folbricht folbricht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you

@folbricht folbricht merged commit f914eb9 into folbricht:master Apr 8, 2022
@folbricht
Copy link
Owner

Hmm, looks like the MacOS build breaks due to a lib issue, likely need some built-tags.

@folbricht
Copy link
Owner

Nope, unrelated build issue in Go 1.18. Fixed in #218

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants