Skip to content

Commit 5d29711

Browse files
authored
Replace jobs_user_guid_index with jobs_user_guid_state_index (#3934)
When the jobs table is large and single users have also a large amount of jobs, which are mostly in state `COMPLETED`, postgres might decide to not use the index on `user_guid` column and scan the table with other methods, which are much slower. This commit removes the index on user_guid column and adds a new partial index on user_guid and state where state is either `POLLING` or `PROCESSING`. This keeps the index much smaller and improves the query performance drastically. MySQL does not support partial indexes.
1 parent 9683d6c commit 5d29711

File tree

2 files changed

+121
-0
lines changed

2 files changed

+121
-0
lines changed
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
Sequel.migration do
2+
# adding an index concurrently cannot be done within a transaction
3+
no_transaction
4+
5+
up do
6+
if database_type == :postgres
7+
VCAP::Migration.with_concurrent_timeout(self) do
8+
drop_index :jobs, :user_guid, name: :jobs_user_guid_index, if_exists: true, concurrently: true
9+
add_index :jobs, %i[user_guid state], name: :jobs_user_guid_state_index, where: "state IN ('PROCESSING', 'POLLING')", if_not_exists: true, concurrently: true
10+
end
11+
end
12+
end
13+
14+
down do
15+
if database_type == :postgres
16+
VCAP::Migration.with_concurrent_timeout(self) do
17+
drop_index :jobs, %i[user_guid state], name: :jobs_user_guid_state_index, if_exists: true, concurrently: true
18+
add_index :jobs, :user_guid, name: :jobs_user_guid_index, if_not_exists: true, concurrently: true
19+
end
20+
end
21+
end
22+
end
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
require 'spec_helper'
2+
require 'migrations/helpers/migration_shared_context'
3+
4+
def partial_index_present
5+
# partial indexes are not returned in `db.indexes`. That's why we have to query this information manually.
6+
partial_indexes = db.fetch("SELECT * FROM pg_indexes WHERE tablename = 'jobs' AND indexname = 'jobs_user_guid_state_index';")
7+
8+
index_present = false
9+
partial_indexes.each do |_index|
10+
index_present = true
11+
end
12+
13+
index_present
14+
end
15+
16+
RSpec.describe 'migration to replace user_guid_index with user_guid_state_index on jobs table', isolation: :truncation, type: :migration do
17+
include_context 'migration' do
18+
let(:migration_filename) { '20240820070742_add_jobs_user_guid_state_index.rb' }
19+
end
20+
21+
describe 'jobs table' do
22+
it 'removes index `jobs_user_guid_index`' do
23+
skip if db.database_type != :postgres
24+
expect(db.indexes(:jobs)).to include(:jobs_user_guid_index)
25+
expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
26+
expect(db.indexes(:jobs)).not_to include(:jobs_user_guid_index)
27+
end
28+
29+
it 'adds an index `jobs_user_guid_state_index`' do
30+
skip if db.database_type != :postgres
31+
expect(partial_index_present).to be_falsey
32+
expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
33+
expect(partial_index_present).to be_truthy
34+
end
35+
36+
describe 'idempotency of up' do
37+
context '`jobs_user_guid_index` does not exist' do
38+
before do
39+
skip if db.database_type != :postgres
40+
db.drop_index :jobs, :user_guid, name: :jobs_user_guid_index
41+
end
42+
43+
it 'continues to create the index' do
44+
expect(db.indexes(:jobs)).not_to include(:jobs_user_guid_index)
45+
expect(partial_index_present).to be_falsey
46+
expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
47+
expect(partial_index_present).to be_truthy
48+
expect(db.indexes(:jobs)).not_to include(:jobs_user_guid_index)
49+
end
50+
end
51+
52+
context '`jobs_user_guid_state_index` already exists' do
53+
before do
54+
skip if db.database_type != :postgres
55+
db.add_index :jobs, %i[user_guid state], name: :jobs_user_guid_state_index, where: "state IN ('PROCESSING', 'POLLING')"
56+
end
57+
58+
it 'does not fail' do
59+
expect(partial_index_present).to be_truthy
60+
expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
61+
end
62+
end
63+
end
64+
65+
describe 'idempotency of down' do
66+
context '`jobs_user_guid_state_index` does not exist' do
67+
before do
68+
skip if db.database_type != :postgres
69+
Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true)
70+
db.drop_index :jobs, %i[user_guid state], name: :jobs_user_guid_state_index
71+
end
72+
73+
it 'restores `jobs_user_guid_index`' do
74+
expect(partial_index_present).to be_falsey
75+
expect(db.indexes(:jobs)).not_to include(:jobs_user_guid_index)
76+
expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error
77+
expect(partial_index_present).to be_falsey
78+
expect(db.indexes(:jobs)).to include(:jobs_user_guid_index)
79+
end
80+
end
81+
82+
context '`jobs_user_guid_index` already exists' do
83+
before do
84+
skip if db.database_type != :postgres
85+
Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true)
86+
db.add_index :jobs, :user_guid, name: :jobs_user_guid_index
87+
end
88+
89+
it 'does not fail' do
90+
expect(db.indexes(:jobs)).to include(:jobs_user_guid_index)
91+
expect(partial_index_present).to be_truthy
92+
expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error
93+
expect(partial_index_present).to be_falsey
94+
expect(db.indexes(:jobs)).to include(:jobs_user_guid_index)
95+
end
96+
end
97+
end
98+
end
99+
end

0 commit comments

Comments
 (0)