fix(trace): disable stale recovery loop until last_span_at lands

Stale recovery sweeps traces by `start_time < NOW() - threshold`, which
measures trace age rather than inactivity. Any threshold low enough to
be useful (2-10 min) kills legitimate long-running agent runs: research
chains, large code generation, extended shell commands routinely exceed
10 minutes.

Disabled in Start() — function kept in place for easy re-enable once a
`last_span_at` column is added so recovery can gate on "no activity for
N minutes" instead of "started > N min ago".

Trade-off: zombie traces from gateway crashes may remain `running` in
DB. Accepted: primary abort path (router 2-phase + trace.status WS
event) handles the common case; safety-net gap preferred over false
kills of healthy runs.

Integration test RecoverStaleNow() still works (manual trigger, not
loop-dependent) so coverage of the recovery function itself is
preserved for when it's re-enabled.
This commit is contained in:
viettranx
2026-04-14 19:53:11 +07:00
parent 1ac08155b0
commit b68b3b12d7
2 changed files with 17 additions and 4 deletions
+1 -1
View File
@@ -39,7 +39,7 @@ During cancellation:
Trace finalization persists independently with a detached context (`context.WithoutCancel`), 5-second timeout, and exponential backoff retry (3 tries, max 10 total via retry queue). This ensures trace status writes don't fail silently due to cancellation.
Stale recovery worker runs every 30 seconds and catches zombie traces (stuck in "running" state) older than 10 minutes. Threshold is conservative because it measures against `start_time`, not last activity — a tighter value would kill healthy long-running agent runs. (Follow-up: add `last_span_at` column to enable aggressive recovery of inactive runs.) Context values (traceID, collector) survive cancellation — only `ctx.Done()` and `ctx.Err()` change.
Stale recovery is **currently disabled**. The implementation exists but the background loop is not started in `Collector.Start()`. Reason: the sweep condition is `start_time < NOW() - threshold`, which measures trace age rather than inactivity. Any threshold low enough to be useful (210 min) would kill healthy long-running agent runs (research chains, large code generation, extended shell commands). Re-enable only after adding a `last_span_at` column so recovery can gate on "no activity for N minutes" instead of "started > N min ago". Until then, zombie traces from gateway crashes may remain `running` in the DB — an accepted safety-net gap traded against false kills. The primary abort path (router 2-phase abort + `trace.status` WS event) handles the common case. Context values (traceID, collector) survive cancellation — only `ctx.Done()` and `ctx.Err()` change.
---
+16 -3
View File
@@ -146,15 +146,28 @@ func (c *Collector) SetStatusBroadcaster(b StatusBroadcaster) {
c.broadcastStatus = b
}
// Start begins the background flush loop, retry worker, and stale recovery loop.
// Start begins the background flush loop and retry worker.
//
// NOTE: staleRecoveryLoop is intentionally NOT started. The current implementation
// sweeps traces by `start_time`, which would kill legitimate long-running agent
// runs (research chains, large code generation, long shell commands routinely
// exceed 10 minutes). Re-enable only after adding a `last_span_at` column so
// recovery can gate on "no activity for N minutes" instead of "started > N min
// ago". Until then, crashed/orphaned traces may remain `running` in DB — the
// primary abort path (router 2-phase + trace.status WS event) handles the
// common case; this is a safety-net gap we accept over false kills.
func (c *Collector) Start() {
c.wg.Add(3) // flushLoop + retryWorker + staleRecoveryLoop
c.wg.Add(2) // flushLoop + retryWorker (staleRecoveryLoop disabled — see note above)
go c.flushLoop()
go c.retryWorker()
go c.staleRecoveryLoop()
// go c.staleRecoveryLoop() // disabled: would kill healthy long runs. See Start() godoc.
slog.Info("tracing collector started")
}
// keep staleRecoveryLoop reachable to silence "unused" linter; re-enabled in
// Start() once last_span_at-based recovery lands.
var _ = (*Collector).staleRecoveryLoop
// Stop gracefully shuts down the collector, flushing remaining spans.
func (c *Collector) Stop() {
close(c.stopCh)