Hi again, I rewrote the solution completely, please review the following commits: 88a77860 https://github.com/MariaDB/server/commit/88a77860a8bf78bf058e3315c27d14d72eb... MDEV-31804 Assertion `thd->m_transaction_psi == __null' fails 73a677f0 https://github.com/MariaDB/server/commit/73a677f03061fa188329b8e1148cbacbe65... Cleanup: make slave_exec_mode of its enum type and pack Log_event better On Mon, 7 Aug 2023 at 19:54, Nikita Malyavin wrote:

On Mon, 7 Aug 2023 at 18:25, Sergei Golubchik wrote:

...

Hi, Nikita,

...

revision-id: 63c93f89af9 (mariadb-11.0.1-192-g63c93f89af9) parent(s): a75ba4caa51 author: Nikita Malyavin committer: Nikita Malyavin timestamp: 2023-08-05 20:17:41 +0400 message:

MDEV-31804 Assertion `thd->m_transaction_psi == __null' fails ... index 1c8d56cafd1..66b2b1ee4cc 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -1459,7 +1459,7 @@ void trans_register_ha(THD *thd, bool all, handlerton *ht_arg, ulonglong trxid) Do not register transactions in which binary log is the only

On Aug 07, Nikita Malyavin wrote: participating

...

transactional storage engine. */ - if (thd->m_transaction_psi == NULL && ht_arg->db_type != DB_TYPE_BINLOG) + if (thd->m_transaction_psi == NULL && ht_arg != binlog_hton)

it would've been better to fix it in 10.4, but if it doesn't show before online alter then ok

...

{ thd->m_transaction_psi= MYSQL_START_TRANSACTION(&thd->m_transaction_state, thd->get_xid(), trxid, thd->tx_isolation, thd->tx_read_only, diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 88302096272..944807ea422 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -65,6 +65,7 @@ #include "rpl_rli.h" #include "log.h" #include "sql_debug.h" +#include "mysql/psi/mysql_transaction.h"

#ifdef _WIN32 #include @@ -11653,6 +11654,9 @@ bool mysql_trans_commit_alter_copy_data(THD *thd)

DEBUG_SYNC(thd, "alter_table_copy_trans_commit");

+ MYSQL_COMMIT_TRANSACTION(thd->m_transaction_psi); + thd->m_transaction_psi= NULL;

I don't understand it.

ha_enable_transaction() commits the transaction it calls ha_commit_trans(). And ha_commit_trans() does the above psi manipulations. Why do you need to do that explicitly?

Also, as a cleanup, I suspect, you can remove explicit commit that follows ha_enable_transaction() in mysql_trans_commit_alter_copy_data(). Seems to be redundant.

Me neither☺ psi commit is done under if(is_real_trans) condition. That is, maybe it's not supposed to start the psi transaction at all for non-trans engines? But since there are DMLs running concurrently to ALTER TABLE, maybe binlog/replication recognizes it as needed for some "binlog transactioning"?

*Before* ALTER TABLE replication begins, replica receives a Gtid_log_event:

Thread 2 hit Hardware watchpoint 2: -location thd->m_transaction_psi

Old value = (PSI_transaction_locker *) 0x62b0001d21d8 New value = (PSI_transaction_locker *) 0x0 0x00005622d5c3c76c in trans_begin (thd=0x62b0001ce218, flags=0) at /home/nik/mariadb/sql/transaction.cc:235 235 thd->m_transaction_psi= MYSQL_START_TRANSACTION(&thd->m_transaction_state, (gdb) bt #0 0x00005622d5c3c76c in trans_begin (thd=0x62b0001ce218, flags=0) at /home/nik/mariadb/sql/transaction.cc:235 #1 0x00005622d5026c32 in Gtid_log_event::do_apply_event (this=0x616000a2a598, rgi=0x61d0002dfa80) at /home/nik/mariadb/sql/log_event_server.cc:3205 #2 0x00005622d4fe632f in Log_event::apply_event (this=0x616000a2a598, rgi=0x61d0002dfa80) at /home/nik/mariadb/sql/log_event.cc:3880 #3 0x00005622d5212a4f in apply_event_and_update_pos_apply (ev=0x616000a2a598, thd=0x62b0001ce218, rgi=0x61d0002dfa80, reason=0) at /home/nik/mariadb/sql/slave.cc:3877 #4 0x00005622d5211e72 in apply_event_and_update_pos (ev=0x616000a2a598, thd=0x62b0001ce218, rgi=0x61d0002dfa80) at /home/nik/mariadb/sql/slave.cc:4043 #5 0x00005622d522bed3 in exec_relay_log_event (thd=0x62b0001ce218, rli=0x62a0000202c0, serial_rgi=0x61d0002dfa80) at /home/nik/mariadb/sql/slave.cc:4440 #6 0x00005622d5201589 in handle_slave_sql (arg=0x62a00001e200) at /home/nik/mariadb/sql/slave.cc:5626

Which doesn't happen during lock=shared copy.

...

...

if (ha_enable_transaction(thd, TRUE)) DBUG_RETURN(TRUE);

Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org

-- Yours truly, Nikita Malyavin

-- Yours truly, Nikita Malyavin