Skip to content

Commit 32dbeb8

Browse files
aparajonclaude
andcommitted
refactor(storage): claim applies only through the operation ladder
FindNextApply is gone: drivers claim work through the operation ladder (FindNextApplyOperation and friends), and a specific apply is claimed with ClaimApplyByID, which shares the same six-arm claim predicate and lease rotation. The predicate's coverage — pending-with-child-rows, stale-heartbeat reclaim, retryable budget, and start control requests — now lives entirely in the ClaimApplyByID tests, and the tern operator integration helpers claim each dispatched apply by ID the way api.Service drivers do. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent f111c95 commit 32dbeb8

16 files changed

Lines changed: 148 additions & 526 deletions

pkg/api/config.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,8 @@ type ServerConfig struct {
106106
EnvironmentOrder []string `yaml:"environment_order"`
107107

108108
// Drivers is the number of concurrent operator drivers that claim and drive
109-
// applies. Each driver independently polls FindNextApply with FOR UPDATE
110-
// SKIP LOCKED to prevent races. Defaults to DefaultDrivers.
109+
// applies. Each driver independently polls for claimable work with FOR
110+
// UPDATE SKIP LOCKED queries to prevent races. Defaults to DefaultDrivers.
111111
Drivers int `yaml:"drivers"`
112112

113113
// MetricsPort is the TCP port of the dedicated HTTP listener serving

pkg/storage/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ See [`pkg/state`](../state/) for the full state hierarchy (apply states, task st
5555
The apply store supports crash recovery through heartbeat-based leasing:
5656

5757
- **Heartbeat**: Drivers call `Heartbeat(applyID)` every 10 seconds to signal they're alive
58-
- **FindNextApply**: Claims one apply with a stale heartbeat (>1 minute since last update) by selecting it and refreshing its heartbeat in one transaction
58+
- **ClaimApplyByID**: Claims an apply with a stale heartbeat (>1 minute since last update) by selecting it and refreshing its heartbeat in one transaction
5959
- If a driver crashes, its apply becomes claimable after the heartbeat times out
6060

6161
## Webhook Inbox Retention

pkg/storage/mysqlstore/applies.go

Lines changed: 26 additions & 133 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,7 +1112,7 @@ func (s *applyStore) UpdateDerivedState(ctx context.Context, applyID int64, expe
11121112
}
11131113

11141114
// GetInProgress returns all applies in non-terminal states.
1115-
// Note: For recovery, use FindNextApply which handles locking and heartbeat staleness.
1115+
// Note: For recovery, use ClaimApplyByID which handles locking and heartbeat staleness.
11161116
func (s *applyStore) GetInProgress(ctx context.Context) ([]*storage.Apply, error) {
11171117
statePredicate, args := nonTerminalApplyStatePredicate("state")
11181118
rows, err := s.db.QueryContext(ctx, fmt.Sprintf(`
@@ -1130,8 +1130,9 @@ func (s *applyStore) GetInProgress(ctx context.Context) ([]*storage.Apply, error
11301130
}
11311131

11321132
// FindStuckPendingApplies returns pending applies older than olderThan that
1133-
// carry child rows — the child-rows arm of FindNextApply's pending predicate, so
1134-
// every row returned is one a driver should already have claimed. It is a
1133+
// carry child rows — the child-rows arm of the claim predicate (see
1134+
// ClaimApplyByID), so every row returned is one a driver should already have
1135+
// claimed. It is a
11351136
// read-only diagnostic (no lease, no FOR UPDATE): apply creation rejects a
11361137
// second active apply for the same target instead of queuing it, so a pending
11371138
// apply this old is never legitimately waiting its turn. Ordered oldest first
@@ -1381,33 +1382,34 @@ func (s *applyStore) CountRecentByState(ctx context.Context, filter storage.Rece
13811382
return counts, nil
13821383
}
13831384

1384-
// FindNextApply atomically claims the next apply that needs attention.
1385-
// A claim selects one stale apply and refreshes its heartbeat in the same
1386-
// transaction. That heartbeat is the operator's lease while it reloads state
1387-
// and resumes the apply.
1388-
// Returns the claimed apply, or nil if nothing needs work.
1389-
//
1390-
// Matches queued pending applies with persisted tasks, pending, stopped, or
1391-
// waiting-for-deploy applies with a pending start control request, stale active
1392-
// applies whose heartbeat expired beyond the lease staleness window, and
1393-
// recently failed_retryable applies that still have retry budget.
1394-
// Apply creation/update enforces one active apply per database/type/environment,
1395-
// so claims only need to lease one row and avoid driver races on that row.
1396-
func (s *applyStore) FindNextApply(ctx context.Context, owner string) (*storage.Apply, error) {
1385+
// ClaimApplyByID atomically claims one specific apply by ID: a claim selects
1386+
// the row when it needs a driver — pending with persisted child rows; a stale
1387+
// active state whose heartbeat expired beyond the lease staleness window;
1388+
// recently failed_retryable with retry budget; or pending, stopped, or
1389+
// waiting-for-deploy with a pending start control request — and rotates a
1390+
// fresh lease onto it (owner, token, heartbeat) in the same transaction. The
1391+
// operation-level claim loop calls this after claiming an apply_operations row
1392+
// to acquire the parent apply lease that lease-guarded writes (ResumeApply,
1393+
// MarkCompleted, Heartbeat) require. Returns nil when the apply does not
1394+
// exist, is locked by a peer (SKIP LOCKED), is not currently claimable, or —
1395+
// for a stopped apply with a pending start request — the claim was refused
1396+
// because another active apply owns the target (the start request is failed in
1397+
// that case; see persistApplyClaim).
1398+
func (s *applyStore) ClaimApplyByID(ctx context.Context, applyID int64, owner string) (*storage.Apply, error) {
13971399
if owner == "" {
1398-
return nil, fmt.Errorf("operator owner is required to claim apply: %w", storage.ErrApplyLeaseLost)
1400+
return nil, fmt.Errorf("operator owner is required to claim apply %d: %w", applyID, storage.ErrApplyLeaseLost)
13991401
}
14001402
// Read committed keeps concurrent SKIP LOCKED claims from taking next-key
14011403
// range locks that can serialize drivers across otherwise independent targets.
14021404
tx, err := s.db.BeginTx(ctx, &sql.TxOptions{Isolation: sql.LevelReadCommitted})
14031405
if err != nil {
1404-
return nil, fmt.Errorf("begin claim apply transaction: %w", err)
1406+
return nil, fmt.Errorf("begin claim apply %d transaction: %w", applyID, err)
14051407
}
1406-
defer rollbackTx(ctx, tx, "claim apply")
1408+
defer rollbackTx(ctx, tx, "claim apply by id")
14071409

14081410
activeStates := claimableApplyStates()
14091411
activeStatePlaceholders := placeholders(len(activeStates))
1410-
queryArgs := []any{state.Apply.Pending}
1412+
queryArgs := []any{applyID, state.Apply.Pending}
14111413
queryArgs = append(queryArgs, stringArgs(activeStates)...)
14121414
queryArgs = append(queryArgs, state.Apply.FailedRetryable, maxRecoveryAttempts, retryableRecoveryFreshnessDays)
14131415
queryArgs = append(queryArgs,
@@ -1423,122 +1425,13 @@ func (s *applyStore) FindNextApply(ctx context.Context, owner string) (*storage.
14231425
staleClaimCutoff := s.dialect.RelativeTime(TimestampPrecisionDefault, BeforeCurrentTime, LiteralIntervalAmount(uint64(storage.ApplyLeaseStaleAfter.Microseconds())), IntervalMicrosecond)
14241426
retryFreshnessCutoff := s.dialect.RelativeTime(TimestampPrecisionDefault, BeforeCurrentTime, ParameterIntervalAmount(), IntervalDay)
14251427

1426-
// Apply creation/update enforces at most one active apply per
1427-
// database/type/environment. The claim query only needs to find stale work;
1428-
// FOR UPDATE SKIP LOCKED prevents concurrent drivers from claiming the same row.
1428+
// FOR UPDATE SKIP LOCKED prevents concurrent drivers from claiming the
1429+
// same row.
14291430
//
14301431
// The pending clause requires child rows so a half-created apply is never
14311432
// claimed. Creation dual-writes tasks and the apply_operations row in one
14321433
// transaction, so either proves the create committed fully; a VSchema-only
14331434
// apply carries an operation row but no tasks, so tasks alone would strand it.
1434-
row := tx.QueryRowContext(ctx, fmt.Sprintf(`
1435-
SELECT %s
1436-
FROM applies a
1437-
WHERE (
1438-
(a.state = ? AND (
1439-
EXISTS (SELECT 1 FROM tasks t WHERE t.apply_id = a.id)
1440-
OR EXISTS (SELECT 1 FROM apply_operations ao WHERE ao.apply_id = a.id)
1441-
))
1442-
OR (a.state IN (%s) AND a.updated_at < %s)
1443-
OR (a.state = ? AND a.attempt < ? AND a.updated_at >= %s)
1444-
OR (
1445-
a.state = ?
1446-
AND EXISTS (
1447-
SELECT 1
1448-
FROM apply_control_requests cr
1449-
WHERE cr.apply_id = a.id AND cr.operation = ? AND cr.status = ?
1450-
)
1451-
)
1452-
OR (
1453-
a.state = ?
1454-
AND EXISTS (
1455-
SELECT 1
1456-
FROM apply_control_requests cr
1457-
WHERE cr.apply_id = a.id AND cr.operation = ? AND cr.status = ?
1458-
)
1459-
)
1460-
OR (
1461-
a.state = ?
1462-
AND EXISTS (
1463-
SELECT 1
1464-
FROM apply_control_requests cr
1465-
WHERE cr.apply_id = a.id AND cr.operation = ? AND cr.status = ?
1466-
AND (
1467-
a.lease_acquired_at IS NULL
1468-
OR a.lease_acquired_at < cr.updated_at
1469-
OR a.updated_at < %s
1470-
)
1471-
)
1472-
)
1473-
)
1474-
ORDER BY a.created_at
1475-
LIMIT 1
1476-
FOR UPDATE SKIP LOCKED
1477-
`, applyColumns, activeStatePlaceholders, staleClaimCutoff, retryFreshnessCutoff, staleClaimCutoff), queryArgs...)
1478-
1479-
apply, err := scanApplyInto(row)
1480-
if errors.Is(err, sql.ErrNoRows) {
1481-
return nil, nil // No apply to claim
1482-
}
1483-
if err != nil {
1484-
return nil, fmt.Errorf("query next claimable apply: %w", err)
1485-
}
1486-
1487-
outcome, err := persistApplyClaim(ctx, s.db, s.locker, tx, apply, owner)
1488-
if err != nil {
1489-
return nil, err
1490-
}
1491-
if outcome.claimedAndComplete() {
1492-
return apply, nil
1493-
}
1494-
if outcome != claimAcquired {
1495-
return nil, nil
1496-
}
1497-
1498-
if err := tx.Commit(); err != nil {
1499-
return nil, fmt.Errorf("commit claim apply %d (%s): %w", apply.ID, apply.ApplyIdentifier, err)
1500-
}
1501-
1502-
return apply, nil
1503-
}
1504-
1505-
// ClaimApplyByID atomically claims one specific apply by ID using the same
1506-
// claimability rules as FindNextApply, scoped to a single row. The operation-
1507-
// level claim loop calls this after claiming an apply_operations row to acquire
1508-
// the parent apply lease that lease-guarded writes (ResumeApply, MarkCompleted,
1509-
// Heartbeat) require. Returns nil when the apply does not exist, is locked by a
1510-
// peer (SKIP LOCKED), is not currently claimable, or — for a stopped apply with
1511-
// a pending start request — the claim was refused because another active apply
1512-
// owns the target (the start request is failed in that case; see
1513-
// persistApplyClaim).
1514-
func (s *applyStore) ClaimApplyByID(ctx context.Context, applyID int64, owner string) (*storage.Apply, error) {
1515-
if owner == "" {
1516-
return nil, fmt.Errorf("operator owner is required to claim apply %d: %w", applyID, storage.ErrApplyLeaseLost)
1517-
}
1518-
tx, err := s.db.BeginTx(ctx, &sql.TxOptions{Isolation: sql.LevelReadCommitted})
1519-
if err != nil {
1520-
return nil, fmt.Errorf("begin claim apply %d transaction: %w", applyID, err)
1521-
}
1522-
defer rollbackTx(ctx, tx, "claim apply by id")
1523-
1524-
activeStates := claimableApplyStates()
1525-
activeStatePlaceholders := placeholders(len(activeStates))
1526-
queryArgs := []any{applyID, state.Apply.Pending}
1527-
queryArgs = append(queryArgs, stringArgs(activeStates)...)
1528-
queryArgs = append(queryArgs, state.Apply.FailedRetryable, maxRecoveryAttempts, retryableRecoveryFreshnessDays)
1529-
queryArgs = append(queryArgs,
1530-
state.Apply.Pending,
1531-
storage.ControlOperationStart, storage.ControlRequestPending)
1532-
queryArgs = append(queryArgs,
1533-
state.Apply.Stopped,
1534-
storage.ControlOperationStart, storage.ControlRequestPending)
1535-
queryArgs = append(queryArgs,
1536-
state.Apply.WaitingForDeploy,
1537-
storage.ControlOperationStart, storage.ControlRequestPending)
1538-
1539-
staleClaimCutoff := s.dialect.RelativeTime(TimestampPrecisionDefault, BeforeCurrentTime, LiteralIntervalAmount(uint64(storage.ApplyLeaseStaleAfter.Microseconds())), IntervalMicrosecond)
1540-
retryFreshnessCutoff := s.dialect.RelativeTime(TimestampPrecisionDefault, BeforeCurrentTime, ParameterIntervalAmount(), IntervalDay)
1541-
15421435
row := tx.QueryRowContext(ctx, fmt.Sprintf(`
15431436
SELECT %s
15441437
FROM applies a
@@ -1914,7 +1807,7 @@ func failPendingStartControlRequestTx(ctx context.Context, tx *sql.Tx, applyID i
19141807

19151808
// Heartbeat updates the apply's updated_at timestamp to maintain the lease.
19161809
// Should be called every 10 seconds while working on an apply.
1917-
// If not called for > 1 minute, another driver can claim the apply via FindNextApply.
1810+
// If not called for > 1 minute, another driver can claim the apply via ClaimApplyByID.
19181811
// When ctx has an apply lease, a stale token returns ErrApplyLeaseLost so the
19191812
// old operator owner stops before writing state or external side effects.
19201813
// SetRevertSkipped records when skip-revert was dispatched for an apply. It is a
@@ -1924,7 +1817,7 @@ func failPendingStartControlRequestTx(ctx context.Context, tx *sql.Tx, applyID i
19241817
//
19251818
// updated_at is pinned to its current value so this write does not trip the
19261819
// column's ON UPDATE CURRENT_TIMESTAMP. updated_at is the apply's lease
1927-
// heartbeat (the staleness gate in FindNextApply); bumping it here would renew
1820+
// heartbeat (the staleness gate in the claim predicate); bumping it here would renew
19281821
// the heartbeat from a non-lease caller and could delay another driver's
19291822
// recovery claim.
19301823
func (s *applyStore) SetRevertSkipped(ctx context.Context, applyID int64, at time.Time) error {

0 commit comments

Comments
 (0)