maria-developers team mailing list archive

[Michael Widenius <michael.widenius@xxxxxxxxx>]

Hi!

On Fri, Sep 18, 2020 at 1:54 PM Sergei Golubchik <serg@xxxxxxxxxxx> 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