Batch support: fix completion races, accounting, and API polish

[jpcamara]

Implements every finding from the deep review of rails#142, one commit per theme. Each item below maps to a review finding.

Correctness fixes

1. Batches could get permanently stuck when jobs finished before enqueued_at was set

check_completion bails when the batch isn't started yet, and nothing ever re-triggered it. The EmptyJob was also enqueued before enqueued_at was set, so a fast worker could finish it inside the gap and the batch would hang forever. Same window existed for enqueue_after_transaction_commit = true and for the legacy after_commit path.

Fix: start_batch now sets enqueued_at before enqueueing the EmptyJob, and calls check_completion at the end as a sweep for work that finished during the window. (A process crash between the jobs' commit and start_batch is still unrecoverable without a supervisor-side sweep — noted as follow-up.)

2. Bulk discards orphaned batch executions → stuck batches

Execution.discard_jobs uses delete_all, bypassing callbacks: SolidQueue::Queue#clear / Mission Control "discard all" on batch jobs left batch_executions rows pointing at deleted jobs, and the batch could never finish.

Fix (lock-free, zero core changes): Execution is untouched — byte-identical to upstream. Row cleanup is handled declaratively by new FK constraints with on_delete: :cascade (consistent with every other execution table), and completion detection is handled by Batch.sweep_stalled, run automatically by the dispatcher's maintenance timer — ConcurrencyMaintenance is generalized into Dispatcher::Maintenance, so concurrency and batch maintenance share one timer thread and one worst-case database connection (the dispatcher's resource profile is unchanged from before batches; batch_maintenance: false disables the batch routine). ConcurrencyMaintenance remains as a compatibility shim with its original signature and behavior, so no shipped API is removed. Also documented as a recurring task for setups without a dispatcher. The sweep also closes the crash-before-start_batch window and retries completions whose callback enqueueing failed.

3. Callbacks could be lost between the finish CAS and callback enqueue

The update_all(finished_at:) CAS committed immediately; the counts + callbacks happened afterwards in a separate transaction. A crash (or a callback class that fails to deserialize) in between left the batch finished with zero counts and no callbacks, permanently.

Fix (CAS preserved): the single-statement CAS stays exactly as designed — losers still resolve with a 0-row UPDATE, no SELECT FOR UPDATE anywhere — but CAS + counters + callback enqueueing now share one transaction, so they commit atomically. The batch row is only locked by the CAS winner, once per batch lifetime, for the few milliseconds of finalization. Bonus fix: on PostgreSQL READ COMMITTED, a CAS that waits on a concurrent adder re-evaluates its NOT EXISTS against the original snapshot and could finish a batch with freshly committed executions (this existed in the original design too; MySQL's current reads and SQLite's single writer are immune) — a current-snapshot re-check after winning the CAS closes it. The empty_executions scope is now arel instead of raw SQL.

4. Progress counters double-counted failed jobs

Pre-finish completed_jobs was total - pending, which counts failed jobs (their tracking row is also gone), so progress_percentage could exceed 100% (3 jobs / 1 pending / 2 failed → 133%). Pre- and post-finish semantics also disagreed.

Fix: pre-finish completed_jobs is now total - pending - failed, consistent with the finished-batch definition. progress_percentage is now bounded.

5. Stale-state guard on mutable batches

Batch#enqueue only checked the in-memory finished?, so jobs could be added to a batch another process had just finished.

Fix (zero added locking): the existing total_jobs increment in BatchExecution.create_all_from_jobs gains an unfinished condition and raises AlreadyFinished on 0 rows, rolling back the enqueue transaction (jobs and tracking rows included). That statement already touched the batch row for one statement's duration; the WHERE clause is free. Interleaving resolves correctly in both directions: if completion's CAS wins, the adder's increment matches 0 rows and rolls back; if the adder wins, the CAS re-evaluates and sees the new executions.

6. on_conflict: :discard accounting differed between enqueue paths

perform_later counted a conflict-discarded job in total_jobs (as completed); perform_all_later didn't count it at all, because batch_all ran after dispatch on the survivors only.

Fix: prepare_all_for_execution now creates tracking rows before dispatching; a discarded job's row is cleaned up via dependent: :destroy exactly like the single-job path. Both paths now agree (discarded = counted + completed).

API changes

