Hi Serg,

On Tue, Jun 28, 2016 at 5:02 PM, Sergei Golubchik <serg@mariadb.org> wrote:

Hi, Nirbhay!

On Jun 27, Nirbhay Choubey wrote:
> >
> > That seems quite ugly, why not call it from the SST code, after it
> > has called reload_acl_and_cache()? You're basically making FLUSH
> > LOGS behave differently in Galera and non-Galera (if my
> > understanding is correct), which might lead to subtle bugs?
>
> I initially thought of adding the call after reload_acl_and_cache(),
> but there could still be a case when user performs a
> REFRESH_BINARY_LOG before LOCK_log is acquired.

Right, but you didn't fix it. You have

1> FTWRL
2> reload_acl_and_cache()
3> wait_for_last_checkpoint_event()
4> SET global innodb_disallow_writes=1
5> mysql_mutex_lock(LOCK_log)

You've described your case correctly: "when user performs
REFRESH_BINARY_LOG before LOCK_log is acquired". That is, you care when
a user performs REFRESH_BINARY_LOG between 3 and 5. You don't care if
somebody does REFRESH_BINARY_LOG between 2 and 3. So, you can as well
move wait_for_last_checkpoint_event() out of reload_acl_and_cache().

If wait is moved outside wait_for_last_checkpoint_event() (say 3') and user's

REFRESH_BINARY_LOG kicks in right after the wait (3') but before (5), will

trigger creation of another new binlog file for which the last checkpoint event

(logged asynchronously by a separate thread) may not make it into time and

will cause the same issue on joiner node.

Another workable option was to move wait outside and after reload_acl_and_cache

and not release LOCK_log until the file transfer is complete.

1> FTWRL

2> reload_acl_and_cache()

3> wait for last checkpoint event & lock(LOCK_log)

4> SET global innodb_disallow_writes=1

... file transfer ...

5> mysql_mutex_unlock(LOCK_log)

But with LOCK_log locked in #3, mysql_mutex_assert_not_owner(mysql_bin_log.get_log_lock())

will fail for #4.

With wait_for_last_checkpoint_event inside reload_acl_and_cache or
outside, you still don't have anything that would prevent user from
doing REFRESH_BINARY_LOG between 3 and 5.

It wouldn't prevent the user from doing REFRESH_BINARY_LOG, but with

wait_for_last_checkpoint_event() added to reload_acl_and_cache(), it would

ensure every REFRESH_BINARY_LOG (either from user or #2 above) waits

until last checkpoint event makes into the new binary log file.

Best,

Nirbhay

Regards,
Sergei
Chief Architect MariaDB
and security@mariadb.org