From 1113831c15c11d671271f2182cdcf7a62d2682b3 Mon Sep 17 00:00:00 2001
From: Matthias Kaeppler <mkaeppler@gitlab.com>
Date: Tue, 8 Nov 2022 16:41:33 +0100
Subject: [PATCH] Add lifecycle hook for worker stop event
This will be used soon in order to produce
heap dumps when a worker was terminated due
to high memory use.
---
config/initializers/sidekiq.rb | 4 ++
config/puma.example.development.rb | 5 ++
config/puma.rb.example | 5 ++
lib/gitlab/cluster/lifecycle_events.rb | 17 ++++++
.../gitlab/cluster/lifecycle_events_spec.rb | 59 ++++++++++++-------
5 files changed, 69 insertions(+), 21 deletions(-)
diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb
index d1b734fb4bccd..c68ba000e0443 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 3164ffe3ef42f..71a4e9b36f1e2 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 c70baf6570e60..59844b4aecffe 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 be08ada9d2f61..b39d2a02f0285 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 5eea78acd98cc..45becb8370c98 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
--
GitLab