Bug#19704825 TEMPORARY SLAVE TYPE CONVERSION TABLES RELEASED TO EARLY

Merging fix from mysql-5.6
mysql · Oct 30, 2014 · 81f812b · 81f812b
2 parents 561a3dc + efc0025
commit 81f812b
Show file tree

Hide file tree

Showing 10 changed files with 122 additions and 113 deletions.
diff --git a/mysql-test/suite/rpl/r/rpl_colSize.result b/mysql-test/suite/rpl/r/rpl_colSize.result
@@ -185,8 +185,22 @@ t1	CREATE TABLE `t1` (
   `d` bit(25) DEFAULT NULL,
   `e` bit(13) DEFAULT NULL
 ) ENGINE=ENGINE DEFAULT CHARSET=latin1
+CREATE TABLE t2(i INTEGER);
+CREATE TRIGGER t1_tr BEFORE INSERT ON t1 FOR EACH ROW
+BEGIN
+INSERT INTO t2 VALUES(1);
+END $$
+INSERT INTO t1 VALUES (
+b'1010101',
+b'10101011',
+b'101010110101010101111',
+b'10101010101',
+b'10101011111'
+  );
+include/sync_slave_sql_with_master.inc
 *** Cleanup  ***
 DROP TABLE t1;
+DROP TABLE t2;
 include/sync_slave_sql_with_master.inc
 SET GLOBAL SLAVE_TYPE_CONVERSIONS = @saved_slave_type_conversions;
 include/rpl_end.inc
diff --git a/mysql-test/suite/rpl/t/rpl_colSize.test b/mysql-test/suite/rpl/t/rpl_colSize.test
@@ -221,17 +221,64 @@ SHOW CREATE TABLE t1;
 --let $sync_slave_connection= slave
 --source include/sync_slave_sql_with_master.inc
 
-
-
 --echo Insert some values and select them on master
 SELECT BIN(a), BIN(b), BIN(c), BIN(d), BIN(e) FROM t1;
 #Replace default engine value with static engine string          WL#6921
 --replace_result $DEFAULT_ENGINE ENGINE default DEFAULT
 SHOW CREATE TABLE t1;
 
+###############################################################################
+# Bug#19704825 TEMPORARY SLAVE TYPE CONVERSION TABLES RELEASED TO EARLY
+# Problem: Memory used in preparing slave type conversion
+# temporary table is getting released early and causing unexpected results
+# in case of nested events.
+# Eg: If a insert statement is calling a trigger which contains another
+# insert statement.
+# Steps to reproduce:
+#  1) Create a table t1
+#  2) Change it's definition on slave, so that insert will create conversion
+#     temporary table on slave (set SLAVE_TYPE_CONVERSIONS appropriately to
+#     allow the scenario)
+#  3) Create a table t2
+#  4) Create a trigger on top of table t1. Inside the trigger, insert a tuple
+#      into table t2
+#  5) Insert a tuple into table t1 on Master
+#  6) Make sure this insert is replicated without any issues.
+###############################################################################
+
+# Step 1, 2 are already done by above test code.
+
+# Step 3: Create a table t2
+connection master;
+CREATE TABLE t2(i INTEGER);
+
+# Step 4: Create a trigger on top of table t1. Inside the trigger, insert a
+#          tuple into table t2
+--delimiter $$
+CREATE TRIGGER t1_tr BEFORE INSERT ON t1 FOR EACH ROW
+BEGIN
+INSERT INTO t2 VALUES(1);
+END $$
+--delimiter ;
+
+# Step 5: Insert a tuple into table t1 on Master
+INSERT INTO t1 VALUES (
+  b'1010101',
+  b'10101011',
+  b'101010110101010101111',
+  b'10101010101',
+  b'10101011111'
+  );
+
+# Step 6) Make sure this insert is replicated without any issues.
+--source include/sync_slave_sql_with_master.inc
+
+# End of test for Bug#19704825
+
 --echo *** Cleanup  ***
 connection master;
 DROP TABLE t1;
