Skip to content

Commit

Permalink
Ensure "shutdown?" behavior is consistent between J Ruby and C Ruby
Browse files Browse the repository at this point in the history
  • Loading branch information
bensheldon committed Feb 29, 2024
1 parent 3933005 commit bc12050
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 10 deletions.
9 changes: 6 additions & 3 deletions lib/good_job/scheduler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,11 @@ def initialize(performer, max_threads: nil, max_cache: nil, warm_cache_on_initia
# @return [Boolean, nil]
delegate :running?, to: :executor, allow_nil: true

# Tests whether the scheduler is shutdown.
# Tests whether the scheduler is shutdown and no tasks are running.
# @return [Boolean, nil]
delegate :shutdown?, to: :executor, allow_nil: true
def shutdown?
@executor.nil? || (executor.shutdown? && !executor.shuttingdown?)
end

# Shut down the scheduler.
# This stops all threads in the thread pool.
Expand All @@ -85,7 +87,7 @@ def initialize(performer, max_threads: nil, max_cache: nil, warm_cache_on_initia
# * A positive number will wait that many seconds before stopping any remaining active tasks.
# @return [void]
def shutdown(timeout: -1)
return if executor.nil? || executor.shutdown?
return if executor.nil? || (executor.shutdown? && !executor.shuttingdown?)

instrument("scheduler_shutdown_start", { timeout: timeout })
instrument("scheduler_shutdown", { timeout: timeout }) do
Expand All @@ -100,6 +102,7 @@ def shutdown(timeout: -1)
unless executor.wait_for_termination(executor_wait)
instrument("scheduler_shutdown_kill", { active_job_ids: @performer.performing_active_job_ids.to_a })
executor.kill
executor.wait_for_termination
end
end
end
Expand Down
15 changes: 8 additions & 7 deletions lib/good_job/shared_executor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,10 @@ def running?
@executor&.running?
end

# Tests whether the scheduler is shutdown and no tasks are running.
# @return [Boolean, nil]
def shutdown?
if @executor
@executor.shutdown?
else
true
end
@executor.nil? || (@executor.shutdown? && !@executor.shuttingdown?)
end

# Shut down the SharedExecutor.
Expand All @@ -38,13 +36,16 @@ def shutdown?
# * A positive number will wait that many seconds before stopping any remaining active threads.
# @return [void]
def shutdown(timeout: -1)
return if @executor.nil? || @executor.shutdown?
return if @executor.nil? || (@executor.shutdown? && !@executor.shuttingdown?)

@executor.shutdown if @executor.running?

if @executor.shuttingdown? && timeout # rubocop:disable Style/GuardClause
executor_wait = timeout.negative? ? nil : timeout
@executor.kill unless @executor.wait_for_termination(executor_wait)
unless @executor.wait_for_termination(executor_wait)
@executor.kill
@executor.wait_for_termination
end
end
end

Expand Down

0 comments on commit bc12050

Please sign in to comment.