Merge branch 'tc-geo-gently-log-prune' into 'master'

Truncate Geo event log with a delay

Closes #6221

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

app/workers/all_queues.yml

@@ -158,6 +158,7 @@
 - geo:geo_repository_verification_primary_shard
 - geo:geo_repository_verification_primary_single
 - geo:geo_repository_verification_secondary_single
+- geo:geo_truncate_event_log
 
 - object_storage_upload
 - object_storage:object_storage_background_move

ee/app/workers/geo/prune_event_log_worker.rb

@@ -6,18 +6,17 @@ module Geo
     include ::Gitlab::Geo::LogHelpers
 
     LEASE_TIMEOUT = 60.minutes
-
-    def lease_timeout
-      LEASE_TIMEOUT
-    end
+    TRUNCATE_DELAY = 10.minutes
 
     def perform
-      return unless Gitlab::Geo.primary?
+      return if Gitlab::Database.read_only?
 
       try_obtain_lease do
         if Gitlab::Geo.secondary_nodes.empty?
-          log_info('No secondary nodes, truncate the Geo Event Log table')
-          ActiveRecord::Base.connection.truncate(Geo::EventLog.table_name)
+          log_info('No secondary nodes configured, scheduling truncation of the Geo Event Log')
+
+          ::Geo::TruncateEventLogWorker.perform_in(TRUNCATE_DELAY)
+
           break
         end
 
@@ -35,5 +34,9 @@ module Geo
                      .each_batch { |batch| batch.delete_all }
       end
     end
+
+    def lease_timeout
+      LEASE_TIMEOUT
+    end
   end
 end

ee/app/workers/geo/truncate_event_log_worker.rb

+module Geo
+  class TruncateEventLogWorker
+    include ApplicationWorker
+    include GeoQueue
+    include ::Gitlab::Geo::LogHelpers
+
+    def perform
+      if Gitlab::Geo.secondary_nodes.any?
+        log_info('Some secondary nodes configured, Geo Event Log should not be truncated', geo_node_count: Gitlab::Geo.secondary_nodes.count)
+      else
+        log_info('Still no secondary nodes configured, truncating the Geo Event Log')
+        ActiveRecord::Base.connection.truncate(Geo::EventLog.table_name)
+      end
+    end
+  end
+end

ee/changelogs/unreleased/tc-geo-gently-log-prune-delay.yml

+---
+title: Truncate Geo event log with a delay
+merge_request: 5897
+author:
+type: changed

ee/spec/workers/geo/prune_event_log_worker_spec.rb

@@ -32,7 +32,7 @@ describe Geo::PruneEventLogWorker, :geo do
       it 'logs error when it cannot obtain lease' do
         allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain) { nil }
 
-        expect(worker).to receive(:log_error).with('Cannot obtain an exclusive lease. There must be another instance already in execution.')
+        expect(worker).to receive(:log_error).with(/^Cannot obtain an exclusive lease/)
 
         worker.perform
       end
@@ -45,10 +45,9 @@ describe Geo::PruneEventLogWorker, :geo do
         it 'deletes everything from the Geo event log' do
           create_list(:geo_event_log, 2)
 
-          expect(worker).to receive(:log_info).with('No secondary nodes, truncate the Geo Event Log table')
-          expect(ActiveRecord::Base.connection).to receive(:truncate).with('geo_event_log').and_call_original
+          expect(Geo::TruncateEventLogWorker).to receive(:perform_in).with(described_class::TRUNCATE_DELAY)
 
-          expect { worker.perform }.to change { Geo::EventLog.count }.by(-2)
+          worker.perform
         end
       end
 
@@ -58,12 +57,11 @@ describe Geo::PruneEventLogWorker, :geo do
         let(:unhealthy_status) { build(:geo_node_status, :unhealthy) }
 
         it 'contacts all secondary nodes for their status' do
-          events = create_list(:geo_event_log, 5)
+          status = spy(:status)
 
-          create(:geo_node_status, :healthy, cursor_last_event_id: events.last.id, geo_node_id: secondary.id)
-          create(:geo_node_status, :healthy, cursor_last_event_id: events[3].id, geo_node_id: secondary2.id)
+          allow_any_instance_of(GeoNode).to receive(:status).and_return(status)
 
-          expect(worker).to receive(:log_info).with('Delete Geo Event Log entries up to id', anything)
+          expect(status).to receive(:cursor_last_event_id).twice.and_return(0)
 
           worker.perform
         end
@@ -74,7 +72,7 @@ describe Geo::PruneEventLogWorker, :geo do
           create(:geo_node_status, :healthy, cursor_last_event_id: events.last.id, geo_node_id: secondary.id)
           create(:geo_node_status, :unhealthy, geo_node_id: secondary2.id)
 
-          expect(worker).to receive(:log_info).with('Could not get status of all nodes, not deleting any entries from Geo Event Log', unhealthy_node_count: 1)
+          expect(worker).to receive(:log_info).with(/^Could not get status of all nodes/, unhealthy_node_count: 1)
 
           expect { worker.perform }.not_to change { Geo::EventLog.count }
         end
@@ -84,7 +82,7 @@ describe Geo::PruneEventLogWorker, :geo do
 
           create(:geo_node_status, :healthy, cursor_last_event_id: events[3].id, geo_node_id: secondary.id)
           create(:geo_node_status, :healthy, cursor_last_event_id: events.last.id, geo_node_id: secondary2.id)
-          expect(worker).to receive(:log_info).with('Delete Geo Event Log entries up to id', geo_event_log_id: events[3].id)
+          expect(worker).to receive(:log_info).with(/^Delete Geo Event Log/, geo_event_log_id: events[3].id)
 
           expect { worker.perform }.to change { Geo::EventLog.count }.by(-4)
         end

ee/spec/workers/geo/truncate_event_log_worker_spec.rb

+require 'spec_helper'
+
+describe Geo::TruncateEventLogWorker, :geo do
+  include ::EE::GeoHelpers
+
+  subject(:worker) { described_class.new }
+  set(:primary) { create(:geo_node, :primary) }
+
+  describe '#perform' do
+    context 'current node primary' do
+      before do
+        stub_current_geo_node(primary)
+      end
+
+      it 'deletes everything from the Geo event log' do
+        create_list(:geo_event_log, 2)
+
+        expect(ActiveRecord::Base.connection).to receive(:truncate).with('geo_event_log').and_call_original
+
+        expect { worker.perform }.to change { Geo::EventLog.count }.by(-2)
+      end
+
+      it 'deletes nothing when a secondary node exists' do
+        create(:geo_node)
+        create_list(:geo_event_log, 2)
+
+        expect { worker.perform }.not_to change { Geo::EventLog.count }
+      end
+    end
+  end
+end