Code review comment for lp:~gl-az/percona-server/BT-26980-5.5-bug1068210

Revision history for this message
George Ormond Lorch III (gl-az) wrote :

> - Any reason to mark this test for SBR/MBR and not RBR? If I
> understand correctly, it is only the SBR that triggers the
> bug. MBR would use RBR for all t1 events. So, let's just test
> SBR.

This test will not work with RBR. It seems that if RBR is used, the slave side trigger is not fired and thus the rows replicated into t1 never create the corresponding rows in slave_only, causing the test case to fail on SELECT id FROM slave_only; IT _does_ seem to work fine for SBR and MBR though.

> - Please replace connection slave; with sync_slave_with_master at
> diff line 72, so that the presence of t1 is guaranteed at slave
> at the trigger create time.

Good catch.

> - Redundant "conection slave;" at line 85.

Another good catch.

> - Lines 89 and 90 can be replaced by include/, but
> this for future reference, as in this instance the testcase
> cleanup should be done in another way:
> - Tables t1 and t2 would remain on the slave at the end of the
> testcase end, because their DROPs are not replicated. Thus,
> omit the slave stop, perform the slave-only cleanup, perform the
> (replicated) cleanup on master, and then include,
> which will sync, stop slave, and cleanup.

Ahh, very cool. I was a bit uncertain as to the 'correct' way to clean up the slave in this situation and didn't know about Much cleaner now, Thanks!

« Back to merge proposal