From 671fc71067fe8d50730eae0580ee42e2481bc877 Mon Sep 17 00:00:00 2001 From: Paulo Bittencourt Date: Tue, 13 Sep 2016 13:58:52 -0400 Subject: [PATCH 01/10] Add test case for altering a column's charset This catches a bug in `sql.BuildDMLInsertQuery` where we are using the target column's Charset to drive the value conversion. In the case where we are altering the charset, the Charset used for conversion will be different than the original column's charset, resulting in an erroneous conversion. --- localtests/alter-charset/create.sql | 23 +++++++++++++++++++++++ localtests/alter-charset/extra_args | 1 + 2 files changed, 24 insertions(+) create mode 100644 localtests/alter-charset/create.sql create mode 100644 localtests/alter-charset/extra_args diff --git a/localtests/alter-charset/create.sql b/localtests/alter-charset/create.sql new file mode 100644 index 0000000..e2861d1 --- /dev/null +++ b/localtests/alter-charset/create.sql @@ -0,0 +1,23 @@ +drop table if exists gh_ost_test; +create table gh_ost_test ( + id int auto_increment, + t varchar(128) charset latin1 collate latin1_swedish_ci, + tutf8 varchar(128) charset utf8, + tutf8mb4 varchar(128) charset utf8mb4, + 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, md5(rand()), md5(rand()), md5(rand())); + insert into gh_ost_test values (null, 'átesting', 'átesting', 'átesting'); + insert into gh_ost_test values (null, 'testátest', 'testátest', '🍻😀'); +end ;; diff --git a/localtests/alter-charset/extra_args b/localtests/alter-charset/extra_args new file mode 100644 index 0000000..29e526e --- /dev/null +++ b/localtests/alter-charset/extra_args @@ -0,0 +1 @@ +--alter='MODIFY `t` varchar(128) CHARACTER SET utf8mb4 NOT NULL' From 25b5474cfa8927730ef464d20866e76b9fa427e7 Mon Sep 17 00:00:00 2001 From: Paulo Bittencourt Date: Wed, 14 Sep 2016 10:59:52 -0400 Subject: [PATCH 02/10] Add test case for renaming a column and only applying inserts This catches a bug in `sql.BuildDMLInsertQuery` where we we are fetching the insert values using the renamed column's name, and end up fetching the value of the wrong column. The test in `localtests/rename` did not catch this because binlog update events were "correcting" the error, as they follow a different code path that does not contain the bug. --- localtests/rename-inserts-only/create.sql | 22 ++++++++++++++++++++++ localtests/rename-inserts-only/extra_args | 1 + 2 files changed, 23 insertions(+) create mode 100644 localtests/rename-inserts-only/create.sql create mode 100644 localtests/rename-inserts-only/extra_args diff --git a/localtests/rename-inserts-only/create.sql b/localtests/rename-inserts-only/create.sql new file mode 100644 index 0000000..be847ee --- /dev/null +++ b/localtests/rename-inserts-only/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, 11, 23); + insert into gh_ost_test values (null, 13, 23); + insert into gh_ost_test values (null, rand(), rand()); +end ;; diff --git a/localtests/rename-inserts-only/extra_args b/localtests/rename-inserts-only/extra_args new file mode 100644 index 0000000..d36d5ee --- /dev/null +++ b/localtests/rename-inserts-only/extra_args @@ -0,0 +1 @@ +--alter="change column c2 c3 int not null" --approve-renamed-columns From 2f80c9d424056a10e44e2e5c13f9e96abf2b8522 Mon Sep 17 00:00:00 2001 From: Paulo Bittencourt Date: Wed, 14 Sep 2016 11:07:36 -0400 Subject: [PATCH 03/10] Used the source column definition to fetch and convert insert values When processing binlog insert statements, we want to use `sharedColumns` to decide which values to fetch and convert from the insert DML event. We only want to `mappedShareColumns` to define the column names in the `replace into ...` statement. --- go/sql/builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/sql/builder.go b/go/sql/builder.go index 260f276..4d6cb88 100644 --- a/go/sql/builder.go +++ b/go/sql/builder.go @@ -344,7 +344,7 @@ func BuildDMLInsertQuery(databaseName, tableName string, tableColumns, sharedCol databaseName = EscapeName(databaseName) tableName = EscapeName(tableName) - for _, column := range mappedSharedColumns.Columns() { + for _, column := range sharedColumns.Columns() { tableOrdinal := tableColumns.Ordinals[column.Name] arg := column.convertArg(args[tableOrdinal]) sharedArgs = append(sharedArgs, arg) From 8c5dcca187a42d24e38b8f3dd8c572e0f90c4ef3 Mon Sep 17 00:00:00 2001 From: Paulo Bittencourt Date: Mon, 19 Sep 2016 09:37:08 -0400 Subject: [PATCH 04/10] Add test for modifying columns to different charsets --- localtests/alter-charset/create.sql | 9 +++++---- localtests/alter-charset/extra_args | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/localtests/alter-charset/create.sql b/localtests/alter-charset/create.sql index e2861d1..e473073 100644 --- a/localtests/alter-charset/create.sql +++ b/localtests/alter-charset/create.sql @@ -1,7 +1,8 @@ drop table if exists gh_ost_test; create table gh_ost_test ( id int auto_increment, - t varchar(128) charset latin1 collate latin1_swedish_ci, + t1 varchar(128) charset latin1 collate latin1_swedish_ci, + t2 varchar(128) charset latin1 collate latin1_swedish_ci, tutf8 varchar(128) charset utf8, tutf8mb4 varchar(128) charset utf8mb4, primary key(id) @@ -17,7 +18,7 @@ create event gh_ost_test enable do begin - insert into gh_ost_test values (null, md5(rand()), md5(rand()), md5(rand())); - insert into gh_ost_test values (null, 'átesting', 'átesting', 'átesting'); - insert into gh_ost_test values (null, 'testátest', 'testátest', '🍻😀'); + insert into gh_ost_test values (null, md5(rand()), md5(rand()), md5(rand()), md5(rand())); + insert into gh_ost_test values (null, 'átesting', 'átesting', 'átesting', 'átesting'); + insert into gh_ost_test values (null, 'testátest', 'testátest', 'testátest', '🍻😀'); end ;; diff --git a/localtests/alter-charset/extra_args b/localtests/alter-charset/extra_args index 29e526e..c39b022 100644 --- a/localtests/alter-charset/extra_args +++ b/localtests/alter-charset/extra_args @@ -1 +1 @@ ---alter='MODIFY `t` varchar(128) CHARACTER SET utf8mb4 NOT NULL' +--alter='MODIFY `t1` varchar(128) CHARACTER SET utf8mb4 NOT NULL, MODIFY `t2` varchar(128) CHARACTER SET latin2 NOT NULL, MODIFY `tutf8` varchar(128) CHARACTER SET latin1 NOT NULL' From d32534710e460cad60b881a54f2faeed17ebaf3d Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Tue, 27 Sep 2016 12:41:46 +0200 Subject: [PATCH 05/10] added tests for unsigned rename/reorder --- localtests/unsigned-rename/create.sql | 24 +++++++++++++++++++++++ localtests/unsigned-rename/extra_args | 1 + localtests/unsigned-rename/ghost_columns | 1 + localtests/unsigned-rename/orig_columns | 1 + localtests/unsigned-reorder/create.sql | 24 +++++++++++++++++++++++ localtests/unsigned-reorder/extra_args | 1 + localtests/unsigned-reorder/ghost_columns | 1 + localtests/unsigned-reorder/orig_columns | 1 + 8 files changed, 54 insertions(+) create mode 100644 localtests/unsigned-rename/create.sql create mode 100644 localtests/unsigned-rename/extra_args create mode 100644 localtests/unsigned-rename/ghost_columns create mode 100644 localtests/unsigned-rename/orig_columns create mode 100644 localtests/unsigned-reorder/create.sql create mode 100644 localtests/unsigned-reorder/extra_args create mode 100644 localtests/unsigned-reorder/ghost_columns create mode 100644 localtests/unsigned-reorder/orig_columns diff --git a/localtests/unsigned-rename/create.sql b/localtests/unsigned-rename/create.sql new file mode 100644 index 0000000..d98bb07 --- /dev/null +++ b/localtests/unsigned-rename/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-rename/extra_args b/localtests/unsigned-rename/extra_args new file mode 100644 index 0000000..39bac0c --- /dev/null +++ b/localtests/unsigned-rename/extra_args @@ -0,0 +1 @@ +--alter="change column iu iu_renamed int unsigned" --approve-renamed-columns diff --git a/localtests/unsigned-rename/ghost_columns b/localtests/unsigned-rename/ghost_columns new file mode 100644 index 0000000..cdee3f1 --- /dev/null +++ b/localtests/unsigned-rename/ghost_columns @@ -0,0 +1 @@ +id, i, bi, iu_renamed, biu diff --git a/localtests/unsigned-rename/orig_columns b/localtests/unsigned-rename/orig_columns new file mode 100644 index 0000000..96a3ec8 --- /dev/null +++ b/localtests/unsigned-rename/orig_columns @@ -0,0 +1 @@ +id, i, bi, iu, biu diff --git a/localtests/unsigned-reorder/create.sql b/localtests/unsigned-reorder/create.sql new file mode 100644 index 0000000..d98bb07 --- /dev/null +++ b/localtests/unsigned-reorder/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-reorder/extra_args b/localtests/unsigned-reorder/extra_args new file mode 100644 index 0000000..4b0bfc7 --- /dev/null +++ b/localtests/unsigned-reorder/extra_args @@ -0,0 +1 @@ +--alter="change column iu iu int unsigned after id" --approve-renamed-columns diff --git a/localtests/unsigned-reorder/ghost_columns b/localtests/unsigned-reorder/ghost_columns new file mode 100644 index 0000000..96a3ec8 --- /dev/null +++ b/localtests/unsigned-reorder/ghost_columns @@ -0,0 +1 @@ +id, i, bi, iu, biu diff --git a/localtests/unsigned-reorder/orig_columns b/localtests/unsigned-reorder/orig_columns new file mode 100644 index 0000000..96a3ec8 --- /dev/null +++ b/localtests/unsigned-reorder/orig_columns @@ -0,0 +1 @@ +id, i, bi, iu, biu From 253e0a140687820c62426e33d29561e93b151e15 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Tue, 27 Sep 2016 12:45:22 +0200 Subject: [PATCH 06/10] columns not null in test --- localtests/unsigned-rename/extra_args | 2 +- localtests/unsigned-reorder/extra_args | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/localtests/unsigned-rename/extra_args b/localtests/unsigned-rename/extra_args index 39bac0c..1e32037 100644 --- a/localtests/unsigned-rename/extra_args +++ b/localtests/unsigned-rename/extra_args @@ -1 +1 @@ ---alter="change column iu iu_renamed int unsigned" --approve-renamed-columns +--alter="change column iu iu_renamed int unsigned not null" --approve-renamed-columns diff --git a/localtests/unsigned-reorder/extra_args b/localtests/unsigned-reorder/extra_args index 4b0bfc7..ccd3fe3 100644 --- a/localtests/unsigned-reorder/extra_args +++ b/localtests/unsigned-reorder/extra_args @@ -1 +1 @@ ---alter="change column iu iu int unsigned after id" --approve-renamed-columns +--alter="change column iu iu int unsigned not null after id" --approve-renamed-columns From 109ddb0452a617972cfb9b30ddbccf2629a51e89 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Tue, 27 Sep 2016 12:56:37 +0200 Subject: [PATCH 07/10] fixed integer random values --- localtests/rename-inserts-only/create.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/localtests/rename-inserts-only/create.sql b/localtests/rename-inserts-only/create.sql index be847ee..5a3f9e6 100644 --- a/localtests/rename-inserts-only/create.sql +++ b/localtests/rename-inserts-only/create.sql @@ -18,5 +18,5 @@ create event gh_ost_test begin insert into gh_ost_test values (null, 11, 23); insert into gh_ost_test values (null, 13, 23); - insert into gh_ost_test values (null, rand(), rand()); + insert into gh_ost_test values (null, floor(rand()*pow(2,32)), floor(rand()*pow(2,32))); end ;; From db2c8a254f09819adc40e7cf4c8b288dd98d7779 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Tue, 27 Sep 2016 12:57:05 +0200 Subject: [PATCH 08/10] added update/delete tests for multi-charset/alter tests --- localtests/alter-charset-all-dml/create.sql | 28 +++++++++++++++++++++ localtests/alter-charset-all-dml/extra_args | 1 + 2 files changed, 29 insertions(+) create mode 100644 localtests/alter-charset-all-dml/create.sql create mode 100644 localtests/alter-charset-all-dml/extra_args diff --git a/localtests/alter-charset-all-dml/create.sql b/localtests/alter-charset-all-dml/create.sql new file mode 100644 index 0000000..3126a04 --- /dev/null +++ b/localtests/alter-charset-all-dml/create.sql @@ -0,0 +1,28 @@ +drop table if exists gh_ost_test; +create table gh_ost_test ( + id int auto_increment, + t1 varchar(128) charset latin1 collate latin1_swedish_ci, + t2 varchar(128) charset latin1 collate latin1_swedish_ci, + tutf8 varchar(128) charset utf8, + tutf8mb4 varchar(128) charset utf8mb4, + random_value varchar(128) charset ascii, + 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, md5(rand()), md5(rand()), md5(rand()), md5(rand()), md5(rand())); + insert into gh_ost_test values (null, 'átesting', 'átesting', 'átesting', 'átesting', md5(rand())); + insert into gh_ost_test values (null, 'átesting_del', 'átesting', 'átesting', 'átesting', md5(rand())); + insert into gh_ost_test values (null, 'testátest', 'testátest', 'testátest', '🍻😀', md5(rand())); + update gh_ost_test set t1='átesting2' where t1='átesting' order by id desc limit 1; + delete from gh_ost_test where t1='átesting_del' order by id desc limit 1; +end ;; diff --git a/localtests/alter-charset-all-dml/extra_args b/localtests/alter-charset-all-dml/extra_args new file mode 100644 index 0000000..c39b022 --- /dev/null +++ b/localtests/alter-charset-all-dml/extra_args @@ -0,0 +1 @@ +--alter='MODIFY `t1` varchar(128) CHARACTER SET utf8mb4 NOT NULL, MODIFY `t2` varchar(128) CHARACTER SET latin2 NOT NULL, MODIFY `tutf8` varchar(128) CHARACTER SET latin1 NOT NULL' From 51f0c0d95797abdf0d0d291e7e217acd38386641 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Tue, 27 Sep 2016 13:08:31 +0200 Subject: [PATCH 09/10] fix arg conversion in BuildDMLUpdateQuery --- go/sql/builder.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/go/sql/builder.go b/go/sql/builder.go index 4d6cb88..2a88e3e 100644 --- a/go/sql/builder.go +++ b/go/sql/builder.go @@ -392,10 +392,9 @@ func BuildDMLUpdateQuery(databaseName, tableName string, tableColumns, sharedCol databaseName = EscapeName(databaseName) tableName = EscapeName(tableName) - for i, column := range sharedColumns.Columns() { - mappedColumn := mappedSharedColumns.Columns()[i] + for _, column := range sharedColumns.Columns() { tableOrdinal := tableColumns.Ordinals[column.Name] - arg := mappedColumn.convertArg(valueArgs[tableOrdinal]) + arg := column.convertArg(valueArgs[tableOrdinal]) sharedArgs = append(sharedArgs, arg) } From 52d7bc31ea22b47272404c366b49858053229ed3 Mon Sep 17 00:00:00 2001 From: Josh Bodah Date: Wed, 28 Sep 2016 15:20:48 -0400 Subject: [PATCH 10/10] Change docs from referencing gh-osc Changes docs to use new exe name of `gh-ost` instead of old name `gh-osc` --- doc/testing-on-replica.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/testing-on-replica.md b/doc/testing-on-replica.md index cd3cfd1..ae5fc03 100644 --- a/doc/testing-on-replica.md +++ b/doc/testing-on-replica.md @@ -49,12 +49,12 @@ It's your job to: Simple: ```shell -$ gh-osc --host=myhost.com --conf=/etc/gh-ost.cnf --database=test --table=sample_table --alter="engine=innodb" --chunk-size=2000 --max-load=Threads_connected=20 --initially-drop-ghost-table --initially-drop-old-table --test-on-replica --verbose --execute +$ gh-ost --host=myhost.com --conf=/etc/gh-ost.cnf --database=test --table=sample_table --alter="engine=innodb" --chunk-size=2000 --max-load=Threads_connected=20 --initially-drop-ghost-table --initially-drop-old-table --test-on-replica --verbose --execute ``` Elaborate: ```shell -$ gh-osc --host=myhost.com --conf=/etc/gh-ost.cnf --database=test --table=sample_table --alter="engine=innodb" --chunk-size=2000 --max-load=Threads_connected=20 --switch-to-rbr --initially-drop-ghost-table --initially-drop-old-table --test-on-replica --postpone-cut-over-flag-file=/tmp/ghost-postpone.flag --exact-rowcount --concurrent-rowcount --allow-nullable-unique-key --verbose --execute +$ gh-ost --host=myhost.com --conf=/etc/gh-ost.cnf --database=test --table=sample_table --alter="engine=innodb" --chunk-size=2000 --max-load=Threads_connected=20 --switch-to-rbr --initially-drop-ghost-table --initially-drop-old-table --test-on-replica --postpone-cut-over-flag-file=/tmp/ghost-postpone.flag --exact-rowcount --concurrent-rowcount --allow-nullable-unique-key --verbose --execute ``` - Count exact number of rows (makes ETA estimation very good). This goes at the expense of paying the time for issuing a `SELECT COUNT(*)` on your table. We use this lovingly. - Automatically switch to `RBR` if replica is configured as `SBR`. See also: [migrating with SBR](migrating-with-sbr.md)