From 7d4c684633529650ae3bf3654d6b9845c86c44d7 Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 23 Feb 2006 13:34:03 +0100 Subject: [PATCH 1/2] Bug#17181 (mysqlslap test server crash): Moving assignments to table_map_id for thread-safe handling of table shares. sql/ha_ndbcluster_binlog.cc: Assign_new_table_id() now takes table share instead of table. sql/mysql_priv.h: New protptype for assign_new_table_id(). sql/sql_base.cc: Assign_new_table_id() now takes a table share instead of a table. Moving call to assign_new_table_id() into get_table_share(). sql/table.cc: Setting default values of table_map_id and table_map_version inside alloc_table_share() and init_tmp_table_share(). Removing the settings from open_table_from_share(). --- sql/ha_ndbcluster_binlog.cc | 2 +- sql/mysql_priv.h | 2 +- sql/sql_base.cc | 61 +++++++++++++++++++++---------------- sql/table.cc | 46 +++++++++++++++++----------- 4 files changed, 65 insertions(+), 46 deletions(-) diff --git a/sql/ha_ndbcluster_binlog.cc b/sql/ha_ndbcluster_binlog.cc index 06f946b013c..2c2624358af 100644 --- a/sql/ha_ndbcluster_binlog.cc +++ b/sql/ha_ndbcluster_binlog.cc @@ -310,7 +310,7 @@ void ndbcluster_binlog_init_share(NDB_SHARE *share, TABLE *_table) table= 0; break; } - assign_new_table_id(table); + assign_new_table_id(table_share); if (!table->record[1] || table->record[1] == table->record[0]) { table->record[1]= alloc_root(&table->mem_root, diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index fc777c47818..d4f9ddfa167 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -657,7 +657,7 @@ bool table_cache_init(void); void table_cache_free(void); bool table_def_init(void); void table_def_free(void); -void assign_new_table_id(TABLE *table); +void assign_new_table_id(TABLE_SHARE *share); uint cached_open_tables(void); uint cached_table_definitions(void); void kill_mysql(void); diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 2d6576bff8b..f48df61e9a1 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -313,6 +313,22 @@ TABLE_SHARE *get_table_share(THD *thd, TABLE_LIST *table_list, char *key, conflicts */ (void) pthread_mutex_lock(&share->mutex); + + /* + We assign a new table id under the protection of the LOCK_open and + the share's own mutex. We do this insted of creating a new mutex + and using it for the sole purpose of serializing accesses to a + static variable, we assign the table id here. We assign it to the + share before inserting it into the table_def_cache to be really + sure that it cannot be read from the cache without having a table + id assigned. + + CAVEAT. This means that the table cannot be used for + binlogging/replication purposes, unless get_table_share() has been + called directly or indirectly. + */ + assign_new_table_id(share); + if (my_hash_insert(&table_def_cache, (byte*) share)) { #ifdef NOT_YET @@ -2383,43 +2399,50 @@ void abort_locked_tables(THD *thd,const char *db, const char *table_name) /* - Function to assign a new table map id to a table. + Function to assign a new table map id to a table share. PARAMETERS - table - Pointer to table structure + share - Pointer to table share structure PRE-CONDITION(S) - table is non-NULL + share is non-NULL The LOCK_open mutex is locked + The share->mutex is locked POST-CONDITION(S) - table->s->table_map_id is given a value that with a high certainty - is not used by any other table. + share->table_map_id is given a value that with a high certainty is + not used by any other table (the only case where a table id can be + reused is on wrap-around, which means more than 4 billion table + shares open at the same time). - table->s->table_map_id is not ULONG_MAX. + share->table_map_id is not ULONG_MAX. */ -void assign_new_table_id(TABLE *table) +void assign_new_table_id(TABLE_SHARE *share) { static ulong last_table_id= ULONG_MAX; - DBUG_ENTER("assign_new_table_id(TABLE*)"); + DBUG_ENTER("assign_new_table_id"); /* Preconditions */ - DBUG_ASSERT(table != NULL); + DBUG_ASSERT(share != NULL); safe_mutex_assert_owner(&LOCK_open); + safe_mutex_assert_owner(&share->mutex); ulong tid= ++last_table_id; /* get next id */ - /* There is one reserved number that cannot be used. */ + /* + There is one reserved number that cannot be used. Remember to + change this when 6-byte global table id's are introduced. + */ if (unlikely(tid == ULONG_MAX)) tid= ++last_table_id; - table->s->table_map_id= tid; + share->table_map_id= tid; DBUG_PRINT("info", ("table_id=%lu", tid)); /* Post conditions */ - DBUG_ASSERT(table->s->table_map_id != ULONG_MAX); + DBUG_ASSERT(share->table_map_id != ULONG_MAX); DBUG_VOID_RETURN; } @@ -2573,20 +2596,6 @@ retry: break; } - /* - We assign a new table id under the protection of the LOCK_open - mutex. We assign a new table id here instead of inside openfrm() - since that function can be used without acquiring any lock (e.g., - inside ha_create_table()). Insted of creatint a new mutex and - using it for the sole purpose of serializing accesses to a static - variable, we assign the table id here. - - CAVEAT. This means that the table cannot be used for - binlogging/replication purposes, unless open_table() has been called - directly or indirectly. - */ - assign_new_table_id(entry); - if (Table_triggers_list::check_n_load(thd, share->db.str, share->table_name.str, entry, 0)) { diff --git a/sql/table.cc b/sql/table.cc index 7c266243d29..3e766fe6c0f 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -130,6 +130,24 @@ TABLE_SHARE *alloc_table_share(TABLE_LIST *table_list, char *key, share->version= refresh_version; share->flush_version= flush_version; +#ifdef HAVE_ROW_BASED_REPLICATION + /* + This constant is used to mark that no table map version has been + assigned. No arithmetic is done on the value: it will be + overwritten with a value taken from MYSQL_BIN_LOG. + */ + share->table_map_version= ~(ulonglong)0; + + /* + Since alloc_table_share() can be called without any locking (for + example, ha_create_table... functions), we do not assign a table + map id here. Instead we assign a value that is not used + elsewhere, and then assign a table map id inside open_table() + under the protection of the LOCK_open mutex. + */ + share->table_map_id= ULONG_MAX; +#endif + memcpy((char*) &share->mem_root, (char*) &mem_root, sizeof(mem_root)); pthread_mutex_init(&share->mutex, MY_MUTEX_INIT_FAST); pthread_cond_init(&share->cond, NULL); @@ -180,6 +198,15 @@ void init_tmp_table_share(TABLE_SHARE *share, const char *key, share->path.length= share->normalized_path.length= strlen(path); share->frm_version= FRM_VER_TRUE_VARCHAR; +#ifdef HAVE_ROW_BASED_REPLICATION + /* + Temporary tables are not replicated, but we set up these fields + anyway to be able to catch errors. + */ + share->table_map_version= ~(ulonglong)0; + share->table_map_id= ULONG_MAX; +#endif + DBUG_VOID_RETURN; } @@ -371,6 +398,7 @@ err_not_open: share->error= error; open_table_error(share, error, (share->open_errno= my_errno), 0); } + DBUG_RETURN(error); } @@ -1503,24 +1531,6 @@ int open_table_from_share(THD *thd, TABLE_SHARE *share, const char *alias, *root_ptr= old_root; thd->status_var.opened_tables++; -#ifdef HAVE_REPLICATION - - /* - This constant is used to mark that no table map version has been - assigned. No arithmetic is done on the value: it will be - overwritten with a value taken from MYSQL_BIN_LOG. - */ - share->table_map_version= ~(ulonglong)0; - - /* - Since openfrm() can be called without any locking (for example, - ha_create_table... functions), we do not assign a table map id - here. Instead we assign a value that is not used elsewhere, and - then assign a table map id inside open_table() under the - protection of the LOCK_open mutex. - */ - share->table_map_id= ULONG_MAX; -#endif DBUG_RETURN (0); From e2994b285731bbf9aaf54133c5add4104b4500d6 Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 23 Feb 2006 22:19:47 +0100 Subject: [PATCH 2/2] Bug#17678 (RBR format change: moving from VLE to net encoding): Switching to use net_*_length() instead of VLE functions. sql/log_event.cc: Switching to use net_*_length() instead of VLE functions. --- sql/log_event.cc | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/sql/log_event.cc b/sql/log_event.cc index cd16745df90..3c7dcc250c8 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -28,7 +28,6 @@ #endif /* MYSQL_CLIENT */ #include #include -#include #define log_cs &my_charset_latin1 @@ -5128,7 +5127,8 @@ Rows_log_event::Rows_log_event(const char *buf, uint event_len, byte const *const var_start= (const byte *)buf + common_header_len + post_header_len; byte const *const ptr_width= var_start; - byte const *const ptr_after_width= my_vle_decode(&m_width, ptr_width); + uchar *ptr_after_width= (uchar*) ptr_width; + m_width = net_field_length(&ptr_after_width); const uint byte_count= (m_width + 7) / 8; const byte* const ptr_rows_data= var_start + byte_count + 1; @@ -5598,13 +5598,13 @@ bool Rows_log_event::write_data_body(IO_CACHE*file) Note that this should be the number of *bits*, not the number of bytes. */ - byte sbuf[my_vle_sizeof(m_width)]; + byte sbuf[sizeof(m_width)]; my_ptrdiff_t const data_size= m_rows_cur - m_rows_buf; - char *const sbuf_end= (char *const)my_vle_encode(sbuf, sizeof(sbuf), m_width); - DBUG_ASSERT(static_cast(sbuf_end - (char *const)sbuf) <= sizeof(sbuf)); + char *const sbuf_end= net_store_length(sbuf, (uint) m_width); + DBUG_ASSERT(static_cast(sbuf_end - (char*) sbuf) <= sizeof(sbuf)); - return (my_b_safe_write(file, sbuf, sbuf_end - (char *const)sbuf) || + return (my_b_safe_write(file, sbuf, sbuf_end - (char*) sbuf) || my_b_safe_write(file, reinterpret_cast(m_cols.bitmap), no_bytes_in_map(&m_cols)) || my_b_safe_write(file, m_rows_buf, data_size)); @@ -5731,7 +5731,8 @@ Table_map_log_event::Table_map_log_event(const char *buf, uint event_len, /* Length of table name + counter + terminating null */ byte const* const ptr_colcnt= ptr_tbllen + m_tbllen + 2; - byte const* const ptr_after_colcnt= my_vle_decode(&m_colcnt, ptr_colcnt); + uchar *ptr_after_colcnt= (uchar*) ptr_colcnt; + m_colcnt= net_field_length(&ptr_after_colcnt); DBUG_PRINT("info",("m_dblen=%d off=%d m_tbllen=%d off=%d m_colcnt=%d off=%d", m_dblen, ptr_dblen-(const byte*)vpart, @@ -6043,9 +6044,9 @@ bool Table_map_log_event::write_data_body(IO_CACHE *file) byte const dbuf[]= { m_dblen }; byte const tbuf[]= { m_tbllen }; - byte cbuf[my_vle_sizeof(m_colcnt)]; - byte *const cbuf_end= my_vle_encode(cbuf, sizeof(cbuf), m_colcnt); - DBUG_ASSERT(static_cast(cbuf_end - cbuf) <= sizeof(cbuf)); + byte cbuf[sizeof(m_colcnt)]; + char *const cbuf_end= net_store_length(cbuf, (uint) m_colcnt); + DBUG_ASSERT(static_cast(cbuf_end - (char*) cbuf) <= sizeof(cbuf)); return (my_b_safe_write(file, dbuf, sizeof(dbuf)) || my_b_safe_write(file, (const byte*)m_dbnam, m_dblen+1) ||