Commit 65be69cf authored by Stan Hu's avatar Stan Hu

Cache column_exists? for Elasticsearch columns

Any time a searchable object (e.g. issue, merge request, project, etc.)
is updated, the `after_commit` hook checks whetether two Elasticsearch
columns are present in the application_settings table. This leads to
unnecessary queries in production as the value can just be cached by
connection.

Closes gitlab-org/gitlab-ce#43355
parent 8b4ed40e
...@@ -133,11 +133,11 @@ module EE ...@@ -133,11 +133,11 @@ module EE
end end
def elasticsearch_indexing_column_exists? def elasticsearch_indexing_column_exists?
ActiveRecord::Base.connection.column_exists?(:application_settings, :elasticsearch_indexing) ::Gitlab::Database.cached_column_exists?(:application_settings, :elasticsearch_indexing)
end end
def elasticsearch_search_column_exists? def elasticsearch_search_column_exists?
ActiveRecord::Base.connection.column_exists?(:application_settings, :elasticsearch_search) ::Gitlab::Database.cached_column_exists?(:application_settings, :elasticsearch_search)
end end
end end
end end
---
title: Cache column_exists? for Elasticsearch columns
merge_request:
author:
type: performance
...@@ -199,6 +199,10 @@ module Gitlab ...@@ -199,6 +199,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