← Back to team overview

maria-developers team mailing list archive

Re: [Commits] 97d212a: MDEV-10545: Server crashed in my_copy_fix_mb on querying I_S and P_S tables

 

Hi Svoj!

Thanks for the review.

On Fri, Sep 2, 2016 at 1:41 AM, Sergey Vojtovich <svoj@xxxxxxxxxxx> wrote:

> Hi Nirbhay,
>
> Ok to push, one question below.
>
> On Thu, Sep 01, 2016 at 12:46:58PM -0400, Nirbhay Choubey wrote:
> > revision-id: 97d212a34ff4e0558126bc393bbef97036611d83
> (mariadb-10.1.17-4-g97d212a)
> > parent(s): a322651b8aa702e58d473edfae26606f10a089fb
> > author: Nirbhay Choubey
> > committer: Nirbhay Choubey
> > timestamp: 2016-09-01 12:46:56 -0400
> > message:
> >
> > MDEV-10545: Server crashed in my_copy_fix_mb on querying I_S and P_S
> tables
> >
> > Once THDs have been added to the global "threads" list,
> > they must modify query_string only after acquiring per-
> > thread LOCK_thd_data mutex.
> >
> > ---
> >  sql/log_event.cc | 2 +-
> >  sql/sql_acl.cc   | 8 ++++----
> >  sql/sql_parse.cc | 7 +++----
> >  3 files changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/sql/log_event.cc b/sql/log_event.cc
> > index afa58af..66e7c60 100644
> > --- a/sql/log_event.cc
> > +++ b/sql/log_event.cc
> > @@ -6022,7 +6022,7 @@ int Load_log_event::do_apply_event(NET* net,
> rpl_group_info *rgi,
> >    new_db.str= (char *) rpl_filter->get_rewrite_db(db, &new_db.length);
> >    thd->set_db(new_db.str, new_db.length);
> >    DBUG_ASSERT(thd->query() == 0);
> > -  thd->reset_query_inner();                    // Should not be needed
> > +  if (thd->query()) thd->reset_query();         // Should not be needed
> Why if? Original code didn't have it.
>

Now that we call reset_query(), the check has been added to avoid it and an
extra mutex.
And from the above assert, I think it will be ever better have this check
within unlikely().

Best,
Nirbhay


>
> Regards,
> Sergey
> _______________________________________________
> commits mailing list
> commits@xxxxxxxxxxx
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

Follow ups