agent-substrate / agent-substrate/substrate

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

Đang mở
#613 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
area/network kind/bug prio/P2
Ngôn ngữ chính
Go
Star
1.8k
Fork
316
Merge trung bình
2 ngày 43 phút
Pull request đã merge (30 ngày)
287

Mô tả

**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").

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.