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.

Sign up for GitHub

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

Jump to bottom

fix/optimize/refactor(udp): fix potential stuck UDP and optimize reroute logic #204

Merged
merged 10 commits into from
Jul 13, 2023
Merged

fix/optimize/refactor(udp): fix potential stuck UDP and optimize reroute logic #204

merged 10 commits into from
Jul 13, 2023

Conversation

mzz2017
Copy link
Contributor

@mzz2017 mzz2017 commented Jul 10, 2023 •
edited

Background

Fix

The stuck UDP dialing would block all other UDP dialing. This causes UDP to fail to work. This PR fixes it.

Range:

  1. This problem impacts all previous versions.
  2. This problem only impacts UDP except DNS.

Optimize

domain++ may reroute every UDP packet, which may consume many resources and result in unexpected results.

This PR optimizes it. If a UDP connection has already been established, we shouldn't reroute it (find another outbound/dialer) while we should reuse the dialer to write a request.

Changelogs

  • Force to give 8s timeout for TCP/UDP dialers.
  • Use fine-grained lock for UDP.
  • Only re-route for the new UDP connection.

Checklist

@mzz2017 mzz2017 changed the title optimize(udp): not reroute if connection established fix/optimize(udp): fix potential stuck UDP and optimize reroute logic Jul 10, 2023
@dae-prow

This comment was marked as resolved.

Sign in to view
@dae-prow

This comment was marked as resolved.

Sign in to view
@dae-prow

This comment was marked as resolved.

Sign in to view
@dae-prow

This comment was marked as resolved.

Sign in to view
@mzz2017 mzz2017 changed the title fix/optimize(udp): fix potential stuck UDP and optimize reroute logic fix/optimize/refactor(udp): fix potential stuck UDP and optimize reroute logic Jul 12, 2023
miooochi
miooochi approved these changes Jul 13, 2023
View reviewed changes
Copy link
Contributor

@miooochi miooochi left a comment

Choose a reason for hiding this comment

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

This PR has been tested, working fine in my environment.

dae-prow[bot]
dae-prow bot approved these changes Jul 13, 2023
View reviewed changes
Copy link
Contributor

@dae-prow dae-prow bot left a comment

Choose a reason for hiding this comment

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

🧪 Since the PR has been fully tested, please consider merging it.

@miooochi miooochi requested a review from a team July 13, 2023 03:17
@mzz2017 mzz2017 merged commit acfc1db into main Jul 13, 2023
16 checks passed
@mzz2017 mzz2017 deleted the optimize_udp_reroute branch July 13, 2023 11:04
@mzz2017 mzz2017 mentioned this pull request Jul 13, 2023
Merged
@miooochi miooochi mentioned this pull request Jul 17, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miooochi miooochi miooochi approved these changes

@dae-prow dae-prow[bot] dae-prow approved these changes

Assignees

@mzz2017 mzz2017

Labels
fix optimize priority/high tested
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mzz2017 @miooochi