fix: show run as 'running' when task heartbeats active but run heartbeat expired - #485
fix: show run as 'running' when task heartbeats active but run heartbeat expired#485npow wants to merge 4 commits into
Conversation
…artbeat expired When a run's orchestrator-level heartbeat expires (e.g. after a transient failure in a foreach coordinator) but individual tasks are still heartbeating, the UI was incorrectly showing the run as 'failed' even with active tasks. Fix: add a LEFT JOIN LATERAL that pre-computes max(last_heartbeat_ts) across all tasks for each run, then use it as a fallback in both the `status` and `finished_at` CASE expressions before the final ELSE 'failed'. Uses the same RUN_INACTIVE_CUTOFF_TIME threshold (6 min default) as the run-level heartbeat check for consistency. The join is computed once per run row and the result referenced from both expressions, avoiding the O(runs×tasks) cost that an inline correlated EXISTS would incur on list endpoints (which also call find_records with enable_joins=True). Reproducer: foreach flow with 200+ tasks where one task hits a transient infrastructure error; orchestrator heartbeat lapses but remaining tasks continue. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
722e355 to
e194bf7
Compare
|
Addressed all three points — force-pushed. 1. Performance (correlated subquery → lateral join) Switched from an inline correlated An index on 2. Threshold ( Changed to 3. Skipped test The skipped test ( |
…tics When a task has an active heartbeat, the run now correctly shows 'running' even if the run-level heartbeat has expired. Update the test that was asserting 'failed' in this scenario — it was written under the old semantics where run heartbeat was the sole signal. Also apply black formatting to pass pre-commit. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
The previous fix was too broad: any run with an expired run heartbeat would check task heartbeats and potentially show 'running', even for zombie runs dead for days. Add a staleness bound (RUN_INACTIVE_CUTOFF_TIME * 10 = 3600s): the task-HB fallback only fires when the run heartbeat expired recently. A 7-day-old run heartbeat (604800s >> 3600s) still falls through to 'failed'; a 7-minute-old run heartbeat (420s <= 3600s) gets the task-HB check. Also revert the test assertion that was incorrectly updated to expect 'running' for the zombie-run scenario — it should remain 'failed'. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…oof of liveness The staleness bound on the run heartbeat was unnecessary. The lateral JOIN already constrains to task heartbeats within RUN_INACTIVE_CUTOFF_TIME (360s), so any task HB that passes the check is genuinely fresh. No scenario exists where a <6-min-old task HB is wrong evidence that a task is alive. Restore test assertion to 'running' — the prior 'failed' expectation was documenting the old buggy behavior that this PR is fixing. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
No immediate objections to the change, only some performance concerns. If the queries work fine in your environments with a bunch of runs, then it should be fine.
I'm wondering about the premise for the change though, where a run level heartbeat goes stale, but tasks are still running. How common is this? To my understanding, task and run heartbeats update in lockstep from the same source
| """ | ||
| LEFT JOIN LATERAL ( | ||
| SELECT max(last_heartbeat_ts) as last_heartbeat_ts | ||
| FROM {task_table} t | ||
| WHERE t.flow_id = {table_name}.flow_id | ||
| AND t.run_number = {table_name}.run_number | ||
| ) as latest_task_heartbeat ON true | ||
| """.format(table_name=table_name, task_table=task_table), |
There was a problem hiding this comment.
does this perform well at scale for the runs listing?
There was a problem hiding this comment.
I measured this locally on PostgreSQL 11 with 50,000 runs and 1,000,000 tasks (20 per run), using warm-cache EXPLAIN ANALYZE with timing overhead disabled. The current max lookup and task index took 0.190 ms for a 50-row listing and 120.791 ms for a status scan across all 50,000 runs. A partial index on (flow_id, run_number, last_heartbeat_ts DESC) for non-null heartbeats, paired with ORDER BY last_heartbeat_ts DESC LIMIT 1, reduced those to 0.076 ms and 63.821 ms. The ordinary paged listing is already cheap because the outer scan stops at the limit; the index mainly protects status-filter and high-scan cases. I pushed the working version at Aryan95614@9b12cc6. It also moves the task-heartbeat rule into the shared run-status definition used by both ui_backend and metadata-service filtering, so the two services do not classify the same run differently. Full integration result: 135 passed, 13 skipped.
3 participants
Problem
In foreach flows, when a transient failure causes the orchestrator to stop updating the run-level heartbeat but individual tasks continue executing, the Metaflow UI shows the run as
failedeven though tasks are still running.Concrete example:
CoplayBucketBackfillFlowrun with 226 tasks — 1 task hit a transient DNS error (SparkGenieStderrUnknownHost), orchestrator heartbeat lapsed, but 30 tasks were still running. UI showedfailedand therunningfilter returned nothing.Root Cause
The run
statusSQL inrun.pyfalls toELSE 'failed'when the run-levellast_heartbeat_tshas exceededRUN_INACTIVE_CUTOFF_TIME(6 min default), regardless of whether individual tasks are still heartbeating.Fix
Add an
EXISTSfallback that checks task-level heartbeats (usingHEARTBEAT_THRESHOLD) before the finalELSE 'failed'. Bothstatusandfinished_atare updated consistently:status: returns'running'if any task has a heartbeat withinHEARTBEAT_THRESHOLDfinished_at: returnsNULL(not yet finished) in the same conditionAlso adds a new integration test covering this scenario.
Test
New test:
test_run_status_running_when_task_heartbeat_active_and_run_heartbeat_expiredstatus == 'running'andfinished_at is None🤖 Generated with Claude Code