← Back to team overview

maria-developers team mailing list archive

Re: [Commits] dc3d6c9edb3: MDEV-15477: SESSION_SYSVARS_TRACKER does not track last_gtid

 

Hi, Oleksandr!

On Mar 09, Oleksandr Byelkin wrote:
> revision-id: dc3d6c9edb3536e5f0c87ac1e9c04891d50fc420 (mariadb-10.2.13-32-gdc3d6c9edb3)
> parent(s): 6cf25e5eb76923550173a0059036d80c80a053d6
> author: Oleksandr Byelkin
> committer: Oleksandr Byelkin
> timestamp: 2018-03-09 14:39:40 +0100
> message:
> 
> MDEV-15477: SESSION_SYSVARS_TRACKER does not track last_gtid
> 
> register changes of last_gtid
> 
> diff --git a/mysql-test/t/session_tracker_last_gtid.test b/mysql-test/t/session_tracker_last_gtid.test
> --- /dev/null
> +++ b/mysql-test/t/session_tracker_last_gtid.test
> @@ -0,0 +1,18 @@
> +
> +--source include/have_innodb.inc
> +--source include/have_binlog_format_mixed_or_statement.inc

Why "mixed or statement"?

> diff --git a/sql/log.cc b/sql/log.cc
> --- a/sql/log.cc
> +++ b/sql/log.cc
> @@ -5945,7 +5945,20 @@ MYSQL_BIN_LOG::write_gtid_event(THD *thd, bool standalone,
>    }
>    if (err)
>      DBUG_RETURN(true);
> +  bool changed_gtid= (thd->last_commit_gtid.seq_no != gtid.seq_no);
>    thd->last_commit_gtid= gtid;
> +  if (changed_gtid &&
> +      thd->session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)->is_enabled())
> +  {
> +    sys_var *svar;
> +    mysql_mutex_lock(&LOCK_plugin);
> +    if ((svar= find_sys_var_ex(thd, "last_gtid",
> +                               sizeof("last_gtid") - 1,
> +                               false, true)))
> +      thd->session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)->
> +        mark_as_changed(thd, (LEX_CSTRING*)svar);
> +    mysql_mutex_unlock(&LOCK_plugin);
> + }

No, please. You svar will always be Sys_last_gtid. There is no need to
search for it on every write_gtid_event(), under a mutex. See in
sys_vars.cc around Sys_autocommit how to do it easier.

Also, perhaps it'd be safer to rename last_commit_gtid to
m_last_commit_gtid, make it private and provide a setter and a getter.
The setter will update the tracker. I'll make sure that last_commit_gtid
is not changed anywhere else bypassing the tracker.

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx