Skip to content

Commit 1b4efbe

Browse files
emoonrainemoonrain
committed
MDEV-35207 ignored error at binlogging by CREATE-TABLE-SELECT leads to assert
MDEV-35499 Errored-out CREATE-or-REPLACE-SELECT does not log DROP table into binlog MDEV-35502 Failed at ROW-format binlogging CREATE-TABLE-SELECT should not generate Incident event When a CREATE TABLE .. SELECT errors while inserting data, a user would expect that all changes are rolled back and the table would not exist after executing the query. However CREATE-TABLE-SELECT can face an error near the end of its execution select_create::send_eof() so that the error was never checked which led to various assert inside binlogging path that should not be attended at all. Specifically when binlog_commit() of ha_commit_one_phase() that CREATE-TABLE-SELECT employs errored out because of a limited cache size (binlog_commit may try writing to a transactional cache) the cache was not flushed to binlog. The missed error check allowed further execution down to trans_commit_implicit() in whose stack DBUG_ASSERT(!(entry->using_trx_cache && !mngr->trx_cache.empty() && mngr->get_binlog_cache_log(TRUE)->error)); fired. In a non-debug build that table remains created/populated inconsistently with binlog. The fixes need and install the error checking in select_create::send_eof(). That prevents from any further execution when ha_commit_one_phase() fails for any reason (typically due to binlog_commit()). This commit also covers CREATE-or-REPLACE-SELECT that additionally had a specific issue in that DROP TABLE was not logged the binary log, MDEV-35499. See changes select_create::abort_result_set(). The current commit also corrects an unnecessary Incident event logging when CREATE-TABLE-SELECT encounters a binloging issue, MDEV-35502. The Incident was actually only harmful in this case as the table was never going to be created, therefore replicated, in such a case. In "normal" cases when the SELECT phase errors due to binlogging, an internal incident flag gets reset inside select_create::abort_result_set(). A hunk in select_insert::prepare_eof() addresses a specific kind of this issue that deals with incorrect computation of the binlog cache type. Because of that in the OLD version execution was allowed to proceed along ha_commit_trans()..binlog_commit() while a Pending event was not flushed to the transactional cache. That might lead to the unnecessary binlogged Incident despite the select_create::abort_result_set() measures. However now with the corrected cache type any binlogging error to flush the Pending event is covered according to the normal case. non-transaction table, updates to the non-transactional table NOTE the commit contains few tests overlapping with unfixed yet MDEV-36027. Thanks to Brandon Nesterenko and Kristian Nielsen for thorough review, and Kristian additionally for ideas to simplify the patch and some code contribution.
1 parent 58a3677 commit 1b4efbe

File tree

5 files changed

+374
-7
lines changed

5 files changed

+374
-7
lines changed

