hardening: completion-path scale + lock-order + terminal-effect ownership
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 7
- Forks
- 3
- Avg merge
- 2h 12m
- Merged PRs (30d)
- 30
Description
Problem
The job-completion + terminalization paths have latent lock-order and scale hazards that a serialized
rollup (needed for correct canceled accounting) can worsen, plus some terminal effects are missing.
Areas
- Universal intra-table lock order for multi-run ops. Supersede locks victims
counter DESC
(supersede.go), the reaper batch locks byid, cancel batches differ — two txs can grab the same
runs in opposite orders and deadlock (40P01). Adopt a UNIVERSALORDER BY id ASC … FOR UPDATEfor
supersede/reaper/cancel/reconciliation; include the supersede TRIGGER run in the ordered set
(CompleteJobholds its own run thensupersedeAfterCascadelocks others). Present victims by counter
separately from the lock order. - Completion-path scale. Serializing every parallel job completion of one run on
run FOR UPDATE
funnels connections onto one row lock (pool exhaustion under wide stages) and lets stale/duplicate/
old-attempt results queue before losing the CAS. Design: a cheapstatus/attemptprecheck before any
row lock; the per-completion hot path locks onlystage_run(the actual rollup serialization) + the
job CAS;run FOR UPDATEonly on the completion that finalizes the run (once per run) + terminalizers;
boundedlock_timeout+ idempotent retry. Benchmark p95 AND p99 + connection-pool saturation +
duplicate-result storm with a hard budget before rollout. - Global lock order incl. deploy tables:
Lane → Pipeline → Freeze → deploy_target → run → stage → job → artifact → deployment_revision → deploy_watch.StartNativeDeploy/StartNativeDeployDeclaredandFinalizeDeployWatch
(watch→revision today — the odd one out) must be reordered to fit; a fencedDELETEhitting 0 rows ⇒
no-op. Every path that touches these together adopts it (CompleteJob, approve/reject, cancel, reaper,
native). - Supersede terminalization STAYS synchronous — the at-ready cancel of older lane siblings
(supersedeLaneSiblings/supersedeOne,supersede.go, already bounded + skip-on-contention) is the
authorization block for the window where a newer run's gate is READY but not yet PASSED (no
run_gate_passmarker exists yet,gatepass.go). Do NOT move it to a worker. The only async supersede
piece is the EXISTING external-effects worker (ClaimSupersedeEffects). - Terminal-effect ownership. Manual cancel cleans up but does NOT close the GitHub Check today
(actions.go— noReportRunCompleted); the queued/reaper cancel paths that finalize a run discard
JobCompletion(Check + services stayin_progress). Add per-terminalizer, single-emitter (CAS
winner), post-commit,service_generation-fenced effects on a detached context
(context.WithoutCancel+ own deadline). The GitHub Check close is best-effort (a stale check is
cosmetic); the exposed-services:teardown must be DURABLE (outbox/reconciliation) — a leaked exposed
workload is a security gap, not cosmetic.
Concurrency tests run under a timeout and assert no 40P01, not just correct final state.
Baseline: origin/main. Much of this is pre-existing latent; verify line numbers against current
origin/main.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start from CompleteJob and the named paths in supersede.go, gatepass.go, and actions.go, comparing them with the current origin/main baseline. Run the concurrency tests under a timeout and verify no 40P01 errors, then assess completion, terminalization, lock ordering, and durable service teardown against the stated benchmark and ownership requirements.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- github, go, postgresql
- Domain
- backend, ci-cd, databases, distributed-systems, security
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100