feat: implement worker supervisor for process management - #407
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a supervisor-based, multi-process worker runtime (Sidekiq-style) with CLI-configured concurrency, coordinated signal handling, and updated docs/tests to match the new model.
Changes:
- Added
Karya::WorkerSupervisorto fork/manage child worker processes and coordinate shutdown/drain/force-stop. - Updated
karya workerCLI to start the supervisor (with--concurrency) and to own signal trapping/restoration. - Extended worker behavior and test coverage for shutdown/drain semantics and supervisor lifecycle.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/pages/runtime/workers.md | Documents the new supervisor-owned process model and shutdown behavior. |
| core/karya/spec/karya/worker_supervisor_spec.rb | Adds coverage for supervisor child management, drain/force-stop, and runtime hooks. |
| core/karya/spec/karya/worker_spec.rb | Adds shutdown/drain behavior tests and validates signal_subscriber. |
| core/karya/spec/karya/cli_spec.rb | Updates CLI expectations to construct/run WorkerSupervisor, adds concurrency and signal subscription tests. |
| core/karya/lib/karya/worker_supervisor.rb | Implements the supervisor, runtime hooks, and configuration validation. |
| core/karya/lib/karya/worker.rb | Adds signal-driven drain/force-stop behavior and reservation-release-on-shutdown semantics. |
| core/karya/lib/karya/cli.rb | Wires CLI options to supervisor + adds signal subscription helper and header suppression. |
| core/karya/lib/karya.rb | Requires the new supervisor implementation. |
| core/karya/README.md | Updates documentation to describe supervisor-based concurrency and shutdown behavior. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 12 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
core/karya/lib/karya/cli.rb:40
CLI#versioncurrently relies on the header being printed (it immediatelyexit(0)without output), butCLI.startcan now suppress the header viasuppress_header. Withsuppress_header: true,karya version/--versionwill produce no output despite the command description saying it prints the version. Consider printing the version explicitly inversion, or only suppressing the ASCII header while still emitting a minimal version line when this command is invoked.
def self.start(given_args = ARGV, config = {})
puts header unless config[:suppress_header]
super
end
def self.header
art = <<~'TEXT'
_ __ _ ____ __ __ _
| |/ / / \ | _ \ \ \ / / / \
| ' / / _ \ | |_) | \ V / / _ \
| . \ / ___ \ | _ < | | / ___ \
|_|\_\ /_/ \_\|_| \_\ |_| /_/ \_\
TEXT
"#{art}\n#{Karya::TAGLINE} · v#{Karya::VERSION}\n"
end
map %w[--help -h] => :help
map %w[--version -v] => :version
desc 'version', 'Print the current version'
def version
# version is printed in the header, so we can just exit here
exit(0)
end
- Changed direct method calls to use `send` for state manager methods to enhance encapsulation. - Added a new StateQueries module for internal state query and value helpers. - Updated state manager specs to reflect changes in method access.
- Changed error messages to use "unknown keyword options" for clarity. - Updated method signatures to accept keyword arguments using double splat (**). - Enhanced tests to reflect the updated error messages and keyword argument handling.
- Updated signal subscriber handling in runtime and child process runner to ensure that false values are rejected. - Enhanced error messages for clarity when invalid signal subscribers are provided. - Added tests to validate behavior for false signal subscriber restorers.
- Introduced UNSET constant to manage default values for options. - Updated Worker and WorkerSupervisor classes to use UNSET for instrumenter, logger, and sleeper options. - Added validation methods for logger and ensured they respond to required methods. - Enhanced tests to reject false values for instrumenter, logger, and forker in both Worker and WorkerSupervisor runtimes.
- Updated the minimum coverage configuration for SimpleCov to explicitly define line and branch coverage as zero for forked processes. - This change enhances clarity and ensures consistent coverage reporting across different process executions.
- Added a test to ensure blocking helper reaping continues after encountering unknown waited child PIDs. - Validates that the child PIDs are emptied correctly and that the process alive check is performed.
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.
Karya::WorkerSupervisorto manage child worker processes.closes: #15