‎mysql-test/suite/rpl/r/rpl_create_select_row.result

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
include/master-slave.inc
2+
[connection master]
3+
connection master;
4+
set @max_binlog_cache_size = @@global.max_binlog_cache_size;
5+
set @binlog_cache_size = @@global.binlog_cache_size;
6+
set @@global.max_binlog_cache_size = 4096;
7+
set @@global. binlog_cache_size = 4096;
8+
#
9+
# MDEV-35207 ignored error at binlogging by CREATE-TABLE-SELECT leads to assert
10+
#
11+
connect conn_err,localhost,root,,;
12+
call mtr.add_suppression("Multi-statement transaction required more than 'max_binlog_cache_size' bytes of storage");
13+
create table t engine=myisam select repeat ('a',4096*3) AS a;
14+
ERROR HY000: Multi-statement transaction required more than 'max_binlog_cache_size' bytes of storage; increase this mariadbd variable and try again
15+
create table t engine=innodb select repeat ('a',4096*3) AS a;
16+
ERROR HY000: Multi-statement transaction required more than 'max_binlog_cache_size' bytes of storage; increase this mariadbd variable and try again
17+
create table t (a int unique, b char) select 1 AS a, 'b' as b union select 1 as a, 'c' as b;
18+
ERROR 23000: Duplicate entry '1' for key 'a'
19+
select * from t;
20+
ERROR 42S02: Table 'test.t' doesn't exist
21+
disconnect conn_err;
22+
connection master;
23+
24+
#
25+
# MDEV-35499 errored CREATE-OR-REPLACE-SELECT does not DROP table in binlog
26+
#
27+
#
28+
# Engine = innodb
29+
#
30+
set statement binlog_format=statement for create table t (a int) select 1 as a;
31+
set statement binlog_format=row for create or replace table t (a int primary key, b char) engine=innodb select 1 AS a, 'b' as b union select 1 as a, 'c' as b;
32+
ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
33+
select * from t;
34+
ERROR 42S02: Table 'test.t' doesn't exist
35+
#
36+
# Prove an expected lonely `DROP table t'
37+
include/show_binlog_events.inc
38+
Log_name Pos Event_type Server_id End_log_pos Info
39+
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
40+
master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS `test`.`t`/* Generated to handle failed CREATE OR REPLACE */
41+
master-bin.000001 # Query # # ROLLBACK
42+
set statement binlog_format=statement for create table t (a int) select 1 as a;
43+
set statement binlog_format=row for create or replace table t (a text) engine=innodb select repeat ('a',1024) AS a union select repeat ('a',3*4096) AS a union select repeat ('a',3*4096) AS a;
44+
ERROR HY000: Multi-statement transaction required more than 'max_binlog_cache_size' bytes of storage; increase this mariadbd variable and try again
45+
select * from t;
46+
ERROR 42S02: Table 'test.t' doesn't exist
47+
#
48+
# Prove an expected lonely `DROP table t'
49+
include/show_binlog_events.inc
50+
Log_name Pos Event_type Server_id End_log_pos Info
51+
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
52+
master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS `test`.`t`/* Generated to handle failed CREATE OR REPLACE */
53+
master-bin.000001 # Query # # ROLLBACK
54+
set statement binlog_format=statement for create table t (a int) select 1 as a;
55+
set statement binlog_format=row for create or replace table t (a text) engine=innodb select repeat ('a',4096*3) AS a;;
56+
ERROR HY000: Multi-statement transaction required more than 'max_binlog_cache_size' bytes of storage; increase this mariadbd variable and try again
57+
select * from t;
58+
ERROR 42S02: Table 'test.t' doesn't exist
59+
#
60+
# Prove an expected lonely `DROP table t'
61+
include/show_binlog_events.inc
62+
Log_name Pos Event_type Server_id End_log_pos Info
63+
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
64+
master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS `test`.`t`/* Generated to handle failed CREATE OR REPLACE */
65+
master-bin.000001 # Query # # ROLLBACK
66+
#
67+
# Engine = myisam
68+
#
69+
set statement binlog_format=statement for create table t (a int) select 1 as a;
70+
set statement binlog_format=row for create or replace table t (a int primary key, b char) engine=myisam select 1 AS a, 'b' as b union select 1 as a, 'c' as b;
71+
ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
72+
select * from t;
73+
ERROR 42S02: Table 'test.t' doesn't exist
74+
#
75+
# Prove an expected lonely `DROP table t'
76+
include/show_binlog_events.inc
77+
Log_name Pos Event_type Server_id End_log_pos Info
78+
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
79+
master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS `test`.`t`/* Generated to handle failed CREATE OR REPLACE */
80+
master-bin.000001 # Query # # ROLLBACK
81+
set statement binlog_format=statement for create table t (a int) select 1 as a;
82+
set statement binlog_format=row for create or replace table t (a text) engine=myisam select repeat ('a',1024) AS a union select repeat ('a',3*4096) AS a union select repeat ('a',3*4096) AS a;
83+
ERROR HY000: Multi-statement transaction required more than 'max_binlog_cache_size' bytes of storage; increase this mariadbd variable and try again
84+
select * from t;
85+
ERROR 42S02: Table 'test.t' doesn't exist
86+
#
87+
# Prove an expected lonely `DROP table t'
88+
include/show_binlog_events.inc
89+
Log_name Pos Event_type Server_id End_log_pos Info
90+
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
91+
master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS `test`.`t`/* Generated to handle failed CREATE OR REPLACE */
92+
master-bin.000001 # Query # # ROLLBACK
93+
set statement binlog_format=statement for create table t (a int) select 1 as a;
94+
set statement binlog_format=row for create or replace table t (a text) engine=myisam select repeat ('a',4096*3) AS a;;
95+
ERROR HY000: Multi-statement transaction required more than 'max_binlog_cache_size' bytes of storage; increase this mariadbd variable and try again
96+
select * from t;
97+
ERROR 42S02: Table 'test.t' doesn't exist
98+
#
99+
# Prove an expected lonely `DROP table t'
100+
include/show_binlog_events.inc
101+
Log_name Pos Event_type Server_id End_log_pos Info
102+
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
103+
master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS `test`.`t`/* Generated to handle failed CREATE OR REPLACE */
104+
master-bin.000001 # Query # # ROLLBACK
105+
create table ti_pk (a int primary key) engine=innodb;
106+
create table ta (a int) engine=aria;
107+
create function f_ia(arg int)
108+
returns integer
109+
begin
110+
insert into ti_pk set a=1;
111+
insert into ta set a=1;
112+
insert into ti_pk set a=arg;
113+
return 1;
114+
end |
115+
set statement binlog_format = ROW for create table t_y (a int) engine=aria select f_ia(1 /* err */) as a;
116+
ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
117+
select * from t_y;
118+
ERROR 42S02: Table 'test.t_y' doesn't exist
119+
# correct execution: `ta` is modified and its new record is binlogged
120+
include/show_binlog_events.inc
121+
Log_name Pos Event_type Server_id End_log_pos Info
122+
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
123+
master-bin.000001 # Table_map # # table_id: # (test.ta)
124+
master-bin.000001 # Write_rows_v1 # # table_id: # flags: STMT_END_F
125+
master-bin.000001 # Query # # COMMIT
126+
select * from ta;
127+
a
128+
1
129+
select * from ti_pk;
130+
a
131+
connection slave;
132+
include/diff_tables.inc [master:ta,slave:ta]
133+
connection master;
134+
delete from ta;
135+
connection slave;
136+
connection master;
137+
set statement binlog_format = STATEMENT for create table t_y (a int) engine=aria select f_ia(1 /* err */) as a;
138+
ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
139+
select * from t_y;
140+
ERROR 42S02: Table 'test.t_y' doesn't exist
141+
# ***TODO: fix MDEV-36027***. As of now `ta` is modified but that's not binlogged
142+
include/show_binlog_events.inc
143+
select *,'on_master' from ta;
144+
a on_master
145+
1 on_master
146+
select * from ti_pk;
147+
a
148+
connection slave;
149+
select *,'on_slave' from ta;
150+
a on_slave
151+
connection master;
152+
drop function f_ia;
153+
drop table ti_pk, ta;
154+
SET @@global.max_binlog_cache_size = @max_binlog_cache_size;
155+
SET @@global. binlog_cache_size = @binlog_cache_size;
156+
connection slave;
157+
End of the tests
158+
include/rpl_end.inc

