Repository navigation
bug: backend kill between slot ZADD and finally-block ZREM leaks Redis slots; recovery doesn't sweep #749
Description
Summary
When the backend process is killed between capacity.acquire() (Redis ZADD on agent:slots:{name}) and the finally-block capacity.release() (ZREM) inside task_execution_service.execute_task, the slot is leaked. Startup recovery (recover_orphaned_executions) is asymmetric — it scans SQL for running rows and patches SQL, but never scans Redis for slots that have no matching SQL row. The leaked slot only goes away on the next acquire that hits _cleanup_stale_slots_for_agent (1200s TTL).
Context
Same locally-observed incident as the companion bug — full timeline and forensic artifacts in that issue. The relevant excerpt for this bug:
20:39:00.534 capacity.acquire() ZADDs slot for execution RxbyJHq9Kx9QLfwwr7fcsQ
20:39:00.542 task_execution_service starts agent /api/task call
20:39:04.88 Backend restarts (uvicorn reload). In-flight handler killed.
finally: capacity.release() ZREM never runs.
20:39:06.17 Third backend restart. recover_orphaned_executions scans SQL,
finds no running rows for this execution (it's already failed),
does nothing. Slot stays in Redis with no SQL counterpart.
The canary harness (Issue #411) S-01 invariant detects exactly this — Redis-only slot members. It fired three consecutive cycles on this leak (rows #26/#27/#28 in canary_violations).
In dev mode, uvicorn auto-reloads on every code change; in prod, restarts are infrequent and graceful but the bug class is the same — any kill between ZADD and ZREM leaks.
Acceptance Criteria
- Startup recovery (
recover_orphaned_executions) scansagent:slots:*and ZREMs any member whose execution_id is either (a) terminal in SQL or (b) has no SQL row at all. - Reproduction: SIGKILL backend mid-execute (after ZADD, before ZREM); restart; assert Redis slot is gone after recovery completes.
- No regression: in-flight executions that did survive a graceful restart are not falsely cleaned (covered by symmetry with the SQL recovery's grace check).
- Canary S-01 stays clean for ≥1 full cycle after a forced restart on a stack with active scheduled load.
Technical Notes
The fix lives in services/cleanup_service.py:_reconcile_orphaned_executions (called by recover_orphaned_executions at startup) and the shared _recover_execution helper at cleanup_service.py:792. Currently only one direction is reconciled:
SQL says running, no slot in Redis → mark SQL failed
SQL terminal/missing, slot in Redis → ★ NOT HANDLED ★
Add the second case. Implementation sketch:
- SCAN
{prefix}*(the same logic the canary snapshot collector already runs incanary/snapshot.py:_collect_redis_slot_state). - For each (agent_name, execution_id), check SQL: if status is terminal or row is missing, ZREM the slot member and DELETE its metadata key.
- Apply the same age grace as the SQL side (skip slots whose ZSET score is within the last N seconds — those may be in-flight on another worker once we have multi-instance backends).
Companion startup-recovery race bug: see #748.
Files:
src/backend/services/cleanup_service.py:367-470(watchdog reconcile, currently SQL→Redis only)src/backend/services/cleanup_service.py:724-805(recover_orphaned_executionsstartup hook)src/backend/services/slot_service.py:283-310(_cleanup_stale_slots_for_agent, the only current Redis-only cleaner; runs at acquire time)src/backend/canary/snapshot.py:_collect_redis_slot_state(already has the SCAN logic to mirror)
Activity
dolho commentedon May 12, 2026
PR #814 ships the Redis-side sweep. Mirror of #748's SQL-side fix; same forensic incident, opposite direction.
_reconcile_orphaned_slots()SCANsagent:slots:*, applies a 15s grace window (symmetric with bug: startup execution recovery races with concurrent in-flight /internal/execute_task calls #748's SQL grace), ZREMs members whose SQL row is terminal or missing.drain-*) skipped; metadata keys deleted alongside the ZSET member.recover_orphaned_executions(); runs even when SQL has zero running rows (the textbook 'killed-mid-execute' bug shape).test_no_running_executionsassertion so future result-dict additions don't break it.Acceptance criterion #4 (canary S-01 stays clean after forced restart on staging) still needs live verification.
fix(startup): sweep Redis for orphan slots whose SQL row is terminal …
merge(dev): resolve conflict with #749 Redis orphan-slot sweep
fix(tests): resolve merge conflict markers in orphaned execution reco…
obasilakis commentedon May 18, 2026
Closing per acceptance criterion #4 — verified on dev.
6 days post-#814 merge (2026-05-12 → 2026-05-18):
CL21BYbc6ERvzNEb1JPIfwoncanary-fleet-burst)Pre-#814 the same leak class produced repeated multi-hour stuck slots (the forensic trail in the original ticket). Post-#814: one ≤5-min blip in 6 days. The Redis-side recovery sweep is doing its job.