bzr push into mysql-trunk branch (marko.makela:4016 to 4017) Bug#14198766

[marko.makela]

 4017 Marko Mäkelä	2012-06-15
      Bug#14198766 row_drop_table_for_mysql() releases dict_operation_lock latch
      while holding record locks on data dictionary tables
      
      This bug mostly affects a table rebuild, such as ADD PRIMARY KEY.
      
      ha_innobase::commit_inplace_alter_table(): Invoke
      dict_stats_wait_bg_to_stop_using_tables() on both tables upfront.
      
      row_drop_table_for_mysql(): Do not invoke
      dict_stats_wait_bg_to_stop_using_tables() if data dictionary locks
      were already acquired by the caller (signalled by a new parameter,
      nonatomic=false). Remove the code that waits for the full-text-search
      related background threads to exit. According to Jimmy, this is stale
      code that should be removed. Add a debug assertion to enforce
      !table->fts->add_wq.
      
      row_merge_drop_table(): Add the parameter nonatomic.
      
      rb:1112 approved by Jimmy Yang

    modified:
      storage/innobase/handler/handler0alter.cc
      storage/innobase/include/row0merge.h
      storage/innobase/include/row0mysql.h
      storage/innobase/row/row0merge.cc
      storage/innobase/row/row0mysql.cc
 4016 Yasufumi Kinoshita	2012-06-15
      reorder sql/share/errmsg-utf8.txt about Japanese support
       removed 'jps' entries (because not used at all);
       translated/re-translated major 'jpn' entries accurately;
      
      * reviewed among Japanese-native technical members.

    modified:
      sql/share/errmsg-utf8.txt
