Merge lp:~percona-toolkit-dev/percona-toolkit/pt-table-checksum-fails-on-BINARY-field-in-PK-1381280 into lp:~percona-toolkit-dev/percona-toolkit/release-2.2.14

Proposed by Frank Cizmich
Status: Merged
Approved by: Daniel Nichter
Approved revision: 615
Merged at revision: 615
Proposed branch: lp:~percona-toolkit-dev/percona-toolkit/pt-table-checksum-fails-on-BINARY-field-in-PK-1381280
Merge into: lp:~percona-toolkit-dev/percona-toolkit/release-2.2.14
Diff against target: 115 lines (+42/-13)
2 files modified
bin/pt-table-checksum (+41/-12)
t/pt-table-checksum/progress.t (+1/-1)
To merge this branch: bzr merge lp:~percona-toolkit-dev/percona-toolkit/pt-table-checksum-fails-on-BINARY-field-in-PK-1381280
Reviewer Review Type Date Requested Status
Daniel Nichter Approve
Review via email: mp+255002@code.launchpad.net

Description of the change

Problem:

pt-table-checksum fails on tables where a binary column is used as key. This is because the upper and lower boundary column of the checksum table is of type text and fails when storing these keys.

Solution/Workaround:

Good results have been obtained changing checksum datatype to binary (blob)
Since a foolproof testing of this approach in all cases is very difficult, a less risky compromise approach is used: the --binary-key option is provided and documented so as to permit users to create the checksum table with blob datatype.

To post a comment you must log in.
Revision history for this message
Daniel Nichter (daniel-nichter) wrote :

PT tries to use "index" instead of "key" because it's a little more proper (CREATE INDEX, not CREATE KEY; --chunk-index; etc.), so I'd suggest --binary-index as the option name.

MySQL columns types are usually written as uppercase...

Revision history for this message
Daniel Nichter (daniel-nichter) :
615. By Frank Cizmich

pt-t-c changed text of warnings and docs. Minor tweak of test for Ubuntu 14

Revision history for this message
Frank Cizmich (frank-cizmich) wrote :

Changed texts and option name as suggested.

Also, unrelated, tweaked progress.t test, which somehow failed on ubuntu 14.04 unless I added a second of delay in one of the tests (both on virtualbox and my own)

Revision history for this message
Daniel Nichter (daniel-nichter) wrote :

s/datatype/data type/g :-) Otherwise fine by me.

review: Approve
616. By Frank Cizmich

