diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index d1b734fb4bccdbb641bf0fca74ad4413a7e4bcf5..c68ba000e04435f961f08d3bff258a8738179353 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -83,6 +83,10 @@ def enable_semi_reliable_fetch_mode? end end + config.on(:shutdown) do + Gitlab::Cluster::LifecycleEvents.do_worker_stop + end + if enable_reliable_fetch? config.options[:semi_reliable_fetch] = enable_semi_reliable_fetch_mode? Sidekiq::ReliableFetch.setup_reliable_fetch!(config) diff --git a/config/puma.example.development.rb b/config/puma.example.development.rb index 3164ffe3ef42fb5126336347324a6e39e4985c84..71a4e9b36f1e21b54b3b5eb332f943e83ff69f1a 100644 --- a/config/puma.example.development.rb +++ b/config/puma.example.development.rb @@ -67,6 +67,11 @@ Gitlab::Cluster::LifecycleEvents.do_worker_start end +on_worker_shutdown do + # Signal application hooks that a worker is shutting down + Gitlab::Cluster::LifecycleEvents.do_worker_stop +end + # Preload the application before starting the workers; this conflicts with # phased restart feature. (off by default) diff --git a/config/puma.rb.example b/config/puma.rb.example index c70baf6570e60eec05ef7093fffeb64c5c403867..59844b4aecffe16b1ef58714f870cc00b5361776 100644 --- a/config/puma.rb.example +++ b/config/puma.rb.example @@ -57,6 +57,11 @@ on_worker_boot do Gitlab::Cluster::LifecycleEvents.do_worker_start end +on_worker_shutdown do + # Signal application hooks that a worker is shutting down + Gitlab::Cluster::LifecycleEvents.do_worker_stop +end + # Preload the application before starting the workers; this conflicts with # phased restart feature. (off by default) preload_app! diff --git a/lib/gitlab/cluster/lifecycle_events.rb b/lib/gitlab/cluster/lifecycle_events.rb index be08ada9d2f610ae55db253be9fc97c79c90d693..b39d2a02f0285ce7c628d31280b6055fc901c82b 100644 --- a/lib/gitlab/cluster/lifecycle_events.rb +++ b/lib/gitlab/cluster/lifecycle_events.rb @@ -63,6 +63,15 @@ module Cluster # # Sidekiq/Puma Single: This is called immediately. # + # - on_worker_stop (on worker process): + # + # Puma Cluster: Called in the worker process + # exactly once after it stops processing requests + # but before it shuts down. + # + # Sidekiq: Called after the scheduler shuts down but + # before the worker finishes ongoing jobs. + # # Blocks will be executed in the order in which they are registered. # class LifecycleEvents @@ -113,6 +122,10 @@ def on_master_start(&block) end end + def on_worker_stop(&block) + (@worker_stop_hooks ||= []) << block + end + # # Lifecycle integration methods (called from puma.rb, etc.) # @@ -137,6 +150,10 @@ def do_before_master_restart call(:master_restart_hooks, @master_restart_hooks) end + def do_worker_stop + call(:worker_stop_hooks, @worker_stop_hooks) + end + # DEPRECATED alias_method :do_master_restart, :do_before_master_restart diff --git a/spec/lib/gitlab/cluster/lifecycle_events_spec.rb b/spec/lib/gitlab/cluster/lifecycle_events_spec.rb index 5eea78acd98cc5e996bac5473116968707ce2884..45becb8370c985953c10784cd2fed5e1199978db 100644 --- a/spec/lib/gitlab/cluster/lifecycle_events_spec.rb +++ b/spec/lib/gitlab/cluster/lifecycle_events_spec.rb @@ -3,38 +3,55 @@ require 'spec_helper' RSpec.describe Gitlab::Cluster::LifecycleEvents do + using RSpec::Parameterized::TableSyntax + # we create a new instance to ensure that we do not touch existing hooks let(:replica) { Class.new(described_class) } - context 'hooks execution' do - using RSpec::Parameterized::TableSyntax + before do + # disable blackout period to speed-up tests + stub_config(shutdown: { blackout_seconds: 0 }) + end - where(:method, :hook_names) do - :do_worker_start | %i[worker_start_hooks] - :do_before_fork | %i[before_fork_hooks] - :do_before_graceful_shutdown | %i[master_blackout_period master_graceful_shutdown] - :do_before_master_restart | %i[master_restart_hooks] + context 'outside of clustered environments' do + where(:hook, :was_executed_immediately) do + :on_worker_start | true + :on_before_fork | false + :on_before_graceful_shutdown | false + :on_before_master_restart | false + :on_worker_stop | false end - before do - # disable blackout period to speed-up tests - stub_config(shutdown: { blackout_seconds: 0 }) + with_them do + it 'executes the given block immediately' do + was_executed = false + replica.public_send(hook, &proc { was_executed = true }) + + expect(was_executed).to eq(was_executed_immediately) + end end + end - with_them do - subject { replica.public_send(method) } + context 'in clustered environments' do + before do + allow(Gitlab::Runtime).to receive(:puma?).and_return(true) + replica.set_puma_options(workers: 2) + end - it 'executes all hooks' do - hook_names.each do |hook_name| - hook = double - replica.instance_variable_set(:"@#{hook_name}", [hook]) + where(:hook, :execution_helper) do + :on_worker_start | :do_worker_start + :on_before_fork | :do_before_fork + :on_before_graceful_shutdown | :do_before_graceful_shutdown + :on_before_master_restart | :do_before_master_restart + :on_worker_stop | :do_worker_stop + end - # ensure that proper hooks are called - expect(hook).to receive(:call) - expect(replica).to receive(:call).with(hook_name, anything).and_call_original - end + with_them do + it 'requires explicit execution via do_* helper' do + was_executed = false + replica.public_send(hook, &proc { was_executed = true }) - subject + expect { replica.public_send(execution_helper) }.to change { was_executed }.from(false).to(true) end end end