From d7bfd59a5a7c94ca337714377975c5927b09b69a Mon Sep 17 00:00:00 2001 From: Mattias Jonsson Date: Thu, 30 Sep 2010 15:57:33 +0200 Subject: [PATCH 1/2] Bug#55458: Partitioned MyISAM table gets crashed by multi-table update Bug#57113: ha_partition::extra(ha_extra_function): Assertion `m_extra_cache' failed Fix for bug#55458 included DBUG_ASSERTS causing debug builds of the server to crash on another multi-table update. Removed the asserts since they where wrong. (updated after testing the patch in 5.5). --- mysql-test/r/partition.result | 15 +++++++++++++++ mysql-test/t/partition.test | 22 ++++++++++++++++++++++ sql/ha_partition.cc | 18 +++++++++++------- 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/mysql-test/r/partition.result b/mysql-test/r/partition.result index bb1635b8621..1267ec70b0a 100644 --- a/mysql-test/r/partition.result +++ b/mysql-test/r/partition.result @@ -1,5 +1,20 @@ drop table if exists t1, t2; # +# Bug#57113: ha_partition::extra(ha_extra_function): +# Assertion `m_extra_cache' failed +CREATE TABLE t1 +(id INT NOT NULL PRIMARY KEY, +name VARCHAR(16) NOT NULL, +year YEAR, +INDEX name (name(8)) +) +PARTITION BY HASH(id) PARTITIONS 2; +INSERT INTO t1 VALUES ( 1, 'FooBar', '1924' ); +CREATE TABLE t2 (id INT); +INSERT INTO t2 VALUES (1),(2); +UPDATE t1, t2 SET t1.year = '1955' WHERE t1.name = 'FooBar'; +DROP TABLE t1, t2; +# # Bug#55458: Partitioned MyISAM table gets crashed by multi-table update # CREATE TABLE t1 ( diff --git a/mysql-test/t/partition.test b/mysql-test/t/partition.test index 95d702ee6b8..86e2603cd01 100644 --- a/mysql-test/t/partition.test +++ b/mysql-test/t/partition.test @@ -14,6 +14,28 @@ drop table if exists t1, t2; --enable_warnings +--echo # +--echo # Bug#57113: ha_partition::extra(ha_extra_function): +--echo # Assertion `m_extra_cache' failed +CREATE TABLE t1 +(id INT NOT NULL PRIMARY KEY, + name VARCHAR(16) NOT NULL, + year YEAR, + INDEX name (name(8)) +) +PARTITION BY HASH(id) PARTITIONS 2; + +INSERT INTO t1 VALUES ( 1, 'FooBar', '1924' ); + +CREATE TABLE t2 (id INT); + +INSERT INTO t2 VALUES (1),(2); + +UPDATE t1, t2 SET t1.year = '1955' WHERE t1.name = 'FooBar'; + +DROP TABLE t1, t2; + + --echo # --echo # Bug#55458: Partitioned MyISAM table gets crashed by multi-table update --echo # diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc index 4fff6cc703c..42920ef18f7 100644 --- a/sql/ha_partition.cc +++ b/sql/ha_partition.cc @@ -5550,7 +5550,6 @@ int ha_partition::extra(enum ha_extra_function operation) DBUG_RETURN(prepare_for_rename()); break; case HA_EXTRA_PREPARE_FOR_UPDATE: - DBUG_ASSERT(m_extra_cache); /* Needs to be run on the first partition in the range now, and later in late_extra_cache, when switching to a new partition to scan. @@ -5558,6 +5557,8 @@ int ha_partition::extra(enum ha_extra_function operation) m_extra_prepare_for_update= TRUE; if (m_part_spec.start_part != NO_CURRENT_PART_ID) { + if (!m_extra_cache) + m_extra_cache_part_id= m_part_spec.start_part; DBUG_ASSERT(m_extra_cache_part_id == m_part_spec.start_part); VOID(m_file[m_part_spec.start_part]->extra(HA_EXTRA_PREPARE_FOR_UPDATE)); } @@ -5820,19 +5821,22 @@ void ha_partition::late_extra_cache(uint partition_id) { handler *file; DBUG_ENTER("ha_partition::late_extra_cache"); - DBUG_PRINT("info", ("extra_cache %u partid %u size %u", m_extra_cache, + DBUG_PRINT("info", ("extra_cache %u prepare %u partid %u size %u", + m_extra_cache, m_extra_prepare_for_update, partition_id, m_extra_cache_size)); if (!m_extra_cache && !m_extra_prepare_for_update) DBUG_VOID_RETURN; file= m_file[partition_id]; - if (m_extra_cache_size == 0) - VOID(file->extra(HA_EXTRA_CACHE)); - else - VOID(file->extra_opt(HA_EXTRA_CACHE, m_extra_cache_size)); + if (m_extra_cache) + { + if (m_extra_cache_size == 0) + VOID(file->extra(HA_EXTRA_CACHE)); + else + VOID(file->extra_opt(HA_EXTRA_CACHE, m_extra_cache_size)); + } if (m_extra_prepare_for_update) { - DBUG_ASSERT(m_extra_cache); VOID(file->extra(HA_EXTRA_PREPARE_FOR_UPDATE)); } m_extra_cache_part_id= partition_id; From a01773dbee1318e0894057daed544188b14f90c8 Mon Sep 17 00:00:00 2001 From: Mattias Jonsson Date: Fri, 1 Oct 2010 13:39:04 +0200 Subject: [PATCH 2/2] Bug#51851: Server with SBR locks mutex twice on LOAD DATA into partitioned MyISAM table Problem was that both partitioning and myisam used the same table_share->mutex for different protections (auto inc and repair). Solved by adding a specific mutex for the partitioning auto_increment. Also adding destroying the ha_data structure in free_table_share (which is to be propagated into 5.5). This is a 5.1 ONLY patch, already fixed in 5.5+. --- mysql-test/r/partition_binlog_stmt.result | 13 ++++++++++++ mysql-test/t/partition_binlog_stmt.test | 26 +++++++++++++++++++++++ sql/ha_partition.cc | 17 +++++++++++++++ sql/ha_partition.h | 7 ++++-- sql/table.cc | 10 +++++++++ sql/table.h | 1 + 6 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 mysql-test/r/partition_binlog_stmt.result create mode 100644 mysql-test/t/partition_binlog_stmt.test diff --git a/mysql-test/r/partition_binlog_stmt.result b/mysql-test/r/partition_binlog_stmt.result new file mode 100644 index 00000000000..9be23636ca6 --- /dev/null +++ b/mysql-test/r/partition_binlog_stmt.result @@ -0,0 +1,13 @@ +DROP TABLE IF EXISTS t1; +# +# Bug#51851: Server with SBR locks mutex twice on LOAD DATA into +# partitioned MyISAM table +CREATE TABLE t1 +(id INT NOT NULL AUTO_INCREMENT PRIMARY KEY, +name TINYBLOB NOT NULL, +modified TIMESTAMP DEFAULT '0000-00-00 00:00:00', +INDEX namelocs (name(255))) ENGINE = MyISAM +PARTITION BY HASH(id) PARTITIONS 2; +LOAD DATA LOCAL INFILE 'init_file.txt' +INTO TABLE t1 (name); +DROP TABLE t1; diff --git a/mysql-test/t/partition_binlog_stmt.test b/mysql-test/t/partition_binlog_stmt.test new file mode 100644 index 00000000000..c426de9f303 --- /dev/null +++ b/mysql-test/t/partition_binlog_stmt.test @@ -0,0 +1,26 @@ +--source include/have_partition.inc +--source include/have_binlog_format_statement.inc + +--disable_warnings +DROP TABLE IF EXISTS t1; +--enable_warnings + +--echo # +--echo # Bug#51851: Server with SBR locks mutex twice on LOAD DATA into +--echo # partitioned MyISAM table +--write_file init_file.txt +abcd +EOF + +CREATE TABLE t1 +(id INT NOT NULL AUTO_INCREMENT PRIMARY KEY, + name TINYBLOB NOT NULL, + modified TIMESTAMP DEFAULT '0000-00-00 00:00:00', + INDEX namelocs (name(255))) ENGINE = MyISAM +PARTITION BY HASH(id) PARTITIONS 2; + +LOAD DATA LOCAL INFILE 'init_file.txt' +INTO TABLE t1 (name); + +--remove_file init_file.txt +DROP TABLE t1; diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc index 624ef1aff5b..6806e9c5ac0 100644 --- a/sql/ha_partition.cc +++ b/sql/ha_partition.cc @@ -2449,6 +2449,21 @@ err1: /**************************************************************************** MODULE open/close object ****************************************************************************/ + + +/** + A destructor for partition-specific TABLE_SHARE data. +*/ + +void ha_data_partition_destroy(void *ha_data) +{ + if (ha_data) + { + HA_DATA_PARTITION *ha_part_data= (HA_DATA_PARTITION*) ha_data; + pthread_mutex_destroy(&ha_part_data->LOCK_auto_inc); + } +} + /* Open handler object @@ -2605,6 +2620,8 @@ int ha_partition::open(const char *name, int mode, uint test_if_locked) } DBUG_PRINT("info", ("table_share->ha_data 0x%p", ha_data)); bzero(ha_data, sizeof(HA_DATA_PARTITION)); + table_share->ha_data_destroy= ha_data_partition_destroy; + VOID(pthread_mutex_init(&ha_data->LOCK_auto_inc, MY_MUTEX_INIT_FAST)); } if (is_not_tmp_table) pthread_mutex_unlock(&table_share->mutex); diff --git a/sql/ha_partition.h b/sql/ha_partition.h index e3dc7d17c6d..cb5440a0b69 100644 --- a/sql/ha_partition.h +++ b/sql/ha_partition.h @@ -44,6 +44,7 @@ typedef struct st_partition_share typedef struct st_ha_data_partition { ulonglong next_auto_inc_val; /**< first non reserved value */ + pthread_mutex_t LOCK_auto_inc; bool auto_inc_initialized; } HA_DATA_PARTITION; @@ -944,8 +945,9 @@ private: DBUG_ASSERT(table_share->ha_data && !auto_increment_lock); if(table_share->tmp_table == NO_TMP_TABLE) { + HA_DATA_PARTITION *ha_data= (HA_DATA_PARTITION*) table_share->ha_data; auto_increment_lock= TRUE; - pthread_mutex_lock(&table_share->mutex); + pthread_mutex_lock(&ha_data->LOCK_auto_inc); } } virtual void unlock_auto_increment() @@ -958,7 +960,8 @@ private: */ if(auto_increment_lock && !auto_increment_safe_stmt_log_lock) { - pthread_mutex_unlock(&table_share->mutex); + HA_DATA_PARTITION *ha_data= (HA_DATA_PARTITION*) table_share->ha_data; + pthread_mutex_unlock(&ha_data->LOCK_auto_inc); auto_increment_lock= FALSE; } } diff --git a/sql/table.cc b/sql/table.cc index e989ab039a0..18523f08551 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -425,6 +425,11 @@ void free_table_share(TABLE_SHARE *share) key_info->flags= 0; } } + if (share->ha_data_destroy) + { + share->ha_data_destroy(share->ha_data); + share->ha_data_destroy= NULL; + } /* We must copy mem_root from share because share is allocated through it */ memcpy((char*) &mem_root, (char*) &share->mem_root, sizeof(mem_root)); free_root(&mem_root, MYF(0)); // Free's share @@ -1616,6 +1621,11 @@ static int open_binary_frm(THD *thd, TABLE_SHARE *share, uchar *head, delete crypted; delete handler_file; hash_free(&share->name_hash); + if (share->ha_data_destroy) + { + share->ha_data_destroy(share->ha_data); + share->ha_data_destroy= NULL; + } open_table_error(share, error, share->open_errno, errarg); DBUG_RETURN(error); diff --git a/sql/table.h b/sql/table.h index bbb39aae6f7..132279169cb 100644 --- a/sql/table.h +++ b/sql/table.h @@ -463,6 +463,7 @@ typedef struct st_table_share /** place to store storage engine specific data */ void *ha_data; + void (*ha_data_destroy)(void *); /* An optional destructor for ha_data. */ /*