reset max query time of blocking queries in client after retries #25039
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 a blocking query on the client hits a retryable error, we change the max query time so that it falls within the `RPCHoldTimeout` timeout. But when the retry succeeds we don't reset it to the original value. Because the calls to `Node.GetClientAllocs` reuse the same request struct instead of reallocating it, any retry will cause the agent to poll at a faster frequency until the agent restarts. No other current RPC on the client has this behavior, but we'll fix this in the `rpc` method rather than in the caller so that any future users of the `rpc` method don't have to remember this detail. Fixes: #25033
tgross
force-pushed
the
client-rpc-retry-reset-block-time
branch
from
2701924 to
6d9d27d
Compare
tgross
added
backport/ent/1.7.x+ent
schmichael
reviewed
Feb 6, 2025
schmichael
approved these changes
Feb 6, 2025
| @@ -191,3 +192,57 @@ func Test_resolveServer(t *testing.T) { | |||
| } | |||
|
|
|||
| } | |||
|
|
|||
| func TestRpc_RetryBlockTime(t *testing.T) { | |||
There was a problem hiding this comment.
The test took 10x longer to write than the fix, unfortunately. Not having any way of controlling the behavior of the lower layers of the RPC "stack" we have is probably why we have fairly poor test coverage of the error handling paths. 😿
6 tasks
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 a blocking query on the client hits a retryable error, we change the max query time so that it falls within the
RPCHoldTimeouttimeout. But when the retry succeeds we don't reset it to the original value.Because the calls to
Node.GetClientAllocsreuse the same request struct instead of reallocating it, any retry will cause the agent to poll at a faster frequency until the agent restarts. No other RPC on the client currently has this behavior, but we'll fix this in therpcmethod rather than in the caller so that any future users of therpcmethod don't have to remember this detail.Fixes: #25033
Ref: https://hashicorp.atlassian.net/browse/NET-12116
Contributor Checklist
changelog entry using the
make clcommand.ensure regressions will be caught.
and job configuration, please update the Nomad website documentation to reflect this. Refer to
the website README for docs guidelines. Please also consider whether the
change requires notes within the upgrade guide.
Reviewer Checklist
backporting document.
in the majority of situations. The main exceptions are long-lived feature branches or merges where
history should be preserved.
within the public repository.