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

Proposed by Daniel Nichter on 2013-01-22
Status: Merged
Merged at revision: 522
Proposed branch: lp:~percona-toolkit-dev/percona-toolkit/fix-pt-heartbeat-2.1.8
Merge into: lp:percona-toolkit/2.1
Diff against target: 133 lines (+46/-10)
2 files modified
bin/pt-heartbeat (+20/-8)
t/pt-heartbeat/bugs.t (+26/-2)
To merge this branch: bzr merge lp:~percona-toolkit-dev/percona-toolkit/fix-pt-heartbeat-2.1.8
Reviewer Review Type Date Requested Status
Brian Fraser (community) 2013-01-22 Approve on 2013-01-23
Daniel Nichter Approve on 2013-01-22
Review via email: mp+144416@code.launchpad.net
To post a comment you must log in.
review: Approve
Daniel Nichter (daniel-nichter) wrote :

I tested manually with this version vs. 2.1.7, each one monitoring and/or updating. It seems to work, especially given that I tried this version with --utc (i.e. make it like 2.1.8) then monitor with 2.1.7 and the same huge diff was created, demonstrating/proving that 2.1.9 will be backwards-compat with 2.1.7 as long as --utc is not used. The --utc docs state this.

Brian Fraser (fraserbn) wrote :

What happens if I mix instances of pthb, some that use --utc and some that don't? Maybe add a test for that?

Daniel Nichter (daniel-nichter) wrote :

