Commit 070aeff1 authored by Rémy Coutable's avatar Rémy Coutable Committed by Ruben Davila

Merge branch 'hash-concurrent-foreign-key-names' into 'master'

Hash concurrent foreign key names similar to Rails

See merge request !9415
Conflicts:
	lib/gitlab/database/migration_helpers.rb
	spec/lib/gitlab/database/migration_helpers_spec.rb
parent 51d55c4b
...@@ -26,6 +26,63 @@ module Gitlab ...@@ -26,6 +26,63 @@ module Gitlab
add_index(table_name, column_name, options) add_index(table_name, column_name, options)
end end
# Adds a foreign key with only minimal locking on the tables involved.
#
# This method only requires minimal locking when using PostgreSQL. When
# using MySQL this method will use Rails' default `add_foreign_key`.
#
# source - The source table containing the foreign key.
# target - The target table the key points to.
# column - The name of the column to create the foreign key on.
# on_delete - The action to perform when associated data is removed,
# defaults to "CASCADE".
def add_concurrent_foreign_key(source, target, column:, on_delete: :cascade)
# Transactions would result in ALTER TABLE locks being held for the
# duration of the transaction, defeating the purpose of this method.
if transaction_open?
raise 'add_concurrent_foreign_key can not be run inside a transaction'
end
# While MySQL does allow disabling of foreign keys it has no equivalent
# of PostgreSQL's "VALIDATE CONSTRAINT". As a result we'll just fall
# back to the normal foreign key procedure.
if Database.mysql?
return add_foreign_key(source, target,
column: column,
on_delete: on_delete)
end
disable_statement_timeout
key_name = concurrent_foreign_key_name(source, column)
# Using NOT VALID allows us to create a key without immediately
# validating it. This means we keep the ALTER TABLE lock only for a
# short period of time. The key _is_ enforced for any newly created
# data.
execute <<-EOF.strip_heredoc
ALTER TABLE #{source}
ADD CONSTRAINT #{key_name}
FOREIGN KEY (#{column})
REFERENCES #{target} (id)
ON DELETE #{on_delete} NOT VALID;
EOF
# Validate the existing constraint. This can potentially take a very
# long time to complete, but fortunately does not lock the source table
# while running.
execute("ALTER TABLE #{source} VALIDATE CONSTRAINT #{key_name};")
end
# Returns the name for a concurrent foreign key.
#
# PostgreSQL constraint names have a limit of 63 bytes. The logic used
# here is based on Rails' foreign_key_name() method, which unfortunately
# is private so we can't rely on it directly.
def concurrent_foreign_key_name(table, column)
"fk_#{Digest::SHA256.hexdigest("#{table}_#{column}_fk").first(10)}"
end
# Long-running migrations may take more than the timeout allowed by # Long-running migrations may take more than the timeout allowed by
# the database. Disable the session's statement timeout to ensure # the database. Disable the session's statement timeout to ensure
# migrations don't get killed prematurely. (PostgreSQL only) # migrations don't get killed prematurely. (PostgreSQL only)
......
...@@ -59,6 +59,81 @@ describe Gitlab::Database::MigrationHelpers, lib: true do ...@@ -59,6 +59,81 @@ describe Gitlab::Database::MigrationHelpers, lib: true do
end end
end end
describe '#add_concurrent_foreign_key' do
context 'inside a transaction' do
it 'raises an error' do
expect(model).to receive(:transaction_open?).and_return(true)
expect do
model.add_concurrent_foreign_key(:projects, :users, column: :user_id)
end.to raise_error(RuntimeError)
end
end
context 'outside a transaction' do
before do
allow(model).to receive(:transaction_open?).and_return(false)
end
context 'using MySQL' do
it 'creates a regular foreign key' do
allow(Gitlab::Database).to receive(:mysql?).and_return(true)
expect(model).to receive(:add_foreign_key).
with(:projects, :users, column: :user_id, on_delete: :cascade)
model.add_concurrent_foreign_key(:projects, :users, column: :user_id)
end
end
context 'using PostgreSQL' do
before do
allow(Gitlab::Database).to receive(:mysql?).and_return(false)
end
it 'creates a concurrent foreign key' do
expect(model).to receive(:disable_statement_timeout)
expect(model).to receive(:execute).ordered.with(/NOT VALID/)
expect(model).to receive(:execute).ordered.with(/VALIDATE CONSTRAINT/)
model.add_concurrent_foreign_key(:projects, :users, column: :user_id)
end
end
end
end
describe '#concurrent_foreign_key_name' do
it 'returns the name for a foreign key' do
name = model.concurrent_foreign_key_name(:this_is_a_very_long_table_name,
:with_a_very_long_column_name)
expect(name).to be_an_instance_of(String)
expect(name.length).to eq(13)
end
end
describe '#disable_statement_timeout' do
context 'using PostgreSQL' do
it 'disables statement timeouts' do
expect(Gitlab::Database).to receive(:postgresql?).and_return(true)
expect(model).to receive(:execute).with('SET statement_timeout TO 0')
model.disable_statement_timeout
end
end
context 'using MySQL' do
it 'does nothing' do
expect(Gitlab::Database).to receive(:postgresql?).and_return(false)
expect(model).not_to receive(:execute)
model.disable_statement_timeout
end
end
end
describe '#update_column_in_batches' do describe '#update_column_in_batches' do
before do before do
create_list(:empty_project, 5) create_list(:empty_project, 5)
......
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