From a62f9e0754825433af33ac6bea233f173a1d06e9 Mon Sep 17 00:00:00 2001 From: Paulo Bittencourt Date: Mon, 15 Aug 2016 12:56:17 -0400 Subject: [PATCH 01/29] Add --test-on-replica-manual-replication-control flag This will wait indefinitely for the replication status to change. This allows us to run test schema changes in RDS without needing custom RDS commands in gh-ost. --- go/base/context.go | 15 ++++++++------- go/cmd/gh-ost/main.go | 8 ++++++++ go/logic/applier.go | 37 ++++++++++++++++++++++++++++++++----- 3 files changed, 48 insertions(+), 12 deletions(-) diff --git a/go/base/context.go b/go/base/context.go index 4264de8..e2ff510 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -82,13 +82,14 @@ type MigrationContext struct { ServeSocketFile string ServeTCPPort int64 - Noop bool - TestOnReplica bool - MigrateOnReplica bool - OkToDropTable bool - InitiallyDropOldTable bool - InitiallyDropGhostTable bool - CutOverType CutOver + Noop bool + TestOnReplica bool + MigrateOnReplica bool + ManualReplicationControl bool + OkToDropTable bool + InitiallyDropOldTable bool + InitiallyDropGhostTable bool + CutOverType CutOver TableEngine string RowsEstimate int64 diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index def84a3..3ca7ad5 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -60,6 +60,7 @@ func main() { flag.BoolVar(&migrationContext.SkipRenamedColumns, "skip-renamed-columns", false, "in case your `ALTER` statement renames columns, gh-ost will note that and offer its interpretation of the rename. By default gh-ost does not proceed to execute. This flag tells gh-ost to skip the renamed columns, i.e. to treat what gh-ost thinks are renamed columns as unrelated columns. NOTE: you may lose column data") executeFlag := flag.Bool("execute", false, "actually execute the alter & migrate the table. Default is noop: do some tests and exit") + testOnReplicaWithManualReplicationControl := flag.Bool("test-on-replica-manual-replication-control", false, "Same as --test-on-replica, but waits for replication to be stopped, instead of stopping it automatically. (Useful in RDS.)") flag.BoolVar(&migrationContext.TestOnReplica, "test-on-replica", false, "Have the migration run on a replica, not on the master. At the end of migration replication is stopped, and tables are swapped and immediately swap-revert. Replication remains stopped and you can compare the two tables for building trust") flag.BoolVar(&migrationContext.MigrateOnReplica, "migrate-on-replica", false, "Have the migration run on a replica, not on the master. This will do the full migration on the replica including cut-over (as opposed to --test-on-replica)") @@ -149,6 +150,13 @@ func main() { if migrationContext.SwitchToRowBinlogFormat && migrationContext.AssumeRBR { log.Fatalf("--switch-to-rbr and --assume-rbr are mutually exclusive") } + if *testOnReplicaWithManualReplicationControl { + if migrationContext.TestOnReplica { + log.Fatalf("--test-on-replica-manual-replication-control and --test-on-replica are mutually exclusive") + } + migrationContext.TestOnReplica = true + migrationContext.ManualReplicationControl = true + } switch *cutOver { case "atomic", "default", "": migrationContext.CutOverType = base.CutOverAtomic diff --git a/go/logic/applier.go b/go/logic/applier.go index 951addb..1523c4f 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -566,14 +566,41 @@ func (this *Applier) StartSlaveSQLThread() error { return nil } +func (this *Applier) isReplicationStopped() bool { + query := `show slave status` + replicationStopped := false + + err := sqlutils.QueryRowsMap(this.db, query, func(rowMap sqlutils.RowMap) error { + replicationStopped = rowMap["Slave_IO_Running"].String == "No" && rowMap["Slave_SQL_Running"].String == "No" + return nil + }) + + if err != nil { + return false + } + return replicationStopped +} + // StopReplication is used by `--test-on-replica` and stops replication. func (this *Applier) StopReplication() error { - if err := this.StopSlaveIOThread(); err != nil { - return err - } - if err := this.StopSlaveSQLThread(); err != nil { - return err + if this.migrationContext.ManualReplicationControl { + for { + log.Info("Waiting for replication to stop...") + if this.isReplicationStopped() { + log.Info("Replication stopped.") + break + } + time.Sleep(5 * time.Second) + } + } else { + if err := this.StopSlaveIOThread(); err != nil { + return err + } + if err := this.StopSlaveSQLThread(); err != nil { + return err + } } + readBinlogCoordinates, executeBinlogCoordinates, err := mysql.GetReplicationBinlogCoordinates(this.db) if err != nil { return err From 6d80340e4f4aa00d6ada4e0f77136656848f811a Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Fri, 19 Aug 2016 09:06:00 +0200 Subject: [PATCH 02/29] setting time_zone on DML apply --- build.sh | 2 +- go/logic/applier.go | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/build.sh b/build.sh index 150250b..4548f79 100755 --- a/build.sh +++ b/build.sh @@ -2,7 +2,7 @@ # # -RELEASE_VERSION="1.0.10" +RELEASE_VERSION="1.0.11" function build { osname=$1 diff --git a/go/logic/applier.go b/go/logic/applier.go index 10da785..189108a 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -871,6 +871,9 @@ func (this *Applier) ApplyDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) error { if err != nil { return err } + if _, err := tx.Exec("SET SESSION time_zone = '+00:00'"); err != nil { + return err + } if _, err := tx.Exec(query, args...); err != nil { return err } From 2e43718ef38032b9589c32e59c2e9174d9fe44f7 Mon Sep 17 00:00:00 2001 From: Paulo Bittencourt Date: Fri, 19 Aug 2016 17:34:08 -0400 Subject: [PATCH 03/29] Add --test-on-replica-skip-replica-stop flag --- go/base/context.go | 16 ++++++++-------- go/cmd/gh-ost/main.go | 12 ++++++------ go/logic/applier.go | 26 ++------------------------ 3 files changed, 16 insertions(+), 38 deletions(-) diff --git a/go/base/context.go b/go/base/context.go index e2ff510..21e3b59 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -82,14 +82,14 @@ type MigrationContext struct { ServeSocketFile string ServeTCPPort int64 - Noop bool - TestOnReplica bool - MigrateOnReplica bool - ManualReplicationControl bool - OkToDropTable bool - InitiallyDropOldTable bool - InitiallyDropGhostTable bool - CutOverType CutOver + Noop bool + TestOnReplica bool + MigrateOnReplica bool + TestOnReplicaSkipReplicaStop bool + OkToDropTable bool + InitiallyDropOldTable bool + InitiallyDropGhostTable bool + CutOverType CutOver TableEngine string RowsEstimate int64 diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index 3ca7ad5..e152737 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -60,8 +60,8 @@ func main() { flag.BoolVar(&migrationContext.SkipRenamedColumns, "skip-renamed-columns", false, "in case your `ALTER` statement renames columns, gh-ost will note that and offer its interpretation of the rename. By default gh-ost does not proceed to execute. This flag tells gh-ost to skip the renamed columns, i.e. to treat what gh-ost thinks are renamed columns as unrelated columns. NOTE: you may lose column data") executeFlag := flag.Bool("execute", false, "actually execute the alter & migrate the table. Default is noop: do some tests and exit") - testOnReplicaWithManualReplicationControl := flag.Bool("test-on-replica-manual-replication-control", false, "Same as --test-on-replica, but waits for replication to be stopped, instead of stopping it automatically. (Useful in RDS.)") flag.BoolVar(&migrationContext.TestOnReplica, "test-on-replica", false, "Have the migration run on a replica, not on the master. At the end of migration replication is stopped, and tables are swapped and immediately swap-revert. Replication remains stopped and you can compare the two tables for building trust") + flag.BoolVar(&migrationContext.TestOnReplicaSkipReplicaStop, "test-on-replica-skip-replica-stop", false, "When --test-on-replica is enabled, do not issue commands stop replication (requires --test-on-replica)") flag.BoolVar(&migrationContext.MigrateOnReplica, "migrate-on-replica", false, "Have the migration run on a replica, not on the master. This will do the full migration on the replica including cut-over (as opposed to --test-on-replica)") flag.BoolVar(&migrationContext.OkToDropTable, "ok-to-drop-table", false, "Shall the tool drop the old table at end of operation. DROPping tables can be a long locking operation, which is why I'm not doing it by default. I'm an online tool, yes?") @@ -150,13 +150,13 @@ func main() { if migrationContext.SwitchToRowBinlogFormat && migrationContext.AssumeRBR { log.Fatalf("--switch-to-rbr and --assume-rbr are mutually exclusive") } - if *testOnReplicaWithManualReplicationControl { - if migrationContext.TestOnReplica { - log.Fatalf("--test-on-replica-manual-replication-control and --test-on-replica are mutually exclusive") + if migrationContext.TestOnReplicaSkipReplicaStop { + if !migrationContext.TestOnReplica { + log.Fatalf("--test-on-replica-skip-replica-stop requires --test-on-replica to be enabled") } - migrationContext.TestOnReplica = true - migrationContext.ManualReplicationControl = true + log.Warning("--test-on-replica-skip-replica-stop enabled. We will not stop replication before cut-over. Ensure you have a plugin that does this.") } + switch *cutOver { case "atomic", "default", "": migrationContext.CutOverType = base.CutOverAtomic diff --git a/go/logic/applier.go b/go/logic/applier.go index 1523c4f..9eb115c 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -566,32 +566,10 @@ func (this *Applier) StartSlaveSQLThread() error { return nil } -func (this *Applier) isReplicationStopped() bool { - query := `show slave status` - replicationStopped := false - - err := sqlutils.QueryRowsMap(this.db, query, func(rowMap sqlutils.RowMap) error { - replicationStopped = rowMap["Slave_IO_Running"].String == "No" && rowMap["Slave_SQL_Running"].String == "No" - return nil - }) - - if err != nil { - return false - } - return replicationStopped -} - // StopReplication is used by `--test-on-replica` and stops replication. func (this *Applier) StopReplication() error { - if this.migrationContext.ManualReplicationControl { - for { - log.Info("Waiting for replication to stop...") - if this.isReplicationStopped() { - log.Info("Replication stopped.") - break - } - time.Sleep(5 * time.Second) - } + if this.migrationContext.TestOnReplicaSkipReplicaStop { + log.Warningf("--test-on-replica-skip-replica-stop enabled, we are not stopping replication.") } else { if err := this.StopSlaveIOThread(); err != nil { return err From 1376f0af23780a80885df49ea7d66ef1298f8fcb Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 08:49:27 +0200 Subject: [PATCH 04/29] fixed UPDATE dml on renamed column --- go/logic/applier.go | 7 +++---- go/logic/migrator.go | 2 +- go/sql/builder.go | 15 ++++++++------- go/sql/builder_test.go | 33 +++++++++++++++++++++++++-------- 4 files changed, 37 insertions(+), 20 deletions(-) diff --git a/go/logic/applier.go b/go/logic/applier.go index 10da785..d240abb 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -107,7 +107,7 @@ func (this *Applier) ValidateOrDropExistingTables() error { } } if this.tableExists(this.migrationContext.GetGhostTableName()) { - return fmt.Errorf("Table %s already exists. Panicking. Use --initially-drop-ghost-table to force dropping it", sql.EscapeName(this.migrationContext.GetGhostTableName())) + return fmt.Errorf("Table %s already exists. Panicking. Use --initially-drop-ghost-table to force dropping it, though I really prefer that you drop it or rename it away", sql.EscapeName(this.migrationContext.GetGhostTableName())) } if this.migrationContext.InitiallyDropOldTable { if err := this.DropOldTable(); err != nil { @@ -115,7 +115,7 @@ func (this *Applier) ValidateOrDropExistingTables() error { } } if this.tableExists(this.migrationContext.GetOldTableName()) { - return fmt.Errorf("Table %s already exists. Panicking. Use --initially-drop-old-table to force dropping it", sql.EscapeName(this.migrationContext.GetOldTableName())) + return fmt.Errorf("Table %s already exists. Panicking. Use --initially-drop-old-table to force dropping it, though I really prefer that you drop it or rename it away", sql.EscapeName(this.migrationContext.GetOldTableName())) } return nil @@ -837,7 +837,7 @@ func (this *Applier) buildDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) (query } case binlog.UpdateDML: { - query, sharedArgs, uniqueKeyArgs, err := sql.BuildDMLUpdateQuery(dmlEvent.DatabaseName, this.migrationContext.GetGhostTableName(), this.migrationContext.OriginalTableColumns, this.migrationContext.MappedSharedColumns, &this.migrationContext.UniqueKey.Columns, dmlEvent.NewColumnValues.AbstractValues(), dmlEvent.WhereColumnValues.AbstractValues()) + query, sharedArgs, uniqueKeyArgs, err := sql.BuildDMLUpdateQuery(dmlEvent.DatabaseName, this.migrationContext.GetGhostTableName(), this.migrationContext.OriginalTableColumns, this.migrationContext.SharedColumns, this.migrationContext.MappedSharedColumns, &this.migrationContext.UniqueKey.Columns, dmlEvent.NewColumnValues.AbstractValues(), dmlEvent.WhereColumnValues.AbstractValues()) args = append(args, sharedArgs...) args = append(args, uniqueKeyArgs...) return query, args, 0, err @@ -853,7 +853,6 @@ func (this *Applier) ApplyDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) error { if err != nil { return err } - // TODO The below is in preparation for transactional writes on the ghost tables. // Such writes would be, for example: // - prepended with sql_mode setup diff --git a/go/logic/migrator.go b/go/logic/migrator.go index 600381e..998f386 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -354,7 +354,7 @@ func (this *Migrator) validateStatement() (err error) { if this.parser.HasNonTrivialRenames() && !this.migrationContext.SkipRenamedColumns { this.migrationContext.ColumnRenameMap = this.parser.GetNonTrivialRenames() if !this.migrationContext.ApproveRenamedColumns { - return fmt.Errorf("Alter statement has column(s) renamed. gh-ost suspects the following renames: %v; but to proceed you must approve via `--approve-renamed-columns` (or you can skip renamed columns via `--skip-renamed-columns`)", this.parser.GetNonTrivialRenames()) + return fmt.Errorf("gh-ost believes the ALTER statement renames columns, as follows: %v; as precation, you are asked to confirm gh-ost is correct, and provide with `--approve-renamed-columns`, and we're all happy. Or you can skip renamed columns via `--skip-renamed-columns`, in which case column data may be lost", this.parser.GetNonTrivialRenames()) } log.Infof("Alter statement has column(s) renamed. gh-ost finds the following renames: %v; --approve-renamed-columns is given and so migration proceeds.", this.parser.GetNonTrivialRenames()) } diff --git a/go/sql/builder.go b/go/sql/builder.go index 7c670a9..dedd323 100644 --- a/go/sql/builder.go +++ b/go/sql/builder.go @@ -393,7 +393,7 @@ func BuildDMLInsertQuery(databaseName, tableName string, tableColumns, sharedCol return result, sharedArgs, nil } -func BuildDMLUpdateQuery(databaseName, tableName string, tableColumns, sharedColumns, uniqueKeyColumns *ColumnList, valueArgs, whereArgs []interface{}) (result string, sharedArgs, uniqueKeyArgs []interface{}, err error) { +func BuildDMLUpdateQuery(databaseName, tableName string, tableColumns, sharedColumns, mappedSharedColumns, uniqueKeyColumns *ColumnList, valueArgs, whereArgs []interface{}) (result string, sharedArgs, uniqueKeyArgs []interface{}, err error) { if len(valueArgs) != tableColumns.Len() { return result, sharedArgs, uniqueKeyArgs, fmt.Errorf("value args count differs from table column count in BuildDMLUpdateQuery") } @@ -415,9 +415,10 @@ func BuildDMLUpdateQuery(databaseName, tableName string, tableColumns, sharedCol databaseName = EscapeName(databaseName) tableName = EscapeName(tableName) - for _, column := range sharedColumns.Names { + for i, column := range sharedColumns.Names { + mappedColumn := mappedSharedColumns.Names[i] tableOrdinal := tableColumns.Ordinals[column] - arg := fixArgType(valueArgs[tableOrdinal], sharedColumns.IsUnsigned(column)) + arg := fixArgType(valueArgs[tableOrdinal], mappedSharedColumns.IsUnsigned(mappedColumn)) sharedArgs = append(sharedArgs, arg) } @@ -427,11 +428,11 @@ func BuildDMLUpdateQuery(databaseName, tableName string, tableColumns, sharedCol uniqueKeyArgs = append(uniqueKeyArgs, arg) } - sharedColumnNames := duplicateNames(sharedColumns.Names) - for i := range sharedColumnNames { - sharedColumnNames[i] = EscapeName(sharedColumnNames[i]) + mappedSharedColumnNames := duplicateNames(mappedSharedColumns.Names) + for i := range mappedSharedColumnNames { + mappedSharedColumnNames[i] = EscapeName(mappedSharedColumnNames[i]) } - setClause, err := BuildSetPreparedClause(sharedColumnNames) + setClause, err := BuildSetPreparedClause(mappedSharedColumnNames) equalsComparison, err := BuildEqualsPreparedComparison(uniqueKeyColumns.Names) result = fmt.Sprintf(` diff --git a/go/sql/builder_test.go b/go/sql/builder_test.go index db2617e..565954e 100644 --- a/go/sql/builder_test.go +++ b/go/sql/builder_test.go @@ -544,7 +544,7 @@ func TestBuildDMLUpdateQuery(t *testing.T) { { sharedColumns := NewColumnList([]string{"id", "name", "position", "age"}) uniqueKeyColumns := NewColumnList([]string{"position"}) - query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs) + query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs) test.S(t).ExpectNil(err) expected := ` update /* gh-ost mydb.tbl */ @@ -560,7 +560,7 @@ func TestBuildDMLUpdateQuery(t *testing.T) { { sharedColumns := NewColumnList([]string{"id", "name", "position", "age"}) uniqueKeyColumns := NewColumnList([]string{"position", "name"}) - query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs) + query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs) test.S(t).ExpectNil(err) expected := ` update /* gh-ost mydb.tbl */ @@ -576,7 +576,7 @@ func TestBuildDMLUpdateQuery(t *testing.T) { { sharedColumns := NewColumnList([]string{"id", "name", "position", "age"}) uniqueKeyColumns := NewColumnList([]string{"age"}) - query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs) + query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs) test.S(t).ExpectNil(err) expected := ` update /* gh-ost mydb.tbl */ @@ -592,7 +592,7 @@ func TestBuildDMLUpdateQuery(t *testing.T) { { sharedColumns := NewColumnList([]string{"id", "name", "position", "age"}) uniqueKeyColumns := NewColumnList([]string{"age", "position", "id", "name"}) - query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs) + query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs) test.S(t).ExpectNil(err) expected := ` update /* gh-ost mydb.tbl */ @@ -608,15 +608,32 @@ func TestBuildDMLUpdateQuery(t *testing.T) { { sharedColumns := NewColumnList([]string{"id", "name", "position", "age"}) uniqueKeyColumns := NewColumnList([]string{"age", "surprise"}) - _, _, _, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs) + _, _, _, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs) test.S(t).ExpectNotNil(err) } { sharedColumns := NewColumnList([]string{"id", "name", "position", "age"}) uniqueKeyColumns := NewColumnList([]string{}) - _, _, _, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs) + _, _, _, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs) test.S(t).ExpectNotNil(err) } + { + sharedColumns := NewColumnList([]string{"id", "name", "position", "age"}) + mappedColumns := NewColumnList([]string{"id", "name", "role", "age"}) + uniqueKeyColumns := NewColumnList([]string{"id"}) + query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, mappedColumns, uniqueKeyColumns, valueArgs, whereArgs) + test.S(t).ExpectNil(err) + expected := ` + update /* gh-ost mydb.tbl */ + mydb.tbl + set id=?, name=?, role=?, age=? + where + ((id = ?)) + ` + test.S(t).ExpectEquals(normalizeQuery(query), normalizeQuery(expected)) + test.S(t).ExpectTrue(reflect.DeepEqual(sharedArgs, []interface{}{3, "testname", 17, 23})) + test.S(t).ExpectTrue(reflect.DeepEqual(uniqueKeyArgs, []interface{}{3})) + } } func TestBuildDMLUpdateQuerySignedUnsigned(t *testing.T) { @@ -629,7 +646,7 @@ func TestBuildDMLUpdateQuerySignedUnsigned(t *testing.T) { uniqueKeyColumns := NewColumnList([]string{"position"}) { // test signed - query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs) + query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs) test.S(t).ExpectNil(err) expected := ` update /* gh-ost mydb.tbl */ @@ -646,7 +663,7 @@ func TestBuildDMLUpdateQuerySignedUnsigned(t *testing.T) { // test unsigned sharedColumns.SetUnsigned("age") uniqueKeyColumns.SetUnsigned("position") - query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs) + query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs) test.S(t).ExpectNil(err) expected := ` update /* gh-ost mydb.tbl */ From 0bb8d70fcebc0970f815cce92f85e12bdb20dc60 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 09:20:17 +0200 Subject: [PATCH 05/29] initial preparation for local tests --- localtests/test.sh | 14 ++++++++++++++ localtests/unsigned/create.sql | 24 ++++++++++++++++++++++++ localtests/unsigned/test | 20 ++++++++++++++++++++ 3 files changed, 58 insertions(+) create mode 100755 localtests/test.sh create mode 100644 localtests/unsigned/create.sql create mode 100644 localtests/unsigned/test diff --git a/localtests/test.sh b/localtests/test.sh new file mode 100755 index 0000000..6c03507 --- /dev/null +++ b/localtests/test.sh @@ -0,0 +1,14 @@ +#!/bin/bash + +set -e + +tests_path=$(dirname $0) +cd $tests_path + +gh-ost-test-mysql-master -e "select 1" +gh-ost-test-mysql-replica -e "select 1" + + +find . ! -path . -type d | cut -d "/" -f 2 | while read test_name ; do + echo "Testing: $test_name" +done diff --git a/localtests/unsigned/create.sql b/localtests/unsigned/create.sql new file mode 100644 index 0000000..d98bb07 --- /dev/null +++ b/localtests/unsigned/create.sql @@ -0,0 +1,24 @@ +drop table if exists gh_ost_test; +create table gh_ost_test ( + id int auto_increment, + i int not null, + bi bigint not null, + iu int unsigned not null, + biu bigint unsigned not null, + primary key(id) +) auto_increment=1; + +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 + insert into gh_ost_test values (null, -2147483647, -9223372036854775807, 4294967295, 18446744073709551615); + set @last_insert_id := cast(last_insert_id() as signed); + update gh_ost_test set i=-2147483647+@last_insert_id, bi=-9223372036854775807+@last_insert_id, iu=4294967295-@last_insert_id, biu=18446744073709551615-@last_insert_id where id < @last_insert_id order by id desc limit 1; +end ;; diff --git a/localtests/unsigned/test b/localtests/unsigned/test new file mode 100644 index 0000000..b76c03f --- /dev/null +++ b/localtests/unsigned/test @@ -0,0 +1,20 @@ +go run go/cmd/gh-ost/main.go \ + --conf=/Users/shlomi-noach/tmp/gh-osc.cnf \ + --port=22297 \ + --database=test \ + --table=gh_ost_test \ + --alter="engine=innodb" \ + --exact-rowcount \ + --switch-to-rbr \ + --initially-drop-old-table \ + --initially-drop-ghost-table \ + --throttle-query="select timestampdiff(second, min(last_update), now()) < 5 from _gh_ost_test_ghc" \ + --serve-socket-file=/tmp/gh-ost.test.sock \ + --initially-drop-socket-file \ + --postpone-cut-over-flag-file=/tmp/gh-ost.postpone.flag \ + --execute \ + --verbose \ + --debug \ + --stack \ + + ~/sandboxes/rsandbox_mysql-5_6_28/m test -e "select * from $t" | md5sum ; ~/sandboxes/rsandbox_mysql-5_6_28/m test -e "select * from _${t}_del" | md5sum ; ~/sandboxes/rsandbox_mysql-5_6_28/m test -e "select * from $t limit 5" From a50f7637a480b44709b9363bbe40dbdfc98ac8bf Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 11:13:18 +0200 Subject: [PATCH 06/29] working testing suite --- localtests/test.sh | 86 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 81 insertions(+), 5 deletions(-) diff --git a/localtests/test.sh b/localtests/test.sh index 6c03507..524cf9e 100755 --- a/localtests/test.sh +++ b/localtests/test.sh @@ -3,12 +3,88 @@ set -e tests_path=$(dirname $0) -cd $tests_path +test_logfile=/tmp/gh-ost-test.log -gh-ost-test-mysql-master -e "select 1" -gh-ost-test-mysql-replica -e "select 1" +master_host= +master_port= +replica_host= +replica_port= +verify_master_and_replica() { + if [ "$(gh-ost-test-mysql-master -e "select 1" -ss)" != "1" ] ; then + echo "Cannot verify gh-ost-test-mysql-master" + exit 1 + fi + read master_host master_port <<< $(gh-ost-test-mysql-master -e "select @@hostname, @@port" -ss) + if [ "$(gh-ost-test-mysql-replica -e "select 1" -ss)" != "1" ] ; then + echo "Cannot verify gh-ost-test-mysql-replica" + exit 1 + fi + read replica_host replica_port <<< $(gh-ost-test-mysql-replica -e "select @@hostname, @@port" -ss) +} + +test_single() { + local test_name + test_name="$1" -find . ! -path . -type d | cut -d "/" -f 2 | while read test_name ; do echo "Testing: $test_name" -done + + gh-ost-test-mysql-replica -e "start slave" + gh-ost-test-mysql-master test < $tests_path/$test_name/create.sql + + if [ -f $tests_path/$test_name/extra_args ] ; then + extra_args=$(cat $tests_path/$test_name/extra_args) + fi + columns="*" + if [ -f $tests_path/$test_name/test_columns ] ; then + columns=$(cat $tests_path/$test_name/test_columns) + fi + + go run go/cmd/gh-ost/main.go \ + --user=gh-ost \ + --password=gh-ost \ + --host=$replica_host \ + --port=$replica_port \ + --database=test \ + --table=gh_ost_test \ + --alter="engine=innodb" \ + --exact-rowcount \ + --switch-to-rbr \ + --initially-drop-old-table \ + --initially-drop-ghost-table \ + --throttle-query="select timestampdiff(second, min(last_update), now()) < 5 from _gh_ost_test_ghc" \ + --serve-socket-file=/tmp/gh-ost.test.sock \ + --initially-drop-socket-file \ + --postpone-cut-over-flag-file=/tmp/gh-ost.postpone.flag \ + --test-on-replica \ + --verbose \ + --debug \ + --stack \ + --execute $extra_args \ + 1> $test_logfile 2>&1 + + if [ $? -ne 0 ] ; then + echo "ERROR $test_name execution failure. See $test_logfile" + return 1 + fi + + orig_checksum=$(gh-ost-test-mysql-replica test -e "select ${columns} from gh_ost_test" -ss | md5sum) + ghost_checksum=$(gh-ost-test-mysql-replica test -e "select ${columns} from _gh_ost_test_gho" -ss | md5sum) + + gh-ost-test-mysql-replica -e "start slave" + + if [ "$orig_checksum" != "$ghost_checksum" ] ; then + echo "ERROR $test_name: checksum mismatch" + return 1 + fi + echo "pass" +} + +test_all() { + find $tests_path ! -path . -type d -mindepth 1 -maxdepth 1 | cut -d "/" -f 3 | while read test_name ; do + test_single "$test_name" + done +} + +verify_master_and_replica +test_all From 744f009b3b140141216cb3f89d9f5a76e984a58f Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 11:14:06 +0200 Subject: [PATCH 07/29] remove redundant file --- localtests/unsigned/test | 20 -------------------- 1 file changed, 20 deletions(-) delete mode 100644 localtests/unsigned/test diff --git a/localtests/unsigned/test b/localtests/unsigned/test deleted file mode 100644 index b76c03f..0000000 --- a/localtests/unsigned/test +++ /dev/null @@ -1,20 +0,0 @@ -go run go/cmd/gh-ost/main.go \ - --conf=/Users/shlomi-noach/tmp/gh-osc.cnf \ - --port=22297 \ - --database=test \ - --table=gh_ost_test \ - --alter="engine=innodb" \ - --exact-rowcount \ - --switch-to-rbr \ - --initially-drop-old-table \ - --initially-drop-ghost-table \ - --throttle-query="select timestampdiff(second, min(last_update), now()) < 5 from _gh_ost_test_ghc" \ - --serve-socket-file=/tmp/gh-ost.test.sock \ - --initially-drop-socket-file \ - --postpone-cut-over-flag-file=/tmp/gh-ost.postpone.flag \ - --execute \ - --verbose \ - --debug \ - --stack \ - - ~/sandboxes/rsandbox_mysql-5_6_28/m test -e "select * from $t" | md5sum ; ~/sandboxes/rsandbox_mysql-5_6_28/m test -e "select * from _${t}_del" | md5sum ; ~/sandboxes/rsandbox_mysql-5_6_28/m test -e "select * from $t limit 5" From 2831a6dc869e2b14e29ab0fa067295c64d0b2cef Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 11:50:50 +0200 Subject: [PATCH 08/29] adding rename test --- localtests/rename/create.sql | 22 ++++++++++++++++++++++ localtests/rename/extra_args | 1 + 2 files changed, 23 insertions(+) create mode 100644 localtests/rename/create.sql create mode 100644 localtests/rename/extra_args diff --git a/localtests/rename/create.sql b/localtests/rename/create.sql new file mode 100644 index 0000000..f1e458f --- /dev/null +++ b/localtests/rename/create.sql @@ -0,0 +1,22 @@ +drop table if exists gh_ost_test; +create table gh_ost_test ( + id int auto_increment, + c1 int not null, + c2 int not null, + primary key (id) +) auto_increment=1; + +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 + insert into gh_ost_test values (null, 17, 23); + set @last_insert_id := last_insert_id(); + update gh_ost_test set c1=c1+@last_insert_id, c2=c2+@last_insert_id where id < @last_insert_id order by id desc limit 1; +end ;; diff --git a/localtests/rename/extra_args b/localtests/rename/extra_args new file mode 100644 index 0000000..55af79a --- /dev/null +++ b/localtests/rename/extra_args @@ -0,0 +1 @@ +--approve-renamed-columns --alter="change column c3 c4 int not null" From 745fce249c4fb4b87f110effa37b82f98642f299 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 11:51:03 +0200 Subject: [PATCH 09/29] working on extra_args, incomplete --- localtests/test.sh | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/localtests/test.sh b/localtests/test.sh index 524cf9e..d670949 100755 --- a/localtests/test.sh +++ b/localtests/test.sh @@ -1,6 +1,6 @@ #!/bin/bash -set -e +# set -e tests_path=$(dirname $0) test_logfile=/tmp/gh-ost-test.log @@ -23,6 +23,12 @@ verify_master_and_replica() { read replica_host replica_port <<< $(gh-ost-test-mysql-replica -e "select @@hostname, @@port" -ss) } +exec_cmd() { + echo "$@" + command "$@" 1> $test_logfile 2>&1 + return $? +} + test_single() { local test_name test_name="$1" @@ -32,8 +38,12 @@ test_single() { gh-ost-test-mysql-replica -e "start slave" gh-ost-test-mysql-master test < $tests_path/$test_name/create.sql + extra_args="" if [ -f $tests_path/$test_name/extra_args ] ; then - extra_args=$(cat $tests_path/$test_name/extra_args) + # mapfile -t <$tests_path/$test_name/extra_args + # echo "${MAPFILE[@]}" + extra_args=($(cat $tests_path/$test_name/extra_args)) + echo ${extra_args[@]} fi columns="*" if [ -f $tests_path/$test_name/test_columns ] ; then @@ -60,8 +70,7 @@ test_single() { --verbose \ --debug \ --stack \ - --execute $extra_args \ - 1> $test_logfile 2>&1 + --execute "${extra_args[@]}" if [ $? -ne 0 ] ; then echo "ERROR $test_name execution failure. See $test_logfile" @@ -77,12 +86,17 @@ test_single() { echo "ERROR $test_name: checksum mismatch" return 1 fi - echo "pass" } test_all() { find $tests_path ! -path . -type d -mindepth 1 -maxdepth 1 | cut -d "/" -f 3 | while read test_name ; do test_single "$test_name" + if [ $? -ne 0 ] ; then + echo "+ FAIL" + return 1 + else + echo "+ pass" + fi done } From cf5fdb971b988851e6492b9a2b2c4a8f666a7e66 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 11:55:33 +0200 Subject: [PATCH 10/29] updated version --- build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.sh b/build.sh index 4548f79..bae5040 100755 --- a/build.sh +++ b/build.sh @@ -2,7 +2,7 @@ # # -RELEASE_VERSION="1.0.11" +RELEASE_VERSION="1.0.12" function build { osname=$1 From 4c78520f3d8b1c6753ddc05ff8321be7931da528 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 12:18:02 +0200 Subject: [PATCH 11/29] fixed argument extrapolation --- localtests/rename/extra_args | 2 +- localtests/test.sh | 38 ++++++++++++++++++------------------ 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/localtests/rename/extra_args b/localtests/rename/extra_args index 55af79a..c5f65da 100644 --- a/localtests/rename/extra_args +++ b/localtests/rename/extra_args @@ -1 +1 @@ ---approve-renamed-columns --alter="change column c3 c4 int not null" +--alter="change column c3 c4 int not null" --approve-renamed-columns diff --git a/localtests/test.sh b/localtests/test.sh index d670949..9144ccb 100755 --- a/localtests/test.sh +++ b/localtests/test.sh @@ -4,6 +4,7 @@ tests_path=$(dirname $0) test_logfile=/tmp/gh-ost-test.log +exec_command_file=/tmp/gh-ost-test.bash master_host= master_port= @@ -40,29 +41,26 @@ test_single() { extra_args="" if [ -f $tests_path/$test_name/extra_args ] ; then - # mapfile -t <$tests_path/$test_name/extra_args - # echo "${MAPFILE[@]}" - extra_args=($(cat $tests_path/$test_name/extra_args)) - echo ${extra_args[@]} + extra_args=$(cat $tests_path/$test_name/extra_args) fi columns="*" if [ -f $tests_path/$test_name/test_columns ] ; then columns=$(cat $tests_path/$test_name/test_columns) fi - go run go/cmd/gh-ost/main.go \ + cmd="go run go/cmd/gh-ost/main.go \ --user=gh-ost \ --password=gh-ost \ --host=$replica_host \ --port=$replica_port \ --database=test \ --table=gh_ost_test \ - --alter="engine=innodb" \ + --alter='engine=innodb' \ --exact-rowcount \ --switch-to-rbr \ --initially-drop-old-table \ --initially-drop-ghost-table \ - --throttle-query="select timestampdiff(second, min(last_update), now()) < 5 from _gh_ost_test_ghc" \ + --throttle-query='select timestampdiff(second, min(last_update), now()) < 5 from _gh_ost_test_ghc' \ --serve-socket-file=/tmp/gh-ost.test.sock \ --initially-drop-socket-file \ --postpone-cut-over-flag-file=/tmp/gh-ost.postpone.flag \ @@ -70,22 +68,24 @@ test_single() { --verbose \ --debug \ --stack \ - --execute "${extra_args[@]}" + --execute ${extra_args[@]}" + echo $cmd > $exec_command_file + bash $exec_command_file - if [ $? -ne 0 ] ; then - echo "ERROR $test_name execution failure. See $test_logfile" - return 1 - fi + if [ $? -ne 0 ] ; then + echo "ERROR $test_name execution failure. See $test_logfile" + return 1 + fi - orig_checksum=$(gh-ost-test-mysql-replica test -e "select ${columns} from gh_ost_test" -ss | md5sum) - ghost_checksum=$(gh-ost-test-mysql-replica test -e "select ${columns} from _gh_ost_test_gho" -ss | md5sum) + orig_checksum=$(gh-ost-test-mysql-replica test -e "select ${columns} from gh_ost_test" -ss | md5sum) + ghost_checksum=$(gh-ost-test-mysql-replica test -e "select ${columns} from _gh_ost_test_gho" -ss | md5sum) - gh-ost-test-mysql-replica -e "start slave" + gh-ost-test-mysql-replica -e "start slave" - if [ "$orig_checksum" != "$ghost_checksum" ] ; then - echo "ERROR $test_name: checksum mismatch" - return 1 - fi + if [ "$orig_checksum" != "$ghost_checksum" ] ; then + echo "ERROR $test_name: checksum mismatch" + return 1 + fi } test_all() { From 6a20808389c2582407cfba6288e722bbf018febe Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 15:42:06 +0200 Subject: [PATCH 12/29] adding timezone tests --- localtests/atz/create.sql | 41 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 localtests/atz/create.sql diff --git a/localtests/atz/create.sql b/localtests/atz/create.sql new file mode 100644 index 0000000..f908e6e --- /dev/null +++ b/localtests/atz/create.sql @@ -0,0 +1,41 @@ +drop table if exists gh_ost_test; +create table gh_ost_test ( + id int auto_increment, + i int not null, + ts0 timestamp default current_timestamp, + ts1 timestamp, + ts2 timestamp, + updated tinyint unsigned default 0, + primary key(id), + key i_idx(i) +) auto_increment=1; + +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 + insert into gh_ost_test values (null, 11, null, now(), now(), 0); + update gh_ost_test set ts2=now() + interval 10 minute, updated = 1 where i = 11 order by id desc limit 1; + + set session time_zone='system'; + insert into gh_ost_test values (null, 13, null, now(), now(), 0); + update gh_ost_test set ts2=now() + interval 10 minute, updated = 1 where i = 13 order by id desc limit 1; + + set session time_zone='+00:00'; + insert into gh_ost_test values (null, 17, null, now(), now(), 0); + update gh_ost_test set ts2=now() + interval 10 minute, updated = 1 where i = 17 order by id desc limit 1; + + set session time_zone='-03:00'; + insert into gh_ost_test values (null, 19, null, now(), now(), 0); + update gh_ost_test set ts2=now() + interval 10 minute, updated = 1 where i = 19 order by id desc limit 1; + + set session time_zone='+05:00'; + insert into gh_ost_test values (null, 23, null, now(), now(), 0); + update gh_ost_test set ts2=now() + interval 10 minute, updated = 1 where i = 23 order by id desc limit 1; +end ;; From 2faa27a2c402506518113cc4f2afb286825c387e Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 15:43:00 +0200 Subject: [PATCH 13/29] elaborate output on failure --- localtests/rename/create.sql | 2 +- localtests/test.sh | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/localtests/rename/create.sql b/localtests/rename/create.sql index f1e458f..ed062dc 100644 --- a/localtests/rename/create.sql +++ b/localtests/rename/create.sql @@ -18,5 +18,5 @@ create event gh_ost_test begin insert into gh_ost_test values (null, 17, 23); set @last_insert_id := last_insert_id(); - update gh_ost_test set c1=c1+@last_insert_id, c2=c2+@last_insert_id where id < @last_insert_id order by id desc limit 1; + update gh_ost_test set c1=c1+@last_insert_id, c2=c2+@last_insert_id where id = @last_insert_id order by id desc limit 1; end ;; diff --git a/localtests/test.sh b/localtests/test.sh index 9144ccb..83e8537 100755 --- a/localtests/test.sh +++ b/localtests/test.sh @@ -65,12 +65,13 @@ test_single() { --initially-drop-socket-file \ --postpone-cut-over-flag-file=/tmp/gh-ost.postpone.flag \ --test-on-replica \ + --default-retries=1 \ --verbose \ --debug \ --stack \ --execute ${extra_args[@]}" echo $cmd > $exec_command_file - bash $exec_command_file + bash $exec_command_file 1> $test_logfile 2>&1 if [ $? -ne 0 ] ; then echo "ERROR $test_name execution failure. See $test_logfile" @@ -80,10 +81,12 @@ test_single() { orig_checksum=$(gh-ost-test-mysql-replica test -e "select ${columns} from gh_ost_test" -ss | md5sum) ghost_checksum=$(gh-ost-test-mysql-replica test -e "select ${columns} from _gh_ost_test_gho" -ss | md5sum) - gh-ost-test-mysql-replica -e "start slave" - if [ "$orig_checksum" != "$ghost_checksum" ] ; then echo "ERROR $test_name: checksum mismatch" + echo "---" + gh-ost-test-mysql-replica test -e "select ${columns} from gh_ost_test" -ss + echo "---" + gh-ost-test-mysql-replica test -e "select ${columns} from _gh_ost_test_gho" -ss return 1 fi } @@ -97,6 +100,7 @@ test_all() { else echo "+ pass" fi + gh-ost-test-mysql-replica -e "start slave" done } From 950fde2ad6bdd82216115c60e0ca0aec68e8d1eb Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 15:45:38 +0200 Subject: [PATCH 14/29] row_event converts timestamps to UTC --- vendor/github.com/siddontang/go-mysql/replication/row_event.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vendor/github.com/siddontang/go-mysql/replication/row_event.go b/vendor/github.com/siddontang/go-mysql/replication/row_event.go index 3fe8598..0e4eb17 100644 --- a/vendor/github.com/siddontang/go-mysql/replication/row_event.go +++ b/vendor/github.com/siddontang/go-mysql/replication/row_event.go @@ -609,7 +609,7 @@ func decodeTimestamp2(data []byte, dec uint16) (string, int, error) { return "0000-00-00 00:00:00", n, nil } - t := time.Unix(sec, usec*1000) + t := time.Unix(sec, usec*1000).UTC() return t.Format(TimeFormat), n, nil } From 61237f9e93b506500066543152334d8b08509b65 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 15:49:48 +0200 Subject: [PATCH 15/29] rename --- localtests/{atz => tz}/create.sql | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename localtests/{atz => tz}/create.sql (100%) diff --git a/localtests/atz/create.sql b/localtests/tz/create.sql similarity index 100% rename from localtests/atz/create.sql rename to localtests/tz/create.sql From 7a2e3146a2d04d64a8a7404cc4b72dd15d486bfe Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 15:53:44 +0200 Subject: [PATCH 16/29] fixed alter statement in rename test --- localtests/rename/extra_args | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/localtests/rename/extra_args b/localtests/rename/extra_args index c5f65da..d36d5ee 100644 --- a/localtests/rename/extra_args +++ b/localtests/rename/extra_args @@ -1 +1 @@ ---alter="change column c3 c4 int not null" --approve-renamed-columns +--alter="change column c2 c3 int not null" --approve-renamed-columns From b63cc3e75e448071122fbe67e0f3f15f95f11997 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 16:00:15 +0200 Subject: [PATCH 17/29] fix INSERT DML handling on renamed column --- go/logic/applier.go | 2 +- go/sql/builder.go | 16 ++++++++-------- go/sql/builder_test.go | 14 +++++++------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/go/logic/applier.go b/go/logic/applier.go index 0de59bb..a122b71 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -832,7 +832,7 @@ func (this *Applier) buildDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) (query } case binlog.InsertDML: { - query, sharedArgs, err := sql.BuildDMLInsertQuery(dmlEvent.DatabaseName, this.migrationContext.GetGhostTableName(), this.migrationContext.OriginalTableColumns, this.migrationContext.MappedSharedColumns, dmlEvent.NewColumnValues.AbstractValues()) + query, sharedArgs, err := sql.BuildDMLInsertQuery(dmlEvent.DatabaseName, this.migrationContext.GetGhostTableName(), this.migrationContext.OriginalTableColumns, this.migrationContext.SharedColumns, this.migrationContext.MappedSharedColumns, dmlEvent.NewColumnValues.AbstractValues()) return query, sharedArgs, 1, err } case binlog.UpdateDML: diff --git a/go/sql/builder.go b/go/sql/builder.go index dedd323..79cea47 100644 --- a/go/sql/builder.go +++ b/go/sql/builder.go @@ -354,7 +354,7 @@ func BuildDMLDeleteQuery(databaseName, tableName string, tableColumns, uniqueKey return result, uniqueKeyArgs, nil } -func BuildDMLInsertQuery(databaseName, tableName string, tableColumns, sharedColumns *ColumnList, args []interface{}) (result string, sharedArgs []interface{}, err error) { +func BuildDMLInsertQuery(databaseName, tableName string, tableColumns, sharedColumns, mappedSharedColumns *ColumnList, args []interface{}) (result string, sharedArgs []interface{}, err error) { if len(args) != tableColumns.Len() { return result, args, fmt.Errorf("args count differs from table column count in BuildDMLInsertQuery") } @@ -367,17 +367,17 @@ func BuildDMLInsertQuery(databaseName, tableName string, tableColumns, sharedCol databaseName = EscapeName(databaseName) tableName = EscapeName(tableName) - for _, column := range sharedColumns.Names { + for _, column := range mappedSharedColumns.Names { tableOrdinal := tableColumns.Ordinals[column] - arg := fixArgType(args[tableOrdinal], sharedColumns.IsUnsigned(column)) + arg := fixArgType(args[tableOrdinal], mappedSharedColumns.IsUnsigned(column)) sharedArgs = append(sharedArgs, arg) } - sharedColumnNames := duplicateNames(sharedColumns.Names) - for i := range sharedColumnNames { - sharedColumnNames[i] = EscapeName(sharedColumnNames[i]) + mappedSharedColumnNames := duplicateNames(mappedSharedColumns.Names) + for i := range mappedSharedColumnNames { + mappedSharedColumnNames[i] = EscapeName(mappedSharedColumnNames[i]) } - preparedValues := buildPreparedValues(sharedColumns.Len()) + preparedValues := buildPreparedValues(mappedSharedColumns.Len()) result = fmt.Sprintf(` replace /* gh-ost %s.%s */ into @@ -387,7 +387,7 @@ func BuildDMLInsertQuery(databaseName, tableName string, tableColumns, sharedCol (%s) `, databaseName, tableName, databaseName, tableName, - strings.Join(sharedColumnNames, ", "), + strings.Join(mappedSharedColumnNames, ", "), strings.Join(preparedValues, ", "), ) return result, sharedArgs, nil diff --git a/go/sql/builder_test.go b/go/sql/builder_test.go index 565954e..55f27f9 100644 --- a/go/sql/builder_test.go +++ b/go/sql/builder_test.go @@ -442,7 +442,7 @@ func TestBuildDMLInsertQuery(t *testing.T) { args := []interface{}{3, "testname", "first", 17, 23} { sharedColumns := NewColumnList([]string{"id", "name", "position", "age"}) - query, sharedArgs, err := BuildDMLInsertQuery(databaseName, tableName, tableColumns, sharedColumns, args) + query, sharedArgs, err := BuildDMLInsertQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, args) test.S(t).ExpectNil(err) expected := ` replace /* gh-ost mydb.tbl */ @@ -456,7 +456,7 @@ func TestBuildDMLInsertQuery(t *testing.T) { } { sharedColumns := NewColumnList([]string{"position", "name", "age", "id"}) - query, sharedArgs, err := BuildDMLInsertQuery(databaseName, tableName, tableColumns, sharedColumns, args) + query, sharedArgs, err := BuildDMLInsertQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, args) test.S(t).ExpectNil(err) expected := ` replace /* gh-ost mydb.tbl */ @@ -470,12 +470,12 @@ func TestBuildDMLInsertQuery(t *testing.T) { } { sharedColumns := NewColumnList([]string{"position", "name", "surprise", "id"}) - _, _, err := BuildDMLInsertQuery(databaseName, tableName, tableColumns, sharedColumns, args) + _, _, err := BuildDMLInsertQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, args) test.S(t).ExpectNotNil(err) } { sharedColumns := NewColumnList([]string{}) - _, _, err := BuildDMLInsertQuery(databaseName, tableName, tableColumns, sharedColumns, args) + _, _, err := BuildDMLInsertQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, args) test.S(t).ExpectNotNil(err) } } @@ -489,7 +489,7 @@ func TestBuildDMLInsertQuerySignedUnsigned(t *testing.T) { // testing signed args := []interface{}{3, "testname", "first", int8(-1), 23} sharedColumns := NewColumnList([]string{"id", "name", "position", "age"}) - query, sharedArgs, err := BuildDMLInsertQuery(databaseName, tableName, tableColumns, sharedColumns, args) + query, sharedArgs, err := BuildDMLInsertQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, args) test.S(t).ExpectNil(err) expected := ` replace /* gh-ost mydb.tbl */ @@ -505,7 +505,7 @@ func TestBuildDMLInsertQuerySignedUnsigned(t *testing.T) { // testing unsigned args := []interface{}{3, "testname", "first", int8(-1), 23} sharedColumns.SetUnsigned("position") - query, sharedArgs, err := BuildDMLInsertQuery(databaseName, tableName, tableColumns, sharedColumns, args) + query, sharedArgs, err := BuildDMLInsertQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, args) test.S(t).ExpectNil(err) expected := ` replace /* gh-ost mydb.tbl */ @@ -521,7 +521,7 @@ func TestBuildDMLInsertQuerySignedUnsigned(t *testing.T) { // testing unsigned args := []interface{}{3, "testname", "first", int32(-1), 23} sharedColumns.SetUnsigned("position") - query, sharedArgs, err := BuildDMLInsertQuery(databaseName, tableName, tableColumns, sharedColumns, args) + query, sharedArgs, err := BuildDMLInsertQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, args) test.S(t).ExpectNil(err) expected := ` replace /* gh-ost mydb.tbl */ From 9dc378feaa62c66851c29aa3b694c0f3ade36bda Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 16:20:11 +0200 Subject: [PATCH 18/29] updated version --- build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.sh b/build.sh index bae5040..36c4215 100755 --- a/build.sh +++ b/build.sh @@ -2,7 +2,7 @@ # # -RELEASE_VERSION="1.0.12" +RELEASE_VERSION="1.0.13" function build { osname=$1 From 1bd93bda70fc8330aa2f3d11f04313d4ad649781 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 16:28:40 +0200 Subject: [PATCH 19/29] localtest: rename: testing for DELETE --- localtests/rename/create.sql | 1 + 1 file changed, 1 insertion(+) diff --git a/localtests/rename/create.sql b/localtests/rename/create.sql index ed062dc..10c211a 100644 --- a/localtests/rename/create.sql +++ b/localtests/rename/create.sql @@ -19,4 +19,5 @@ begin insert into gh_ost_test values (null, 17, 23); set @last_insert_id := last_insert_id(); update gh_ost_test set c1=c1+@last_insert_id, c2=c2+@last_insert_id where id = @last_insert_id order by id desc limit 1; + delete from gh_ost_test where id <= 2 order by id asc limit 1; end ;; From f33bebb52795d91dd31b433edfd7006a9c6e18d1 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 16:33:03 +0200 Subject: [PATCH 20/29] improved rename:DELETE test --- localtests/rename/create.sql | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/localtests/rename/create.sql b/localtests/rename/create.sql index 10c211a..093e8c8 100644 --- a/localtests/rename/create.sql +++ b/localtests/rename/create.sql @@ -16,8 +16,10 @@ create event gh_ost_test enable do begin + insert ignore into gh_ost_test values (1, 11, 23); + insert ignore into gh_ost_test values (2, 13, 23); insert into gh_ost_test values (null, 17, 23); set @last_insert_id := last_insert_id(); update gh_ost_test set c1=c1+@last_insert_id, c2=c2+@last_insert_id where id = @last_insert_id order by id desc limit 1; - delete from gh_ost_test where id <= 2 order by id asc limit 1; + delete from gh_ost_test where id in (1,2); end ;; From f947c46e3c008c190a8d170bfd5066954bd2a682 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 16:34:00 +0200 Subject: [PATCH 21/29] improved rename:DELETE test --- localtests/rename/create.sql | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/localtests/rename/create.sql b/localtests/rename/create.sql index 093e8c8..3b53f71 100644 --- a/localtests/rename/create.sql +++ b/localtests/rename/create.sql @@ -21,5 +21,6 @@ begin insert into gh_ost_test values (null, 17, 23); set @last_insert_id := last_insert_id(); update gh_ost_test set c1=c1+@last_insert_id, c2=c2+@last_insert_id where id = @last_insert_id order by id desc limit 1; - delete from gh_ost_test where id in (1,2); + delete from gh_ost_test where id=1; + delete from gh_ost_test where c1=11; -- id=2 end ;; From b380578f5368f62fa0d0dc0c84e31dae56de6b54 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 22 Aug 2016 16:35:21 +0200 Subject: [PATCH 22/29] improved rename:DELETE test --- localtests/rename/create.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/localtests/rename/create.sql b/localtests/rename/create.sql index 3b53f71..d28a7c1 100644 --- a/localtests/rename/create.sql +++ b/localtests/rename/create.sql @@ -20,7 +20,7 @@ begin insert ignore into gh_ost_test values (2, 13, 23); insert into gh_ost_test values (null, 17, 23); set @last_insert_id := last_insert_id(); - update gh_ost_test set c1=c1+@last_insert_id, c2=c2+@last_insert_id where id = @last_insert_id order by id desc limit 1; + update gh_ost_test set c1=c1+@last_insert_id, c2=c2+@last_insert_id where id=@last_insert_id order by id desc limit 1; delete from gh_ost_test where id=1; - delete from gh_ost_test where c1=11; -- id=2 + delete from gh_ost_test where c1=13; -- id=2 end ;; From bf7d33efe5bad9ac0d4976946822cdb7faa170d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Eeden?= Date: Tue, 23 Aug 2016 09:03:33 +0200 Subject: [PATCH 23/29] Improve documentation for throttle-control-replicas interactive command --- doc/interactive-commands.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/interactive-commands.md b/doc/interactive-commands.md index 5449627..fa971b0 100644 --- a/doc/interactive-commands.md +++ b/doc/interactive-commands.md @@ -24,7 +24,7 @@ Both interfaces may serve at the same time. Both respond to simple text command, - `critical-load=`: change critical load setting (exceeding given thresholds causes panic and abort) - `nice-ratio=`: change _nice_ ratio: 0 for aggressive (not nice, not sleeping), positive integer `n`: for any `1ms` spent copying rows, spend `n*1ms` units of time sleeping. Examples: assume a single rows chunk copy takes `100ms` to complete. `nice-ratio=0.5` will cause `gh-ost` to sleep for `50ms` immediately following. `nice-ratio=1` will cause `gh-ost` to sleep for `100ms`, effectively doubling runtime; value of `2` will effectively triple the runtime; etc. - `throttle-query`: change throttle query -- `throttle-control-replicas`: change list of throttle-control replicas, these are replicas `gh-ost` will check +- `throttle-control-replicas='replica1,replica2'`: change list of throttle-control replicas, these are replicas `gh-ost` will check. This takes a comma separated list of replica's to check and replaces the previous list. - `throttle`: force migration suspend - `no-throttle`: cancel forced suspension (though other throttling reasons may still apply) - `unpostpone`: at a time where `gh-ost` is postponing the [cut-over](cut-over.md) phase, instruct `gh-ost` to stop postponing and proceed immediately to cut-over. From d8cfd49e2c645fa9d671bd20210e515983cef287 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Eeden?= Date: Tue, 23 Aug 2016 09:41:07 +0200 Subject: [PATCH 24/29] Message about waiting should be INFO not DEBUG --- go/logic/migrator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/logic/migrator.go b/go/logic/migrator.go index 600381e..cf9706e 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -387,7 +387,7 @@ func (this *Migrator) Migrate() (err error) { return err } - log.Debugf("Waiting for tables to be in place") + log.Infof("Waiting for tables to be in place") <-this.tablesInPlace log.Debugf("Tables are in place") // Yay! We now know the Ghost and Changelog tables are good to examine! From 8b76d0e75b11a536caf5098841c2554f8a7197b5 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Tue, 23 Aug 2016 11:58:52 +0200 Subject: [PATCH 25/29] DML write sets sql_mode to STRICT ALL TABLES --- go/logic/applier.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/go/logic/applier.go b/go/logic/applier.go index a122b71..4a3f318 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -870,7 +870,10 @@ func (this *Applier) ApplyDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) error { if err != nil { return err } - if _, err := tx.Exec("SET SESSION time_zone = '+00:00'"); err != nil { + if _, err := tx.Exec(`SET + SESSION time_zone = '+00:00', + sql_mode = CONCAT(@@session.sql_mode, ',STRICT_ALL_TABLES') + `); err != nil { return err } if _, err := tx.Exec(query, args...); err != nil { From ec7f641ecb89b7268620e214bacdc8e2ca8ac63b Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Tue, 23 Aug 2016 12:13:40 +0200 Subject: [PATCH 26/29] added enum tests --- localtests/enum/create.sql | 27 +++++++++++++++++++++++++++ localtests/enum/extra_args | 1 + 2 files changed, 28 insertions(+) create mode 100644 localtests/enum/create.sql create mode 100644 localtests/enum/extra_args diff --git a/localtests/enum/create.sql b/localtests/enum/create.sql new file mode 100644 index 0000000..6420233 --- /dev/null +++ b/localtests/enum/create.sql @@ -0,0 +1,27 @@ +drop table if exists gh_ost_test; +create table gh_ost_test ( + id int auto_increment, + i int not null, + e enum('red', 'green', 'blue', 'orange') null default null collate 'utf8_bin', + primary key(id) +) auto_increment=1; + +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 + insert into gh_ost_test values (null, 11, 'red'); + insert into gh_ost_test values (null, 13, 'green'); + insert into gh_ost_test values (null, 17, 'blue'); + set @last_insert_id := last_insert_id(); + update gh_ost_test set e='orange' where id = @last_insert_id; + insert into gh_ost_test values (null, 23, null); + set @last_insert_id := last_insert_id(); + update gh_ost_test set i=i+1, e=null where id = @last_insert_id; +end ;; diff --git a/localtests/enum/extra_args b/localtests/enum/extra_args new file mode 100644 index 0000000..f369a56 --- /dev/null +++ b/localtests/enum/extra_args @@ -0,0 +1 @@ +--alter="change e e enum('red', 'green', 'blue', 'orange', 'yellow') null default null collate 'utf8_bin'" From 6b21ade6d087d356ce6c4b14fb3f066c431681e3 Mon Sep 17 00:00:00 2001 From: Paulo Bittencourt Date: Tue, 23 Aug 2016 18:34:10 -0400 Subject: [PATCH 27/29] Check for --test-on-replica-skip-replica-stop in cutOver method --- go/logic/applier.go | 14 +++++--------- go/logic/migrator.go | 11 ++++++++--- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/go/logic/applier.go b/go/logic/applier.go index 9eb115c..ab91fb0 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -568,15 +568,11 @@ func (this *Applier) StartSlaveSQLThread() error { // StopReplication is used by `--test-on-replica` and stops replication. func (this *Applier) StopReplication() error { - if this.migrationContext.TestOnReplicaSkipReplicaStop { - log.Warningf("--test-on-replica-skip-replica-stop enabled, we are not stopping replication.") - } else { - if err := this.StopSlaveIOThread(); err != nil { - return err - } - if err := this.StopSlaveSQLThread(); err != nil { - return err - } + if err := this.StopSlaveIOThread(); err != nil { + return err + } + if err := this.StopSlaveSQLThread(); err != nil { + return err } readBinlogCoordinates, executeBinlogCoordinates, err := mysql.GetReplicationBinlogCoordinates(this.db) diff --git a/go/logic/migrator.go b/go/logic/migrator.go index cc418a9..a5d5e62 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -477,9 +477,14 @@ func (this *Migrator) cutOver() (err error) { // the same cut-over phase as the master would use. That means we take locks // and swap the tables. // The difference is that we will later swap the tables back. - log.Debugf("testing on replica. Stopping replication IO thread") - if err := this.retryOperation(this.applier.StopReplication); err != nil { - return err + + if this.migrationContext.TestOnReplicaSkipReplicaStop { + log.Warningf("--test-on-replica-skip-replica-stop enabled, we are not stopping replication.") + } else { + log.Debugf("testing on replica. Stopping replication IO thread") + if err := this.retryOperation(this.applier.StopReplication); err != nil { + return err + } } // We're merly testing, we don't want to keep this state. Rollback the renames as possible defer this.applier.RenameTablesRollback() From 79399f446beccb9ff7e1501b3440e8219a0ca4ab Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Wed, 24 Aug 2016 12:51:01 +0200 Subject: [PATCH 28/29] added documentation for local tests --- doc/local-tests.md | 22 ++++++++++++++++++++++ localtests/test.sh | 4 +++- 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 doc/local-tests.md diff --git a/doc/local-tests.md b/doc/local-tests.md new file mode 100644 index 0000000..1614ee4 --- /dev/null +++ b/doc/local-tests.md @@ -0,0 +1,22 @@ +# Local tests + +`gh-ost` is continuously tested in production via `--test-on-replica alter='engine=innodb'`. These tests check the GitHub workload and usage, but not necessarily the general case. + +Local tests are an additional layer of tests. They will eventually be part of continuous integration tests. + +Local tests test explicit use cases, such as column renames, mix of time zones, special types and alters. Traits of a single test: + +- Composed of a single table. +- A single alter. +- By default the alter is `engine=innodb`, but this can be overridden per-test +- Scheduled DML operations, executed via `event_scheduler`. +- `gh-ost` is set to execute and throttle for `5` seconds, at which time all tested DMLs are expected to operate. +- The test requires a replication topology and utilizes `--test-on-replica` +- The test checksums the two tables (original and _ghost_) and expects identical checksum +- By default the test selects all (`*`) columns, but this can be overridden per-test + +Tests are found under [localtests](https://github.com/github/gh-ost/tree/master/localtests). A single test is a subdirectory and tests are iterated alphabetically. + +New data-integrity, synchronization issues or otherwise concerns are expected to be tested by new test cases. + +While this is merged work is still ongoing. diff --git a/localtests/test.sh b/localtests/test.sh index 83e8537..c622edf 100755 --- a/localtests/test.sh +++ b/localtests/test.sh @@ -1,6 +1,8 @@ #!/bin/bash -# set -e +# Local integration tests. To be used by CI. +# See https://github.com/github/gh-ost/tree/doc/local-tests.md +# tests_path=$(dirname $0) test_logfile=/tmp/gh-ost-test.log From b0a2e4c65079c275b442a134a57e2ccaedf84881 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Wed, 24 Aug 2016 14:18:49 +0200 Subject: [PATCH 29/29] graceful sleep for replica lag --- localtests/test.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/localtests/test.sh b/localtests/test.sh index c622edf..ee82623 100755 --- a/localtests/test.sh +++ b/localtests/test.sh @@ -49,7 +49,9 @@ test_single() { if [ -f $tests_path/$test_name/test_columns ] ; then columns=$(cat $tests_path/$test_name/test_columns) fi - + # graceful sleep for replica to catch up + sleep 1 + # cmd="go run go/cmd/gh-ost/main.go \ --user=gh-ost \ --password=gh-ost \