-
Notifications
You must be signed in to change notification settings - Fork 28
Introduce endpoint provider preference #1609
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
base: main
Are you sure you want to change the base?
Conversation
.unreleased/LLT-6880
Outdated
| @@ -0,0 +1 @@ | |||
| LLT-6880: Introduce endpoint provider preference order. Prefer Local endpoints more than others. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will result in line like
- LLT-6880: LLT-6880: Introduce....
generate_changelog.py takes the ticket from the filename (_match_ticket(filename)) and always prepends it when entry_ticket_id is present:
it uses ENTRY_FORMAT_W_TICKET = "* {}: {}\n" and calls ENTRY_FORMAT_W_TICKET.format(entry_ticket_id, line.strip())
it does not strip a leading ticket from the file content, so if line.strip() already begins with LLT-6880:, the output becomes * LLT-6880: LLT-6880: Introduce...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point!
Fixed.
Whenever there exists multiple paths between two devices to connect within meshnet - we were choosing _any_ valid path. But those paths are not always equal, therefore it was entirely possible that unpreferable path would be chosen (e.g. going via STUN via public IP and hairpining on router instead of LAN IPs). This PR introduces the following preference of endpoint providers: * Most prefered - local * Medium prefered - UPnP * Least prefered - STUN and whenever an upgrade is required - this preference is reflected in the endpoint choice. Note: There is a limitation, that if one endpoint is published significnalty earlier than "more" prefered one - then there preference will only be evaluated after next path transition from relay to direct.
c023b33 to
9b746ab
Compare
sfraczek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Hasan6979
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than that. +1
| // Define Endpoint-Provider preference | ||
| // higher == more preferred | ||
| let ep_preference_score = |ep| match ep { | ||
| ApiEndpointProvider::Local => 4, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 4 instead of 3 ?
Problem
Whenever there exists multiple paths between two devices to connect within meshnet - we were choosing any valid path. But those paths are not always equal, therefore it was entirely possible that unpreferable path would be chosen (e.g. going via STUN via public IP and hairpining on router instead of LAN IPs).
Solution
This PR introduces the following preference of endpoint providers:
And this order is evaluated everytime there is a transition from relayed path to direct.
Note: There is a limitation, that if one endpoint is published significnalty earlier than "more" prefered one - then there preference will only be evaluated after next path transition from relay to direct.
☑️ Definition of Done checklist