From 5f2cdad29973df310ddf4eee1254584b8c458761 Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Tue, 13 Sep 2011 09:27:59 -0600 Subject: [PATCH] Implement MasterSlave::get_slaves() to get cxns from a DSN table. Add comments explaining use_repl_db(). --- bin/pt-table-checksum | 56 ++++++++++--- lib/MasterSlave.pm | 89 +++++++++++++++++++++ t/lib/MasterSlave.t | 102 +++++++++++++++++++++++- t/lib/samples/MasterSlave/dsn_table.sql | 12 +++ 4 files changed, 245 insertions(+), 14 deletions(-) create mode 100644 t/lib/samples/MasterSlave/dsn_table.sql diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index e0045826..0fce1494 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -5265,14 +5265,47 @@ sub check_repl_table { return; } -# This sub should be called before any work is done with the -# --replicate table. It will USE the correct replicate db. -# If there's a tbl arg then its db will be used unless --replicate-database -# was specified. A tbl arg means we're checksumming that table, -# so we've been called from do_tbl_replicate(). Other callers -# won't pass a tbl arg because they're just doing something to -# the --replicate table. -# See http://code.google.com/p/maatkit/issues/detail?id=982 + +# Sub: use_repl_db +# USE the correct database for the --replicate table. +# This sub must be called before any work is done with the --replicatte +# table because replication filters can really complicate replicating the +# checksums. The originally issue is, +# http://code.google.com/p/maatkit/issues/detail?id=982, +# but here's what you need to know: +# - If there is no active DB, then if there's any do-db or ignore-db +# settings, the checksums will get filtered out of replication. So we +# have to have some DB be the current one. +# - Other places in the code may change the DB and we might not know it. +# Opportunity for bugs. The SHOW CREATE TABLE, for example. In the +# end, a bunch of USE statements isn't a big deal, it just looks noisy +# when you analyze the logs this tool creates. But it's better to just +# have them even if they're no-op. +# - We need to always let the user specify, because there are so many +# possibilities that the tool can't guess the right thing in all of +# them. +# - The right default behavior, which the user can override, is: +# * When running queries on the --replicate table itself, such as +# emptying it, USE that table's database. +# * When running checksum queries, USE the database of the table that's +# being checksummed. +# * When the user specifies --replicate-database, in contrast, always +# USE that database. +# - This behavior is the best compromise by default, because users who +# explicitly replicate some databases and filter out others will be +# very likely to run pt-table-checksum and limit its checksumming to +# only the databases that are replicated. I've seen people do this, +# including Peter. In this case, the tool will work okay even without +# an explicit --replicate-database setting. +# +# Required Arguments: +# dbh - dbh +# repl_table - Full quoted --replicate table name +# OptionParser - +# Quoter - +# +# Returns: +# Nothing or dies on error { my $current_db; @@ -5286,8 +5319,11 @@ sub use_repl_db { my ($db, $tbl) = $q->split_unquote($repl_table); if ( my $tbl = $args{tbl} ) { - # Caller is checksumming this table, USE its db unless - # --replicate-database is in effect. + # If there's a tbl arg then its db will be used unless + # --replicate-database was specified. A tbl arg means + # we're checksumming that table. Other callers won't + # pass a tbl arg when they're just doing something to + # the --replicate table. $db = $o->get('replicate-database') ? $o->get('replicate-database') : $tbl->{db}; } diff --git a/lib/MasterSlave.pm b/lib/MasterSlave.pm index 5fa195f3..22a0d91f 100644 --- a/lib/MasterSlave.pm +++ b/lib/MasterSlave.pm @@ -36,6 +36,56 @@ sub new { return bless $self, $class; } +sub get_slaves { + my ($self, %args) = @_; + my @required_args = qw(OptionParser DSNParser Quoter); + foreach my $arg ( @required_args ) { + die "I need a $arg argument" unless $args{$arg}; + } + my ($o, $dp) = @args{@required_args}; + + my $slaves = []; + my $method = $o->get('recursion-method'); + MKDEBUG && _d('Slave recursion method:', $method); + if ( !$method || $method =~ m/proocesslist|hosts/i ) { + my @required_args = qw(dbh dsn); + foreach my $arg ( @required_args ) { + die "I need a $arg argument" unless $args{$arg}; + } + my ($dbh, $dsn) = @args{@required_args}; + $self->recurse_to_slaves( + { dbh => $dbh, + dsn => $dsn, + dsn_parser => $dp, + recurse => $o->get('recurse'), + method => $o->get('recursion-method'), + callback => sub { + my ( $dsn, $dbh, $level, $parent ) = @_; + return unless $level; + MKDEBUG && _d('Found slave:', $dp->as_string($dsn)); + $dbh->{InactiveDestroy} = 1; # Prevent destroying on fork. + $dbh->{FetchHashKeyName} = 'NAME_lc'; + push @$slaves, { dsn=>$dsn, dbh=>$dbh }; + return; + }, + } + ); + } + elsif ( $method =~ m/^dsn=/i ) { + my ($dsn_table_dsn) = $method =~ m/^dsn=(.+)/i; + $slaves = $self->get_cxn_from_dsn_table( + %args, + dsn_table_dsn => $dsn_table_dsn, + ); + } + else { + die "Invalid --recusion-method: $method. Valid values are: " + . "dsn=DSN, hosts, or processlist.\n"; + } + + return $slaves; +} + # Sub: recurse_to_slaves # Descend to slaves by examining SHOW SLAVE HOSTS. # The callback gets the slave's DSN, dbh, parent, and the recursion level @@ -797,6 +847,45 @@ sub reset_known_replication_threads { return; } +sub get_cxn_from_dsn_table { + my ($self, %args) = @_; + my @required_args = qw(dsn_table_dsn DSNParser Quoter); + foreach my $arg ( @required_args ) { + die "I need a $arg argument" unless $args{$arg}; + } + my ($dsn_table_dsn, $dp, $q) = @args{@required_args}; + MKDEBUG && _d('DSN table DSN:', $dsn_table_dsn); + + my $dsn = $dp->parse($dsn_table_dsn); + my $dsn_table; + if ( $dsn->{D} && $dsn->{t} ) { + $dsn_table = $q->quote($dsn->{D}, $dsn->{t}); + } + elsif ( $dsn->{t} && $dsn->{t} =~ m/\./ ) { + $dsn_table = $q->quote($q->split_unquote($dsn->{t})); + } + else { + die "DSN table DSN does not specify a database (D) " + . "or a database-qualified table (t)"; + } + + my @cxn; + my $dbh = $dp->get_dbh($dp->get_cxn_params($dsn)); + my $sql = "SELECT dsn FROM $dsn_table ORDER BY id"; + MKDEBUG && _d($sql); + my $dsns = $dbh->selectcol_arrayref($sql); + if ( $dsns ) { + foreach my $dsn ( @$dsns ) { + MKDEBUG && _d('DSN from DSN table:', $dsn); + my $dsn = $dp->parse($dsn); + my $dbh = $dp->get_dbh($dp->get_cxn_params($dsn)); + push @cxn, {dsn=>$dsn, dbh=>$dbh}; + } + } + $dbh->disconnect(); + return \@cxn; +} + sub _d { my ($package, undef, $line) = caller 0; @_ = map { (my $temp = $_) =~ s/\n/\n# /g; $temp; } diff --git a/t/lib/MasterSlave.t b/t/lib/MasterSlave.t index a29e845b..974f13a7 100644 --- a/t/lib/MasterSlave.t +++ b/t/lib/MasterSlave.t @@ -9,11 +9,13 @@ BEGIN { use strict; use warnings FATAL => 'all'; use English qw(-no_match_vars); -use Test::More tests => 42; +use Test::More tests => 46; use MasterSlave; use DSNParser; use VersionParser; +use OptionParser; +use Quoter; use Sandbox; use PerconaTest; @@ -22,10 +24,66 @@ my $ms = new MasterSlave(VersionParser => $vp); my $dp = new DSNParser(opts=>$dsn_opts); my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp); +my $master_dbh = $sb->get_dbh_for('master'); +my $slave_dbh = $sb->get_dbh_for('slave1'); + +# ############################################################################ +# get_slaves() wrapper around recurse_to_slaves() +# ############################################################################ +my $q = new Quoter; +my $o = new OptionParser(description => 'MasterSlave'); +$o->get_specs("$trunk/bin/pt-table-checksum"); + +SKIP: { + skip "Cannot connect to sandbox master", 2 unless $master_dbh; + @ARGV = (); + $o->get_opts(); + + my $slaves = $ms->get_slaves( + dbh => $master_dbh, + dsn => { + h => '127.1', + P => '12345', + u => 'msandbox', + p => 'msandbox', + }, + OptionParser => $o, + DSNParser => $dp, + Quoter => $q, + ); + + is_deeply( + $slaves->[0]->{dsn}, + { A => undef, + D => undef, + F => undef, + P => '12346', + S => undef, + h => '127.0.0.1', + p => 'msandbox', + t => undef, + u => 'msandbox', + server_id => 12346, + master_id => 12345, + source => 'hosts', + }, + 'get_slaves() from recurse_to_slaves()' + ); + + my ($id) = $slaves->[0]->{dbh}->selectrow_array('SELECT @@SERVER_ID'); + is( + $id, + '12346', + 'dbh created from get_slaves()' + ); + + $slaves->[0]->{dbh}->disconnect(); +} + # ############################################################################# # First we need to setup a special replication sandbox environment apart from # the usual persistent sandbox servers on ports 12345 and 12346. -# The tests in this script require a master with 3 slaves in a setup like: +# The tests in this script require a master with 3 slaves in a setup like:ggn # 127.0.0.1:master # +- 127.0.0.1:slave0 # | +- 127.0.0.1:slave1 @@ -400,8 +458,6 @@ ok( # ############################################################################# # get_replication_filters() # ############################################################################# -my $master_dbh = $sb->get_dbh_for('master'); -my $slave_dbh = $sb->get_dbh_for('slave1'); SKIP: { skip "Cannot connect to sandbox master", 3 unless $master_dbh; skip "Cannot connect to sandbox slave", 3 unless $slave_dbh; @@ -463,6 +519,44 @@ ok( "get_slave_lag() for slave" ); +# ############################################################################ +# get_slaves() and DSN table +# ############################################################################ +$sb->load_file('master', "t/lib/samples/MasterSlave/dsn_table.sql"); + +@ARGV = ('--recursion-method', 'dsn=F=/tmp/12345/my.sandbox.cnf,D=dsn_t,t=dsns'); +$o->get_opts(); + +my $slaves = $ms->get_slaves( + OptionParser => $o, + DSNParser => $dp, + Quoter => $q, +); + +is_deeply( + $slaves->[0]->{dsn}, + { A => undef, + D => undef, + F => undef, + P => '12346', + S => undef, + h => '127.1', + p => 'msandbox', + t => undef, + u => 'msandbox' + }, + 'get_slaves() from DSN table' +); + +my ($id) = $slaves->[0]->{dbh}->selectrow_array('SELECT @@SERVER_ID'); +is( + $id, + '12346', + 'dbh created from DSN table works' +); + +$slaves->[0]->{dbh}->disconnect(); + # ############################################################################# # Done. # ############################################################################# diff --git a/t/lib/samples/MasterSlave/dsn_table.sql b/t/lib/samples/MasterSlave/dsn_table.sql new file mode 100644 index 00000000..56ad9fd0 --- /dev/null +++ b/t/lib/samples/MasterSlave/dsn_table.sql @@ -0,0 +1,12 @@ +DROP DATABASE IF EXISTS dsn_t; +CREATE DATABASE dsn_t; +USE dsn_t; + +CREATE TABLE dsns ( + id int auto_increment primary key, + parent_id int default null, + dsn varchar(255) not null +); + +INSERT INTO dsns VALUES + (null, null, 'h=127.1,P=12346,u=msandbox,p=msandbox');