search

canonical / multipass

Multipass orchestrates virtual Ubuntu instances
https://multipass.run
GNU General Public License v3.0
7.76k stars 642 forks source link

Retry on download failure #3487

Closed georgeliao closed 4 months ago

georgeliao commented 5 months ago

fix #3464

This PR adds binary exponential backoff based retry mechanism to async periodic download tasks, the checking exception (download failure an exception) in the main thread is done in a non-blocking fashion via QFutureWatcher.

  1. Changed std::async + std::future pair to QtConcurrent::run + QFuture in order to use some QFuture specific features.
  2. Added QFutureWatcher to watch QFuture and react correspondingly in the successful and failed cases when QFuture is finished.
  3. Changed DownloadException to a QException-based class so it can cross the qt thread boundary.
  4. Made the CRTP BaseQException class to reuse the boilerplate code for all the QException derived classes.

To test the behavior, you can modify the default delay time in daemon.h line 222 to a small number, so the download will run more frequently. After that, the wifi network can be manually turned off in ubuntu settings, which in a way causes download failure.

codecov[bot] commented 5 months ago

Codecov Report

Attention: Patch coverage is 85.00000% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 88.83%. Comparing base (2a16af2) to head (42d1612).

Files Patch % Lines
include/multipass/async_periodic_download_task.h 85.18% 4 Missing :warning:
include/multipass/exceptions/base_qexception.h 71.42% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3487 +/- ## ========================================== + Coverage 88.82% 88.83% +0.01% ========================================== Files 254 253 -1 Lines 14115 14119 +4 ========================================== + Hits 12537 12542 +5 + Misses 1578 1577 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sharder996 commented 5 months ago

Hey @georgeliao, thanks for the work on this!

Would you be able to edit the PR description to link the github issues that this fixes in the format 'Fixes #xxxx'? That way I can get some more context on what behavior this is fixing.

Thanks!

georgeliao commented 5 months ago

@sharder996 , sorry that I forgot it, the link is added now. I also added more text in the intro to explain the context. Be free to ask any questions if something is not clear.