From 5589deb9cce3afd9e5ef1342757b452cd8e35400 Mon Sep 17 00:00:00 2001 From: Maxime Orefice Date: Tue, 16 Dec 2025 10:42:46 +0100 Subject: [PATCH 1/2] Add RecordCountMonitor warning This MR introduces a new class to warn in development when more than 1_000 records are loaded in memory. --- config/environments/development.rb | 3 -- config/initializers/record_count_monitor.rb | 3 ++ lib/gitlab/database/record_count_monitor.rb | 25 ++++++++++ .../database/record_count_monitor_spec.rb | 46 +++++++++++++++++++ 4 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 config/initializers/record_count_monitor.rb create mode 100644 lib/gitlab/database/record_count_monitor.rb create mode 100644 spec/lib/gitlab/database/record_count_monitor_spec.rb diff --git a/config/environments/development.rb b/config/environments/development.rb index 33d9e12cf4ab87..7f3a6aaa681f23 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -21,9 +21,6 @@ config.action_controller.perform_caching = false end - # Show a warning when a large data set is loaded into memory - config.active_record.warn_on_records_fetched_greater_than = 1000 - # Raise an error on page load if there are pending migrations config.active_record.migration_error = :page_load diff --git a/config/initializers/record_count_monitor.rb b/config/initializers/record_count_monitor.rb new file mode 100644 index 00000000000000..19e345fff4dea9 --- /dev/null +++ b/config/initializers/record_count_monitor.rb @@ -0,0 +1,3 @@ +# frozen_string_literal: true + +Gitlab::Database::RecordCountMonitor.subscribe if Rails.env.development? diff --git a/lib/gitlab/database/record_count_monitor.rb b/lib/gitlab/database/record_count_monitor.rb new file mode 100644 index 00000000000000..2420df6f82bb0f --- /dev/null +++ b/lib/gitlab/database/record_count_monitor.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Gitlab + module Database + class RecordCountMonitor + THRESHOLD = 1000 + + def self.subscribe + ActiveSupport::Notifications.subscribe('sql.active_record') do |_name, _start, _finish, _id, payload| + next unless payload[:row_count] + next if payload[:row_count] <= THRESHOLD + + warn_large_result_set(payload) + end + end + + def self.warn_large_result_set(payload) + message = "Query fetched #{payload[:row_count]} rows (threshold: #{THRESHOLD})" + message += "\nSQL: #{payload[:sql]}" if payload[:sql] + + Gitlab::AppLogger.warn(message) + end + end + end +end diff --git a/spec/lib/gitlab/database/record_count_monitor_spec.rb b/spec/lib/gitlab/database/record_count_monitor_spec.rb new file mode 100644 index 00000000000000..126add3efb059f --- /dev/null +++ b/spec/lib/gitlab/database/record_count_monitor_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::RecordCountMonitor, feature_category: :database do + describe '.subscribe' do + before do + allow(Gitlab::AppLogger).to receive(:warn) + described_class.subscribe + end + + it 'warns when row count exceeds threshold' do + payload = { row_count: 1001, sql: 'SELECT * FROM users' } + + ActiveSupport::Notifications.instrument('sql.active_record', payload) + + expect(Gitlab::AppLogger).to have_received(:warn).with(/Query fetched 1001 rows/) + end + + it 'does not warn when row count is at or below threshold' do + payload = { row_count: 1000, sql: 'SELECT * FROM users' } + + ActiveSupport::Notifications.instrument('sql.active_record', payload) + + expect(Gitlab::AppLogger).not_to have_received(:warn) + end + end + + describe '.warn_large_result_set' do + before do + allow(Gitlab::AppLogger).to receive(:warn) + end + + it 'logs warning with row count' do + described_class.warn_large_result_set({ row_count: 5000 }) + + expect(Gitlab::AppLogger).to have_received(:warn).with(/Query fetched 5000 rows/) + end + + it 'includes SQL when present' do + described_class.warn_large_result_set({ row_count: 5000, sql: 'SELECT * FROM users' }) + + expect(Gitlab::AppLogger).to have_received(:warn).with(/SELECT \* FROM users/) + end + end +end -- GitLab From 09a3802ba81e7ea0e6f27d7631339fa4d1838e0e Mon Sep 17 00:00:00 2001 From: Maxime Orefice Date: Wed, 17 Dec 2025 09:14:17 +0100 Subject: [PATCH 2/2] apply code review feedback --- lib/gitlab/database/record_count_monitor.rb | 20 ++++++++------ .../database/record_count_monitor_spec.rb | 27 +++++++------------ 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/lib/gitlab/database/record_count_monitor.rb b/lib/gitlab/database/record_count_monitor.rb index 2420df6f82bb0f..1fefccb1ccfe71 100644 --- a/lib/gitlab/database/record_count_monitor.rb +++ b/lib/gitlab/database/record_count_monitor.rb @@ -3,20 +3,24 @@ module Gitlab module Database class RecordCountMonitor - THRESHOLD = 1000 + THRESHOLD = 1_000 def self.subscribe - ActiveSupport::Notifications.subscribe('sql.active_record') do |_name, _start, _finish, _id, payload| - next unless payload[:row_count] - next if payload[:row_count] <= THRESHOLD + return if @subscribed - warn_large_result_set(payload) + ActiveSupport::Notifications.subscribe('sql.active_record') do |event| + next unless event[:row_count] + next if event[:row_count] <= THRESHOLD + + warn_large_result_set(event) end + + @subscribed = true end - def self.warn_large_result_set(payload) - message = "Query fetched #{payload[:row_count]} rows (threshold: #{THRESHOLD})" - message += "\nSQL: #{payload[:sql]}" if payload[:sql] + def self.warn_large_result_set(event) + message = "Query fetched #{event[:row_count]} rows (threshold: #{THRESHOLD})" + message += "\nSQL: #{event[:sql]}" if event[:sql] Gitlab::AppLogger.warn(message) end diff --git a/spec/lib/gitlab/database/record_count_monitor_spec.rb b/spec/lib/gitlab/database/record_count_monitor_spec.rb index 126add3efb059f..b69e14f52d6b44 100644 --- a/spec/lib/gitlab/database/record_count_monitor_spec.rb +++ b/spec/lib/gitlab/database/record_count_monitor_spec.rb @@ -4,25 +4,10 @@ RSpec.describe Gitlab::Database::RecordCountMonitor, feature_category: :database do describe '.subscribe' do - before do - allow(Gitlab::AppLogger).to receive(:warn) - described_class.subscribe - end - - it 'warns when row count exceeds threshold' do - payload = { row_count: 1001, sql: 'SELECT * FROM users' } - - ActiveSupport::Notifications.instrument('sql.active_record', payload) - - expect(Gitlab::AppLogger).to have_received(:warn).with(/Query fetched 1001 rows/) - end + it 'subscribes to sql.active_record notifications' do + expect(ActiveSupport::Notifications).to receive(:subscribe).with('sql.active_record') - it 'does not warn when row count is at or below threshold' do - payload = { row_count: 1000, sql: 'SELECT * FROM users' } - - ActiveSupport::Notifications.instrument('sql.active_record', payload) - - expect(Gitlab::AppLogger).not_to have_received(:warn) + described_class.subscribe end end @@ -42,5 +27,11 @@ expect(Gitlab::AppLogger).to have_received(:warn).with(/SELECT \* FROM users/) end + + it 'does not include SQL when not present' do + described_class.warn_large_result_set({ row_count: 5000 }) + + expect(Gitlab::AppLogger).to have_received(:warn).with("Query fetched 5000 rows (threshold: 1000)") + end end end -- GitLab