Skip to content

Commit 705bbfd

Browse files
committed
Name the parked and canceled resume outcomes on the route metric
ate.router.resume put budget exhaustion, lot rejection, client cancellation and real failures all under unknown. Add parked_exhausted, parked_rejected and canceled; unknown now means a failed resume or a direction that resumes nothing.
1 parent dd03982 commit 705bbfd

10 files changed

Lines changed: 82 additions & 35 deletions

File tree

‎cmd/atenet/internal/router/extproc/handler.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ type Result struct {
6161

6262
// Resume is the actor-resume outcome, as one of the ateattr.RouterResume*
6363
// values. Empty means "unknown" — the direction never resumes an actor, or
64-
// the request never got far enough to learn whether an activation ran.
64+
// the request never got far enough to attempt one.
6565
Resume string
6666

6767
// DynamicMetadata is attached to the ProcessingResponse alongside Response,

‎cmd/atenet/internal/router/extproc/metrics_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ func TestResultResume_DefaultsToUnknown(t *testing.T) {
195195
{name: "none preserved", resume: ateattr.RouterResumeNone, want: ateattr.RouterResumeNone},
196196
{name: "triggered preserved", resume: ateattr.RouterResumeTriggered, want: ateattr.RouterResumeTriggered},
197197
{name: "joined preserved", resume: ateattr.RouterResumeJoined, want: ateattr.RouterResumeJoined},
198+
{name: "parked_exhausted preserved", resume: ateattr.RouterResumeParkedExhausted, want: ateattr.RouterResumeParkedExhausted},
198199
{name: "unknown preserved", resume: ateattr.RouterResumeUnknown, want: ateattr.RouterResumeUnknown},
199200
}
200201
for _, tt := range tests {

‎cmd/atenet/internal/router/ingress/flight.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package ingress
1616

1717
import (
18+
"errors"
1819
"sync/atomic"
1920

2021
"go.ate.dev/substrate/pkg/proto/ateapipb"
@@ -71,17 +72,20 @@ func (f *resumeActorFlight) retryErr() error {
7172
// be called after f.done is closed.
7273
//
7374
// Only a resume that completed without an error says whether an activation
74-
// ran, so the three activation labels are reserved for that case. A failed
75-
// resume reports "unknown": the gRPC code alone does not carry the fact. A
76-
// canceled leader's flight outlives its request and keeps restoring the actor,
77-
// and a DeadlineExceeded can land in the middle of a restore.
75+
// ran, so the three activation labels are reserved for that case. A flight
76+
// that waited out its budget reports "parked_exhausted"; any other failure
77+
// reports "unknown", since the gRPC code alone does not carry the fact.
7878
func (f *resumeActorFlight) callerResult(reqID uint64) (*ateapipb.Actor, ResumeOutcome, error) {
7979
res := f.result
8080
if res == nil {
8181
return nil, ResumeOutcomeUnknown, status.Error(codes.Internal, "resume call returned nil result")
8282
}
8383

8484
if res.err != nil {
85+
var budget *budgetExhaustedError
86+
if errors.As(res.err, &budget) {
87+
return nil, ResumeOutcomeParkedExhausted, res.err
88+
}
8589
return nil, ResumeOutcomeUnknown, res.err
8690
}
8791

‎cmd/atenet/internal/router/ingress/resumer.go‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,13 @@ func (e *budgetExhaustedError) Unwrap() error { return e.lastErr }
7070
type ResumeOutcome string
7171

7272
const (
73-
ResumeOutcomeNone ResumeOutcome = ateattr.RouterResumeNone
74-
ResumeOutcomeTriggered ResumeOutcome = ateattr.RouterResumeTriggered
75-
ResumeOutcomeJoined ResumeOutcome = ateattr.RouterResumeJoined
76-
ResumeOutcomeUnknown ResumeOutcome = ateattr.RouterResumeUnknown
73+
ResumeOutcomeNone ResumeOutcome = ateattr.RouterResumeNone
74+
ResumeOutcomeTriggered ResumeOutcome = ateattr.RouterResumeTriggered
75+
ResumeOutcomeJoined ResumeOutcome = ateattr.RouterResumeJoined
76+
ResumeOutcomeParkedExhausted ResumeOutcome = ateattr.RouterResumeParkedExhausted
77+
ResumeOutcomeParkedRejected ResumeOutcome = ateattr.RouterResumeParkedRejected
78+
ResumeOutcomeCanceled ResumeOutcome = ateattr.RouterResumeCanceled
79+
ResumeOutcomeUnknown ResumeOutcome = ateattr.RouterResumeUnknown
7780
)
7881

7982
type resumeCallResult struct {
@@ -274,10 +277,10 @@ func flightResult(bgCtx context.Context, resumeResp *ateapipb.ResumeActorRespons
274277
func (r *ActorResumer) awaitFlight(ctx context.Context, f *resumeActorFlight, actorRef resources.ActorRef, reqID uint64) (*ateapipb.Actor, ResumeOutcome, error) {
275278
select {
276279
case <-ctx.Done():
277-
// The caller's request context was canceled before the shared resume
280+
// The caller's request context ended before the shared resume
278281
// completed. The flight continues and may still activate the actor, so
279-
// the outcome is "unknown", not "none".
280-
return nil, ResumeOutcomeUnknown, ctx.Err()
282+
// the outcome is "canceled", not "none".
283+
return nil, ResumeOutcomeCanceled, ctx.Err()
281284
case <-f.done:
282285
// Fast path: the flight finished without ever retrying, or before this
283286
// caller saw retrying. The lot is never touched.
@@ -298,15 +301,15 @@ func (r *ActorResumer) awaitFlight(ctx context.Context, f *resumeActorFlight, ac
298301
retryErr := f.retryErr()
299302
release, ok := r.enterLot(ctx, shedOutcome(retryErr))
300303
if !ok {
301-
return nil, ResumeOutcomeUnknown, parkingFullErr(actorRef.String(), retryErr)
304+
return nil, ResumeOutcomeParkedRejected, parkingFullErr(actorRef.String(), retryErr)
302305
}
303306
var finalErr error
304307
defer func() { release(parkOutcomeFor(finalErr)) }()
305308

306309
select {
307310
case <-ctx.Done():
308311
finalErr = ctx.Err()
309-
return nil, ResumeOutcomeUnknown, finalErr
312+
return nil, ResumeOutcomeCanceled, finalErr
310313
case <-f.done:
311314
actor, outcome, err := f.callerResult(reqID)
312315
finalErr = err

‎cmd/atenet/internal/router/ingress/resumer_test.go‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,8 @@ func TestActorResumer_ResumeActor(t *testing.T) {
180180
if !errors.Is(err, context.Canceled) {
181181
t.Errorf("expected context.Canceled, got %v", err)
182182
}
183-
if outcome != ResumeOutcomeUnknown {
184-
t.Errorf("expected outcome %q on a canceled caller, got %q", ResumeOutcomeUnknown, outcome)
183+
if outcome != ResumeOutcomeCanceled {
184+
t.Errorf("expected outcome %q on a canceled caller, got %q", ResumeOutcomeCanceled, outcome)
185185
}
186186
})
187187

@@ -368,7 +368,7 @@ func TestActorResumer_Parking(t *testing.T) {
368368
// Budget large enough for a few ~100ms-spaced retries before it elapses;
369369
// the pool never frees up.
370370
resumer := NewActorResumer(mock, withParking(ParkedRequestConfig{Max: 1, Budget: 1500 * time.Millisecond}))
371-
_, _, err := resumer.ResumeActor(context.Background(), testActorRef)
371+
_, outcome, err := resumer.ResumeActor(context.Background(), testActorRef)
372372
// The client must see the meaningful capacity error, not a generic
373373
// timeout: status.Code must unwrap through the budget-exhaustion marker.
374374
if got := status.Code(err); got != codes.FailedPrecondition {
@@ -378,6 +378,9 @@ func TestActorResumer_Parking(t *testing.T) {
378378
if !errors.As(err, &budget) {
379379
t.Errorf("expected the error to be marked as budget exhaustion, got %T (%v)", err, err)
380380
}
381+
if outcome != ResumeOutcomeParkedExhausted {
382+
t.Errorf("expected outcome %q after park budget elapsed, got %q", ResumeOutcomeParkedExhausted, outcome)
383+
}
381384
mu.Lock()
382385
defer mu.Unlock()
383386
if calls < 2 {
@@ -564,14 +567,17 @@ func TestActorResumer_Parking(t *testing.T) {
564567
}
565568

566569
resumer := NewActorResumer(mock, withParking(ParkedRequestConfig{Max: 1, Budget: budget}))
567-
_, _, err := resumer.ResumeActor(context.Background(), testActorRef)
570+
_, outcome, err := resumer.ResumeActor(context.Background(), testActorRef)
568571
if got := status.Code(err); got != codes.ResourceExhausted {
569572
t.Errorf("expected ResourceExhausted after a late retryable failure, got %v (err=%v)", got, err)
570573
}
571574
var budgetErr *budgetExhaustedError
572575
if !errors.As(err, &budgetErr) {
573576
t.Errorf("expected the error to be marked as budget exhaustion, got %T (%v)", err, err)
574577
}
578+
if outcome != ResumeOutcomeParkedExhausted {
579+
t.Errorf("expected outcome %q after a late retryable failure, got %q", ResumeOutcomeParkedExhausted, outcome)
580+
}
575581
mu.Lock()
576582
defer mu.Unlock()
577583
if calls != 1 {
@@ -836,8 +842,8 @@ func TestActorResumer_LotAdmission(t *testing.T) {
836842
if !errors.As(err, &reqErr) || reqErr.StatusCode != int(envoy_type.StatusCode_ServiceUnavailable) {
837843
t.Fatalf("expected a 503 router-at-capacity denial, got %v", err)
838844
}
839-
if outcome != ResumeOutcomeUnknown {
840-
t.Errorf("shed caller outcome = %q, want %q", outcome, ResumeOutcomeUnknown)
845+
if outcome != ResumeOutcomeParkedRejected {
846+
t.Errorf("shed caller outcome = %q, want %q", outcome, ResumeOutcomeParkedRejected)
841847
}
842848
// The caller was turned away at the transition: exactly one attempt
843849
// had run.
@@ -901,8 +907,8 @@ func TestActorResumer_LotAdmission(t *testing.T) {
901907
if !errors.As(err, &reqErr) || reqErr.StatusCode != int(envoy_type.StatusCode_ServiceUnavailable) {
902908
t.Fatalf("joiner: expected a 503 router-at-capacity denial, got %v", err)
903909
}
904-
if outcome != ResumeOutcomeUnknown {
905-
t.Errorf("joiner outcome = %q, want %q", outcome, ResumeOutcomeUnknown)
910+
if outcome != ResumeOutcomeParkedRejected {
911+
t.Errorf("joiner outcome = %q, want %q", outcome, ResumeOutcomeParkedRejected)
906912
}
907913

908914
close(proceed)

‎docs/metrics/registry/metrics.yaml‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -467,15 +467,33 @@ groups:
467467
and the resume activated the actor. This value is separate
468468
because it is not a correct sample. One cold start with 50
469469
requests is one slow activation and not 51.
470+
- id: parked_exhausted
471+
stability: development
472+
value: parked_exhausted
473+
brief: >
474+
The parking budget ended while the resume was still blocked.
475+
The request failed with the last capacity error. Every request
476+
on that flight has this value. Not an activation time.
477+
- id: parked_rejected
478+
stability: development
479+
value: parked_rejected
480+
brief: >
481+
The parking area was full, thus the router refused the request
482+
at once. atenet.router.parking.rejected counts the same requests.
483+
- id: canceled
484+
stability: development
485+
value: canceled
486+
brief: >
487+
The request ended before the resume gave an answer: the client
488+
disconnected, or its deadline ended (see ate.router.outcome).
489+
The flight continues, thus the actor can still start.
470490
- id: unknown
471491
stability: development
472492
value: unknown
473493
brief: >
474-
The resume did not complete, thus the router cannot tell whether
475-
an activation ran. The resume failed, or the request stopped
476-
before the resume gave an answer, or the direction does not
477-
resume an actor. Do not read the time in this series as an
478-
activation time.
494+
The resume failed for another reason (see ate.router.outcome),
495+
or the direction does not resume an actor. Not an activation
496+
time.
479497
- id: ate.router.outcome
480498
stability: development
481499
brief: The result of the route attempt.
@@ -1366,8 +1384,10 @@ groups:
13661384
ate.router.resume before you read it. The triggered series is the
13671385
activation time. The none series is the usual warm route. A sum across the
13681386
two series puts milliseconds and tens of seconds in one distribution. The
1369-
unknown series holds the resumes that did not complete, thus it is neither
1370-
an activation time nor a warm route.
1387+
parked_exhausted, parked_rejected, canceled and unknown series hold the
1388+
resumes that did not complete, thus they are neither an activation time
1389+
nor a warm route. The count of parked_exhausted is the number of requests
1390+
that capacity, not a fault, turned away.
13711391
annotations:
13721392
substrate:
13731393
emitted_by: [atenet-router]

‎docs/observability.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ For `ate.workerpool.desired_workers` and `ate.workerpool.ready_workers`:
306306
For `atenet.router.route.duration`:
307307
* `ate.router.outcome` categorizes the route attempt result: `ok`, `cancelled`, `timeout`, `no_capacity`, `failed_precondition`, `lock_conflict`, `not_found`, `unavailable`, `rate_limited`, or `resume_error`.
308308
* The router and the scheduler both report `no_capacity` for one condition, but they count different events. The scheduler records one sample on `ate.scheduler.assignment.duration` for each resume. The router records one sample for each request. Many requests can join one resume, and one parked request can cause more than one resume. Thus do not compare the two counts directly.
309-
* `ate.router.resume` indicates the singleflight execution state of actor resumption: `none` (the resume found the actor already running), `triggered` (this request completed a cold activation), `joined` (this request waited on another request's resume, which activated the actor), or `unknown` (the resume did not complete, so whether an activation ran is unknown). `ate.template.atespace` and `ate.template.name` hold `unknown` when the router has no template to name.
309+
* `ate.router.resume` indicates the singleflight execution state of actor resumption: `none` (the resume found the actor already running), `triggered` (this request completed a cold activation), `joined` (this request waited on another request's resume, which activated the actor), `parked_exhausted` (the request waited out the parking budget and failed with the last capacity error), `parked_rejected` (the parking lot was full, so the request was shed at once), `canceled` (the request ended, by a client disconnect or its deadline, before the resume answered), or `unknown` (the resume failed for another reason, so whether an activation ran is unknown). `ate.template.atespace` and `ate.template.name` hold `unknown` when the router has no template to name.
310310

311311
For `ate.imagecache.requests`:
312312
* `ate.imagecache.outcome` is `hit` when the node holds a complete image record — every layer directory the record names is present — and `miss` when the lookup must pull. A failed lookup is neither: `error` is a failed lookup whatever the cause, and `cancelled` or `timeout` is the caller giving up, as on `ate.router.outcome`. So the hit ratio is `hit / (hit + miss)`, with failures and abandoned lookups out of the denominator.

‎docs/request-parking.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,10 @@ bounds the wait.
174174
when its resume failed with `ResourceExhausted`, and `unavailable` otherwise.
175175
The `ate.router.outcome` label of this counter has the same value. Because
176176
`no_capacity` combines fast shed requests with requests that waited out the
177-
parking budget, use `atenet.router.parking.rejected{ate_router_outcome="no_capacity"}`
178-
to count the shed requests in it.
177+
parking budget, read `ate.router.resume` on the route histogram to separate
178+
them: a shed request reports `parked_rejected`, and a request that waited out
179+
the budget reports `parked_exhausted`. A request whose client disconnected or
180+
whose own deadline ended while parked reports `canceled`.
179181

180182
**Status page** (`/statusz`): a "Request Parking" card shows whether parking is
181183
enabled, the current vs. maximum parked count, and the max wait.

‎internal/ateattr/ateattr.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,9 +220,17 @@ const (
220220
// RouterResumeJoined indicates this request waited on another request's
221221
// singleflight resume, which completed a cold activation.
222222
RouterResumeJoined = "joined"
223-
// RouterResumeUnknown indicates the resume did not complete, so the router
224-
// cannot tell whether an activation ran. The resume failed, or the request
225-
// stopped first, or the direction never resumes an actor.
223+
// RouterResumeParkedExhausted: the parking budget elapsed while the resume
224+
// was still blocked, so the request failed with the last capacity error.
225+
RouterResumeParkedExhausted = "parked_exhausted"
226+
// RouterResumeParkedRejected: the parking lot was full, so the request was
227+
// shed at once.
228+
RouterResumeParkedRejected = "parked_rejected"
229+
// RouterResumeCanceled: the request ended (client disconnect or deadline)
230+
// before the shared resume answered. The flight goes on.
231+
RouterResumeCanceled = "canceled"
232+
// RouterResumeUnknown: the resume failed for another reason, or the
233+
// direction never resumes an actor.
226234
RouterResumeUnknown = "unknown"
227235
)
228236

‎internal/ateattr/ateattr_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,9 @@ func TestMetricLabelValues(t *testing.T) {
417417
{RouterResumeNone, "none"},
418418
{RouterResumeTriggered, "triggered"},
419419
{RouterResumeJoined, "joined"},
420+
{RouterResumeParkedExhausted, "parked_exhausted"},
421+
{RouterResumeParkedRejected, "parked_rejected"},
422+
{RouterResumeCanceled, "canceled"},
420423
{RouterResumeUnknown, "unknown"},
421424
}
422425
for _, tt := range tests {

0 commit comments

Comments
 (0)