Add system check for missing indexes.

As part of #18965 we want to enable admins to see if one or
more indexes are missing. This adds a new "system check" to
display a warning in case indexes are missing.

The list of indexes we expect to be present are extracted from
`db/schema.rb` using ruby's new `prism` parser.
This commit is contained in:
David Roetzel 2024-07-01 14:50:26 +02:00
parent 2babfafaff
commit b5bec6f142
No known key found for this signature in database
8 changed files with 239 additions and 0 deletions

View File

@ -227,3 +227,4 @@ gem 'rubyzip', '~> 2.3'
gem 'hcaptcha', '~> 7.1'
gem 'mail', '~> 2.8'
gem 'prism'

View File

@ -601,6 +601,7 @@ GEM
actionmailer (>= 3)
net-smtp
premailer (~> 1.7, >= 1.7.9)
prism (0.30.0)
propshaft (1.0.0)
actionpack (>= 7.0.0)
activesupport (>= 7.0.0)
@ -1002,6 +1003,7 @@ DEPENDENCIES
pg (~> 1.5)
pghero
premailer-rails
prism
propshaft
public_suffix (~> 6.0)
puma (~> 6.3)

View File

@ -0,0 +1,83 @@
# frozen_string_literal: true
class Admin::Db::SchemaParser
Index = Data.define(:name, :table_name, :columns, :options)
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

View File

@ -8,6 +8,7 @@ class Admin::SystemCheck
Admin::SystemCheck::SidekiqProcessCheck,
Admin::SystemCheck::RulesCheck,
Admin::SystemCheck::ElasticsearchCheck,
Admin::SystemCheck::MissingIndexesCheck,
].freeze
def self.perform(current_user)

View File

@ -0,0 +1,36 @@
# 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)).compact
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

View File

@ -871,6 +871,8 @@ en:
elasticsearch_version_check:
message_html: 'Incompatible Elasticsearch version: %{value}'
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:
action: Manage server rules
message_html: You haven't defined any server rules.

View File

@ -0,0 +1,49 @@
# 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

View File

@ -0,0 +1,65 @@
# 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