List:Commits« Previous MessageNext Message »
From:Mats Kindahl Date:October 13 2008 12:52pm
Subject:bzr commit into mysql-5.1 branch (mats:2771) Bug#39934
View as plain text  
#At file:///home/bzr/bugs/b39934-5.1-5.1.29-rc/

 2771 Mats Kindahl	2008-10-13
      Bug #39934: Slave stops for engine that only support row-based logging
      
      The slave SQL thread is now executing in STATEMENT mode, meaning that 
      for engines that do not support statement-based replication, it will 
      not be possible to switch to row-format.
      
      If a Rows_log_event arrives to the slave with rows to apply, lock_tables()
      will be executed which in turn will call lock_tables() to open the tables
      to feed the rows into.
      
      For engines that does not support statement-based replication, the slave
      will stop with an error message since it is not possible to switch to 
      row format. However, since this use is supposed to feed rows into the
      table, there should be no problems in executing this event.
      
      Bug is fixed by extending lock_tables() and friends with an extra parameter
      check_caps that will check capabilities if TRUE, and not otherwise. The 
      parameter is then set to FALSE for all calls from inside Rows_log_event
      (and the old version of the same), and subclasses.
      
      Bug is reported for 6.0 but is present in 5.1 as well, so patch is submitted
      for 5.1 but will be extended when merged to 6.0.
modified:
  sql/ha_ndbcluster_binlog.cc
  sql/log_event.cc
  sql/log_event_old.cc
  sql/mysql_priv.h
  sql/sql_base.cc
  sql/sql_table.cc

=== modified file 'sql/ha_ndbcluster_binlog.cc'
--- a/sql/ha_ndbcluster_binlog.cc	2008-02-26 16:34:02 +0000
+++ b/sql/ha_ndbcluster_binlog.cc	2008-10-13 12:28:55 +0000
@@ -2380,7 +2380,8 @@ int ndb_add_ndb_binlog_index(THD *thd, v
       goto add_ndb_binlog_index_err;
     }
 
