Commit 0e31ff0c authored by Dylan Griffith's avatar Dylan Griffith

Dont use DB connection in GlobalSearchSampler

As discovered in https://gitlab.com/gitlab-org/gitlab/-/issues/231248
the `GlobalSearchSampler` which invokes this
`Elastic::MetricsUpdateService` is using an extra DB connection since
it runs in it's own thread. This extra DB connection is not accounted
for and can cause issues with the connection pool.

We could account for this extra connection but it turns out the 2
database queries happening in here aren't really necessary anyway. The
first checks if the Elasticsearch integration is enabled. Skipping this
is safe since it will just set `0` for these metrics which is not a
problem. The 2nd checks to see if prometheus monitoring is enabled. But
this check was already happening before spawning the
`GlobalSearchSampler` in
[`config/initializers/7_prometheus_metrics.rb`](
https://gitlab.com/gitlab-org/gitlab/-/blob/17ab3d1634f5f1ca82334f270cf8239d7170b0a2/config/initializers/7_prometheus_metrics.rb#L41
) so we can safely skip that too.

The only other things happening now in this class are requests to Redis
and don't require a DB connection.
parent 17ab3d16
---
title: Dont use DB connection in GlobalSearchSampler
merge_request: 38138
author:
type: fixed
...@@ -3,9 +3,6 @@ ...@@ -3,9 +3,6 @@
module Elastic module Elastic
class MetricsUpdateService class MetricsUpdateService
def execute def execute
return unless elasticsearch_enabled?
return unless prometheus_enabled?
incremental_gauge = Gitlab::Metrics.gauge(:global_search_bulk_cron_queue_size, 'Number of incremental database updates waiting to be synchronized to Elasticsearch', {}, :max) incremental_gauge = Gitlab::Metrics.gauge(:global_search_bulk_cron_queue_size, 'Number of incremental database updates waiting to be synchronized to Elasticsearch', {}, :max)
incremental_gauge.set({}, Elastic::ProcessBookkeepingService.queue_size) incremental_gauge.set({}, Elastic::ProcessBookkeepingService.queue_size)
...@@ -15,15 +12,5 @@ module Elastic ...@@ -15,15 +12,5 @@ module Elastic
awaiting_indexing_gauge = Gitlab::Metrics.gauge(:global_search_awaiting_indexing_queue_size, 'Number of database updates waiting to be synchronized to Elasticsearch while indexing is paused.', {}, :max) awaiting_indexing_gauge = Gitlab::Metrics.gauge(:global_search_awaiting_indexing_queue_size, 'Number of database updates waiting to be synchronized to Elasticsearch while indexing is paused.', {}, :max)
awaiting_indexing_gauge.set({}, Elastic::IndexingControlService.queue_size) awaiting_indexing_gauge.set({}, Elastic::IndexingControlService.queue_size)
end end
private
def elasticsearch_enabled?
::Gitlab::CurrentSettings.elasticsearch_indexing?
end
def prometheus_enabled?
::Gitlab::Metrics.prometheus_metrics_enabled?
end
end end
end end
...@@ -5,11 +5,6 @@ require 'spec_helper' ...@@ -5,11 +5,6 @@ require 'spec_helper'
RSpec.describe Elastic::MetricsUpdateService, :prometheus do RSpec.describe Elastic::MetricsUpdateService, :prometheus do
subject { described_class.new } subject { described_class.new }
before do
stub_ee_application_setting(elasticsearch_indexing: true)
allow(Gitlab::Metrics).to receive(:prometheus_metrics_enabled?).and_return(true)
end
describe '#execute' do describe '#execute' do
it 'sets gauges' do it 'sets gauges' do
expect(Elastic::ProcessBookkeepingService).to receive(:queue_size).and_return(4) expect(Elastic::ProcessBookkeepingService).to receive(:queue_size).and_return(4)
...@@ -37,29 +32,5 @@ RSpec.describe Elastic::MetricsUpdateService, :prometheus do ...@@ -37,29 +32,5 @@ RSpec.describe Elastic::MetricsUpdateService, :prometheus do
subject.execute subject.execute
end end
context 'when prometheus metrics is disabled' do
before do
allow(Gitlab::Metrics).to receive(:prometheus_metrics_enabled?).and_return(false)
end
it 'does not set a gauge' do
expect(Gitlab::Metrics).not_to receive(:gauge)
subject.execute
end
end
context 'when elasticsearch indexing and search is disabled' do
before do
stub_ee_application_setting(elasticsearch_indexing: false)
end
it 'does not set a gauge' do
expect(Gitlab::Metrics).not_to receive(:gauge)
subject.execute
end
end
end end
end end
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