Merge branch '338836-add-advanced-search-timeout' into 'master'

Add Advanced Search timeout

See merge request gitlab-org/gitlab!73650

config/initializers/elastic_client_setup.rb

@@ -45,6 +45,23 @@ Gitlab.ee do
     end
   end
 
+  ### Modified from elasticsearch-model/lib/elasticsearch/model/searching.rb
+
+  module Elasticsearch
+    module Model
+      module Searching
+        class SearchRequest
+          def execute!
+            response = klass.client.search(@definition)
+            raise Elastic::TimeoutError if response['timed_out']
+
+            response
+          end
+        end
+      end
+    end
+  end
+
   ### Modified from elasticsearch-model/lib/elasticsearch/model.rb
 
   [

ee/app/controllers/ee/search_controller.rb

@@ -15,6 +15,8 @@ module EE
       # for self-managed we check if the licensed feature available
       track_redis_hll_event :show, name: 'i_search_paid',
         if: :track_search_paid?
+
+      rescue_from Elastic::TimeoutError, with: :render_timeout
     end
 
     private

ee/lib/elastic/latest/application_class_proxy.rb

@@ -13,7 +13,7 @@ module Elastic
         # Counts need to be fast as we load one count per type of document
         # on every page load. Fail early if they are slow since they don't
         # need to be accurate.
-        es_options[:timeout] = '1s' if search_options[:count_only]
+        es_options[:timeout] = search_options[:count_only] ? '1s' : '30s'
 
         # Calling elasticsearch-ruby method
         super(query, es_options)

ee/lib/elastic/timeout_error.rb

+# frozen_string_literal: true
+
+module Elastic
+  TimeoutError = Class.new(StandardError)
+end

ee/spec/features/search/elastic/global_search_spec.rb

@@ -38,6 +38,26 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
     end
   end
 
+  shared_examples 'advanced search timeouts' do
+    context 'when search times out' do
+      before do
+        allow_next_instance_of(SearchService) do |service|
+          allow(service).to receive(:search_objects).and_raise(Elastic::TimeoutError)
+        end
+
+        visit path
+      end
+
+      it 'renders timeout information' do
+        expect(page).to have_content('Your search timed out')
+      end
+
+      it 'sets tab count to 0' do
+        expect(page.find('.search-filter .active')).to have_text('0')
+      end
+    end
+  end
+
   describe 'I do not overload the database' do
     let(:creation_traits) { [] }
 
@@ -48,6 +68,7 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
       let(:query_count_multiplier) { 0 }
 
       it_behaves_like 'an efficient database result'
+      it_behaves_like 'advanced search timeouts'
     end
 
     context 'searching projects' do
@@ -60,6 +81,7 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
       let(:query_count_multiplier) { 4 }
 
       it_behaves_like 'an efficient database result'
+      it_behaves_like 'advanced search timeouts'
     end
 
     context 'searching merge requests' do
@@ -70,6 +92,7 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
       let(:query_count_multiplier) { 0 }
 
       it_behaves_like 'an efficient database result'
+      it_behaves_like 'advanced search timeouts'
     end
 
     context 'searching milestones' do
@@ -84,6 +107,8 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
     context 'searching code' do
       let(:path) { search_path(search: '*', scope: 'blobs') }
 
+      it_behaves_like 'advanced search timeouts'
+
       it 'avoids N+1 database queries' do
         project.repository.index_commits_and_blobs
 
@@ -107,6 +132,10 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
       let(:path_for_one) { search_path(search: '*', scope: 'commits', per_page: 1) }
       let(:path_for_multiple) { search_path(search: '*', scope: 'commits', per_page: 5) }
 
+      it_behaves_like 'advanced search timeouts' do
+        let(:path) { path_for_one }
+      end
+
       it 'avoids N+1 database queries' do
         project.repository.index_commits_and_blobs
 

ee/spec/lib/peek/views/elasticsearch_spec.rb

@@ -14,6 +14,7 @@ RSpec.describe Peek::Views::Elasticsearch, :elastic, :request_store do
   describe '#results' do
     let(:results) { described_class.new.results }
     let(:project) { create(:project, :repository) }
+    let(:timeout) { '30s' }
 
     it 'includes performance details' do
       ::Gitlab::SafeRequestStore.clear!
@@ -24,9 +25,9 @@ RSpec.describe Peek::Views::Elasticsearch, :elastic, :request_store do
       expect(results[:duration]).to be_kind_of(String)
       expect(results[:details].last[:method]).to eq('GET')
       expect(results[:details].last[:path]).to eq('gitlab-test/doc/_search')
-      expect(results[:details].last[:params]).to eq({ routing: "project_#{project.id}" })
+      expect(results[:details].last[:params]).to eq({ routing: "project_#{project.id}", timeout: timeout })
 
-      expect(results[:details].last[:request]).to eq("GET gitlab-test/doc/_search?routing=project_#{project.id}")
+      expect(results[:details].last[:request]).to eq("GET gitlab-test/doc/_search?routing=project_#{project.id}&timeout=#{timeout}")
     end
   end
 end

ee/spec/support/shared_examples/lib/gitlab/elastic/search_results_shared_examples.rb

@@ -13,7 +13,10 @@ RSpec.shared_examples 'does not hit Elasticsearch twice for objects and counts'
         results.objects(scope)
         results.public_send("#{scope}_count")
 
+        request = ::Gitlab::Instrumentation::ElasticsearchTransport.detail_store.first
+
         expect(::Gitlab::Instrumentation::ElasticsearchTransport.get_request_count).to eq(1)
+        expect(request.dig(:params, :timeout)).to eq('30s')
       end
     end
   end