minor typo

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'bin/pt-table-checksum'
--- bin/pt-table-checksum 2015-01-23 10:19:56 +0000
+++ bin/pt-table-checksum 2015-04-07 21:06:09 +0000
@@ -10866,6 +10866,7 @@
10866 }10866 }
10867 }10867 }
10868 }10868 }
10869
10869 10870
10870 # USE the correct db (probably the repl db, but maybe --replicate-database).10871 # USE the correct db (probably the repl db, but maybe --replicate-database).
10871 use_repl_db(%args);10872 use_repl_db(%args);
@@ -10888,6 +10889,7 @@
10888 . "You need to create the table.\n";10889 . "You need to create the table.\n";
10889 }10890 }
1089010891
10892
10891 # We used to check the table privs here, but:10893 # We used to check the table privs here, but:
10892 # https://bugs.launchpad.net/percona-toolkit/+bug/91616810894 # https://bugs.launchpad.net/percona-toolkit/+bug/916168
1089310895
@@ -10974,6 +10976,18 @@
10974 }10976 }
10975 }10977 }
1097610978
10979 if ( $o->get('binary-index') ) {
10980 PTDEBUG && _d('--binary-index : checking if replicate table has binary type columns');
10981 my $create_table = $tp->get_create_table( $dbh, $db, $tbl );
10982 if ( $create_table !~ /lower_boundary`?\s+BLOB/si
10983 || $create_table !~ /upper_boundary`?\s+BLOB/si )
10984 {
10985 die "--binary-index was specified but the current checksum table ($db.$tbl) uses"
10986 ." TEXT columns. To use BLOB columns, drop the current checksum table, then recreate"
10987 ." it by specifying --create-replicate-table --binary-index.";
10988 }
10989 }
10990
10977 return; # success, repl table is ready to go10991 return; # success, repl table is ready to go
10978}10992}
1097910993
@@ -11132,6 +11146,10 @@
11132 my $sql = $o->read_para_after(__FILE__, qr/MAGIC_create_replicate/);11146 my $sql = $o->read_para_after(__FILE__, qr/MAGIC_create_replicate/);
11133 $sql =~ s/CREATE TABLE checksums/CREATE TABLE IF NOT EXISTS $repl_table/;11147 $sql =~ s/CREATE TABLE checksums/CREATE TABLE IF NOT EXISTS $repl_table/;
11134 $sql =~ s/;$//;11148 $sql =~ s/;$//;
11149 if ( $o->get('binary-index') ) {
11150 $sql =~ s/`?lower_boundary`?\s+TEXT/`lower_boundary` BLOB/is;
11151 $sql =~ s/`?upper_boundary`?\s+TEXT/`upper_boundary` BLOB/is;
11152 }
11135 PTDEBUG && _d($dbh, $sql);11153 PTDEBUG && _d($dbh, $sql);
11136 eval {11154 eval {
11137 $dbh->do($sql);11155 $dbh->do($sql);
@@ -11808,6 +11826,15 @@
1180811826
11809See "Replicas using row-based replication" under L<"LIMITATIONS">.11827See "Replicas using row-based replication" under L<"LIMITATIONS">.
1181011828
11829=item --binary-index
11830
11831This option modifies the behavior of L<"--create-replicate-table"> such that the
11832replicate table's upper and lower boundary columns are created with the BLOB
11833data type.
11834This is useful in cases where you have trouble checksuming tables with keys that
11835include a binary data type or that have non-standard character sets.
11836See L<"--replicate">.
11837
11811=item --check-interval11838=item --check-interval
1181211839
11813type: time; default: 1; group: Throttle11840type: time; default: 1; group: Throttle
@@ -12342,22 +12369,24 @@
12342structure (MAGIC_create_replicate):12369structure (MAGIC_create_replicate):
1234312370
12344 CREATE TABLE checksums (12371 CREATE TABLE checksums (
12345 db char(64) NOT NULL,12372 db CHAR(64) NOT NULL,
12346 tbl char(64) NOT NULL,12373 tbl CHAR(64) NOT NULL,
12347 chunk int NOT NULL,12374 chunk INT NOT NULL,
12348 chunk_time float NULL,12375 chunk_time FLOAT NULL,
12349 chunk_index varchar(200) NULL,12376 chunk_index VARCHAR(200) NULL,
12350 lower_boundary text NULL,12377 lower_boundary TEXT NULL,
12351 upper_boundary text NULL,12378 upper_boundary TEXT NULL,
12352 this_crc char(40) NOT NULL,12379 this_crc CHAR(40) NOT NULL,
12353 this_cnt int NOT NULL,12380 this_cnt INT NOT NULL,
12354 master_crc char(40) NULL,12381 master_crc CHAR(40) NULL,
12355 master_cnt int NULL,12382 master_cnt INT NULL,
12356 ts timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,12383 ts TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
12357 PRIMARY KEY (db, tbl, chunk),12384 PRIMARY KEY (db, tbl, chunk),
12358 INDEX ts_db_tbl (ts, db, tbl)12385 INDEX ts_db_tbl (ts, db, tbl)
12359 ) ENGINE=InnoDB;12386 ) ENGINE=InnoDB;
1236012387
12388Note: lower_boundary and upper_boundary data type can be BLOB. See L<"--binary-index">.
12389
12361By default, L<"--[no]create-replicate-table"> is true, so the database and12390By default, L<"--[no]create-replicate-table"> is true, so the database and
12362the table specified by this option are created automatically if they do not12391the table specified by this option are created automatically if they do not
12363exist.12392exist.
1236412393
=== modified file 't/pt-table-checksum/progress.t'
--- t/pt-table-checksum/progress.t 2013-03-02 02:02:13 +0000
+++ t/pt-table-checksum/progress.t 2015-04-07 21:06:09 +0000
@@ -68,7 +68,7 @@
68# then starts it again.68# then starts it again.
69# TEST_WISHLIST PLUGIN_WISHLIST: do this with a plugin to the tool itself,69# TEST_WISHLIST PLUGIN_WISHLIST: do this with a plugin to the tool itself,
70# not in this unreliable fashion.70# not in this unreliable fashion.
71system("$trunk/util/wait-to-exec '$scripts/wait-for-chunk.sh 12345 sakila city 1' '$scripts/exec-wait-exec.sh 12347 \"stop slave sql_thread\" 2 \"start slave sql_thread\"' 3 >/dev/null &");71system("$trunk/util/wait-to-exec '$scripts/wait-for-chunk.sh 12345 sakila city 1' '$scripts/exec-wait-exec.sh 12347 \"stop slave sql_thread\" 2 \"start slave sql_thread\"' 4 >/dev/null &");
7272
73$output = output(73$output = output(
74 sub { pt_table_checksum::main(@args, qw(-d sakila)); },74 sub { pt_table_checksum::main(@args, qw(-d sakila)); },

Subscribers

People subscribed via source and target branches

to all changes: