Fix for BUG#1686

"If 2 master threads with same-name temp table, slave makes bad binlog"
and (two birds with one stone) for
BUG#1240 "slave of slave breaks when STOP SLAVE was issud on parent slave
and temp tables".

Here is the design change:
in a slave running with --log-slave-updates, events are now logged with the
thread id they had on the master. So no more id conflicts between master threads,
but introduces id conflicts between one master thread and one normal 
client thread connected to the slave. This is solved by storing the server id
in the temp table's name.

New test which requires mysql-test-run to be run with --manager,
otherwise it will be skipped.

Undoing a Monty's change (hum, a chill runs down my spine ;) which was
"Cleanup temporary tables when slave ends" in ChangeSet 1.1572.1.1.

mysql-test/mysql-test-run.sh

@@ -1161,7 +1161,7 @@ run_testcase ()
  echo $tname > $CURRENT_TEST
  SKIP_SLAVE=`$EXPR \( $tname : rpl \) = 0`
  if [ $USE_MANAGER = 1 ] ; then
-  many_slaves=`$EXPR \( $tname : rpl_failsafe \) != 0`
+  many_slaves=`$EXPR \( \( $tname : rpl_failsafe \) != 0 \) \| \( \( $tname : rpl_chain_temp_table \) != 0 \)`
  fi
 
  if [ -n "$SKIP_TEST" ] ; then

mysql-test/r/rpl_chain_temp_table.result

+slave stop;
+drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
+reset master;
+reset slave;
+drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
+slave start;
+reset master;
+change master to master_host='127.0.0.1',master_port=9307, master_user='root';
+start slave;
+create temporary table t1 (a int);
+create temporary table t1 (a int);
+show status like 'slave_open_temp_tables';
+Variable_name	Value
+Slave_open_temp_tables	2
+create temporary table t1 (a int);
+create temporary table t1 (a int);
+show status like 'slave_open_temp_tables';
+Variable_name	Value
+Slave_open_temp_tables	4
+stop slave;
+insert into t1 values(1);
+create table t2 as select * from t1;
+start slave;
+show status like 'slave_open_temp_tables';
+Variable_name	Value
+Slave_open_temp_tables	4
+select * from t2;
+a
+1
+drop table t2;

mysql-test/t/rpl_chain_temp_table.test

+ # This test makes some assumptions about values of thread ids, which should be
+# true if the servers have been restarted for this test. So we want to
+# stop/restart servers. Note that if assumptions are wrong, the test will not
+# fail; it will just fail to test the error-prone scenario.
+# Using the manager is the only way to have more than one slave server.
+
+require_manager;
+server_stop master;
+server_start master;
+server_stop slave;
+server_start slave;
+# no need for slave_sec (no assumptions on thread ids for this server).
+
+source include/master-slave.inc;
+connect (slave_sec,localhost,root,,test,0,slave.sock-1); 
+connection master;
+save_master_pos;
+connection slave;
+sync_with_master;
+reset master;
+save_master_pos;
+connection slave_sec;
+eval change master to master_host='127.0.0.1',master_port=$SLAVE_MYPORT, master_user='root';
+start slave;
+sync_with_master;
+
+# :P now we have a chain ready-to-test.
+
+connection master;
+create temporary table t1 (a int);
+save_master_pos;
+connection slave;
+sync_with_master;
+connection master1;
+create temporary table t1 (a int);
+save_master_pos;
+connection slave;
+sync_with_master;
+save_master_pos;
+
+# First test:
+
+connection slave_sec;
+# Before BUG#1686 ("If 2 master threads with same-name temp table, slave makes
+# bad binlog") was fixed, sync_with_master failed
+sync_with_master;
+show status like 'slave_open_temp_tables';
+
+# 'master' and 'master1' usually have thread id 2-3 or 3-4.
+# 'slave' and 'slave1' usually have thread id 2-3.
+connection slave;
+create temporary table t1 (a int);
+connection slave1;
+create temporary table t1 (a int);
+# So it's likely that in the binlog of slave we get
+# server_id=of_master thread_id=3 create temp...
+# server_id=of_slave  thread_id=3 create temp...
+# which would confuse slave-sec unless slave-sec uses server id to distinguish
+# between temp tables (here thread id is obviously not enough to distinguish).
+
+save_master_pos;
+
+# Second test:
+
+connection slave_sec;
+# If we did not use the server id to distinguish between temp tables,
+# sync_with_master would fail
+sync_with_master;
+show status like 'slave_open_temp_tables';
+
+# Third test (BUG#1240 "slave of slave breaks when STOP SLAVE was issud on
+# parent slave and temp tables").
+stop slave;
+connection slave;
+insert into t1 values(1);
+create table t2 as select * from t1;
+save_master_pos;
+connection slave_sec;
+start slave;
+sync_with_master;
+show status like 'slave_open_temp_tables';
+select * from t2;
+
+# clean up
+connection slave;
+drop table t2;
+save_master_pos;
+connection slave_sec;
+sync_with_master;

sql/log_event.cc

@@ -834,8 +834,11 @@ Query_log_event::Query_log_event(THD* thd_arg, const char* query_arg,
 				 ulong query_length, bool using_trans)
   :Log_event(thd_arg, 0, using_trans), data_buf(0), query(query_arg),
    db(thd_arg->db), q_len((uint32) query_length),
-  error_code(thd_arg->killed ? ER_SERVER_SHUTDOWN: thd_arg->net.last_errno),
-  thread_id(thd_arg->thread_id)
+   error_code(thd_arg->killed ? ER_SERVER_SHUTDOWN: thd_arg->net.last_errno),
+   thread_id(thd_arg->thread_id),
+   /* save the original thread id; we already know the server id */
+   slave_proxy_id(thd_arg->slave_proxy_id)
+
 {
   time_t end_time;
   time(&end_time);
@@ -918,7 +921,42 @@ int Query_log_event::write_data(IO_CACHE* file)
     return -1;
   
   char buf[QUERY_HEADER_LEN]; 
-  int4store(buf + Q_THREAD_ID_OFFSET, thread_id);
+  /*
+    We want to store the thread id:
+    (- as an information for the user when he reads the binlog)
+    - if the query uses temporary table: for the slave SQL thread to know to
+    which master connection the temp table belongs.
+    Now imagine we (write_data()) are called by the slave SQL thread (we are
+    logging a query executed by this thread; the slave runs with
+    --log-slave-updates). Then this query will be logged with
+    thread_id=the_thread_id_of_the_SQL_thread. Imagine that 2 temp tables of the
+    same name were created simultaneously on the master (in the master binlog
+    you have
+    CREATE TEMPORARY TABLE t; (thread 1)
+    CREATE TEMPORARY TABLE t; (thread 2)
+    ...)
+    then in the slave's binlog there will be
+    CREATE TEMPORARY TABLE t; (thread_id_of_the_slave_SQL_thread)
+    CREATE TEMPORARY TABLE t; (thread_id_of_the_slave_SQL_thread)
+    which is bad (same thread id!).
+    To avoid this, we log the thread's thread id EXCEPT for the SQL slave thread
+    for which we log the original (master's) thread id.
+    Now this moves the bug: what happens if the thread id on the master was 10
+    and when the slave replicates the query, a connection number 10 is opened by
+    a normal client on the slave, and updates a temp table of the same name? We
+    get a problem again. To avoid this, in the handling of temp tables
+    (sql_base.cc) we use thread_id AND server_id.
+    TODO when this is merged into 4.1: in 4.1, slave_proxy_id has been renamed
+    to pseudo_thread_id and is a session variable: that's to make mysqlbinlog
+    work with temp tables. We probably need to introduce
+    SET PSEUDO_SERVER_ID
+    for mysqlbinlog in 4.1. mysqlbinlog would print:
+    SET PSEUDO_SERVER_ID=
+    SET PSEUDO_THREAD_ID=
+    for each query using temp tables.
+  */
+  int4store(buf + Q_THREAD_ID_OFFSET, (slave_proxy_id ? slave_proxy_id :
+                                       thread_id));
   int4store(buf + Q_EXEC_TIME_OFFSET, exec_time);
   buf[Q_DB_LEN_OFFSET] = (char) db_len;
   int2store(buf + Q_ERR_CODE_OFFSET, error_code);
@@ -1019,7 +1057,8 @@ void Rand_log_event::print(FILE* file, bool short_form, char* last_db)
 int Load_log_event::write_data_header(IO_CACHE* file)
 {
   char buf[LOAD_HEADER_LEN];
-  int4store(buf + L_THREAD_ID_OFFSET, thread_id);
+  int4store(buf + L_THREAD_ID_OFFSET, (slave_proxy_id ? slave_proxy_id :
+                                       thread_id));
   int4store(buf + L_EXEC_TIME_OFFSET, exec_time);
   int4store(buf + L_SKIP_LINES_OFFSET, skip_lines);
   buf[L_TBL_LEN_OFFSET] = (char)table_name_len;
@@ -1143,10 +1182,11 @@ Load_log_event::Load_log_event(THD* thd_arg, sql_exchange* ex,
 			       enum enum_duplicates handle_dup,
 			       bool using_trans)
   :Log_event(thd_arg, 0, using_trans),thread_id(thd_arg->thread_id),
-  num_fields(0),fields(0),
-  field_lens(0),field_block_len(0),
-  table_name(table_name_arg ? table_name_arg : ""),
-  db(db_arg), fname(ex->file_name)
+   slave_proxy_id(thd_arg->slave_proxy_id),
+   num_fields(0),fields(0),
+   field_lens(0),field_block_len(0),
+   table_name(table_name_arg ? table_name_arg : ""),
+   db(db_arg), fname(ex->file_name)
 {
   time_t end_time;
   time(&end_time);

sql/log_event.h

@@ -320,6 +320,13 @@ class Query_log_event: public Log_event
   uint32 db_len;
   uint16 error_code;
   ulong thread_id;
+  /* 
+     For events created by Query_log_event::exec_event (and
+     Load_log_event::exec_event()) we need the *original* thread id, to be able
+     to log the event with the original (=master's) thread id (fix for
+     BUG#1686).
+  */
+  ulong slave_proxy_id;
 #ifndef MYSQL_CLIENT
 
   Query_log_event(THD* thd_arg, const char* query_arg, ulong query_length,
@@ -390,6 +397,7 @@ class Load_log_event: public Log_event
 
 public:
   ulong thread_id;
+  ulong slave_proxy_id;
   uint32 table_name_len;
   uint32 db_len;
   uint32 fname_len;

sql/slave.cc

@@ -2719,8 +2719,6 @@ the slave SQL thread with \"SLAVE START\". We stopped at log \
 		  RPL_LOG_NAME, llstr(rli->master_log_pos,llbuff));
 
  err:
-  /* Free temporary tables etc */
-  thd->cleanup();
   VOID(pthread_mutex_lock(&LOCK_thread_count));
   thd->query = thd->db = 0; // extra safety
   VOID(pthread_mutex_unlock(&LOCK_thread_count));

sql/sql_base.cc

@@ -589,6 +589,8 @@ TABLE **find_temporary_table(THD *thd, const char *db, const char *table_name)
   uint	key_length= (uint) (strmov(strmov(key,db)+1,table_name)-key)+1;
   TABLE *table,**prev;
 
+  int4store(key+key_length,thd->server_id);
+  key_length += 4;
   int4store(key+key_length,thd->slave_proxy_id);
   key_length += 4;
 
@@ -617,18 +619,27 @@ bool close_temporary_table(THD *thd, const char *db, const char *table_name)
   return 0;
 }
 
+/*
+  Used by ALTER TABLE when the table is a temporary one. It changes something
+  only if the ALTER contained a RENAME clause (otherwise, table_name is the old
+  name).
+  Prepares a table cache key, which is the concatenation of db, table_name and
+  thd->slave_proxy_id, separated by '\0'.
+*/
 bool rename_temporary_table(THD* thd, TABLE *table, const char *db,
 			    const char *table_name)
 {
   char *key;
   if (!(key=(char*) alloc_root(&table->mem_root,
 			       (uint) strlen(db)+
-			       (uint) strlen(table_name)+6)))
+			       (uint) strlen(table_name)+6+4)))
     return 1;				/* purecov: inspected */
   table->key_length=(uint)
     (strmov((table->real_name=strmov(table->table_cache_key=key,
 				     db)+1),
 	    table_name) - table->table_cache_key)+1;
+  int4store(key+table->key_length,thd->server_id);
+  table->key_length += 4;
   int4store(key+table->key_length,thd->slave_proxy_id);
   table->key_length += 4;
   return 0;
@@ -783,12 +794,13 @@ TABLE *open_table(THD *thd,const char *db,const char *table_name,
   if (thd->killed)
     DBUG_RETURN(0);
   key_length= (uint) (strmov(strmov(key,db)+1,table_name)-key)+1;
-  int4store(key + key_length, thd->slave_proxy_id);
+  int4store(key + key_length, thd->server_id);
+  int4store(key + key_length + 4, thd->slave_proxy_id);
 
   for (table=thd->temporary_tables; table ; table=table->next)
   {
-    if (table->key_length == key_length+4 &&
-	!memcmp(table->table_cache_key,key,key_length+4))
+    if (table->key_length == key_length+8 &&
+	!memcmp(table->table_cache_key,key,key_length+8))
     {
       if (table->query_id == thd->query_id)
       {
@@ -1596,7 +1608,7 @@ TABLE *open_temporary_table(THD *thd, const char *path, const char *db,
     total of 6 extra bytes in my_malloc in addition to table/db stuff
   */
   if (!(tmp_table=(TABLE*) my_malloc(sizeof(*tmp_table)+(uint) strlen(db)+
-				     (uint) strlen(table_name)+6,
+				     (uint) strlen(table_name)+6+4,
 				     MYF(MY_WME))))
     DBUG_RETURN(0);				/* purecov: inspected */
 
@@ -1618,6 +1630,9 @@ TABLE *open_temporary_table(THD *thd, const char *path, const char *db,
 					 strmov(tmp_table->table_cache_key,db)
 					 +1), table_name)
 				 - tmp_table->table_cache_key)+1;
+  int4store(tmp_table->table_cache_key + tmp_table->key_length,
+	    thd->server_id);
+  tmp_table->key_length += 4;
   int4store(tmp_table->table_cache_key + tmp_table->key_length,
 	    thd->slave_proxy_id);
   tmp_table->key_length += 4;

sql/unireg.h

@@ -43,7 +43,7 @@
 
 #define ERRMAPP 1				/* Errormap f|r my_error */
 #define LIBLEN FN_REFLEN-FN_LEN			/* Max l{ngd p} dev */
-#define MAX_DBKEY_LENGTH (FN_LEN*2+6)           /* extra 4 bytes for slave tmp
+#define MAX_DBKEY_LENGTH (FN_LEN*2+1+1+4+4)     /* extra 4+4 bytes for slave tmp
 						 * tables */
 #define MAX_FIELD_NAME 34			/* Max colum name length +2 */
 #define MAX_SYS_VAR_LENGTH 32