Merge lp:~percona-toolkit-dev/percona-toolkit/fix-pt-heartbeat-dupe-key-bug-1004567 into lp:percona-toolkit/2.1

Proposed by Daniel Nichter on 2012-07-20
Status: Merged
Merged at revision: 320
Proposed branch: lp:~percona-toolkit-dev/percona-toolkit/fix-pt-heartbeat-dupe-key-bug-1004567
Merge into: lp:percona-toolkit/2.1
Diff against target: 173 lines (+82/-16)
2 files modified
bin/pt-heartbeat (+5/-1)
t/pt-heartbeat/basics.t (+77/-15)
To merge this branch: bzr merge lp:~percona-toolkit-dev/percona-toolkit/fix-pt-heartbeat-dupe-key-bug-1004567
Reviewer Review Type Date Requested Status
Daniel Nichter Approve on 2012-07-20
Review via email: mp+116083@code.launchpad.net
To post a comment you must log in.
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/pt-heartbeat'
2--- bin/pt-heartbeat 2012-07-20 20:25:10 +0000
3+++ bin/pt-heartbeat 2012-07-20 22:10:25 +0000
4@@ -3352,10 +3352,14 @@
5 PTDEBUG && _d($sql);
6 $dbh->do($sql);
7
8- $sql = "INSERT INTO $db_tbl (ts, server_id) VALUES (NOW(), $server_id)";
9+ $sql = ($o->get('replace') ? "REPLACE" : "INSERT")
10+ . qq/ INTO $db_tbl (ts, server_id) VALUES (NOW(), $server_id)/;
11 PTDEBUG && _d($sql);
12 # This may fail if the table already existed and already had this row.
13 # We eval to ignore this possibility.
14+ # NOTE: This can break replication though! See:
15+ # https://bugs.launchpad.net/percona-toolkit/+bug/1004567
16+ # So --replace should be used in most cases.
17 eval { $dbh->do($sql); };
18 }
19
20
21=== modified file 't/pt-heartbeat/basics.t'
22--- t/pt-heartbeat/basics.t 2012-06-09 18:43:33 +0000
23+++ t/pt-heartbeat/basics.t 2012-07-20 22:10:25 +0000
24@@ -17,16 +17,20 @@
25
26 my $dp = new DSNParser(opts=>$dsn_opts);
27 my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp);
28-my $dbh = $sb->get_dbh_for('master');
29+my $master_dbh = $sb->get_dbh_for('master');
30+my $slave1_dbh = $sb->get_dbh_for('slave1');
31
32-if ( !$dbh ) {
33+if ( !$master_dbh ) {
34 plan skip_all => 'Cannot connect to sandbox master';
35 }
36+elsif ( !$slave1_dbh ) {
37+ plan skip_all => 'Cannot connect to sandbox slave1';
38+}
39 else {
40- plan tests => 18;
41+ plan tests => 22;
42 }
43
44-$sb->create_dbs($dbh, ['test']);
45+$sb->create_dbs($master_dbh, ['test']);
46
47 my $output;
48 my $cnf = '/tmp/12345/my.sandbox.cnf';
49@@ -35,10 +39,10 @@
50 my $sent_file = "/tmp/pt-heartbeat-sentinel";
51 my $ps_grep_cmd = "ps x | grep pt-heartbeat | grep daemonize | grep -v grep";
52
53-`rm $sent_file 2>/dev/null`;
54+diag(`rm $sent_file 2>/dev/null`);
55
56-$dbh->do('drop table if exists test.heartbeat');
57-$dbh->do(q{CREATE TABLE test.heartbeat (
58+$master_dbh->do('drop table if exists test.heartbeat');
59+$master_dbh->do(q{CREATE TABLE test.heartbeat (
60 id int NOT NULL PRIMARY KEY,
61 ts datetime NOT NULL
62 ) ENGINE=MEMORY});
63@@ -53,7 +57,7 @@
64 $output = output(
65 sub { pt_heartbeat::main('-F', $cnf, qw(-D test --check)) },
66 );
67-my $row = $dbh->selectall_hashref('select * from test.heartbeat', 'id');
68+my $row = $master_dbh->selectall_hashref('select * from test.heartbeat', 'id');
69 is(
70 $row->{1}->{id},
71 1,
72@@ -62,7 +66,7 @@
73
74 # Run one instance with --replace to create the table.
75 `$cmd -D test --update --replace --run-time 1s`;
76-ok($dbh->selectrow_array('select id from test.heartbeat'), 'Record is there');
77+ok($master_dbh->selectrow_array('select id from test.heartbeat'), 'Record is there');
78
79 # Check the delay and ensure it is only a single line with nothing but the
80 # delay (no leading whitespace or anything).
81@@ -105,7 +109,7 @@
82 unlike($output, qr/$cmd/, 'It is not running');
83 ok(-f $sent_file, 'Sentinel file is there');
84 unlink($sent_file);
85-$dbh->do('drop table if exists test.heartbeat'); # This will kill it
86+$master_dbh->do('drop table if exists test.heartbeat'); # This will kill it
87
88 # #############################################################################
89 # Issue 353: Add --create-table to mk-heartbeat
90@@ -114,10 +118,10 @@
91 # These creates the new table format, whereas the preceding tests used the
92 # old format, so tests from here on may need --master-server-id.
93
94-$dbh->do('drop table if exists test.heartbeat');
95+$master_dbh->do('drop table if exists test.heartbeat');
96 diag(`$cmd --update --run-time 1s --database test --table heartbeat --create-table`);
97-$dbh->do('use test');
98-$output = $dbh->selectcol_arrayref("SHOW TABLES LIKE 'heartbeat'");
99+$master_dbh->do('use test');
100+$output = $master_dbh->selectcol_arrayref("SHOW TABLES LIKE 'heartbeat'");
101 is(
102 $output->[0],
103 'heartbeat',
104@@ -136,9 +140,67 @@
105 );
106
107 # #############################################################################
108+# Bug 1004567: pt-heartbeat --update --replace causes duplicate key error
109+# #############################################################################
110+
111+# Create the heartbeat table on the master and slave.
112+$master_dbh->do("DROP TABLE IF EXISTS test.heartbeat");
113+$sb->wait_for_slaves();
114+
115+$output = output(
116+ sub { pt_heartbeat::main("F=/tmp/12345/my.sandbox.cnf",
117+ qw(-D test --update --replace --create-table --run-time 1))
118+ }
119+);
120+
121+$row = $master_dbh->selectrow_arrayref('SELECT server_id FROM test.heartbeat');
122+is(
123+ $row->[0],
124+ '12345',
125+ 'Heartbeat on master'
126+);
127+
128+$sb->wait_for_slaves();
129+
130+$row = $slave1_dbh->selectrow_arrayref('SELECT server_id FROM test.heartbeat');
131+is(
132+ $row->[0],
133+ '12345',
134+ 'Heartbeat on slave1'
135+);
136+
137+# Drop the heartbeat table only on the master.
138+$master_dbh->do("SET SQL_LOG_BIN=0");
139+$master_dbh->do("DROP TABLE test.heartbeat");
140+$master_dbh->do("SET SQL_LOG_BIN=1");
141+
142+# Re-create the heartbeat table on the master.
143+$output = output(
144+ sub { pt_heartbeat::main("F=/tmp/12345/my.sandbox.cnf",
145+ qw(-D test --update --replace --create-table --run-time 1))
146+ }
147+);
148+
149+$row = $master_dbh->selectrow_arrayref('SELECT server_id FROM test.heartbeat');
150+is(
151+ $row->[0],
152+ '12345',
153+ 'New heartbeat on master'
154+);
155+
156+$sb->wait_for_slaves();
157+
158+$row = $slave1_dbh->selectrow_hashref("SHOW SLAVE STATUS");
159+is(
160+ $row->{last_error},
161+ '',
162+ "No slave error"
163+);
164+
165+# #############################################################################
166 # Done.
167 # #############################################################################
168-`rm $pid_file $sent_file 2>/dev/null`;
169-$sb->wipe_clean($dbh);
170+diag(`rm $pid_file $sent_file 2>/dev/null`);
171+$sb->wipe_clean($master_dbh);
172 ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox");
173 exit;

Subscribers

People subscribed via source and target branches