Avoid N+1 queries

Reduce the number of queries to the minimum.

ee/app/models/geo/event_log.rb

@@ -47,6 +47,10 @@ module Geo
       order(id: :desc).first
     end
 
+    def self.includes_events
+      includes(reflections.keys)
+    end
+
     def event
       repository_created_event ||
         repository_updated_event ||

ee/lib/gitlab/geo/event_gap_tracking.rb

@@ -50,13 +50,12 @@ module Gitlab
         with_redis do |redis|
           redis.zremrangebyscore(GEO_EVENT_LOG_GAPS, '-inf', outdated_timestamp)
 
-          gap_ids = redis.zrangebyscore(GEO_EVENT_LOG_GAPS, '-inf', grace_timestamp, with_scores: true)
+          gap_ids = redis.zrangebyscore(GEO_EVENT_LOG_GAPS, '-inf', grace_timestamp).map(&:to_i)
 
-          gap_ids.each do |event_id, score|
-            handled = yield event_id.to_i
+          event_logs = ::Geo::EventLog.includes_events.where(id: gap_ids)
+          event_logs.each { |event_log| yield event_log }
 
-            redis.zrem(GEO_EVENT_LOG_GAPS, event_id) if handled
-          end
+          redis.zrem(GEO_EVENT_LOG_GAPS, event_logs.map(&:id)) if event_logs.any?
         end
       end
 

ee/lib/gitlab/geo/log_cursor/daemon.rb

@@ -34,7 +34,7 @@ module Gitlab
         end
 
         def run_once!
-          gap_tracking.fill_gaps { |event_id| handle_gap_event(event_id) }
+          gap_tracking.fill_gaps { |event_log| handle_single_event(event_log) }
 
           # Wrap this with the connection to make it possible to reconnect if
           # PGbouncer dies: https://github.com/rails/rails/issues/29189
@@ -82,15 +82,6 @@ module Gitlab
           raise e
         end
 
-        def handle_gap_event(event_id)
-          event_log = ::Geo::EventLog.find_by(id: event_id)
-
-          return false unless event_log
-
-          handle_single_event(event_log)
-          true
-        end
-
         def event_klass_for(event)
           event_klass_name = event.class.name.demodulize
           current_namespace = self.class.name.deconstantize

ee/spec/lib/gitlab/geo/event_gap_tracking_spec.rb

@@ -86,9 +86,13 @@ describe Gitlab::Geo::EventGapTracking, :clean_gitlab_redis_cache do
     it 'handles gaps that are more than 10 minutes old' do
       gap_tracking.check!(event_id_with_gap)
 
+      gap_event = create(:geo_event_log, id: gap_id)
+
       Timecop.travel(12.minutes) do
-        expect { |blk| gap_tracking.fill_gaps(&blk) }.to yield_with_args(gap_id)
+        expect { |blk| gap_tracking.fill_gaps(&blk) }.to yield_with_args(gap_event)
       end
+
+      expect(read_gaps).to be_empty
     end
 
     it 'drops gaps older than 1 hour' do
@@ -100,6 +104,34 @@ describe Gitlab::Geo::EventGapTracking, :clean_gitlab_redis_cache do
 
       expect(read_gaps).to be_empty
     end
+
+    it 'avoids N+1 queries to fetch event logs and their associated events' do
+      yielded = []
+
+      blk = lambda do |event_log|
+        event_log.event
+        yielded << event_log
+      end
+
+      Timecop.travel(13.minutes.ago) do
+        gap_tracking.check!(event_id_with_gap)
+      end
+      create(:geo_event_log, :updated_event, id: gap_id)
+
+      control_count = ActiveRecord::QueryRecorder.new do
+        expect { gap_tracking.fill_gaps(&blk) }.to change { yielded.count }.by(1)
+      end.count
+
+      Timecop.travel(12.minutes.ago) do
+        gap_tracking.check!(event_id_with_gap + 3)
+      end
+      create(:geo_event_log, :updated_event, id: event_id_with_gap + 1)
+      create(:geo_event_log, :updated_event, id: event_id_with_gap + 2)
+
+      expect do
+        expect { gap_tracking.fill_gaps(&blk) }.to change { yielded.count }.by(2)
+      end.not_to exceed_query_limit(control_count)
+    end
   end
 
   describe '#track_gaps' do

ee/spec/lib/gitlab/geo/log_cursor/daemon_spec.rb

@@ -86,10 +86,13 @@ describe Gitlab::Geo::LogCursor::Daemon, :postgresql, :clean_gitlab_redis_shared
       end
 
       it 'calls #handle_gap_event for each gap the gap tracking finds' do
-        allow(daemon.send(:gap_tracking)).to receive(:fill_gaps).and_yield(1).and_yield(5)
+        second_event_log = create(:geo_event_log, repository_updated_event: repository_updated_event)
 
-        expect(daemon).to receive(:handle_gap_event).with(1)
-        expect(daemon).to receive(:handle_gap_event).with(5)
+        allow_any_instance_of(::Gitlab::Geo::LogCursor::EventLogs).to receive(:fetch_in_batches)
+        allow(daemon.send(:gap_tracking)).to receive(:fill_gaps).and_yield(event_log).and_yield(second_event_log)
+
+        expect(daemon).to receive(:handle_single_event).with(event_log)
+        expect(daemon).to receive(:handle_single_event).with(second_event_log)
 
         daemon.run_once!
       end
@@ -131,6 +134,8 @@ describe Gitlab::Geo::LogCursor::Daemon, :postgresql, :clean_gitlab_redis_shared
 
         daemon.run_once!
 
+        create(:geo_event_log, id: event_log.id + 1)
+
         expect(read_gaps).to eq([event_log.id + 1])
 
         expect(::Geo::EventLogState.last_processed.id).to eq(new_event.id)
@@ -146,6 +151,9 @@ describe Gitlab::Geo::LogCursor::Daemon, :postgresql, :clean_gitlab_redis_shared
 
         daemon.run_once!
 
+        create(:geo_event_log, id: new_event.id + 1, repository_updated_event: updated_event)
+        create(:geo_event_log, id: new_event.id + 2, repository_updated_event: updated_event)
+
         expect(read_gaps).to eq([new_event.id + 1, new_event.id + 2])
       end
 
@@ -246,7 +254,7 @@ describe Gitlab::Geo::LogCursor::Daemon, :postgresql, :clean_gitlab_redis_shared
     gaps = []
 
     Timecop.travel(12.minutes) do
-      daemon.send(:gap_tracking).send(:fill_gaps, { |id| gaps << id })
+      daemon.send(:gap_tracking).send(:fill_gaps) { |event| gaps << event.id }
     end
 
     gaps