-    if (lock_tables(thd, &binlog_tables, 1, &need_reopen))
+    if (lock_tables(thd, &binlog_tables, 1, &need_reopen,
+                    /* check_caps */ FALSE))
     {
       if (need_reopen)
       {

=== modified file 'sql/log_event.cc'
--- a/sql/log_event.cc	2008-10-07 13:21:17 +0000
+++ b/sql/log_event.cc	2008-10-13 12:28:55 +0000
@@ -7067,7 +7067,10 @@ int Rows_log_event::do_apply_event(Relay
     /* A small test to verify that objects have consistent types */
     DBUG_ASSERT(sizeof(thd->options) == sizeof(OPTION_RELAXED_UNIQUE_CHECKS));
 
-    if (simple_open_n_lock_tables(thd, rli->tables_to_lock))
+    /*
+      We do not check engine capabilities when replicating row events.
+     */
+    if (simple_open_n_lock_tables(thd, rli->tables_to_lock, FALSE))
     {
       uint actual_error= thd->main_da.sql_errno();
       if (thd->is_slave_error || thd->is_fatal_error)

=== modified file 'sql/log_event_old.cc'
--- a/sql/log_event_old.cc	2008-05-12 17:50:53 +0000
+++ b/sql/log_event_old.cc	2008-10-13 12:28:55 +0000
@@ -76,7 +76,10 @@ Old_rows_log_event::do_apply_event(Old_r
       thd->set_current_stmt_binlog_row_based();
     }
 
-    if (simple_open_n_lock_tables(thd, rli->tables_to_lock))
+    /*
+      We do not check the engine capabilities when applying rows.
+     */
+    if (simple_open_n_lock_tables(thd, rli->tables_to_lock, FALSE))
     {
       uint actual_error= thd->main_da.sql_errno();
       if (thd->is_slave_error || thd->is_fatal_error)

=== modified file 'sql/mysql_priv.h'
--- a/sql/mysql_priv.h	2008-10-02 12:53:08 +0000
+++ b/sql/mysql_priv.h	2008-10-13 12:28:55 +0000
@@ -1520,22 +1520,26 @@ void wait_for_condition(THD *thd, pthrea
                         pthread_cond_t *cond);
 int open_tables(THD *thd, TABLE_LIST **tables, uint *counter, uint flags);
 /* open_and_lock_tables with optional derived handling */
-int open_and_lock_tables_derived(THD *thd, TABLE_LIST *tables, bool derived);
+int open_and_lock_tables_derived(THD *thd, TABLE_LIST *tables, bool derived,
+                                 bool check_caps= TRUE);
 /* simple open_and_lock_tables without derived handling */
-inline int simple_open_n_lock_tables(THD *thd, TABLE_LIST *tables)
+inline int simple_open_n_lock_tables(THD *thd, TABLE_LIST *tables,
+                                     bool check_caps= TRUE)
 {
-  return open_and_lock_tables_derived(thd, tables, FALSE);
+  return open_and_lock_tables_derived(thd, tables, FALSE, check_caps);
 }
 /* open_and_lock_tables with derived handling */
-inline int open_and_lock_tables(THD *thd, TABLE_LIST *tables)
+inline int open_and_lock_tables(THD *thd, TABLE_LIST *tables,
+                                bool check_caps= TRUE)
 {
-  return open_and_lock_tables_derived(thd, tables, TRUE);
+  return open_and_lock_tables_derived(thd, tables, TRUE, check_caps);
 }
 /* simple open_and_lock_tables without derived handling for single table */
 TABLE *open_n_lock_single_table(THD *thd, TABLE_LIST *table_l,
-                                thr_lock_type lock_type);
+                                thr_lock_type lock_type, bool check_caps= TRUE);
 bool open_normal_and_derived_tables(THD *thd, TABLE_LIST *tables, uint flags);
-int lock_tables(THD *thd, TABLE_LIST *tables, uint counter, bool *need_reopen);
+int lock_tables(THD *thd, TABLE_LIST *tables, uint counter, bool *need_reopen,
+                bool check_caps= TRUE);
 int decide_logging_format(THD *thd, TABLE_LIST *tables);
 TABLE *open_temporary_table(THD *thd, const char *path, const char *db,
 			    const char *table_name, bool link_in_list);

=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2008-10-08 08:46:25 +0000
+++ b/sql/sql_base.cc	2008-10-13 12:28:55 +0000
@@ -4818,7 +4818,8 @@ static bool check_lock_and_start_stmt(TH
 */
 
 TABLE *open_n_lock_single_table(THD *thd, TABLE_LIST *table_l,
-                                thr_lock_type lock_type)
+                                thr_lock_type lock_type,
+                                bool check_caps)
 {
   TABLE_LIST *save_next_global;
   DBUG_ENTER("open_n_lock_single_table");
@@ -4834,7 +4835,7 @@ TABLE *open_n_lock_single_table(THD *thd
   table_l->required_type= FRMTYPE_TABLE;
 
   /* Open the table. */
-  if (simple_open_n_lock_tables(thd, table_l))
+  if (simple_open_n_lock_tables(thd, table_l, check_caps))
     table_l->table= NULL; /* Just to be sure. */
 
   /* Restore list. */
@@ -4928,6 +4929,7 @@ TABLE *open_ltable(THD *thd, TABLE_LIST 
     thd		- thread handler
     tables	- list of tables for open&locking
     derived     - if to handle derived tables
+    check_caps  - Check engine capabilities
 
   RETURN
     FALSE - ok
@@ -4944,7 +4946,8 @@ TABLE *open_ltable(THD *thd, TABLE_LIST 
     the third argument set appropriately.
 */
 
-int open_and_lock_tables_derived(THD *thd, TABLE_LIST *tables, bool derived)
+int open_and_lock_tables_derived(THD *thd, TABLE_LIST *tables, bool derived,
+                                 bool check_caps)
 {
   uint counter;
   bool need_reopen;
@@ -4962,7 +4965,7 @@ int open_and_lock_tables_derived(THD *th
       my_sleep(6000000);
       thd->proc_info= old_proc_info;});
 
-    if (!lock_tables(thd, tables, counter, &need_reopen))
+    if (!lock_tables(thd, tables, counter, &need_reopen, check_caps))
       break;
     if (!need_reopen)
       DBUG_RETURN(-1);
@@ -5186,6 +5189,7 @@ int decide_logging_format(THD *thd, TABL
                         and therefore invoker should reopen tables and
                         try to lock them once again (in this case
                         lock_tables() will also return error).
+    check_caps          Check engine capabilities
 
   NOTES
     You can't call lock_tables twice, as this would break the dead-lock-free
@@ -5201,7 +5205,8 @@ int decide_logging_format(THD *thd, TABL
    -1	Error
 */
 
-int lock_tables(THD *thd, TABLE_LIST *tables, uint count, bool *need_reopen)
+int lock_tables(THD *thd, TABLE_LIST *tables, uint count, bool *need_reopen,
+                bool check_caps)
 {
   TABLE_LIST *table;
 
@@ -5214,7 +5219,7 @@ int lock_tables(THD *thd, TABLE_LIST *ta
   *need_reopen= FALSE;
 
   if (!tables && !thd->lex->requires_prelocking())
-    DBUG_RETURN(decide_logging_format(thd, tables));
+    DBUG_RETURN(check_caps ? decide_logging_format(thd, tables) : 0);
 
   /*
     We need this extra check for thd->prelocked_mode because we want to avoid
@@ -5353,7 +5358,7 @@ int lock_tables(THD *thd, TABLE_LIST *ta
     }
   }
 
-  DBUG_RETURN(decide_logging_format(thd, tables));
+  DBUG_RETURN(check_caps ? decide_logging_format(thd, tables) : 0);
 }
 
 

=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2008-10-08 09:15:00 +0000
+++ b/sql/sql_table.cc	2008-10-13 12:28:55 +0000
@@ -7417,7 +7417,12 @@ bool mysql_checksum_table(THD *thd, TABL
 
     strxmov(table_name, table->db ,".", table->table_name, NullS);
 
-    t= table->table= open_n_lock_single_table(thd, table, TL_READ);
+    /*
+      Since the tables are opened in TL_READ_NO_INSERT, nothing will
+      be logged. Therefore, we can improve speed slightly by not
+      checking engine capabilities.
+    */
+    t= table->table= open_n_lock_single_table(thd, table, TL_READ, FALSE);
     thd->clear_error();			// these errors shouldn't get client
 
     protocol->prepare_for_resend();

Thread
bzr commit into mysql-5.1 branch (mats:2771) Bug#39934Mats Kindahl13 Oct
  • Re: bzr commit into mysql-5.1 branch (mats:2771) Bug#39934Sven Sandberg14 Oct
    • Re: bzr commit into mysql-5.1 branch (mats:2771) Bug#39934Andrei Elkin15 Oct