Skip to content

Commit c76072d

Browse files
FooBarriorvuvova
authored andcommitted
MDEV-31033 ER_KEY_NOT_FOUND upon online COPY ALTER on a partitioned table
The row events were applied "twice": once for the ha_partition, and one more time for the underlying storage engine. There's no such problem in binlog/rpl, because ha_partiton::row_logging is normally set to false. The fix makes the events replicate only when the handler is a root handler. We will try to *guess* this by comparing it to table->file. The same approach is used in the MDEV-21540 fix, 231feab. The assumption is made, that the row methods are only called for table->file (and never for a cloned handler), hence the assertions are added in ha_innobase and ha_myisam to make sure that this is true at least for those engines Also closes MDEV-31040, however the test is not included, since we have no convenient way to construct a deterministic version.
1 parent 0775c7b commit c76072d

File tree

5 files changed

+68
-21
lines changed

5 files changed

+68
-21
lines changed

mysql-test/main/alter_table_online_debug.result

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1231,6 +1231,27 @@ a
12311231
223
12321232
889
12331233
drop table t1;
1234+
#
1235+
# MDEV-31033 ER_KEY_NOT_FOUND upon online COPY ALTER on a partitioned
1236+
# table
1237+
create table t (a int) partition by hash(a) partitions 2;
1238+
insert into t values (1),(3);
1239+
set debug_sync= 'alter_table_online_downgraded SIGNAL downgraded wait_for goon';
1240+
alter table t force, algorithm=copy, lock=none;
1241+
connection con1;
1242+
set debug_sync= 'now WAIT_FOR downgraded';
1243+
update t set a = a + 1;
1244+
insert t values (1),(2);
1245+
delete from t where a = 4 limit 1;
1246+
set debug_sync= 'now SIGNAL goon';
1247+
connection default;
1248+
select * from t;
1249+
a
1250+
2
1251+
2
1252+
1
1253+
drop table t;
1254+
set debug_sync= reset;
12341255
disconnect con1;
12351256
#
12361257
# End of 11.2 tests

mysql-test/main/alter_table_online_debug.test

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1418,6 +1418,29 @@ select * from t1;
14181418

14191419
drop table t1;
14201420

1421+
--echo #
1422+
--echo # MDEV-31033 ER_KEY_NOT_FOUND upon online COPY ALTER on a partitioned
1423+
--echo # table
1424+
create table t (a int) partition by hash(a) partitions 2;
1425+
insert into t values (1),(3);
1426+
set debug_sync= 'alter_table_online_downgraded SIGNAL downgraded wait_for goon';
1427+
send alter table t force, algorithm=copy, lock=none;
1428+
1429+
--connection con1
1430+
set debug_sync= 'now WAIT_FOR downgraded';
1431+
update t set a = a + 1;
1432+
insert t values (1),(2);
1433+
delete from t where a = 4 limit 1;
1434+
set debug_sync= 'now SIGNAL goon';
1435+
1436+
--connection default
1437+
--reap
1438+
1439+
select * from t;
1440+
1441+
drop table t;
1442+
1443+
set debug_sync= reset;
14211444
--disconnect con1
14221445
--echo #
14231446
--echo # End of 11.2 tests

sql/handler.cc

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7290,7 +7290,7 @@ int handler::binlog_log_row(const uchar *before_record,
72907290
log_func, row_logging_has_trans);
72917291

72927292
#ifdef HAVE_REPLICATION
7293-
if (unlikely(!error && table->s->online_alter_binlog))
7293+
if (unlikely(!error && table->s->online_alter_binlog && is_root_handler()))
72947294
error= binlog_log_row_online_alter(table, before_record, after_record,
72957295
log_func);
72967296
#endif // HAVE_REPLICATION
@@ -7815,18 +7815,7 @@ int handler::ha_write_row(const uchar *buf)
78157815
if ((error= ha_check_overlaps(NULL, buf)))
78167816
DBUG_RETURN(error);
78177817

7818-
/*
7819-
NOTE: this != table->file is true in 3 cases:
7820-
7821-
1. under copy_partitions() (REORGANIZE PARTITION): that does not
7822-
require long unique check as it does not introduce new rows or new index.
7823-
2. under partition's ha_write_row() (INSERT): check_duplicate_long_entries()
7824-
was already done by ha_partition::ha_write_row(), no need to check it
7825-
again for each single partition.
7826-
3. under ha_mroonga::wrapper_write_row()
7827-
*/
7828-
7829-
if (table->s->long_unique_table && this == table->file)
7818+
if (table->s->long_unique_table && is_root_handler())
78307819
{
78317820
DBUG_ASSERT(inited == NONE || lookup_handler != this);
78327821
if ((error= check_duplicate_long_entries(buf)))
@@ -7877,14 +7866,7 @@ int handler::ha_update_row(const uchar *old_data, const uchar *new_data)
78777866
uint saved_status= table->status;
78787867
error= ha_check_overlaps(old_data, new_data);
78797868

7880-
/*
7881-
NOTE: this != table->file is true under partition's ha_update_row():
7882-
check_duplicate_long_entries_update() was already done by
7883-
ha_partition::ha_update_row(), no need to check it again for each single
7884-
partition. Same applies to ha_mroonga wrapper.
7885-
*/
7886-
7887-
if (!error && table->s->long_unique_table && this == table->file)
7869+
if (!error && table->s->long_unique_table && is_root_handler())
78887870
error= check_duplicate_long_entries_update(new_data);
78897871
table->status= saved_status;
78907872

sql/handler.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3557,6 +3557,11 @@ class handler :public Sql_alloc
35573557
if (org_keyread != MAX_KEY)
35583558
ha_start_keyread(org_keyread);
35593559
}
3560+
3561+
protected:
3562+
bool is_root_handler() const;
3563+
3564+
public:
35603565
int check_collation_compatibility();
35613566
int check_long_hash_compatibility() const;
35623567
int ha_check_for_upgrade(HA_CHECK_OPT *check_opt);

sql/sql_class.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7561,6 +7561,22 @@ inline bool handler::has_long_unique()
75617561
return table->s->long_unique_table;
75627562
}
75637563

7564+
/**
7565+
Return whether the handler is root.
7566+
@return false if table is maintained by different handlerton, true otherwise.
7567+
@note The implementation supposes that the same handler can't be found as both
7568+
root and non-root.
7569+
7570+
There are two known cases when it's non-root:
7571+
1. under partition's ha_write_row() (also true for copy_partitions())
7572+
2. under ha_mroonga::wrapper_write_row();
7573+
same applies for ha_delete_row/ha_update_row.
7574+
*/
7575+
inline bool handler::is_root_handler() const
7576+
{
7577+
return ht == table->file->ht;
7578+
}
7579+
75647580
extern pthread_attr_t *get_connection_attrib(void);
75657581

75667582
/**

0 commit comments

Comments
 (0)