Merge branch...

Merge branch '11595-use-elasticsearch-bulk-indexing-api-for-database-index-operations' into 'master'

Use ES bulk-indexing API for project associations

Closes #11595

See merge request gitlab-org/gitlab-ee!13917

ee/app/models/concerns/elastic/application_search.rb

@@ -286,6 +286,22 @@ module Elastic
     end
 
     class_methods do
+      # Support STI models
+      def inherited(subclass)
+        super
+
+        # Avoid SystemStackError in Model.import
+        # See https://github.com/elastic/elasticsearch-rails/issues/144
+        subclass.include Elasticsearch::Model
+
+        # Use ES configuration from parent model
+        # TODO: Revisit after upgrading to elasticsearch-model 7.0.0
+        # See https://github.com/elastic/elasticsearch-rails/commit/b8455db186664e21927bfb271bab6390853e7ff3
+        subclass.__elasticsearch__.index_name = self.index_name
+        subclass.__elasticsearch__.document_type = self.document_type
+        subclass.__elasticsearch__.instance_variable_set(:@mapping, self.mapping.dup)
+      end
+
       # Should be overridden for all nested models
       def nested?
         false

ee/app/models/concerns/elastic/notes_search.rb

@@ -7,14 +7,6 @@ module Elastic
     included do
       include ApplicationSearch
 
-      def self.inherited(subclass)
-        super
-
-        subclass.__elasticsearch__.index_name = self.index_name
-        subclass.__elasticsearch__.document_type = self.document_type
-        subclass.__elasticsearch__.instance_variable_set(:@mapping, self.mapping.dup)
-      end
-
       def es_type
         'note'
       end

ee/app/models/ee/legacy_diff_note.rb

@@ -5,9 +5,6 @@ module EE
     extend ActiveSupport::Concern
 
     prepended do
-      # Elastic search configuration (it does not support STI properly)
-      document_type 'doc'
-      index_name [Rails.application.class.parent_name.downcase, Rails.env].join('-')
       include Elastic::NotesSearch
     end
   end

ee/app/models/ee/personal_snippet.rb

@@ -5,8 +5,6 @@ module EE
     extend ActiveSupport::Concern
 
     prepended do
-      document_type 'doc'
-      index_name [Rails.application.class.parent_name.downcase, Rails.env].join('-')
       include Elastic::SnippetsSearch
     end
   end

ee/app/models/ee/project_snippet.rb

@@ -5,8 +5,6 @@ module EE
     extend ActiveSupport::Concern
 
     prepended do
-      document_type 'doc'
-      index_name [Rails.application.class.parent_name.downcase, Rails.env].join('-')
       include Elastic::SnippetsSearch
     end
   end

ee/app/services/elastic/index_record_service.rb

@@ -35,8 +35,7 @@ module Elastic
 
     def initial_index_project(project)
       project.each_indexed_association do |klass, objects|
-        nested = klass.nested?
-        objects.find_each { |object| import(object, nested, true) }
+        objects.es_import
       end
 
       # Finally, index blobs/commits/wikis

ee/changelogs/unreleased/11595-use-elasticsearch-bulk-indexing-api-for-database-index-operations.yml

+---
+title: Use bulk-indexing API for project associations
+merge_request: 13917
+author:
+type: changed

ee/spec/models/concerns/elastic/snippet_spec.rb

@@ -138,4 +138,12 @@ describe Snippet, :elastic do
 
     expect(snippet.as_indexed_json).to eq(expected_hash)
   end
+
+  it 'uses same index for Snippet subclasses' do
+    Snippet.subclasses.each do |snippet_class|
+      expect(snippet_class.index_name).to eq(Snippet.index_name)
+      expect(snippet_class.document_type).to eq(Snippet.document_type)
+      expect(snippet_class.mappings.to_hash).to eq(Snippet.mappings.to_hash)
+    end
+  end
 end

ee/spec/services/elastic/index_record_service_spec.rb

@@ -51,31 +51,48 @@ describe Elastic::IndexRecordService, :elastic do
     end
   end
 
-  it 'indexes all nested objects for a Project' do
-    # To be able to access it outside the following block
-    project = nil
+  context 'with nested associations' do
+    let(:project) { create :project, :repository }
+
+    before do
+      Sidekiq::Testing.disable! do
+        create :issue, project: project
+        create :milestone, project: project
+        create :note, project: project
+        create :merge_request, target_project: project, source_project: project
+        create :project_snippet, project: project
+      end
 
-    Sidekiq::Testing.disable! do
-      project = create :project, :repository
-      create :issue, project: project
-      create :milestone, project: project
-      create :note, project: project
-      create :merge_request, target_project: project, source_project: project
-      create :project_snippet, project: project
+      # Nothing should be in the index at this point
+      expect(Elasticsearch::Model.search('*').total_count).to be(0)
     end
 
-    expect(ElasticCommitIndexerWorker).to receive(:perform_async).with(project.id).and_call_original
+    it 'indexes records associated with the project' do
+      expect(ElasticCommitIndexerWorker).to receive(:perform_async).with(project.id).and_call_original
 
-    # Nothing should be in the index at this point
-    expect(Elasticsearch::Model.search('*').total_count).to be(0)
+      Sidekiq::Testing.inline! do
+        subject.execute(project, true)
+      end
+      Gitlab::Elastic::Helper.refresh_index
 
-    Sidekiq::Testing.inline! do
-      subject.execute(project, true)
+      ## All database objects + data from repository. The absolute value does not matter
+      expect(Elasticsearch::Model.search('*').total_count).to be > 40
     end
-    Gitlab::Elastic::Helper.refresh_index
 
-    ## All database objects + data from repository. The absolute value does not matter
-    expect(Elasticsearch::Model.search('*').total_count).to be > 40
+    it 'does not index records not associated with the project' do
+      other_project = create :project
+
+      expect(ElasticCommitIndexerWorker).to receive(:perform_async).with(other_project.id).and_call_original
+
+      Sidekiq::Testing.inline! do
+        subject.execute(other_project, true)
+      end
+      Gitlab::Elastic::Helper.refresh_index
+
+      # Only the project itself should be in the index
+      expect(Elasticsearch::Model.search('*').total_count).to be 1
+      expect(Project.elastic_search('*').records).to contain_exactly(other_project)
+    end
   end
 
   it 'indexes changes during indexing gap' do