Merge branch '214440-revert-redis-cache-for-elasticsearch-enabled' into 'master'

Revert non-scalable caching strategy for Elasticsearch enabled

Closes #214440

See merge request gitlab-org/gitlab!30012

ee/app/models/concerns/elasticsearch_indexed_container.rb

@@ -3,16 +3,9 @@
 module ElasticsearchIndexedContainer
   extend ActiveSupport::Concern
 
-  CACHE_EXPIRES_IN = 10.minutes
-
   included do
     after_commit :index, on: :create
     after_commit :delete_from_index, on: :destroy
-    after_commit :drop_limited_ids_cache!, on: [:create, :destroy]
-  end
-
-  def drop_limited_ids_cache!
-    self.class.drop_limited_ids_cache!
   end
 
   class_methods do
@@ -20,28 +13,6 @@ module ElasticsearchIndexedContainer
       pluck(target_attr_name)
     end
 
-    def limited_ids
-      limited.pluck(:id)
-    end
-
-    def limited_ids_cache_key
-      [self.name.underscore.to_sym, :limited_ids]
-    end
-
-    def limited_ids_cached
-      Rails.cache.fetch(limited_ids_cache_key, expires_in: CACHE_EXPIRES_IN) do
-        limited_ids
-      end
-    end
-
-    def drop_limited_ids_cache!
-      Rails.cache.delete(limited_ids_cache_key)
-    end
-
-    def limited_include?(namespace_id)
-      limited_ids_cached.include?(namespace_id)
-    end
-
     def remove_all(except: [])
       self.where.not(target_attr_name => except).each_batch do |batch, _index|
         batch.destroy_all # #rubocop:disable Cop/DestroyAll

ee/app/models/ee/application_setting.rb

@@ -131,26 +131,37 @@ module EE
       ElasticsearchIndexedProject.target_ids
     end
 
-    def elasticsearch_limited_namespaces(ignore_descendants: false)
-      ElasticsearchIndexedNamespace.limited(ignore_descendants: ignore_descendants)
-    end
-
-    def elasticsearch_limited_projects(ignore_namespaces: false)
-      ElasticsearchIndexedProject.limited(ignore_namespaces: ignore_namespaces)
-    end
-
     def elasticsearch_indexes_project?(project)
       return false unless elasticsearch_indexing?
       return true unless elasticsearch_limit_indexing?
 
-      ElasticsearchIndexedProject.limited_include?(project.id)
+      elasticsearch_limited_projects.exists?(project.id)
     end
 
     def elasticsearch_indexes_namespace?(namespace)
       return false unless elasticsearch_indexing?
       return true unless elasticsearch_limit_indexing?
 
-      ElasticsearchIndexedNamespace.limited_include?(namespace.id)
+      elasticsearch_limited_namespaces.exists?(namespace.id)
+    end
+
+    def elasticsearch_limited_projects(ignore_namespaces = false)
+      return ::Project.where(id: ElasticsearchIndexedProject.select(:project_id)) if ignore_namespaces
+
+      union = ::Gitlab::SQL::Union.new([
+                                         ::Project.where(namespace_id: elasticsearch_limited_namespaces.select(:id)),
+                                         ::Project.where(id: ElasticsearchIndexedProject.select(:project_id))
+                                       ]).to_sql
+
+      ::Project.from("(#{union}) projects")
+    end
+
+    def elasticsearch_limited_namespaces(ignore_descendants = false)
+      namespaces = ::Namespace.where(id: ElasticsearchIndexedNamespace.select(:namespace_id))
+
+      return namespaces if ignore_descendants
+
+      ::Gitlab::ObjectHierarchy.new(namespaces).base_and_descendants
     end
 
     def pseudonymizer_available?

ee/app/models/elasticsearch_indexed_namespace.rb

@@ -18,20 +18,6 @@ class ElasticsearchIndexedNamespace < ApplicationRecord
     :namespace_id
   end
 
-  def self.limited(ignore_descendants: false)
-    namespaces = Namespace.with_route.where(id: target_ids)
-
-    return namespaces if ignore_descendants
-
-    Gitlab::ObjectHierarchy.new(namespaces).base_and_descendants
-  end
-
-  def self.drop_limited_ids_cache!
-    # To prevent stale cache we also drop ElasticsearchIndexedProject cache since it uses ElasticsearchIndexedNamespace
-    ElasticsearchIndexedProject.drop_limited_ids_cache!
-    super
-  end
-
   def self.index_first_n_namespaces_of_plan(plan, number_of_namespaces)
     indexed_namespaces = self.select(:namespace_id)
     now = Time.now
