Hi, Luis.
Thanks for review.
See comments in-line (some of them copy-pasted from reply to Alfranio's
review)
Luís Soares пишет:
> STATUS
> ------
>
> Not Approved (some minor changes are requested).
>
> REQUIRED CHANGES
> ----------------
>
> RC1. Please, add compare_dbs to the standard library, and maybe
> name it ok_compare_dbs. It is a very good candidate to be
> reused in other tests.
ompare_dbs uses sql() so it means that we need add to header use
My::Nuts::Instance::Server. It is not good to bind Server and Result
classes. May be I can create a new one class (for instance,
ServerResult) where we can put such things?
>
> RC2. get_data_from_source does not check for exceptions while
> loading class. What happens if I require a non-existing
> class?
>
> Maybe add something like:
>
> eval "require $class";
> if ($@)
> {
> ... handle exception code ...
> }
>
ok, agree
> REQUESTS
> --------
> n/a
>
> SUGGESTIONS
> -----------
>
> S1. There is an extra variable used in the loop:
>
> # Run 1st piece of statements on master (A)
>
> Can't you just use $next_i instead of $i?
ok, agree.
>
> S2. I am thinking on moving the code that sets the default data
> source into Driver.pm? That's where all the defaults are
> handled, in opt_configure sub.
>
> So opt_configure would set:
>
> if (not defined($Parameters::data_source))
> {
> $Parameters::data_source= 'DataSource::Simple';
> }
>
> NOTE: (not needed for this WL, but something to think
> about)
ok, agree.
>
> Maybe the next step would be to make an implementation
> more close to the Factory design pattern. This would
> require it to create a class DataSourceFactory:
>
> DataSourceFactory:
> - properties
> class
> options
>
> - methods
> set_class(class)
> set_options(options): void
> get_instance(): DataSource
>
>
> class can be set from --data-source= and options from
> --data-source-option=. Then get_data_from_source could be
> implemented as:
>
> sub get_data_from_source
> {
> $my data_source= DataSourceFactory->get_instance;
> return $data_source->get_data_from_source;
> }
>
> DataSource.pm would be the API for the datasource, which
> today is just get_data_from_source, but needs some
> refactoring/improvement later on...
>
> I wonder if this could then be added to standard NUTS
> library.
Yes probably we need it but at this time I would not like to focus on
improvements for DataSource because:
1. I've no feedback from team (look my email with subject "NUTS
DataSources")
2. Internal changes for DataSource doesn't affect to tests so we can do
it at any time
3. Currently I plan to use RQG (may be with updated grammars) that will
guarantee some new results of testing. It should be enough to get new
bug reports )))
>
> DETAILS
> -------
> On Mon, 2009-09-14 at 13:59 +0400, Serge Kozlov wrote:
>> #At file:///home/ksm/sun/repo/WL5056/nuts-commit/ based on
> revid:serge.kozlov@stripped
>>
>> 349 Serge Kozlov 2009-09-14
>> WL#5056. Test case for NUTS:
>> 1. based on rep::hot_standby
>> 2. added data source support
>> 3. added checking of integrity of data via db comparing
>> modified:
>> suites/rep/hot_standby.pm
>>
>> === modified file 'suites/rep/hot_standby.pm'
>> --- a/suites/rep/hot_standby.pm 2009-06-10 22:25:44 +0000
>> +++ b/suites/rep/hot_standby.pm 2009-09-14 09:59:41 +0000
>> @@ -10,6 +10,7 @@ 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
>> {
>> @@ -24,76 +25,156 @@ sub startup
>> sub fire
>> {
>> my ($test) = @_;
>> - my $master1 = ok_server ($test);
>> - my $master2 = ok_server ($test);
>> - my $slave = ok_server ($test);
>> - plan tests => 24;
>> + my $master = ok_server ($test);
>> + my $master_extra = ok_server ($test);
>> + my $slave = ok_server ($test);
>> +
>> + # read list of statements from data source
>> + my $data = get_data_from_source();
>> +
>> + # summarize number of statements and ok_* command in test
>> + plan tests => scalar(@$data) + 18;
>>
>> - my $dbname = "test";
>> # Setup topology
>> - attach ( $master1, $slave );
>> - attach ( $master1, $master2 );
>> + attach ( $master, $slave );
>> + attach ( $master, $master_extra );
>>
>> # Initial workload
>> - ok_sql ( $master1, "CREATE DATABASE IF NOT EXISTS " . $dbname );
>> - ok_sql ( $master1, "USE " . $dbname );
>> - ok_sql ( $master1, "CREATE TABLE t1 (a INT PRIMARY KEY, b VARCHAR(10));" );
>> - ok_sql ( $master1, "INSERT INTO t1 VALUES(1, \"test\");" );
>> - synchronize ( $master1, $slave );
>> - synchronize ( $master1, $master2 );
>> - ok_sql ( $master2, "USE " . $dbname );
>> + my $dbname = "test";
>> + ok_sql ( $master, "DROP DATABASE IF EXISTS " . $dbname );
>> + ok_sql ( $master, "CREATE DATABASE " . $dbname );
>> + ok_sql ( $master, "USE " . $dbname );
>> +
>> + # Replicate db to B,C
>> + ok_synchronize ( $master, $slave );
>> + ok_synchronize ( $master, $master_extra );
>> + ok_sql ( $master_extra, "USE " . $dbname );
>> ok_sql ( $slave, "USE " . $dbname );
>> - my $master1_rs = ok_sql ( $master1, "SELECT * FROM t1;" );
>> - my $master2_rs = ok_sql ( $master2, "SELECT * FROM t1;" );
>> - my $slave_rs = ok_sql ( $slave, "SELECT * FROM t1;" );
>> - ok_synchronize ( $master1, $slave );
>> - ok_synchronize ( $master1, $master2 );
>> - ok ( ( compare_results ( $master1_rs, $slave_rs ) == 1 ),
>> - "Slave is correctly replicating master1." );
>> - ok ( ( compare_results ( $master1_rs, $master2_rs ) == 1 ),
>> - "Master2 is correctly replicating master1." );
>> +
>> + # Run 1st piece of statements on master (A)
>> + my $next_i;
>> + for (my $i = 0; $i < ((scalar(@$data))/2);$i++)
>> + {
>> + ok_sql ( $master, $data->[$i] );
>> + $next_i= $i;
>> + }
>> + # Store position where we will start next time
>> + $next_i++;
>> +
>> + # Compare DBs
>> + ok_synchronize ( $master, $slave );
>> + ok_synchronize ( $master, $master_extra );
>> + ok ( (compare_dbs([
>> + {"conn" => $master, "dbname" => $dbname},
>> + {"conn" => $master_extra, "dbname" => $dbname},
>> + {"conn" => $slave, "dbname" => $dbname}
>> + ]) == 1), "DBs are equal");
>>
>> # Start switchover {A->B, A->C} -> {B->C}
>> ok_stop_replication ($slave);
>>
>> # Stop C
>> ok_wait_stop_replication ($slave);
>> - my $master1_pos = retrieve_slave_status ( $slave, "Read_Master_Log_Pos" );
>> - my $master1_file = retrieve_slave_status ( $slave, "Master_Log_File" );
>> + my $master_pos = retrieve_slave_status ( $slave, "Read_Master_Log_Pos" );
>> + my $master_file = retrieve_slave_status ( $slave, "Master_Log_File" );
>>
>> # Make B have everything that A has
>> - ok_stop_replication ($master2);
>> - ok_wait_stop_replication ($master2);
>> + ok_stop_replication ($master_extra);
>> + ok_wait_stop_replication ($master_extra);
>> ok_start_replication (
>> - $master2,
>> - master_log_file => $master1_file,
>> - master_log_pos => $master1_pos
>> + $master_extra,
>> + master_log_file => $master_file,
>> + master_log_pos => $master_pos
>> );
>> - ok_wait_stop_sql_replication ($master2);
>> + ok_wait_stop_sql_replication ($master_extra);
>>
>> # Store position of B's binlog
>> - my $master2_file = retrieve_master_status ( $master2, "File" );
>> - my $master2_pos = retrieve_master_status ( $master2, "Position" );
>> + my $master_extra_file = retrieve_master_status ( $master_extra, "File" );
>> + my $master_extra_pos = retrieve_master_status ( $master_extra, "Position"
> );
>>
>> - # Extra workload that should be correctly replicated B->C
>> - ok_sql ( $master2, "INSERT INTO t1 VALUES(2, \"test\");" );
>> + # Run 2nd piece of statements on extra master (B) from stored position
>> + for (my $i = $next_i; $i < scalar(@$data);$i++)
>> + {
>> + ok_sql ( $master_extra, $data->[$i] );
>> + }
>>
>> # Start B->C, stop A
>> - attach ( $master2, $slave, $master2_file, $master2_pos );
>> - stop_server ($master1);
>> + attach ( $master_extra, $slave, $master_extra_file, $master_extra_pos );
>> + stop_server ($master);
>>
>> # Check that B and C have same data
>> - ok_synchronize ( $master2, $slave );
>> - $master2_rs = ok_sql ( $master2, "SELECT * FROM t1;" );
>> - $slave_rs = ok_sql ( $slave, "SELECT * FROM t1;" );
>> - ok ( ( compare_results ( $master2_rs, $slave_rs ) == 1 ),
>> - " Slave correctly restored." );
>> + ok_synchronize ( $master_extra, $slave );
>> + ok ( (compare_dbs([
>> + {"conn" => $master_extra, "dbname" => $dbname},
>> + {"conn" => $slave, "dbname" => $dbname}
>> + ]) == 1), "DBs are equal");
>> }
>>
>> sub shutdown
>> {
>> return;
>> }
>> +
>> +# Compare databases
>> +sub compare_dbs
>> +{
>> + my $params = shift;
>> + my $ok = 1;
>> + my $table_list = {};
>> + my $text = "DBs are equal";
>> + # Find tables
>> + foreach my $option (@$params)
>> + {
>> + my $conn = $option->{"conn"};
>> + my $dbname = $option->{"dbname"};
>> + my $rs = sql ( $conn, "SHOW TABLES FROM $dbname;" );
>> + my @rs_data = get_next($rs);
>> + while (defined $rs_data[0])
>> + {
>> + $table_list->{$rs_data[0]}->{$conn} = 1;
>> + @rs_data = get_next($rs);
>> + }
>> + }
>> + # Compare table lists
>> + foreach my $table_name (keys %$table_list)
>> + {
>> + if (keys(%{$table_list->{$table_name}}) < scalar(@$params))
>> + {
>> + $ok= 0;
>> + last;
>> + $text = "DBs have different output for SHOW TABLE statement";
>> + }
>> + }
>> + if ($ok == 1)
>> + {
>> + foreach my $table_name (keys %$table_list)
>> + {
>> + my $conn1 = undef;
>> + foreach my $option (@$params)
>> + {
>> + my $conn = $option->{"conn"};
>> + if (defined $conn1)
>> + {
>> + my $rs = sql ($conn, "SELECT * FROM $table_name;" );
>> + my $rs1 = sql ($conn1, "SELECT * FROM $table_name;" );
>> + if ( (compare_results($rs, $rs1)) != 1)
>> + {
>> + $ok = 0;
>> + $text = "Table $table_name has different output for different DBs";
>> + last;
>> + }
>> + }
>> + else
>> + {
>> + $conn1 = $conn;
>> + }
>> + }
>> + }
>> + }
>> + #print $text;
>> + return $ok;
>> +}
>> +
>> 1;
>> __END__;
>>
>> @@ -103,6 +184,6 @@ rep::hot_standby - Replication for 2 mas
>>
>> =head1 SYNOPSIS
>>
>> -Replication for 2 master and 1 slave with hot stansby mode.
>> +Replication for 2 master and 1 slave with hot standby mode.
>>
>> =back
>>
>>
--
Serge Kozlov, QA Developer
MySQL AB, Moscow, Russia, www.mysql.com
Office:
Are you MySQL certified? www.mysql.com/certification
