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

Fix smart routing algorithm to select closest relay #6871

Merged

Conversation

rablador
Copy link
Contributor

@rablador rablador commented Sep 26, 2024

Currently, smart routing picks a random DAITA relay. It should pick the closest relay using the haversine distance. It should group relays that are the same distance away and use the roulette wheel selection algorithm to pick one between the group of closest relays.

Also includes: update-the-relay-selector-wrapper-to-enable-smart-routing-ios-832


This change is Reviewable

@rablador rablador added the iOS Issues related to iOS label Sep 26, 2024
@rablador rablador self-assigned this Sep 26, 2024
Copy link

linear bot commented Sep 26, 2024

@rablador rablador force-pushed the fix-smart-routing-algorithm-to-select-closest-relay-ios-830 branch 2 times, most recently from f995eee to 4b856d7 Compare September 27, 2024 08:12
@rablador rablador requested review from buggmagnet and mojganii and removed request for buggmagnet September 27, 2024 08:13
@rablador rablador marked this pull request as ready for review September 27, 2024 08:13
@rablador rablador marked this pull request as draft September 27, 2024 08:14
@rablador rablador force-pushed the fix-smart-routing-algorithm-to-select-closest-relay-ios-830 branch 3 times, most recently from 77bdb03 to b60f37d Compare October 2, 2024 14:28
@rablador rablador marked this pull request as ready for review October 2, 2024 14:29
@rablador rablador force-pushed the fix-smart-routing-algorithm-to-select-closest-relay-ios-830 branch 3 times, most recently from ebccf37 to c7e0ff9 Compare October 7, 2024 13:30
buggmagnet
buggmagnet previously approved these changes Oct 7, 2024
Copy link
Contributor

@buggmagnet buggmagnet left a comment

Choose a reason for hiding this comment

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

Reviewed 11 of 12 files at r1, all commit messages.
Reviewable status: 11 of 12 files reviewed, 1 unresolved discussion (waiting on @rablador)


ios/MullvadREST/Relay/RelayPicking.swift line 132 at r1 (raw file):

            )
        } catch let error as NoRelaysSatisfyingConstraintsError where error.reason == .noDaitaRelaysFound {
            // If DAITA and smart routing are enabled and no supported relays are found, we should try to find the nearest

nit
Should we still refer to the feature as "Smart routing" ? 🤔

Copy link
Contributor Author

@rablador rablador left a comment

Choose a reason for hiding this comment

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

Reviewable status: 11 of 12 files reviewed, 1 unresolved discussion (waiting on @buggmagnet)


ios/MullvadREST/Relay/RelayPicking.swift line 132 at r1 (raw file):

Previously, buggmagnet wrote…

nit
Should we still refer to the feature as "Smart routing" ? 🤔

Nope!

@rablador rablador force-pushed the fix-smart-routing-algorithm-to-select-closest-relay-ios-830 branch from c7e0ff9 to 3ce1b39 Compare October 8, 2024 08:29
Copy link
Contributor

@buggmagnet buggmagnet left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion

@buggmagnet buggmagnet force-pushed the fix-smart-routing-algorithm-to-select-closest-relay-ios-830 branch from 3ce1b39 to a7b8c35 Compare October 8, 2024 09:14
@buggmagnet buggmagnet merged commit a6edf5c into main Oct 8, 2024
10 of 11 checks passed
@buggmagnet buggmagnet deleted the fix-smart-routing-algorithm-to-select-closest-relay-ios-830 branch October 8, 2024 09:22
Copy link

github-actions bot commented Oct 8, 2024

🚨 End to end tests failed. Please check the failed workflow run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iOS Issues related to iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants