Details
-
Bug
-
Status: Closed (View Workflow)
-
Critical
-
Resolution: Fixed
-
10.6
-
None
Description
Currently execution of
commit in one phase proceeds to commit by engines when binlog_commit() does not succeed.
There are two issues with that:
1. absence of binlog_rollback() or lower-level `binlog_cache_data::reset()` along the following execution of the failing statement eventually will raise an assert on non-empty binlog cache;
2. engines, including ones that are rollback capable, commit in this particular error situation.
P.1,2 can be verified with the following mtr script
source include/have_innodb.inc;
|
source include/have_log_bin.inc;
|
|
|
create table ta (a int) engine=aria; |
create table ti (a int) engine=innodb; |
delimiter |;
|
|
|
create function f_ia()
|
returns integer
|
begin
|
insert into ti set a=1; |
insert into ta set a=1; |
return 1; |
end |
|
delimiter ;|
|
|
|
set @@global.gtid_strict_mode=0; set @@session.gtid_seq_no=1; |
set @@global.gtid_strict_mode=1; |
--replace_regex /GTID 1-1-[0-9]+/GTID VALUE/ |
--error ER_GTID_STRICT_OUT_OF_ORDER
|
set statement binlog_format=row FOR create table f_x (a int) select f_ia() as a; |
|
|
# --error assert(sql/log.cc:1712(binlog_close_connection)) |
# --disconnect default |
|
Attachments
Issue Links
- relates to
-
-
- Closed
-
Activity
Sergei, salve.
I renewed the reviewed branch
63fa5d717ca...162b8ff486b HEAD -> bb-10.6-MDEV-35506
An incremental commit addresses MDEV-35207's test which managed to
slip an Incident event into binlog.
It is tackled.
serg, taking it back to myself in order to resolve an MSAN finging in `162b8ff486`.
serg: bb-10.6-MDEV-35506 is updated with your notes addressed. Tests are extended to involve a stored function with side effect of modified non-transactional engines (which resulted in a new bug opened, sadly not easy to fix though hopefully not urgent either).
Howdy Sergei.
While I offer a patch that tries to undo as much as it's possible I am not really sure that's the way to go. My doubt remains as it's one-phase commit, we may stick to any resolution
.
that satisfy the assert of p.1 description - so a binlog_cache_data::reset() could be done instead.
Anyway my sympathy lies with the patch