-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Move exponential backoff to DNS resolver from resolver.ClientConn #4270
Merged
Merged
Changes from all commits
Commits
Show all changes
58 commits
Select commit
Hold shift + click to select a range
dcab165
Save progress
zasweq 8754401
Save progress
zasweq 6bb242d
Clean up
zasweq 8299dad
Clean up
zasweq da102eb
vet
zasweq fbda038
vet
zasweq 0ebd685
Added logic to tests
zasweq 7cb488e
Added unit test for exponential backoff
zasweq ba01815
Vet
zasweq 15ec449
Added another unit test
zasweq f0a6aed
Changed test
zasweq 72f5538
Fixed failing tests
zasweq 3bc67ff
Vet
zasweq b31f41f
Deleted old tests
zasweq 8bc20c1
Vet
zasweq d20f77a
Fixed data race
zasweq 9fe80c1
Vet
zasweq 6b1401c
Fixed timeout
zasweq 797bf3e
Fixed race
zasweq e5fa7fc
Fixed race
zasweq a0a24b5
Rewrote failing unit test
zasweq f39b7b2
Vet
zasweq fb44fe1
Vet
zasweq 0721699
Fixed race conditions in tests
zasweq 5574d9a
Changed time
zasweq 8572acb
Responded to Doug's comments
zasweq 54d1fd2
Responded to Doug's comments
zasweq 33695df
vet
zasweq 100ba1b
vet
zasweq 8646061
vet
zasweq e75a12b
Vet
zasweq f54ecb4
Vet
zasweq 2ecb953
Vet
zasweq 513a51d
Deleted callback in tests
zasweq c61426f
Added polling to wait group
zasweq c3d8c01
Responded to Doug's comments
zasweq 32801ec
Vet
zasweq 090069d
Cleaned up comments
zasweq ac3e18e
Responded to Doug's comments
zasweq 653a564
Responded to Doug's comments
zasweq f51aa71
Vet
zasweq 625a9e2
Save progress before cleanup
zasweq e384bab
Clean up
zasweq d8ffa44
Vet
zasweq e923d61
Vet
zasweq 0960018
More cleanup
zasweq 98ed270
Save Progress
zasweq e8f3fff
Changed logic
zasweq 96f2bc0
Incorporated Doug's idea
zasweq e325361
Responded to Doug's comments
zasweq e90b3a4
Responded to Doug's comments
zasweq d9e7620
Responded to Doug's comments
zasweq fc31662
Vet
zasweq 7c26bb3
Responded to Doug's comments
zasweq e40bbaa
Vet
zasweq 40b52ef
Responded to Doug's comments
zasweq 6d02b03
Vet
zasweq e9eed86
Switched send to blocking
zasweq File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
IMO this should use
newTimer
, too, or else we can't test all the timing aspects that we should be testing.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.
newTimer is extremely coupled to like 15 tests. The success path (with MinDNSResRate) is already tested within the unit tests. Ex: https://github.com/zasweq/grpc-go/blob/move-backoff-to-dns-resolver/internal/resolver/dns/dns_resolver_test.go#L1046.