Hi, Luis.
A agree with your notes from review but have just one comment.
For semi-sync like for some other specific things we need to have an
ability either use own (test) my.cnf for starting server or set command
line options. For this patch we need set system variable --plugin_dir
and point to the proper location.
Could we do that in nuts or not?
Luís Soares пишет:
> Hi Serge,
>
> Good work. Please find my review comments below.
>
> STATUS
> ------
>
> Not approved.
>
> REQUIRED CHANGES
> ----------------
>
> RC1. I could not execute the test case:
>
> $ perl bin/nuts.pl --verbose --build ../mysql-5.1-rep+2/ --test
> semi_sync
> [12:11:19] rep::semi_sync ..
> 1..0
> skipped: (no reason given)
> [12:12:13]
> Files=1, Tests=0, 54 wallclock secs ( 0.31 usr 0.17 sys + 0.79 cusr
> 0.54 csys = 1.81 CPU)
> Result: NOTESTS
>
> I think this is because the check for semisync
> availability may be wrong. Shouldn't one:
>
> 1. Load the master plugin
> 2. Load the slave plugin
> 3. Check that the plugins were installed
> successfully. Maybe, check results for:
>
> select * from mysql.plugin where name like '%semi_sync_master';
> select * from mysql.plugin where name like '%
> semi_sync_slave';
>
> It's probably safe to load both plugins on every host and
> just activate them when needed (as it is today in the:
> change_rpl_mode function).
>
> RC2. I think one needs to sync master and slave for both async
> and semisync replication. If one does not sync in the case
> of semisync replication, there can be the case that some
> events were not replayed by the SQL thread.
>
> If it was fully synchronous replication (meaning that the
> slave would only reply to the client once the transaction
> was replayed in all (a majority, or just one slave -
> depends on the resilience wanted), then we would not need
> to sync the slave(s).
>
> So the loop below the comment:
> # Sync master and slave for async replication
>
> needs to sync all slaves, not just the async ones.
>
> REQUESTS
> --------
>
> R1. "attach" is starting the slave implicitely... I wonder if
> it should do it or not. We do have the "start_replication"
> function.
>
> In this test attach starts the slave to immediately after
> stop it, reconfigure semisync and start it again. Perhaps
> we should consider removing the:
>
> return ( $slave->execute ("START SLAVE") );
>
> from attach function.
>
> SUGGESTIONS
> -----------
>
> S1. Perhaps I would consider making the function
> change_rpl_mode as part of the replication library. This
> function could:
>
> 1. Check whether semisync is active.
> 2. stop replication.
> 3. wait for replication to stop.
> 4. Set the correct active profile (master/slave).
> 5. start replication.
> 6. wait for replication to start.
>
> We could name it:
>
> enable_semi_sync or ok_enable_semisync.
>
> S2. As part of the replication library, why not also make the
> functions (this is just a suggestion):
>
> 1. is_plugin_available_semi_sync_master
> 2. install_plugin_semi_sync_master
> 3. install_plugin_semi_sync_slave
>
> Each of these functions would act as a facade for a two
> functions:
>
> 1. is_plugin_available(PLUGIN_ID)
> 2. install_plugin(PLUGIN_ID)
>
> So that, one could have something like:
>
> sub install_plugin_semi_sync_master()
> {
> return install_plugin(PLUGIN_SEMI_SYNC_MASTER_ID);
> }
>
> These are just suggestions, but something done in this context
> would be great (I think). But Alfranio should have a saying in
> this as well.
>
> DETAILS
> -------
>
> Please find some minor comments inline.
>
> On Tue, 2009-09-29 at 01:48 +0400, Serge Kozlov wrote:
>> #At file:///home/ksm/sun/repo/WL5038/nuts-commit/ based on
> revid:serge.kozlov@stripped
>>
>> 356 Serge Kozlov 2009-09-29
>> WL#5038 Test case
>> added:
>> suites/rep/semi_sync.pm
>> modified:
>> lib/My/Nuts/Library/Kernel/ServerResult.pm
>>
>> === modified file 'lib/My/Nuts/Library/Kernel/ServerResult.pm'
>> --- a/lib/My/Nuts/Library/Kernel/ServerResult.pm 2009-09-25 11:24:12 +0000
>> +++ b/lib/My/Nuts/Library/Kernel/ServerResult.pm 2009-09-28 21:48:49 +0000
>> @@ -2,7 +2,7 @@ package My::Nuts::Library::Kernel::Serve
>> use Exporter;
>> our @ISA = qw(Exporter);
>> our @EXPORT =
>> - qw(compare_dbs ok_compare_dbs get_supported_engines get_table_list);
>> + qw(compare_dbs ok_compare_dbs get_supported_engines get_table_list
> get_variable);
>> use strict;
>> use warnings;
>> use My;
>> @@ -139,6 +139,21 @@ sub get_table_list
>> return $tables;
>> }
>>
>> +sub get_variable
>> +{
>> + my $params = shift;
>> + my $conn = $params->{"conn"};
>> + my $var = $params->{"var"};
>> + my $result = undef;
>> + my $rs = sql ($conn, "SHOW VARIABLES LIKE '" . $var . "';");
>> + my @rs_data = get_next($rs);
>> + if (defined $rs_data[0])
>> + {
>> + $result = $rs_data[1];
>> + }
>> + return $result;
>> +}
>
> Good. Nice that you've done it.
>
>> 1;
>>
>> __END__;
>>
>> === added file 'suites/rep/semi_sync.pm'
>> --- a/suites/rep/semi_sync.pm 1970-01-01 00:00:00 +0000
>> +++ b/suites/rep/semi_sync.pm 2009-09-28 21:48:49 +0000
>> @@ -0,0 +1,171 @@
>> +package rep::semi_sync;
>> +use Exporter;
>> +our @ISA = qw(Exporter My::Nuts::Library::Tests::SimpleTest);
>> +use strict;
>> +use warnings;
>> +use My;
>> +use My::Nuts::Library::Kernel::Server;
>> +use My::Nuts::Library::Kernel::ServerResult;
>> +use My::Nuts::Library::Kernel::Manager;
>> +use My::Nuts::Library::Kernel::Result;
>> +use My::Nuts::Library::Tests::SimpleTest;
>> +use My::Nuts::Library::Kernel::Replication;
>> +use Test::More;
>> +use DataSource;
>> +
>> +sub prepare
>> +{
>> + return;
>> +}
>> +
>> +sub startup
>> +{
>> + return;
>> +}
>> +
>> +sub fire
>> +{
>> + my ($test) = @_;
>> + # Create 3 servers
>> + my $server1 = ok_server ($test);
>> + my $server2 = ok_server ($test);
>> + my $server3 = ok_server ($test);
>> + # Move server to hash
>> + my $servers = {
>> + "A" => $server1,
>> + "B" => $server2,
>> + "C" => $server3
>> + };
>> + # List of test schemas
>> + my $schemas = [
>> + [
>> + {"topology" => "A->B"},
>> + {"A->B" => "async"},
>> + {"A->B" => "semisync"}
>> + ],
>> + [
>> + {"topology" => "A->B,B->C"},
>> + {"A->B" => "semisync", "B->C" => "semisync"},
>> + {"A->B" => "semisync", "B->C" => "async"}
>> + ],
>> + [
>> + {"topology" => "A->B,A->C"},
>> + {"A->B" => "semisync", "A->C" => "semisync"},
>> + {"A->B" => "semisync", "A->C" => "async"}
>> + ]
>> + ];
>> + # Init stuff
>> + my $dbname = "test";
>> + my $init_queries =[
>> + "DROP DATABASE IF EXISTS " . $dbname,
>> + "CREATE DATABASE " . $dbname,
>> + "USE " . $dbname
>> + ];
>> +
>> + # Check semisync support
>> + my $semisync_enabled = 1;
>> + foreach my $server (values %$servers)
>> + {
>> + $semisync_enabled = 0 if (!defined(get_variable ( {"conn" => $server, "var"
> => "rpl_semi_sync_master_enabled"})));
>
> Before this, we need the plugin to be loaded. Do we load
> it elsewhere? I cannot find the INSTALL PLUGIN clause and the
> test run I did would not succeed, bailing out with skipped...
>
>> + }
>> + if ($semisync_enabled == 1)
>> + {
>> + # Get statements from data source and add creating db
>> + my $data = get_data_from_source();
>> + unshift(@$data, @$init_queries);
>> +
>> + # Plan
>> + plan tests => scalar(@$schemas) * scalar(@$data) + 53;
>> +
>> + # Main loop
>> + foreach my $schema (@$schemas)
>> + {
>> + # Create pairs master->slave and store them into hash
>> + #print $schema->[0]->{"topology"} . "\n";
>> + my @atomic = split(",", $schema->[0]->{"topology"});
>> + my $cur_servers = {};
>> + foreach my $pair (@atomic)
>> + {
>> + $pair =~ m/(.+)\-\>(.+)/i;
>> + $cur_servers->{$pair}->{"master"} = $servers->{$1};
>> + $cur_servers->{$pair}->{"slave"} = $servers->{$2};
>> + attach($cur_servers->{$pair}->{"master"},
> $cur_servers->{$pair}->{"slave"});
>
> I don't recall why attach does an implicit start slave,
> but maybe it should not... What do you think? Alfranio?
>
>> + ok_wait_start_replication($cur_servers->{$pair}->{"slave"});
>> + }
>> + my $num = scalar(@$schema) - 1;
>> + my $block_start = 0;
>> + my $block_end = 0;
>> + for (my $i = 1; $i <= $num; $i++)
>> + {
>> + # Prepare replication
>> + foreach my $detail (keys %{$schema->[$i]})
>> + {
>> + change_rpl_mode($cur_servers->{$detail}->{"master"},
> $cur_servers->{$detail}->{"slave"}, $schema->[$i]->{$detail})
>> + }
>> + # Run bulk of statements on master
>> + $block_start = $block_end;
>> + $block_end = int (($i/$num) * scalar(@$data));
>> + for (my $j = $block_start; $j < $block_end; $j++)
>> + {
>> + ok_sql($servers->{"A"}, $data->[$j]);
>> + }
>> + # Sync master and slave for async replication
>> + foreach my $detail (keys %{$schema->[$i]})
>> + {
>> + if ($schema->[$i]->{$detail} =~ m/async/i)
>> + {
>> + ok_synchronize($cur_servers->{$detail}->{"master"},
> $cur_servers->{$detail}->{"slave"});
>> + }
>
> I think one needs to synchronize both async and semisync
> master->slave(s).
>
>> + }
>> + }
>> + # Check DBs
>> + foreach my $pair (keys %{$cur_servers})
>> + {
>> + ok_compare_dbs([
>> + {"conn" => $cur_servers->{$pair}->{"master"}, "dbname" =>
> $dbname}
>> + ]);
>> + }
>> + # Stop replication links and clean up servers
>> + foreach my $pair (keys %{$cur_servers})
>> + {
>> + stop_replication($cur_servers->{$pair}->{"slave"});
>> + reset_master($cur_servers->{$pair}->{"master"});
>> + reset_slave($cur_servers->{$pair}->{"slave"});
>> + }
>> + }
>> + }
>> +}
>> +
>> +sub shutdown
>> +{
>> + return;
>> +}
>> +
>> +sub change_rpl_mode
>> +{
>> + my ($master, $slave, $mode) = @_;
>> + my $modes = {
>> + "async" => 0,
>> + "semisync" => 1
>> + };
>> + sql($master, "SET GLOBAL rpl_semi_sync_master_enabled = " .
> $modes->{$mode});
>> + sql($slave, "SET GLOBAL rpl_semi_sync_slave_enabled = " .
> $modes->{$mode});
>> + ok_stop_replication($slave);
>> + ok_wait_stop_replication($slave);
>> + ok_start_replication($slave);
>> + ok_wait_start_replication($slave);
>> +}
>> +
>> +1;
>> +__END__;
>> +
>> +=head1 NAME
>> +
>> +rep::semi_sync - Semisynchronous replication
>> +
>> +=head1 SYNOPSIS
>> +
>> +Semisynchronous replication for different topologies (A->B, A->B->C,
> B<-A->C) and switching
>> +to asynchronous and back.
>> +
>> +=back
>>
>>
--
Serge Kozlov, QA Developer
MySQL AB, Moscow, Russia, www.mysql.com
Office:
Are you MySQL certified? www.mysql.com/certification
