Skip to content

Commit

Permalink
Merge pull request #11 from freshworks/v2-lhm-trigger-drop-on-migrati…
Browse files Browse the repository at this point in the history
…on-fix

[FD-120307] : LHM Trigger Drop Fix
  • Loading branch information
bhuvanesh-sankar authored Feb 14, 2024
2 parents 90e6de8 + d78d79d commit ab53bca
Show file tree
Hide file tree
Showing 7 changed files with 144 additions and 4 deletions.
9 changes: 9 additions & 0 deletions lib/lhm/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,15 @@ def execute(sql)
def table_exists?(table_name)
@adapter.table_exists?(table_name)
end

def trigger_exists?(table_name, trigger_name)
!!select_one(%Q{
select trigger_name
from information_schema.triggers
where event_object_table='#{table_name}'
and trigger_name like '#{trigger_name}'
})
end
end
end
end
7 changes: 5 additions & 2 deletions lib/lhm/invoker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
require 'lhm/atomic_switcher'
require 'lhm/locked_switcher'
require 'lhm/migrator'
require 'lhm/trigger_switcher'

module Lhm
# Copies an origin table to an altered destination table. Live activity is
Expand All @@ -16,9 +17,10 @@ module Lhm
class Invoker
include SqlHelper

attr_reader :migrator, :connection
attr_reader :migrator, :connection, :origin

def initialize(origin, connection)
@origin = origin
@connection = connection
@migrator = Migrator.new(origin, connection)
end
Expand All @@ -40,8 +42,9 @@ def run(options = {})
Chunker.new(migration, @connection, options).run
if options[:atomic_switch]
AtomicSwitcher.new(migration, @connection).run
TriggerSwitcher.new.copy_triggers(@origin, migration.archive_name, @connection) if options[:retain_triggers]
else
LockedSwitcher.new(migration, @connection).run
LockedSwitcher.new(migration, @connection, options[:retain_triggers]).run
end
end
end
Expand Down
18 changes: 16 additions & 2 deletions lib/lhm/locked_switcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require 'lhm/command'
require 'lhm/migration'
require 'lhm/sql_helper'
require 'lhm/trigger_switcher'

module Lhm
# Switches origin with destination table nonatomically using a locked write.
Expand All @@ -22,25 +23,38 @@ class LockedSwitcher

attr_reader :connection

def initialize(migration, connection = nil)
def initialize(migration, connection = nil, retain_triggers = false)
@migration = migration
@connection = connection
@origin = migration.origin
@destination = migration.destination
@retain_triggers = retain_triggers
end

def statements
uncommitted { switch }
end

