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

Use /mirrors/status/json/ instead of /mirrorlist/ to get mirror lists #2599

Merged
merged 17 commits into from
Aug 2, 2024

Conversation

Torxed
Copy link
Member

@Torxed Torxed commented Jul 30, 2024

PR Description:

This is more of a performance tweak, as the /json/ endpoint is cached in the backend and machine readable, whereas the ASCII human version of mirror status is not.

  • Improves mirror-list grab latency
  • Introduces pydantic as a dependency
  • Creates a MirrorStatus model (with latency and speed performance checks)
  • Emulates reflector.service behavior after mirror selection (Sort order: score -> download speed)
  • Fixes mirror selection overwriting mirrorlist, instead of appending it after mirrors (making them never being used essentially)
  • Implemented a dependency-free ping() function. We're currently not using mirror.latency but there's a possibility we could in the future if we wanted to.

Future improvements could be that we could use pacman-mirrorlist package as source, convert that to JSON that we could consume, solving potential issues such as #2598.

Tests and Checks

  • I have tested the code!

@svartkanin
Copy link
Collaborator

This is great, wgen doing the migration to the new menu I was wondering why the mirror menu takes so long to open

…the mirrors 'score' rather than just the URL name. This will emulate the reflector.service/rankmirrors behavior and thus reducing the need to re-rank the mirrors.
@Torxed
Copy link
Member Author

Torxed commented Jul 30, 2024

Yeah it's a bit faster, and also we get more useful data from this.
I have one more commit before I'm satisfied, I thought I was done but now realized I have to filter a bit more.

I'm currently sorting the mirrors based on score which is effectively how healthy they are. But in my final commit I'll sort on multiple values, and by default it will be:
speed -> score. The mirror-list should then be as fast as possible, as well as the most healthy it can be :)

Edit: Also need to fix the flake8/mypy obviously heh

@Torxed Torxed marked this pull request as draft July 30, 2024 12:47
@Torxed
Copy link
Member Author

Torxed commented Jul 30, 2024

I will have to tinker a bit.
Because there's no way for the backend to know what speed the users source of origin will have.

So we have to adopt:

  • Do a quick rankmirrors ping check to get early indications
  • Then do what reflector does with bandwidth testing after we select mirrors to not bandwidth test every mirror possible, only the ones the user needs/wants. (Need to figure out where/how to trigger 'okay, I've now selected a region/mirrors.. now perform the bandwidth test')

This should give us a speedy UI since ping is pretty fast.
While giving the best installation time by performing bandwidth test against the specific selected mirror-list.

And if the user opt-out and just use what reflector.service already created on bootup, we'll do nothing of course.

… sorted(mirror_list, key=lambda mirror: (mirror.score, mirror.speed)) - but I have not implemented the sorting via the menu yet, and I have not integrated the new MirrorStatus model into the handling of URL's. I still need to figure out where the {region: [url, url]} is being used, so that i can convert to {region: [mirror.url, mirror.url]} logic.
@Torxed
Copy link
Member Author

Torxed commented Jul 31, 2024

So I have a method of sorting now, I just need to trigger it after region(s) have been selected to avoid:
screenshot

@Torxed
Copy link
Member Author

Torxed commented Jul 31, 2024

Can't recall if there was a purpose behind doing a rather than w on the /etc/pacman.d/mirrorlist.
But currently when setting mirrors through the menu - they get appended after the default ISO mirrorlist - making the selection a bit redundant.

So I made a small change to #2350 which is a got replaced by w when saving the mirrorlist.
I left the /etc/pacman.conf as a still, as it probably only adds custom mirrors? which should go at the end hehe.

@Torxed Torxed marked this pull request as ready for review August 1, 2024 08:20
@Torxed Torxed requested a review from grazzolini as a code owner August 1, 2024 08:20
@Torxed
Copy link
Member Author

Torxed commented Aug 1, 2024

I think I'm satisfied, comments would be welcome.

screenshot

The reason why the fastest mirror isn't at the top, is because score (sync reliability) is preferred over speed, followed by speed sorting order. Which is also why I used round() on the score, as score is a float. So making score a rounded int it's more likely that mirrors are grouped with the same score thus making the speed factor come into play.

ai

@property
def speed(self):
if self._speed is None:
info(f"Checking download speed of {self._hostname}[{self.score}] by fetching: {self.url}core/os/x86_64/core.db")
Copy link
Collaborator

Choose a reason for hiding this comment

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

info won't be shown to the user, so maybe just use debug

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking if the speed checks are slow, it's nice that they get printed during installation with info():
screenshot

It's shown between selecting the region - and returning to the menu, but just briefly.
But it does disrupt the look and feel of the menu system for sure, so we want to skip it?

@Torxed Torxed merged commit 98518e4 into archlinux:master Aug 2, 2024
6 checks passed
@Torxed Torxed deleted the use-json-mirrorlist branch August 2, 2024 13:24
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.

2 participants