MDEV-32614 LeakSanitizer errors in copy_data_between_tables

The memory leak occurs on error when backup_reset_alter_copy_lock fails
with timeout. This leads to the alter rollback, but flush_unused is not
called.

Move table flushing on error handling to a single place and mind more
possible failures this time.
This commit is contained in:
Nikita Malyavin 2024-01-02 18:45:43 +01:00
parent c2e16b3ad5
commit 50095046f3
3 changed files with 78 additions and 19 deletions

View File

@ -1807,6 +1807,28 @@ a b
456 NULL
789 NULL
drop table t1;
# MDEV-32614 LeakSanitizer errors in copy_data_between_tables
create table t (a int, b int) engine=aria;
insert into t select seq, seq from seq_1_to_5;
backup stage start;
connect con_lock,localhost,root,,;
set lock_wait_timeout= 1;
set debug_sync='copy_data_between_tables_before_reset_backup_lock wait_for continue';
alter table t add index (b), algorithm=copy, lock=none;
connection default;
backup stage block_commit;
set debug_sync='now signal continue';
connection con_lock;
ERROR HY000: Lock wait timeout exceeded; try restarting transaction
set debug_sync='copy_data_between_tables_before_reset_backup_lock wait_for continue';
alter table t add index (a), algorithm=copy, lock=none;
connection default;
backup stage end;
set debug_sync='now signal continue';
connection con_lock;
disconnect con_lock;
connection default;
drop table t;
set global default_storage_engine= MyISAM;
disconnect con1;
disconnect con2;

View File

@ -2066,6 +2066,41 @@ select * from t1;
drop table t1;
--echo # MDEV-32614 LeakSanitizer errors in copy_data_between_tables
create table t (a int, b int) engine=aria;
insert into t select seq, seq from seq_1_to_5;
backup stage start;
--connect (con_lock,localhost,root,,)
set lock_wait_timeout= 1;
set debug_sync='copy_data_between_tables_before_reset_backup_lock wait_for continue';
send alter table t add index (b), algorithm=copy, lock=none;
--connection default
backup stage block_commit;
set debug_sync='now signal continue';
--connection con_lock
--error ER_LOCK_WAIT_TIMEOUT
--reap
# --echo # error $mysql_errno
set debug_sync='copy_data_between_tables_before_reset_backup_lock wait_for continue';
send alter table t add index (a), algorithm=copy, lock=none;
--connection default
backup stage end;
set debug_sync='now signal continue';
--connection con_lock
--reap
--disconnect con_lock
--connection default
drop table t;
eval set global default_storage_engine= $default_storage_engine;
--disconnect con1

View File

@ -12130,6 +12130,7 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
#ifdef HAVE_REPLICATION
if (online)
{
DBUG_ASSERT(from->s->online_alter_binlog == NULL);
from->s->online_alter_binlog= new Cache_flip_event_log();
if (!from->s->online_alter_binlog)
goto err;
@ -12363,29 +12364,10 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
thd_progress_next_stage(thd);
error= online_alter_read_from_binlog(thd, &rgi, binlog, &found_count);
}
if (error)
from->s->tdc->flush_unused(1); // to free the binlog
to->pos_in_table_list= NULL; // Safety
DBUG_ASSERT(thd->lex->sql_command == saved_sql_command);
thd->lex->sql_command= saved_sql_command; // Just in case
}
else if (online) // error was on copy stage
{
/*
We can't free the resources properly now, as we can still be in
non-exclusive state. So this s->online_alter_binlog will be used
until all transactions will release it.
Once the transaction commits, it can release online_alter_binlog
by decreasing ref_count.
online_alter_binlog->ref_count can be reached 0 only once.
Proof:
If share exists, we'll always have ref_count >= 1.
Once it reaches destroy(), nobody can acquire it again,
therefore, only release() is possible at this moment.
*/
from->s->tdc->flush_unused(1); // to free the binlog
}
#endif
if (error > 0 && !from->s->tmp_table)
@ -12400,6 +12382,26 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
if (unlikely(mysql_trans_commit_alter_copy_data(thd)))
error= 1;
if (unlikely(error) && online)
{
/*
We can't free the resources properly now, as we can still be in
non-exclusive state. So this s->online_alter_binlog will be used
until all transactions will release it.
Once the transaction commits, it can release online_alter_binlog
by decreasing ref_count.
online_alter_binlog->ref_count can be reached 0 only once.
Proof:
If share exists, we'll always have ref_count >= 1.
Once it reaches destroy(), nobody can acquire it again,
therefore, only release() is possible at this moment.
Also, this will release the binlog.
*/
from->s->tdc->flush_unused(1);
}
err:
if (bulk_insert_started)