+DROP TABLE t2;
 --source include/sync_slave_sql_with_master.inc
 
 SET GLOBAL SLAVE_TYPE_CONVERSIONS = @saved_slave_type_conversions;

diff --git a/sql/field.cc b/sql/field.cc
@@ -10293,8 +10293,7 @@ Field *make_field(TABLE_SHARE *share, uchar *ptr, size_t field_length,
 		  Field::geometry_type geom_type,
 		  Field::utype unireg_check,
 		  TYPELIB *interval,
-		  const char *field_name,
-		  MEM_ROOT *mem_root /* default= NULL */)
+		  const char *field_name)
 {
   uchar *bit_ptr= NULL;
   uchar bit_offset= 0;
@@ -10330,8 +10329,6 @@ Field *make_field(TABLE_SHARE *share, uchar *ptr, size_t field_length,
                        FLAGSTR(pack_flag, FIELDFLAG_NUMBER),
                        FLAGSTR(pack_flag, FIELDFLAG_PACK),
                        FLAGSTR(pack_flag, FIELDFLAG_BLOB)));
-  if (mem_root == NULL)
-    mem_root= current_thd->mem_root;
 
   if (f_is_alpha(pack_flag))
   {
@@ -10340,11 +10337,11 @@ Field *make_field(TABLE_SHARE *share, uchar *ptr, size_t field_length,
       if (field_type == MYSQL_TYPE_STRING ||
           field_type == MYSQL_TYPE_DECIMAL ||   // 3.23 or 4.0 string
           field_type == MYSQL_TYPE_VAR_STRING)
-        return new (mem_root) Field_string(ptr,field_length,null_pos,null_bit,
+        return new Field_string(ptr,field_length,null_pos,null_bit,
                                 unireg_check, field_name,
                                 field_charset);
       if (field_type == MYSQL_TYPE_VARCHAR)
-        return new (mem_root) Field_varstring(ptr,field_length,
+        return new Field_varstring(ptr,field_length,
                                    HA_VARCHAR_PACKLENGTH(field_length),
                                    null_pos,null_bit,
                                    unireg_check, field_name,
@@ -10358,114 +10355,114 @@ Field *make_field(TABLE_SHARE *share, uchar *ptr, size_t field_length,
 				      field_length);
 
     if (f_is_geom(pack_flag))
-      return new (mem_root) Field_geom(ptr,null_pos,null_bit,
+      return new Field_geom(ptr,null_pos,null_bit,
 			    unireg_check, field_name, share,
 			    pack_length, geom_type);
     if (f_is_blob(pack_flag))
-      return new (mem_root) Field_blob(ptr,null_pos,null_bit,
+      return new Field_blob(ptr,null_pos,null_bit,
 			    unireg_check, field_name, share,
 			    pack_length, field_charset);
     if (interval)
     {
       if (f_is_enum(pack_flag))
-	return new (mem_root) Field_enum(ptr,field_length,null_pos,null_bit,
+	return new Field_enum(ptr,field_length,null_pos,null_bit,
 				  unireg_check, field_name,
 				  pack_length, interval, field_charset);
       else
-	return new (mem_root) Field_set(ptr,field_length,null_pos,null_bit,
+	return new Field_set(ptr,field_length,null_pos,null_bit,
 			     unireg_check, field_name,
 			     pack_length, interval, field_charset);
     }
   }
 
   switch (field_type) {
   case MYSQL_TYPE_DECIMAL:
-    return new (mem_root) Field_decimal(ptr,field_length,null_pos,null_bit,
+    return new Field_decimal(ptr,field_length,null_pos,null_bit,
 			     unireg_check, field_name,
 			     f_decimals(pack_flag),
 			     f_is_zerofill(pack_flag) != 0,
 			     f_is_dec(pack_flag) == 0);
   case MYSQL_TYPE_NEWDECIMAL:
-    return new (mem_root) Field_new_decimal(ptr,field_length,null_pos,null_bit,
+    return new Field_new_decimal(ptr,field_length,null_pos,null_bit,
                                  unireg_check, field_name,
                                  f_decimals(pack_flag),
                                  f_is_zerofill(pack_flag) != 0,
                                  f_is_dec(pack_flag) == 0);
   case MYSQL_TYPE_FLOAT:
-    return new (mem_root) Field_float(ptr,field_length,null_pos,null_bit,
+    return new Field_float(ptr,field_length,null_pos,null_bit,
 			   unireg_check, field_name,
 			   f_decimals(pack_flag),
 			   f_is_zerofill(pack_flag) != 0,
 			   f_is_dec(pack_flag)== 0);
   case MYSQL_TYPE_DOUBLE:
-    return new (mem_root) Field_double(ptr,field_length,null_pos,null_bit,
+    return new Field_double(ptr,field_length,null_pos,null_bit,
 			    unireg_check, field_name,
 			    f_decimals(pack_flag),
 			    f_is_zerofill(pack_flag) != 0,
 			    f_is_dec(pack_flag)== 0);
   case MYSQL_TYPE_TINY:
-    return new (mem_root) Field_tiny(ptr,field_length,null_pos,null_bit,
+    return new Field_tiny(ptr,field_length,null_pos,null_bit,
 			  unireg_check, field_name,
 			  f_is_zerofill(pack_flag) != 0,
 			  f_is_dec(pack_flag) == 0);
   case MYSQL_TYPE_SHORT:
-    return new (mem_root) Field_short(ptr,field_length,null_pos,null_bit,
+    return new Field_short(ptr,field_length,null_pos,null_bit,
 			   unireg_check, field_name,
 			   f_is_zerofill(pack_flag) != 0,
 			   f_is_dec(pack_flag) == 0);
   case MYSQL_TYPE_INT24:
-    return new (mem_root) Field_medium(ptr,field_length,null_pos,null_bit,
+    return new Field_medium(ptr,field_length,null_pos,null_bit,
 			    unireg_check, field_name,
 			    f_is_zerofill(pack_flag) != 0,
 			    f_is_dec(pack_flag) == 0);
   case MYSQL_TYPE_LONG:
-    return new (mem_root) Field_long(ptr,field_length,null_pos,null_bit,
+    return new Field_long(ptr,field_length,null_pos,null_bit,
 			   unireg_check, field_name,
 			   f_is_zerofill(pack_flag) != 0,
 			   f_is_dec(pack_flag) == 0);
   case MYSQL_TYPE_LONGLONG:
-    return new (mem_root) Field_longlong(ptr,field_length,null_pos,null_bit,
+    return new Field_longlong(ptr,field_length,null_pos,null_bit,
 			      unireg_check, field_name,
 			      f_is_zerofill(pack_flag) != 0,
 			      f_is_dec(pack_flag) == 0);
   case MYSQL_TYPE_TIMESTAMP:
-    return new (mem_root) Field_timestamp(ptr, field_length, null_pos, null_bit,
+    return new Field_timestamp(ptr, field_length, null_pos, null_bit,
                                unireg_check, field_name);
   case MYSQL_TYPE_TIMESTAMP2:
-    return new (mem_root) Field_timestampf(ptr, null_pos, null_bit,
+    return new Field_timestampf(ptr, null_pos, null_bit,
                                 unireg_check, field_name,
                                 field_length > MAX_DATETIME_WIDTH ?
                                 field_length - 1 - MAX_DATETIME_WIDTH : 0);
   case MYSQL_TYPE_YEAR:
-    return new (mem_root) Field_year(ptr,field_length,null_pos,null_bit,
+    return new Field_year(ptr,field_length,null_pos,null_bit,
 			  unireg_check, field_name);
   case MYSQL_TYPE_NEWDATE:
-    return new (mem_root) Field_newdate(ptr, null_pos, null_bit, unireg_check, field_name);
+    return new Field_newdate(ptr, null_pos, null_bit, unireg_check, field_name);
 
   case MYSQL_TYPE_TIME:
-    return new (mem_root) Field_time(ptr, null_pos, null_bit,
+    return new Field_time(ptr, null_pos, null_bit,
                           unireg_check, field_name);
   case MYSQL_TYPE_TIME2:
-    return new (mem_root) Field_timef(ptr, null_pos, null_bit,
+    return new Field_timef(ptr, null_pos, null_bit,
                            unireg_check, field_name, 
                            (field_length > MAX_TIME_WIDTH) ?
                            field_length - 1 - MAX_TIME_WIDTH : 0);
   case MYSQL_TYPE_DATETIME:
-    return new (mem_root) Field_datetime(ptr, null_pos, null_bit,
+    return new Field_datetime(ptr, null_pos, null_bit,
                               unireg_check, field_name);
   case MYSQL_TYPE_DATETIME2:
-    return new (mem_root) Field_datetimef(ptr, null_pos, null_bit,
+    return new Field_datetimef(ptr, null_pos, null_bit,
                                unireg_check, field_name,
                                (field_length > MAX_DATETIME_WIDTH) ?
                                field_length - 1 - MAX_DATETIME_WIDTH : 0);
   case MYSQL_TYPE_NULL:
-    return new (mem_root) Field_null(ptr, field_length, unireg_check, field_name,
+    return new Field_null(ptr, field_length, unireg_check, field_name,
                           field_charset);
   case MYSQL_TYPE_BIT:
     return f_bit_as_char(pack_flag) ?
-           new (mem_root) Field_bit_as_char(ptr, field_length, null_pos, null_bit,
+           new Field_bit_as_char(ptr, field_length, null_pos, null_bit,
                                  unireg_check, field_name) :
-           new (mem_root) Field_bit(ptr, field_length, null_pos, null_bit, bit_ptr,
+           new Field_bit(ptr, field_length, null_pos, null_bit, bit_ptr,
                          bit_offset, unireg_check, field_name);
 
   default:					// Impossible (Wrong version)

diff --git a/sql/field.h b/sql/field.h
@@ -4167,8 +4167,7 @@ Field *make_field(TABLE_SHARE *share, uchar *ptr, size_t field_length,
 		  const CHARSET_INFO *cs,
 		  Field::geometry_type geom_type,
 		  Field::utype unireg_check,
-		  TYPELIB *interval, const char *field_name,
-		  MEM_ROOT *mem_root= NULL);
+		  TYPELIB *interval, const char *field_name);
 uint pack_length_to_packflag(uint type);
 enum_field_types get_blob_type_from_length(ulong length);
 size_t calc_pack_length(enum_field_types type, size_t length);

diff --git a/sql/log_event.cc b/sql/log_event.cc
@@ -880,9 +880,6 @@ Log_event::Log_event(THD* thd_arg, uint16 flags_arg,
   server_id= thd->server_id;
   unmasked_server_id= server_id;
   when= thd->start_time;
-#ifdef HAVE_REPLICATION
-  init_sql_alloc(PSI_INSTRUMENT_ME, &m_event_mem_root, 4096, 0);
-#endif //HAVE_REPLICATION
 }
 
 /**
@@ -907,9 +904,6 @@ Log_event::Log_event(enum_event_cache_type cache_type_arg,
   when.tv_sec=  0;
   when.tv_usec= 0;
   log_pos=	0;
-#ifdef HAVE_REPLICATION
-  init_sql_alloc(PSI_INSTRUMENT_ME, &m_event_mem_root, 4096, 0);
-#endif //HAVE_REPLICATION
 }
 #endif /* !MYSQL_CLIENT */
 
@@ -927,11 +921,7 @@ Log_event::Log_event(const char* buf,
 {
 #ifndef MYSQL_CLIENT
   thd = 0;
-#ifdef HAVE_REPLICATION
-  init_sql_alloc(PSI_INSTRUMENT_ME, &m_event_mem_root, 4096, 0);
-#endif //HAVE_REPLICATION
 #endif
-
   when.tv_sec= uint4korr(buf);
   when.tv_usec= 0;
   server_id = uint4korr(buf + SERVER_ID_OFFSET);
@@ -10232,8 +10222,7 @@ Rows_log_event::decide_row_lookup_algorithm_and_key()
   this->m_key_index= search_key_in_table(table, cols, (PRI_KEY_FLAG | UNIQUE_KEY_FLAG | MULTIPLE_KEY_FLAG));
   this->m_rows_lookup_algorithm= ROW_LOOKUP_HASH_SCAN;
   if (m_key_index < MAX_KEY)
-    m_distinct_key_spare_buf= (uchar*) alloc_root(&m_event_mem_root,
-                                                  table->key_info[m_key_index].key_length);
+    m_distinct_key_spare_buf= (uchar*) thd->alloc(table->key_info[m_key_index].key_length);
   DBUG_PRINT("info", ("decide_row_lookup_algorithm_and_key: decided - HASH_SCAN"));
   goto end;
 
@@ -10764,8 +10753,7 @@ Rows_log_event::add_key_to_distinct_keyset()
   if (ret.second)
   {
     /* Insert is successful, so allocate a new buffer for next key */
-    m_distinct_key_spare_buf= (uchar*) alloc_root(&m_event_mem_root,
-                                                  m_key_info->key_length);
+    m_distinct_key_spare_buf= (uchar*) thd->alloc(m_key_info->key_length);
     if (!m_distinct_key_spare_buf)
     {
       error= HA_ERR_OUT_OF_MEM;
@@ -11475,13 +11463,8 @@ int Rows_log_event::do_apply_event(Relay_log_info const *rli)
       {
         DBUG_ASSERT(ptr->m_tabledef_valid);
         TABLE *conv_table;
-        /*
-          Use special mem_root 'Log_event::m_event_mem_root' while doing
-          compatiblity check (i.e., while creating temporary table)
-         */
         if (!ptr->m_tabledef.compatible_with(thd, const_cast<Relay_log_info*>(rli),
-                                             ptr->table,
-                                             &conv_table,&m_event_mem_root))
+                                             ptr->table, &conv_table))
         {
           DBUG_PRINT("debug", ("Table: %s.%s is not compatible with master",
                                ptr->table->s->db.str,
@@ -11761,13 +11744,26 @@ int Rows_log_event::do_apply_event(Relay_log_info const *rli)
     DBUG_RETURN(error);
   }
 
-  if (get_flags(STMT_END_F) && (error= rows_event_stmt_cleanup(rli, thd)))
+  if (get_flags(STMT_END_F))
+  {
+   if((error= rows_event_stmt_cleanup(rli, thd)))
     slave_rows_error_report(ERROR_LEVEL,
                             thd->is_error() ? 0 : error,
                             rli, thd, table,
                             get_type_str(),
                             const_cast<Relay_log_info*>(rli)->get_rpl_log_name(),
                             (ulong) log_pos);
+   /* We are at end of the statement (STMT_END_F flag), lets clean
+     the memory which was used from thd's mem_root now.
+     This needs to be done only if we are here in SQL thread context.
+     In other flow ( in case of a regular thread which can happen
+     when the thread is applying BINLOG'...' row event) we should
+     *not* try to free the memory here. It will be done latter
+     in dispatch_command() after command execution is completed.
+    */
+   if (thd->slave_thread)
+     free_root(thd->mem_root, MYF(MY_KEEP_PREALLOC));
+  }
   DBUG_RETURN(error);
 }
 

diff --git a/sql/log_event.h b/sql/log_event.h
@@ -1456,13 +1456,7 @@ class Log_event
   }
   Log_event(const char* buf, const Format_description_log_event
             *description_event);
-  virtual ~Log_event()
-  {
-    free_temp_buf();
-#if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
-    free_root(&m_event_mem_root, MYF(MY_KEEP_PREALLOC));
-#endif //!MYSQL_CLIENT && HAVE_REPLICATION
-  }
+  virtual ~Log_event() { free_temp_buf();}
   void register_temp_buf(char* buf) { temp_buf = buf; }
   void free_temp_buf()
   {
@@ -1735,13 +1729,6 @@ class Log_event
 
   virtual int do_apply_event_worker(Slave_worker *w);
 
-  /*
-    Mem root whose scope is equalent to event's scope.
-    This mem_root will be initialized in constructor
-    Log_event() and freed in destructor ~Log_event().
-   */
-  MEM_ROOT m_event_mem_root;
-
 protected:
 
   /**