List:Commits« Previous MessageNext Message »
From:marko.makela Date:April 7 2011 7:24pm
Subject:bzr commit into mysql-5.1-innodb branch (marko.makela:3725) Bug#11760042
View as plain text  
#At file:///home/marko/innobase/dev/mysql2a/5.1-innodb/ based on revid:marko.makela@strippeduaznzdrzcp3a

 3725 Marko Mäkelä	2011-04-07
      Bug #11760042 - 52409: Assertion failure: long semaphore wait
      
      In ha_innobase::create(), we check some things while holding an
      exclusive lock on the data dictionary. Defer the locking and the
      creation of transactions until after the checks have passed. The
      THDVAR could hang due to a mutex wait (see Bug #11750569 - 41163:
      deadlock in mysqld: LOCK_global_system_variables and LOCK_open), and
      we want to avoid waiting while holding InnoDB mutexes.
      
      innobase_index_name_is_reserved(): Replace the parameter trx_t with
      THD, so that the test can be performed before starting an InnoDB
      transaction. We only needed trx->mysql_thd.
      
      ha_innobase::create(): Create transaction and lock the data dictionary
      only after passing the basic tests.
      
      create_table_def(): Move the IS_MAGIC_TABLE_AND_USER_DENIED_ACCESS
      check to ha_innobase::create(). Assign to srv_lower_case_table_names
      while holding dict_sys->mutex.
      
      ha_innobase::delete_table(), ha_innobase::rename_table(),
      innobase_rename_table(): Assign srv_lower_case_table_names as late as
      possible. Here, the variable is not necessarily protected by
      dict_sys->mutex.
      
      ha_innobase::add_index(): Invoke innobase_index_name_is_reserved() and
      innobase_check_index_keys() before allocating anything.
      
      rb:618 approved by Jimmy Yang

    modified:
      storage/innobase/handler/ha_innodb.cc
      storage/innodb_plugin/ChangeLog
      storage/innodb_plugin/handler/ha_innodb.cc
      storage/innodb_plugin/handler/ha_innodb.h
      storage/innodb_plugin/handler/handler0alter.cc
=== modified file 'storage/innobase/handler/ha_innodb.cc'
--- a/storage/innobase/handler/ha_innodb.cc	revid:marko.makela@strippediq5puaznzdrzcp3a
+++ b/storage/innobase/handler/ha_innodb.cc	revid:marko.makela@strippedn9pa6aeq
@@ -189,7 +189,7 @@ innobase_index_name_is_reserved(
 /*============================*/
 					/* out: true if index name matches a
 					reserved name */
-	const trx_t*	trx,		/* in: InnoDB transaction handle */
+	THD*		thd,		/* in/out: MySQL connection */
 	const TABLE*	form,		/* in: information on table
 					columns and indexes */
 	const char*	norm_name);	/* in: table name */
@@ -5285,10 +5285,6 @@ create_table_def(
 	DBUG_PRINT("enter", ("table_name: %s", table_name));
 
 	ut_a(trx->mysql_thd != NULL);
-	if (IS_MAGIC_TABLE_AND_USER_DENIED_ACCESS(table_name,
-						  (THD*) trx->mysql_thd)) {
-		DBUG_RETURN(HA_ERR_GENERIC);
-	}
 
 	n_cols = form->s->fields;
 
@@ -5397,6 +5393,8 @@ err_col:
 			col_len);
 	}
 
+	srv_lower_case_table_names = lower_case_table_names;
+
 	error = row_create_table_for_mysql(table, trx);
 
 	innodb_check_for_record_too_big_error(flags & DICT_TF_COMPACT, error);
@@ -5642,6 +5640,35 @@ ha_innobase::create(
 		DBUG_RETURN(HA_ERR_TO_BIG_ROW);
 	}
 
+	strcpy(name2, name);
+
+	normalize_table_name(norm_name, name2);
+
+	/* Create the table definition in InnoDB */
+
+	flags = form->s->row_type != ROW_TYPE_REDUNDANT ? DICT_TF_COMPACT : 0;
+
+	/* Look for a primary key */
+
+	primary_key_no= (form->s->primary_key != MAX_KEY ?
+			 (int) form->s->primary_key :
+			 -1);
+
+	/* Our function row_get_mysql_key_number_for_index assumes
+	the primary key is always number 0, if it exists */
+
+	DBUG_ASSERT(primary_key_no == -1 || primary_key_no == 0);
+
+	/* Check for name conflicts (with reserved name) for
+	any user indices to be created. */
+	if (innobase_index_name_is_reserved(thd, form, norm_name)) {
+		DBUG_RETURN(-1);
+	}
+
+	if (IS_MAGIC_TABLE_AND_USER_DENIED_ACCESS(norm_name, thd)) {
+		DBUG_RETURN(HA_ERR_GENERIC);
+	}
+
 	/* Get the transaction associated with the current thd, or create one
 	if not yet created */
 
