From 7a90836369b4fe1b1f0ee139b68b96d7403d57cb Mon Sep 17 00:00:00 2001
From: John Mason <9717668-johnmason@users.noreply.gitlab.com>
Date: Wed, 16 Oct 2024 19:06:28 +0000
Subject: [PATCH] Add feature flag for zoekt index circuit breakers
Until we have index level eviction logic, we don't want to skip indexing
when an index watermark is exceeded.
Changelog: changed
EE: true
---
.../search/zoekt/indexing_task_service.rb | 22 +++++++++++------
.../ops/zoekt_index_circuit_breaker.yml | 9 +++++++
.../zoekt/indexing_task_service_spec.rb | 24 +++++++++++++++++++
3 files changed, 48 insertions(+), 7 deletions(-)
create mode 100644 ee/config/feature_flags/ops/zoekt_index_circuit_breaker.yml
diff --git a/ee/app/services/search/zoekt/indexing_task_service.rb b/ee/app/services/search/zoekt/indexing_task_service.rb
index f6080c11e0393..ace02b490f67d 100644
--- a/ee/app/services/search/zoekt/indexing_task_service.rb
+++ b/ee/app/services/search/zoekt/indexing_task_service.rb
@@ -28,13 +28,7 @@ def execute
current_task_type = random_force_reindexing? ? :force_index_repo : task_type
Router.fetch_indices_for_indexing(project_id, root_namespace_id: root_namespace_id).find_each do |idx|
- # Note: we skip indexing tasks depending on storage watermark levels.
- #
- # If the low watermark is exceeded, we don't allow any new initial indexing tasks through,
- # but we permit incremental indexing or force reindexing for existing repos.
- #
- # If the high watermark is exceeded, we don't allow any indexing tasks at all anymore.
- if idx.high_watermark_exceeded? || (idx.low_watermark_exceeded? && initial_indexing?)
+ if index_circuit_breaker_enabled? && index_circuit_broken?(idx)
IndexingTaskWorker.perform_in(WATERMARK_RESCHEDULE_INTERVAL, project_id, task_type, { index_id: idx.id })
logger.info(
build_structured_payload(
@@ -92,6 +86,20 @@ def random_force_reindexing?
def eligible_for_force_reindexing?
task_type == :index_repo && Feature.enabled?(:zoekt_random_force_reindexing, project, type: :ops)
end
+
+ def index_circuit_broken?(idx)
+ # Note: we skip indexing tasks depending on storage watermark levels.
+ #
+ # If the low watermark is exceeded, we don't allow any new initial indexing tasks through,
+ # but we permit incremental indexing or force reindexing for existing repos.
+ #
+ # If the high watermark is exceeded, we don't allow any indexing tasks at all anymore.
+ idx.high_watermark_exceeded? || (idx.low_watermark_exceeded? && initial_indexing?)
+ end
+
+ def index_circuit_breaker_enabled?
+ Feature.enabled?(:zoekt_index_circuit_breaker, ::Project.actor_from_id(project_id), type: :ops)
+ end
end
end
end
diff --git a/ee/config/feature_flags/ops/zoekt_index_circuit_breaker.yml b/ee/config/feature_flags/ops/zoekt_index_circuit_breaker.yml
new file mode 100644
index 0000000000000..6a612ec0f27d3
--- /dev/null
+++ b/ee/config/feature_flags/ops/zoekt_index_circuit_breaker.yml
@@ -0,0 +1,9 @@
+---
+name: zoekt_index_circuit_breaker
+feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/494260
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/168566
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/498316
+milestone: '17.5'
+group: group::global search
+type: ops
+default_enabled: false
diff --git a/ee/spec/services/search/zoekt/indexing_task_service_spec.rb b/ee/spec/services/search/zoekt/indexing_task_service_spec.rb
index 3c8e7ad3ee001..7fed1356508c9 100644
--- a/ee/spec/services/search/zoekt/indexing_task_service_spec.rb
+++ b/ee/spec/services/search/zoekt/indexing_task_service_spec.rb
@@ -20,6 +20,18 @@
end
describe '#execute' do
+ RSpec.shared_examples 'creates a task when circuit breaker is disabled' do
+ context 'with index circuit breaker feature flag disabled' do
+ before do
+ stub_feature_flags(zoekt_index_circuit_breaker: false)
+ end
+
+ it 'creates Search::Zoekt::Task record' do
+ expect { service.execute }.to change { Search::Zoekt::Task.count }.by(1)
+ end
+ end
+ end
+
context 'when a watermark is exceeded' do
let(:service) { described_class.new(project.id, task_type) }
let(:task_type) { :index_repo }
@@ -49,6 +61,8 @@
service.execute
end
+
+ it_behaves_like 'creates a task when circuit breaker is disabled'
end
context 'with force reindexing' do
@@ -67,6 +81,8 @@
service.execute
end
+
+ it_behaves_like 'creates a task when circuit breaker is disabled'
end
context 'when a repo already exists' do
@@ -75,6 +91,8 @@
create(:zoekt_repository, project: project, zoekt_index: zoekt_index, state: repo_state)
end
+ it_behaves_like 'creates a task when circuit breaker is disabled'
+
context 'and is ready' do
let_it_be(:repo_state) { ::Search::Zoekt::Repository.states.fetch(:ready) }
@@ -90,6 +108,8 @@
service.execute
end
+
+ it_behaves_like 'creates a task when circuit breaker is disabled'
end
context 'and is not ready' do
@@ -107,6 +127,8 @@
service.execute
end
+
+ it_behaves_like 'creates a task when circuit breaker is disabled'
end
end
end
@@ -131,6 +153,8 @@
it 'does not create Search::Zoekt::Task record' do
expect { service.execute }.not_to change { Search::Zoekt::Task.count }
end
+
+ it_behaves_like 'creates a task when circuit breaker is disabled'
end
end
--
GitLab