=== modified file 'storage/innobase/handler/handler0alter.cc'
--- a/storage/innobase/handler/handler0alter.cc	revid:yasufumi.kinoshita@stripped20615075617-my3og0kau16q0x0p
+++ b/storage/innobase/handler/handler0alter.cc	revid:marko.makela@stripped5093509-az4y1kjcfee03j1c
@@ -2364,7 +2364,7 @@ error_handled:
 	if (new_clustered) {
 		if (indexed_table != user_table) {
 			dict_table_close(indexed_table, TRUE, FALSE);
-			row_merge_drop_table(trx, indexed_table);
+			row_merge_drop_table(trx, indexed_table, true);
 		}
 
 		trx_commit_for_mysql(trx);
@@ -3093,7 +3093,8 @@ rollback_inplace_alter_table(
 	if (prebuilt->table != ctx->indexed_table) {
 		/* Drop the table. */
 		dict_table_close(ctx->indexed_table, TRUE, FALSE);
-		switch (row_merge_drop_table(ctx->trx, ctx->indexed_table)) {
+		switch (row_merge_drop_table(ctx->trx, ctx->indexed_table,
+					     true)) {
 		case DB_SUCCESS:
 			break;
 		default:
@@ -3452,11 +3453,9 @@ ha_innobase::commit_inplace_alter_table(
 
 	/* Wait for background stats processing to stop using the
 	indexes that we are going to drop (if any). */
-	if (ctx != NULL && ctx->num_to_drop > 0) {
+	if (ctx) {
 		dict_stats_wait_bg_to_stop_using_tables(
-			prebuilt->table,
-			ctx->drop[0]->table,
-			trx);
+			prebuilt->table, ctx->indexed_table, trx);
 	}
 
 	if (new_clustered) {
@@ -3467,6 +3466,22 @@ ha_innobase::commit_inplace_alter_table(
 		requested to be dropped were not created in the copy
 		of the table. */
 
+		if (prebuilt->table->fts || ctx->indexed_table->fts) {
+			row_mysql_unlock_data_dictionary(trx);
+
+			if (prebuilt->table->fts) {
+				ut_ad(!prebuilt->table->fts->add_wq);
+				fts_optimize_remove_table(prebuilt->table);
+			}
+
+			if (ctx->indexed_table->fts) {
+				ut_ad(!ctx->indexed_table->fts->add_wq);
+				fts_optimize_remove_table(ctx->indexed_table);
+			}
+
+			row_mysql_lock_data_dictionary(trx);
+		}
+
 		/* A new clustered index was defined for the table
 		and there was no error at this point. We can
 		now rename the old table as a temporary table,
@@ -3488,7 +3503,7 @@ ha_innobase::commit_inplace_alter_table(
 			dict_table_t*	old_table = prebuilt->table;
 			trx_commit_for_mysql(prebuilt->trx);
 			row_prebuilt_free(prebuilt, TRUE);
-			error = row_merge_drop_table(trx, old_table);
+			error = row_merge_drop_table(trx, old_table, false);
 			prebuilt = row_create_prebuilt(
 				ctx->indexed_table, table->s->reclength);
 		}
@@ -3511,7 +3526,7 @@ ha_innobase::commit_inplace_alter_table(
 			err = -1;
 		drop_new_clustered:
 			dict_table_close(ctx->indexed_table, TRUE, FALSE);
-			row_merge_drop_table(trx, ctx->indexed_table);
+			row_merge_drop_table(trx, ctx->indexed_table, false);
 			ctx->indexed_table = NULL;
 			goto trx_commit;
 		}
@@ -3883,5 +3898,10 @@ ret:
 	if (err == 0) {
 		MONITOR_ATOMIC_DEC(MONITOR_PENDING_ALTER_TABLE);
 	}
+
+	if (prebuilt->table->fts) {
+		fts_optimize_add_table(prebuilt->table);
+	}
+
 	DBUG_RETURN(err != 0);
 }

=== modified file 'storage/innobase/include/row0merge.h'
--- a/storage/innobase/include/row0merge.h	revid:[e-mail]-20120615075617-my3og0kau16q0x0p
+++ b/storage/innobase/include/row0merge.h	revid:marko.makela@stripped0120615093509-az4y1kjcfee03j1c
@@ -269,7 +269,11 @@ dberr_t
 row_merge_drop_table(
 /*=================*/
 	trx_t*		trx,		/*!< in: transaction */
-	dict_table_t*	table);		/*!< in: table instance to drop */
+	dict_table_t*	table,		/*!< in: table instance to drop */
+	bool		nonatomic)	/*!< in: whether it is permitted
+					to release and reacquire
+					dict_operation_lock */
+	__attribute__((nonnull));
 /*********************************************************************//**
 Build indexes on a table by reading a clustered index,
 creating a temporary file containing index entries, merge sorting

=== modified file 'storage/innobase/include/row0mysql.h'
--- a/storage/innobase/include/row0mysql.h	revid:yasufumi.kinoshita@stripped20120615075617-my3og0kau16q0x0p
+++ b/storage/innobase/include/row0mysql.h	revid:marko.makela@stripped93509-az4y1kjcfee03j1c
@@ -473,7 +473,10 @@ row_drop_table_for_mysql(
 /*=====================*/
 	const char*	name,	/*!< in: table name */
 	trx_t*		trx,	/*!< in: dictionary transaction handle */
-	bool		drop_db)/*!< in: true=dropping whole database */
+	bool		drop_db,/*!< in: true=dropping whole database */
+	bool		nonatomic = true)
+				/*!< in: whether it is permitted
+				to release and reacquire dict_operation_lock */
 	__attribute__((nonnull));
 /*********************************************************************//**
 Drop all temporary tables during crash recovery. */

=== modified file 'storage/innobase/row/row0merge.cc'
--- a/storage/innobase/row/row0merge.cc	revid:yasufumi.kinoshita@stripped6q0x0p
+++ b/storage/innobase/row/row0merge.cc	revid:marko.makela@stripped
@@ -3123,12 +3123,15 @@ dberr_t
 row_merge_drop_table(
 /*=================*/
 	trx_t*		trx,		/*!< in: transaction */
-	dict_table_t*	table)		/*!< in: table to drop */
+	dict_table_t*	table,		/*!< in: table to drop */
+	bool		nonatomic)	/*!< in: whether it is permitted
+					to release and reacquire
+					dict_operation_lock */
 {
 	/* There must be no open transactions on the table. */
 	ut_a(table->n_ref_count == 0);
 
-	return(row_drop_table_for_mysql(table->name, trx, FALSE));
+	return(row_drop_table_for_mysql(table->name, trx, false, nonatomic));
 }
 
 

=== modified file 'storage/innobase/row/row0mysql.cc'
--- a/storage/innobase/row/row0mysql.cc	revid:[e-mail]-20120615075617-my3og0kau16q0x0p
+++ b/storage/innobase/row/row0mysql.cc	revid:marko.makela@stripped615093509-az4y1kjcfee03j1c
@@ -3522,7 +3522,10 @@ row_drop_table_for_mysql(
 /*=====================*/
 	const char*	name,	/*!< in: table name */
 	trx_t*		trx,	/*!< in: transaction handle */
-	bool		drop_db)/*!< in: true=dropping whole database */
+	bool		drop_db,/*!< in: true=dropping whole database */
+	bool		nonatomic)
+				/*!< in: whether it is permitted
+				to release and reacquire dict_operation_lock */
 {
 	dberr_t		err;
 	dict_foreign_t*	foreign;
@@ -3532,8 +3535,7 @@ row_drop_table_for_mysql(
 	const char*	table_name;
 	bool		ibd_file_missing;
 	ulint		namelen;
-	ibool		locked_dictionary	= FALSE;
-	ibool		fts_bg_thread_exited	= FALSE;
+	bool		locked_dictionary	= false;
 	pars_info_t*	info			= NULL;
 	mem_heap_t*	heap			= NULL;
 
@@ -3603,10 +3605,10 @@ row_drop_table_for_mysql(
 
 		row_mysql_lock_data_dictionary(trx);
 
-		locked_dictionary = TRUE;
+		locked_dictionary = true;
+		nonatomic = true;
 	}
 
-retry:
 	ut_ad(mutex_own(&(dict_sys->mutex)));
 #ifdef UNIV_SYNC_DEBUG
 	ut_ad(rw_lock_own(&dict_operation_lock, RW_LOCK_EX));
@@ -3636,39 +3638,19 @@ retry:
 		goto funct_exit;
 	}
 
-	if (table->fts) {
-		fts_t*          fts = table->fts;
-
-		/* It is possible that background 'Add' thread fts_add_thread()
-		just gets called and the fts_optimize_thread()
-		is processing deleted records. There could be undetected
-		deadlock between threads synchronization and dict_sys_mutex
-		since fts_parse_sql() requires dict_sys->mutex. Ask the
-		background thread to exit before proceeds to drop table to
-		avoid undetected deadlocks */
-		row_mysql_unlock_data_dictionary(trx);
-
-		if (fts->add_wq && (!fts_bg_thread_exited)) {
-			/* Wait for any background threads accessing the table
-			to exit. */
-			mutex_enter(&fts->bg_threads_mutex);
-			fts->fts_status |= BG_THREAD_STOP;
-
-			dict_table_wait_for_bg_threads_to_exit(table, 250000);
-
-			mutex_exit(&fts->bg_threads_mutex);
-
-			row_mysql_lock_data_dictionary(trx);
-			fts_bg_thread_exited = TRUE;
-			goto retry;
-		} else {
+	if (nonatomic) {
+		/* This trx did not acquire any locks on dictionary
+		table records yet. Thus it is safe to release and
+		reacquire the data dictionary latches. */
+		if (table->fts) {
+			ut_ad(!table->fts->add_wq);
+			row_mysql_unlock_data_dictionary(trx);
 			fts_optimize_remove_table(table);
 			row_mysql_lock_data_dictionary(trx);
 		}
+		dict_stats_wait_bg_to_stop_using_tables(table, NULL, trx);
 	}
 
-	dict_stats_wait_bg_to_stop_using_tables(table, NULL, trx);
-
 	dict_stats_remove_table_from_auto_recalc(table);
 
 	/* Remove stats for this table and all of its indexes from the

No bundle (reason: useless for push emails).