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

Disable multicurl by default #480

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dirkmueller
Copy link
Member

Multicurl is not quite an improvement anymore. It picks up chunks based on the filesize that make it connect to 4-5 mirrors for each file. The thing is that the connection overhead on high latency mirrors is so high that there isn't a way to get decent performance, and given the really bad connection reusage we are always in tcp slow start.

Multicurl is not quite an improvement anymore. It picks up chunks
based on the filesize that make it connect to 4-5 mirrors for each file.
The thing is that the connection overhead on high latency mirrors
is so high that there isn't a way to get decent performance, and given
the really bad connection reusage we are always in tcp slow start.
@bzeller bzeller requested a review from mlandres August 17, 2023 08:24
@bzeller
Copy link
Contributor

bzeller commented Aug 17, 2023

Question to this: Does that also mean we do not need to care anymore about zsync downloads or mirrors we get from the servers? In that case we might also remove that from the new media backend making the code easier to maintain.

Michael is on vacation for a week, lets also see what he has to say about it

@dirkmueller dirkmueller marked this pull request as draft August 19, 2023 07:33
@dirkmueller
Copy link
Member Author

@bzeller I think zsync would still be beneficial. I can btw disable the multicurl also server side, so we can first run some experiments globally.

@bzeller
Copy link
Contributor

bzeller commented Aug 21, 2023

@dirkmueller Without the MultiCurl backend or the new one in development there is no zsync though. The MediaCurl backend does not support it, only MultiCurl and MediaNetwork do.

But if the server does not respond with a metalink file then the MultiCurl code is not triggered anyway and the file is downloaded directly.

@bzeller
Copy link
Contributor

bzeller commented Aug 25, 2023

@dirkmueller so we just had a discussion about this in our meeting, our suggestion would be:

  • We disable metalink server side, this will effectively disable MultiCurl because the initial request is made via MediaCurl and if it does not yield a MetaLink file the download is done.

  • For now that means no zsync and no metalink but we can teach MultiCurl zsync so it would only download in chunks for files that return a zsync file. ( I thought MultiCurl can already do zsync but seems I never added it there )

  • I will remove all the overhead from MediaNetwork that downloads chunks over mirrors and make it use MultiPart HTTP Requests instead. This means it will leverage zsync to figure out the chunks it needs but downloads them from ONE server, if that one fails it will try the next in the mirror list if it got one. Doing this will drop lots of extra steps we do in that code and make it faster. This code will then also be used by the upcoming async zypper.

Thoughts?

@dirkmueller
Copy link
Member Author

@dirkmueller so we just had a discussion about this in our meeting, our suggestion would be:

  • We disable metalink server side, this will effectively disable MultiCurl because the initial request is made via MediaCurl and if it does not yield a MetaLink file the download is done.

Yes, that seems to be a more flexible approach to quicker evaluate the impact. I'll see what I can do about that.

  • For now that means no zsync and no metalink but we can teach MultiCurl zsync so it would only download in chunks for files that return a zsync file. ( I thought MultiCurl can already do zsync but seems I never added it there )

do you mean mediacurl here rather than multicurl? there are reports btw that zsync is not as good as believed together with cdn (still needs a deeper investigation, see openSUSE/openSUSE-repos#27 (comment) for details

  • I will remove all the overhead from MediaNetwork that downloads chunks over mirrors and make it use MultiPart HTTP Requests instead. This means it will leverage zsync to figure out the chunks it needs but downloads them from ONE server, if that one fails it will try the next in the mirror list if it got one. Doing this will drop lots of extra steps we do in that code and make it faster. This code will then also be used by the upcoming async zypper.

I think that's a great idea. lets do that, then we can default to medianetwork eventually.

@bzeller
Copy link
Contributor

bzeller commented Sep 4, 2023

  • For now that means no zsync and no metalink but we can teach MultiCurl zsync so it would only download in chunks for files that return a zsync file. ( I thought MultiCurl can already do zsync but seems I never added it there )

do you mean mediacurl here rather than multicurl? there are reports btw that zsync is not as good as believed together with cdn (still needs a deeper investigation, see openSUSE/openSUSE-repos#27 (comment) for details

No, really only MultiCurl could do this. MediaCurl is just a very thin layer around libcurl, it can't do much more than simply download a plain file. Complex things like multipart requests is not possible there.

  • I will remove all the overhead from MediaNetwork that downloads chunks over mirrors and make it use MultiPart HTTP Requests instead. This means it will leverage zsync to figure out the chunks it needs but downloads them from ONE server, if that one fails it will try the next in the mirror list if it got one. Doing this will drop lots of extra steps we do in that code and make it faster. This code will then also be used by the upcoming async zypper.

I think that's a great idea. lets do that, then we can default to medianetwork eventually.

Great , lets do that 👍

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.

None yet

2 participants