Merge latest upstream solid_queue v1.5.0 (adopts PR #11 + adds a semaphore guard) by tblais1224 · Pull Request #12 · starburstlabs/solid_queue

tblais1224 · 2026-07-23T17:20:08Z

What does this PR do? (required)

Merges the latest upstream rails/solid_queue release, v1.5.0, into our fork and reconciles it with the fork's local changes. Our fork was based on upstream v1.4.0; this brings it up to v1.5.0 (23 upstream commits) while preserving everything we carry on top (batches, concurrency_max_blocked, security bumps, and the fork-only half of PR #11).

The headline: upstream v1.5.0 adopted our PR #11 (ported as 4f263b2) and added a second concurrency fix we didn't have (2124a51). This PR gets us both, plus the rest of v1.5.0.

version.rb moves 1.4.0 → 1.5.0 (the fork tracks upstream's version exactly).

Changes introduced by this bump

Everything upstream landed between v1.4.0 and v1.5.0, grouped by the kind of change.

🔒 Concurrency & correctness fixes (the main reason for the bump)

4f263b2 Prevent wrong release of blocked jobs and semaphore corruption — upstream's port of our PR Prevent false worker pruning from leaking concurrency permits #11. Locks the claimed-execution row before terminal bookkeeping so a stale/pruned execution can't return a concurrency permit twice.
2124a51 Guard signal_all against incrementing a semaphore beyond its limit — the fix we didn't have. Semaphore#signal already capped at value < limit, but the bulk signal_all path (used when releasing locks in bulk, e.g. discarding ready jobs) did not, letting a semaphore climb over its limit and leak permits.
e43f204 Fix recurring task double-enqueue caused by a wall-clock race — a recurring task could be enqueued twice; now fixed.

⚠️ Behavior changes to be aware of

546864f Recurring schedules now follow the app timezone — schedules without an explicit zone are interpreted in Rails.application.config.time_zone (defaults to UTC) instead of the server's system time. Schedules that already specify a zone (Fugit's 6th cron field) are untouched. Override with config.solid_queue.time_zone. (Verified a non-issue for crm-web — see below.)
d093f62 Default polling_interval 0.1s → 1s — install-template default only; existing config/queue.yml is unaffected.
4f72ea3 Undersized thread pool no longer blocks boot — when the DB connection pool is smaller than the worker thread pool, SolidQueue now logs a one-time warning instead of refusing to start (valid? stays about whether we can boot).
dfc3644 Rescued recurring enqueue errors are reported to Rails.error — previously swallowed; now surfaced to the error subscriber.

🧰 Compatibility

42db567 Drops Ruby 3.1 — required Ruby floor rises >= 3.1 → >= 3.2. (crm-web runs 3.4.8.)
23100e0 Supports strict_loading_by_default — SolidQueue no longer breaks when the app enables that Rails setting.

⚡ Performance / internal

e66e435 Improve DISTINCT queries on PostgreSQL via a recursive CTE.
0d5134b Avoid reloading job rows after dispatch and schedule.
e99dd40 Memoize Runnable#mode to stop allocating a String + StringInquirer per poll.

🛠️ Tooling / DX / misc

98af3c9 bin/jobs check to validate config before starting.
ecc170c Refactor of config parsing errors/warnings.
a8c8937 stopping? now accepts an optional job argument.
dd68a9c Stop trapping QUIT on Windows.
4546345 / 546864f doc updates; b11ce85 dynamic port allocation in Puma plugin tests; 142d2ac / a19771d / 952d852 upstream test-suite flakiness fixes.

Reconciliation with the fork (PR #11 and local changes)

git merge upstream/main produced conflicts in only 4 files, all the expected PR #11 overlap:

app/models/solid_queue/claimed_execution.rb → took upstream's canonical version. It calls Job#unblock_next_blocked_job, which our concurrency_controls.rb still defines, so it composes with our concurrency_max_blocked feature. Nuance vs our 85dd166: the semaphore signal now happens after the finish/fail commit instead of inside the claim transaction — both are exactly-once safe; upstream's ordering avoids rolling back an already-finished job.
claimed_execution_test.rb / claimed_execution_concurrency_test.rb → took upstream's versions (which match that finalization); confirmed no fork-only test was dropped.
README.md → kept our paragraph documenting supervisor-liveness pruning.

Preserved (upstream did NOT take this half of PR #11 — and these files didn't conflict): the supervisor-liveness prune guard (process/prunable.rb) and deregister-on-reap (supervisor/maintenance.rb). So we keep upstream's canonical claim-lock and our extra protection against pruning a worker whose supervisor is still alive.

Also preserved & unchanged by the merge: the batches feature and its schema, concurrency_max_blocked, and all security pins — rack 3.2.6, nokogiri 1.19.3, puma 7.2.1, concurrent-ruby 1.3.7, rack-session 2.1.2, rails 7.2.3.1.

Impact on crm-web (the consumer)

Checked crm-web; it can take this upgrade with no config changes.

Recurring timezone change is a non-issue: the only wall-clock-sensitive jobs (purge_admin_report_attachments at 1am, email_enable_retention_flag at 6am) already pin America/New_York, so the new default doesn't touch them. Every other recurring task is interval-based (every N minutes, minute 12 of every hour) and timezone-independent. config.time_zone is UTC and prod runs on UTC containers, so there's no shift.
polling_interval: set explicitly to 1 in config/queue.yml — the default change doesn't apply.
Ruby: on 3.4.8, so the 3.2 floor is fine.
Batches: actively used (SolidQueue::Batch.clear_finished_in_batches, complete_stalled_batches, Batch::EmptyJob) — preserved and tested.
The concurrency fixes directly benefit crm-web's concurrency-limited jobs (the calendar-sync job that triggered PR Prevent false worker pruning from leaking concurrency permits #11).

Release step: crm-web pins this gem by git SHA, so after this merges to main, bump the solid_queue revision in crm-web's Gemfile.lock (bundle update solid_queue). No tag or gem publish needed.

Link to Basecamp to-do, Trello card, New Relic or Honeybadger (required)

N/A — upstream reconciliation. Related: our PR #11 (starburstlabs/solid_queue#11) and upstream rails/solid_queue v1.5.0.

QA

What platforms should be included in QA?

QA steps

Full test suite on Postgres: TARGET_DB=postgres bin/setup && TARGET_DB=postgres bin/rails test → 286 runs, 0 failures (the one ProcessRecoveryTest process-count assertion is pre-existing full-suite timing flakiness — passes in isolation and on re-run).
Concurrency-limited jobs still honor their limit under a worker prune/reap race — covered by claimed_execution_concurrency_test.rb, which exercises real row locking on Postgres (0 skips on Postgres).
After merge to main, bump crm-web's solid_queue git revision (bundle update solid_queue).

Screenshots (if appropriate)

N/A

Docs

Update changelog after deployment if the changes in Admin are valuable for Sales, Support or Success teams.

Rails commit 6d1c401dc7 ("Pass job instance to stopping?") changed ActiveJob::Continuable#checkpoint! to call queue_adapter.stopping?(self), passing the running job so adapters can decide whether to checkpoint based on it. This caused errors such as: ArgumentError: wrong number of arguments (given 1, expected 0) The new argument is ignored. Solid Queue still relies solely on the worker shutdown flag. But the signature is now compatible with both the new and old Rails calling conventions.

The Puma plugin tests bind to a hardcoded port (9222). When a test teardown completes and the next test setup starts, the OS may still hold the port in TCP TIME_WAIT state, causing EADDRINUSE failures that cascade into nil process lookups and false test failures. Replace the hardcoded port with a dynamically allocated one via TCPServer, so each test gets a guaranteed available port.

…thread pool Solid Queue refused to boot when the Active Record connection pool was smaller than the worker thread pool. This blocked legitimate setups, such as I/O-bound queues that run many threads against a deliberately small pool. Per the discussion in rails#736, downgrade the check from a validation error to a SolidQueue.logger warning emitted once on the boot path in Supervisor.start, so undersized configurations boot while still surfacing the advisory. valid? stays purely about whether we can boot. Closes rails#736

Lets you catch broken recurring.yml etc before the supervisor boots and silently exits. Works as `bin/jobs check` or the `solid_queue:check` rake task. Skips the DB-pool check when DB creds aren't around (for isntance on CI). See rails#722 for more info PS: 🐶 <- this is mochi!

We had two different warnings being surfaced in totally different ways, and at the same time, warnings being totally independent and different from errors. Unify everything and make them consistent. This is a follow-up to rails#661, rails#753 and rails#745

…r poll Before 960694e (Dec 2023, "Extract Supervised concern from Runnable"), the mode was set once in #start as `@mode = mode.to_s.inquiry` and read back via `attr_reader :mode`. After that commit, the reader was replaced with a method that re-runs `(@mode || DEFAULT_MODE).to_s.inquiry` on every call. `#mode` is invoked in the poll-loop hot path via `running_async?` / `running_as_fork?` / `running_inline?`, themselves called from `shutting_down?` on every iteration of the dispatcher, worker, and scheduler loops. With the default `queue.yml` (1s dispatcher, 0.1s worker), this is ~3600 redundant allocations per hour per process — a String and an ActiveSupport::StringInquirer for a value that doesn't change after the supervisor sets it once. Restoring the original "compute the inquiry at write time, cache it afterwards" semantic via a custom `mode=` writer eliminates the allocations with no behaviour change. The default-mode fallback (used when `mode=` was never called) memoizes on first read of `#mode`. Reproduction harness and validation: https://github.com/<TODO>/solid-queue-idle-memory-repro heapy diff with `GC.start(full_mark: true, immediate_sweep: true)` before each `ObjectSpace.dump_all` confirms the `runnable.rb` line disappears from retained-allocation reports after this change. Full test suite (227 runs, 1299 assertions) passes with no regressions. Refs rails#262, rails#330, rails#405.

In `Job.dispatch_all` and `Job.schedule_all` (the hot path of `enqueue_all` / `ActiveJob.perform_all_later`), the post-insert step rebuilt the returned set with: where(id: <Execution>.where(job_id: jobs.map(&:id)).pluck(:job_id)) which always issues two statements per execution table -- a `pluck` on the execution table and a follow-up `SELECT ... FROM solid_queue_jobs WHERE id IN (...)` -- to re-read rows we already hold in memory. The `Job` instances passed in are the ones just returned by `create_all_from_active_jobs`, so they are persisted and have ids. Filter `jobs` in memory against the plucked execution ids instead. Per `enqueue_all` batch: * dispatch_all: 4 queries -> 2 (drops the two job-table reloads) * schedule_all: 2 queries -> 1 (drops the job-table reload) The avoided `SELECT` is the most expensive of the four: it scans the wide `solid_queue_jobs` row including the serialized `arguments` payload, so the saving is bytes-over-the-wire and not just a round trip. Semantic equivalence: * Each job is inserted into at most one of ready_executions / blocked_executions / scheduled_executions in this code path, so the in-memory filter selects exactly the same job ids the prior `where(id: ...)` would have returned. * Both call sites (`prepare_all_for_execution` which concatenates with `+`, and `Execution::Dispatching#dispatch_jobs` which calls `.map(&:id)`) already coerce the result to an Array and do not depend on it being an `ActiveRecord::Relation` or on row order. * `jobs` is the same collection that was just queried back in `create_all_from_active_jobs`, so attribute freshness is unchanged versus the previous reload. Verified against the existing `job_test`, `ready_execution_test`, `dispatcher_test`, and `concurrency_controls_test` suites on SQLite (55 runs, 0 failures).

Rather than have SolidQueue act on the server configured time, this change allows you to use either the Rails application's current timezone or a specified default.

Replace fixed sleeps with deterministic waits in the "don't block claimed executions that get released" test. The test was using sleep(0.2) to wait for claiming and sleep(shutdown_timeout + 0.6) to wait for shutdown, both of which are timing sensitive and unreliable on slow CI. Instead, use wait_for to wait for the job to be claimed, and terminate_process to wait for the supervisor to actually exit.

Forked processes write to both the queue tables (SolidQueue::Record, on the queue database) and the app's JobResult rows (ApplicationRecord, on the primary database). skip_active_record_query_cache only bypassed the queue connection's query cache, so JobResult COUNT/find reads stayed cached and stale across forks — a source of intermittent integration-test failures. Nest JobResult.uncached so both connections' caches are bypassed. wait_for and wait_while_with_timeout use this helper, so they're covered too.

Harden integration tests that fail intermittently on CI: - Wait for prior supervisors to deregister and clear leftover records before creating JobResult rows, avoiding recycled-PK overwrites from orphaned writers. - Replace fixed sleeps with wait_* conditions and lengthen timeouts. - Scope recurring-task assertions to the task's own results. - Retune the timing-sensitive tests so their intended results hold deterministically rather than relaxing the assertions: the "000" overwrite and the limiting-concurrency "C"-wins cases (widened pause margins), and "process a job that exits" (enqueue all "no exit" jobs before the exiting one so they always complete first).

Apps that set `config.active_record.strict_loading_by_default = true` hit StrictLoadingViolationError when Solid Queue accesses its own associations internally. Turn strict loading off for Solid Queue's models by setting it on the shared SolidQueue::Record base class, so every model — and any future association — is covered in one place. Enable strict_loading_by_default in the dummy app so the suite guards against regressions.

RecurringTask#enqueue rescues Job::EnqueueError (and other-adapter enqueue failures) but forwarded only the message string to the notification payload, never calling Rails.error.report. Reporters subscribed via ActiveSupport::ErrorReporter (Sentry, etc.) therefore never saw recurring enqueue failures — only a log line — so DB write failures during recurring enqueue produced missed ticks with no alerting. Report the rescued exception via Rails.error.report(handled: true) from both the Solid Queue and other-adapter paths, keeping the non-bubbling behavior and the notification payload. This aligns the recurring path with the gem's default on_thread_error reporting. Fixes rails#746.

Ruby 3.1 reached end-of-life in March 2025. Raise the minimum required Ruby version to 3.2 and remove 3.1 from the CI matrix, together with the exclusions that kept it off Rails >= 8.0. This also lets us unpin rdoc in the Rails 7.1 and 7.2 appraisals, which was held at 6.13 only because rdoc 6.14 dropped Ruby 3.1 compatibility. Apps running Ruby 3.1 will continue to resolve solid_queue 1.4.x, as the resolver honors required_ruby_version. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

`Semaphore#signal` (the single-job path) already caps its increment with `value < limit`, but the batch `signal_all` path — used when releasing concurrency locks in bulk, e.g. when discarding ready jobs — did not. That let a semaphore's value climb above the concurrency limit, effectively leaking permits and admitting more concurrent executions than allowed. Group the jobs by their concurrency limit and cap each group's increment with `value < limit`, matching the single-job path.

Ported from #11, a production-diagnosed fix for a concurrency-limited job whose semaphore count could be permanently corrupted, letting it run well past its configured limit. The problem: a worker stays alive on a long-running job but its database heartbeat fails transiently. Another supervisor prunes it as dead, fails its claimed execution, releases its concurrency lock, and unblocks a replacement — but the original execution keeps running. When it finally returns, its stale `ClaimedExecution`, which exists only in memory, because the DB record was deleted by the supervisor prune, also goes through the semaphore and blocked execution release, potentially violating the limit and allowing more concurrent jobs. With this change, we take a lock on the claimed execution row so that it's required that it still exists and we're the only ones acting over it, before running the finalization code: deleting the claimed execution row, signaling the semaphore and unblocking blocked jobs. In the scenario described above, we'd see the claimed execution record as gone and would do nothing. Note that the semaphore signal + blocked jobs release is still done after the transaction that marks the job as finished/failed, so that these two actions can't rollback a job that finished/failed (see 873760d). Because this can only happen for the ones who got to clear the claimed execution row, we still guarantee the lock is released just once. Co-Authored-By: Brice Burgess <brice.burgess@wealthbox.com> Co-Authored-By: Tom Blais <tom.blais@wealthbox.com>

* Upstream v1.5.0 landed our PR #11 fix (ported as 4f263b2) plus a second fix we didn't have: 2124a51, which guards Semaphore.signal_all from incrementing a semaphore beyond its limit (bulk-release permit leak). * Reconciled ClaimedExecution finalization to upstream's canonical version: it locks the claim row before terminal bookkeeping and signals the semaphore / promotes the next blocked job after commit, exactly once. It calls Job#unblock_next_blocked_job, which the fork's concurrency_controls still defines, so it composes with the concurrency_max_blocked feature. * Kept the fork-only half of PR #11 that upstream did not take: the supervisor-liveness prune guard (process/prunable.rb) and deregister on reap (supervisor/maintenance.rb). These files did not conflict. * Ruby floor rises to >= 3.2 (drops 3.1); the consuming app runs 3.4.8. * Security pins verified intact: rack 3.2.6, nokogiri 1.19.3, puma 7.2.1, concurrent-ruby 1.3.7, rack-session 2.1.2, rails 7.2.3.1. * Batches feature and its schema are unchanged by the merge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

briceburg-wb · 2026-07-23T20:32:15Z

      fail-fast: false
      matrix:
        ruby-version:
-        - 3.1


thoughtless and others added 23 commits July 9, 2026 18:36

docs: use single splat in key argument for limits_concurrency

4546345

Remove trapping QUIT on Windows

dd68a9c

Change default polling_interval to 1 sec

d093f62

Fix recurring task double-enqueue caused by wall-clock race condition

e43f204

Improve DISTINCT queries on PostgreSQL via recursive CTE

e66e435

Automatically use Rails or a default timezone for the schedule

546864f

Rather than have SolidQueue act on the server configured time, this change allows you to use either the Rails application's current timezone or a specified default.

Bump solid_queue to 1.5.0

61a9f08

tblais1224 self-assigned this Jul 23, 2026

tblais1224 changed the title ~~Merge upstream solid_queue v1.5.0 and reconcile with PR #11~~ Merge latest upstream solid_queue v1.5.0 (adopts PR #11 + adds a semaphore guard) Jul 23, 2026

tblais1224 marked this pull request as ready for review July 23, 2026 20:24

tblais1224 requested review from briceburg-wb, jpcamara and treiff July 23, 2026 20:24

briceburg-wb approved these changes Jul 23, 2026

View reviewed changes

Comment thread .github/workflows/main.yml

fail-fast: false

matrix:

ruby-version:

- 3.1

briceburg-wb Jul 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🫗

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Merge latest upstream solid_queue v1.5.0 (adopts PR #11 + adds a semaphore guard)#12

Merge latest upstream solid_queue v1.5.0 (adopts PR #11 + adds a semaphore guard)#12
tblais1224 wants to merge 23 commits into
mainfrom
merge-upstream-v1.5.0

tblais1224 commented Jul 23, 2026 •

edited

Loading

Uh oh!

briceburg-wb Jul 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

17 participants

Uh oh!

Conversation

tblais1224 commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do? (required)

Changes introduced by this bump

🔒 Concurrency & correctness fixes (the main reason for the bump)

⚠️ Behavior changes to be aware of

🧰 Compatibility

⚡ Performance / internal

🛠️ Tooling / DX / misc

Reconciliation with the fork (PR #11 and local changes)

Impact on crm-web (the consumer)

Link to Basecamp to-do, Trello card, New Relic or Honeybadger (required)

QA

What platforms should be included in QA?

QA steps

Screenshots (if appropriate)

Docs

Uh oh!

briceburg-wb Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

17 participants

tblais1224 commented Jul 23, 2026 •

edited

Loading