@@ -5665,48 +5692,12 @@ ha_innobase::create(
 		trx->check_unique_secondary = FALSE;
 	}
 
-	if (lower_case_table_names) {
-		srv_lower_case_table_names = TRUE;
-	} else {
-		srv_lower_case_table_names = FALSE;
-	}
-
-	strcpy(name2, name);
-
-	normalize_table_name(norm_name, name2);
-
 	/* Latch the InnoDB data dictionary exclusively so that no deadlocks
 	or lock waits can happen in it during a table create operation.
 	Drop table etc. do this latching in row0mysql.c. */
 
 	row_mysql_lock_data_dictionary(trx);
 
-	/* Create the table definition in InnoDB */
-
-	flags = 0;
-
-	if (form->s->row_type != ROW_TYPE_REDUNDANT) {
-		flags |= DICT_TF_COMPACT;
-	}
-
-	/* Look for a primary key */
-
-	primary_key_no= (form->s->primary_key != MAX_KEY ?
-			 (int) form->s->primary_key :
-			 -1);
-
-	/* Our function row_get_mysql_key_number_for_index assumes
-	the primary key is always number 0, if it exists */
-
-	DBUG_ASSERT(primary_key_no == -1 || primary_key_no == 0);
-
-	/* Check for name conflicts (with reserved name) for
-	any user indices to be created. */
-	if (innobase_index_name_is_reserved(trx, form, norm_name)) {
-		error = -1;
-		goto cleanup;
-	}
-
 	error = create_table_def(trx, form, norm_name,
 		create_info->options & HA_LEX_CREATE_TMP_TABLE ? name2 : NULL,
 		flags);
