Commit 9b75e1e0 authored by Nick Thomas's avatar Nick Thomas

Merge branch '208717-remove-ff-for-bulk-indexer' into 'master'

Remove feature flag elastic_bulk_incremental_updates

See merge request gitlab-org/gitlab!27293
parents 068f6983 d0524d47
...@@ -36,11 +36,7 @@ Additionally, if you need large repos or multiple forks for testing, please cons ...@@ -36,11 +36,7 @@ Additionally, if you need large repos or multiple forks for testing, please cons
The Elasticsearch integration depends on an external indexer. We ship an [indexer written in Go](https://gitlab.com/gitlab-org/gitlab-elasticsearch-indexer). The user must trigger the initial indexing via a rake task but, after this is done, GitLab itself will trigger reindexing when required via `after_` callbacks on create, update, and destroy that are inherited from [/ee/app/models/concerns/elastic/application_versioned_search.rb](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/app/models/concerns/elastic/application_versioned_search.rb). The Elasticsearch integration depends on an external indexer. We ship an [indexer written in Go](https://gitlab.com/gitlab-org/gitlab-elasticsearch-indexer). The user must trigger the initial indexing via a rake task but, after this is done, GitLab itself will trigger reindexing when required via `after_` callbacks on create, update, and destroy that are inherited from [/ee/app/models/concerns/elastic/application_versioned_search.rb](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/app/models/concerns/elastic/application_versioned_search.rb).
After initial indexing is complete, updates proceed in one of two ways, depending on the `:elastic_bulk_incremental_updates` feature flag. After initial indexing is complete, create, update, and delete operations for all models except projects (see [#207494](https://gitlab.com/gitlab-org/gitlab/issues/207494)) are tracked in a Redis [`ZSET`](https://redis.io/topics/data-types#sorted-sets). A regular `sidekiq-cron` `ElasticIndexBulkCronWorker` processes this queue, updating many Elasticsearch documents at a time with the [Bulk Request API](https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-bulk.html).
If disabled, every create, update, or delete operation on an Elasticsearch-tracked model enqueues a new `ElasticIndexerWorker` Sidekiq job which takes care of updating just that document. This is quite inefficient.
If the feature flag is enabled, create, update, and delete operations for all models except projects (see [#207494](https://gitlab.com/gitlab-org/gitlab/issues/207494)) are tracked in a Redis [`ZSET`](https://redis.io/topics/data-types#sorted-sets) instead. A regular `sidekiq-cron` `ElasticIndexBulkCronWorker` processes this queue, updating many Elasticsearch documents at a time with the [Bulk Request API](https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-bulk.html).
Search queries are generated by the concerns found in [ee/app/models/concerns/elastic](https://gitlab.com/gitlab-org/gitlab/tree/master/ee/app/models/concerns/elastic). These concerns are also in charge of access control, and have been a historic source of security bugs so please pay close attention to them! Search queries are generated by the concerns found in [ee/app/models/concerns/elastic](https://gitlab.com/gitlab-org/gitlab/tree/master/ee/app/models/concerns/elastic). These concerns are also in charge of access control, and have been a historic source of security bugs so please pay close attention to them!
......
...@@ -45,36 +45,15 @@ module Elastic ...@@ -45,36 +45,15 @@ module Elastic
end end
def maintain_elasticsearch_create def maintain_elasticsearch_create
return if maintain_elasticsearch_incremental_bulk ::Elastic::ProcessBookkeepingService.track!(self)
ElasticIndexerWorker.perform_async(:index, self.class.to_s, self.id, self.es_id)
end end
def maintain_elasticsearch_update def maintain_elasticsearch_update
return if maintain_elasticsearch_incremental_bulk ::Elastic::ProcessBookkeepingService.track!(self)
ElasticIndexerWorker.perform_async(
:update,
self.class.to_s,
self.id,
self.es_id
)
end end
def maintain_elasticsearch_destroy def maintain_elasticsearch_destroy
return if maintain_elasticsearch_incremental_bulk
ElasticIndexerWorker.perform_async(
:delete, self.class.to_s, self.id, self.es_id, es_parent: self.es_parent
)
end
def maintain_elasticsearch_incremental_bulk
return false unless Feature.enabled?(:elastic_bulk_incremental_updates, self.project)
::Elastic::ProcessBookkeepingService.track!(self) ::Elastic::ProcessBookkeepingService.track!(self)
true
end end
class_methods do class_methods do
......
...@@ -19,12 +19,20 @@ module Elastic ...@@ -19,12 +19,20 @@ module Elastic
::Gitlab::CurrentSettings.elasticsearch_indexes_project?(self) ::Gitlab::CurrentSettings.elasticsearch_indexes_project?(self)
end end
def maintain_elasticsearch_incremental_bulk
# TODO: ElasticIndexerWorker does extra work for project hooks, so we # TODO: ElasticIndexerWorker does extra work for project hooks, so we
# can't use the incremental-bulk indexer for projects yet. # can't use the incremental-bulk indexer for projects yet.
# #
# https://gitlab.com/gitlab-org/gitlab/issues/207494 # https://gitlab.com/gitlab-org/gitlab/issues/207494
false def maintain_elasticsearch_create
ElasticIndexerWorker.perform_async(:index, self.class.to_s, self.id, self.es_id)
end
def maintain_elasticsearch_update
ElasticIndexerWorker.perform_async(:update, self.class.to_s, self.id, self.es_id)
end
def maintain_elasticsearch_destroy
ElasticIndexerWorker.perform_async(:delete, self.class.to_s, self.id, self.es_id, es_parent: self.es_parent)
end end
def each_indexed_association def each_indexed_association
......
---
title: Remove feature flag elastic_bulk_incremental_updates
merge_request: 27293
author:
type: performance
...@@ -106,21 +106,7 @@ describe Note, :elastic do ...@@ -106,21 +106,7 @@ describe Note, :elastic do
expect(note.__elasticsearch__.as_indexed_json).to eq(expected_hash) expect(note.__elasticsearch__.as_indexed_json).to eq(expected_hash)
end end
it "does not create ElasticIndexerWorker job for system messages" do it 'does not track system note updates' do
stub_feature_flags(elastic_bulk_incremental_updates: false)
project = create :project, :repository
# We have to set one minute delay because of https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/15682
issue = create :issue, project: project, updated_at: 1.minute.ago
# Only issue should be updated
expect(ElasticIndexerWorker).to receive(:perform_async).with(:update, 'Issue', anything, anything)
create :note, :system, project: project, noteable: issue
end
it 'does not track system note updates via the bulk updater' do
stub_feature_flags(elastic_bulk_incremental_updates: true)
note = create(:note, :system) note = create(:note, :system)
expect(Elastic::ProcessBookkeepingService).not_to receive(:track!) expect(Elastic::ProcessBookkeepingService).not_to receive(:track!)
......
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