coordinator: failed ready-barrier release is not persisted when the store status read fails, re-opening the #2998 reconnect-hang window #3115

Summary

In broadcast_all_nodes_ready (binaries/coordinator/src/lib.rs), the ready-barrier release is recorded in memory unconditionally but persisted only conditionally on the failed-barrier path. When the status lookup used to build the persisted record fails, the durable write is skipped entirely — even though the in-memory ready_barrier_released flag has already been flipped to true. This re-opens exactly the reconnect-hang window that #2998 (PR #3013, commit 2ce3a97) set out to close, and it directly contradicts the invariant stated in that function's own comment.

Where

binaries/coordinator/src/lib.rs, introduced by commit 2ce3a97 / PR #3013 ("fix(coordinator): replay the ready barrier to a daemon that reconnects after it released").

release_barrier_message sets the in-memory flag unconditionally:

fn release_barrier_message(uuid, dataflow, clock) -> eyre::Result<Vec<u8>> {
    dataflow.ready_barrier_released = true;   // lib.rs:4582 — always
    all_nodes_ready_message(uuid, dataflow, clock)
}

But in broadcast_all_nodes_ready the persist is gated on resolving a status, and on the failure path a store read error (or Ok(None)) skips the write:

let message = release_barrier_message(uuid, dataflow, clock)?;   // lib.rs:4649 — flag now true

let persist_status = if dataflow.exited_before_subscribe.is_empty() {
    Some(StoreDataflowStatus::Running)
} else {
    match store.get_dataflow(&uuid) {
        Ok(Some(existing)) => Some(existing.status),
        Ok(None) => None,                                        // → skip persist
        Err(e) => {
            tracing::warn!(dataflow = %uuid, "cannot read status to persist failed barrier: {e}");
            None                                                 // → skip persist
        }
    }
};
if let Some(status) = persist_status
    && let Err(e) = dataflow.make_record(status).and_then(|r| store.put_dataflow(&r))
{
    tracing::warn!(dataflow = %uuid, "failed to persist ready-barrier release: {e}");
}

The comment immediately above this block states the intended invariant explicitly:

This must happen on a failed barrier too. [...] deferring the write to whatever failure path runs next leaves a restart window that reproduces #2998. Only the status is conditional [...]

The code makes the entire write conditional, not just the status.

Risk / affected code path

On a failed barrier (exited_before_subscribe non-empty), if store.get_dataflow(&uuid) returns Err(_) (transient store I/O error) the barrier release + verdict are never persisted, yet the in-memory entry already reports ready_barrier_released = true. If the in-memory RunningDataflow is then destroyed before any later status-persist captures the flag — coordinator restart, or orphan reclaim — a daemon that was disconnected when the broadcast fired reconnects, reestablish_running_dataflow reads record.ready_barrier_released == false from the (stale) persisted record, replay_all_nodes_ready is never called, and that daemon's nodes park in init_from_env() for the life of the dataflow. That is the #2998 symptom.

Reachability is narrow / low severity, to be clear:

• The Ok(None) branch is nearly unreachable in practice — the record is normally persisted as Pending at dataflow start, so a subsequent None would require the record to have been deleted.
• The realistic trigger is the Err(_) store-read branch (transient DB/disk error) coinciding with a failed barrier, a disconnected-then-reconnecting daemon, and entry destruction before the next status-persist re-captures the flag.

It is a genuine latent gap rather than a routinely-hit bug, but it undermines the durability guarantee the PR was specifically written to provide, on the one path (store I/O trouble) where durability matters most.

Suggested fix

Decouple persisting the release + verdict from resolving the status. The ready_barrier_released flag and the exited_before_subscribe verdict must be written whenever the barrier fires; only the status field should stay conditional. When the current status cannot be read, persist the release with a non-promoting fallback status (i.e. never Running for a failed barrier) rather than skipping the write entirely — e.g. fall back to persisting with the record's last-known/Pending status, or capture the status to preserve at an earlier reliable point. The key property: a failed-barrier release should still leave record.ready_barrier_released == true on disk even when the status read fails.

This issue was created by a scheduled, automated Claude code-review check (not a human). It reviewed the last few days of merged commits/PRs in this repo. The finding above was verified against the code, but please confirm before acting on it. The rest of the reviewed window (the daemon lifecycle fixes #2967/#2936/#2937, the memory-pool/input fixes #3011/#2935/#2968, the flume→tokio migration #2956, the uhlc 0.5→0.9 bump #2446, the runtime operator-payload fix #2742, and the CLI/CI/schema changes) reviewed clean.