-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Simplify DNS throttle implementation #9454
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #9454 +/- ##
=======================================
Coverage 98.58% 98.59%
=======================================
Files 107 105 -2
Lines 35026 35083 +57
Branches 4151 4176 +25
=======================================
+ Hits 34531 34589 +58
+ Misses 330 329 -1
Partials 165 165
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
nevermind , bad test on my part |
|
works fine in production. Lot more coverage now as well |
Backport to 3.10: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply b20908e on top of patchback/backports/3.10/b20908e531ff349545cff5e90de24975bc27afda/pr-9454 Backporting merged PR #9454 into master
🤖 @patchback |
Backport to 3.11: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply b20908e on top of patchback/backports/3.11/b20908e531ff349545cff5e90de24975bc27afda/pr-9454 Backporting merged PR #9454 into master
🤖 @patchback |
(cherry picked from commit b20908e)
(cherry picked from commit b20908e)
What do these changes do?
Simplify DNS throttle implementation
When testing #9447 I found there were so many race opportunities that it made sense to refactor the code to reduce the chance for races and make it quite a bit simpler.
This should also improve the performance a bit when we have a thundering herd backing up waiting for the resolver as they no longer need to create tasks and we only need a single task for the resolution that is happening to be able to shield it from cancellation.
Are there changes in behavior for the user?
no
Is it a substantial burden for the maintainers to support this?
no
likely fixes #3431