Merge branch...

Merge branch '207808-geo-bug-filedownloaddispatchworker-may-sometimes-excessively-resync-files-and-block-new' into 'master'

Use uncached SQL queries for Geo long running workers

Closes #207808

See merge request gitlab-org/gitlab!26187

changelogs/unreleased/207808-geo-bug-filedownloaddispatchworker-may-sometimes-excessively-resyn.yml

+---
+title: 'Use uncached SQL queries for Geo long-running workers'
+merge_request: 26187
+author:
+type: fixed

ee/app/workers/geo/scheduler/scheduler_worker.rb

@@ -170,7 +170,7 @@ module Geo
 
       def update_pending_resources
         if reload_queue?
-          @pending_resources = load_pending_resources
+          @pending_resources = Gitlab::Database.geo_uncached_queries { load_pending_resources }
           set_backoff_time! if should_apply_backoff?
         end
       end

ee/lib/ee/gitlab/database.rb

@@ -24,6 +24,16 @@ module EE
 
           ActiveRecord::Base.establish_connection(config)
         end
+
+        def geo_uncached_queries
+          raise 'No block given' unless block_given?
+
+          Geo::TrackingBase.uncached do
+            ActiveRecord::Base.uncached do
+              yield
+            end
+          end
+        end
       end
     end
   end

ee/spec/support/caching.rb

+# frozen_string_literal: true
+
+RSpec.configure do |config|
+  config.around(:each, :use_sql_query_cache_for_tracking_db) do |example|
+    Geo::TrackingBase.cache do
+      example.run
+    end
+  end
+end

ee/spec/workers/geo/container_repository_sync_dispatch_worker_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Geo::ContainerRepositorySyncDispatchWorker, :geo, :geo_fdw do
+describe Geo::ContainerRepositorySyncDispatchWorker, :geo, :geo_fdw, :use_sql_query_cache_for_tracking_db do
   include ::EE::GeoHelpers
   include ExclusiveLeaseHelpers
 

ee/spec/workers/geo/file_download_dispatch_worker_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Geo::FileDownloadDispatchWorker, :geo, :geo_fdw do
+describe Geo::FileDownloadDispatchWorker, :geo, :geo_fdw, :use_sql_query_cache_for_tracking_db do
   include ::EE::GeoHelpers
   include ExclusiveLeaseHelpers
 
@@ -59,6 +59,30 @@ describe Geo::FileDownloadDispatchWorker, :geo, :geo_fdw do
     subject.perform
   end
 
+  it 'does not schedule duplicated jobs because of query cache' do
+    lfs_object_1 = create(:lfs_object, :with_file)
+    lfs_object_2 = create(:lfs_object, :with_file)
+    lfs_object_3 = create(:lfs_object, :with_file)
+    create(:geo_lfs_object_registry, :never_synced, lfs_object: lfs_object_1)
+    create(:geo_lfs_object_registry, :never_synced, lfs_object: lfs_object_2)
+    create(:geo_lfs_object_registry, :never_synced, lfs_object: lfs_object_3)
+
+    stub_const('Geo::Scheduler::SchedulerWorker::DB_RETRIEVE_BATCH_SIZE', 3)
+    secondary.update!(files_max_capacity: 3)
+
+    expect(Geo::FileDownloadWorker).to receive(:perform_async).with('lfs', lfs_object_1.id).once do
+      Thread.new do
+        # Rails will invalidate the query cache if the update happens in the same thread
+        Geo::LfsObjectRegistry.update(success: true)
+      end
+    end
+
+    expect(Geo::FileDownloadWorker).to receive(:perform_async).with('lfs', lfs_object_2.id).once
+    expect(Geo::FileDownloadWorker).to receive(:perform_async).with('lfs', lfs_object_3.id).once
+
+    subject.perform
+  end
+
   context 'with attachments (Upload records)' do
     let(:upload) { create(:upload) }
 

ee/spec/workers/geo/migrated_local_files_clean_up_worker_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Geo::MigratedLocalFilesCleanUpWorker, :geo, :geo_fdw do
+describe Geo::MigratedLocalFilesCleanUpWorker, :geo, :geo_fdw, :use_sql_query_cache_for_tracking_db do
   include ::EE::GeoHelpers
   include ExclusiveLeaseHelpers
 

ee/spec/workers/geo/repository_shard_sync_worker_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Geo::RepositoryShardSyncWorker, :geo, :geo_fdw, :clean_gitlab_redis_cache do
+describe Geo::RepositoryShardSyncWorker, :geo, :geo_fdw, :clean_gitlab_redis_cache, :use_sql_query_cache_for_tracking_db do
   include ::EE::GeoHelpers
   include ExclusiveLeaseHelpers
 

ee/spec/workers/geo/repository_verification/secondary/shard_worker_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Geo::RepositoryVerification::Secondary::ShardWorker, :geo, :geo_fdw, :request_store, :clean_gitlab_redis_cache do
+describe Geo::RepositoryVerification::Secondary::ShardWorker, :geo, :geo_fdw, :request_store, :clean_gitlab_redis_cache, :use_sql_query_cache_for_tracking_db do
   include ::EE::GeoHelpers
   include ExclusiveLeaseHelpers