database/mariadb
1
0
Fork 0
mirror of https://github.com/MariaDB/server.git synced 2025-08-08 11:22:35 +03:00
Code Activity

MDEV-36906: RBR crashes upon DML after CONVERT PARTITION

Browse Source 
MDEV-33658 part 1’s refactoring ecaedbe299
introduced a new function init_key_info which (in part) aims to
calculate the total key length; however, it doesn’t account for the
key already having been initialized (as happens when called via
ALTER TABLE .. CONVERT PARTITION .. TO TABLE). This leads to crashes
when this key is later iterated over, because the iterator will try
to iterate over additional key parts which don’t exist because the
length reports as longer than the actual memory owned. The crash
reported by MDEV-36906 highlights this in function key_copy.

To explain how the keys already have been initialized, init_key_info
is called multiple times. That is, init_key_info is called from
mysql_prepare_create_table, which prepares a table and its key
structures for table creation, which is in turn called by
mysql_write_frm when using flags MFRM_WRITE_SHADOW and
MFRM_WRITE_CONVERTED_TO. The
ALTER TABLE .. CONVERT PARTITION .. TO TABLE use case (see function
fast_alter_partition_table), calls mysql_write_frm multiple times with
both of these flags set (first with MFRM_WRITE_CONVERTED_TO and then
with MFRM_WRITE_SHADOW).

Raising it up a level, mysql_prepare_create_table doesn't need to be
called again after it has already been invoked when just writing frms.
Init_key_info is the only place in that function which leads to side
effects, but the rest is redundant and can be skipped on the second
call (i.e. when writing the shadow).

The patch fixes this by skipping the call to mysql_prepare_create_table
in mysql_write_frm in the MFRM_WRITE_SHADOW block when it has already
been called previously. To track whether or not it has been previously
called, we add a new flag for the mysql_write_frm function,
MFRM_ALTER_INFO_PREPARED, which is hard-coded into the function call on
the later invocation.

Test case based on work by Elena Stepanova <elenst@mariadb.com>

Reviewed By:
============
Sergei Golubchik <serg@mariadb.org>
Nikita Malyavin <nikita.malyavin@mariadb.com>
This commit is contained in:
Brandon Nesterenko
2025-07-08 15:53:18 -06:00
committed by Brandon Nesterenko
parent 33e845595d
commit 5f51a3a6eb
5 changed files with 239 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

80
mysql-test/suite/rpl/r/rpl_alter_convert_partition.result Normal file
View File

@@ -0,0 +1,80 @@
include/master-slave.inc
[connection master]
#
# Ensure initial CREATE TABLE with partitioned data is replicated
# correctly
connection master;
create table t (a int, b int, key(a)) engine=innodb partition by range (b) (partition p1 values less than (10), partition pn values less than (maxvalue));
insert into t values (1,5),(2,100);
include/save_master_gtid.inc
connection slave;
include/sync_with_master_gtid.inc
include/diff_tables.inc [master:test.t,slave:test.t]
#
# Ensure ALTER TABLE .. CONVERT PARTITION .. TO TABLE replicates
# correctly
connection master;
alter table t convert partition p1 to table offspring;
include/save_master_gtid.inc
connection slave;
include/sync_with_master_gtid.inc
include/diff_tables.inc [master:test.t,slave:test.t]
include/diff_tables.inc [master:test.offspring,slave:test.offspring]
#
# Ensure data can be inserted into existing table after
# ALTER TABLE .. CONVERT PARTITION .. TO TABLE
connection master;
insert into t values (3, 6);
include/save_master_gtid.inc
connection slave;
include/sync_with_master_gtid.inc
include/diff_tables.inc [master:test.t,slave:test.t]
#
# Ensure data can be inserted into offspring table after
# ALTER TABLE .. CONVERT PARTITION .. TO TABLE
connection master;
insert into offspring values (4, 101);
include/save_master_gtid.inc
connection slave;
include/sync_with_master_gtid.inc
include/diff_tables.inc [master:test.offspring,slave:test.offspring]
#
# Ensure data can be updated in existing table after
# ALTER TABLE .. CONVERT PARTITION .. TO TABLE
connection master;
update t set b=b+1 where a=3;
include/save_master_gtid.inc
connection slave;
include/sync_with_master_gtid.inc
include/diff_tables.inc [master:test.t,slave:test.t]
#
# Ensure data can be updated in offspring table after
# ALTER TABLE .. CONVERT PARTITION .. TO TABLE
connection master;
update offspring set b=b+1 where a=4;
include/save_master_gtid.inc
connection slave;
include/sync_with_master_gtid.inc
include/diff_tables.inc [master:test.offspring,slave:test.offspring]
#
# Ensure data can be deleted in existing table after
# ALTER TABLE .. CONVERT PARTITION .. TO TABLE
connection master;
delete from t;
include/save_master_gtid.inc
connection slave;
include/sync_with_master_gtid.inc
include/diff_tables.inc [master:test.t,slave:test.t]
#
# Ensure data can be deleted in offspring table after
# ALTER TABLE .. CONVERT PARTITION .. TO TABLE
connection master;
delete from offspring;
include/save_master_gtid.inc
connection slave;
include/sync_with_master_gtid.inc
include/diff_tables.inc [master:test.offspring,slave:test.offspring]
connection master;
drop table t, offspring;
include/rpl_end.inc
# End of rpl_alter_convert_partition

117
mysql-test/suite/rpl/t/rpl_alter_convert_partition.test Normal file
View File

@@ -0,0 +1,117 @@
#
# This test ensures that ALTER TABLE ... CONVERT PARTITION ... TO TABLE
# works with replication. I.e., the partitioning is done correctly, and
# after partitioning, both tables can be updated correctly.
#
# References:
# MDEV-36906: RBR crashes upon DML after CONVERT PARTITION
#
--source include/have_innodb.inc
--source include/have_partition.inc
--source include/master-slave.inc
--echo #
--echo # Ensure initial CREATE TABLE with partitioned data is replicated
--echo # correctly
--connection master
create table t (a int, b int, key(a)) engine=innodb partition by range (b) (partition p1 values less than (10), partition pn values less than (maxvalue));
insert into t values (1,5),(2,100);
--source include/save_master_gtid.inc
--connection slave
--source include/sync_with_master_gtid.inc
--let $diff_tables=master:test.t,slave:test.t
--source include/diff_tables.inc
--echo #
--echo # Ensure ALTER TABLE .. CONVERT PARTITION .. TO TABLE replicates
--echo # correctly
--connection master
alter table t convert partition p1 to table offspring;
--source include/save_master_gtid.inc
--connection slave
--source include/sync_with_master_gtid.inc
--let $diff_tables=master:test.t,slave:test.t
--source include/diff_tables.inc
--let $diff_tables=master:test.offspring,slave:test.offspring
--source include/diff_tables.inc
--echo #
--echo # Ensure data can be inserted into existing table after
--echo # ALTER TABLE .. CONVERT PARTITION .. TO TABLE
--connection master
insert into t values (3, 6);
--source include/save_master_gtid.inc
--connection slave
--source include/sync_with_master_gtid.inc
--let $diff_tables=master:test.t,slave:test.t
--source include/diff_tables.inc
--echo #
--echo # Ensure data can be inserted into offspring table after
--echo # ALTER TABLE .. CONVERT PARTITION .. TO TABLE
--connection master
insert into offspring values (4, 101);
--source include/save_master_gtid.inc
--connection slave
--source include/sync_with_master_gtid.inc
--let $diff_tables=master:test.offspring,slave:test.offspring
--source include/diff_tables.inc
--echo #
--echo # Ensure data can be updated in existing table after
--echo # ALTER TABLE .. CONVERT PARTITION .. TO TABLE
--connection master
update t set b=b+1 where a=3;
--source include/save_master_gtid.inc
--connection slave
--source include/sync_with_master_gtid.inc
--let $diff_tables=master:test.t,slave:test.t
--source include/diff_tables.inc
--echo #
--echo # Ensure data can be updated in offspring table after
--echo # ALTER TABLE .. CONVERT PARTITION .. TO TABLE
--connection master
update offspring set b=b+1 where a=4;
--source include/save_master_gtid.inc
--connection slave
--source include/sync_with_master_gtid.inc
--let $diff_tables=master:test.offspring,slave:test.offspring
--source include/diff_tables.inc
--echo #
--echo # Ensure data can be deleted in existing table after
--echo # ALTER TABLE .. CONVERT PARTITION .. TO TABLE
--connection master
delete from t;
--source include/save_master_gtid.inc
--connection slave
--source include/sync_with_master_gtid.inc
--let $diff_tables=master:test.t,slave:test.t
--source include/diff_tables.inc
--echo #
--echo # Ensure data can be deleted in offspring table after
--echo # ALTER TABLE .. CONVERT PARTITION .. TO TABLE
--connection master
delete from offspring;
--source include/save_master_gtid.inc
--connection slave
--source include/sync_with_master_gtid.inc
--let $diff_tables=master:test.offspring,slave:test.offspring
--source include/diff_tables.inc
--connection master
drop table t, offspring;
--source include/rpl_end.inc
--echo # End of rpl_alter_convert_partition

2
sql/sql_partition.cc
View File