‎mysql-test/suite/rpl/t/rpl_create_select_row.test

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
--source include/have_binlog_format_row.inc
2+
--source include/have_innodb.inc
3+
--source include/master-slave.inc
4+
5+
--connection master
6+
set @max_binlog_cache_size = @@global.max_binlog_cache_size;
7+
set @binlog_cache_size = @@global.binlog_cache_size;
8+
set @@global.max_binlog_cache_size = 4096;
9+
set @@global. binlog_cache_size = 4096;
10+
11+
--echo #
12+
--echo # MDEV-35207 ignored error at binlogging by CREATE-TABLE-SELECT leads to assert
13+
--echo #
14+
# fix the current (write) binlog position
15+
--let $binlog_file_0= query_get_value(SHOW MASTER STATUS, File, 1)
16+
--let $binlog_start_0 = query_get_value(SHOW MASTER STATUS, Position, 1)
17+
18+
# use a separate connection also to validate its close will be clean
19+
connect (conn_err,localhost,root,,);
20+
21+
call mtr.add_suppression("Multi-statement transaction required more than 'max_binlog_cache_size' bytes of storage");
22+
--error ER_TRANS_CACHE_FULL
23+
create table t engine=myisam select repeat ('a',4096*3) AS a;
24+
25+
--error ER_TRANS_CACHE_FULL
26+
create table t engine=innodb select repeat ('a',4096*3) AS a;
27+
28+
--error ER_DUP_ENTRY
29+
create table t (a int unique, b char) select 1 AS a, 'b' as b union select 1 as a, 'c' as b;
30+
--error ER_NO_SUCH_TABLE
31+
select * from t;
32+
33+
--disconnect conn_err
34+
35+
--connection master
36+
--let $binlog_file_1= query_get_value(SHOW MASTER STATUS, File, 1)
37+
--let $binlog_start_1= query_get_value(SHOW MASTER STATUS, Position, 1)
38+
39+
--let $cmp = `select strcmp('$binlog_file_1', '$binlog_file_0') <> 0 OR $binlog_start_1 <> $binlog_start_0`
40+
if (!$cmp)
41+
{
42+
--echo *** Error: unexpected advance of binlog position
43+
--die
44+
}
45+
46+
--echo
47+
--echo #
48+
--echo # MDEV-35499 errored CREATE-OR-REPLACE-SELECT does not DROP table in binlog
49+
--echo #
50+
--let $i = 2
51+
while ($i)
52+
{
53+
--let $engine=`select if($i % 2, "myisam", "innodb")`
54+
--echo #
55+
--echo # Engine = $engine
56+
--echo #
57+
set statement binlog_format=statement for create table t (a int) select 1 as a;
58+
--let $binlog_file= query_get_value(SHOW MASTER STATUS, File, 1)
59+
--let $binlog_start = query_get_value(SHOW MASTER STATUS, Position, 1)
60+
--error ER_DUP_ENTRY
61+
--eval set statement binlog_format=row for create or replace table t (a int primary key, b char) engine=$engine select 1 AS a, 'b' as b union select 1 as a, 'c' as b
62+
--error ER_NO_SUCH_TABLE
63+
select * from t;
64+
--echo #
65+
--echo # Prove an expected lonely `DROP table t'
66+
--source include/show_binlog_events.inc
67+
68+
# error before stmt commit
69+
set statement binlog_format=statement for create table t (a int) select 1 as a;
70+
--let $binlog_file= query_get_value(SHOW MASTER STATUS, File, 1)
71+
--let $binlog_start = query_get_value(SHOW MASTER STATUS, Position, 1)
72+
--error ER_TRANS_CACHE_FULL
73+
--eval set statement binlog_format=row for create or replace table t (a text) engine=$engine select repeat ('a',1024) AS a union select repeat ('a',3*4096) AS a union select repeat ('a',3*4096) AS a
74+
--error ER_NO_SUCH_TABLE
75+
select * from t;
76+
--echo #
77+
--echo # Prove an expected lonely `DROP table t'
78+
--source include/show_binlog_events.inc
79+
80+
# error at stmt commit
81+
set statement binlog_format=statement for create table t (a int) select 1 as a;
82+
--let $binlog_file= query_get_value(SHOW MASTER STATUS, File, 1)
83+
--let $binlog_start = query_get_value(SHOW MASTER STATUS, Position, 1)
84+
--error ER_TRANS_CACHE_FULL
85+
--eval set statement binlog_format=row for create or replace table t (a text) engine=$engine select repeat ('a',4096*3) AS a;
86+
--error ER_NO_SUCH_TABLE
87+
select * from t;
88+
--echo #
89+
--echo # Prove an expected lonely `DROP table t'
90+
--source include/show_binlog_events.inc
91+
92+
--dec $i
93+
}
94+
95+
# Tests of mixed engines to demonstrate non-transaction table updates
96+
# are binlogged or otherwise MDEV-36027.
97+
create table ti_pk (a int primary key) engine=innodb;
98+
create table ta (a int) engine=aria;
99+
delimiter |;
100+
create function f_ia(arg int)
101+
returns integer
102+
begin
103+
insert into ti_pk set a=1;
104+
insert into ta set a=1;
105+
insert into ti_pk set a=arg;
106+
return 1;
107+
end |
108+
delimiter ;|
109+
110+
--let $binlog_file= query_get_value(SHOW MASTER STATUS, File, 1)
111+
--let $binlog_start = query_get_value(SHOW MASTER STATUS, Position, 1)
112+
113+
--error ER_DUP_ENTRY
114+
set statement binlog_format = ROW for create table t_y (a int) engine=aria select f_ia(1 /* err */) as a;
115+
--error ER_NO_SUCH_TABLE
116+
select * from t_y;
117+
118+
--echo # correct execution: `ta` is modified and its new record is binlogged
119+
--source include/show_binlog_events.inc
120+
select * from ta;
121+
select * from ti_pk;
122+
123+
--sync_slave_with_master
124+
--let $diff_tables=master:ta,slave:ta
125+
--source include/diff_tables.inc
126+
127+
--connection master
128+
delete from ta;
129+
--sync_slave_with_master
130+
131+
--connection master
132+
# MDEV-36027 Errored-out CREATE-SELECT does not binlog results of any function modifying non-transactional table
133+
--let $binlog_file= query_get_value(SHOW MASTER STATUS, File, 1)
134+
--let $binlog_start = query_get_value(SHOW MASTER STATUS, Position, 1)
135+
--error ER_DUP_ENTRY
136+
set statement binlog_format = STATEMENT for create table t_y (a int) engine=aria select f_ia(1 /* err */) as a;
137+
--error ER_NO_SUCH_TABLE
138+
select * from t_y;
139+
140+
--echo # ***TODO: fix MDEV-36027***. As of now `ta` is modified but that's not binlogged
141+
--source include/show_binlog_events.inc
142+
select *,'on_master' from ta;
143+
select * from ti_pk;
144+
145+
--sync_slave_with_master
146+
select *,'on_slave' from ta;
147+
148+
# Cleanup
149+
--connection master
150+
drop function f_ia;
151+
drop table ti_pk, ta;
152+
153+
SET @@global.max_binlog_cache_size = @max_binlog_cache_size;
154+
SET @@global. binlog_cache_size = @binlog_cache_size;
155+
156+
# test that binlog replicates correctly to slave
157+
# --connection slave
158+
--sync_slave_with_master
159+
160+
--echo End of the tests
161+
--source include/rpl_end.inc

