List:Commits« Previous MessageNext Message »
From:kevin.lewis Date:September 27 2012 6:57pm
Subject:bzr push into mysql-5.6 branch (kevin.lewis:4350 to 4351)
View as plain text  
 4351 kevin.lewis@stripped	2012-09-27
      14606694 - ASSERT,RECORD LOCK WAIT IN DICT OPERATION,
                 DROP TEMP TABLE WHILE IN CREATE INDEX
      
      The assert that is hit in this bug says that if the current transaction
      holds dict_operation_lock, it should not have to wait on any blocks in
      a SYS_* dictionary table.  But this assert was hit;
      
      	default:
      		/* There should never be a lock wait when the
      		dictionary latch is reserved in X mode.  Dictionary
      		transactions should only acquire locks on dictionary
      		tables, not other tables. All access to dictionary
      		tables should be covered by dictionary
      		transactions. */
      		ut_error;
      
      There are two functions that need to assert that dict_operations_lock is
      x-owned; dict_insert_tablespace_and_filepath() and dict_update_filepath().
      These two functions are called from fil_open_single_table_tablespace()
      which is called from 3 places;
      
      1) dict_check_tablespaces_and_store_max_id()
            Run during single threaded startup. But this code does not get
            dict_operations_lock.  That is added in this patch, but since
            this is single threaded, this cannot be the source of the assert
            in this bug.
      2) dict_load_table()
            Only had dict_sys->mutex, not dict_operations_lock.  But this call
            to fil_open_single_table_tablespace() sets 'fix_dict = false'
            so it does not call the dictionary functions.
      3) row_import_for_mysql().
            This calls row_mysql_lock_data_dictionary(trx); so it has both
            locks already.
      
      The actual way in which this assert was hit was probably that a
      transaction created in calls 1 or 3 above needed to be rolled back
      for some reason.  If that happens, the UNDO code will not
      get dict_operations_lock unless those transactions are identified
      as dictionary operations.  This patch does that by starting the
      dictionary transaction in these two functions as TRX_DICT_OP_INDEX;
      
      This patch also identifies the sub-transactions in these two functions
      as owning an X-lock on dict_operation_lock.
      
      Approved by marko in RB:1311

    modified:
      storage/innobase/dict/dict0dict.cc
      storage/innobase/dict/dict0load.cc
      storage/innobase/fil/fil0fil.cc
      storage/innobase/row/row0import.cc
 4350 Dmitry Lenev	2012-09-27
      Fix for bug #14569140 "MYSQL 5.6.6-M9 HAS MORE MUTEX CONTENTION
      THAN MYSQL 5.1".
      
      MySQL 5.6 has shown somewhat worse performance than MySQL 5.1
      for SysBench read-only/InnoDB workload against several tables
      for low number of connections.
      
      This issue was caused by the contention in MDL subsystem
      introduced in 5.5. The particular source of contention was
      MDL_map::m_mutex which protects hash containing all MDL_lock
      objects in the system.
      
      This patch addresses the problem by partitioning this hash
      and corresponding mutex by key for MDL_lock objects, thus
      allowing connections accessing different tables/objects to
      work with different partitions and therefore avoid contention.
      
      New start-up parameter --metadata-locks-hash-instances is
      introduced which allows to specify number of such partitions
      (with 8 partitions as default value).

    added:
      mysql-test/suite/sys_vars/r/metadata_locks_hash_instances_basic.result
      mysql-test/suite/sys_vars/t/metadata_locks_hash_instances_basic.test
    modified:
      mysql-test/r/mysqld--help-notwin.result
      mysql-test/r/mysqld--help-win.result
      sql/mdl.cc
      sql/mdl.h
      sql/sys_vars.cc
      unittest/gunit/mdl-t.cc
      unittest/gunit/mdl_mytap-t.cc
