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

Throttle network path updates in packet tunnel #5511

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

rablador
Copy link
Contributor

@rablador rablador commented Nov 28, 2023

We should throttle the updates we receive from monitoring the default path via KVO. Since flopping back and forth can be detrimental and causes the tunnel to thrash, the changes should be throttled. Finding the optimal parameters and throttling strategy is subject to discussion.

Current implementation removes duplicate updates and throttles the remaining for 2 seconds.


This change is Reviewable

Copy link

linear bot commented Nov 28, 2023

IOS-392 Throttle path updates in packet tunnel.

We should throttle the updates we receive from monitoring the default path via KVO. Since flopping back and forth can be detrimental and causes the tunnel to thrash, the changes should be throttled. Finding the optimal parameters and throttling strategy is subject to discussion.

Emīls: My current hunch is that we should add at least a 2 second delay to any and all path changes.

@rablador rablador force-pushed the throttle-path-updates-in-packet-tunnel-ios-392 branch 3 times, most recently from 7969c77 to 9f9a68f Compare November 28, 2023 10:56
@rablador rablador added the iOS Issues related to iOS label Nov 28, 2023
Copy link
Contributor

@pronebird pronebird 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 2 of 2 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

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:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@rablador rablador force-pushed the throttle-path-updates-in-packet-tunnel-ios-392 branch from 9f9a68f to e6a35ed Compare November 28, 2023 14:50
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 1 of 2 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@buggmagnet buggmagnet force-pushed the throttle-path-updates-in-packet-tunnel-ios-392 branch from e6a35ed to a2fc8fc Compare November 28, 2023 15:01
@buggmagnet buggmagnet merged commit 1a33d42 into main Nov 28, 2023
5 checks passed
@buggmagnet buggmagnet deleted the throttle-path-updates-in-packet-tunnel-ios-392 branch November 28, 2023 15:04
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.

3 participants