@@ -5936,12 +5927,6 @@ ha_innobase::delete_table(
 
 	trx_search_latch_release_if_reserved(parent_trx);
 
-	if (lower_case_table_names) {
-		srv_lower_case_table_names = TRUE;
-	} else {
-		srv_lower_case_table_names = FALSE;
-	}
-
 	trx = trx_allocate_for_mysql();
 
 	trx->mysql_thd = thd;
@@ -5961,6 +5946,8 @@ ha_innobase::delete_table(
 
 	/* Drop the table in InnoDB */
 
+	srv_lower_case_table_names = lower_case_table_names;
+
 	error = row_drop_table_for_mysql(norm_name, trx,
 					 thd_sql_command(thd)
 					 == SQLCOM_DROP_DB);
@@ -6089,12 +6076,6 @@ ha_innobase::rename_table(
 
 	trx_search_latch_release_if_reserved(parent_trx);
 
-	if (lower_case_table_names) {
-		srv_lower_case_table_names = TRUE;
-	} else {
-		srv_lower_case_table_names = FALSE;
-	}
-
 	trx = trx_allocate_for_mysql();
 	trx->mysql_thd = thd;
 	INNOBASE_COPY_STMT(thd, trx);
@@ -6114,6 +6095,8 @@ ha_innobase::rename_table(
 
 	/* Rename the table in InnoDB */
 
+	srv_lower_case_table_names = lower_case_table_names;
+
 	error = row_rename_table_for_mysql(norm_from, norm_to, trx);
 
 	/* Flush the log to reduce probability that the .frm files and
@@ -8826,7 +8809,7 @@ innobase_index_name_is_reserved(
 /*============================*/
 					/* out: true if an index name
 					matches the reserved name */
-	const trx_t*	trx,		/* in: InnoDB transaction handle */
+	THD*		thd,		/* in/out: MySQL connection */
 	const TABLE*	form,		/* in: information on table
 					columns and indexes */
 	const char*	norm_name)	/* in: table name */
@@ -8840,7 +8823,7 @@ innobase_index_name_is_reserved(
 		if (innobase_strcasecmp(key->name,
 					innobase_index_reserve_name) == 0) {
 			/* Push warning to mysql */
-			push_warning_printf((THD*) trx->mysql_thd,
+			push_warning_printf(thd,
 					    MYSQL_ERROR::WARN_LEVEL_WARN,
 					    ER_CANT_CREATE_TABLE,
 					    "Cannot Create Index with name "

=== modified file 'storage/innodb_plugin/ChangeLog'
--- a/storage/innodb_plugin/ChangeLog	revid:marko.makela@stripped407181254-iq5puaznzdrzcp3a
+++ b/storage/innodb_plugin/ChangeLog	revid:marko.makela@stripped686on9pa6aeq
@@ -1,5 +1,10 @@
 2011-04-07	The InnoDB Team
 
+	* handler/ha_innodb.cc, handler/ha_innodb.h, handler/handler0alter.cc:
+	Fix Bug #52409 Assertion failure: long semaphore wait
+
+2011-04-07	The InnoDB Team
+
 	* handler/ha_innodb.cc, include/trx0trx.h, include/trx0undo.h,
 	log/log0log.c, trx/trx0sys.c, trx/trx0trx.c, trx/trx0undo.c:
 	Fix Bug #59641 Prepared XA transaction in system after hard crash

=== modified file 'storage/innodb_plugin/handler/ha_innodb.cc'
--- a/storage/innodb_plugin/handler/ha_innodb.cc	revid:marko.makela@stripped1254-iq5puaznzdrzcp3a
+++ b/storage/innodb_plugin/handler/ha_innodb.cc	revid:marko.makela@stripped5-tjcj686on9pa6aeq
@@ -6023,10 +6023,6 @@ create_table_def(
 	DBUG_PRINT("enter", ("table_name: %s", table_name));
 
 	ut_a(trx->mysql_thd != NULL);
-	if (IS_MAGIC_TABLE_AND_USER_DENIED_ACCESS(table_name,
-						  (THD*) trx->mysql_thd)) {
-		DBUG_RETURN(HA_ERR_GENERIC);
-	}
 
 	/* MySQL does the name length check. But we do additional check
 	on the name length here */
@@ -6146,6 +6142,8 @@ err_col:
 			col_len);
 	}
 
+	srv_lower_case_table_names = lower_case_table_names;
+
 	error = row_create_table_for_mysql(table, trx);
 
 	if (error == DB_DUPLICATE_KEY) {
@@ -6562,42 +6560,17 @@ ha_innobase::create(
 		DBUG_RETURN(HA_ERR_TO_BIG_ROW);
 	}
 
-	/* Get the transaction associated with the current thd, or create one
-	if not yet created */
-
-	parent_trx = check_trx_exists(thd);
-
-	/* In case MySQL calls this in the middle of a SELECT query, release
-	possible adaptive hash latch to avoid deadlocks of threads */
-
-	trx_search_latch_release_if_reserved(parent_trx);
-
-	trx = innobase_trx_allocate(thd);
-
-	if (lower_case_table_names) {
-		srv_lower_case_table_names = TRUE;
-	} else {
-		srv_lower_case_table_names = FALSE;
-	}
-
 	strcpy(name2, name);
 
 	normalize_table_name(norm_name, name2);
 
-	/* Latch the InnoDB data dictionary exclusively so that no deadlocks
-	or lock waits can happen in it during a table create operation.
-	Drop table etc. do this latching in row0mysql.c. */
-
-	row_mysql_lock_data_dictionary(trx);
-
 	/* Create the table definition in InnoDB */
 
 	flags = 0;
 
 	/* Validate create options if innodb_strict_mode is set. */
 	if (!create_options_are_valid(thd, form, create_info)) {
-		error = ER_ILLEGAL_HA_CREATE_OPTION;
-		goto cleanup;
+		DBUG_RETURN(ER_ILLEGAL_HA_CREATE_OPTION);
 	}
 
 	if (create_info->key_block_size) {
@@ -6739,16 +6712,37 @@ ha_innobase::create(
 
 	/* Check for name conflicts (with reserved name) for
 	any user indices to be created. */
-	if (innobase_index_name_is_reserved(trx, form->key_info,
+	if (innobase_index_name_is_reserved(thd, form->key_info,
 					    form->s->keys)) {
-		error = -1;
-		goto cleanup;
+		DBUG_RETURN(-1);
+	}
+
+	if (IS_MAGIC_TABLE_AND_USER_DENIED_ACCESS(norm_name, thd)) {
+		DBUG_RETURN(HA_ERR_GENERIC);
 	}
 
 	if (create_info->options & HA_LEX_CREATE_TMP_TABLE) {
 		flags |= DICT_TF2_TEMPORARY << DICT_TF2_SHIFT;
 	}
 
+	/* Get the transaction associated with the current thd, or create one
+	if not yet created */
+
+	parent_trx = check_trx_exists(thd);
+
+	/* In case MySQL calls this in the middle of a SELECT query, release
+	possible adaptive hash latch to avoid deadlocks of threads */
+
+	trx_search_latch_release_if_reserved(parent_trx);
+
+	trx = innobase_trx_allocate(thd);
+
+	/* Latch the InnoDB data dictionary exclusively so that no deadlocks
+	or lock waits can happen in it during a table create operation.
+	Drop table etc. do this latching in row0mysql.c. */
+
+	row_mysql_lock_data_dictionary(trx);
+
 	error = create_table_def(trx, form, norm_name,
 		create_info->options & HA_LEX_CREATE_TMP_TABLE ? name2 : NULL,
 		flags);
@@ -6992,18 +6986,14 @@ ha_innobase::delete_table(
 
 	trx = innobase_trx_allocate(thd);
 
-	if (lower_case_table_names) {
-		srv_lower_case_table_names = TRUE;
-	} else {
-		srv_lower_case_table_names = FALSE;
-	}
-
 	name_len = strlen(name);
 
 	ut_a(name_len < 1000);
 
 	/* Drop the table in InnoDB */
 
+	srv_lower_case_table_names = lower_case_table_names;
+
 	error = row_drop_table_for_mysql(norm_name, trx,
 					 thd_sql_command(thd)
 					 == SQLCOM_DROP_DB);
@@ -7119,12 +7109,6 @@ innobase_rename_table(
 	char*	norm_to;
 	char*	norm_from;
 
-	if (lower_case_table_names) {
-		srv_lower_case_table_names = TRUE;
-	} else {
-		srv_lower_case_table_names = FALSE;
-	}
-
 	// Magic number 64 arbitrary
 	norm_to = (char*) my_malloc(strlen(to) + 64, MYF(0));
 	norm_from = (char*) my_malloc(strlen(from) + 64, MYF(0));
@@ -7139,6 +7123,8 @@ innobase_rename_table(
 		row_mysql_lock_data_dictionary(trx);
 	}
 
+	srv_lower_case_table_names = lower_case_table_names;
+
 	error = row_rename_table_for_mysql(
 		norm_from, norm_to, trx, lock_and_commit);
 
@@ -10700,19 +10686,19 @@ static int show_innodb_vars(THD *thd, SH
   return 0;
 }
 
-/***********************************************************************
+/*********************************************************************//**
 This function checks each index name for a table against reserved
-system default primary index name 'GEN_CLUST_INDEX'. If a name matches,
-this function pushes an warning message to the client, and returns true. */
+system default primary index name 'GEN_CLUST_INDEX'. If a name
+matches, this function pushes an warning message to the client,
+and returns true.
+@return true if the index name matches the reserved name */
 extern "C" UNIV_INTERN
 bool
 innobase_index_name_is_reserved(
 /*============================*/
-					/* out: true if an index name
-					matches the reserved name */
-	const trx_t*	trx,		/* in: InnoDB transaction handle */
-	const KEY*	key_info,	/* in: Indexes to be created */
-	ulint		num_of_keys)	/* in: Number of indexes to
+	THD*		thd,		/*!< in/out: MySQL connection */
+	const KEY*	key_info,	/*!< in: Indexes to be created */
+	ulint		num_of_keys)	/*!< in: Number of indexes to
 					be created. */
 {
 	const KEY*	key;
@@ -10724,7 +10710,7 @@ innobase_index_name_is_reserved(
 		if (innobase_strcasecmp(key->name,
 					innobase_index_reserve_name) == 0) {
 			/* Push warning to mysql */
-			push_warning_printf((THD*) trx->mysql_thd,
+			push_warning_printf(thd,
 					    MYSQL_ERROR::WARN_LEVEL_WARN,
 					    ER_WRONG_NAME_FOR_INDEX,
 					    "Cannot Create Index with name "

=== modified file 'storage/innodb_plugin/handler/ha_innodb.h'
--- a/storage/innodb_plugin/handler/ha_innodb.h	revid:marko.makela@oracle.com-20110407181254-iq5puaznzdrzcp3a
+++ b/storage/innodb_plugin/handler/ha_innodb.h	revid:marko.makela@stripped-20110407192445-tjcj686on9pa6aeq
@@ -317,15 +317,14 @@ innobase_trx_allocate(
 This function checks each index name for a table against reserved
 system default primary index name 'GEN_CLUST_INDEX'. If a name
 matches, this function pushes an warning message to the client,
-and returns true. */
+and returns true.
+@return true if the index name matches the reserved name */
 extern "C"
 bool
 innobase_index_name_is_reserved(
 /*============================*/
-					/* out: true if the index name
-					matches the reserved name */
-	const trx_t*	trx,		/* in: InnoDB transaction handle */
-	const KEY*	key_info,	/* in: Indexes to be created */
-	ulint		num_of_keys);	/* in: Number of indexes to
+	THD*		thd,		/*!< in/out: MySQL connection */
+	const KEY*	key_info,	/*!< in: Indexes to be created */
+	ulint		num_of_keys);	/*!< in: Number of indexes to
 					be created. */
 

=== modified file 'storage/innodb_plugin/handler/handler0alter.cc'
--- a/storage/innodb_plugin/handler/handler0alter.cc	revid:marko.makela@strippedm-20110407181254-iq5puaznzdrzcp3a
+++ b/storage/innodb_plugin/handler/handler0alter.cc	revid:marko.makela@strippedom-20110407192445-tjcj686on9pa6aeq
@@ -649,44 +649,37 @@ ha_innobase::add_index(
 
 	update_thd();
 
-	heap = mem_heap_create(1024);
-
 	/* In case MySQL calls this in the middle of a SELECT query, release
 	possible adaptive hash latch to avoid deadlocks of threads. */
 	trx_search_latch_release_if_reserved(prebuilt->trx);
-	trx_start_if_not_started(prebuilt->trx);
-
-	/* Create a background transaction for the operations on
-	the data dictionary tables. */
-	trx = innobase_trx_allocate(user_thd);
-	trx_start_if_not_started(trx);
 
 	innodb_table = indexed_table
 		= dict_table_get(prebuilt->table->name, FALSE);
 
 	if (UNIV_UNLIKELY(!innodb_table)) {
-		error = HA_ERR_NO_SUCH_TABLE;
-		goto err_exit;
+		DBUG_RETURN(HA_ERR_NO_SUCH_TABLE);
 	}
 
 	/* Check if the index name is reserved. */
-	if (innobase_index_name_is_reserved(trx, key_info, num_of_keys)) {
-		error = -1;
-	} else {
-		/* Check that index keys are sensible */
-		error = innobase_check_index_keys(key_info, num_of_keys,
-						  innodb_table);
+	if (innobase_index_name_is_reserved(user_thd, key_info, num_of_keys)) {
+		DBUG_RETURN(-1);
 	}
 
+	/* Check that index keys are sensible */
+	error = innobase_check_index_keys(key_info, num_of_keys, innodb_table);
+
 	if (UNIV_UNLIKELY(error)) {
-err_exit:
-		mem_heap_free(heap);
-		trx_general_rollback_for_mysql(trx, NULL);
-		trx_free_for_mysql(trx);
-		trx_commit_for_mysql(prebuilt->trx);
 		DBUG_RETURN(error);
 	}
 
+	heap = mem_heap_create(1024);
+	trx_start_if_not_started(prebuilt->trx);
+
+	/* Create a background transaction for the operations on
+	the data dictionary tables. */
+	trx = innobase_trx_allocate(user_thd);
+	trx_start_if_not_started(trx);
+
 	/* Create table containing all indexes to be built in this
 	alter table add index so that they are in the correct order
 	in the table. */
@@ -758,8 +751,12 @@ err_exit:
 
 			ut_d(dict_table_check_for_dup_indexes(innodb_table,
 							      FALSE));
+			mem_heap_free(heap);
+			trx_general_rollback_for_mysql(trx, NULL);
 			row_mysql_unlock_data_dictionary(trx);
-			goto err_exit;
+			trx_free_for_mysql(trx);
+			trx_commit_for_mysql(prebuilt->trx);
+			DBUG_RETURN(error);
 		}
 
 		trx->table_id = indexed_table->id;

Attachment: [text/bzr-bundle] bzr/marko.makela@oracle.com-20110407192445-tjcj686on9pa6aeq.bundle
Thread
bzr commit into mysql-5.1-innodb branch (marko.makela:3725) Bug#11760042marko.makela7 Apr