add proxy.config.http.cache.max_open_write_retry_timeout parameter #8591
Conversation
This file contains hidden or 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
traeak
requested review from
SolidWallOfCode,
bryancall and
zwoop
as code owners
traeak
force-pushed
the
ts_write_retry
branch
4 times, most recently
from
6f7b816 to
d97b866
Compare
Member
|
[approve ci rocky] |
Contributor
|
@SolidWallOfCode is going to try to take a look at this |
Contributor
|
@cmcfarlen is going to take a look at this |
cmcfarlen
requested changes
Mar 31, 2022
Contributor
cmcfarlen
left a comment
cmcfarlen
left a comment
There was a problem hiding this comment.
Check the logic on the read retry assert.
cmcfarlen
approved these changes
Apr 18, 2022
Contributor
cmcfarlen
left a comment
cmcfarlen
left a comment
There was a problem hiding this comment.
lgtm after removing the backward logic.
Contributor
Author
|
We've been running with this patch in prod but disabled for a bit now. The last squash had some changes from when I ran tests using docker containers, i removed a small optimization that missed evaluating an open_write increment. |
bryancall
approved these changes
Apr 19, 2022
Contributor
bryancall
left a comment
bryancall
left a comment
There was a problem hiding this comment.
Based on @cmcfarlen review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
The current cache write lock retries work by spinning N times at ms intervals, defined by
proxy.config.http.cache.max_open_write_retries(default 1) andproxy.config.http.cache.open_read_retry_time(default 10ms).While performing synthetic load testing with an origin that still accepts connections but stalls, we encountered larger than expected transaction ttms. This was caused by the retry time space going far beyond the configured 10ms.
Using the following setting I was getting too much variation in time spent spinning on the write lock. Anywhere from 1.5s to 8s.
So I added this millisecond timeout/deadline:
When set this timeout is used in preference to the sloppy open_write_retries * open_read_retry_time