1berry/MariaDB-server
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

MDEV-35207 ignored error at binlogging by CREATE-TABLE-SELECT leads to assert

Browse Source 
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.
This commit is contained in:
Andrei Elkin 2024-11-25 19:48:23 +02:00
parent 58a3677309
commit 1b4efbeb8c
5 changed files with 374 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

158
mysql-test/suite/rpl/r/rpl_create_select_row.result Normal file
View File

@ -0,0 +1,158 @@
include/master-slave.inc
[connection master]
connection master;
set @max_binlog_cache_size = @@global.max_binlog_cache_size;
set @binlog_cache_size = @@global.binlog_cache_size;
set @@global.max_binlog_cache_size = 4096;
set @@global. binlog_cache_size = 4096;
#
# MDEV-35207 ignored error at binlogging by CREATE-TABLE-SELECT leads to assert
#
connect conn_err,localhost,root,,;
call mtr.add_suppression("Multi-statement transaction required more than 'max_binlog_cache_size' bytes of storage");
create table t engine=myisam select repeat ('a',4096*3) AS a;
ERROR HY000: Multi-statement transaction required more than 'max_binlog_cache_size' bytes of storage; increase this mariadbd variable and try again
create table t engine=innodb select repeat ('a',4096*3) AS a;
ERROR HY000: Multi-statement transaction required more than 'max_binlog_cache_size' bytes of storage; increase this mariadbd variable and try again
create table t (a int unique, b char) select 1 AS a, 'b' as b union select 1 as a, 'c' as b;
ERROR 23000: Duplicate entry '1' for key 'a'
select * from t;
ERROR 42S02: Table 'test.t' doesn't exist
disconnect conn_err;
connection master;
#
# MDEV-35499 errored CREATE-OR-REPLACE-SELECT does not DROP table in binlog
#
#
# Engine = innodb
#
set statement binlog_format=statement for create table t (a int) select 1 as a;
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;
ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
select * from t;
ERROR 42S02: Table 'test.t' doesn't exist
#
# Prove an expected lonely `DROP table t'
include/show_binlog_events.inc
Log_name Pos Event_type Server_id End_log_pos Info
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS `test`.`t`/* Generated to handle failed CREATE OR REPLACE */
master-bin.000001 # Query # # ROLLBACK
set statement binlog_format=statement for create table t (a int) select 1 as a;
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;
ERROR HY000: Multi-statement transaction required more than 'max_binlog_cache_size' bytes of storage; increase this mariadbd variable and try again
select * from t;
ERROR 42S02: Table 'test.t' doesn't exist
#
# Prove an expected lonely `DROP table t'
include/show_binlog_events.inc
Log_name Pos Event_type Server_id End_log_pos Info
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS `test`.`t`/* Generated to handle failed CREATE OR REPLACE */
master-bin.000001 # Query # # ROLLBACK
set statement binlog_format=statement for create table t (a int) select 1 as a;
set statement binlog_format=row for create or replace table t (a text) engine=innodb select repeat ('a',4096*3) AS a;;
ERROR HY000: Multi-statement transaction required more than 'max_binlog_cache_size' bytes of storage; increase this mariadbd variable and try again
select * from t;
ERROR 42S02: Table 'test.t' doesn't exist
#
# Prove an expected lonely `DROP table t'
include/show_binlog_events.inc
Log_name Pos Event_type Server_id End_log_pos Info
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS `test`.`t`/* Generated to handle failed CREATE OR REPLACE */
master-bin.000001 # Query # # ROLLBACK
#
# Engine = myisam
#
set statement binlog_format=statement for create table t (a int) select 1 as a;
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;
ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
select * from t;
ERROR 42S02: Table 'test.t' doesn't exist
#
# Prove an expected lonely `DROP table t'
include/show_binlog_events.inc
Log_name Pos Event_type Server_id End_log_pos Info
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS `test`.`t`/* Generated to handle failed CREATE OR REPLACE */
master-bin.000001 # Query # # ROLLBACK
set statement binlog_format=statement for create table t (a int) select 1 as a;
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;
ERROR HY000: Multi-statement transaction required more than 'max_binlog_cache_size' bytes of storage; increase this mariadbd variable and try again
select * from t;
ERROR 42S02: Table 'test.t' doesn't exist
#
# Prove an expected lonely `DROP table t'
include/show_binlog_events.inc
Log_name Pos Event_type Server_id End_log_pos Info
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS `test`.`t`/* Generated to handle failed CREATE OR REPLACE */
master-bin.000001 # Query # # ROLLBACK
set statement binlog_format=statement for create table t (a int) select 1 as a;
set statement binlog_format=row for create or replace table t (a text) engine=myisam select repeat ('a',4096*3) AS a;;
ERROR HY000: Multi-statement transaction required more than 'max_binlog_cache_size' bytes of storage; increase this mariadbd variable and try again
select * from t;
ERROR 42S02: Table 'test.t' doesn't exist
#
# Prove an expected lonely `DROP table t'
include/show_binlog_events.inc
Log_name Pos Event_type Server_id End_log_pos Info
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS `test`.`t`/* Generated to handle failed CREATE OR REPLACE */
master-bin.000001 # Query # # ROLLBACK
create table ti_pk (a int primary key) engine=innodb;
create table ta (a int) engine=aria;
create function f_ia(arg int)
returns integer
begin
insert into ti_pk set a=1;
insert into ta set a=1;
insert into ti_pk set a=arg;
return 1;
end |
set statement binlog_format = ROW for create table t_y (a int) engine=aria select f_ia(1 /* err */) as a;
ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
select * from t_y;
ERROR 42S02: Table 'test.t_y' doesn't exist
# correct execution: `ta` is modified and its new record is binlogged
include/show_binlog_events.inc
Log_name Pos Event_type Server_id End_log_pos Info
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
master-bin.000001 # Table_map # # table_id: # (test.ta)
master-bin.000001 # Write_rows_v1 # # table_id: # flags: STMT_END_F
master-bin.000001 # Query # # COMMIT
select * from ta;
a
1
select * from ti_pk;
a
connection slave;
include/diff_tables.inc [master:ta,slave:ta]
connection master;
delete from ta;
connection slave;
connection master;
set statement binlog_format = STATEMENT for create table t_y (a int) engine=aria select f_ia(1 /* err */) as a;
ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
select * from t_y;
ERROR 42S02: Table 'test.t_y' doesn't exist
# ***TODO: fix MDEV-36027***. As of now `ta` is modified but that's not binlogged
include/show_binlog_events.inc
select *,'on_master' from ta;
a on_master
1 on_master
select * from ti_pk;
a
connection slave;
select *,'on_slave' from ta;
a on_slave
connection master;
drop function f_ia;
drop table ti_pk, ta;
SET @@global.max_binlog_cache_size = @max_binlog_cache_size;
SET @@global. binlog_cache_size = @binlog_cache_size;
connection slave;
End of the tests
include/rpl_end.inc