• Batch membership is captured at enqueue-time, not initialize-time. A job instantiated outside the block and enqueued inside joins the batch, and vice versa. Implemented in ActiveJob::BatchId#enqueue plus Job.enqueue_all for perform_all_later, which bypasses per-job #enqueue. Capture-before-deferral under enqueue_after_transaction_commit is guaranteed structurally — the include is nested like Rails' own initializer so BatchId deterministically wraps outside EnqueueAfterTransactionCommit — and pinned by an ancestor-order test plus the deferred-enqueue lifecycle test (verified on Rails 7.1 and 7.2 locally). This also removes the initialize override and its (*args, **kwargs) signature risk.
• active_job_batch_id kept, now documented as reserved — it's not read anywhere yet, but it's intentionally early: a provider-agnostic batch identifier (the batch analogue of solid_queue_jobs.active_job_id). Now assigned in before_create instead of after_initialize so loads don't pay for it, with a comment stating the intent and a test pinning the assignment.
• BatchId#batch no longer re-queries on every call when the job has no batch.

Cleanups

• Removed dead Batch#as_active_job.
• The batchable execution concern moved to FailedExecution::Batchable (single-owner concerns live under their owner, like Job::Batchable), dropped its redundant is_a?(FailedExecution) check, and renamed the callback to say what it does (destroy_job_batch_execution).
• Narrowed the rescue => e blocks in both batchable concerns to ActiveRecord::ActiveRecordError and included the error class in the log line — an unexpected error now propagates instead of silently leaving the batch stuck.

Docs

• Fixed the README callback examples — they showed def perform(batch), but callbacks are enqueued with their original arguments only and would raise ArgumentError. Examples now use the batch accessor, matching the implementation.
• Documented: counter semantics (every retry attempt counts toward total_jobs), discard semantics, manual-retry limitation, enqueue-time batch membership, the wait_until:-resolved-at-creation caveat for callback jobs, Batch.clear_finished_in_batches, and a migration snippet for existing installs.

Tests

New coverage: stale-instance AlreadyFinished via the lock-free guard, in-flight counters with failures (would have caught rails#4), start_batch sweep (regression for #1), sweep_stalled finishing bulk-discarded batches and starting crashed-before-start batches (regression for #2), optional dispatcher batch maintenance, enqueue-time membership in both directions, and single-vs-bulk conflict-discard consistency (regression for rails#6). Also fixed the latent batch.batch_id call in BatchWithArgumentsJob (method doesn't exist; never executed).

Not implemented, flagged in review as acceptable: re-linking manually retried jobs to their batch (documented instead — doing it for retry_all would touch the shared Dispatching path).

Stress testing (MySQL 8.0 / PG 15, 8 threads, local)

Compared three versions: base (original batch-poc), locked (an abandoned row-lock design), final (this PR). Simulated worker finishes / concurrent adds directly against the models.

Scenario	base	locked	final
s0: 5,000 non-batch job finishes	1,017/s	1,009/s	1,176/s
s1: 5,000 finishes across 1,000 batches	551/s	634/s	692/s
s2: 2,000 finishes, ONE batch (completion herd)	730/s	630/s	863/s
s3: 200 concurrent adds to ONE running batch	255/s*	147/s	218/s

* base measured without FKs (its intended schema).

• No throughput regression from the lock-free rework anywhere; the completion herd (s2) is actually fastest on final. The locked design was measurably worst at both completion (s2) and adding (s3) — the row-lock concern was justified.
• Stress testing caught a real deadlock: with the new FK, execution inserts take a shared lock on the batch row (FK validation) and the total_jobs increment then upgrades to exclusive — 163/200 concurrent adds deadlocked (base code deadlocked identically with the FK present: it's FK x statement-order, not specific to this PR's logic). Fixed by incrementing before inserting (exclusive-first); after the fix: 0 errors, 2001/2001 jobs accounted, throughput on par with the FK-less schema. PG was immune (FOR KEY SHARE is compatible with non-key updates) but was verified clean too.
• Sweep cost: 10,000 unfinished batches (1,000 stalled) swept in ~4s, most of it finishing the 1,000 — fine at the 300s default cadence.

Design constraints honored throughout: the batch row is never locked on the per-job hot path — completion keeps the original CAS shape (single-statement, once-per-batch), the mutable-batch guard rides an existing statement, and core Solid Queue files are touched only where the PR already touched them (plus the dispatcher's established maintenance seam for the sweeper).

Verified locally on all three adapters (models + unit + integration suites):

Adapter	Result
SQLite	252/252 green
PostgreSQL 15	252/252 green
MySQL 8.0	252/252 green

(One MySQL run flaked in AsyncProcessesLifecycleTest — the known-flaky supervisor lifecycle tests from upstream main, unrelated to batch code; green in isolation and on rerun.)

🤖 Generated with Claude Code

https://claude.ai/code/session_01P39PSB74nP3a8B2F7Z5xvz

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.