Hi Svoj!

Thanks for the review.

On Fri, Sep 2, 2016 at 1:41 AM, Sergey Vojtovich <svoj@mariadb.org> 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@mariadb.org
https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits