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

feat: implement retry mechanism in image copy operation #12693

Closed

Conversation

Azhovan
Copy link

@Azhovan Azhovan commented Jan 2, 2024

This PR addresses this issue by refactoring the image copy logic with retry mechanism. following changes are made in this PR:

  • Introduced retry strategies in image copy operation.
  • Implemented limit and backoff with jitter strategies for retry attempts.

@Azhovan
Copy link
Author

Azhovan commented Jan 3, 2024

@tomponline @simondeziel
what do you think about this PR?

@MusicDin
Copy link
Member

Thanks for your contribution. We will take a look at this ASAP

@Azhovan
Copy link
Author

Azhovan commented Jan 11, 2024

Thanks for your contribution. We will take a look at this ASAP

@MusicDin

I would like to highlight that this implementation does not resume the download from where it left off in the event of a failed attempt to download images from the server. This could lead to increased network traffic, as each new attempt starts from the beginning.

Implementing a feature to resume the previous operation would involve maintaining state and additional modifications to the current setup on both http client level and createImage() level

all that being said, if we are not fine with this compromise, we may go for a new solution

@simondeziel
Copy link
Member

Thanks for working on this, this will paper over some image server hiccups like this failing after multiple minutes of downloading:

$ lxc launch ubuntu-daily:24.04 vn1 --vm
Creating vn1
Error: Failed instance creation: unexpected EOF

Comment on lines +634 to +635
// It Limits the retries to the number of URLs and spaces out with an increasing delay.
// Retries is starting from 10 milliseconds and a jitter (random deviation) is added to the delay to prevent synchronized retries.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// It Limits the retries to the number of URLs and spaces out with an increasing delay.
// Retries is starting from 10 milliseconds and a jitter (random deviation) is added to the delay to prevent synchronized retries.
// It limits the retries to the number of URLs and spaces out with an exponentially increasing delay.
// Retries are starting from 10 milliseconds and a jitter (random deviation) is added to the delay to prevent synchronized retries.

@Azhovan Azhovan closed this Aug 27, 2024
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.

3 participants