@@ -57,8 +43,6 @@ class ElasticsearchIndexedNamespace < ApplicationRecord
 
       ElasticNamespaceIndexerWorker.bulk_perform_async(jobs) # rubocop:disable Scalability/BulkPerformWithContext, CodeReuse/Worker
     end
-
-    drop_limited_ids_cache!
   end
 
   def self.unindex_last_n_namespaces_of_plan(plan, number_of_namespaces)
@@ -76,8 +60,6 @@ class ElasticsearchIndexedNamespace < ApplicationRecord
 
       ElasticNamespaceIndexerWorker.bulk_perform_async(jobs) # rubocop:disable Scalability/BulkPerformWithContext, CodeReuse/Worker
     end
-
-    drop_limited_ids_cache!
   end
 
   private

ee/app/models/elasticsearch_indexed_project.rb

@@ -14,17 +14,6 @@ class ElasticsearchIndexedProject < ApplicationRecord
     :project_id
   end
 
-  def self.limited(ignore_namespaces: false)
-    return Project.inc_routes.where(id: target_ids) if ignore_namespaces
-
-    Project.inc_routes.from_union(
-      [
-        Project.where(namespace_id: ElasticsearchIndexedNamespace.limited.select(:id)),
-        Project.where(id: target_ids)
-      ]
-    )
-  end
-
   private
 
   def index

ee/app/views/admin/application_settings/_elasticsearch_form.html.haml

@@ -92,14 +92,14 @@
             - if elasticsearch_too_many_namespaces?
               %p= _('Too many namespaces enabled. You will need to manage them via the console or the API.')
             - else
-              = f.text_field :elasticsearch_namespace_ids, class: 'js-elasticsearch-namespaces', value: elasticsearch_namespace_ids, data: { selected: elasticsearch_objects_options(@application_setting.elasticsearch_limited_namespaces(ignore_descendants: true)).to_json }
+              = f.text_field :elasticsearch_namespace_ids, class: 'js-elasticsearch-namespaces', value: elasticsearch_namespace_ids, data: { selected: elasticsearch_objects_options(@application_setting.elasticsearch_limited_namespaces(true)).to_json }
 
           .form-group.js-limit-projects{ class: ('hidden' unless @application_setting.elasticsearch_limit_indexing) }
             = f.label :elasticsearch_project_ids, _('Projects to index'), class: 'label-bold'
             - if elasticsearch_too_many_projects?
               %p= _('Too many projects enabled. You will need to manage them via the console or the API.')
             - else
-              = f.text_field :elasticsearch_project_ids, class: 'js-elasticsearch-projects', value: elasticsearch_project_ids, data: { selected: elasticsearch_objects_options(@application_setting.elasticsearch_limited_projects(ignore_namespaces: true)).to_json }
+              = f.text_field :elasticsearch_project_ids, class: 'js-elasticsearch-projects', value: elasticsearch_project_ids, data: { selected: elasticsearch_objects_options(@application_setting.elasticsearch_limited_projects(true)).to_json }
 
         .sub-section
           %h4= _('Elasticsearch AWS IAM credentials')

ee/spec/models/application_setting_spec.rb

@@ -286,7 +286,7 @@ describe ApplicationSetting do
 
           expect(setting.elasticsearch_limited_namespaces).to match_array(
             [namespaces.last, child_namespace, child_namespace_indexed_through_parent])
