Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

client: Less noisy progress #102

Merged
merged 1 commit into from
Jul 25, 2022
Merged

client: Less noisy progress #102

merged 1 commit into from
Jul 25, 2022

Conversation

nirs
Copy link
Member

@nirs nirs commented Jul 20, 2022

Previously we updated the progress up to 10 times per second, and we
displayed fractional percent values (e.g. 12.57%). This is too noisy and
creates too many uninteresting updates. It also calls
time.monotonic_time() on every update to check if it is time to update
which is waste of resources for very little benefit.

Change the progress to show integer values (12%) and update the progress
only when the progress value changes. With this change we update the
progress up to 100 times during a transfer.

This PR implements part of #72.

@nirs nirs requested review from bennyz, rokkbert and aesteve-rh July 20, 2022 17:45
@nirs nirs added this to the ovirt-4.5.2 milestone Jul 20, 2022
aesteve-rh
aesteve-rh previously approved these changes Jul 21, 2022
Copy link
Member

@aesteve-rh aesteve-rh left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link

@rokkbert rokkbert left a comment

Choose a reason for hiding this comment

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

I think it's better like this and it looks correct to me, except for some typos and an outdated comment.
But the purpose of line.ljust(self.width, " ") is not clear to me. It's not changed in this PR though, so doesn't matter maybe.

aesteve-rh
aesteve-rh previously approved these changes Jul 25, 2022
@nirs nirs marked this pull request as draft July 25, 2022 09:38
@nirs
Copy link
Member Author

nirs commented Jul 25, 2022

Example output when downloading real image:

$ ./ovirt-img download-disk -c engine adb9b2c7-32a9-4e87-894c-710de7165086 \
    /data/fc/tmp/dl.qcow2 | tr '\r' '\n'
[ ---- ] 0 bytes, 0.00 seconds, 0 bytes/s                                      
[   0% ] 960.00 KiB, 0.06 seconds, 17.01 MiB/s                                 
[   2% ] 159.12 MiB, 0.06 seconds, 2.81 GiB/s                                  
[   6% ] 415.00 MiB, 0.06 seconds, 7.32 GiB/s                                  
[  10% ] 663.38 MiB, 0.06 seconds, 11.69 GiB/s                                 
[  14% ] 914.75 MiB, 0.06 seconds, 16.11 GiB/s                                 
[  15% ] 932.06 MiB, 0.06 seconds, 16.39 GiB/s                                 
[  30% ] 1.81 GiB, 0.06 seconds, 28.14 GiB/s                                   
[  31% ] 1.86 GiB, 0.21 seconds, 8.91 GiB/s                                    
[  44% ] 2.68 GiB, 0.25 seconds, 10.84 GiB/s                                   
[  45% ] 2.74 GiB, 0.42 seconds, 6.54 GiB/s                                    
[  46% ] 2.81 GiB, 0.59 seconds, 4.74 GiB/s                                    
[  64% ] 3.88 GiB, 0.61 seconds, 6.34 GiB/s                                    
[  65% ] 3.95 GiB, 0.68 seconds, 5.83 GiB/s                                    
[  66% ] 3.96 GiB, 0.70 seconds, 5.64 GiB/s                                    
[  67% ] 4.03 GiB, 0.72 seconds, 5.59 GiB/s                                    
[  68% ] 4.11 GiB, 0.77 seconds, 5.35 GiB/s                                    
[  83% ] 5.03 GiB, 0.85 seconds, 5.96 GiB/s                                    
[  84% ] 5.05 GiB, 0.87 seconds, 5.79 GiB/s                                    
[  86% ] 5.17 GiB, 0.88 seconds, 5.85 GiB/s                                    
[  87% ] 5.25 GiB, 1.20 seconds, 4.38 GiB/s                                    
[  88% ] 5.29 GiB, 1.20 seconds, 4.40 GiB/s                                    
[  89% ] 5.37 GiB, 1.47 seconds, 3.65 GiB/s                                    
[  91% ] 5.50 GiB, 1.64 seconds, 3.36 GiB/s                                    
[  92% ] 5.57 GiB, 1.73 seconds, 3.23 GiB/s                                    
[  93% ] 5.62 GiB, 1.73 seconds, 3.25 GiB/s                                    
[  94% ] 5.69 GiB, 1.83 seconds, 3.10 GiB/s                                    
[  95% ] 5.70 GiB, 1.99 seconds, 2.86 GiB/s                                    
[  96% ] 5.80 GiB, 2.25 seconds, 2.58 GiB/s                                    
[  97% ] 5.83 GiB, 2.35 seconds, 2.49 GiB/s                                    
[  99% ] 5.94 GiB, 2.39 seconds, 2.49 GiB/s                                    
[ 100% ] 6.00 GiB, 2.42 seconds, 2.48 GiB/s                                    
[ 100% ] 6.00 GiB, 2.45 seconds, 2.45 GiB/s                                    

Previously we updated the progress up to 10 times per second, and we
displayed fractional percent values (e.g. 12.57%). This is too noisy and
creates too many uninteresting updates. It also calls
time.monotonic_time() on every update to check if it is time to update
which is waste of resources for very little benefit.

Change the progress to show integer values (12%) and update the progress
only when the progress value changes. With this change we update the
progress up to 100 times during a transfer.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
@nirs nirs marked this pull request as ready for review July 25, 2022 10:56
@nirs nirs merged commit ed29f76 into oVirt:master Jul 25, 2022
@nirs nirs deleted the progress branch July 25, 2022 10:56
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants