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.
After #1389 phase 5 there are three implementations of one claim/lease/fence protocol:
PgQueue(C++,indexing_requests),ReanalysisQueue(C++,reanalysis_requests), andIndexingRequestDao(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 theSETclauses do not:id = $1 AND owner_id = $2 AND status IN ('PENDING','PROCESSING') AND lease_expires_at > NOW()Progress,Complete,Failid = $1 AND owner_id = $2 AND status IN ('PENDING','PROCESSING')HandBack,Releasestatus IN (...) AND attempts < N AND (owner_id IS NULL OR lease_expires_at IS NULL OR lease_expires_at <= NOW())attempts = CASE WHEN owner_id = $1 THEN attempts ELSE attempts + 1 ENDattempts = CASE WHEN attempts > 0 THEN attempts - 1 ELSE 0 ENDHandBackThe
SETlists andRETURNINGprojections are payload-shaped and legitimately differ —games_indexed/dedupe_keyon one table,games_processed/games_failed/cursor_game_urlon 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_CANDIDATESids and then trying a conditionalUPDATEon each; C++ does it in a single statement withFOR UPDATE SKIP LOCKED. That is not an inconsistency to clean up — H2 has noSKIP 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());HandBackandReleasedeliberately do not. The comment aboveProgressgives the reason: at expiry a takeover is already licensed even before a rival has written its ownowner_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:
C++
Heartbeatand JavarenewLeaseare the same protocol step. One accepts aPENDINGowned row, the other does not.This is currently harmless:
claimsetsstatus = 'PROCESSING', so no owned row is everPENDING, 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:
HandBackrefunds the attempt;Releasespends itRun it against
PgQueueandReanalysisQueue(C++, real Postgres — these are properties of concurrent statements, not of code, aspg_queue_test.ccalready argues), and againstIndexingRequestDaoon the Java side. This is what actually kills the bug class, and it forces theHeartbeatdivergence above to be settled deliberately.2. Then, if still worth it, fragments. Something like a
LeasedTablein C++ holding the table name and exposingClaimable(max_attempts),Fenced(),Owned()as composable WHERE clauses, withSETlists 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-openingsis not a leased job and should not grow one — it has no chess in it and no claim protocol.