Followup: BUG#22516559 MYSQL INSTANCE STALLS WHEN SYNCING FTS INDEX

Shaohua Wang · Shaohua Wang · commit 802b76add9c4 · 2016-06-01T14:39:49.000+08:00
BUG#23320569 INNODB FTS:TRY TO ACQUIRE DICT_OPERATION_LOCK AGAIN
WHEN HOLD IT IN FTS SYNC

Problem:
We hold dict_operation_lock when sync fts in background to prevent
DDL like DROP INDEX, etc. But if sync fails, we need to rollback
(row_undo()), in which we acquire dict_operation_lock again, so
assert fails.

Solution:
Set trx-&gt;dict_operation_lock_mode in fts_sync() to avoid acquiring
dict_operation_lock again in rollback.

Reviewed-by: Jimmy Yang &lt;jimmy.yang@oracle.com&gt;
RB: 12840
diff --git a/mysql-test/suite/innodb_fts/r/sync.result b/mysql-test/suite/innodb_fts/r/sync.result
@@ -100,3 +100,19 @@ FTS_DOC_ID	title
 1	database
 2	mysql
 DROP TABLE t1;
+# Case 4: Test sync commit & rollback in background
+CREATE TABLE t1(
+id INT AUTO_INCREMENT,
+title VARCHAR(100),
+FULLTEXT(title),
+PRIMARY KEY(id)) ENGINE=InnoDB;
+SET SESSION debug="+d,fts_instrument_sync";
+INSERT INTO t1(title) VALUES('database');
+SET GLOBAL debug="+d,fts_instrument_sync,fts_instrument_sync_interrupted";
+INSERT INTO t1(title) VALUES('mysql');
+SET GLOBAL debug="-d,fts_instrument_sync,fts_instrument_sync_interrupted";
+SELECT * FROM t1 WHERE MATCH(title) AGAINST ('mysql database');
+id	title
+1	database
+2	mysql
+DROP TABLE t1;
diff --git a/mysql-test/suite/innodb_fts/t/sync.test b/mysql-test/suite/innodb_fts/t/sync.test
@@ -139,4 +139,25 @@ SELECT * FROM t1 WHERE MATCH(title) AGAINST ('mysql database');
 
 DROP TABLE t1;
 
+--echo # Case 4: Test sync commit & rollback in background
+CREATE TABLE t1(
+        id INT AUTO_INCREMENT,
+        title VARCHAR(100),
+        FULLTEXT(title),
+        PRIMARY KEY(id)) ENGINE=InnoDB;
+
+SET SESSION debug="+d,fts_instrument_sync";
+
+INSERT INTO t1(title) VALUES('database');
+
+SET GLOBAL debug="+d,fts_instrument_sync,fts_instrument_sync_interrupted";
+
+INSERT INTO t1(title) VALUES('mysql');
+
+SET GLOBAL debug="-d,fts_instrument_sync,fts_instrument_sync_interrupted";
+
+SELECT * FROM t1 WHERE MATCH(title) AGAINST ('mysql database');
+
+DROP TABLE t1;
+
 --source include/wait_until_count_sessions.inc
diff --git a/storage/innobase/fts/fts0fts.cc b/storage/innobase/fts/fts0fts.cc
@@ -265,13 +265,15 @@ FTS auxiliary INDEX table and clear the cache at the end.
 @param[in,out]	sync		sync state
 @param[in]	unlock_cache	whether unlock cache lock when write node
 @param[in]	wait		whether wait when a sync is in progress
+@param[in]      has_dict        whether has dict operation lock
 @return DB_SUCCESS if all OK */
 static
 dberr_t
 fts_sync(
 	fts_sync_t*	sync,
 	bool		unlock_cache,
-	bool		wait);
+	bool		wait,
+	bool		has_dict);
 
 /****************************************************************//**
 Release all resources help by the words rb tree e.g., the node ilist. */
@@ -3566,7 +3568,7 @@ fts_add_doc_by_id(
 
 				DBUG_EXECUTE_IF(
 					"fts_instrument_sync_debug",
-					fts_sync(cache->sync, true, true);
+					fts_sync(cache->sync, true, true, false);
 				);
 
 				DEBUG_SYNC_C("fts_instrument_sync_request");
@@ -4467,6 +4469,8 @@ fts_sync_commit(
 			(double) n_nodes/ (double) elapsed_time);
 	}
 
+	/* Avoid assertion in trx_free(). */
+	trx->dict_operation_lock_mode = 0;
 	trx_free_for_background(trx);
 
 	return(error);
@@ -4514,6 +4518,9 @@ fts_sync_rollback(
 	rw_lock_x_unlock(&cache->lock);
 
 	fts_sql_rollback(trx);
+
+	/* Avoid assertion in trx_free(). */
+	trx->dict_operation_lock_mode = 0;
 	trx_free_for_background(trx);
 }
 
@@ -4522,13 +4529,15 @@ FTS auxiliary INDEX table and clear the cache at the end.
 @param[in,out]	sync		sync state
 @param[in]	unlock_cache	whether unlock cache lock when write node
 @param[in]	wait		whether wait when a sync is in progress