‎sql/log.cc

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,11 @@ class binlog_cache_data
322322
incident= TRUE;
323323
}
324324

325+
void clear_incident(void)
326+
{
327+
incident= FALSE;
328+
}
329+
325330
bool has_incident(void)
326331
{
327332
return(incident);
@@ -2540,6 +2545,18 @@ void binlog_reset_cache(THD *thd)
25402545
}
25412546

25422547

2548+
void binlog_clear_incident(THD *thd)
2549+
{
2550+
binlog_cache_mngr *const cache_mngr= opt_bin_log ?
2551+
(binlog_cache_mngr*) thd_get_ha_data(thd, binlog_hton) : 0;
2552+
if (cache_mngr)
2553+
{
2554+
cache_mngr->stmt_cache.clear_incident();
2555+
cache_mngr->trx_cache.clear_incident();
2556+
}
2557+
}
2558+
2559+
25432560
void MYSQL_BIN_LOG::set_write_error(THD *thd, bool is_transactional)
25442561
{
25452562
DBUG_ENTER("MYSQL_BIN_LOG::set_write_error");

‎sql/log.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1186,6 +1186,7 @@ File open_binlog(IO_CACHE *log, const char *log_file_name,
11861186

11871187
void make_default_log_name(char **out, const char* log_ext, bool once);
11881188
void binlog_reset_cache(THD *thd);
1189+
void binlog_clear_incident(THD *thd);
11891190
bool write_annotated_row(THD *thd);
11901191

11911192
extern MYSQL_PLUGIN_IMPORT MYSQL_BIN_LOG mysql_bin_log;

0 commit comments

Comments
 (0)