Commit 4acbc941 authored by Stan Hu's avatar Stan Hu

Cache column_exists? for application settings

This is most a backport of
https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/4987/diffs but includes
other columns that use column_exists? in a way that may cause unnecessary
schema loads.
parent ff292d40
...@@ -347,15 +347,15 @@ class ApplicationSetting < ActiveRecord::Base ...@@ -347,15 +347,15 @@ class ApplicationSetting < ActiveRecord::Base
end end
def home_page_url_column_exists? def home_page_url_column_exists?
ActiveRecord::Base.connection.column_exists?(:application_settings, :home_page_url) ::Gitlab::Database.cached_column_exists?(:application_settings, :home_page_url)
end end
def help_page_support_url_column_exists? def help_page_support_url_column_exists?
ActiveRecord::Base.connection.column_exists?(:application_settings, :help_page_support_url) ::Gitlab::Database.cached_column_exists?(:application_settings, :help_page_support_url)
end end
def sidekiq_throttling_column_exists? def sidekiq_throttling_column_exists?
ActiveRecord::Base.connection.column_exists?(:application_settings, :sidekiq_throttling_enabled) ::Gitlab::Database.cached_column_exists?(:application_settings, :sidekiq_throttling_enabled)
end end
def domain_whitelist_raw def domain_whitelist_raw
......
---
title: Cache column_exists? for application settings
merge_request:
author:
type: performance
...@@ -183,6 +183,10 @@ module Gitlab ...@@ -183,6 +183,10 @@ module Gitlab
ActiveRecord::Base.connection ActiveRecord::Base.connection
end end
def self.cached_column_exists?(table_name, column_name)
connection.schema_cache.columns_hash(table_name).has_key?(column_name.to_s)
end
private_class_method :connection private_class_method :connection
def self.database_version def self.database_version
......
...@@ -287,6 +287,17 @@ describe Gitlab::Database do ...@@ -287,6 +287,17 @@ describe Gitlab::Database do
end end
end end
describe '.cached_column_exists?' do
it 'only retrieves data once' do
expect(ActiveRecord::Base.connection).to receive(:columns).once.and_call_original
2.times do
expect(described_class.cached_column_exists?(:projects, :id)).to be_truthy
expect(described_class.cached_column_exists?(:projects, :bogus_column)).to be_falsey
end
end
end
describe '#true_value' do describe '#true_value' do
it 'returns correct value for PostgreSQL' do it 'returns correct value for PostgreSQL' do
expect(described_class).to receive(:postgresql?).and_return(true) expect(described_class).to receive(:postgresql?).and_return(true)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment