[MariaDB developers] Re: 63c93f89af9: MDEV-31804 Assertion `thd->m_transaction_psi == __null' fails

7 Aug 2023

...
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☺
On Mon, 7 Aug 2023 at 18:25, Sergei Golubchik  wrote:

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

    