Repository navigation
Conversation
AdaptiveTimeoutPolicy kept an endpoint tightened as long as requests timed out, including timeouts at the tightened budget itself. A healthy endpoint whose heavy queries legitimately need longer than tightenedMs could then only recover when some cheap query happened to succeed, losing every heavy stage in between. A timeout at the tightened budget no longer counts toward tightening; instead the next request runs at defaultMs. The endpoint relaxes when that probe succeeds and stays tightened when it times out as well. BREAKING CHANGE: AfterRequestContext has a required timeoutMs property: the budget beforeRequest returned for the attempt. Code that calls TimeoutPolicy.afterRequest itself must pass it.
This branch has not been deployed
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
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.
Once
AdaptiveTimeoutPolicytightened an endpoint, it stayed tightened for as long as requests kept timing out, even when they were timing out at the tightened budget. On a healthy endpoint whose heavy queries take longer thantightenedMs, every heavy stage then failed until some cheap query happened to succeed. In the run in the issue, that took out ten stages.The rule now: a timeout at the tightened budget doesn’t count toward tightening, because it says nothing about whether the endpoint is healthy. The policy re-probes instead, giving the next request on that endpoint
defaultMs. If the probe succeeds, the endpoint relaxes. If it times out as well, the endpoint stays tightened, and the next timeout at the tightened budget schedules the next probe. Only one probe is handed out at a time; concurrent requests keep the tightened budget.What this costs and buys:
triples.rqwould still time out at 10 s, butclass-partition.rqwould get the full 5 minutes and relax the endpoint.defaultMsnow waits out the full budget on every other request rather than never again. That makes it slower to fast-fail than before, but it still takes about half the time it would without the policy.To know which budget a timed-out request ran under,
afterRequestnow receivestimeoutMs, the budgetbeforeRequestreturned for that attempt. With requests in flight at the same time, the policy can’t reliably work that out from its own state or fromdurationMs(an upstream 504 can come back fast). That makestimeoutMsa required field ofAfterRequestContext, hence the!.The reference and the how-to guide describe the new rule.
Fixes #513