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

Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

    - The code looks OK. I am not sure why there is no
      deferred_events::rewind() call immediatelly after or in
      d_e::execute() method, which would seem to resolve this issue,
      but oh well.
    - The testcase:
    - 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.
    - 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.
    - Redundant "conection slave;" at line 85.
    - Lines 89 and 90 can be replaced by include/stop_slave.inc, 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 rpl_end.inc,
      which will sync, stop slave, and cleanup.

review: Needs Fixing

« Back to merge proposal