Commit fd2e0dfa authored by Matthias Käppler's avatar Matthias Käppler Committed by Heinrich Lee Yu

Run SidekiqExporter only on first worker

parent 16a9ff55
......@@ -28,12 +28,16 @@ Gitlab::Application.configure do |config|
config.middleware.insert_after(Labkit::Middleware::Rack, Gitlab::Metrics::RequestsRackMiddleware)
end
Sidekiq.configure_server do |config|
if Gitlab::Runtime.sidekiq? && (!ENV['SIDEKIQ_WORKER_ID'] || ENV['SIDEKIQ_WORKER_ID'] == '0')
# The single worker outside of a sidekiq-cluster, or the first worker (sidekiq_0)
# in a cluster of processes, is responsible for serving health checks.
Sidekiq.configure_server do |config|
config.on(:startup) do
# Do not clean the metrics directory here - the supervisor script should
# have already taken care of that
Gitlab::Metrics::Exporter::SidekiqExporter.instance.start
end
end
end
if !Rails.env.test? && Gitlab::Metrics.prometheus_metrics_enabled?
......
......@@ -15,29 +15,6 @@ module Gitlab
File::NULL
end
end
private
# Sidekiq Exporter does not work properly in sidekiq-cluster
# mode. It tries to start the service on the same port for
# each of the cluster workers, this results in failure
# due to duplicate binding.
#
# For now we ignore this error, as metrics are still "kind of"
# valid as they are rendered from shared directory.
#
# Issue: https://gitlab.com/gitlab-org/gitlab/issues/5714
def start_working
super
rescue Errno::EADDRINUSE => e
Sidekiq.logger.error(
class: self.class.to_s,
message: 'Cannot start sidekiq_exporter',
'exception.message' => e.message
)
false
end
end
end
end
......
......@@ -50,40 +50,4 @@ RSpec.describe Gitlab::Metrics::Exporter::SidekiqExporter do
expect(exporter.log_filename).to end_with('sidekiq_exporter.log')
end
end
context 'when port is already taken' do
let(:first_exporter) { described_class.new }
before do
stub_config(
monitoring: {
sidekiq_exporter: {
enabled: true,
port: 9992,
address: '127.0.0.1'
}
}
)
first_exporter.start
end
after do
first_exporter.stop
end
it 'does print error message' do
expect(Sidekiq.logger).to receive(:error)
.with(
class: described_class.to_s,
message: 'Cannot start sidekiq_exporter',
'exception.message' => anything)
exporter.start
end
it 'does not start thread' do
expect(exporter.start).to be_nil
end
end
end
......@@ -7,19 +7,26 @@ require_relative '../../sidekiq_cluster/sidekiq_cluster'
RSpec.describe Gitlab::SidekiqCluster do # rubocop:disable RSpec/FilePath
describe '.start' do
it 'starts Sidekiq with the given queues, environment and options' do
expected_options = {
env: :production,
directory: 'foo/bar',
max_concurrency: 20,
min_concurrency: 10,
timeout: 25,
dryrun: true
process_options = {
pgroup: true,
err: $stderr,
out: $stdout
}
expect(described_class).to receive(:start_sidekiq).ordered.with(%w(foo), expected_options.merge(worker_id: 0))
expect(described_class).to receive(:start_sidekiq).ordered.with(%w(bar baz), expected_options.merge(worker_id: 1))
described_class.start([%w(foo), %w(bar baz)], env: :production, directory: 'foo/bar', max_concurrency: 20, min_concurrency: 10, dryrun: true)
expect(Process).to receive(:spawn).ordered.with({
"ENABLE_SIDEKIQ_CLUSTER" => "1",
"SIDEKIQ_WORKER_ID" => "0"
},
"bundle", "exec", "sidekiq", "-c10", "-eproduction", "-t25", "-gqueues:foo", "-rfoo/bar", "-qfoo,1", process_options
)
expect(Process).to receive(:spawn).ordered.with({
"ENABLE_SIDEKIQ_CLUSTER" => "1",
"SIDEKIQ_WORKER_ID" => "1"
},
"bundle", "exec", "sidekiq", "-c10", "-eproduction", "-t25", "-gqueues:bar,baz", "-rfoo/bar", "-qbar,1", "-qbaz,1", process_options
)
described_class.start([%w(foo), %w(bar baz)], env: :production, directory: 'foo/bar', max_concurrency: 20, min_concurrency: 10)
end
it 'starts Sidekiq with the given queues and sensible default options' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment