Re: [Maria-developers] 2564334b4c1: Backported setting of transcation.on=1 in THD::reset_for_reuse()
Hi, Michael!
diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 7327f270c33..8f6356b15c7 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -1640,6 +1640,7 @@ void THD::reset_for_reuse() abort_on_warning= 0; free_connection_done= 0; m_command= COM_CONNECT; + transaction.on= 1; #if defined(ENABLED_PROFILING) profiling.reset(); #endif
This looks risky, to change transaction.on at some random point in time. I understand that it's not a random point and it should be actually safe here. But perhaps you can add an assert to document that it is safe? Like, transaction.on should be already 1 or there should be no active transaction. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Hi! On Fri, Sep 18, 2020 at 1:54 PM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Michael!
diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 7327f270c33..8f6356b15c7 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -1640,6 +1640,7 @@ void THD::reset_for_reuse() abort_on_warning= 0; free_connection_done= 0; m_command= COM_CONNECT; + transaction.on= 1; #if defined(ENABLED_PROFILING) profiling.reset(); #endif
This looks risky, to change transaction.on at some random point in time.
This is only for change user (when there can't be any transactions around) or when reusing THD for a new connection. Before we left it at a random value. Better to ensure it's always set.
I understand that it's not a random point and it should be actually safe here. But perhaps you can add an assert to document that it is safe?
Assert for what? The THD is reset from a random state to its initial state here. It's quite hard to test that 'is THD valid for all cases' and is not really part of this patch.
Like, transaction.on should be already 1 or there should be no active transaction.
Wrong place to test this. Any test for 'active transactions' should be done when THD is removed from reuse. Regards, Monty
participants (2)
-
Michael Widenius -
Sergei Golubchik