Skip to content

Coordinator: enable-job lease has no fencing token — an expired-lease pod's writes stomp the new claimant (progress reset, spurious failures, claim released out from under it) #232

Description

@nikhilunni

Problem

claim_enable_jobs implements the lease acquisition correctly (atomic claim of NULL/expired with FOR UPDATE SKIP LOCKED, engram-postgres/src/lib.rs:2160-2192). But every post-claim write is unfenced — the classic lease-without-fencing-token bug: the lease arbitrates who starts, nothing arbitrates who may write.

  • update_enable_job_progress — WHERE id = $1 only, and it re-stamps claimed_at = NOW() (lib.rs:2194-2220, renewal at 2205) — extending the lease on behalf of whoever writes, including a pod whose lease already expired.
  • set_enable_job_state — WHERE id = $1 only (lib.rs:2222-2245).
  • record_enable_job_failure — WHERE id = $1 only, and it clears claimed_by/claimed_at unconditionally (lib.rs:2247-2269) — releasing a lease it may no longer hold.
  • Only retry_enable_job does CAS (AND state = 'failed', lib.rs:2277).

Failure scenario

  1. Pod A claims job J (chunking a 30 GiB image); GCS is slow; A blows past lease_secs.
  2. Pod B's sweep legitimately re-claims J and restarts chunk-enable.
  3. A's stale ticks keep writing progress — each resets chunks_done to A's smaller number and renews claimed_at, making B's claim look perpetually fresh-but-contested; the UI progress bar (GET /enable-jobs/:id) jumps backward.
  4. A hits a transient error → record_enable_job_failure clears B's claim and stamps error on a job B is actively completing → a third pod claims it. Duplicate chunk uploads, attempts budget burned by phantom failures, jobs oscillating working→failed→pending while actually succeeding.

Proposed fix (mechanical)

  1. Thread the claimant identity through the trait: update_enable_job_progress / set_enable_job_state / record_enable_job_failure in engram-core/src/traits/metadata.rs take claimant: &str.
  2. Add AND claimed_by = $claimant to each UPDATE (lib.rs:2194-2269). On rows_affected() == 0 with the row existing, return MetaError::Conflict("lease lost to <claimed_by>") instead of success/NotFound.
  3. In the enable-job worker (caller in the coordinator's enable scanner), treat Conflict as "stop work on this job immediately" — drop the local task without touching state.
  4. Keep the claimed_at renewal in progress updates — it's now safe because it's fenced; document it as the lease heartbeat.
  5. Tests: extend crates/engram-coordinator/tests/enable_jobs_live_pg.rs with a two-claimant scenario — claim as "pod-a", expire, claim as "pod-b", assert pod-a's progress/state/failure writes return Conflict and mutate nothing.

Acceptance criteria

  • All three post-claim writers carry claimed_by fencing; zero-row UPDATE on an existing row surfaces as Conflict.
  • Live-PG two-claimant test proves a stale claimant cannot change state, chunks_done, attempts, or claimed_at.
  • Worker abandons a job on Conflict (mock-store unit test).

Risk/scope

Small and mechanical — 3 SQL statements, 3 trait signatures, one worker branch, one test file. No schema change (claimed_by exists). 1-2 days.


Found by automated structural analysis (parallel codebase audit, 2026-06-12). Same fencing pattern as the session-lease release issue (#212) — consider one PR series establishing "every lease write is fenced" as a repo convention.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions