Hi, Nikita, On May 05, Nikita Malyavin wrote: > revision-id: da5a72f32d4 (mariadb-11.0.1-114-gda5a72f32d4) > parent(s): 0d6584c019c > author: Nikita Malyavin > committer: Nikita Malyavin > timestamp: 2023-05-01 01:15:41 +0300 > message: > > 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.
s/ha_partiton/ha_partition/ > The proposed fix adds table->skip_online_logging field, preventing the > underlying engines from replicating online events. Only upper layer > engine should do this. > > table->skip_online_logging is initialized to 0 together with the table > itself. Normally it is always 0, except for the intermediate > overriding part, once it is checked in handler::binlog_log_row. > > Note on MDEV-31040 (Server crashes in MYSQL_LOG::is_open upon ALTER on > partitioned table): > First the solution was to override table->s->online_alter_binlog, and > set it to NULL during the partitioning operation, which was > ridiculously wrong: no-one is supposed to modify that field outside > ALTER! That field write access is protected by EXCLUSIVE metadata > lock. I don't think you need to list your different (non-working) attemps to fix the bug in the commit comment. Just explain the fix that works, not the fix that doesn't. > In comparison, table->skip_online_logging is a connection-local field. > > The test is a non-deterministic one. It wasn't convenient to construct > a deterministic test here, besides for the case that will never be > repeated. Is it still the case? Your test in alter_table_online_debug.test with debug_sync - is it non-deterministic? > diff --git a/sql/handler.cc b/sql/handler.cc > index 3fcbb5171d6..a7e229e8f8d 100644 > --- a/sql/handler.cc > +++ b/sql/handler.cc > @@ -7280,7 +7280,8 @@ int handler::binlog_log_row(const uchar *before_record, > log_func, row_logging_has_trans); > > #ifdef HAVE_REPLICATION > - if (unlikely(!error && table->s->online_alter_binlog)) > + if (unlikely(!error && table->s->online_alter_binlog && > + !table->skip_online_logging)) this doesn't need any juggling with skip_online_logging. This problem is already solved for, say, long uniques. You can use this == table->file condition to filter out individual partitions and have your binlog_log_row_online_alter called only for the main ha_partition handler. > error= binlog_log_row_online_alter(table, before_record, after_record, > log_func); > #endif // HAVE_REPLICATION Regards, Sergei VP of MariaDB Server Engineering and secur...@mariadb.org _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp