Merge branch 'feature/lower-subtransactions-logging-thresholds' into 'master'

Lower subtransactions loggging thresholds

See merge request gitlab-org/gitlab!68932

lib/gitlab/database/transaction/context.rb

@@ -6,9 +6,8 @@ module Gitlab
       class Context
         attr_reader :context
 
-        LOG_DEPTH_THRESHOLD = 4         # 3 nested subtransactions + 1 real transaction
-        LOG_SAVEPOINTS_THRESHOLD = 5    # 5 `SAVEPOINTS` created in sequence or nested
-        LOG_DURATION_S_THRESHOLD = 120  # 2 minutes long transaction
+        LOG_SAVEPOINTS_THRESHOLD = 1    # 1 `SAVEPOINT` created in a transaction
+        LOG_DURATION_S_THRESHOLD = 120  # transaction that is running for 2 minutes or longer
         LOG_THROTTLE_DURATION = 1
 
         def initialize
@@ -19,6 +18,10 @@ module Gitlab
           @context[:start_time] = current_timestamp
         end
 
+        def set_depth(depth)
+          @context[:depth] = [@context[:depth].to_i, depth].max
+        end
+
         def increment_savepoints
           @context[:savepoints] = @context[:savepoints].to_i + 1
         end
@@ -31,10 +34,6 @@ module Gitlab
           @context[:releases] = @context[:releases].to_i + 1
         end
 
-        def set_depth(depth)
-          @context[:depth] = [@context[:depth].to_i, depth].max
-        end
-
         def track_sql(sql)
           (@context[:queries] ||= []).push(sql)
         end
@@ -45,10 +44,6 @@ module Gitlab
           current_timestamp - @context[:start_time]
         end
 
-        def depth_threshold_exceeded?
-          @context[:depth].to_i >= LOG_DEPTH_THRESHOLD
-        end
-
         def savepoints_threshold_exceeded?
           @context[:savepoints].to_i >= LOG_SAVEPOINTS_THRESHOLD
         end
@@ -57,16 +52,10 @@ module Gitlab
           duration.to_i >= LOG_DURATION_S_THRESHOLD
         end
 
-        def log_savepoints?
-          depth_threshold_exceeded? || savepoints_threshold_exceeded?
-        end
-
-        def log_duration?
-          duration_threshold_exceeded?
-        end
-
         def should_log?
-          !logged_already? && (log_savepoints? || log_duration?)
+          return false if logged_already?
+
+          savepoints_threshold_exceeded? || duration_threshold_exceeded?
         end
 
         def commit

lib/gitlab/database/transaction/observer.rb

@@ -21,7 +21,7 @@ module Gitlab
             context.set_start_time
             context.set_depth(0)
             context.track_sql(event.payload[:sql])
-          elsif cmd.start_with?('SAVEPOINT ')
+          elsif cmd.start_with?('SAVEPOINT', 'EXCEPTION')
             context.set_depth(manager.open_transactions)
             context.increment_savepoints
           elsif cmd.start_with?('ROLLBACK TO SAVEPOINT')

spec/lib/gitlab/database/transaction/context_spec.rb

@@ -70,24 +70,6 @@ RSpec.describe Gitlab::Database::Transaction::Context do
     it { expect(subject.duration).to be >= 0 }
   end
 
-  context 'when depth is low' do
-    it 'does not log data upon COMMIT' do
-      expect(subject).not_to receive(:application_info)
-
-      subject.commit
-    end
-
-    it 'does not log data upon ROLLBACK' do
-      expect(subject).not_to receive(:application_info)
-
-      subject.rollback
-    end
-
-    it '#should_log? returns false' do
-      expect(subject.should_log?).to be false
-    end
-  end
-
   shared_examples 'logs transaction data' do
     it 'logs once upon COMMIT' do
       expect(subject).to receive(:application_info).and_call_original
@@ -116,17 +98,9 @@ RSpec.describe Gitlab::Database::Transaction::Context do
     end
   end
 
-  context 'when depth exceeds threshold' do
-    before do
-      subject.set_depth(described_class::LOG_DEPTH_THRESHOLD + 1)
-    end
-
-    it_behaves_like 'logs transaction data'
-  end
-
   context 'when savepoints count exceeds threshold' do
     before do
-      data[:savepoints] = described_class::LOG_SAVEPOINTS_THRESHOLD + 1
+      data[:savepoints] = 1
     end
 
     it_behaves_like 'logs transaction data'