From d66b1c05840dfbd049882d761775e53373b20056 Mon Sep 17 00:00:00 2001
From: Dmitry Gruzd <dgruzd@gitlab.com>
Date: Tue, 2 Jul 2024 15:16:57 +0000
Subject: [PATCH] Apply 2 suggestion(s) to 2 file(s)
Co-authored-by: DANGER_GITLAB_API_TOKEN <project_278964_bot_445da1b60fc7336b6b6776383134d10f@noreply.gitlab.com>
---
.../search/zoekt/scheduling_service.rb | 88 ++++++++++---------
.../ops/zoekt_node_assignment.yml | 9 ++
.../search/zoekt/scheduling_service_spec.rb | 10 +++
3 files changed, 67 insertions(+), 40 deletions(-)
create mode 100644 ee/config/feature_flags/ops/zoekt_node_assignment.yml
diff --git a/ee/app/services/search/zoekt/scheduling_service.rb b/ee/app/services/search/zoekt/scheduling_service.rb
index f9f6dcf76ac19..7eb7aef4b24cc 100644
--- a/ee/app/services/search/zoekt/scheduling_service.rb
+++ b/ee/app/services/search/zoekt/scheduling_service.rb
@@ -15,7 +15,7 @@ class SchedulingService
auto_index_self_managed
].freeze
- BUFFER_FACTOR = ::Gitlab::Saas.feature_available?(:exact_code_search) ? 2 : 3
+ BUFFER_FACTOR = 3
WATERMARK_LIMIT_LOW = 0.7
WATERMARK_LIMIT_HIGH = 0.8
@@ -192,53 +192,61 @@ def remove_expired_subscriptions
end
def node_assignment
- nodes = ::Search::Zoekt::Node.online.find_each.to_a
-
- return false if nodes.empty?
+ return false if Feature.disabled?(:zoekt_node_assignment) # rubocop:disable Gitlab/FeatureFlagWithoutActor -- this is a global feature flag
- zoekt_indices = []
+ execute_every 1.hour, cache_key: :node_assignment do
+ nodes = ::Search::Zoekt::Node.online.find_each.to_a
- EnabledNamespace.with_missing_indices.preload_storage_statistics.find_each do |zoekt_enabled_namespace|
- storage_statistics = zoekt_enabled_namespace.namespace.root_storage_statistics
- unless storage_statistics
- logger.error(build_structured_payload(task: :node_assignment,
- message: "RootStorageStatistics isn't available", zoekt_enabled_namespace_id: zoekt_enabled_namespace.id))
- next
- end
+ break false if nodes.empty?
- space_required = BUFFER_FACTOR * storage_statistics.repository_size
+ zoekt_indices = []
- node = nodes.max_by { |n| n.total_bytes - n.used_bytes }
+ EnabledNamespace.with_missing_indices.preload_storage_statistics.find_each do |zoekt_enabled_namespace|
+ storage_statistics = zoekt_enabled_namespace.namespace.root_storage_statistics
+ unless storage_statistics
+ logger.error(build_structured_payload(
+ task: :node_assignment,
+ message: "RootStorageStatistics isn't available",
+ zoekt_enabled_namespace_id: zoekt_enabled_namespace.id
+ ))
- if (node.used_bytes + space_required) <= node.total_bytes * WATERMARK_LIMIT_LOW
- # TODO: Once we have the task which moves pending to ready then remove the state attribute from here
- # https://gitlab.com/gitlab-org/gitlab/-/issues/439042
+ next
+ end
- zoekt_index = Search::Zoekt::Index.new(
- namespace_id: zoekt_enabled_namespace.root_namespace_id,
- zoekt_node_id: node.id,
- zoekt_enabled_namespace: zoekt_enabled_namespace,
- state: :ready,
- replica: Replica.for_enabled_namespace!(zoekt_enabled_namespace)
- )
- zoekt_indices << zoekt_index
- node.used_bytes += space_required
- else
- logger.error(build_structured_payload(
- task: :node_assignment,
- message: 'Space is not available in Node', zoekt_enabled_namespace_id: zoekt_enabled_namespace.id,
- meta: {
- "zoekt.node_name" => node.metadata['name'],
- "zoekt.node_id" => node.id
- }
- ))
+ space_required = BUFFER_FACTOR * storage_statistics.repository_size
+
+ node = nodes.max_by { |n| n.total_bytes - n.used_bytes }
+
+ if (node.used_bytes + space_required) <= node.total_bytes * WATERMARK_LIMIT_LOW
+ # TODO: Once we have the task which moves pending to ready then remove the state attribute from here
+ # https://gitlab.com/gitlab-org/gitlab/-/issues/439042
+
+ zoekt_index = Search::Zoekt::Index.new(
+ namespace_id: zoekt_enabled_namespace.root_namespace_id,
+ zoekt_node_id: node.id,
+ zoekt_enabled_namespace: zoekt_enabled_namespace,
+ state: :ready,
+ replica: Replica.for_enabled_namespace!(zoekt_enabled_namespace)
+ )
+ zoekt_indices << zoekt_index
+ node.used_bytes += space_required
+ else
+ logger.error(build_structured_payload(
+ task: :node_assignment,
+ message: 'Space is not available in Node', zoekt_enabled_namespace_id: zoekt_enabled_namespace.id,
+ meta: {
+ "zoekt.node_name" => node.metadata['name'],
+ "zoekt.node_id" => node.id
+ }
+ ))
+ end
end
- end
- zoekt_indices.each do |zoekt_index|
- unless zoekt_index.save
- logger.error(build_structured_payload(task: :node_assignment,
- message: 'Could not save Search::Zoekt::Index', zoekt_index: zoekt_index.attributes.compact))
+ zoekt_indices.each do |zoekt_index|
+ unless zoekt_index.save
+ logger.error(build_structured_payload(task: :node_assignment,
+ message: 'Could not save Search::Zoekt::Index', zoekt_index: zoekt_index.attributes.compact))
+ end
end
end
end
diff --git a/ee/config/feature_flags/ops/zoekt_node_assignment.yml b/ee/config/feature_flags/ops/zoekt_node_assignment.yml
new file mode 100644
index 0000000000000..eecceebb864eb
--- /dev/null
+++ b/ee/config/feature_flags/ops/zoekt_node_assignment.yml
@@ -0,0 +1,9 @@
+---
+name: zoekt_node_assignment
+feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/470044
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/158044
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/470149
+milestone: '17.2'
+group: group::global search
+type: ops
+default_enabled: true
diff --git a/ee/spec/services/search/zoekt/scheduling_service_spec.rb b/ee/spec/services/search/zoekt/scheduling_service_spec.rb
index 47e6ae4fb9c7d..b068c4d019112 100644
--- a/ee/spec/services/search/zoekt/scheduling_service_spec.rb
+++ b/ee/spec/services/search/zoekt/scheduling_service_spec.rb
@@ -208,6 +208,16 @@
let_it_be(:namespace_statistics) { create(:namespace_root_storage_statistics, repository_size: 1000) }
let_it_be(:namespace_with_statistics) { create(:group, root_storage_statistics: namespace_statistics) }
+ context 'when feature flag is disabled' do
+ before do
+ stub_feature_flags(zoekt_node_assignment: false)
+ end
+
+ it 'returns false' do
+ expect(execute_task).to eq(false)
+ end
+ end
+
context 'when some zoekt enabled namespaces missing zoekt index' do
let(:logger) { instance_double(::Search::Zoekt::Logger) }
let_it_be(:zkt_enabled_namespace) { create(:zoekt_enabled_namespace, namespace: namespace.root_ancestor) }
--
GitLab