Re: bk commit into 5.1 tree (skozlov:1.2567), please review

[Sven Sandberg]

Serge Kozlov wrote:
> Hi, Sven
> 
> Thanks for review, see my comments marked as "skozlov"
> 
> Sven Sandberg wrote:
>> Hi Serge,
>>
>> Thanks for implementing circular rpl testing! The code basically seems 
>> to work, but I think it needs a few high-level descriptions to be easy 
>> to use for developers, and some more things. See below.
[...]
>> Serge Kozlov wrote:
>> [...]
>>> diff -Nrup a/mysql-test/include/circular_rpl_for_4_hosts_init.inc 
>>> b/mysql-test/include/circular_rpl_for_4_hosts_init.inc
>>> --- /dev/null    Wed Dec 31 16:00:00 196900
>>> +++ b/mysql-test/include/circular_rpl_for_4_hosts_init.inc    
>>> 2008-03-20 23:42:51 +03:00
>>> @@ -0,0 +1,130 @@
>>> +#############################################################
>>> +#
>>> +# Author:  Serge Kozlov <skozlov@stripped>
>>> +# Date:    03/11/2008
>>
>> I think author and date should not be included - you can use 'bk 
>> annotate' to find out. Author and time usually change over time.
> 
> skozlov:
> It is impossible when tests are part of binary distribution so I add 
> additional info at beginning of test.  Also sometimes tests are not in bk
> and header helps in this case.

OK, sure, do as you like. I have no strong opinion about this.

[...]
> skozlov: If want to use master_[0-9] then it possible to add second 
> connection to each server with such name :)

OK.

[...]
>>> +
>>> +--connection master_a
>>> +SELECT 'Master A',b,COUNT(*) FROM t2 WHERE c = 1 GROUP BY b ORDER BY b;
>>> +--connection master_b
>>> +SELECT 'Master B',b,COUNT(*) FROM t2 WHERE c = 1 GROUP BY b ORDER BY b;
>>> +--connection master_c
>>> +SELECT 'Master C',b,COUNT(*) FROM t2 WHERE c = 1 GROUP BY b ORDER BY b;
>>> +--connection master_d
>>> +SELECT 'Master D',b,COUNT(*) FROM t2 WHERE c = 1 GROUP BY b ORDER BY b;
>>> +--connection master_a
>>
>> I'd suggest to use include/diff_tables.inc to compare the tables on 
>> all servers instead. (It needs to be augmented to understand more 
>> servers than "master" and "slave", but that's very easy.)
> 
> Skozlov: Not for all cases there it is possible because failure add diff 
> to tables

OK, I see.

[...]
>>> +# Wait replication between clusters
>>> +--connection master
>>> +let $wait_condition= SELECT COUNT(*)=400 FROM t1 WHERE c = 2;
>>> +--source include/wait_condition.inc
>>> +--connection slave
>>> +--source include/wait_condition.inc
>>
>> Why not 'sync_slave_with_master master' and 'sync_slave_with_master 
>> slave'?
> skozlov: there is an issue to sync clusters. NDB Injector needs a time 
> for adding event to mysqqld binlog so sync_slave_with_master have no 
> effect for that

I see, good to know! Hmmm, would be good to have a generic 
sync_slave_with_master that works for all storage engines... Not part of 
this worklog of course.

-- 
Sven Sandberg, Software Engineer
MySQL AB, www.mysql.com