Merge lp:~gl-az/percona-server/ST-41544-5.5 into lp:percona-server/5.5
Status: | Merged |
---|---|
Approved by: | Laurynas Biveinis on 2014-10-23 |
Approved revision: | 699 |
Merged at revision: | 713 |
Proposed branch: | lp:~gl-az/percona-server/ST-41544-5.5 |
Merge into: | lp:percona-server/5.5 |
Diff against target: |
2662 lines (+779/-833) 43 files modified
mysql-test/extra/binlog_tests/drop_temp_table.test (+1/-0) mysql-test/extra/binlog_tests/tmp_table.test (+0/-1) mysql-test/extra/rpl_tests/rpl_drop_temp.test (+0/-4) mysql-test/extra/rpl_tests/rpl_implicit_commit_binlog.test (+40/-15) mysql-test/extra/rpl_tests/rpl_rewrt_db.test (+1/-3) mysql-test/suite/binlog/r/binlog_database.result (+1/-2) mysql-test/suite/binlog/r/binlog_format_switch_in_tmp_table.result (+0/-78) mysql-test/suite/binlog/r/binlog_mix_drop_tmp_tbl.result (+57/-0) mysql-test/suite/binlog/r/binlog_mix_tmp_table.result (+42/-0) mysql-test/suite/binlog/r/binlog_row_tmp_table.result (+42/-0) mysql-test/suite/binlog/r/binlog_stm_binlog.result (+8/-8) mysql-test/suite/binlog/t/binlog_format_switch_in_tmp_table.test (+0/-76) mysql-test/suite/binlog/t/binlog_mix_drop_tmp_tbl.test (+4/-0) mysql-test/suite/binlog/t/binlog_mix_tmp_table.test (+2/-0) mysql-test/suite/binlog/t/binlog_row_tmp_table.test (+2/-0) mysql-test/suite/binlog/t/binlog_stm_drop_tmp_tbl.test (+1/-2) mysql-test/suite/binlog/t/binlog_stm_tmp_table.test (+2/-0) mysql-test/suite/rpl/r/rpl_bug58546.result (+44/-0) mysql-test/suite/rpl/r/rpl_mix_drop_temp.result (+40/-0) mysql-test/suite/rpl/r/rpl_mix_rewrt_db.result (+218/-0) mysql-test/suite/rpl/r/rpl_mixed_drop_create_temp_table.result (+94/-118) mysql-test/suite/rpl/r/rpl_mixed_implicit_commit_binlog.result (+3/-7) mysql-test/suite/rpl/r/rpl_mixed_mixing_engines.result (+0/-12) mysql-test/suite/rpl/r/rpl_non_direct_mixed_mixing_engines.result (+0/-12) mysql-test/suite/rpl/r/rpl_stop_slave.result (+0/-46) mysql-test/suite/rpl/r/rpl_temp_table_mix_row.result (+0/-111) mysql-test/suite/rpl/t/rpl_bug58546.test (+69/-0) mysql-test/suite/rpl/t/rpl_create_tmp_table_if_not_exists.test (+2/-1) mysql-test/suite/rpl/t/rpl_mix_drop_temp-slave.opt (+2/-0) mysql-test/suite/rpl/t/rpl_mix_drop_temp.test (+6/-0) mysql-test/suite/rpl/t/rpl_mix_rewrt_db-slave.opt (+1/-0) mysql-test/suite/rpl/t/rpl_mix_rewrt_db.test (+7/-0) mysql-test/suite/rpl/t/rpl_row_reset_slave.test (+2/-2) mysql-test/suite/rpl/t/rpl_stm_drop_temp.test (+6/-0) mysql-test/suite/rpl/t/rpl_stm_reset_slave.test (+1/-1) mysql-test/suite/rpl/t/rpl_stm_rewrt_db.test (+7/-0) mysql-test/suite/rpl/t/rpl_stop_slave.test (+8/-88) mysql-test/suite/rpl/t/rpl_temp_table_mix_row.test (+0/-210) mysql-test/suite/rpl/t/rpl_trunc_temp.test (+3/-2) sql/sql_class.cc (+38/-15) sql/sql_class.h (+3/-18) sql/sql_table.cc (+14/-1) sql/sql_truncate.cc (+8/-0) |
To merge this branch: | bzr merge lp:~gl-az/percona-server/ST-41544-5.5 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Laurynas Biveinis (community) | 2014-06-26 | Approve on 2014-10-23 | |
Review via email:
|
Description of the change
bug 1313901 : When a session creates a temporary table and that table is used
within queries, the binlog format of the session is unconditionally switched to
ROW for the remainder of the session or until all temporary tables have been
dropped.
This is a result of an old fix for upstream 20499.
The consequence this has on the slave is that, the slave is single threaded and
executes all queries from a single session. If the slave is also binlogging, the
slave will unconditionally binlog every statement it receives in ROW format
until all temp tables are dropped. This means statements get logged in different
format on the master and the slave and can introduce huge slave lag.
Through discussion with our varous experts, we decided to undo the original
20499 fix and change the fundamental way MIXED replication works with regard to
temp tables. The new MIXED replication behavior can be summed up as follows:
If using MIXED mode replication, all DDL and DML statements that touch a
temporary table will be marked to binlog in ROW format. This means that any
statement that is using exclusively temporary tables will not be binlogged at
all. Any DDL or DML that uses a mix of temporary and non-temporary tables will
be binlogged in ROW format for the non-temporary table operations. The one
exception is DROP table when dropping a temporary table which will always be
binlogged as DROP TEMPORARY TABLE IF EXISTS on the master.
The code changes to make this happen are very minor but have a huge impact.
Many test cases in the rpl and binlog suite need to be changes and some
eliminated entirely as they test for functionality that no longer exists.
During the testing of this feature, each and every mtr test failure was closely
scruitinized to ensure that the failure was a legitimate and expected change
in behavior. This caused a lot of back-and-forth debugging and fixing to ensure
that the resulting server behavior and mtr test cases were 100% as described in
the new behavior above.
George Ormond Lorch III (gl-az) wrote : | # |
- For setting creating_temp_table to TRUE, add
!
statement.
- the header comment in the tests that include
extra/
removed from the tests themselves.
- extra/binlog_
bug #46572 is now outdated for MBR.
- Is this bit really correct? (and the rest of binlog_stm_binlog)
We shouldn't be affecting SBR replication of temp tables, should
we?
--- mysql-test/
+++ mysql-test/
@@ -635,9 +635,7 @@
COERCIBILITY(s1) d3;
DROP TEMPORARY TABLE tmp1;
END
-master-bin.000001 # Query # # use `bug39182`; CREATE TEMPORARY TABLE tmp1
-SELECT * FROM t1 WHERE a LIKE CONCAT("%", NAME_CONST(
-master-bin.000001 # Query # # use `bug39182`; DROP TEMPORARY TABLE `tmp1` /* generated by server */
+master-bin.000001 # Query # # DROP TEMPORARY TABLE IF EXISTS `bug39182`.`tmp1` /* generated by server */
DROP PROCEDURE p1;
DROP TABLE t1;
DROP DATABASE bug39182;
- Would it be a good idea to record RBR/MBR version(s) of
binlog_
- rpl_row_reset_slave and rpl_mix_reset_slave result files are
identical. Let's join the testcases?
- rpl_stop_slave tests more than temp table replication. Possible
to restore it for MBR?
George Ormond Lorch III (gl-az) wrote : | # |
OK, it's been a bit since I updated this so I wanted to give a quick status update on it...
> - For setting creating_temp_table to TRUE, add
> !create_
> statement.
You have a typo here, at first I couldn't figure out what you were suggesting but I finally saw it. Since that 'if' statement is executed within the loop iterating all tables in the query, and the 'if' statement does find_temporary_
>
> - the header comment in the tests that include
> extra/rpl_
> removed from the tests themselves.
>
> - extra/binlog_
> bug #46572 is now outdated for MBR.
Yeah, good eye, got these easily fixed.
>
> - Is this bit really correct? (and the rest of binlog_stm_binlog)
> We shouldn't be affecting SBR replication of temp tables, should
> we?
>
> --- mysql-test/
> +0000
> +++ mysql-test/
> +0000
> @@ -635,9 +635,7 @@
> COERCIBILITY(s1) d3;
> DROP TEMPORARY TABLE tmp1;
> END
> -master-bin.000001 # Query # # use `bug39182`; CREATE
> TEMPORARY TABLE tmp1
> -SELECT * FROM t1 WHERE a LIKE CONCAT("%", NAME_CONST(
> COLLATE 'utf8_unicode_ci'), "%")
> -master-bin.000001 # Query # # use `bug39182`; DROP
> TEMPORARY TABLE `tmp1` /* generated by server */
> +master-bin.000001 # Query # # DROP TEMPORARY TABLE
> IF EXISTS `bug39182`.`tmp1` /* generated by server */
> DROP PROCEDURE p1;
> DROP TABLE t1;
> DROP DATABASE bug39182;
Still looking into this one but IIRC from the earlier work on this we are setting the statement as ROW but that then doesn't block it from the binlog, it transforms it into a DROP TABLE IF EXISTS instead. Since this test is for STMT only, I don't quite know why this has changed. It may be a valid side effect of the changes to avoid a lot more coding around the issue or it may be a bug in the fix. Need to investigate this deeper to figure out what the right answer is.
>
> - Would it be a good idea to record RBR/MBR version(s) of
> binlog_
Why yes it would, a very good idea, and in fact, it found a bug. TRUNCATE TABLE on a temp table is still getting binlogged as STMT in mixed mode...working on a somewhat simple fix here.
>
> - rpl_row_reset_slave and rpl_mix_reset_slave result files are
> identical. Let's join the testcases?
Sure, done, changed existing rpl_row_
>
> - rpl_stop_slave tests more than temp table replication. Possible
> to restore it for MBR?
...
> > - rpl_stop_slave tests more than temp table replication. Possible
> > to restore it for MBR?
>
> Not easily, the test is really designed for STMT only as it does diff_tables
> between the master and slave, mostly on temp tables. Since this change makes
> temp tables operate more like ROW when doing MIXED it means we would almost
> have to rewrite a new stop_slave test for MBR that just skipped anything
> related to temp tables in order to test the non-temp bits. If you think we
> should go that far then yeah, I can do that.
Are you referring to "bug 56118" part of rpl_stop_slave? There is "bug 58546" part too, which does not seem to be related to temp tables. Maybe split the testcase then?
George Ormond Lorch III (gl-az) wrote : | # |
On 10/12/2014 01:49 AM, Laurynas Biveinis wrote:
>>> - rpl_stop_slave tests more than temp table replication. Possible
>>> to restore it for MBR?
>> Not easily, the test is really designed for STMT only as it does diff_tables
>> between the master and slave, mostly on temp tables. Since this change makes
>> temp tables operate more like ROW when doing MIXED it means we would almost
>> have to rewrite a new stop_slave test for MBR that just skipped anything
>> related to temp tables in order to test the non-temp bits. If you think we
>> should go that far then yeah, I can do that.
> Are you referring to "bug 56118" part of rpl_stop_slave? There is "bug 58546" part too, which does not seem to be related to temp tables. Maybe split the testcase then?
Yes, the 56118 is now pretty specific to SBR and temp tables. I will
split 58546 out to a unique test case that does both SBR and MBR.
--
George O. Lorch III
Software Engineer, Percona
+1-888-401-3401 x542 US/Arizona (GMT -7)
skype: george.
George Ormond Lorch III (gl-az) wrote : | # |
On 07/05/2014 05:45 AM, Laurynas Biveinis wrote:
> - Is this bit really correct? (and the rest of binlog_stm_binlog)
> We shouldn't be affecting SBR replication of temp tables, should
> we?
>
> --- mysql-test/
> +++ mysql-test/
> @@ -635,9 +635,7 @@
> COERCIBILITY(s1) d3;
> DROP TEMPORARY TABLE tmp1;
> END
> -master-bin.000001 # Query # # use `bug39182`; CREATE TEMPORARY TABLE tmp1
> -SELECT * FROM t1 WHERE a LIKE CONCAT("%", NAME_CONST(
> -master-bin.000001 # Query # # use `bug39182`; DROP TEMPORARY TABLE `tmp1` /* generated by server */
> +master-bin.000001 # Query # # DROP TEMPORARY TABLE IF EXISTS `bug39182`.`tmp1` /* generated by server */
> DROP PROCEDURE p1;
> DROP TABLE t1;
> DROP DATABASE bug39182;
>
OK, yes, this is correct. I didn't catch it before but the issue is that
the test case name implies SBR, but in fact it is specifically testing
MBR instead, thus these result changes are correct. I suppose that could
be a bug in itself that the test is binlog_
would probably be a good idea to add/record a binlog_
fix the _stm_ to actualy test SBR.
--
George O. Lorch III
Software Engineer, Percona
+1-888-401-3401 x542 US/Arizona (GMT -7)
skype: george.
George Ormond Lorch III (gl-az) wrote : | # |
OK, so I think I have everything addressed and a new push to the branch.
Here is a new jenkins run: http://
There are a few test failures, most of which are pre-existing. Two in particular that show up in replication are:
rpl.rpl_
rpl.rpl_row_until 'row' - This is a pre-existing issue and I can sporadically reproduce it locally on revno 670 (before this fix).
- Please rebase on the current 5.5 to 5.6 GCA. It will include at
least one more upstream merge, which always touches replication
and its testsuite. Perhaps that will resolve the rpl_stop_slave
conflicts too.
- suite/rpl/
extra/
suite/
give a smaller diff, and the upstream merges will cause
conflicts or automerge at the right place.
- Likewise for rpl_rewrt_db.
- In rpl_mixed_
N-Temp T-SELECT-N-Temp N-Temp C"), it seems that binlog is
missing the row events for INSERT INTO nt_xx_1 SELECT * FROM
tt_tmp_xx_1;
- Add a comment to the end of rpl_stop_slave "bug 58546 part was
moved to ... " to help with future merges.
- There is a couple of diff comments.
George Ormond Lorch III (gl-az) wrote : | # |
> - Please rebase on the current 5.5 to 5.6 GCA. It will include at
> least one more upstream merge, which always touches replication
> and its testsuite. Perhaps that will resolve the rpl_stop_slave
> conflicts too.
Sorry about that, I did rebase what seem like not too long ago but must have just missed the 5.5.39-36.0 merge.
I am beginning to have a concern over the long term maintenance of this fix. This is the 3rd or 4th time I have rebased and each time I have had to fixup mtr tests to work correctly with this fix :-\
George Ormond Lorch III (gl-az) wrote : | # |
> - In rpl_mixed_
> N-Temp T-SELECT-N-Temp N-Temp C"), it seems that binlog is
> missing the row events for INSERT INTO nt_xx_1 SELECT * FROM
> tt_tmp_xx_1;
Are you sure about this, that you are looking at the same test correctly? Diff line 1792+ isn't:
'B N N-Temp T-SELECT-N-Temp N-Temp C'
it is:
'B N N-Temp N-SELECT-T-Temp N-Temp C'
This test and the results seem fine...Here is the portion of the result file for 'B N N-Temp T-SELECT-N-Temp N-Temp C':
SET @commands= 'B N N-Temp T-SELECT-N-Temp N-Temp C';
BEGIN;
INSERT INTO nt_xx_1() VALUES (1);
INSERT INTO nt_tmp_xx_1() VALUES (1);
INSERT INTO tt_xx_1 SELECT * FROM nt_tmp_xx_1;
INSERT INTO nt_tmp_xx_1() VALUES (1);
COMMIT;
-b-b-b-
Log_name Pos Event_type Server_id End_log_pos Info
master-bin.000001 # Query # # BEGIN
master-bin.000001 # Query # # use `test`; INSERT INTO nt_xx_1() VALUES (1)
master-bin.000001 # Query # # COMMIT
master-bin.000001 # Query # # BEGIN
master-bin.000001 # Table_map # # table_id: # (test.tt_xx_1)
master-bin.000001 # Write_rows # # table_id: # flags: STMT_END_F
master-bin.000001 # Xid # # COMMIT /* XID */
-e-e-e-
The only things that should be binlogged here is the 'INSERT INTO nt_xx_1' as STMT, then the 'INSERT INTO tt_xx_1 SELECT' as ROW and that seems to be happening correctly right?
Maybe you are looking at the following case 'B N N-Temp N-SELECT-T-Temp N-Temp C', this is file as well, here is the snippet from the result file:
SET @commands= 'B N N-Temp N-SELECT-T-Temp N-Temp C';
BEGIN;
INSERT INTO nt_xx_1() VALUES (1);
INSERT INTO nt_tmp_xx_1() VALUES (1);
INSERT INTO nt_xx_1 SELECT * FROM tt_tmp_xx_1;
INSERT INTO nt_tmp_xx_1() VALUES (1);
COMMIT;
-b-b-b-
Log_name Pos Event_type Server_id End_log_pos Info
master-bin.000001 # Query # # BEGIN
master-bin.000001 # Query # # use `test`; INSERT INTO nt_xx_1() VALUES (1)
master-bin.000001 # Query # # COMMIT
-e-e-e-
At first this looked wrong, like it was missing row events for the third insert 'INSERT INTO nt_xx_1 SELECT * FROM tt_tmp_xx_1;', which it is, because there are no rows in tt_tmp_xx_1 at this point in the test, it seems that it is an empty temp table that never gets anything inserted the entire test. So this case seems right to me as well.
George Ormond Lorch III (gl-az) wrote : | # |
Addressed issues noted except for the 'missing row' as mentioned in previous comment. New jenkins run: http://
- This now looks good, thanks for explaining the
rpl_
bzr bits that need addressing. As for the patch maintenance, it
looks like the hard part is already behind us, at least until
upstream fixes the same.
- Please recommit with --file option instead of -m ;)
- The old rpl_drop_
rpl_
rpl_
files too? All this would help with bzr merges.
George Ormond Lorch III (gl-az) wrote : | # |
> - Please recommit with --file option instead of -m ;)
/facepalm
>
> - The old rpl_drop_
> rpl_stm_
> rpl_stm_
> files too? All this would help with bzr merges.
Done, I wasn't sure which way was going to be best, mv or add, had a 50/50 shot at guessing :)
And another jenkins run for good measure:
http://
Looks good. Will be merged once the 5.6 MP is ready.
Jenkins http:// jenkins. percona. com/view/ PS%205. 5/job/percona- server- 5.5-param/ 1014/