Merge lp:~percona-toolkit-dev/percona-toolkit/fix-886059-pt-heartbeat-timezones into lp:percona-toolkit/2.1

Proposed by Daniel Nichter on 2012-12-13
Status: Merged
Approved by: Daniel Nichter on 2012-12-14
Approved revision: 505
Merged at revision: 507
Proposed branch: lp:~percona-toolkit-dev/percona-toolkit/fix-886059-pt-heartbeat-timezones
Merge into: lp:percona-toolkit/2.1
Diff against target: 198 lines (+127/-10)
2 files modified
bin/pt-heartbeat (+14/-10)
t/pt-heartbeat/bugs.t (+113/-0)
To merge this branch: bzr merge lp:~percona-toolkit-dev/percona-toolkit/fix-886059-pt-heartbeat-timezones
Reviewer Review Type Date Requested Status
Brian Fraser (community) 2012-12-13 Approve on 2012-12-13
Daniel Nichter Approve on 2012-12-13
Review via email: mp+139737@code.launchpad.net
To post a comment you must log in.
review: Approve
Brian Fraser (fraserbn) :
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-12-03 03:48:11 +0000
3+++ bin/pt-heartbeat 2012-12-13 16:24:27 +0000
4@@ -4734,7 +4734,7 @@
5 $dbh->do($sql);
6
7 $sql = ($o->get('replace') ? "REPLACE" : "INSERT")
8- . qq/ INTO $db_tbl (ts, server_id) VALUES (NOW(), $server_id)/;
9+ . qq/ INTO $db_tbl (ts, server_id) VALUES (UTC_TIMESTAMP(), $server_id)/;
10 PTDEBUG && _d($sql);
11 # This may fail if the table already existed and already had this row.
12 # We eval to ignore this possibility.
13@@ -4831,7 +4831,7 @@
14 PTDEBUG && _d('No heartbeat row in table');
15 if ( $o->get('insert-heartbeat-row') ) {
16 my $sql = "INSERT INTO $db_tbl ($pk_col, ts) "
17- . "VALUES ('$pk_val', NOW())";
18+ . "VALUES ('$pk_val', UTC_TIMESTAMP())";
19 PTDEBUG && _d($sql);
20 $dbh->do($sql);
21 }
22@@ -4920,7 +4920,8 @@
23 }
24 }
25
26- $sth->execute(ts(time), @vals);
27+ my ($now) = $dbh->selectrow_array(q{SELECT UTC_TIMESTAMP()});
28+ $sth->execute($now, @vals);
29 PTDEBUG && _d($sth->{Statement});
30 $sth->finish();
31
32@@ -4930,8 +4931,12 @@
33 else { # --monitor or --check
34 my $dbi_driver = lc $o->get('dbi-driver');
35
36+ # UNIX_TIMESTAMP(UTC_TIMESTAMP()) instead of UNIX_TIMESTAMP() alone,
37+ # so we make sure that we aren't being fooled by a timezone.
38+ # UNIX_TIMESTAMP(ts) replaces unix_timestamp($ts) -- MySQL is the
39+ # authority here, so let it calculate everything.
40 $heartbeat_sql
41- = "SELECT ts"
42+ = "SELECT UNIX_TIMESTAMP(UTC_TIMESTAMP()), UNIX_TIMESTAMP(ts)"
43 . ($dbi_driver eq 'mysql' ? '/*!50038, @@hostname AS host*/' : '')
44 . ($id ? "" : ", server_id")
45 . " FROM $db_tbl "
46@@ -4945,13 +4950,12 @@
47 my ($sth) = @_;
48 $sth->execute();
49 PTDEBUG && _d($sth->{Statement});
50- my ($ts, $hostname, $server_id) = $sth->fetchrow_array();
51- my $now = time;
52+ my ($now, $ts, $hostname, $server_id) = $sth->fetchrow_array();
53 PTDEBUG && _d("Heartbeat from server", $server_id, "\n",
54- " now:", ts($now), "\n",
55+ " now:", $now, "\n",
56 " ts:", $ts, "\n",
57 "skew:", $skew);
58- my $delay = $now - unix_timestamp($ts) - $skew;
59+ my $delay = $now - $ts - $skew;
60 PTDEBUG && _d('Delay', sprintf('%.6f', $delay), 'on', $hostname);
61
62 # Because we adjust for skew, if the ts are less than skew seconds
63@@ -5446,7 +5450,7 @@
64 The heartbeat table requires at least one row. If you manually create the
65 heartbeat table, then you must insert a row by doing:
66
67- INSERT INTO heartbeat (ts, server_id) VALUES (NOW(), N);
68+ INSERT INTO heartbeat (ts, server_id) VALUES (UTC_TIMESTAMP(), N);
69
70 where C<N> is the server's ID; do not use @@server_id because it will replicate
71 and slaves will insert their own server ID instead of the master's server ID.
72@@ -5464,7 +5468,7 @@
73 of a multi-slave hierarchy like "master -> slave1 -> slave2".
74 To manually insert the one required row into a legacy table:
75
76- INSERT INTO heartbeat (id, ts) VALUES (1, NOW());
77+ INSERT INTO heartbeat (id, ts) VALUES (1, UTC_TIMESTAMP());
78
79 The tool automatically detects if the heartbeat table is legacy.
80
81
82=== added file 't/pt-heartbeat/bugs.t'
83--- t/pt-heartbeat/bugs.t 1970-01-01 00:00:00 +0000
84+++ t/pt-heartbeat/bugs.t 2012-12-13 16:24:27 +0000
85@@ -0,0 +1,113 @@
86+#!/usr/bin/env perl
87+
88+BEGIN {
89+ die "The PERCONA_TOOLKIT_BRANCH environment variable is not set.\n"
90+ unless $ENV{PERCONA_TOOLKIT_BRANCH} && -d $ENV{PERCONA_TOOLKIT_BRANCH};
91+ unshift @INC, "$ENV{PERCONA_TOOLKIT_BRANCH}/lib";
92+};
93+
94+use strict;
95+use warnings FATAL => 'all';
96+use English qw(-no_match_vars);
97+use Test::More;
98+
99+use POSIX qw( tzset );
100+use File::Temp qw(tempfile);
101+
102+use PerconaTest;
103+use Sandbox;
104+require "$trunk/bin/pt-heartbeat";
105+
106+my $dp = new DSNParser(opts=>$dsn_opts);
107+my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp);
108+my $master_dbh = $sb->get_dbh_for('master');
109+my $slave1_dbh = $sb->get_dbh_for('slave1');
110+my $slave2_dbh = $sb->get_dbh_for('slave2');
111+
112+if ( !$master_dbh ) {
113+ plan skip_all => 'Cannot connect to sandbox master';
114+}
115+elsif ( !$slave1_dbh ) {
116+ plan skip_all => 'Cannot connect to sandbox slave1';
117+}
118+elsif ( !$slave2_dbh ) {
119+ plan skip_all => 'Cannot connect to sandbox slave2';
120+}
121+
122+unlink '/tmp/pt-heartbeat-sentinel';
123+$sb->create_dbs($master_dbh, ['test']);
124+$sb->wait_for_slaves();
125+
126+my $output;
127+my $base_pidfile = (tempfile("/tmp/pt-heartbeat-test.XXXXXXXX", OPEN => 0, UNLINK => 0))[1];
128+my $master_port = $sb->port_for('master');
129+
130+my @exec_pids;
131+my @pidfiles;
132+
133+sub start_update_instance {
134+ my ($port) = @_;
135+ my $pidfile = "$base_pidfile.$port.pid";
136+ push @pidfiles, $pidfile;
137+
138+ my $pid = fork();
139+ if ( $pid == 0 ) {
140+ my $cmd = "$trunk/bin/pt-heartbeat";
141+ exec { $cmd } $cmd, qw(-h 127.0.0.1 -u msandbox -p msandbox -P), $port,
142+ qw(--database test --table heartbeat --create-table),
143+ qw(--update --interval 0.5 --pid), $pidfile;
144+ exit 1;
145+ }
146+ push @exec_pids, $pid;
147+
148+ PerconaTest::wait_for_files($pidfile);
149+ ok(
150+ -f $pidfile,
151+ "--update on $port started"
152+ );
153+}
154+
155+sub stop_all_instances {
156+ my @pids = @exec_pids, map { chomp; $_ } map { slurp_file($_) } @pidfiles;
157+ diag(`$trunk/bin/pt-heartbeat --stop >/dev/null`);
158+
159+ waitpid($_, 0) for @pids;
160+ PerconaTest::wait_until(sub{ !-e $_ }) for @pidfiles;
161+
162+ unlink '/tmp/pt-heartbeat-sentinel';
163+}
164+
165+# ############################################################################
166+# pt-heartbeat handles timezones inconsistently
167+# https://bugs.launchpad.net/percona-toolkit/+bug/886059
168+# ############################################################################
169+
170+start_update_instance( $master_port );
171+
172+my $slave1_dsn = $sb->dsn_for('slave1');
173+# Using full_output here to work around a Perl bug: Only the first explicit
174+# tzset works.
175+($output) = full_output(sub {
176+ local $ENV{TZ} = '-09:00';
177+ tzset();
178+ pt_heartbeat::main($slave1_dsn, qw(--database test --table heartbeat),
179+ qw(--check --master-server-id), $master_port)
180+});
181+
182+# If the servers use UTC then the lag should be 0.00, or at least
183+# no greater than 9.99 for slow test boxes. When this fails, the
184+# lag is like 25200.00 becaues the servers are hours off.
185+like(
186+ $output,
187+ qr/\A\d.\d{2}$/,
188+ "Bug 886059: pt-heartbeat doesn't get confused with differing timezones"
189+);
190+
191+stop_all_instances();
192+
193+# ############################################################################
194+# Done.
195+# ############################################################################
196+$sb->wipe_clean($master_dbh);
197+ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox");
198+done_testing;

Subscribers

People subscribed via source and target branches