From 94631e4c6ac157d0c9ada18e6000ab859613f112 Mon Sep 17 00:00:00 2001
From: Simon Tomlinson <stomlinson@gitlab.com>
Date: Fri, 4 Oct 2024 21:17:56 +0000
Subject: [PATCH] Memoize table information in schema_spec.rb
This improves the runtime of the test dramatically.
---
spec/db/schema_spec.rb | 15 ++++---
spec/support/helpers/dns_helpers.rb | 17 ++++++--
spec/support/stack_prof.rb | 68 +++++++++++++++++++++++++++++
3 files changed, 91 insertions(+), 9 deletions(-)
create mode 100644 spec/support/stack_prof.rb
diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb
index 4b3de618e555..3ec0b7ef8ce0 100644
--- a/spec/db/schema_spec.rb
+++ b/spec/db/schema_spec.rb
@@ -3,7 +3,12 @@
require 'spec_helper'
require Rails.root.join('ee', 'spec', 'db', 'schema_support') if Gitlab.ee?
-RSpec.describe 'Database schema', feature_category: :database do
+RSpec.describe 'Database schema',
+ # These skip a bit of unnecessary setup for each spec invocation,
+ # and there are thousands of specs in this file. In total, this improves runtime by roughly 30%
+ :do_not_mock_admin_mode_setting, :do_not_stub_snowplow_by_default,
+ stackprof: { interval: 101000 },
+ feature_category: :database do
prepend_mod_with('DB::SchemaSupport')
let(:tables) { connection.tables }
@@ -396,7 +401,7 @@
end
context 'existence of Postgres schemas' do
- def get_schemas
+ let_it_be(:schemas) do
sql = <<~SQL
SELECT schema_name FROM
information_schema.schemata
@@ -411,17 +416,17 @@ def get_schemas
end
it 'we have a public schema' do
- expect(get_schemas).to include('public')
+ expect(schemas).to include('public')
end
Gitlab::Database::EXTRA_SCHEMAS.each do |schema|
it "we have a '#{schema}' schema'" do
- expect(get_schemas).to include(schema.to_s)
+ expect(schemas).to include(schema.to_s)
end
end
it 'we do not have unexpected schemas' do
- expect(get_schemas.size).to eq(Gitlab::Database::EXTRA_SCHEMAS.size + 1)
+ expect(schemas.size).to eq(Gitlab::Database::EXTRA_SCHEMAS.size + 1)
end
end
diff --git a/spec/support/helpers/dns_helpers.rb b/spec/support/helpers/dns_helpers.rb
index 27310c7e04c9..d96571f392f5 100644
--- a/spec/support/helpers/dns_helpers.rb
+++ b/spec/support/helpers/dns_helpers.rb
@@ -2,6 +2,8 @@
module DnsHelpers
include ViteHelper
+ # strong_memoize is used in a class method, so we extend it instead of including it.
+ extend Gitlab::Utils::StrongMemoize
def block_dns!
stub_all_dns!
@@ -57,14 +59,21 @@ def db_hosts
ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).map(&:host).compact.uniq
end
+ def self.redis_hosts
+ # This needs to be a class method so that the memoization sticks across different specs.
+ # Without memoization, this adds a considerable amount of time to each spec execution.
+ strong_memoize(:redis_hosts) do
+ Gitlab::Redis::ALL_CLASSES.flat_map do |redis_instance|
+ redis_instance.params[:host] || redis_instance.params[:nodes]&.map { |n| n[:host] }
+ end.uniq.compact
+ end
+ end
+
def permit_redis!
# https://github.com/redis-rb/redis-client/blob/v0.11.2/lib/redis_client/ruby_connection.rb#L51 uses Socket.tcp that
# calls Addrinfo.getaddrinfo internally.
- hosts = Gitlab::Redis::ALL_CLASSES.flat_map do |redis_instance|
- redis_instance.params[:host] || redis_instance.params[:nodes]&.map { |n| n[:host] }
- end.uniq.compact
- hosts.each do |host|
+ DnsHelpers.redis_hosts.each do |host|
allow(Addrinfo).to receive(:getaddrinfo).with(host, anything, nil, :STREAM, anything, anything, any_args).and_call_original
end
end
diff --git a/spec/support/stack_prof.rb b/spec/support/stack_prof.rb
new file mode 100644
index 000000000000..1f0e5cfaebc9
--- /dev/null
+++ b/spec/support/stack_prof.rb
@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+
+require 'stackprof'
+
+# Applies stackprof instrumentation to a given rspec context
+# Use as context "your context", :stackprof do
+# or as context "your context", stackprof: { mode: wall, interval: 100000 } do
+# to change arguments to stackprof.
+#
+# Results will be gzipped and placed in tmp/ if running locally, or rspec/ if running in CI, and can be viewed
+# with any json-compatible flamegraph viewer, such as speedscope.
+
+module Support
+ module StackProf
+ def self.start(example)
+ puts "Starting stackprof"
+ raise "Cannot nest stackprof calls!" if ::StackProf.running?
+
+ ::StackProf.start(**stackprof_args_for(example))
+ end
+
+ def self.finish(example)
+ raise "Stackprof was not running!" unless ::StackProf.running?
+
+ ::StackProf.stop
+ puts 'finishing stackprof'
+ location = example.class.metadata[:location]
+ # Turn a file path like ./spec/path/to/spec.rb:123 into spec_path_to_spec_123
+ location_sanitized = location.gsub('./', '').tr('/', '_').gsub('.rb', '').tr(':', '_')
+
+ out_filepath = if ENV['CI']
+ "rspec/stackprof_#{location_sanitized}-#{ENV['CI_JOB_NAME_SLUG']}.json.gz"
+ else
+ "tmp/stackprof_#{location_sanitized}.json.gz"
+ end
+
+ start_time = ::Gitlab::Metrics::System.monotonic_time
+ Zlib::GzipWriter.open(out_filepath) do |gz|
+ gz.puts Gitlab::Json.generate(::StackProf.results)
+ end
+ end_time = ::Gitlab::Metrics::System.monotonic_time
+ puts "Wrote stackprof dump to #{out_filepath} in #{(end_time - start_time).round(2)}s"
+ end
+
+ def self.stackprof_args_for(example)
+ caller_config = example.class.metadata[:stackprof]
+ default = {
+ mode: :wall,
+ interval: 10100, # in us, 99hz
+ raw: true # Needed for flamegraphs
+ }
+ # If called as `:stackprof`, the value will be a literal `true`
+ return default unless caller_config.is_a?(Hash)
+
+ default.merge(caller_config)
+ end
+ end
+end
+
+RSpec.configure do |config|
+ config.before(:all, :stackprof) do |example|
+ Support::StackProf.start(example)
+ end
+
+ config.after(:all, :stackprof) do |example|
+ Support::StackProf.finish(example)
+ end
+end
--
GitLab