Hi, Nikita,
On Aug 07, Nikita Malyavin wrote:
> 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 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 <io.h>
> @@ -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