Repository: github/gh-ost

Issue:
Backport openark/gh-ost/pull/30 to this repo - add triggers support
Backport the changes from https://github.com/openark/gh-ost/pull/30 to this repo
This change will add support for triggers, it's in our production for long time and working perfect.
I'll also add localtests. 

Thank you, Yakir. 

Discussion:

Solution patch:
diff --git a/go/base/context.go b/go/base/context.go
index f901716..8a09c43 100644
--- a/go/base/context.go
+++ b/go/base/context.go
@@ -14,6 +14,7 @@ import (
 	"sync"
 	"sync/atomic"
 	"time"
+	"unicode/utf8"
 
 	uuid "github.com/google/uuid"
 
@@ -237,6 +238,11 @@ type MigrationContext struct {
 	MigrationIterationRangeMaxValues *sql.ColumnValues
 	ForceTmpTableName                string
 
+	IncludeTriggers     bool
+	RemoveTriggerSuffix bool
+	TriggerSuffix       string
+	Triggers            []mysql.Trigger
+
 	recentBinlogCoordinates mysql.BinlogCoordinates
 
 	BinlogSyncerMaxReconnectAttempts int
@@ -924,3 +930,20 @@ func (this *MigrationContext) ReadConfigFile() error {
 
 	return nil
 }
+
+// getGhostTriggerName generates the name of a ghost trigger, based on original trigger name
+// or a given trigger name
+func (this *MigrationContext) GetGhostTriggerName(triggerName string) string {
+	if this.RemoveTriggerSuffix && strings.HasSuffix(triggerName, this.TriggerSuffix) {
+		return strings.TrimSuffix(triggerName, this.TriggerSuffix)
+	}
+	// else
+	return triggerName + this.TriggerSuffix
+}
+
+// validateGhostTriggerLength check if the ghost trigger name length is not more than 64 characters
+func (this *MigrationContext) ValidateGhostTriggerLengthBelowMaxLength(triggerName string) bool {
+	ghostTriggerName := this.GetGhostTriggerName(triggerName)
+
+	return utf8.RuneCountInString(ghostTriggerName) <= mysql.MaxTableNameLength
+}
diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go
index 0829429..2256763 100644
--- a/go/cmd/gh-ost/main.go
+++ b/go/cmd/gh-ost/main.go
@@ -11,6 +11,7 @@ import (
 	"net/url"
 	"os"
 	"os/signal"
+	"regexp"
 	"syscall"
 
 	"github.com/github/gh-ost/go/base"
@@ -137,6 +138,10 @@ func main() {
 	flag.UintVar(&migrationContext.ReplicaServerId, "replica-server-id", 99999, "server id used by gh-ost process. Default: 99999")
 	flag.IntVar(&migrationContext.BinlogSyncerMaxReconnectAttempts, "binlogsyncer-max-reconnect-attempts", 0, "when master node fails, the maximum number of binlog synchronization attempts to reconnect. 0 is unlimited")
 
+	flag.BoolVar(&migrationContext.IncludeTriggers, "include-triggers", false, "When true, the triggers (if exist) will be created on the new table")
+	flag.StringVar(&migrationContext.TriggerSuffix, "trigger-suffix", "", "Add a suffix to the trigger name (i.e '_v2'). Requires '--include-triggers'")
+	flag.BoolVar(&migrationContext.RemoveTriggerSuffix, "remove-trigger-suffix-if-exists", false, "Remove given suffix from name of trigger. Requires '--include-triggers' and '--trigger-suffix'")
+
 	maxLoad := flag.String("max-load", "", "Comma delimited status-name=threshold. e.g: 'Threads_running=100,Threads_connected=500'. When status exceeds threshold, app throttles writes")
 	criticalLoad := flag.String("critical-load", "", "Comma delimited status-name=threshold, same format as --max-load. When status exceeds threshold, app panics and quits")
 	flag.Int64Var(&migrationContext.CriticalLoadIntervalMilliseconds, "critical-load-interval-millis", 0, "When 0, migration immediately bails out upon meeting critical-load. When non-zero, a second check is done after given interval, and migration only bails out if 2nd check still meets critical load")
@@ -257,7 +262,20 @@ func main() {
 		migrationContext.Log.Fatal("--ssl-allow-insecure requires --ssl")
 	}
 	if *replicationLagQuery != "" {
-		migrationContext.Log.Warning("--replication-lag-query is deprecated")
+		migrationContext.Log.Warningf("--replication-lag-query is deprecated")
+	}
+	if migrationContext.IncludeTriggers && migrationContext.TriggerSuffix == "" {
+		migrationContext.Log.Fatalf("--trigger-suffix must be used with --include-triggers")
+	}
+	if !migrationContext.IncludeTriggers && migrationContext.TriggerSuffix != "" {
+		migrationContext.Log.Fatalf("--trigger-suffix cannot be be used without --include-triggers")
+	}
+	if migrationContext.TriggerSuffix != "" {
+		regex := regexp.MustCompile(`^[\da-zA-Z_]+$`)
+
+		if !regex.Match([]byte(migrationContext.TriggerSuffix)) {
+			migrationContext.Log.Fatalf("--trigger-suffix must contain only alpha numeric characters and underscore (0-9,a-z,A-Z,_)")
+		}
 	}
 	if *storageEngine == "rocksdb" {
 		migrationContext.Log.Warning("RocksDB storage engine support is experimental")
diff --git a/go/logic/applier.go b/go/logic/applier.go
index b647d5d..6c8ac71 100644
--- a/go/logic/applier.go
+++ b/go/logic/applier.go
@@ -415,6 +415,56 @@ func (this *Applier) dropTable(tableName string) error {
 	return nil
 }
 
+// dropTriggers drop the triggers on the applied host
+func (this *Applier) DropTriggersFromGhost() error {
+	if len(this.migrationContext.Triggers) > 0 {
+		for _, trigger := range this.migrationContext.Triggers {
+			triggerName := this.migrationContext.GetGhostTriggerName(trigger.Name)
+			query := fmt.Sprintf("drop trigger if exists %s", sql.EscapeName(triggerName))
+			_, err := sqlutils.ExecNoPrepare(this.db, query)
+			if err != nil {
+				return err
+			}
+			this.migrationContext.Log.Infof("Trigger '%s' dropped", triggerName)
+		}
+	}
+	return nil
+}
+
+// createTriggers creates the triggers on the applied host
+func (this *Applier) createTriggers(tableName string) error {
+	if len(this.migrationContext.Triggers) > 0 {
+		for _, trigger := range this.migrationContext.Triggers {
+			triggerName := this.migrationContext.GetGhostTriggerName(trigger.Name)
+			query := fmt.Sprintf(`create /* gh-ost */ trigger %s %s %s on %s.%s for each row
+		%s`,
+				sql.EscapeName(triggerName),
+				trigger.Timing,
+				trigger.Event,
+				sql.EscapeName(this.migrationContext.DatabaseName),
+				sql.EscapeName(tableName),
+				trigger.Statement,
+			)
+			this.migrationContext.Log.Infof("Createing trigger %s on %s.%s",
+				sql.EscapeName(triggerName),
+				sql.EscapeName(this.migrationContext.DatabaseName),
+				sql.EscapeName(tableName),
+			)
+			if _, err := sqlutils.ExecNoPrepare(this.db, query); err != nil {
+				return err
+			}
+		}
+		this.migrationContext.Log.Infof("Triggers created on %s", tableName)
+	}
+	return nil
+}
+
+// CreateTriggers creates the original triggers on applier host
+func (this *Applier) CreateTriggersOnGhost() error {
+	err := this.createTriggers(this.migrationContext.GetGhostTableName())
+	return err
+}
+
 // DropChangelogTable drops the changelog table on the applier host
 func (this *Applier) DropChangelogTable() error {
 	return this.dropTable(this.migrationContext.GetChangelogTableName())
diff --git a/go/logic/inspect.go b/go/logic/inspect.go
index 7e1c08e..aec95cd 100644
--- a/go/logic/inspect.go
+++ b/go/logic/inspect.go
@@ -531,7 +531,7 @@ func (this *Inspector) validateTableForeignKeys(allowChildForeignKeys bool) erro
 	return nil
 }
 
-// validateTableTriggers makes sure no triggers exist on the migrated table
+// validateTableTriggers makes sure no triggers exist on the migrated table. if --include_triggers is used then it fetches the triggers
 func (this *Inspector) validateTableTriggers() error {
 	query := `
 		SELECT /* gh-ost */ COUNT(*) AS num_triggers
@@ -553,12 +553,72 @@ func (this *Inspector) validateTableTriggers() error {
 		return err
 	}
 	if numTriggers > 0 {
-		return this.migrationContext.Log.Errorf("Found triggers on %s.%s. Triggers are not supported at this time. Bailing out", sql.EscapeName(this.migrationContext.DatabaseName), sql.EscapeName(this.migrationContext.OriginalTableName))
+		if this.migrationContext.IncludeTriggers {
+			this.migrationContext.Log.Infof("Found %d triggers on %s.%s.", numTriggers, sql.EscapeName(this.migrationContext.DatabaseName), sql.EscapeName(this.migrationContext.OriginalTableName))
+			this.migrationContext.Triggers, err = mysql.GetTriggers(this.db, this.migrationContext.DatabaseName, this.migrationContext.OriginalTableName)
+			if err != nil {
+				return err
+			}
+			if err := this.validateGhostTriggersDontExist(); err != nil {
+				return err
+			}
+			if err := this.validateGhostTriggersLength(); err != nil {
+				return err
+			}
+			return nil
+		}
+		return this.migrationContext.Log.Errorf("Found triggers on %s.%s. Tables with triggers are supported only when using \"include-triggers\" flag. Bailing out", sql.EscapeName(this.migrationContext.DatabaseName), sql.EscapeName(this.migrationContext.OriginalTableName))
 	}
 	this.migrationContext.Log.Debugf("Validated no triggers exist on table")
 	return nil
 }
 
+// verifyTriggersDontExist verifies before createing new triggers we want to make sure these triggers dont exist already in the DB
+func (this *Inspector) validateGhostTriggersDontExist() error {
+	if len(this.migrationContext.Triggers) > 0 {
+		var foundTriggers []string
+		for _, trigger := range this.migrationContext.Triggers {
+			triggerName := this.migrationContext.GetGhostTriggerName(trigger.Name)
+			query := "select 1 from information_schema.triggers where trigger_name = ? and trigger_schema = ? and event_object_table = ?"
+			err := sqlutils.QueryRowsMap(this.db, query, func(rowMap sqlutils.RowMap) error {
+				triggerExists := rowMap.GetInt("1")
+				if triggerExists == 1 {
+					foundTriggers = append(foundTriggers, triggerName)
+				}
+				return nil
+			},
+				triggerName,
+				this.migrationContext.DatabaseName,
+				this.migrationContext.OriginalTableName,
+			)
+			if err != nil {
+				return err
+			}
+		}
+		if len(foundTriggers) > 0 {
+			return this.migrationContext.Log.Errorf("Found gh-ost triggers (%s). Please use a different suffix or drop them. Bailing out", strings.Join(foundTriggers, ","))
+		}
+	}
+
+	return nil
+}
+
+func (this *Inspector) validateGhostTriggersLength() error {
+	if len(this.migrationContext.Triggers) > 0 {
+		var foundTriggers []string
+		for _, trigger := range this.migrationContext.Triggers {
+			triggerName := this.migrationContext.GetGhostTriggerName(trigger.Name)
+			if ok := this.migrationContext.ValidateGhostTriggerLengthBelowMaxLength(triggerName); !ok {
+				foundTriggers = append(foundTriggers, triggerName)
+			}
+		}
+		if len(foundTriggers) > 0 {
+			return this.migrationContext.Log.Errorf("Gh-ost triggers (%s) length > %d characters. Bailing out", strings.Join(foundTriggers, ","), mysql.MaxTableNameLength)
+		}
+	}
+	return nil
+}
+
 // estimateTableRowsViaExplain estimates number of rows on original table
 func (this *Inspector) estimateTableRowsViaExplain() error {
 	query := fmt.Sprintf(`explain select /* gh-ost */ * from %s.%s where 1=1`, sql.EscapeName(this.migrationContext.DatabaseName), sql.EscapeName(this.migrationContext.OriginalTableName))
diff --git a/go/logic/migrator.go b/go/logic/migrator.go
index d5f81f3..5e4e4ec 100644
--- a/go/logic/migrator.go
+++ b/go/logic/migrator.go
@@ -632,6 +632,12 @@ func (this *Migrator) cutOverTwoStep() (err error) {
 	if err := this.retryOperation(this.waitForEventsUpToLock); err != nil {
 		return err
 	}
+	// If we need to create triggers we need to do it here (only create part)
+	if this.migrationContext.IncludeTriggers && len(this.migrationContext.Triggers) > 0 {
+		if err := this.retryOperation(this.applier.CreateTriggersOnGhost); err != nil {
+			return err
+		}
+	}
 	if err := this.retryOperation(this.applier.SwapTablesQuickAndBumpy); err != nil {
 		return err
 	}
@@ -676,6 +682,13 @@ func (this *Migrator) atomicCutOver() (err error) {
 		return this.migrationContext.Log.Errore(err)
 	}
 
+	// If we need to create triggers we need to do it here (only create part)
+	if this.migrationContext.IncludeTriggers && len(this.migrationContext.Triggers) > 0 {
+		if err := this.applier.CreateTriggersOnGhost(); err != nil {
+			this.migrationContext.Log.Errore(err)
+		}
+	}
+
 	// Step 2
 	// We now attempt an atomic RENAME on original & ghost tables, and expect it to block.
 	this.migrationContext.RenameTablesStartTime = time.Now()
diff --git a/go/mysql/utils.go b/go/mysql/utils.go
index 531f498..9217fb3 100644
--- a/go/mysql/utils.go
+++ b/go/mysql/utils.go
@@ -30,6 +30,13 @@ type ReplicationLagResult struct {
 	Err error
 }
 
+type Trigger struct {
+	Name      string
+	Event     string
+	Statement string
+	Timing    string
+}
+
 func NewNoReplicationLagResult() *ReplicationLagResult {
 	return &ReplicationLagResult{Lag: 0, Err: nil}
 }
@@ -224,3 +231,24 @@ func Kill(db *gosql.DB, connectionID string) error {
 	_, err := db.Exec(`KILL QUERY %s`, connectionID)
 	return err
 }
+
+// GetTriggers reads trigger list from given table
+func GetTriggers(db *gosql.DB, databaseName, tableName string) (triggers []Trigger, err error) {
+	query := fmt.Sprintf(`select trigger_name as name, event_manipulation as event, action_statement as statement, action_timing as timing
+	from information_schema.triggers 
+	where trigger_schema = '%s' and event_object_table = '%s'`, databaseName, tableName)
+
+	err = sqlutils.QueryRowsMap(db, query, func(rowMap sqlutils.RowMap) error {
+		triggers = append(triggers, Trigger{
+			Name:      rowMap.GetString("name"),
+			Event:     rowMap.GetString("event"),
+			Statement: rowMap.GetString("statement"),
+			Timing:    rowMap.GetString("timing"),
+		})
+		return nil
+	})
+	if err != nil {
+		return nil, err
+	}
+	return triggers, nil
+}



Test patch:
diff --git a/go/base/context_test.go b/go/base/context_test.go
index 0abba90..f3323b9 100644
--- a/go/base/context_test.go
+++ b/go/base/context_test.go
@@ -7,6 +7,7 @@ package base
 
 import (
 	"os"
+	"strings"
 	"testing"
 	"time"
 
@@ -58,6 +59,68 @@ func TestGetTableNames(t *testing.T) {
 	}
 }
 
+func TestGetTriggerNames(t *testing.T) {
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_gho"
+		require.Equal(t, "my_trigger"+context.TriggerSuffix, context.GetGhostTriggerName("my_trigger"))
+	}
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_gho"
+		context.RemoveTriggerSuffix = true
+		require.Equal(t, "my_trigger"+context.TriggerSuffix, context.GetGhostTriggerName("my_trigger"))
+	}
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_gho"
+		context.RemoveTriggerSuffix = true
+		require.Equal(t, "my_trigger", context.GetGhostTriggerName("my_trigger_gho"))
+	}
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_gho"
+		context.RemoveTriggerSuffix = false
+		require.Equal(t, "my_trigger_gho_gho", context.GetGhostTriggerName("my_trigger_gho"))
+	}
+}
+
+func TestValidateGhostTriggerLengthBelowMaxLength(t *testing.T) {
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_gho"
+		require.True(t, context.ValidateGhostTriggerLengthBelowMaxLength("my_trigger"))
+	}
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_ghost"
+		require.False(t, context.ValidateGhostTriggerLengthBelowMaxLength(strings.Repeat("my_trigger_ghost", 4))) // 64 characters + "_ghost"
+	}
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_ghost"
+		require.True(t, context.ValidateGhostTriggerLengthBelowMaxLength(strings.Repeat("my_trigger_ghost", 3))) // 48 characters + "_ghost"
+	}
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_ghost"
+		context.RemoveTriggerSuffix = true
+		require.True(t, context.ValidateGhostTriggerLengthBelowMaxLength(strings.Repeat("my_trigger_ghost", 4))) // 64 characters + "_ghost" removed
+	}
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_ghost"
+		context.RemoveTriggerSuffix = true
+		require.False(t, context.ValidateGhostTriggerLengthBelowMaxLength(strings.Repeat("my_trigger_ghost", 4)+"X")) // 65 characters + "_ghost" not removed
+	}
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_ghost"
+		context.RemoveTriggerSuffix = true
+		require.True(t, context.ValidateGhostTriggerLengthBelowMaxLength(strings.Repeat("my_trigger_ghost", 4)+"_ghost")) // 70 characters + last "_ghost"  removed
+	}
+}
+
 func TestReadConfigFile(t *testing.T) {
 	{
 		context := NewMigrationContext()
diff --git a/localtests/trigger-advanced-features/create.sql b/localtests/trigger-advanced-features/create.sql
new file mode 100644
index 0000000..91e393b
--- /dev/null
+++ b/localtests/trigger-advanced-features/create.sql
@@ -0,0 +1,148 @@
+-- Drop triggers to ensure a clean slate
+drop trigger if exists gh_ost_test_bi;
+drop trigger if exists gh_ost_test_ai;
+drop trigger if exists gh_ost_test_bu;
+drop trigger if exists gh_ost_test_au;
+drop trigger if exists gh_ost_test_bd;
+drop trigger if exists gh_ost_test_ad;
+
+-- Drop tables
+drop table if exists gh_ost_test_log;
+drop table if exists gh_ost_test_stats;
+drop table if exists gh_ost_test;
+
+-- Create main table
+create table gh_ost_test (
+  id int auto_increment,
+  i int not null,
+  color varchar(32),
+  ts timestamp default current_timestamp,
+  modified_count int default 0,
+  primary key(id)
+) auto_increment=1;
+
+-- Create log table for trigger operations
+create table gh_ost_test_log (
+  id int auto_increment,
+  test_id int,
+  trigger_name varchar(128),
+  action varchar(16),
+  ts timestamp default current_timestamp,
+  primary key(id)
+);
+
+-- Create stats table for complex test
+create table gh_ost_test_stats (
+  id int auto_increment,
+  event_type varchar(32),
+  i_sum int not null default 0,
+  count_events int not null default 0,
+  primary key(id),
+  unique key event_type_uidx(event_type)
+);
+
+-- Create all types of triggers (BEFORE/AFTER with INSERT/UPDATE/DELETE)
+-- Multiple trigger types from trigger-multiple test
+
+-- BEFORE INSERT
+delimiter ;;
+create trigger gh_ost_test_bi before insert on gh_ost_test
+for each row
+begin
+  insert into gh_ost_test_log (test_id, trigger_name, action) values (NEW.id, 'gh_ost_test_bi', 'BEFORE_INSERT');
+end ;;
+delimiter ;
+
+-- AFTER INSERT
+delimiter ;;
+create trigger gh_ost_test_ai after insert on gh_ost_test
+for each row
+begin
+  insert into gh_ost_test_log (test_id, trigger_name, action) values (NEW.id, 'gh_ost_test_ai', 'AFTER_INSERT');
+  
+  -- Complex logic from trigger-complex
+  insert into gh_ost_test_stats (event_type, i_sum, count_events) 
+  values ('insert', NEW.i, 1) 
+  on duplicate key update i_sum=i_sum+NEW.i, count_events=count_events+1;
+end ;;
+delimiter ;
+
+-- BEFORE UPDATE
+delimiter ;;
+create trigger gh_ost_test_bu before update on gh_ost_test
+for each row
+begin
+  insert into gh_ost_test_log (test_id, trigger_name, action) values (NEW.id, 'gh_ost_test_bu', 'BEFORE_UPDATE');
+end ;;
+delimiter ;
+
+-- AFTER UPDATE
+delimiter ;;
+create trigger gh_ost_test_au after update on gh_ost_test
+for each row
+begin
+  insert into gh_ost_test_log (test_id, trigger_name, action) values (NEW.id, 'gh_ost_test_au', 'AFTER_UPDATE');
+  
+  -- Complex logic from trigger-complex
+  insert into gh_ost_test_stats (event_type, i_sum, count_events) 
+  values ('update', NEW.i-OLD.i, 1) 
+  on duplicate key update i_sum=i_sum+(NEW.i-OLD.i), count_events=count_events+1;
+
+  if NEW.color != OLD.color then
+    update gh_ost_test set modified_count = modified_count + 1 where id = NEW.id;
+  end if;
+end ;;
+delimiter ;
+
+-- BEFORE DELETE
+delimiter ;;
+create trigger gh_ost_test_bd before delete on gh_ost_test
+for each row
+begin
+  insert into gh_ost_test_log (test_id, trigger_name, action) values (OLD.id, 'gh_ost_test_bd', 'BEFORE_DELETE');
+end ;;
+delimiter ;
+
+-- AFTER DELETE
+delimiter ;;
+create trigger gh_ost_test_ad after delete on gh_ost_test
+for each row
+begin
+  insert into gh_ost_test_log (test_id, trigger_name, action) values (OLD.id, 'gh_ost_test_ad', 'AFTER_DELETE');
+  
+  -- Complex logic from trigger-complex
+  insert into gh_ost_test_stats (event_type, i_sum, count_events) 
+  values ('delete', -OLD.i, 1) 
+  on duplicate key update i_sum=i_sum-OLD.i, count_events=count_events+1;
+end ;;
+delimiter ;
+
+-- Insert initial data
+insert into gh_ost_test values (null, 11, 'red', null, 0);
+insert into gh_ost_test values (null, 13, 'green', null, 0);
+insert into gh_ost_test values (null, 17, 'blue', null, 0);
+
+-- Create event to test all trigger types with complex operations
+drop event if exists gh_ost_test;
+delimiter ;;
+create event gh_ost_test
+  on schedule every 1 second
+  starts current_timestamp
+  ends current_timestamp + interval 60 second
+  on completion not preserve
+  enable
+  do
+begin
+  -- Test INSERT triggers
+  insert into gh_ost_test values (null, 23, 'red', null, 0);
+  
+  -- Test UPDATE triggers with numeric change (for stats)
+  update gh_ost_test set i=i+1 where id=1;
+  
+  -- Test UPDATE triggers with color change
+  update gh_ost_test set color='yellow' where color='red' limit 1;
+  
+  -- Test DELETE triggers
+  delete from gh_ost_test where id=2;
+end ;;
+delimiter ; 
diff --git a/localtests/trigger-advanced-features/extra_args b/localtests/trigger-advanced-features/extra_args
new file mode 100644
index 0000000..495fe21
--- /dev/null
+++ b/localtests/trigger-advanced-features/extra_args
@@ -0,0 +1 @@
+--include-triggers --trigger-suffix=_ght --remove-trigger-suffix-if-exists 
\ No newline at end of file