Repository navigation
router: report a shed request as no_capacity after a full fleet - #2138
Merged
Krisztian F (krisztianfekete) merged 3 commits intoOct 7, 2026
Conversation
Krisztian F (krisztianfekete)
left a comment
Krisztian F (krisztianfekete)
left a comment
Collaborator
There was a problem hiding this comment.
Thanks, added some minor comments!
Jeff Luo (JeffLuoo)
force-pushed
the
fix/issue-2127-shed-request-no-capacity
branch
2 times, most recently
from
October 6, 2026 17:01
689c954 to
3dce99a
Compare
Jeff Luo (JeffLuoo)
added
area/observability
kind/bug
Krisztian F (krisztianfekete)
previously approved these changes
Oct 7, 2026
Krisztian F (krisztianfekete)
left a comment
Krisztian F (krisztianfekete)
left a comment
Collaborator
There was a problem hiding this comment.
Thanks, added a nit comment + a follow-up.
| full. On `atenet.router.route.duration`, a shed request reports `no_capacity` | ||
| when its resume failed with `ResourceExhausted`, and `unavailable` otherwise. | ||
| Because `no_capacity` combines fast shed requests with requests that waited | ||
| out the parking budget, use `atenet.router.parking.rejected` to count shed |
Collaborator
There was a problem hiding this comment.
Adding ate.router.outcome to parking.rejected would let users split shed requests by cause and keep fast sheds out of the no_capacity latency. We can do this as a follow-up though.
Collaborator
Author
There was a problem hiding this comment.
Done in this PR. parking.rejected now has an ate.router.outcome label with the values no_capacity and unavailable. One helper, shedOutcome, picks the value for the label and for the error, so the label always agrees with the outcome.
Collaborator
Author
There was a problem hiding this comment.
If you prefer a follow-up, I can revert and create an issue to track it.
The router sheds a request from a full parking lot only while the resume flight of its actor retries. The most frequent retry cause is ResourceExhausted, which means that no worker had room. The shed request did not keep that failure, so atenet.router.route.duration reported it as unavailable. The metric showed less capacity pressure than the actual value when the pressure was highest. A parked request whose budget elapsed under the same condition already reported no_capacity, so the two paths did not agree. The flight now records its latest retryable failure. When that failure is ResourceExhausted, the shed request keeps it as the cause, and classifyOutcome reports no_capacity. The router drops other retry failures, because a shed request is not a lock conflict or a failed precondition. Those shed requests continue to report unavailable. The response body to the client does not change. This changes what existing queries on atenet.router.route.duration with ate_router_outcome="no_capacity" and "unavailable" return.
A request that the router sheds from a full parking lot reports no_capacity or unavailable on atenet.router.route.duration. The parking.rejected counter had no labels, so it could not split shed requests by cause. Now it carries the same ate.router.outcome value as the route metric. A single helper, shedOutcome, picks that value for the counter and for the error, so the two always agree. signalRetrying now takes the retry error and stores it before it closes retrying. A caller that sees retrying closed reads that error to pick its outcome. Before, the order depended on two separate calls at the call site, and no test caught a swap.
The flight stores its latest retry error for shed callers, and it does not clear it. flightResult reads a separate local copy that the retry loop clears on a non-retryable error. A NotFound that arrives after the budget elapses then reaches the client as NotFound, not as the earlier capacity error.
Jeff Luo (JeffLuoo)
force-pushed
the
fix/issue-2127-shed-request-no-capacity
branch
from
October 7, 2026 14:39
1453df7 to
9345a27
Compare
Krisztian F (krisztianfekete)
approved these changes
Oct 7, 2026
Collaborator
Author
|
Can you help add the change to merge queue? Thanks! Krisztian F (@krisztianfekete) |
2 tasks done
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.
Fixes #2127.
The router reported a request that it shed from a full parking lot as
unavailable, also when the resume failed withResourceExhausted. Now such a request reportsno_capacityonatenet.router.route.duration. A shed request with a different retry cause still reportsunavailable. The response body to the client does not change.A parked request whose budget elapsed under
ResourceExhaustedalready reportedno_capacity. This change makes a shed request agree with it.Breaking change
On
atenet.router.route.duration, requests shed from a full parking lot whileResumeActorretries onResourceExhaustednow reportate_router_outcome="no_capacity"instead of"unavailable". Useatenet.router.parking.rejectedto count shed requests on their own.Testing:
go test -race ./cmd/atenet/internal/router/... ./internal/ateattr/...passes.TestHandleRequestHeaders_FullLotShedsParkedRequestfails without the fix (status.Code(err) = Unknown, want ResourceExhausted) and passes with it.Tests pass
Appropriate changes to documentation are included in the PR