Add cancellable context to Fetch requests #31
Merged
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
Fetch requests that are Consul blocking queries leak on shutdown and hold onto the TCP connection causing Consul server to hold until reaped. This can quickly result in reaching Consul client connection limits and causing subsequent client restarts to error due to agent rate limits, 429 and EOFs.
eikenb
reviewed
Dec 9, 2020
eikenb
reviewed
Dec 9, 2020
eikenb
reviewed
Dec 9, 2020
eikenb
reviewed
Dec 9, 2020
eikenb
approved these changes
Dec 9, 2020
lornasong
reviewed
Dec 9, 2020
|
Thanks for the review everyone! I do want to note that context is currently used for Consul dependencies, but are not piped through to the Vault dependencies yet. I'm not as familiar with those and for timing purposes, I focused on Consul. |
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.
Fetch requests that are Consul blocking queries leak on shutdown and hold onto the TCP connection causing Consul server to also hold until reaped. This can quickly result in reaching Consul client connection limits and causing subsequent client restarts to error due to agent rate limits, 429 and EOFs.
The PR adds a cancellable context to the QueryOptions for the Consul API client to use for HTTP requests. This allows
view.Stop()to cancel the context which cancels the in-flight request. Canceling the request releases the underlying TCP connection from active to now possible idle state, which could then be cleaned up whenClientSet.Stop()->http.Client.CloseIdleConnections()is called.