Skip to content

Simplify the worker and reorganize the execution pools#1

Open
rosa wants to merge 11 commits into
crmne:async-worker-execution-modefrom
rails:fiber-worker-refinements
Open

Simplify the worker and reorganize the execution pools#1
rosa wants to merge 11 commits into
crmne:async-worker-execution-modefrom
rails:fiber-worker-refinements

Conversation

@rosa

@rosa rosa commented Jul 23, 2026

Copy link
Copy Markdown

Hey @crmne! While finishing the review of rails#728 (thanks so much for all the work on it and for your patience! 🙏), I ended up with some changes that go beyond style tweaks, so rather than force-pushing them over your branch directly, here's a PR against it so you can see and weigh in on them:

  • Make worker metadata static again (two commits: first limiting refreshes to heartbeats, then removing the gauge altogether): keeping the inflight gauge fresh required a mutex + dirty-flag in the worker, an extra UPDATE per claiming poll (up to 10/s per worker at the default polling interval), and had the poller and heartbeat threads racing to update the same row. And once refreshed only on heartbeats, the gauge could be stale by up to a heartbeat interval — better not to show it than to show stale info, especially since the exact count is always available from claimed executions. Workers now register pool_type and pool_size once, both constant. With the dirty-tracking gone, the pool callback's only job is waking the worker when capacity frees up, so it gets its old on_idle name back.
  • Start the fiber pool's reactor lazily, on first use: this makes the pool fork-safe by construction — safe to build at any point relative to forking, rather than relying on careful call ordering in the worker — and matches the thread pool, which already builds its executor lazily. The async gem is also required lazily there, so setups without fiber workers never load it. There's a new unit test covering the build-then-fork case, plus an integration test exercising the full supervised fork + fiber worker lifecycle.
  • Simplify worker initialization: with both pools lazy, the worker can build its pool directly in initialize again — no boot override or deferred pool options needed — merge defaults unconditionally, and pick the pool type with a single check. Instantiated directly with no pool options, a worker defaults to threads; with both, fibers win (the configuration forbids that combination anyway).
  • Consolidate validation in Configuration: the worker no longer duplicates the threads + fibers rejection, and the async-dependency and isolation-level checks moved from the fiber pool into Configuration itself, where they're surfaced through valid? and check on every supervised path, before any process is instantiated. Internal classes trust their callers, same as the scheduler does with recurring tasks.
  • Extract SolidQueue::Pool as the pools' base class: both pools shared the capacity ledger (reserve/restore accounting, idle notification, and the reserve-then-schedule shape of post), so that now lives in an abstract Pool that also builds the right pool for a worker via Pool.build, mirroring how Supervisor relates to ForkSupervisor and AsyncSupervisor. Each pool is left with just its scheduling mechanism, and the pool type is derived from the class name. (The old SolidQueue::Pool back-compat alias is removed early in the branch; the name returns here as the actual abstraction — it was internal API with no remaining references.)
  • Report errors escaping the execution future: the thread pool's inline rescue covers job execution, but an error raised while restoring capacity or waking the worker would reject the future with nobody watching — this restores the on_rejection backstop the old pool had. Relatedly, posting now scopes its rescue to the code that runs after capacity is reserved, instead of tracking reservation with a local flag.

Each commit is independently green, so the history bisects cleanly. Merging this into your branch will update the main PR. If you'd rather discuss any of these, happy to!

🤖 Generated with Claude Code

rosa and others added 3 commits July 23, 2026 09:35
Nothing references it anymore, inside or outside the gem: it was
internal API, always instantiated by the worker and never something
apps would configure or subclass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Keeping the inflight gauge fresh on every claim and job completion
required a mutex and dirty-tracking in the worker, an extra metadata
update per claiming poll (up to 10 per second per worker at the
default polling interval), and had the poller and heartbeat threads
racing to update the same row.

The gauge doesn't need that freshness: it's a monitoring convenience,
the exact count is always available from claimed executions, and the
heartbeat already refreshes it with bounded staleness.

With the dirty-tracking gone, the pool callback's only job is waking
the worker when capacity frees up, so it also gets its old name back:
on_idle.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The configuration already validates that workers specify either
threads or fibers but not both, and that's where option validation
belongs; the worker doesn't validate any of its other options.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rosa
rosa force-pushed the fiber-worker-refinements branch from 58fd807 to 29a1b25 Compare July 23, 2026 08:12
rosa and others added 2 commits July 23, 2026 10:38
In the default fork supervisor mode, workers are instantiated by the
supervisor before forking, so a reactor thread started when the pool
is built wouldn't survive the fork. Starting it when the first
execution is posted makes the fiber pool safe to build at any point,
matching the thread pool, which already builds its executor lazily on
first use.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Build the execution pool directly when the worker is initialized,
which is safe now that both pools start their threads lazily. Merge
the worker defaults unconditionally and choose the pool type with a
single check on the fibers option, relying on the configuration's
validation to prevent threads and fibers from being combined.

If a worker is instantiated directly with no pool options, it defaults
to a thread pool; if it somehow gets both options, fibers win.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rosa
rosa force-pushed the fiber-worker-refinements branch from ebb8e18 to 6608b6e Compare July 23, 2026 08:38
@rosa rosa changed the title Simplify worker metadata tracking and remove the Pool alias Simplify the worker and make the fiber pool fork-safe by construction Jul 23, 2026
@rosa
rosa force-pushed the fiber-worker-refinements branch from 29a1b25 to ebb8e18 Compare July 23, 2026 08:47
rosa and others added 2 commits July 23, 2026 11:23
The in-flight gauge was refreshed on heartbeats only, so it could show
counts that were stale by up to a heartbeat interval; better not to
show it than to show stale information, especially given the exact
count is always available from claimed executions. The pool type and
size are constant, and the pool knows both, so the worker can store
them directly when registering, as it did before, and the pools don't
need to provide changing metadata at all.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The rescue inside the future covers job execution, but an error raised
when restoring capacity or waking up the worker would escape it and
reject the future with nobody watching. Attach the on_rejection
callback the old pool had as a backstop, so those errors are also
reported via handle_thread_error.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rosa
rosa force-pushed the fiber-worker-refinements branch from aba14ce to d66bb3f Compare July 23, 2026 09:24
Instead of tracking whether capacity had been reserved with a local
flag, scope the rescue to the code that runs after the reservation,
which is the only part that needs to restore it on failure.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rosa
rosa force-pushed the fiber-worker-refinements branch 2 times, most recently from ff25296 to e6ed969 Compare July 23, 2026 09:42
Both pools share the capacity ledger: the reserve/restore accounting,
the idle notification, and the reserve-then-schedule shape of post.
Move all of that to ExecutionPools::Base, leaving each pool with just
its scheduling mechanism: the thread pool schedules executions on a
Concurrent::ThreadPoolExecutor via schedule, and the fiber pool hands
them to its reactor, adding its fatal-error and shutdown guards on
top of the base's post.

The default way of performing an execution, wrapped in the app
executor, reporting errors and restoring capacity, also lives in the
base class; the fiber pool overrides it to treat Async::Stop as fatal.

The pool type is derived from the class name, and the factory performs
the lookup in reverse, trusting the type it receives: the worker only
ever passes :thread or :fiber.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rosa
rosa force-pushed the fiber-worker-refinements branch from e6ed969 to 2298a9e Compare July 23, 2026 09:45
rosa and others added 2 commits July 23, 2026 11:55
The async dependency and isolation level checks were surfaced through
Configuration's validations but implemented in the fiber pool, which
also ran them when built. Configuration is where these belong: it
validates every supervised path before any process is instantiated,
and reports through valid? and check. The pool now just requires the
async gem lazily when starting its reactor, keeping setups without
fiber workers from ever loading it, and otherwise trusts its caller,
like the rest of the internal classes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the ExecutionPools module with a Pool abstract class that the
thread and fiber pools inherit from, and that builds the right pool
for a worker, mirroring how Supervisor relates to ForkSupervisor and
AsyncSupervisor.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rosa rosa changed the title Simplify the worker and make the fiber pool fork-safe by construction Simplify the worker and reorganize the execution pools Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant