feat(durable): add durable execution engine for v2 - #264
mpangrazzi wants to merge 22 commits into
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Manual pre-GA failure and recovery canariesThese checks were run against a real Hayhooks server and Redis service, outside the unit/integration suites. Hayhooks process killed while durable work was active
Result: both executions survived abrupt host-process loss and remained inspectable/reconnectable without resubmission. Redis stopped during an active execution
Result: Redis loss degraded health without losing persisted work, and both execution processing and health recovered when Redis returned. SSE terminal/chunk race under concurrent workers
Result: the canary found a real release-blocking stream race, and the repeated run verified the fix under concurrency. Concurrent submission contention follow-up
Result: the final contention run accepted and completed 80/80 executions across repeated concurrent bursts. Cooperative cancellation on the real runtime
Result: cancellation propagated through the running workload and committed the expected durable terminal state. Durable A2A execution was intentionally not included in these canaries because it is outside the v2 scope of this stack. |
ArzelaAscoIi
left a comment
There was a problem hiding this comment.
mighty PR! 👀
Looks good! I went through it with claude for me to digest it better 😅 I think all the comments are rather race conditions/things we should tackle later. Lets go!
| lease_owner=None, | ||
| lease_expires_at_ms=None, | ||
| ) | ||
| return TransitionPlan(next_control, lease_index_update=LeaseIndexUpdate(None, control.fence)) |
There was a problem hiding this comment.
This is the one owned-outcome branch that doesn't check cancel_requested_at_ms. If a worker releases while cancellation is pending, this re-queues instead of finalizing to CANCELED — the run can sit reporting non-terminal state indefinitely. Should this mirror Suspend/ScheduleRetry and terminal-cancel here?
| ), | ||
| ) | ||
| except ExecutionNotFoundError: | ||
| await self.redis.zrem(self.keys.lease_expiry, member) |
There was a problem hiding this comment.
This only catches ExecutionNotFoundError. A lease renewed between the zrange snapshot and this call raises InvalidExecutionTransitionError (same case claim(candidate=True) already handles a bit further down), and that will abort the rest of this maintenance batch — leaving every other already-due lease unprocessed. Worth catching it here too (and in MemoryExecutionStore.maintain).
| ), | ||
| ) | ||
| except ExecutionNotFoundError: | ||
| self._lease_expiry.pop((run_id, fence), None) |
There was a problem hiding this comment.
Same gap as the Redis store's maintain(): only ExecutionNotFoundError is caught. claim() above (line 260) already catches (ExecutionNotFoundError, InvalidExecutionTransitionError) for the equivalent race — this loop should too, otherwise one raced lease recovery kills the whole maintenance batch.
| lease = plan.lease_index_update | ||
| if lease is None or lease.deadline_ms is None: | ||
| raise AssertionError("heartbeat must renew a lease") | ||
| pipe.hset(control_key, "lease_expires_at_ms", lease.deadline_ms) |
There was a problem hiding this comment.
Heartbeat writes only lease_expires_at_ms here instead of going through _apply_plan, so the rest of plan.next_control (version, updated_at_ms, etc.) that decide() computed is never persisted. The caller gets a plan claiming those fields changed, but a subsequent read() won't reflect it — and it diverges from MemoryExecutionStore, which applies the full plan for every command. Intentional perf shortcut, or should this call _apply_plan?
| ) -> dict[str, Any]: | ||
| """Run the synchronous Pipeline without letting its thread block shutdown.""" | ||
| context._require_owned() | ||
| result, _ = start_daemon_thread( |
There was a problem hiding this comment.
thread_done from start_daemon_thread is dropped (_) here — this nested Pipeline thread isn't registered in _draining_runs the way the top-level runner thread is. On cancellation/shutdown it keeps running unobserved, and a later retry can race a genuine concurrent duplicate execution. Should this track/drain the thread the same way the top-level runner does?
| log.bind(pipeline_name=prepared.name, exception_type=type(error).__name__).error( | ||
| "Failed to restore pipeline registry publication" | ||
| ) | ||
| old_published = False |
There was a problem hiding this comment.
Rollback failures here (and at line 1012) are swallowed to a log line, then the original error is re-raised as-is. The caller only learns the deploy failed, not that the previous pipeline registration/routes may not have been fully restored (old_published = False). Given the PR description's claim that failed publication "restores files, routes, registry state, and the previous durable deployment independently," should a partial-restore failure surface differently — e.g. a distinct error or a flag callers can check — instead of looking identical to a clean rollback?
Stack
mainWhat
Adds opt-in durable execution for Haystack 3.1 Pipelines and Agents. Executions are detached from the submit request and can survive process loss when backed by Redis.
Features
/status, operational documentation, examples, and real-Redis CI coverageScope
This PR deliberately does not add durable A2A task execution or recovery. Existing A2A execution remains request-bound. Durable dashboard observability is isolated in stacked PR #265.
Reviewer guide
This is a large foundational change. A useful review order is:
src/hayhooks/durable/engine.py,models.py, andstore.py: lifecycle, public projection, and persistence contract.src/hayhooks/durable/redis.py: atomic Redis implementation, indexes, fencing, limits, and TTL behavior.src/hayhooks/durable/context.py,runtime.py, andhaystack.py: worker ownership, heartbeats, retries, checkpoints, thread shutdown, and Haystack recovery.src/hayhooks/durable/fastapi.py: ownership, idempotency, REST status codes, and bounded SSE replay.server/app.py,server/utils/deploy_utils.py, andserver/utils/module_loader.py: startup, dynamic deploy rollback, and live-work protection.Please keep these invariants in view:
Pre-GA verification
hatch run test:unit: 827 passed, 4 skipped, 26 deselected, 3 xfailedhatch run fmt-checkandhatch run test:typeshatch run docs:build --strictSIGKILLrecovery for waiting and running executions; SSE reattachment and cooperative cancellation verified/statusdegraded during interruption, persisted execution recovered on attempt 2, then health returned to Up