def switch
[
queries = [
"lock table `#{ @origin.name }` write, `#{ @destination.name }` write",
"alter table `#{ @origin.name }` rename `#{ @migration.archive_name }`",
"alter table `#{ @destination.name }` rename `#{ @origin.name }`",
"commit",
"unlock tables"
]
if @retain_triggers
trigger_copy_queries = []
trigger_copy_queries << "lock table `#{ @migration.archive_name }` write, `#{ @origin.name }` write"
@origin.triggers.each do |trigger|
trigger_name = trigger[0]
trigger_copy_queries << "DROP TRIGGER #{trigger_name};"
trigger_copy_queries << TriggerSwitcher.new.fetch_trigger_definition(trigger_name, connection)
end
target_index = queries.index('commit')
queries.insert(target_index, *trigger_copy_queries) if trigger_copy_queries.length > 0 # inserting the trigger queries before commit query
end
queries
end

def uncommitted(&block)
Expand Down
9 changes: 9 additions & 0 deletions lib/lhm/table.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@
module Lhm
class Table
attr_reader :name, :columns, :indices, :pk, :ddl
attr_accessor :triggers

def initialize(name, pk = "id", ddl = nil)
@name = name
@columns = {}
@indices = {}
@triggers = []
@pk = valid_primary_key(pk)
@ddl = ddl
end
Expand Down Expand Up @@ -69,11 +71,18 @@ def parse
extract_indices(read_indices).each do |idx, columns|
table.indices[idx] = columns
end

table.triggers = read_triggers
end
end

private

def read_triggers
sql = "show triggers where `table`= '#{@table_name}';"
@connection.execute(sql)
end

def read_information_schema
@connection.select_all %Q{
select *
Expand Down
36 changes: 36 additions & 0 deletions lib/lhm/trigger_switcher.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
module Lhm
class TriggerSwitcher
def copy_triggers(source_table, destination_table_name, connection)
return unless !destination_table_name.empty? && connection.table_exists?(destination_table_name)

source_table.triggers.each do |trigger|
trigger_name = trigger[0]
trigger_definition = fetch_trigger_definition(trigger_name, connection)
# since source table is renamed to lhma_table_name(destination table name) after migration, triggers associated with the source table will now be associated with lhma_table_name.
# skip if the trigger isn't associated with lhma_table_name.
next unless trigger_definition.include? destination_table_name

old_trigger_name = trigger_name
trigger_name = remove_timestamp_from_trigger_name_if_exists(trigger_name)
modified_trigger_name = trigger_name + '_' + Time.now.getutc.to_s.gsub(/[:\- ]/, '_') #sample trigger name - trigger_name_2023_12_18_03_40_45_UTC
modified_definition = trigger_definition.gsub(destination_table_name, source_table.name)
modified_definition = modified_definition.gsub(old_trigger_name, modified_trigger_name)
connection.execute(modified_definition)
connection.execute("DROP TRIGGER #{old_trigger_name};")
end
end

def fetch_trigger_definition(trigger_name, connection)
result = connection.execute("SHOW CREATE TRIGGER #{trigger_name};").first
# sample result => ["update_message_old_5", "NO_ENGINE_SUBSTITUTION", "CREATE DEFINER=`root`@`localhost` TRIGGER update_message_old_5 BEFORE INSERT ON `lhma_2023_09_04_14_32_54_810_logs_table` FOR EACH ROW\n BEGIN\n IF NEW.message = '1' THEN\n SET NEW.message = 'true';\n END IF;\n END", "utf8", "utf8_general_ci", "utf8_unicode_ci", 2023-09-04 14:32:43 UTC]
# create trigger definition will be stored in the third index in the result, ideally result will always be of length 7 if its executed successfully.
result && result.length > 2 ? result[2] : ''
end

def remove_timestamp_from_trigger_name_if_exists(trigger_name)
timestamp_suffix_regex = /_(\d{4}_\d{2}_\d{2}_\d{2}_\d{2}_\d{2}_UTC)\z/
match_data = trigger_name.match(/^(.*?)(?:#{timestamp_suffix_regex})$/)
match_data ? match_data[1] : trigger_name
end
end
end
16 changes: 16 additions & 0 deletions spec/integration/integration_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,22 @@ def table_exists?(table)
connection.table_exists?(table.name)
end

def trigger_exists?(table_name, trigger_name)
connection.trigger_exists?(table_name, trigger_name)
end

def check_and_create_trigger_on_users_table
table_name = 'users'
trigger_name = 'timestamp_trigger'
unless trigger_exists?(table_name, trigger_name)
query = "CREATE TRIGGER #{trigger_name} BEFORE INSERT ON #{table_name} FOR EACH ROW
BEGIN
SET NEW.created_at = NOW();
END;"
execute query
end
end

#
# Database Helpers
#
Expand Down
53 changes: 53 additions & 0 deletions spec/integration/lhm_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,59 @@
end
end

it "should retain triggers while migration if retain_triggers is set to true in atomic switch" do
table_name = 'users'
trigger_name = 'timestamp_trigger'
check_and_create_trigger_on_users_table
Lhm.change_table(table_name.to_sym, :atomic_switch => true, :cleanup => true, :retain_triggers => true) do |t|
t.add_column(:logins, "INT(12) DEFAULT '0'")
end

slave do
trigger_exists?(table_name, trigger_name).must_equal true
end
end

it "should retain triggers while migration if retain_triggers is set to true in locked switch" do
table_name = 'users'
trigger_name = 'timestamp_trigger'
check_and_create_trigger_on_users_table
Lhm.change_table(table_name.to_sym, :atomic_switch => false, :cleanup => true, :retain_triggers => true) do |t|
t.add_column(:logins, "INT(12) DEFAULT '0'")
end

slave do
trigger_exists?(table_name, trigger_name).must_equal true
end
end

it "should not retain triggers while migration if retain_triggers is set to false" do
table_name = 'users'
trigger_name = 'timestamp_trigger'
check_and_create_trigger_on_users_table
Lhm.change_table(table_name.to_sym, :atomic_switch => false, :cleanup => true, :retain_triggers => false) do |t|
t.add_column(:logins_1, "INT(12) DEFAULT '0'")
end

slave do
trigger_exists?(table_name, trigger_name).must_equal false
end
end

it "should not retain triggers while migration if retain_triggers is not passed in the options" do
table_name = 'users'
trigger_name = 'timestamp_trigger'
check_and_create_trigger_on_users_table
Lhm.change_table(table_name.to_sym, :atomic_switch => false, :cleanup => true) do |t|
t.add_column(:logins_2, "INT(12) DEFAULT '0'")
end

slave do
trigger_exists?(table_name, trigger_name).must_equal false
end
end


it "should copy all rows" do
23.times { |n| execute("insert into users set reference = '#{ n }'") }

Expand Down

0 comments on commit ab53bca

Please sign in to comment.