-          expect(setting.elasticsearch_limited_namespaces(ignore_descendants: true)).to match_array(
+          expect(setting.elasticsearch_limited_namespaces(true)).to match_array(
             [namespaces.last, child_namespace])
         end
       end

ee/spec/models/elasticsearch_indexed_namespace_spec.rb

@@ -28,18 +28,7 @@ describe ElasticsearchIndexedNamespace do
     end
   end
 
-  context 'caching' do
-    it 'invalidates indexed project cache' do
-      expect(ElasticsearchIndexedProject).to receive(:drop_limited_ids_cache!).and_call_original.twice
-      expect(ElasticsearchIndexedNamespace).to receive(:drop_limited_ids_cache!).and_call_original.twice
-
-      n = create(:elasticsearch_indexed_namespace)
-
-      n.destroy
-    end
-  end
-
-  context 'with plans', :use_clean_rails_redis_caching do
+  context 'with plans' do
     Plan::PAID_HOSTED_PLANS.each do |plan|
       plan_factory = "#{plan}_plan"
       let_it_be(plan_factory) { create(plan_factory) }
@@ -54,11 +43,8 @@ describe ElasticsearchIndexedNamespace do
       stub_ee_application_setting(elasticsearch_indexing: false)
     end
 
-    def expect_indexed_namespaces_to_match(array)
-      ids = described_class.order(:created_at).pluck(:namespace_id)
-
-      expect(ids).to eq(array)
-      expect(ids).to match_array(described_class.limited_ids_cached)
+    def get_indexed_namespaces
+      described_class.order(:created_at).pluck(:namespace_id)
     end
 
     def expect_queue_to_contain(*args)
@@ -69,23 +55,21 @@ describe ElasticsearchIndexedNamespace do
 
     describe '.index_first_n_namespaces_of_plan' do
       it 'creates records, scoped by plan and ordered by namespace id' do
-        expect(ElasticsearchIndexedNamespace).to receive(:drop_limited_ids_cache!).and_call_original.exactly(3).times
-
         ids = namespaces.map(&:id)
 
         described_class.index_first_n_namespaces_of_plan('gold', 1)
 
-        expect_indexed_namespaces_to_match([ids[0]])
+        expect(get_indexed_namespaces).to eq([ids[0]])
         expect_queue_to_contain(ids[0], "index")
 
         described_class.index_first_n_namespaces_of_plan('gold', 2)
 
-        expect_indexed_namespaces_to_match([ids[0], ids[2]])
+        expect(get_indexed_namespaces).to eq([ids[0], ids[2]])
         expect_queue_to_contain(ids[2], "index")
 
         described_class.index_first_n_namespaces_of_plan('silver', 1)
 
-        expect_indexed_namespaces_to_match([ids[0], ids[2], ids[1]])
+        expect(get_indexed_namespaces).to eq([ids[0], ids[2], ids[1]])
         expect_queue_to_contain(ids[1], "index")
       end
     end
@@ -97,26 +81,23 @@ describe ElasticsearchIndexedNamespace do
       end
 
       it 'creates records, scoped by plan and ordered by namespace id' do
-        expect(ElasticsearchIndexedNamespace).to receive(:drop_limited_ids_cache!).and_call_original.exactly(3).times
-
         ids = namespaces.map(&:id)
 
-        expect_indexed_namespaces_to_match([ids[0], ids[2], ids[1]])
+        expect(get_indexed_namespaces).to contain_exactly(ids[0], ids[2], ids[1])
 
         described_class.unindex_last_n_namespaces_of_plan('gold', 1)
 
-        expect_indexed_namespaces_to_match([ids[0], ids[1]])
+        expect(get_indexed_namespaces).to contain_exactly(ids[0], ids[1])
         expect_queue_to_contain(ids[2], "delete")
 
         described_class.unindex_last_n_namespaces_of_plan('silver', 1)
 
-        expect_indexed_namespaces_to_match([ids[0]])
-
+        expect(get_indexed_namespaces).to contain_exactly(ids[0])
         expect_queue_to_contain(ids[1], "delete")
 
         described_class.unindex_last_n_namespaces_of_plan('gold', 1)
 
-        expect_indexed_namespaces_to_match([])
+        expect(get_indexed_namespaces).to be_empty
         expect_queue_to_contain(ids[0], "delete")
       end
     end

ee/spec/support/shared_examples/models/elasticsearch_indexed_container_shared_examples.rb

@@ -9,25 +9,6 @@ RSpec.shared_examples 'an elasticsearch indexed container' do
     end
   end
 
-  describe '.limited_ids_cached', :use_clean_rails_memory_store_caching do
-    subject { create container }
-
-    it 'returns correct values' do
-      initial_ids = subject.class.limited_ids
-
-      expect(initial_ids).not_to be_empty
-      expect(subject.class.limited_ids_cached).to match_array(initial_ids)
-
-      new_container = create container
-
-      expect(subject.class.limited_ids_cached).to match_array(initial_ids + [new_container.id])
-
-      new_container.destroy
-
-      expect(subject.class.limited_ids_cached).to match_array(initial_ids)
-    end
-  end
-
   describe 'callbacks' do
     subject { build container }
 
@@ -43,12 +24,6 @@ RSpec.shared_examples 'an elasticsearch indexed container' do
 
         subject.save!
       end
-
-      it 'invalidates limited_ids cache' do
-        is_expected.to receive(:drop_limited_ids_cache!)
-
-        subject.save!
-      end
     end
 
     describe 'on destroy' do
@@ -65,12 +40,6 @@ RSpec.shared_examples 'an elasticsearch indexed container' do
 
         subject.destroy!
       end
-
-      it 'invalidates limited_ids cache' do
-        is_expected.to receive(:drop_limited_ids_cache!)
-
-        subject.destroy!
-      end
     end
   end
 end