+@param[in]      has_dict        whether has dict operation lock
 @return DB_SUCCESS if all OK */
 static
 dberr_t
 fts_sync(
 	fts_sync_t*	sync,
 	bool		unlock_cache,
-	bool		wait)
+	bool		wait,
+	bool		has_dict)
 {
 	ulint		i;
 	dberr_t		error = DB_SUCCESS;
@@ -4557,6 +4566,12 @@ fts_sync(
 	DEBUG_SYNC_C("fts_sync_begin");
 	fts_sync_begin(sync);
 
+	/* When sync in background, we hold dict operation lock
+	to prevent DDL like DROP INDEX, etc. */
+	if (has_dict) {
+		sync->trx->dict_operation_lock_mode = RW_S_LATCH;
+	}
+
 begin_sync:
 	if (cache->total_size > fts_max_cache_size) {
 		/* Avoid the case: sync never finish when
@@ -4635,20 +4650,23 @@ FTS auxiliary INDEX table and clear the cache at the end.
 @param[in,out]	table		fts table
 @param[in]	unlock_cache	whether unlock cache when write node
 @param[in]	wait		whether wait for existing sync to finish
+@param[in]	has_dict	whether has dict operation lock
 @return DB_SUCCESS on success, error code on failure. */
 UNIV_INTERN
 dberr_t
 fts_sync_table(
 	dict_table_t*	table,
 	bool		unlock_cache,
-	bool		wait)
+	bool		wait,
+	bool		has_dict)
 {
 	dberr_t	err = DB_SUCCESS;
 
 	ut_ad(table->fts);
 
 	if (!dict_table_is_discarded(table) && table->fts->cache) {
-		err = fts_sync(table->fts->cache->sync, unlock_cache, wait);
+		err = fts_sync(table->fts->cache->sync,
+			       unlock_cache, wait, has_dict);
 	}
 
 	return(err);
diff --git a/storage/innobase/fts/fts0opt.cc b/storage/innobase/fts/fts0opt.cc
@@ -2986,7 +2986,7 @@ fts_optimize_sync_table(
 
 	if (table) {
 		if (dict_table_has_fts_index(table) && table->fts->cache) {
-			fts_sync_table(table, true, false);
+			fts_sync_table(table, true, false, true);
 		}
 
 		dict_table_close(table, FALSE, FALSE);
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
@@ -11395,7 +11395,7 @@ ha_innobase::optimize(
 	if (innodb_optimize_fulltext_only) {
 		if (prebuilt->table->fts && prebuilt->table->fts->cache
 		    && !dict_table_is_discarded(prebuilt->table)) {
-			fts_sync_table(prebuilt->table, false, true);
+			fts_sync_table(prebuilt->table, false, true, false);
 			fts_optimize_table(prebuilt->table);
 		}
 		return(HA_ADMIN_OK);
diff --git a/storage/innobase/include/fts0fts.h b/storage/innobase/include/fts0fts.h
@@ -840,13 +840,15 @@ FTS auxiliary INDEX table and clear the cache at the end.
 @param[in,out]	table		fts table
 @param[in]	unlock_cache	whether unlock cache when write node
 @param[in]	wait		whether wait for existing sync to finish
+@param[in]      has_dict        whether has dict operation lock
 @return DB_SUCCESS on success, error code on failure. */
 UNIV_INTERN
 dberr_t
 fts_sync_table(
 	dict_table_t*	table,
 	bool		unlock_cache,
-	bool		wait);
+	bool		wait,
+	bool		has_dict);
 
 /****************************************************************//**
 Free the query graph but check whether dict_sys->mutex is already
diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/row0merge.cc
@@ -1987,7 +1987,7 @@ row_merge_read_clustered_index(
 		/* Sync fts cache for other fts indexes to keep all
 		fts indexes consistent in sync_doc_id. */
 		err = fts_sync_table(const_cast<dict_table_t*>(new_table),
-				     false, true);
+				     false, true, false);
 
 		if (err == DB_SUCCESS) {
 			fts_update_next_doc_id(

Original file line number	Diff line number	Diff line change
`@@ -2986,7 +2986,7 @@ fts_optimize_sync_table(`
`2986`	`2986`
`2987`	`2987`	`if (table) {`
`2988`	`2988`	`if (dict_table_has_fts_index(table) && table->fts->cache) {`
`2989`		`- fts_sync_table(table, true, false);`
	`2989`	`+ fts_sync_table(table, true, false, true);`
`2990`	`2990`	`}`
`2991`	`2991`
`2992`	`2992`	`dict_table_close(table, FALSE, FALSE);`
Original file line number	Diff line number	Diff line change
`@@ -11395,7 +11395,7 @@ ha_innobase::optimize(`
`11395`	`11395`	`if (innodb_optimize_fulltext_only) {`
`11396`	`11396`	`if (prebuilt->table->fts && prebuilt->table->fts->cache`
`11397`	`11397`	`&& !dict_table_is_discarded(prebuilt->table)) {`
`11398`		`- fts_sync_table(prebuilt->table, false, true);`
	`11398`	`+ fts_sync_table(prebuilt->table, false, true, false);`
`11399`	`11399`	`fts_optimize_table(prebuilt->table);`
`11400`	`11400`	`}`
`11401`	`11401`	`return(HA_ADMIN_OK);`