Add sleep conditionally (RhBug:1741931) #169
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
j-mracek
force-pushed
the
sleep-revert
branch
from
f1762df
to
0cb0c62
Compare
j-mracek
force-pushed
the
sleep-revert
branch
from
0cb0c62
to
17bc92f
Compare
|
Related bug: https://bugzilla.redhat.com/show_bug.cgi?id=1756400 |
jrohel
reviewed
Oct 8, 2019
| // Iterate over mirror for the target | ||
| for (GSList *elem = target->lrmirrors; elem; elem = g_slist_next(elem)) { | ||
| LrMirror *c_mirror = elem->data; | ||
| gchar *mirrorurl = c_mirror->mirror->url; | ||
|
|
||
| // first iteration, filter out mirrors that failed previously | ||
| if (mirrors_iterated == 0) { | ||
| if (c_mirror->mirror->protocol != LR_PROTOCOL_FILE) |
There was a problem hiding this comment.
So, if one of mirror is not local then reiteration is enabled. But then local paths are retried too, isn't it? You wrote "retry local paths nave no reason" above. I think you forgot add condition code at line 743.
} else if (c_mirror->mirror->protocol == LR_PROTOCOL_FILE) {
continue;
Or am I misunderstand the code?
jrohel
reviewed
Oct 8, 2019
librepo/downloader.c
Outdated
| if (g_str_has_prefix(complete_path_or_baseurl, "file:/")) { | ||
| return FALSE; | ||
| } | ||
| if (download->allowed_mirror_failures > num_of_tried_mirrors) { |
There was a problem hiding this comment.
Next 4 lines do the same as the original one line code.
This is a performance improvement when local repository is not accessible. The sleep step was removed because in case of broken IP from DNS it always tries same IP anyway.
It is required for debugging connection issue and with DNS redirections.
Previously librepo did only one attempt to download the mirror files. And because especially Fedora metalink urls are nowadays by default downloaded from 'mirrors.fedoraproject.org' host - which is just a CNAME to set of DNS-load balanced proxies - it not unlikely that librepo would ask some temporarily broken proxy. Several attempts though will usually mean that librepo will approach several different proxies. In manual dnf-workflow, user would just re-try the whole dnf command manually, but in build systems like Copr [1] this causes severe problems and random build failures. [1] https://pagure.io/fedora-infrastructure/issue/7987
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove sleep step, retries download conditionally, and additional retries for mirrorlist and metalink