=== modified file 'storage/innobase/dict/dict0dict.cc'
--- a/storage/innobase/dict/dict0dict.cc	revid:dmitry.lenev@stripped
+++ b/storage/innobase/dict/dict0dict.cc	revid:kevin.lewis@stripped
@@ -394,7 +394,8 @@ dict_table_stats_unlock(
 }
 
 /**********************************************************************//**
-Try to drop any indexes after an aborted index creation. */
+Try to drop any indexes after an aborted index creation.
+This can also be after a server kill during DROP INDEX. */
 static
 void
 dict_table_try_drop_aborted(
@@ -407,6 +408,7 @@ dict_table_try_drop_aborted(
 	trx_t*		trx;
 
 	trx = trx_allocate_for_background();
+	trx->op_info = "try to drop any indexes after an aborted index creation";
 	row_mysql_lock_data_dictionary(trx);
 	trx_set_dict_operation(trx, TRX_DICT_OP_INDEX);
 
@@ -1779,6 +1781,7 @@ dict_table_remove_from_cache_low(
 		ut_d(table->n_ref_count--);
 		ut_ad(table->n_ref_count == 0);
 		trx_commit_for_mysql(trx);
+		trx->dict_operation_lock_mode = 0;
 		trx_free_for_background(trx);
 	}
 

=== modified file 'storage/innobase/dict/dict0load.cc'
--- a/storage/innobase/dict/dict0load.cc	revid:dmitry.lenev@stripped
+++ b/storage/innobase/dict/dict0load.cc	revid:kevin.lewis@stripped
@@ -861,10 +861,15 @@ dict_update_filepath(
 	dberr_t		err = DB_SUCCESS;
 	trx_t*		trx;
 
+#ifdef UNIV_SYNC_DEBUG
+	ut_ad(rw_lock_own(&dict_operation_lock, RW_LOCK_EX));
+#endif /* UNIV_SYNC_DEBUG */
 	ut_ad(mutex_own(&(dict_sys->mutex)));
 
 	trx = trx_allocate_for_background();
-	trx_start_if_not_started(trx);
+	trx->op_info = "update filepath";
+	trx->dict_operation_lock_mode = RW_X_LATCH;
+	trx_start_for_ddl(trx, TRX_DICT_OP_INDEX);
 
 	pars_info_t*	info = pars_info_create();
 
@@ -880,6 +885,7 @@ dict_update_filepath(
 			   "END;\n", FALSE, trx);
 
 	trx_commit_for_mysql(trx);
+	trx->dict_operation_lock_mode = 0;
 	trx_free_for_background(trx);
 
 	if (err == DB_SUCCESS) {
@@ -914,11 +920,16 @@ dict_insert_tablespace_and_filepath(
 	dberr_t		err = DB_SUCCESS;
 	trx_t*		trx;
 
+#ifdef UNIV_SYNC_DEBUG
+	ut_ad(rw_lock_own(&dict_operation_lock, RW_LOCK_EX));
+#endif /* UNIV_SYNC_DEBUG */
 	ut_ad(mutex_own(&(dict_sys->mutex)));
 	ut_ad(filepath);
 
 	trx = trx_allocate_for_background();
-	trx_start_if_not_started(trx);
+	trx->op_info = "insert tablespace and filepath";
+	trx->dict_operation_lock_mode = RW_X_LATCH;
+	trx_start_for_ddl(trx, TRX_DICT_OP_INDEX);
 
 	/* A record for this space ID was not found in
 	SYS_DATAFILES. Assume the record is also missing in
@@ -927,6 +938,7 @@ dict_insert_tablespace_and_filepath(
 		space, name, fsp_flags, filepath, trx, false);
 
 	trx_commit_for_mysql(trx);
+	trx->dict_operation_lock_mode = 0;
 	trx_free_for_background(trx);
 
 	return(err);
@@ -957,6 +969,7 @@ dict_check_tablespaces_and_store_max_id(
 	ulint		max_space_id;
 	mtr_t		mtr;
 
+	rw_lock_x_lock(&dict_operation_lock);
 	mutex_enter(&(dict_sys->mutex));
 
 	mtr_start(&mtr);
@@ -991,6 +1004,7 @@ loop:
 		fil_set_max_space_id_if_bigger(max_space_id);
 
 		mutex_exit(&(dict_sys->mutex));
+		rw_lock_x_unlock(&dict_operation_lock);
 
 		return;
 	}
@@ -1093,7 +1107,12 @@ loop:
 					space_id, name);
 			}
 
-			/* filepath can be NULL in this call. */
+			/* We set the 2nd param (fix_dict = true)
+			here because we already have an x-lock on
+			dict_operation_lock and dict_sys->mutex. Besides,
+			this is at startup and we are now single threaded.
+			If the filepath is not known, it will need to
+			be discovered. */
 			dberr_t	err = fil_open_single_table_tablespace(
 				false, srv_read_only_mode ? false : true,
 				space_id, dict_tf_to_fsp_flags(flags),
@@ -2312,7 +2331,9 @@ err_exit:
 				}
 			}
 
-			/* Try to open the tablespace */
+			/* Try to open the tablespace.  We set the
+			2nd param (fix_dict = false) here because we
+			do not have an x-lock on dict_operation_lock */
 			err = fil_open_single_table_tablespace(
 				true, false, table->space,
 				dict_tf_to_fsp_flags(table->flags),

=== modified file 'storage/innobase/fil/fil0fil.cc'
--- a/storage/innobase/fil/fil0fil.cc	revid:dmitry.lenev@stripped
+++ b/storage/innobase/fil/fil0fil.cc	revid:kevin.lewis@stripped
@@ -3507,6 +3507,11 @@ fil_open_single_table_tablespace(
 	ulint		tablespaces_found = 0;
 	ulint		valid_tablespaces_found = 0;
 
+#ifdef UNIV_SYNC_DEBUG
+	ut_ad(!fix_dict || rw_lock_own(&dict_operation_lock, RW_LOCK_EX));
+#endif /* UNIV_SYNC_DEBUG */
+	ut_ad(!fix_dict || mutex_own(&(dict_sys->mutex)));
+
 	if (!fsp_flags_is_valid(flags)) {
 		return(DB_CORRUPTION);
 	}

=== modified file 'storage/innobase/row/row0import.cc'
--- a/storage/innobase/row/row0import.cc	revid:dmitry.lenev@stripped
+++ b/storage/innobase/row/row0import.cc	revid:kevin.lewis@stripped
@@ -3051,7 +3051,9 @@ row_import_for_mysql(
 	}
 	ut_a(filepath);
 
-	/* Open the tablespace so that we can access via the buffer pool. */
+	/* Open the tablespace so that we can access via the buffer pool.
+	We set the 2nd param (fix_dict = true) here because we already
+	have an x-lock on dict_operation_lock and dict_sys->mutex. */
 
 	err = fil_open_single_table_tablespace(
 		true, true, table->space,

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-5.6 branch (kevin.lewis:4350 to 4351) kevin.lewis28 Sep