← Back to team overview

maria-developers team mailing list archive

Re: ae1f4e4d727: MDEV-23836: Assertion `! is_set() || m_can_overwrite_status' in Diagnostics_area::set_error_status (interrupted ALTER TABLE under LOCK)

 

Hi, Rucha!

On Jul 23, Rucha Deodhar wrote:
> > > diff --git a/sql/sql_table.cc b/sql/sql_table.cc
> > > index f58ce8f997d..0b722984fca 100644
> > > --- a/sql/sql_table.cc
> > > +++ b/sql/sql_table.cc
> > > @@ -6489,6 +6489,12 @@ handle_if_exists_options(THD *thd, TABLE *table, Alter_info *alter_info,
> > >                              ER_BAD_FIELD_ERROR,
> > >                              ER_THD(thd, ER_BAD_FIELD_ERROR),
> > >                              sql_field->change.str, table->s->table_name.str);
> > > +        /*
> > > +          if thread is killed (for example because local memory used execeeds
> > > +          maximum session memory used), return TRUE and do rollback (done later).
> > > +        */
> > > +        if (thd->killed)
> > > +          DBUG_RETURN(true);
> > >          it.remove();
> >
> > Looks like a completely arbitrary place to put a check in.
> > What if the kill signal arrives later?
> 
> We want to stop execution of the query as soon as we get kill. In this case
> it happens when we are giving out a warning.
> So I put the check here to return error right after giving warning.

You can get KILL anytime. Was there any reason to put this check here
and not few lines earlier or later? What if KILL would arrive before the
warning? or after it.remove() ?

Consider this - thd->killed can be changed literally any time. We cannot
test for it after every second line, can we? Clearly some places are
better for this test than others.

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx


References