Mixing will cause the same differences because --utc is just like using one instance on a box using UTC. So if other instances are not using --utc and are using other time zones, the same differences arise. I don't think it's worth testing this because it's always been a limitation of the tool: you have to use the same tz. --utc just makes it easy to do that now: run all instances with --utc.

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 2013-01-03 00:54:18 +0000
3+++ bin/pt-heartbeat 2013-01-22 23:53:25 +0000
4@@ -4807,6 +4807,8 @@
5 # ########################################################################
6 # Create the heartbeat table if --create-table was given.
7 # ########################################################################
8+ my $utc = $o->get('utc');
9+ my $now_func = $utc ? 'UTC_TIMESTAMP()' : 'NOW()';
10 my $db_tbl = $q->quote($db, $tbl);
11 my $server_id = $dbh->selectrow_array('SELECT @@server_id');
12 if ( $o->get('create-table') ) {
13@@ -4816,7 +4818,7 @@
14 $dbh->do($sql);
15
16 $sql = ($o->get('replace') ? "REPLACE" : "INSERT")
17- . qq/ INTO $db_tbl (ts, server_id) VALUES (UTC_TIMESTAMP(), $server_id)/;
18+ . qq/ INTO $db_tbl (ts, server_id) VALUES ($now_func, $server_id)/;
19 PTDEBUG && _d($sql);
20 # This may fail if the table already existed and already had this row.
21 # We eval to ignore this possibility.
22@@ -4913,7 +4915,7 @@
23 PTDEBUG && _d('No heartbeat row in table');
24 if ( $o->get('insert-heartbeat-row') ) {
25 my $sql = "INSERT INTO $db_tbl ($pk_col, ts) "
26- . "VALUES ('$pk_val', UTC_TIMESTAMP())";
27+ . "VALUES ('$pk_val', $now_func)";
28 PTDEBUG && _d($sql);
29 $dbh->do($sql);
30 }
31@@ -5007,8 +5009,7 @@
32 tries => 3,
33 wait => sub { sleep 0.25; return; },
34 try => sub {
35- my ($now) = $dbh->selectrow_array(q{SELECT UTC_TIMESTAMP()});
36- $sth->execute($now, @vals);
37+ $sth->execute(ts(time, $utc), @vals);
38 PTDEBUG && _d($sth->{Statement});
39 $sth->finish();
40 },
41@@ -5039,7 +5040,7 @@
42 # UNIX_TIMESTAMP(ts) replaces unix_timestamp($ts) -- MySQL is the
43 # authority here, so let it calculate everything.
44 $heartbeat_sql
45- = "SELECT UNIX_TIMESTAMP(UTC_TIMESTAMP()), UNIX_TIMESTAMP(ts)"
46+ = "SELECT " . ($utc ? 'UNIX_TIMESTAMP(ts)' : 'ts')
47 . ($dbi_driver eq 'mysql' ? '/*!50038, @@hostname AS host*/' : '')
48 . ($id ? "" : ", server_id")
49 . " FROM $db_tbl "
50@@ -5053,12 +5054,13 @@
51 my ($sth) = @_;
52 $sth->execute();
53 PTDEBUG && _d($sth->{Statement});
54- my ($now, $ts, $hostname, $server_id) = $sth->fetchrow_array();
55+ my ($ts, $hostname, $server_id) = $sth->fetchrow_array();
56+ my $now = time;
57 PTDEBUG && _d("Heartbeat from server", $server_id, "\n",
58- " now:", $now, "\n",
59+ " now:", ts($now, $utc), "\n",
60 " ts:", $ts, "\n",
61 "skew:", $skew);
62- my $delay = $now - $ts - $skew;
63+ my $delay = $now - unix_timestamp($ts, $utc) - $skew;
64 PTDEBUG && _d('Delay', sprintf('%.6f', $delay), 'on', $hostname);
65
66 # Because we adjust for skew, if the ts are less than skew seconds
67@@ -5863,6 +5865,16 @@
68
69 User for login if not current user.
70
71+=item --utc
72+
73+Ignore system time zones and use only UTC. By default pt-heartbeat does
74+not check or adjust for different system or MySQL time zones which can
75+cause the tool to compute the lag incorrectly. Specifying this option is
76+a good idea because it ensures that the tool works correctly regardless of
77+time zones, but it also makes the tool backwards-incompatible with
78+pt-heartbeat 2.1.7 and older (unless the older version of pt-heartbeat
79+is running on a system that uses UTC).
80+
81 =item --version
82
83 Show version and exit.
84
85=== modified file 't/pt-heartbeat/bugs.t'
86--- t/pt-heartbeat/bugs.t 2012-12-19 16:53:13 +0000
87+++ t/pt-heartbeat/bugs.t 2013-01-22 23:53:25 +0000
88@@ -93,7 +93,7 @@
89 local $ENV{TZ} = '-09:00';
90 tzset();
91 pt_heartbeat::main($slave1_dsn, qw(--database test --table heartbeat),
92- qw(--check --master-server-id), $master_port)
93+ qw(--utc --check --master-server-id), $master_port)
94 });
95
96 # If the servers use UTC then the lag should be 0.00, or at least
97@@ -102,11 +102,35 @@
98 like(
99 $output,
100 qr/\A\d.\d{2}$/,
101- "Bug 886059: pt-heartbeat doesn't get confused with differing timezones"
102+ "--utc bypasses time zone differences (bug 886059, bug 1099665)"
103 );
104
105 stop_all_instances();
106
107+# #############################################################################
108+# pt-heartbeat 2.1.8 doesn't use precision/sub-second timestamps
109+# https://bugs.launchpad.net/percona-toolkit/+bug/1103221
110+# #############################################################################
111+
112+$master_dbh->do('truncate table test.heartbeat');
113+$sb->wait_for_slaves;
114+
115+my $master_dsn = $sb->dsn_for('master');
116+
117+($output) = output(
118+ sub {
119+ pt_heartbeat::main($master_dsn, qw(--database test --update),
120+ qw(--run-time 1))
121+ },
122+);
123+
124+my ($row) = $master_dbh->selectrow_hashref('select * from test.heartbeat');
125+like(
126+ $row->{ts},
127+ qr/\d{4}-\d\d-\d\dT\d+:\d+:\d+\.\d+/,
128+ "Hi-res timestamp (bug 1103221)"
129+);
130+
131 # ############################################################################
132 # Done.
133 # ############################################################################

Subscribers

People subscribed via source and target branches