vtgateproxy: add conn warmup time #465
Merged
+29
−9
Conversation
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
When there are a lot more targets than the number of connections in the pool, then it's possible that if the list of hosts changes, the builder might pick a totally new set of hosts than the previously selected ones, none of which will have established subconns. Instead of giving this new list to the picker immediately, first combine it with the list of hosts that were previously selected, so that those subconns have some time to warm up while the current set is still in the list.
henryr
reviewed
Jul 24, 2024
| // If we've already selected some targets, give the new addresses some time to warm up before removing | ||
| // the old ones from the list | ||
| if r.currentAddrs != nil && warmupTime.Seconds() > 0 { | ||
| combined := append(r.currentAddrs, addrs...) |
There was a problem hiding this comment.
Might be a good idea to de-duplicate and skip if there's nothing new in addrs.
There was a problem hiding this comment.
Sure we could. I thought about that in the original implementation too but there's no real difference since that's basically what UpdateState is going to do under the covers.
There was a problem hiding this comment.
(Ignoring the sleep of course, but I think that's ok since we shouldn't have no-op updates to the file).
dedelala
pushed a commit
that referenced
this pull request
Jul 30, 2024
When there are a lot more targets than the number of connections in the pool, then it's possible that if the list of hosts changes, the builder might pick a totally new set of hosts than the previously selected ones, none of which will have established subconns. Instead of giving this new list to the picker immediately, first combine it with the list of hosts that were previously selected, so that those subconns have some time to warm up while the current set is still in the list.
dedelala
pushed a commit
that referenced
this pull request
Jul 30, 2024
When there are a lot more targets than the number of connections in the pool, then it's possible that if the list of hosts changes, the builder might pick a totally new set of hosts than the previously selected ones, none of which will have established subconns. Instead of giving this new list to the picker immediately, first combine it with the list of hosts that were previously selected, so that those subconns have some time to warm up while the current set is still in the list.
dedelala
pushed a commit
that referenced
this pull request
Sep 9, 2024
When there are a lot more targets than the number of connections in the pool, then it's possible that if the list of hosts changes, the builder might pick a totally new set of hosts than the previously selected ones, none of which will have established subconns. Instead of giving this new list to the picker immediately, first combine it with the list of hosts that were previously selected, so that those subconns have some time to warm up while the current set is still in the list.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When there are a lot more targets than the number of connections in the pool, then it's possible that if the list of hosts changes, the builder might pick a totally new set of hosts than the previously selected ones, none of which will have established subconns.
Instead of giving this new list to the picker immediately, first combine it with the list of hosts that were previously selected, so that those subconns have some time to warm up while the current set is still in the list.