From f0f9ab69c7c75241ce0a28a33132e504c39f4a6d Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 19 Nov 2010 12:17:57 +0100 Subject: [PATCH 1/6] Mbug#677407 / MySQL Bug#48883: Stale data from INNODB_LOCKS table. The logic for how to check when to update the table cache for INNODB_LOCKS with real data was flawed. This could result in both not updating the cache often enough (when the table is queried repeatedly with less than 100 milliseconds in-between) resulting in stale data; as well as updating too often (when multiple queries against the table start at around the same time). This caused occasional test failures in innodb_information_schema. Fix by updating the "last updated" timestamp in the right place, when the cache is updated, not when it is read. --- storage/innodb_plugin/trx/trx0i_s.c | 33 +++++++++-------------------- storage/xtradb/trx/trx0i_s.c | 33 +++++++++-------------------- 2 files changed, 20 insertions(+), 46 deletions(-) diff --git a/storage/innodb_plugin/trx/trx0i_s.c b/storage/innodb_plugin/trx/trx0i_s.c index 5bc8302d0c0..c0b9a73a68c 100644 --- a/storage/innodb_plugin/trx/trx0i_s.c +++ b/storage/innodb_plugin/trx/trx0i_s.c @@ -157,10 +157,6 @@ struct trx_i_s_cache_struct { ullint last_read; /*!< last time the cache was read; measured in microseconds since epoch */ - mutex_t last_read_mutex;/*!< mutex protecting the - last_read member - it is updated - inside a shared lock of the - rw_lock member */ i_s_table_cache_t innodb_trx; /*!< innodb_trx table */ i_s_table_cache_t innodb_locks; /*!< innodb_locks table */ i_s_table_cache_t innodb_lock_waits;/*!< innodb_lock_waits table */ @@ -1101,13 +1097,6 @@ can_cache_be_updated( { ullint now; - /* Here we read cache->last_read without acquiring its mutex - because last_read is only updated when a shared rw lock on the - whole cache is being held (see trx_i_s_cache_end_read()) and - we are currently holding an exclusive rw lock on the cache. - So it is not possible for last_read to be updated while we are - reading it. */ - #ifdef UNIV_SYNC_DEBUG ut_a(rw_lock_own(&cache->rw_lock, RW_LOCK_EX)); #endif @@ -1205,6 +1194,12 @@ trx_i_s_possibly_fetch_data_into_cache( /*===================================*/ trx_i_s_cache_t* cache) /*!< in/out: cache */ { + ullint now; + +#ifdef UNIV_SYNC_DEBUG + ut_a(rw_lock_own(&cache->rw_lock, RW_LOCK_EX)); +#endif + if (!can_cache_be_updated(cache)) { return(1); @@ -1217,6 +1212,10 @@ trx_i_s_possibly_fetch_data_into_cache( mutex_exit(&kernel_mutex); + /* update cache last read time */ + now = ut_time_us(NULL); + cache->last_read = now; + return(0); } @@ -1247,16 +1246,12 @@ trx_i_s_cache_init( release kernel_mutex release trx_i_s_cache_t::rw_lock acquire trx_i_s_cache_t::rw_lock, S - acquire trx_i_s_cache_t::last_read_mutex - release trx_i_s_cache_t::last_read_mutex release trx_i_s_cache_t::rw_lock */ rw_lock_create(&cache->rw_lock, SYNC_TRX_I_S_RWLOCK); cache->last_read = 0; - mutex_create(&cache->last_read_mutex, SYNC_TRX_I_S_LAST_READ); - table_cache_init(&cache->innodb_trx, sizeof(i_s_trx_row_t)); table_cache_init(&cache->innodb_locks, sizeof(i_s_locks_row_t)); table_cache_init(&cache->innodb_lock_waits, @@ -1307,18 +1302,10 @@ trx_i_s_cache_end_read( /*===================*/ trx_i_s_cache_t* cache) /*!< in: cache */ { - ullint now; - #ifdef UNIV_SYNC_DEBUG ut_a(rw_lock_own(&cache->rw_lock, RW_LOCK_SHARED)); #endif - /* update cache last read time */ - now = ut_time_us(NULL); - mutex_enter(&cache->last_read_mutex); - cache->last_read = now; - mutex_exit(&cache->last_read_mutex); - rw_lock_s_unlock(&cache->rw_lock); } diff --git a/storage/xtradb/trx/trx0i_s.c b/storage/xtradb/trx/trx0i_s.c index 5bc8302d0c0..c0b9a73a68c 100644 --- a/storage/xtradb/trx/trx0i_s.c +++ b/storage/xtradb/trx/trx0i_s.c @@ -157,10 +157,6 @@ struct trx_i_s_cache_struct { ullint last_read; /*!< last time the cache was read; measured in microseconds since epoch */ - mutex_t last_read_mutex;/*!< mutex protecting the - last_read member - it is updated - inside a shared lock of the - rw_lock member */ i_s_table_cache_t innodb_trx; /*!< innodb_trx table */ i_s_table_cache_t innodb_locks; /*!< innodb_locks table */ i_s_table_cache_t innodb_lock_waits;/*!< innodb_lock_waits table */ @@ -1101,13 +1097,6 @@ can_cache_be_updated( { ullint now; - /* Here we read cache->last_read without acquiring its mutex - because last_read is only updated when a shared rw lock on the - whole cache is being held (see trx_i_s_cache_end_read()) and - we are currently holding an exclusive rw lock on the cache. - So it is not possible for last_read to be updated while we are - reading it. */ - #ifdef UNIV_SYNC_DEBUG ut_a(rw_lock_own(&cache->rw_lock, RW_LOCK_EX)); #endif @@ -1205,6 +1194,12 @@ trx_i_s_possibly_fetch_data_into_cache( /*===================================*/ trx_i_s_cache_t* cache) /*!< in/out: cache */ { + ullint now; + +#ifdef UNIV_SYNC_DEBUG + ut_a(rw_lock_own(&cache->rw_lock, RW_LOCK_EX)); +#endif + if (!can_cache_be_updated(cache)) { return(1); @@ -1217,6 +1212,10 @@ trx_i_s_possibly_fetch_data_into_cache( mutex_exit(&kernel_mutex); + /* update cache last read time */ + now = ut_time_us(NULL); + cache->last_read = now; + return(0); } @@ -1247,16 +1246,12 @@ trx_i_s_cache_init( release kernel_mutex release trx_i_s_cache_t::rw_lock acquire trx_i_s_cache_t::rw_lock, S - acquire trx_i_s_cache_t::last_read_mutex - release trx_i_s_cache_t::last_read_mutex release trx_i_s_cache_t::rw_lock */ rw_lock_create(&cache->rw_lock, SYNC_TRX_I_S_RWLOCK); cache->last_read = 0; - mutex_create(&cache->last_read_mutex, SYNC_TRX_I_S_LAST_READ); - table_cache_init(&cache->innodb_trx, sizeof(i_s_trx_row_t)); table_cache_init(&cache->innodb_locks, sizeof(i_s_locks_row_t)); table_cache_init(&cache->innodb_lock_waits, @@ -1307,18 +1302,10 @@ trx_i_s_cache_end_read( /*===================*/ trx_i_s_cache_t* cache) /*!< in: cache */ { - ullint now; - #ifdef UNIV_SYNC_DEBUG ut_a(rw_lock_own(&cache->rw_lock, RW_LOCK_SHARED)); #endif - /* update cache last read time */ - now = ut_time_us(NULL); - mutex_enter(&cache->last_read_mutex); - cache->last_read = now; - mutex_exit(&cache->last_read_mutex); - rw_lock_s_unlock(&cache->rw_lock); } From adb9fd9578f61a73ae224ede3bcb7adef863c1e3 Mon Sep 17 00:00:00 2001 From: Igor Babaev Date: Tue, 7 Dec 2010 14:48:04 -0800 Subject: [PATCH 2/6] Made sure that SELECT from the test case for bug BUG#56862/64041 uses the same execution plan that is in the output of the corresponding EXPLAIN. --- mysql-test/r/index_merge_innodb.result | 2 +- mysql-test/t/index_merge_innodb.test | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mysql-test/r/index_merge_innodb.result b/mysql-test/r/index_merge_innodb.result index 4c561da6420..fdc5b57a2e3 100644 --- a/mysql-test/r/index_merge_innodb.result +++ b/mysql-test/r/index_merge_innodb.result @@ -692,7 +692,7 @@ id select_type table type possible_keys key key_len ref rows Extra 1 PRIMARY NULL NULL NULL NULL NULL NULL NULL Select tables optimized away 2 DERIVED t1 index_merge PRIMARY,idx idx,PRIMARY 5,4 NULL 11419 Using sort_union(idx,PRIMARY); Using where SELECT COUNT(*) FROM -(SELECT * FROM t1 +(SELECT * FROM t1 FORCE INDEX(primary,idx) WHERE a BETWEEN 2 AND 7 OR pk=1000000) AS t; COUNT(*) 6145 diff --git a/mysql-test/t/index_merge_innodb.test b/mysql-test/t/index_merge_innodb.test index 7c6ffaace4f..04f8c5a659b 100644 --- a/mysql-test/t/index_merge_innodb.test +++ b/mysql-test/t/index_merge_innodb.test @@ -71,7 +71,7 @@ SELECT COUNT(*) FROM (SELECT * FROM t1 FORCE INDEX(primary,idx) WHERE a BETWEEN 2 AND 7 OR pk=1000000) AS t; SELECT COUNT(*) FROM - (SELECT * FROM t1 + (SELECT * FROM t1 FORCE INDEX(primary,idx) WHERE a BETWEEN 2 AND 7 OR pk=1000000) AS t; --replace_column 9 # From 1bbb55a2603f3571585cc085f05149d3d9cdbdba Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 8 Dec 2010 14:34:08 +0100 Subject: [PATCH 3/6] MBug#687320: Fix sporadic test failures in innodb_mysql.test and partition_innodb_semi_consistent.test Problem is that these tests run with --innodb-lock-wait-timeout=2 in .opt (and this is necessary as built-in innodb does not allow to change this dynamically). This cases another part of the test to occasionally time out an UPDATE, which subsequently caused the test case to timeout due to waiting for a condition (successful UPDATE) that never occurs. Fixed by re-trying the update in case of timeout. Tested by inserting a sleep() in the connection that the UPDATE is waiting for, and checking that the retry loops a couple of times until the other connection is done and COMMITs. --- mysql-test/include/mix1.inc | 12 +++++++++++- mysql-test/r/partition_innodb_semi_consistent.result | 10 ++++++++++ mysql-test/suite/innodb/r/innodb_mysql.result | 10 ++++++++++ mysql-test/suite/innodb_plugin/r/innodb_mysql.result | 10 ++++++++++ mysql-test/t/partition_innodb_semi_consistent.test | 12 +++++++++++- 5 files changed, 52 insertions(+), 2 deletions(-) diff --git a/mysql-test/include/mix1.inc b/mysql-test/include/mix1.inc index 194d9e41108..7ef8adf8e08 100644 --- a/mysql-test/include/mix1.inc +++ b/mysql-test/include/mix1.inc @@ -1359,7 +1359,17 @@ INSERT INTO t1 VALUES (1,'init'); DELIMITER |; CREATE PROCEDURE p1() BEGIN - UPDATE t1 SET b = CONCAT(b, '+con2') WHERE a = 1; + # retry the UPDATE in case it times out the lock before con1 has time + # to COMMIT. + DECLARE do_retry INT DEFAULT 0; + DECLARE CONTINUE HANDLER FOR SQLEXCEPTION SET do_retry = 1; + retry_loop:LOOP + UPDATE t1 SET b = CONCAT(b, '+con2') WHERE a = 1; + IF do_retry = 0 THEN + LEAVE retry_loop; + END IF; + SET do_retry = 0; + END LOOP; INSERT INTO t2 VALUES (); END| DELIMITER ;| diff --git a/mysql-test/r/partition_innodb_semi_consistent.result b/mysql-test/r/partition_innodb_semi_consistent.result index 471da4c1c2e..fbdd70528c5 100644 --- a/mysql-test/r/partition_innodb_semi_consistent.result +++ b/mysql-test/r/partition_innodb_semi_consistent.result @@ -75,7 +75,17 @@ TRUNCATE t1; INSERT INTO t1 VALUES (1,'init'); CREATE PROCEDURE p1() BEGIN +# retry the UPDATE in case it times out the lock before con1 has time +# to COMMIT. +DECLARE do_retry INT DEFAULT 0; +DECLARE CONTINUE HANDLER FOR SQLEXCEPTION SET do_retry = 1; +retry_loop:LOOP UPDATE t1 SET b = CONCAT(b, '+con2') WHERE a = 1; +IF do_retry = 0 THEN +LEAVE retry_loop; +END IF; +SET do_retry = 0; +END LOOP; INSERT INTO t2 VALUES (); END| BEGIN; diff --git a/mysql-test/suite/innodb/r/innodb_mysql.result b/mysql-test/suite/innodb/r/innodb_mysql.result index 5d90a96f61c..86a51b337ff 100644 --- a/mysql-test/suite/innodb/r/innodb_mysql.result +++ b/mysql-test/suite/innodb/r/innodb_mysql.result @@ -1594,7 +1594,17 @@ TRUNCATE t1; INSERT INTO t1 VALUES (1,'init'); CREATE PROCEDURE p1() BEGIN +# retry the UPDATE in case it times out the lock before con1 has time +# to COMMIT. +DECLARE do_retry INT DEFAULT 0; +DECLARE CONTINUE HANDLER FOR SQLEXCEPTION SET do_retry = 1; +retry_loop:LOOP UPDATE t1 SET b = CONCAT(b, '+con2') WHERE a = 1; +IF do_retry = 0 THEN +LEAVE retry_loop; +END IF; +SET do_retry = 0; +END LOOP; INSERT INTO t2 VALUES (); END| BEGIN; diff --git a/mysql-test/suite/innodb_plugin/r/innodb_mysql.result b/mysql-test/suite/innodb_plugin/r/innodb_mysql.result index 32ee3c21946..1aee85b5288 100644 --- a/mysql-test/suite/innodb_plugin/r/innodb_mysql.result +++ b/mysql-test/suite/innodb_plugin/r/innodb_mysql.result @@ -1594,7 +1594,17 @@ TRUNCATE t1; INSERT INTO t1 VALUES (1,'init'); CREATE PROCEDURE p1() BEGIN +# retry the UPDATE in case it times out the lock before con1 has time +# to COMMIT. +DECLARE do_retry INT DEFAULT 0; +DECLARE CONTINUE HANDLER FOR SQLEXCEPTION SET do_retry = 1; +retry_loop:LOOP UPDATE t1 SET b = CONCAT(b, '+con2') WHERE a = 1; +IF do_retry = 0 THEN +LEAVE retry_loop; +END IF; +SET do_retry = 0; +END LOOP; INSERT INTO t2 VALUES (); END| BEGIN; diff --git a/mysql-test/t/partition_innodb_semi_consistent.test b/mysql-test/t/partition_innodb_semi_consistent.test index 2711d79f194..9d3e2570256 100644 --- a/mysql-test/t/partition_innodb_semi_consistent.test +++ b/mysql-test/t/partition_innodb_semi_consistent.test @@ -117,7 +117,17 @@ INSERT INTO t1 VALUES (1,'init'); DELIMITER |; CREATE PROCEDURE p1() BEGIN - UPDATE t1 SET b = CONCAT(b, '+con2') WHERE a = 1; + # retry the UPDATE in case it times out the lock before con1 has time + # to COMMIT. + DECLARE do_retry INT DEFAULT 0; + DECLARE CONTINUE HANDLER FOR SQLEXCEPTION SET do_retry = 1; + retry_loop:LOOP + UPDATE t1 SET b = CONCAT(b, '+con2') WHERE a = 1; + IF do_retry = 0 THEN + LEAVE retry_loop; + END IF; + SET do_retry = 0; + END LOOP; INSERT INTO t2 VALUES (); END| DELIMITER ;| From 84edaac4f14b2fbaf57fef8fbaeb45966b1feda3 Mon Sep 17 00:00:00 2001 From: Michael Widenius Date: Fri, 10 Dec 2010 17:04:09 +0200 Subject: [PATCH 4/6] Better warning message if lock test fails Made archive.test a bit more safe mysql-test/r/archive.result: Added removal of files to make rerun of failed test work mysql-test/t/archive.test: Added removal of files to make rerun of failed test work mysys/thr_lock.c: Better warning message if lock test fails --- mysql-test/r/archive.result | 1 + mysql-test/t/archive.test | 5 +++++ mysys/thr_lock.c | 10 ++++------ 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/mysql-test/r/archive.result b/mysql-test/r/archive.result index 5a731e6476c..ae79e237ade 100644 --- a/mysql-test/r/archive.result +++ b/mysql-test/r/archive.result @@ -12717,6 +12717,7 @@ COUNT(t1.a) 729 DROP TABLE t1; SET @@join_buffer_size= @save_join_buffer_size; +flush tables; SHOW CREATE TABLE t1; ERROR HY000: Table upgrade required. Please do "REPAIR TABLE `t1`" or dump/reload to fix it! SELECT * FROM t1; diff --git a/mysql-test/t/archive.test b/mysql-test/t/archive.test index 7478c7dcdce..c966b51d861 100644 --- a/mysql-test/t/archive.test +++ b/mysql-test/t/archive.test @@ -1630,6 +1630,11 @@ SET @@join_buffer_size= @save_join_buffer_size; # BUG#47012 archive tables are not upgradeable, and server crashes on any access # let $MYSQLD_DATADIR= `SELECT @@datadir`; + +# Remove files to handle possible restart of test +flush tables; +remove_files_wildcard $MYSQLD_DATADIR/test t1.*; + copy_file std_data/bug47012.frm $MYSQLD_DATADIR/test/t1.frm; copy_file std_data/bug47012.ARZ $MYSQLD_DATADIR/test/t1.ARZ; copy_file std_data/bug47012.ARM $MYSQLD_DATADIR/test/t1.ARM; diff --git a/mysys/thr_lock.c b/mysys/thr_lock.c index 341b2f0058e..60214c9af08 100644 --- a/mysys/thr_lock.c +++ b/mysys/thr_lock.c @@ -158,15 +158,13 @@ static int check_lock(struct st_lock_list *list, const char* lock_type, { THR_LOCK_DATA *data,**prev; uint count=0; - THR_LOCK_OWNER *UNINIT_VAR(first_owner); prev= &list->data; if (list->data) { - enum thr_lock_type last_lock_type=list->data->type; + enum thr_lock_type last_lock_type= list->data->type; + THR_LOCK_OWNER first_owner= list->data->owner; - if (same_owner && list->data) - first_owner= list->data->owner; for (data=list->data; data && count++ < MAX_LOCKS ; data=data->next) { if (data->type != last_lock_type) @@ -184,8 +182,8 @@ static int check_lock(struct st_lock_list *list, const char* lock_type, last_lock_type != TL_WRITE_CONCURRENT_INSERT) { fprintf(stderr, - "Warning: Found locks from different threads in %s: %s\n", - lock_type,where); + "Warning: Found locks from different threads in %s at '%s'. org_lock_type: %d last_lock_type: %d new_lock_type: %d\n", + lock_type, where, list->data->type, last_lock_type, data->type); return 1; } if (no_cond && data->cond) From bd2034dbee16bc90217d44df5059d837d60a6110 Mon Sep 17 00:00:00 2001 From: Michael Widenius Date: Fri, 10 Dec 2010 17:15:18 +0200 Subject: [PATCH 5/6] Fix for Lbug:#686010 maria.optimize corrupts stack around alloca() call storage/maria/ma_check.c: Allocate also memory for nodflag and transid's storage/maria/ma_write.c: Allow nodflag and transid as part of key. (This has nothing to do with the bug report, but it's a safer check). storage/maria/maria_def.h: Define MARIA_MAX_POINTER_LENGTH (length of pointer to node) Added node pointer length to MARIA_INDEX_OVERHEAD_SIZE, as this is part of the key. (Safety fix) --- storage/maria/ma_check.c | 3 ++- storage/maria/ma_write.c | 6 ++---- storage/maria/maria_def.h | 4 +++- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/storage/maria/ma_check.c b/storage/maria/ma_check.c index fcf01385e97..1cb14567804 100644 --- a/storage/maria/ma_check.c +++ b/storage/maria/ma_check.c @@ -3133,7 +3133,8 @@ static int sort_one_index(HA_CHECK *param, MARIA_HA *info, key.keyinfo= keyinfo; if (!(buff= (uchar*) my_alloca((uint) keyinfo->block_length + - keyinfo->maxlength))) + keyinfo->maxlength + + MARIA_INDEX_OVERHEAD_SIZE))) { _ma_check_print_error(param,"Not enough memory for key block"); DBUG_RETURN(-1); diff --git a/storage/maria/ma_write.c b/storage/maria/ma_write.c index 02eeec754ee..49f86fe291b 100644 --- a/storage/maria/ma_write.c +++ b/storage/maria/ma_write.c @@ -22,8 +22,6 @@ #include "ma_key_recover.h" #include "ma_blockrec.h" -#define MAX_POINTER_LENGTH 8 - /* Functions declared in this file */ static int w_search(MARIA_HA *info, uint32 comp_flag, @@ -802,7 +800,7 @@ int _ma_insert(register MARIA_HA *info, MARIA_KEY *key, #endif if (t_length > 0) { - if (t_length >= keyinfo->maxlength*2+MAX_POINTER_LENGTH) + if (t_length >= keyinfo->maxlength*2+MARIA_INDEX_OVERHEAD_SIZE) { my_errno=HA_ERR_CRASHED; DBUG_RETURN(-1); @@ -811,7 +809,7 @@ int _ma_insert(register MARIA_HA *info, MARIA_KEY *key, } else { - if (-t_length >= keyinfo->maxlength*2+MAX_POINTER_LENGTH) + if (-t_length >= keyinfo->maxlength*2+MARIA_INDEX_OVERHEAD_SIZE) { my_errno=HA_ERR_CRASHED; DBUG_RETURN(-1); diff --git a/storage/maria/maria_def.h b/storage/maria/maria_def.h index b998ac4f3a0..85696d04858 100644 --- a/storage/maria/maria_def.h +++ b/storage/maria/maria_def.h @@ -152,11 +152,13 @@ typedef struct st_maria_state_info #define MARIA_COLUMNDEF_SIZE (2*7+1+1+4) #define MARIA_BASE_INFO_SIZE (MY_UUID_SIZE + 5*8 + 6*4 + 11*2 + 6 + 5*2 + 1 + 16) #define MARIA_INDEX_BLOCK_MARGIN 16 /* Safety margin for .MYI tables */ +#define MARIA_MAX_POINTER_LENGTH 7 /* Node pointer */ /* Internal management bytes needed to store 2 transid/key on an index page */ #define MARIA_MAX_PACK_TRANSID_SIZE (TRANSID_SIZE+1) #define MARIA_TRANSID_PACK_OFFSET (256- TRANSID_SIZE - 1) #define MARIA_MIN_TRANSID_PACK_OFFSET (MARIA_TRANSID_PACK_OFFSET-TRANSID_SIZE) -#define MARIA_INDEX_OVERHEAD_SIZE (MARIA_MAX_PACK_TRANSID_SIZE * 2) +#define MARIA_INDEX_OVERHEAD_SIZE (MARIA_MAX_PACK_TRANSID_SIZE * 2 + \ + MARIA_MAX_POINTER_LENGTH) #define MARIA_DELETE_KEY_NR 255 /* keynr for deleted blocks */ /* From 0b20943e9d7e22332ccc14b30b49a78a780cc23d Mon Sep 17 00:00:00 2001 From: Michael Widenius Date: Mon, 13 Dec 2010 15:27:13 +0200 Subject: [PATCH 6/6] Fixed typo that caused compile failure in thr_lock.c mysys/thr_lock.c: Fixed typo that caused compile failure --- mysys/thr_lock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mysys/thr_lock.c b/mysys/thr_lock.c index 60214c9af08..68a3caf6da7 100644 --- a/mysys/thr_lock.c +++ b/mysys/thr_lock.c @@ -163,7 +163,7 @@ static int check_lock(struct st_lock_list *list, const char* lock_type, if (list->data) { enum thr_lock_type last_lock_type= list->data->type; - THR_LOCK_OWNER first_owner= list->data->owner; + THR_LOCK_OWNER *first_owner= list->data->owner; for (data=list->data; data && count++ < MAX_LOCKS ; data=data->next) {