From f3ad4c7d56368be94417ddbb15dccbce6b59bed6 Mon Sep 17 00:00:00 2001 From: Brian Fraser Date: Thu, 13 Dec 2012 00:05:06 -0300 Subject: [PATCH] Fix for 886059: pt-heartbeat handles timezones inconsistently --- bin/pt-heartbeat | 24 ++++++---- t/pt-heartbeat/bugs.t | 108 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 10 deletions(-) create mode 100644 t/pt-heartbeat/bugs.t diff --git a/bin/pt-heartbeat b/bin/pt-heartbeat index fa6da080..a5eed8f3 100755 --- a/bin/pt-heartbeat +++ b/bin/pt-heartbeat @@ -4734,7 +4734,7 @@ sub main { $dbh->do($sql); $sql = ($o->get('replace') ? "REPLACE" : "INSERT") - . qq/ INTO $db_tbl (ts, server_id) VALUES (NOW(), $server_id)/; + . qq/ INTO $db_tbl (ts, server_id) VALUES (UTC_TIMESTAMP(), $server_id)/; PTDEBUG && _d($sql); # This may fail if the table already existed and already had this row. # We eval to ignore this possibility. @@ -4831,7 +4831,7 @@ sub main { PTDEBUG && _d('No heartbeat row in table'); if ( $o->get('insert-heartbeat-row') ) { my $sql = "INSERT INTO $db_tbl ($pk_col, ts) " - . "VALUES ('$pk_val', NOW())"; + . "VALUES ('$pk_val', UTC_TIMESTAMP())"; PTDEBUG && _d($sql); $dbh->do($sql); } @@ -4920,7 +4920,8 @@ sub main { } } - $sth->execute(ts(time), @vals); + my ($now) = $dbh->selectrow_array(q{SELECT UTC_TIMESTAMP()}); + $sth->execute($now, @vals); PTDEBUG && _d($sth->{Statement}); $sth->finish(); @@ -4930,8 +4931,12 @@ sub main { else { # --monitor or --check my $dbi_driver = lc $o->get('dbi-driver'); + # UNIX_TIMESTAMP(UTC_TIMESTAMP()) instead of UNIX_TIMESTAMP() alone, + # so we make sure that we aren't being fooled by a timezone. + # UNIX_TIMESTAMP(ts) replaces unix_timestamp($ts) -- MySQL is the + # authority here, so let it calculate everything. $heartbeat_sql - = "SELECT ts" + = "SELECT UNIX_TIMESTAMP(UTC_TIMESTAMP()), UNIX_TIMESTAMP(ts)" . ($dbi_driver eq 'mysql' ? '/*!50038, @@hostname AS host*/' : '') . ($id ? "" : ", server_id") . " FROM $db_tbl " @@ -4945,13 +4950,12 @@ sub main { my ($sth) = @_; $sth->execute(); PTDEBUG && _d($sth->{Statement}); - my ($ts, $hostname, $server_id) = $sth->fetchrow_array(); - my $now = time; + my ($now, $ts, $hostname, $server_id) = $sth->fetchrow_array(); PTDEBUG && _d("Heartbeat from server", $server_id, "\n", - " now:", ts($now), "\n", + " now:", $now, "\n", " ts:", $ts, "\n", "skew:", $skew); - my $delay = $now - unix_timestamp($ts) - $skew; + my $delay = $now - $ts - $skew; PTDEBUG && _d('Delay', sprintf('%.6f', $delay), 'on', $hostname); # Because we adjust for skew, if the ts are less than skew seconds @@ -5446,7 +5450,7 @@ be created with the following MAGIC_create_heartbeat table definition: The heartbeat table requires at least one row. If you manually create the heartbeat table, then you must insert a row by doing: - INSERT INTO heartbeat (ts, server_id) VALUES (NOW(), N); + INSERT INTO heartbeat (ts, server_id) VALUES (UTC_TIMESTAMP(), N); where C is the server's ID; do not use @@server_id because it will replicate and slaves will insert their own server ID instead of the master's server ID. @@ -5464,7 +5468,7 @@ Legacy tables do not support L<"--update"> instances on each slave of a multi-slave hierarchy like "master -> slave1 -> slave2". To manually insert the one required row into a legacy table: - INSERT INTO heartbeat (id, ts) VALUES (1, NOW()); + INSERT INTO heartbeat (id, ts) VALUES (1, UTC_TIMESTAMP()); The tool automatically detects if the heartbeat table is legacy. diff --git a/t/pt-heartbeat/bugs.t b/t/pt-heartbeat/bugs.t new file mode 100644 index 00000000..46dd5d2d --- /dev/null +++ b/t/pt-heartbeat/bugs.t @@ -0,0 +1,108 @@ +#!/usr/bin/env perl + +BEGIN { + die "The PERCONA_TOOLKIT_BRANCH environment variable is not set.\n" + unless $ENV{PERCONA_TOOLKIT_BRANCH} && -d $ENV{PERCONA_TOOLKIT_BRANCH}; + unshift @INC, "$ENV{PERCONA_TOOLKIT_BRANCH}/lib"; +}; + +use strict; +use warnings FATAL => 'all'; +use English qw(-no_match_vars); +use Test::More; + +use POSIX qw( tzset ); +use File::Temp qw(tempfile); + +use PerconaTest; +use Sandbox; +require "$trunk/bin/pt-heartbeat"; + +my $dp = new DSNParser(opts=>$dsn_opts); +my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp); +my $master_dbh = $sb->get_dbh_for('master'); +my $slave1_dbh = $sb->get_dbh_for('slave1'); +my $slave2_dbh = $sb->get_dbh_for('slave2'); + +if ( !$master_dbh ) { + plan skip_all => 'Cannot connect to sandbox master'; +} +elsif ( !$slave1_dbh ) { + plan skip_all => 'Cannot connect to sandbox slave1'; +} +elsif ( !$slave2_dbh ) { + plan skip_all => 'Cannot connect to sandbox slave2'; +} + +unlink '/tmp/pt-heartbeat-sentinel'; +$sb->create_dbs($master_dbh, ['test']); +$sb->wait_for_slaves(); + +my $output; +my $base_pidfile = (tempfile("/tmp/pt-heartbeat-test.XXXXXXXX", OPEN => 0, UNLINK => 0))[1]; +my $master_port = $sb->port_for('master'); + +my @exec_pids; +my @pidfiles; + +sub start_update_instance { + my ($port) = @_; + my $pidfile = "$base_pidfile.$port.pid"; + push @pidfiles, $pidfile; + + my $pid = fork(); + if ( $pid == 0 ) { + my $cmd = "$trunk/bin/pt-heartbeat"; + exec { $cmd } $cmd, qw(-h 127.0.0.1 -u msandbox -p msandbox -P), $port, + qw(--database test --table heartbeat --create-table), + qw(--update --interval 0.5 --pid), $pidfile; + exit 1; + } + push @exec_pids, $pid; + + PerconaTest::wait_for_files($pidfile); + ok( + -f $pidfile, + "--update on $port started" + ); +} + +sub stop_all_instances { + my @pids = @exec_pids, map { chomp; $_ } map { slurp_file($_) } @pidfiles; + diag(`$trunk/bin/pt-heartbeat --stop >/dev/null`); + + waitpid($_, 0) for @pids; + PerconaTest::wait_until(sub{ !-e $_ }) for @pidfiles; +} + +# ############################################################################ +# pt-heartbeat handles timezones inconsistently +# https://bugs.launchpad.net/percona-toolkit/+bug/886059 +# ############################################################################ + +start_update_instance( $master_port ); + +my $slave1_dsn = $sb->dsn_for('slave1'); +# Using full_output here to work around a Perl bug: Only the first explicit +# tzset works. +($output) = full_output(sub { + local $ENV{TZ} = '-09:00'; + tzset(); + pt_heartbeat::main($slave1_dsn, qw(--database test --table heartbeat), + qw(--check --master-server-id), $master_port) +}); + +like( + $output, + qr/\A\d.\d{2}$/, + "Bug 886059: pt-heartbeat doesn't get confused with differing timezones" +); + +stop_all_instances(); + +# ############################################################################ +# Done. +# ############################################################################ +$sb->wipe_clean($master_dbh); +ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); +done_testing;