Re: bzr commit into mysql-5.4 branch (zhenxing.he:2804) Bug#45672 Bug#45673

[Alfranio Correia]

Hi Jasonh,

Patch approved, nice work.

He Zhenxing wrote:
> #At file:///media/sdb2/hezx/work/mysql/bzrwork/b45672/azalea-bugfixing/ based on
> revid:alik@stripped
>
>  2804 He Zhenxing	2009-07-02
>       BUG#45672 Semisync repl: ActiveTranx:insert_tranx_node: transaction node
> allocation failed
>       BUG#45673 Semisynch reports correct operation even if no slave is connected
>       
>       When semi-sync was enabled on master without any semi-sync slaves
>       connected, it would still think that semi-sync status is ON and
>       keep insert tranx node and finally result in tranx_node allocation
>       error.
>       
>       This is fixed by not consider semi-sync master status as ON if
>       no semi-sync slaves connected.
>      @ plugin/semisync/semisync_master.cc
>         do not consider semi-sync master status as ON if no semi-sync slaves
> connected
>      @ plugin/semisync/semisync_slave_plugin.cc
>         run slave in async mode if master disabled semi-sync
>      @ sql/log.cc
>         set error to 1 when flush binlog or run after_flush hooks fails
>
>     M  mysql-test/suite/rpl/r/rpl_semi_sync.result
>     M  mysql-test/suite/rpl/t/rpl_semi_sync.test
>     M  plugin/semisync/semisync_master.cc
>     M  plugin/semisync/semisync_slave_plugin.cc
>     M  sql/log.cc
> === modified file 'mysql-test/suite/rpl/r/rpl_semi_sync.result'
> --- a/mysql-test/suite/rpl/r/rpl_semi_sync.result	2009-06-17 10:37:04 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_semi_sync.result	2009-07-02 10:17:34 +0000
> @@ -29,6 +29,33 @@ set global rpl_semi_sync_master_enabled 
>  show variables like 'rpl_semi_sync_master_enabled';
>  Variable_name	Value
>  rpl_semi_sync_master_enabled	ON
> +[ status of semi-sync on master should be OFF without any semi-sync slaves ]
> +show status like 'Rpl_semi_sync_master_clients';
> +Variable_name	Value
> +Rpl_semi_sync_master_clients	0
> +show status like 'Rpl_semi_sync_master_status';
> +Variable_name	Value
> +Rpl_semi_sync_master_status	OFF
> +show status like 'Rpl_semi_sync_master_yes_tx';
> +Variable_name	Value
> +Rpl_semi_sync_master_yes_tx	0
> +#
> +# BUG#45672 Semisync repl: ActiveTranx:insert_tranx_node: transaction node
> allocation failed
> +# BUG#45673 Semisynch reports correct operation even if no slave is connected
> +#
> +[ status of semi-sync on master should be OFF ]
> +show status like 'Rpl_semi_sync_master_clients';
> +Variable_name	Value
> +Rpl_semi_sync_master_clients	0
> +show status like 'Rpl_semi_sync_master_status';
> +Variable_name	Value
> +Rpl_semi_sync_master_status	OFF
> +show status like 'Rpl_semi_sync_master_yes_tx';
> +Variable_name	Value
> +Rpl_semi_sync_master_yes_tx	0
> +#
> +# INSTALL PLUGIN semi-sync on slave
> +#
>  [ on slave ]
>  [ default state of semi-sync on slave should be OFF ]
>  show variables like 'rpl_semi_sync_slave_enabled';
> @@ -213,6 +240,7 @@ a
>  2
>  3
>  [ on master ]
> +[ master semi-sync status should be ON ]
>  show status like 'Rpl_semi_sync_master_status';
>  Variable_name	Value
>  Rpl_semi_sync_master_status	ON
> @@ -225,10 +253,10 @@ Rpl_semi_sync_master_yes_tx	3
>  #
>  # Start semi-sync replication without SUPER privilege
>  #
> -reset master;
>  include/stop_slave.inc
>  reset slave;
>  [ on master ]
> +reset master;
>  set sql_log_bin=0;
>  grant replication slave on *.* to rpl@stripped identified by 'rpl';
>  flush privileges;
> @@ -238,7 +266,14 @@ grant replication slave on *.* to rpl@12
>  flush privileges;
>  change master to master_user='rpl',master_password='rpl';
>  include/start_slave.inc
> +show status like 'Rpl_semi_sync_slave_status';
> +Variable_name	Value
> +Rpl_semi_sync_slave_status	ON
>  [ on master ]
> +[ master semi-sync should be ON ]
> +show status like 'Rpl_semi_sync_master_clients';
> +Variable_name	Value
> +Rpl_semi_sync_master_clients	1
>  show status like 'Rpl_semi_sync_master_status';
>  Variable_name	Value
>  Rpl_semi_sync_master_status	ON
> @@ -250,6 +285,10 @@ Variable_name	Value
>  Rpl_semi_sync_master_yes_tx	0
>  insert into t1 values (4);
>  insert into t1 values (5);
> +[ master semi-sync should be ON ]
> +show status like 'Rpl_semi_sync_master_clients';
> +Variable_name	Value
> +Rpl_semi_sync_master_clients	1
>  show status like 'Rpl_semi_sync_master_status';
>  Variable_name	Value
>  Rpl_semi_sync_master_status	ON
> @@ -268,6 +307,34 @@ SHOW STATUS LIKE 'Rpl_semi_sync_slave_st
>  Variable_name	Value
>  Rpl_semi_sync_slave_status	OFF
>  [ on master ]
> +[ Semi-sync status on master should be ON ]
> +show status like 'Rpl_semi_sync_master_clients';
> +Variable_name	Value
> +Rpl_semi_sync_master_clients	0
> +show status like 'Rpl_semi_sync_master_status';
> +Variable_name	Value
> +Rpl_semi_sync_master_status	OFF
> +set global rpl_semi_sync_master_enabled= 0;
> +[ on slave ]
> +SHOW VARIABLES LIKE 'rpl_semi_sync_slave_enabled';
> +Variable_name	Value
> +rpl_semi_sync_slave_enabled	ON
> +include/start_slave.inc
> +[ on master ]
> +insert into t1 values (8);
> +[ master semi-sync clients should be 0, status should be OFF ]
> +show status like 'Rpl_semi_sync_master_clients';
> +Variable_name	Value
> +Rpl_semi_sync_master_clients	0
> +show status like 'Rpl_semi_sync_master_status';
> +Variable_name	Value
> +Rpl_semi_sync_master_status	OFF
> +[ on slave ]
> +show status like 'Rpl_semi_sync_slave_status';
> +Variable_name	Value
> +Rpl_semi_sync_slave_status	OFF
> +include/stop_slave.inc
> +[ on master ]
>  set sql_log_bin=0;
>  UNINSTALL PLUGIN rpl_semi_sync_master;
>  set sql_log_bin=1;
>
> === modified file 'mysql-test/suite/rpl/t/rpl_semi_sync.test'
> --- a/mysql-test/suite/rpl/t/rpl_semi_sync.test	2009-06-17 10:37:04 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_semi_sync.test	2009-07-02 10:17:34 +0000
> @@ -58,6 +58,50 @@ echo [ enable semi-sync on master ];
>  set global rpl_semi_sync_master_enabled = 1;
>  show variables like 'rpl_semi_sync_master_enabled';
>  
> +echo [ status of semi-sync on master should be OFF without any semi-sync slaves ];
> +show status like 'Rpl_semi_sync_master_clients';
> +show status like 'Rpl_semi_sync_master_status';
> +show status like 'Rpl_semi_sync_master_yes_tx';
> +
> +--echo #
> +--echo # BUG#45672 Semisync repl: ActiveTranx:insert_tranx_node: transaction node
> allocation failed
> +--echo # BUG#45673 Semisynch reports correct operation even if no slave is
> connected
> +--echo #
> +
> +# BUG#45672 When semi-sync is enabled on master, it would allocate
> +# transaction node even without semi-sync slave connected, and would
> +# finally result in transaction node allocation error.
> +#
> +# Semi-sync master will pre-allocate 'max_connections' transaction
> +# nodes, so here we do more than that much transactions to check if it
> +# will fail or not.
> +# select @@global.max_connections + 1;
> +let $i= `select @@global.max_connections + 1`;
> +disable_query_log;
> +eval create table t1 (a int) engine=$engine_type;
> +while ($i)
> +{
> +  eval insert into t1 values ($i);
> +  dec $i;
> +}
> +drop table t1;
> +enable_query_log;
> +
> +# BUG#45673
> +echo [ status of semi-sync on master should be OFF ];
> +show status like 'Rpl_semi_sync_master_clients';
> +show status like 'Rpl_semi_sync_master_status';
> +show status like 'Rpl_semi_sync_master_yes_tx';
> +
> +disable_query_log;
> +# reset master to make sure the following test will start with a clean environment
> +reset master;
> +enable_query_log;
> +
> +--echo #
> +--echo # INSTALL PLUGIN semi-sync on slave
> +--echo #
> +
>  connection slave;
>  echo [ on slave ];
>  
> @@ -237,11 +281,35 @@ echo [ on slave ];
>  
>  source include/stop_slave.inc;
>  reset slave;
> +
> +# Kill the dump thread on master for previous slave connection and
> +# wait for it to exit
> +connection master;
> +let $_tid= `select id from information_schema.processlist where command = 'Binlog
> Dump' limit 1`;
> +if ($_tid)
> +{
> +  disable_query_log;
> +  eval kill query $_tid;
> +  enable_query_log;
> +
> +  # After dump thread exit, Rpl_semi_sync_master_clients will be 0
> +  let $status_var= Rpl_semi_sync_master_clients;
> +  let $status_var_value= 0;
> +  source include/wait_for_status_var.inc;
> +}
> +
> +connection slave;
>  source include/start_slave.inc;
>  
>  connection master;
>  echo [ on master ];
>  
> +# Wait for dump thread to start, Rpl_semi_sync_master_clients will be
> +# 1 after dump thread started.
> +let $status_var= Rpl_semi_sync_master_clients;
> +let $status_var_value= 1;
> +source include/wait_for_status_var.inc;
> +
>  replace_result $engine_type ENGINE_TYPE;
>  eval create table t1 (a int) engine = $engine_type;
>  insert into t1 values (1);
> @@ -255,6 +323,7 @@ select * from t1;
>  connection master;
>  echo [ on master ];
>  
> +echo [ master semi-sync status should be ON ];
>  show status like 'Rpl_semi_sync_master_status';
>  show status like 'Rpl_semi_sync_master_no_tx';
>  show status like 'Rpl_semi_sync_master_yes_tx';
> @@ -262,13 +331,27 @@ show status like 'Rpl_semi_sync_master_y
>  --echo #
>  --echo # Start semi-sync replication without SUPER privilege
>  --echo #
> -connection master;
> -reset master;
> -sync_slave_with_master;
> +connection slave;
>  source include/stop_slave.inc;
>  reset slave;
>  connection master;
>  echo [ on master ];
> +reset master;
> +
> +# Kill the dump thread on master for previous slave connection and wait for it to
> exit
> +let $_tid= `select id from information_schema.processlist where command = 'Binlog
> Dump' limit 1`;
> +if ($_tid)
> +{
> +  disable_query_log;
> +  eval kill query $_tid;
> +  enable_query_log;
> +
> +  # After dump thread exit, Rpl_semi_sync_master_clients will be 0
> +  let $status_var= Rpl_semi_sync_master_clients;
> +  let $status_var_value= 0;
> +  source include/wait_for_status_var.inc;
> +}
> +
>  # Do not binlog the following statement because it will generate
>  # different events for ROW and STATEMENT format
>  set sql_log_bin=0;
> @@ -281,13 +364,23 @@ grant replication slave on *.* to rpl@12
>  flush privileges;
>  change master to master_user='rpl',master_password='rpl';
>  source include/start_slave.inc;
> +show status like 'Rpl_semi_sync_slave_status';
>  connection master;
>  echo [ on master ];
> +
> +# Wait for the semi-sync binlog dump thread to start
> +let $status_var= Rpl_semi_sync_master_clients;
> +let $status_var_value= 1;
> +source include/wait_for_status_var.inc;
> +echo [ master semi-sync should be ON ];
> +show status like 'Rpl_semi_sync_master_clients';
>  show status like 'Rpl_semi_sync_master_status';
>  show status like 'Rpl_semi_sync_master_no_tx';
>  show status like 'Rpl_semi_sync_master_yes_tx';
>  insert into t1 values (4);
>  insert into t1 values (5);
> +echo [ master semi-sync should be ON ];
> +show status like 'Rpl_semi_sync_master_clients';
>  show status like 'Rpl_semi_sync_master_status';
>  show status like 'Rpl_semi_sync_master_no_tx';
>  show status like 'Rpl_semi_sync_master_yes_tx';
> @@ -296,6 +389,7 @@ show status like 'Rpl_semi_sync_master_y
>  --echo # Test semi-sync slave connect to non-semi-sync master
>  --echo #
>  
> +# Disable semi-sync on master
>  connection slave;
>  echo [ on slave ];
>  source include/stop_slave.inc;
> @@ -303,6 +397,45 @@ SHOW STATUS LIKE 'Rpl_semi_sync_slave_st
>  
>  connection master;
>  echo [ on master ];
> +
> +# Kill the dump thread on master for previous slave connection and wait for it to
> exit
> +let $_tid= `select id from information_schema.processlist where command = 'Binlog
> Dump' limit 1`;
> +if ($_tid)
> +{
> +  disable_query_log;
> +  eval kill query $_tid;
> +  enable_query_log;
> +
> +  # After dump thread exit, Rpl_semi_sync_master_clients will be 0
> +  let $status_var= Rpl_semi_sync_master_clients;
> +  let $status_var_value= 0;
> +  source include/wait_for_status_var.inc;
> +}
> +
> +echo [ Semi-sync status on master should be ON ];
> +show status like 'Rpl_semi_sync_master_clients';
> +show status like 'Rpl_semi_sync_master_status';
> +set global rpl_semi_sync_master_enabled= 0;
> +
> +connection slave;
> +echo [ on slave ];
> +SHOW VARIABLES LIKE 'rpl_semi_sync_slave_enabled';
> +source include/start_slave.inc;
> +connection master;
> +echo [ on master ];
> +insert into t1 values (8);
> +echo [ master semi-sync clients should be 0, status should be OFF ];
> +show status like 'Rpl_semi_sync_master_clients';
> +show status like 'Rpl_semi_sync_master_status';
> +sync_slave_with_master;
> +echo [ on slave ];
> +show status like 'Rpl_semi_sync_slave_status';
> +
> +# Uninstall semi-sync plugin on master
> +connection slave;
> +source include/stop_slave.inc;
> +connection master;
> +echo [ on master ];
>  set sql_log_bin=0;
>  UNINSTALL PLUGIN rpl_semi_sync_master;
>  set sql_log_bin=1;
>
> === modified file 'plugin/semisync/semisync_master.cc'
> --- a/plugin/semisync/semisync_master.cc	2009-06-17 10:37:04 +0000
> +++ b/plugin/semisync/semisync_master.cc	2009-07-02 10:17:34 +0000
> @@ -138,6 +138,16 @@ ActiveTranx::TranxNode* ActiveTranx::all
>      ptr->next_ = NULL;
>      ptr->hash_next_ = NULL;
>    }
> +  else
> +  {
> +    /*
> +      free_pool should never be NULL here, because we have
> +      max_connections number of pre-allocated nodes.
> +    */
> +    sql_print_error("You have encountered a semi-sync bug (free_pool == NULL), "
> +                    "please report to http://bugs.mysql.com");
> +    assert(free_pool_);
> +  }
>  
>    return ptr;
>  }
> @@ -821,7 +831,7 @@ int ReplSemiSyncMaster::commitTrx(const 
>  
>    l_end:
>      /* Update the status counter. */
> -    if (is_on())
> +    if (is_on() && rpl_semi_sync_master_clients)
>        enabled_transactions_++;
>      else
>        disabled_transactions_++;
> @@ -1083,10 +1093,19 @@ int ReplSemiSyncMaster::writeTranxInBinl
>      commit_file_name_inited_ = true;
>    }
>  
> -  if (is_on())
> +  if (is_on() && rpl_semi_sync_master_clients)
>    {
>      assert(active_tranxs_ != NULL);
> -    result = active_tranxs_->insert_tranx_node(log_file_name, log_file_pos);
> +    if(active_tranxs_->insert_tranx_node(log_file_name, log_file_pos))
> +    {
> +      /*
> +        if insert tranx_node failed, print a warning message
> +        and turn off semi-sync
> +      */
> +      sql_print_warning("Semi-sync failed to insert tranx_node for binlog file: %s,
> position: %ul",
> +                        log_file_name, log_file_pos);
> +      switch_off();
> +    }
>    }
>  
>   l_end:
> @@ -1131,7 +1150,7 @@ void ReplSemiSyncMaster::setExportStats(
>  {
>    lock();
>  
> -  rpl_semi_sync_master_status           = state_ ? 1 : 0;
> +  rpl_semi_sync_master_status           = state_ &&
> rpl_semi_sync_master_clients;
>    rpl_semi_sync_master_yes_transactions = enabled_transactions_;
>    rpl_semi_sync_master_no_transactions  = disabled_transactions_;
>    rpl_semi_sync_master_off_times        = switched_off_times_;
>
> === modified file 'plugin/semisync/semisync_slave_plugin.cc'
> --- a/plugin/semisync/semisync_slave_plugin.cc	2009-06-17 10:37:04 +0000
> +++ b/plugin/semisync/semisync_slave_plugin.cc	2009-07-02 10:17:34 +0000
> @@ -63,7 +63,7 @@ int repl_semi_slave_request_dump(Binlog_
>    }
>  
>    row= mysql_fetch_row(res);
> -  if (!row)
> +  if (!row || strcmp(row[1], "ON"))
>    {
>      /* Master does not support or not configured semi-sync */
>      sql_print_warning("Master server does not support or not configured semi-sync
> replication, fallback to asynchronous");
>
> === modified file 'sql/log.cc'
> --- a/sql/log.cc	2009-06-15 14:00:15 +0000
> +++ b/sql/log.cc	2009-07-02 10:17:34 +0000
> @@ -5994,7 +5994,6 @@ bool MYSQL_BIN_LOG::write(Log_event *eve
>      if (event_info->write(file))
>        goto err;
>  
> -    error=0;
>      if (file == &log_file) // we are writing to the real log (disk)
>      {
>        bool synced;
> @@ -6003,12 +6002,14 @@ bool MYSQL_BIN_LOG::write(Log_event *eve
>  
>        if (RUN_HOOK(binlog_storage, after_flush,
>                     (thd, log_file_name, file->pos_in_file, synced))) {
> +        sql_print_error("Failed to run 'after_flush' hooks");
>          goto err;
>        }
>  
>        signal_update();
>        rotate_and_purge(RP_LOCK_LOG_IS_ALREADY_LOCKED);
>      }
> +    error=0;
>  
>  err:
>      if (error)
> @@ -6383,7 +6384,11 @@ bool MYSQL_BIN_LOG::write(THD *thd, IO_C
>  
>        if (RUN_HOOK(binlog_storage, after_flush,
>                     (thd, log_file_name, log_file.pos_in_file, synced)))
> +      {
> +        sql_print_error("Failed to run 'after_flush' hooks");
> +        write_error=1;
>          goto err;
> +      }
>  
>        signal_update();
>      }
>
>   
> ------------------------------------------------------------------------
>
>
>