161
mysql-test/suite/rpl/t/rpl_create_select_row.test Normal file
View File

@ -0,0 +1,161 @@
--source include/have_binlog_format_row.inc
--source include/have_innodb.inc
--source include/master-slave.inc
--connection master
set @max_binlog_cache_size = @@global.max_binlog_cache_size;
set @binlog_cache_size = @@global.binlog_cache_size;
set @@global.max_binlog_cache_size = 4096;
set @@global. binlog_cache_size = 4096;
--echo #
--echo # MDEV-35207 ignored error at binlogging by CREATE-TABLE-SELECT leads to assert
--echo #
# fix the current (write) binlog position
--let $binlog_file_0= query_get_value(SHOW MASTER STATUS, File, 1)
--let $binlog_start_0 = query_get_value(SHOW MASTER STATUS, Position, 1)
# use a separate connection also to validate its close will be clean
connect (conn_err,localhost,root,,);
call mtr.add_suppression("Multi-statement transaction required more than 'max_binlog_cache_size' bytes of storage");
--error ER_TRANS_CACHE_FULL
create table t engine=myisam select repeat ('a',4096*3) AS a;
--error ER_TRANS_CACHE_FULL
create table t engine=innodb select repeat ('a',4096*3) AS a;
--error ER_DUP_ENTRY
create table t (a int unique, b char) select 1 AS a, 'b' as b union select 1 as a, 'c' as b;
--error ER_NO_SUCH_TABLE
select * from t;
--disconnect conn_err
--connection master
--let $binlog_file_1= query_get_value(SHOW MASTER STATUS, File, 1)
--let $binlog_start_1= query_get_value(SHOW MASTER STATUS, Position, 1)
--let $cmp = `select strcmp('$binlog_file_1', '$binlog_file_0') <> 0 OR $binlog_start_1 <> $binlog_start_0`
if (!$cmp)
{
--echo *** Error: unexpected advance of binlog position
--die
}
--echo
--echo #
--echo # MDEV-35499 errored CREATE-OR-REPLACE-SELECT does not DROP table in binlog
--echo #
--let $i = 2
while ($i)
{
--let $engine=`select if($i % 2, "myisam", "innodb")`
--echo #
--echo # Engine = $engine
--echo #
set statement binlog_format=statement for create table t (a int) select 1 as a;
--let $binlog_file= query_get_value(SHOW MASTER STATUS, File, 1)
--let $binlog_start = query_get_value(SHOW MASTER STATUS, Position, 1)
--error ER_DUP_ENTRY
--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
--error ER_NO_SUCH_TABLE
select * from t;
--echo #
--echo # Prove an expected lonely `DROP table t'
--source include/show_binlog_events.inc
# error before stmt commit
set statement binlog_format=statement for create table t (a int) select 1 as a;
--let $binlog_file= query_get_value(SHOW MASTER STATUS, File, 1)
--let $binlog_start = query_get_value(SHOW MASTER STATUS, Position, 1)
--error ER_TRANS_CACHE_FULL
--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
--error ER_NO_SUCH_TABLE
select * from t;
--echo #
--echo # Prove an expected lonely `DROP table t'
--source include/show_binlog_events.inc
# error at stmt commit
set statement binlog_format=statement for create table t (a int) select 1 as a;
--let $binlog_file= query_get_value(SHOW MASTER STATUS, File, 1)
--let $binlog_start = query_get_value(SHOW MASTER STATUS, Position, 1)
--error ER_TRANS_CACHE_FULL
--eval set statement binlog_format=row for create or replace table t (a text) engine=$engine select repeat ('a',4096*3) AS a;
--error ER_NO_SUCH_TABLE
select * from t;
--echo #
--echo # Prove an expected lonely `DROP table t'
--source include/show_binlog_events.inc
--dec $i
}
# Tests of mixed engines to demonstrate non-transaction table updates
# are binlogged or otherwise MDEV-36027.
create table ti_pk (a int primary key) engine=innodb;
create table ta (a int) engine=aria;
delimiter |;
create function f_ia(arg int)
returns integer
begin
insert into ti_pk set a=1;
insert into ta set a=1;
insert into ti_pk set a=arg;
return 1;
end |
delimiter ;|
--let $binlog_file= query_get_value(SHOW MASTER STATUS, File, 1)
--let $binlog_start = query_get_value(SHOW MASTER STATUS, Position, 1)
--error ER_DUP_ENTRY
set statement binlog_format = ROW for create table t_y (a int) engine=aria select f_ia(1 /* err */) as a;
--error ER_NO_SUCH_TABLE
select * from t_y;
--echo # correct execution: `ta` is modified and its new record is binlogged
--source include/show_binlog_events.inc
select * from ta;
select * from ti_pk;
--sync_slave_with_master
--let $diff_tables=master:ta,slave:ta
--source include/diff_tables.inc
--connection master
delete from ta;
--sync_slave_with_master
--connection master
# MDEV-36027 Errored-out CREATE-SELECT does not binlog results of any function modifying non-transactional table
--let $binlog_file= query_get_value(SHOW MASTER STATUS, File, 1)
--let $binlog_start = query_get_value(SHOW MASTER STATUS, Position, 1)
--error ER_DUP_ENTRY
set statement binlog_format = STATEMENT for create table t_y (a int) engine=aria select f_ia(1 /* err */) as a;
--error ER_NO_SUCH_TABLE
select * from t_y;
--echo # ***TODO: fix MDEV-36027***. As of now `ta` is modified but that's not binlogged
--source include/show_binlog_events.inc
select *,'on_master' from ta;
select * from ti_pk;
--sync_slave_with_master
select *,'on_slave' from ta;
# Cleanup
--connection master
drop function f_ia;
drop table ti_pk, ta;
SET @@global.max_binlog_cache_size = @max_binlog_cache_size;
SET @@global. binlog_cache_size = @binlog_cache_size;
# test that binlog replicates correctly to slave
# --connection slave
--sync_slave_with_master
--echo End of the tests
--source include/rpl_end.inc

17
sql/log.cc
View File

@ -322,6 +322,11 @@ public:
incident= TRUE;
}
void clear_incident(void)
{
incident= FALSE;
}
bool has_incident(void)
{
return(incident);
@ -2540,6 +2545,18 @@ void binlog_reset_cache(THD *thd)
}
void binlog_clear_incident(THD *thd)
{
binlog_cache_mngr *const cache_mngr= opt_bin_log ?
(binlog_cache_mngr*) thd_get_ha_data(thd, binlog_hton) : 0;
if (cache_mngr)
{
cache_mngr->stmt_cache.clear_incident();
cache_mngr->trx_cache.clear_incident();
}
}
void MYSQL_BIN_LOG::set_write_error(THD *thd, bool is_transactional)
{
DBUG_ENTER("MYSQL_BIN_LOG::set_write_error");

1
sql/log.h
View File

@ -1186,6 +1186,7 @@ File open_binlog(IO_CACHE *log, const char *log_file_name,
void make_default_log_name(char **out, const char* log_ext, bool once);
void binlog_reset_cache(THD *thd);
void binlog_clear_incident(THD *thd);
bool write_annotated_row(THD *thd);
extern MYSQL_PLUGIN_IMPORT MYSQL_BIN_LOG mysql_bin_log;

40
sql/sql_insert.cc
View File

@ -4349,7 +4349,11 @@ bool select_insert::store_values(List<Item> &values)
bool select_insert::prepare_eof()
{
int error;
bool const trans_table= table->file->has_transactions_and_rollback();
// make sure any ROW format pending event is logged in the same binlog cache
bool const trans_table= (thd->is_current_stmt_binlog_format_row() &&
table->file->row_logging) ?
table->file->row_logging_has_trans :
table->file->has_transactions_and_rollback();
bool changed;
bool binary_logged= 0;
killed_state killed_status= thd->killed;
@ -4574,7 +4578,8 @@ void select_insert::abort_result_set()
query_cache_invalidate3(thd, table, 1);
}
DBUG_ASSERT(transactional_table || !changed ||
thd->transaction->stmt.modified_non_trans_table);
(thd->transaction->stmt.modified_non_trans_table ||
thd->transaction->all.modified_non_trans_table));
table->s->table_creation_was_logged|= binary_logged;
table->file->ha_release_auto_increment();
@ -5267,9 +5272,14 @@ bool select_create::send_eof()
/* Remember xid's for the case of row based logging */
ddl_log_update_xid(&ddl_log_state_create, thd->binlog_xid);
ddl_log_update_xid(&ddl_log_state_rm, thd->binlog_xid);
trans_commit_stmt(thd);
if (!(thd->variables.option_bits & OPTION_GTID_BEGIN))
trans_commit_implicit(thd);
if (trans_commit_stmt(thd) ||
(!(thd->variables.option_bits & OPTION_GTID_BEGIN) &&
trans_commit_implicit(thd)))
{
abort_result_set();
DBUG_RETURN(true);
}
thd->binlog_xid= 0;
#ifdef WITH_WSREP
@ -5389,7 +5399,13 @@ void select_create::abort_result_set()
/* possible error of writing binary log is ignored deliberately */
(void) thd->binlog_flush_pending_rows_event(TRUE, TRUE);
/*
In the error case, we remove any partially created table. So clear any
incident event generates due to cache error, as it no longer relevant.
*/
binlog_clear_incident(thd);
bool drop_table_was_logged= false;
if (table)
{
bool tmp_table= table->s->tmp_table;
@ -5436,6 +5452,7 @@ void select_create::abort_result_set()
create_info->db_type == partition_hton,
&create_info->tabledef_version,
tmp_table);
drop_table_was_logged= true;
debug_crash_here("ddl_log_create_after_binlog");
thd->binlog_xid= 0;
}
@ -5459,9 +5476,22 @@ void select_create::abort_result_set()
ddl_log_complete(&ddl_log_state_create);
if (create_info->table_was_deleted)
{
if (drop_table_was_logged)
{
/* for DROP binlogging the error status has to be canceled first */
Diagnostics_area new_stmt_da(thd->query_id, false, true);
Diagnostics_area *old_stmt_da= thd->get_stmt_da();
thd->set_stmt_da(&new_stmt_da);
(void) trans_rollback_stmt(thd);
thd->set_stmt_da(old_stmt_da);
}
else
{
/* Unlock locked table that was dropped by CREATE. */
(void) trans_rollback_stmt(thd);
}
thd->locked_tables_list.unlock_locked_table(thd, create_info->mdl_ticket);
}