agent-substrate / agent-substrate/substrate

[P2] Singleflight resume budget is per-flight not per-caller — late joiners experience immediate exhaustion

Ouverte
#613 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
area/network kind/bug prio/P2
Langage dominant
Go
Étoiles
1.8k
Forks
316
Merge moyen
2 j 43 min
PR mergées (30 j)
287

Description

**Severity:** P2 (compounds S12/ISSUE-010; late-joining requests fail without waiting)
**Component:** Network Plane — `cmd/atenet/internal/router/resumer.go`
**Audit ID:** NET-6
**Maps to suspect:** S12 (compound)

---

## Summary

The singleflight in `ActorResumer.ResumeActor` creates a background context
(`bgCtx`) with a fixed timeout when the **first** caller triggers a flight. All
subsequent callers that join the same flight share the **remaining** budget of the
first caller's context. A request joining a flight with 100ms remaining inherits that
100ms and times out almost immediately — despite having just arrived. Under the ISSUE-010
scenario (ateapi crash, 30s lock TTL), this causes every request during the 30s window
to fail quickly, even those that arrive early in each 15s flight cycle.

---

## Root Cause

**File:** `cmd/atenet/internal/router/resumer.go` lines 142–196

```go
func (r *ActorResumer) ResumeActor(ctx context.Context, actorKey ActorKey) error {
_, err, _ := r.sf.Do(actorKey.String(), func() (any, error) {
bgCtx, cancel := context.WithTimeout(context.Background(), r.budget)
defer cancel()
// ... retry loop using bgCtx ...
// bgCtx.Deadline() is set when the FIRST caller arrives
})
return err
}
```

The `singleflight.Do` call shares the result of the **one** executing goroutine across
all concurrent callers. The executing goroutine's `bgCtx` was created when it started.
A second caller arriving at second 14 of a 15-second flight gets at most 1 second of
budget — because `bgCtx` was created 14 seconds ago.

This is documented as an accepted trade-off in a comment at line 151, but the
consequence under the S12 compound scenario is not documented.

---

## Steps to Reproduce

1. Simulate ateapi crash mid-resume (per ISSUE-010): actor locked for 30s.
2. Send a stream of requests to the actor through the router at 1 request/second.
3. Observe request failures grouped by 15s windows:
```
T=0: flight starts, returns budget_exhausted at T=15
T=14: request joins flight → budget remaining = 1s → fails at T=15 (same as T=0)
T=15: new flight starts
T=29: request joins second flight → budget remaining = 1s → fails at T=30
T=30: lock expires, next request succeeds
```
4. All 30 requests in the 30s window fail. Zero requests succeed despite there being
live retry capacity for most of the window.

**Compare to independent budget (expected behavior):**
- T=14 request: gets its own 15s budget, fails at T=29 (lock has expired!) → succeeds.

---

## Expected Behavior

Each caller to `ResumeActor` should experience the full retry budget, regardless of
when the singleflight started. The shared flight should be a de-duplication mechanism,
not a budget-sharing mechanism.

---

## Suggested Fix

Two options:

**Option A (minimal change):** Use a per-caller context as the deadline for the shared
flight, taking the maximum of all active callers' deadlines:

```go
// Store the latest deadline seen; extend bgCtx if a later-arriving caller has a longer one
```
(Complex to implement correctly with singleflight.)

**Option B (cleaner):** Remove singleflight budget sharing. Each `Do` callback gets a
fresh context derived from `context.Background()` with `r.budget`. Because multiple
calls with the same key collapse into one executing goroutine, only the first caller's
budget governs — but extend the context if a subsequent caller arrives with more budget
remaining. Alternatively, switch from `singleflight` to a per-actor in-flight tracker
with an explicit subscription mechanism where each subscriber gets its own deadline.

**Option C (document the behavior):** If the trade-off is accepted, add a prominent
comment explaining that late joiners inherit the remaining budget, and surface this in
the observability dashboard (e.g., a metric for "requests that inherited < 1s of budget").

Guide de contribution

Ouvrir le guide de contribution

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.