Skip to content

one_d4: share the claim/lease protocol between queues — starting with a conformance test, not a SQL mixin #1417

Description

@aaylward

After #1389 phase 5 there are three implementations of one claim/lease/fence protocol: PgQueue (C++, indexing_requests), ReanalysisQueue (C++, reanalysis_requests), and IndexingRequestDao (Java, indexing_requests). The SQL is duplicated. This issue is about what to actually extract, and in what order — the answer is not the obvious one.

What is genuinely duplicated

In pg_queue.cc (214 lines), the predicates repeat verbatim while the SET clauses do not:

Fragment Occurrences Used by
id = $1 AND owner_id = $2 AND status IN ('PENDING','PROCESSING') AND lease_expires_at > NOW() 3 Progress, Complete, Fail
id = $1 AND owner_id = $2 AND status IN ('PENDING','PROCESSING') 2 HandBack, Release
status IN (...) AND attempts < N AND (owner_id IS NULL OR lease_expires_at IS NULL OR lease_expires_at <= NOW()) 1 per table claim candidate scan
attempts = CASE WHEN owner_id = $1 THEN attempts ELSE attempts + 1 END 1 per table claim
attempts = CASE WHEN attempts > 0 THEN attempts - 1 ELSE 0 END 1 per table HandBack

The SET lists and RETURNING projections are payload-shaped and legitimately differ — games_indexed / dedupe_key on one table, games_processed / games_failed / cursor_game_url on the other. An abstraction that owns whole statements would have to model the payload; one that hands out WHERE fragments parameterized by table name would not.

Two constraints on any design

It cannot span languages. Java claims by reading CLAIM_CANDIDATES ids and then trying a conditional UPDATE on each; C++ does it in a single statement with FOR UPDATE SKIP LOCKED. That is not an inconsistency to clean up — H2 has no SKIP LOCKED, and the Java DAO runs against H2 in tests. Any DRY pass is two independent passes, one per language.

Uniformity would erase a deliberate asymmetry. The terminal writes fence on a live lease (lease_expires_at > NOW()); HandBack and Release deliberately do not. The comment above Progress gives the reason: at expiry a takeover is already licensed even before a rival has written its own owner_id, and the old owner must not win that race. A mixin that made all six predicates consistent would silently change that.

The divergence that motivates this

The two implementations of the same operation already fence differently:

pg_queue.cc:89                    WHERE id = $1 AND owner_id = $2 AND status = 'PROCESSING'
IndexingRequestDao.java:656-658   WHERE id = :id AND owner_id = :owner
                                    AND status IN ('PENDING', 'PROCESSING')

C++ Heartbeat and Java renewLease are the same protocol step. One accepts a PENDING owned row, the other does not.

This is currently harmless: claim sets status = 'PROCESSING', so no owned row is ever PENDING, and both predicates match on every reachable state. But nothing asserts that invariant, so the equivalence is an accident that holds rather than a property anything defends — and if a future path ever leaves a row owned-and-PENDING, the two workers would disagree about whether its lease can be renewed.

Proposed order of work

1. A conformance suite first. Parameterized over anything claim/lease-shaped, asserting the protocol rather than the SQL:

  • a fenced write refuses when the owner differs
  • a fenced write refuses when the lease has expired
  • a fenced write refuses when the row is already terminal
  • HandBack refunds the attempt; Release spends it
  • re-presenting the same owner id does not spend an attempt; a different one does
  • a claim is exclusive under concurrency

Run it against PgQueue and ReanalysisQueue (C++, real Postgres — these are properties of concurrent statements, not of code, as pg_queue_test.cc already argues), and against IndexingRequestDao on the Java side. This is what actually kills the bug class, and it forces the Heartbeat divergence above to be settled deliberately.

2. Then, if still worth it, fragments. Something like a LeasedTable in C++ holding the table name and exposing Claimable(max_attempts), Fenced(), Owned() as composable WHERE clauses, with SET lists left explicit at each call site. Java gets its own equivalent or nothing, depending on how much the candidate-scan shape absorbs.

Deliberately in that order: extracting the SQL removes typing, and the conformance test removes the failure. Doing the refactor first means refactoring under no test that would notice if it changed behavior.

Not in scope

/admin/rederive-openings is not a leased job and should not grow one — it has no chess in it and no claim protocol.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions