Simplify the worker and reorganize the execution pools#1
Open
rosa wants to merge 11 commits into
Open
Conversation
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
force-pushed
the
fiber-worker-refinements
branch
from
July 23, 2026 08:12
58fd807 to
29a1b25
Compare
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
force-pushed
the
fiber-worker-refinements
branch
from
July 23, 2026 08:38
ebb8e18 to
6608b6e
Compare
rosa
force-pushed
the
fiber-worker-refinements
branch
from
July 23, 2026 08:47
29a1b25 to
ebb8e18
Compare
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
force-pushed
the
fiber-worker-refinements
branch
from
July 23, 2026 09:24
aba14ce to
d66bb3f
Compare
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
force-pushed
the
fiber-worker-refinements
branch
2 times, most recently
from
July 23, 2026 09:42
ff25296 to
e6ed969
Compare
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
force-pushed
the
fiber-worker-refinements
branch
from
July 23, 2026 09:45
e6ed969 to
2298a9e
Compare
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
inflightgauge fresh required a mutex + dirty-flag in the worker, an extraUPDATEper 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 registerpool_typeandpool_sizeonce, 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 oldon_idlename back.initializeagain — nobootoverride 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).threads+fibersrejection, and the async-dependency and isolation-level checks moved from the fiber pool into Configuration itself, where they're surfaced throughvalid?andcheckon every supervised path, before any process is instantiated. Internal classes trust their callers, same as the scheduler does with recurring tasks.SolidQueue::Poolas the pools' base class: both pools shared the capacity ledger (reserve/restore accounting, idle notification, and the reserve-then-schedule shape ofpost), so that now lives in an abstractPoolthat also builds the right pool for a worker viaPool.build, mirroring howSupervisorrelates toForkSupervisorandAsyncSupervisor. Each pool is left with just its scheduling mechanism, and the pool type is derived from the class name. (The oldSolidQueue::Poolback-compat alias is removed early in the branch; the name returns here as the actual abstraction — it was internal API with no remaining references.)on_rejectionbackstop 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