@@ -7626,7 +7626,7 @@ uint fast_alter_partition_table(THD *thd, TABLE *table,
ERROR_INJECT("convert_partition_1") ||
write_log_drop_shadow_frm(lpt) ||
ERROR_INJECT("convert_partition_2") ||
mysql_write_frm(lpt, WFRM_WRITE_SHADOW) ||
mysql_write_frm(lpt, WFRM_WRITE_SHADOW|WFRM_ALTER_INFO_PREPARED) ||
ERROR_INJECT("convert_partition_3") ||
wait_while_table_is_used(thd, table, HA_EXTRA_NOT_USED) ||
ERROR_INJECT("convert_partition_4") ||

46
sql/sql_table.cc
View File

@@ -721,16 +721,30 @@ uint build_table_shadow_filename(char *buff, size_t bufflen,
lpt Struct carrying many parameters needed for this
method
flags Flags as defined below
WFRM_INITIAL_WRITE If set we need to prepare table before
creating the frm file
WFRM_INSTALL_SHADOW If set we should install the new frm
WFRM_KEEP_SHARE If set we know that the share is to be
WFRM_WRITE_SHADOW If set, we need to prepare the table before
creating the frm file. Note it is possible that
mysql_write_frm was already called with
WFRM_WRITE_CONVERTED_TO, which would have
already called mysql_prepare_create_table, in
which case, we can skip that specific step in
the preparation.
WFRM_INSTALL_SHADOW If set, we should install the new frm
WFRM_KEEP_SHARE If set, we know that the share is to be
retained and thus we should ensure share
object is correct, if not set we don't
set the new partition syntax string since
we know the share object is destroyed.
WFRM_PACK_FRM If set we should pack the frm file and delete
the frm file
WFRM_WRITE_CONVERTED_TO Similar to WFRM_WRITE_SHADOW but for
ALTER TABLE ... CONVERT PARTITION .. TO TABLE,
i.e., we need to prepare the table before
creating the frm file. Though in this case,
mysql_write_frm will be called again with
WFRM_WRITE_SHADOW, where the
prepare_create_table step will be skipped.
WFRM_BACKUP_ORIGINAL If set, will back up the existing frm file
before creating the new frm file.
WFRM_ALTER_INFO_PREPARED If set, the prepare_create_table step should be
skipped when WFRM_WRITE_SHADOW is set.
RETURN VALUES
TRUE Error
@@ -775,7 +789,15 @@ bool mysql_write_frm(ALTER_PARTITION_PARAM_TYPE *lpt, uint flags)
strxmov(shadow_frm_name, shadow_path, reg_ext, NullS);
if (flags & WFRM_WRITE_SHADOW)
{
if (mysql_prepare_create_table(lpt->thd, lpt->create_info, lpt->alter_info,
/*
It is possible mysql_prepare_create_table was already called in our
create/alter_info context and we don't need to call it again. That is, if
in the context of `ALTER TABLE ... CONVERT PARTITION .. TO TABLE` then
mysql_prepare_create_table would have already been called through a prior
invocation of mysql_write_frm with flag MFRM_WRITE_CONVERTED_TO.
*/
if (!(flags & WFRM_ALTER_INFO_PREPARED) &&
mysql_prepare_create_table(lpt->thd, lpt->create_info, lpt->alter_info,
&lpt->db_options, lpt->table->file,
&lpt->key_info_buffer, &lpt->key_count,
C_ALTER_TABLE))
@@ -850,6 +872,11 @@ bool mysql_write_frm(ALTER_PARTITION_PARAM_TYPE *lpt, uint flags)
ERROR_INJECT("create_before_create_frm"))
DBUG_RETURN(TRUE);
/*
For WFRM_WRITE_CONVERTED_TO, we always need to call
mysql_prepare_create_table
*/
DBUG_ASSERT(!(flags & WFRM_ALTER_INFO_PREPARED));
if (mysql_prepare_create_table(thd, create_info, lpt->alter_info,
&lpt->db_options, file,
&lpt->key_info_buffer, &lpt->key_count,
@@ -3004,6 +3031,11 @@ my_bool init_key_info(THD *thd, Alter_info *alter_info,
for (Key &key: alter_info->key_list)
{
/*
Ensure we aren't re-initializing keys that were already initialized.
*/
DBUG_ASSERT(!key.length);
if (key.type == Key::FOREIGN_KEY)
continue;

2
sql/sql_table.h
View File

@@ -55,11 +55,13 @@ enum enum_explain_filename_mode
/* depends on errmsg.txt Database `db`, Table `t` ... */
#define EXPLAIN_FILENAME_MAX_EXTRA_LENGTH 63
/* See mysql_write_frm function comment for explanations of these flags */
#define WFRM_WRITE_SHADOW 1
#define WFRM_INSTALL_SHADOW 2
#define WFRM_KEEP_SHARE 4
#define WFRM_WRITE_CONVERTED_TO 8
#define WFRM_BACKUP_ORIGINAL 16
#define WFRM_ALTER_INFO_PREPARED 32
/* Flags for conversion functions. */
static const uint FN_FROM_IS_TMP= 1 << 0;