Revert "Add system check for missing database indexes" (#30909)
David Roetzel
GitHub
parent
20c749bd45
commit
9be77fc0db
1
Gemfile
1
Gemfile
|
|
@ -229,4 +229,3 @@ gem 'rubyzip', '~> 2.3'
|
||||||
gem 'hcaptcha', '~> 7.1'
|
gem 'hcaptcha', '~> 7.1'
|
||||||
|
|
||||||
gem 'mail', '~> 2.8'
|
gem 'mail', '~> 2.8'
|
||||||
gem 'prism'
|
|
||||||
|
|
|
||||||
|
|
@ -600,7 +600,6 @@ GEM
|
||||||
actionmailer (>= 3)
|
actionmailer (>= 3)
|
||||||
net-smtp
|
net-smtp
|
||||||
premailer (~> 1.7, >= 1.7.9)
|
premailer (~> 1.7, >= 1.7.9)
|
||||||
prism (0.30.0)
|
|
||||||
propshaft (0.9.0)
|
propshaft (0.9.0)
|
||||||
actionpack (>= 7.0.0)
|
actionpack (>= 7.0.0)
|
||||||
activesupport (>= 7.0.0)
|
activesupport (>= 7.0.0)
|
||||||
|
|
@ -1004,7 +1003,6 @@ DEPENDENCIES
|
||||||
pg (~> 1.5)
|
pg (~> 1.5)
|
||||||
pghero
|
pghero
|
||||||
premailer-rails
|
premailer-rails
|
||||||
prism
|
|
||||||
propshaft
|
propshaft
|
||||||
public_suffix (~> 6.0)
|
public_suffix (~> 6.0)
|
||||||
puma (~> 6.3)
|
puma (~> 6.3)
|
||||||
|
|
|
||||||
|
|
@ -1,92 +0,0 @@
|
||||||
# frozen_string_literal: true
|
|
||||||
|
|
||||||
class Admin::Db::SchemaParser
|
|
||||||
class Index
|
|
||||||
attr_reader :name, :table_name, :columns, :options
|
|
||||||
|
|
||||||
def initialize(name:, table_name:, columns:, options:)
|
|
||||||
@name = name
|
|
||||||
@table_name = table_name
|
|
||||||
@columns = columns
|
|
||||||
@options = options
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
attr_reader :indexes_by_table
|
|
||||||
|
|
||||||
def initialize(source)
|
|
||||||
parse(source)
|
|
||||||
end
|
|
||||||
|
|
||||||
private
|
|
||||||
|
|
||||||
def parse(source)
|
|
||||||
@indexes_by_table = {}
|
|
||||||
queue = [Prism.parse(source).value]
|
|
||||||
while (node = queue.shift)
|
|
||||||
if node.type == :call_node && node.name == :create_table
|
|
||||||
parse_create_table(node)
|
|
||||||
elsif node.type == :call_node && node.name == :add_index
|
|
||||||
parse_add_index(node)
|
|
||||||
else
|
|
||||||
queue.concat(node.compact_child_nodes)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
def parse_create_table(node)
|
|
||||||
table_name = parse_arguments(node).first
|
|
||||||
queue = node.compact_child_nodes
|
|
||||||
while (node = queue.shift)
|
|
||||||
if node.type == :call_node && node.name == :index
|
|
||||||
parse_index(node, table_name:)
|
|
||||||
else
|
|
||||||
queue.concat(node.compact_child_nodes)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
def parse_index(node, table_name:)
|
|
||||||
arguments = parse_arguments(node)
|
|
||||||
save_index(
|
|
||||||
name: arguments.last[:name],
|
|
||||||
table_name: table_name,
|
|
||||||
columns: arguments.first,
|
|
||||||
options: arguments.last
|
|
||||||
)
|
|
||||||
end
|
|
||||||
|
|
||||||
def parse_add_index(node)
|
|
||||||
arguments = parse_arguments(node)
|
|
||||||
save_index(
|
|
||||||
name: arguments.last[:name],
|
|
||||||
table_name: arguments.first,
|
|
||||||
columns: arguments[1],
|
|
||||||
options: arguments.last
|
|
||||||
)
|
|
||||||
end
|
|
||||||
|
|
||||||
def parse_arguments(node)
|
|
||||||
node.arguments.arguments.map { |a| parse_argument(a) }
|
|
||||||
end
|
|
||||||
|
|
||||||
def parse_argument(argument)
|
|
||||||
case argument
|
|
||||||
when Prism::StringNode
|
|
||||||
argument.unescaped
|
|
||||||
when Prism::SymbolNode
|
|
||||||
argument.unescaped.to_sym
|
|
||||||
when Prism::ArrayNode
|
|
||||||
argument.elements.map { |e| parse_argument(e) }
|
|
||||||
when Prism::KeywordHashNode
|
|
||||||
argument.elements.to_h do |element|
|
|
||||||
[element.key.unescaped.to_sym, parse_argument(element.value)]
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
def save_index(name:, table_name:, columns:, options:)
|
|
||||||
@indexes_by_table[table_name] ||= []
|
|
||||||
@indexes_by_table[table_name] << Index.new(name:, table_name:, columns:, options:)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
@ -8,7 +8,6 @@ class Admin::SystemCheck
|
||||||
Admin::SystemCheck::SidekiqProcessCheck,
|
Admin::SystemCheck::SidekiqProcessCheck,
|
||||||
Admin::SystemCheck::RulesCheck,
|
Admin::SystemCheck::RulesCheck,
|
||||||
Admin::SystemCheck::ElasticsearchCheck,
|
Admin::SystemCheck::ElasticsearchCheck,
|
||||||
Admin::SystemCheck::MissingIndexesCheck,
|
|
||||||
].freeze
|
].freeze
|
||||||
|
|
||||||
def self.perform(current_user)
|
def self.perform(current_user)
|
||||||
|
|
|
||||||
|
|
@ -1,36 +0,0 @@
|
||||||
# frozen_string_literal: true
|
|
||||||
|
|
||||||
class Admin::SystemCheck::MissingIndexesCheck < Admin::SystemCheck::BaseCheck
|
|
||||||
def skip?
|
|
||||||
!current_user.can?(:view_devops)
|
|
||||||
end
|
|
||||||
|
|
||||||
def pass?
|
|
||||||
missing_indexes.none?
|
|
||||||
end
|
|
||||||
|
|
||||||
def message
|
|
||||||
Admin::SystemCheck::Message.new(:missing_indexes_check, missing_indexes.join(', '))
|
|
||||||
end
|
|
||||||
|
|
||||||
private
|
|
||||||
|
|
||||||
def missing_indexes
|
|
||||||
@missing_indexes ||= begin
|
|
||||||
expected_indexes_by_table.flat_map do |table, indexes|
|
|
||||||
expected_indexes = indexes.map(&:name)
|
|
||||||
expected_indexes - existing_indexes_for(table)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
def expected_indexes_by_table
|
|
||||||
schema_rb = Rails.root.join('db', 'schema.rb').read
|
|
||||||
schema_parser = Admin::Db::SchemaParser.new(schema_rb)
|
|
||||||
schema_parser.indexes_by_table
|
|
||||||
end
|
|
||||||
|
|
||||||
def existing_indexes_for(table)
|
|
||||||
ActiveRecord::Base.connection.indexes(table).map(&:name)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
@ -862,8 +862,6 @@ en:
|
||||||
elasticsearch_version_check:
|
elasticsearch_version_check:
|
||||||
message_html: 'Incompatible Elasticsearch version: %{value}'
|
message_html: 'Incompatible Elasticsearch version: %{value}'
|
||||||
version_comparison: Elasticsearch %{running_version} is running while %{required_version} is required
|
version_comparison: Elasticsearch %{running_version} is running while %{required_version} is required
|
||||||
missing_indexes_check:
|
|
||||||
message_html: 'The following indexes are missing from the database and should be recreated: <code>%{value}</code>.<br> Missing indexes may lead to severely reduced performance and data inconsistencies.'
|
|
||||||
rules_check:
|
rules_check:
|
||||||
action: Manage server rules
|
action: Manage server rules
|
||||||
message_html: You haven't defined any server rules.
|
message_html: You haven't defined any server rules.
|
||||||
|
|
|
||||||
|
|
@ -1,49 +0,0 @@
|
||||||
# frozen_string_literal: true
|
|
||||||
|
|
||||||
require 'rails_helper'
|
|
||||||
|
|
||||||
describe Admin::Db::SchemaParser do
|
|
||||||
let(:dummy_schema) do
|
|
||||||
<<~SCHEMA
|
|
||||||
# Comment
|
|
||||||
ActiveRecord::Schema[7.1].define(version: 23) do
|
|
||||||
create_table "people", force: :cascade do |t|
|
|
||||||
t.string "name"
|
|
||||||
end
|
|
||||||
|
|
||||||
create_table "posts", force: :cascade do |t|
|
|
||||||
t.string "title", null: false
|
|
||||||
t.bigint "size", null: false
|
|
||||||
t.string "description"
|
|
||||||
# t.index ["size", "title"], name: "index_posts_on_size_and_title"
|
|
||||||
t.index ["title"], name: "index_posts_on_title", unique: true
|
|
||||||
t.index ["size"], name: "index_posts_on_size"
|
|
||||||
end
|
|
||||||
|
|
||||||
# add_index "people", ["name"], name: "commented_out_index"
|
|
||||||
add_index "people", ["name"], name: "index_people_on_name"
|
|
||||||
end
|
|
||||||
SCHEMA
|
|
||||||
end
|
|
||||||
let(:schema_parser) { described_class.new(dummy_schema) }
|
|
||||||
|
|
||||||
describe '#indexes_by_table' do
|
|
||||||
subject { schema_parser.indexes_by_table }
|
|
||||||
|
|
||||||
it 'returns index info for all affected tables' do
|
|
||||||
expect(subject.keys).to match_array(%w(people posts))
|
|
||||||
end
|
|
||||||
|
|
||||||
it 'returns all index information for the `people` table' do
|
|
||||||
people_info = subject['people']
|
|
||||||
expect(people_info.map(&:name)).to contain_exactly('index_people_on_name')
|
|
||||||
end
|
|
||||||
|
|
||||||
it 'returns all index information for the `posts` table' do
|
|
||||||
posts_info = subject['posts']
|
|
||||||
expect(posts_info.map(&:name)).to contain_exactly(
|
|
||||||
'index_posts_on_title', 'index_posts_on_size'
|
|
||||||
)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
@ -1,65 +0,0 @@
|
||||||
# frozen_string_literal: true
|
|
||||||
|
|
||||||
require 'rails_helper'
|
|
||||||
|
|
||||||
describe Admin::SystemCheck::MissingIndexesCheck do
|
|
||||||
subject(:check) { described_class.new(user) }
|
|
||||||
|
|
||||||
let(:user) { Fabricate(:user) }
|
|
||||||
let(:schema_parser) do
|
|
||||||
instance_double(Admin::Db::SchemaParser, indexes_by_table: index_info)
|
|
||||||
end
|
|
||||||
let(:index_info) do
|
|
||||||
{
|
|
||||||
'users' => [instance_double(Admin::Db::SchemaParser::Index, name: 'index_users_on_profile_id')],
|
|
||||||
'posts' => [instance_double(Admin::Db::SchemaParser::Index, name: 'index_posts_on_user_id')],
|
|
||||||
}
|
|
||||||
end
|
|
||||||
let(:posts_indexes) { [] }
|
|
||||||
let(:users_indexes) { [] }
|
|
||||||
|
|
||||||
before do
|
|
||||||
allow(Admin::Db::SchemaParser).to receive(:new).and_return(schema_parser)
|
|
||||||
allow(ActiveRecord::Base.connection).to receive(:indexes).with('posts').and_return(posts_indexes)
|
|
||||||
allow(ActiveRecord::Base.connection).to receive(:indexes).with('users').and_return(users_indexes)
|
|
||||||
end
|
|
||||||
|
|
||||||
it_behaves_like 'a check available to devops users'
|
|
||||||
|
|
||||||
describe '#pass?' do
|
|
||||||
context 'when indexes are missing' do
|
|
||||||
let(:posts_indexes) do
|
|
||||||
[instance_double(ActiveRecord::ConnectionAdapters::IndexDefinition, name: 'index_posts_on_user_id')]
|
|
||||||
end
|
|
||||||
|
|
||||||
it 'returns false' do
|
|
||||||
expect(check.pass?).to be false
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
context 'when all expected indexes are present' do
|
|
||||||
let(:posts_indexes) do
|
|
||||||
[instance_double(ActiveRecord::ConnectionAdapters::IndexDefinition, name: 'index_posts_on_user_id')]
|
|
||||||
end
|
|
||||||
let(:users_indexes) do
|
|
||||||
[instance_double(ActiveRecord::ConnectionAdapters::IndexDefinition, name: 'index_users_on_profile_id')]
|
|
||||||
end
|
|
||||||
|
|
||||||
it 'returns true' do
|
|
||||||
expect(check.pass?).to be true
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
describe '#message' do
|
|
||||||
subject { check.message }
|
|
||||||
|
|
||||||
it 'sets the class name as the message key' do
|
|
||||||
expect(subject.key).to eq(:missing_indexes_check)
|
|
||||||
end
|
|
||||||
|
|
||||||
it 'sets a list of missing indexes as message value' do
|
|
||||||
expect(subject.value).to eq('index_users_on_profile_id, index_posts_on_user_id')
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
|
||||||
Loading…
Reference in New Issue