MDEV-16855 Fix fts_sync_synchronization in InnoDB

This is a backport of the following fix from MySQL 5.7.23.
Some code refactoring has been omitted, and the test case has
been adapted to MariaDB.

commit 7a689acaa65e9d602575f7aa53fe36a64a07460f
Author: Krzysztof Kapuścik <krzysztof.kapuscik@oracle.com>
Date:   Tue Mar 13 12:34:03 2018 +0100

Bug#27082268 Invalid FTS sync synchronization

The fix closes two issues:
Bug #27082268 - INNODB: FAILING ASSERTION: SYM_NODE->TABLE != NULL DURING FTS SYNC
Bug #27095935 - DEADLOCK BETWEEN FTS_DROP_INDEX AND FTS_OPTIMIZE_SYNC_TABLE

Both issues were related to a FTS cache sync being done during
operations that perfomed DDL actions on internal FTS tables
(ALTER TABLE, TRUNCATE). In some cases the FTS tables and/or
internal cache structures could get removed while still being
used to perform FTS synchronization leading to crashes. In other
the sync operations could not get finishes as it was waiting for
dict lock which was taken by thread waiting for the background
sync to be finished.

The changes done includes:
- Stopping background operations during ALTER TABLE and TRUNCATE.
- Removal of unused code in FTS.
- Cleanup of FTS sync related code to make it more readable and
easier to maintain.

RB#18262

6 files changed, 78 insertions, 220 deletions

diff --git a/storage/innobase/fts/fts0fts.cc b/storage/innobase/fts/fts0fts.cc
index 86dd76bbf49..6044abd8e79 100644
--- a/storage/innobase/fts/fts0fts.cc
+++ b/storage/innobase/fts/fts0fts.cc
@@ -268,7 +268,7 @@ 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
+@param[in]	has_dict	whether has dict operation lock
 @return DB_SUCCESS if all OK */
 static
 dberr_t
@@ -3961,6 +3961,9 @@ fts_sync_write_words(
 
 		word = rbt_value(fts_tokenizer_word_t, rbt_node);
 
+		DBUG_EXECUTE_IF("fts_instrument_write_words_before_select_index",
+				os_thread_sleep(300000););
+
 		selected = fts_select_index(
 			index_cache->charset, word->text.f_str,
 			word->text.f_len);
@@ -4525,7 +4528,7 @@ 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
+@param[in]	has_dict	whether has dict operation lock
 @return DB_SUCCESS if all OK */
 static
 dberr_t
@@ -4587,15 +4590,13 @@ begin_sync:
 			continue;
 		}
 
+		DBUG_EXECUTE_IF("fts_instrument_sync_before_syncing",
+				os_thread_sleep(300000););
 		index_cache->index->index_fts_syncing = true;
-		DBUG_EXECUTE_IF("fts_instrument_sync_sleep_drop_waits",
-				os_thread_sleep(10000000);
-				);
 
 		error = fts_sync_index(sync, index_cache);
 
-		if (error != DB_SUCCESS && !sync->interrupted) {
-
+		if (error != DB_SUCCESS) {
 			goto end_sync;
 		}
 	}
@@ -4630,8 +4631,8 @@ end_sync:
 	}
 
 	rw_lock_x_lock(&cache->lock);
-	/* Clear fts syncing flags of any indexes incase sync is
-	interrupeted */
+	/* Clear fts syncing flags of any indexes in case sync is
+	interrupted */
 	for (i = 0; i < ib_vector_size(cache->indexes); ++i) {
 		static_cast<fts_index_cache_t*>(
 			ib_vector_get(cache->indexes, i))
diff --git a/storage/innobase/fts/fts0opt.cc b/storage/innobase/fts/fts0opt.cc
index d9f96948000..77293bc867a 100644
--- a/storage/innobase/fts/fts0opt.cc
+++ b/storage/innobase/fts/fts0opt.cc
@@ -1,6 +1,6 @@
 /*****************************************************************************
 
-Copyright (c) 2007, 2017, Oracle and/or its affiliates. All Rights Reserved.
+Copyright (c) 2007, 2018, Oracle and/or its affiliates. All Rights Reserved.
 Copyright (c) 2016, MariaDB Corporation. All Rights reserved.
 
 This program is free software; you can redistribute it and/or modify it under
@@ -58,12 +58,6 @@ static const ulint FTS_WORD_NODES_INIT_SIZE = 64;
 /** Last time we did check whether system need a sync */
 static ib_time_t	last_check_sync_time;
 
-#if 0
-/** Check each table in round robin to see whether they'd
-need to be "optimized" */
-static	ulint	fts_optimize_sync_iterator = 0;
-#endif
-
 /** State of a table within the optimization sub system. */
 enum fts_state_t {
 	FTS_STATE_LOADED,
@@ -75,17 +69,11 @@ enum fts_state_t {
 
 /** FTS optimize thread message types. */
 enum fts_msg_type_t {
-	FTS_MSG_START,			/*!< Start optimizing thread */
-
-	FTS_MSG_PAUSE,			/*!< Pause optimizing thread */
-
 	FTS_MSG_STOP,			/*!< Stop optimizing and exit thread */
 
 	FTS_MSG_ADD_TABLE,		/*!< Add table to the optimize thread's
 					work queue */
 
-	FTS_MSG_OPTIMIZE_TABLE,		/*!< Optimize a table */
-
 	FTS_MSG_DEL_TABLE,		/*!< Remove a table from the optimize
 					threads work queue */
 	FTS_MSG_SYNC_TABLE		/*!< Sync fts cache of a table */
@@ -235,7 +223,7 @@ struct fts_msg_t {
 /** The number of words to read and optimize in a single pass. */
 UNIV_INTERN ulong	fts_num_word_optimize;
 
-// FIXME
+/** Whether to enable additional FTS diagnostic printout. */
 UNIV_INTERN char	fts_enable_diag_print;
 
 /** ZLib compressed block size.*/
@@ -2560,13 +2548,9 @@ fts_optimize_create_msg(
 	return(msg);
 }
 
-/**********************************************************************//**
-Add the table to add to the OPTIMIZER's list. */
-UNIV_INTERN
-void
-fts_optimize_add_table(
-/*===================*/
-	dict_table_t*	table)			/*!< in: table to add */
+/** Add the table to add to the OPTIMIZER's list.
+@param[in]	table	table to add */
+UNIV_INTERN void fts_optimize_add_table(dict_table_t* table)
 {
 	fts_msg_t*	msg;
 
@@ -2585,26 +2569,6 @@ fts_optimize_add_table(
 }
 
 /**********************************************************************//**
-Optimize a table. */
-UNIV_INTERN
-void
-fts_optimize_do_table(
-/*==================*/
-	dict_table_t*	table)			/*!< in: table to optimize */
-{
-	fts_msg_t*	msg;
-
-	/* Optimizer thread could be shutdown */
-	if (!fts_optimize_wq) {
-		return;
-	}
-
-	msg = fts_optimize_create_msg(FTS_MSG_OPTIMIZE_TABLE, table);
-
-	ib_wqueue_add(fts_optimize_wq, msg, msg->heap);
-}
-
-/**********************************************************************//**
 Remove the table from the OPTIMIZER's list. We do wait for
 acknowledgement from the consumer of the message. */
 UNIV_INTERN
@@ -2618,7 +2582,7 @@ fts_optimize_remove_table(
 	fts_msg_del_t*	remove;
 
 	/* if the optimize system not yet initialized, return */
-	if (!fts_optimize_wq) {
+	if (!fts_optimize_is_init()) {
 		return;
 	}
 
@@ -2660,7 +2624,7 @@ fts_optimize_request_sync_table(
 	table_id_t*	table_id;
 
 	/* if the optimize system not yet initialized, return */
-	if (!fts_optimize_wq) {
+	if (!fts_optimize_is_init()) {
 		return;
 	}
 
@@ -2683,54 +2647,6 @@ fts_optimize_request_sync_table(
 }
 
 /**********************************************************************//**
-Find the slot for a particular table.
-@return slot if found else NULL. */
-static
-fts_slot_t*
-fts_optimize_find_slot(
-/*===================*/
-	ib_vector_t*		tables,		/*!< in: vector of tables */
-	const dict_table_t*	table)		/*!< in: table to add */
-{
-	ulint		i;
-
-	for (i = 0; i < ib_vector_size(tables); ++i) {
-		fts_slot_t*	slot;
-
-		slot = static_cast<fts_slot_t*>(ib_vector_get(tables, i));
-
-		if (slot->table->id == table->id) {
-			return(slot);
-		}
-	}
-
-	return(NULL);
-}
-
-/**********************************************************************//**
-Start optimizing table. */
-static
-void
-fts_optimize_start_table(
-/*=====================*/
-	ib_vector_t*		tables,		/*!< in/out: vector of tables */
-	dict_table_t*		table)		/*!< in: table to optimize */
-{
-	fts_slot_t*	slot;
-
-	slot = fts_optimize_find_slot(tables, table);
-
-	if (slot == NULL) {
-		ut_print_timestamp(stderr);
-		fprintf(stderr, " InnoDB: Error: table %s not registered "
-			"with the optimize thread.\n", table->name);
-	} else {
-		slot->last_run = 0;
-		slot->completed = 0;
-	}
-}
-
-/**********************************************************************//**
 Add the table to the vector if it doesn't already exist. */
 static
 ibool
@@ -2912,57 +2828,6 @@ fts_is_sync_needed(
 	return(false);
 }
 
-#if 0
-/*********************************************************************//**
-Check whether a table needs to be optimized. */
-static
-void
-fts_optimize_need_sync(
-/*===================*/
-	ib_vector_t*	tables)	/*!< in: list of tables */
-{
-	dict_table_t*	table = NULL;
-	fts_slot_t*	slot;
-	ulint		num_table = ib_vector_size(tables);
-
-	if (!num_table) {
-		return;
-	}
-
-	if (fts_optimize_sync_iterator >= num_table) {
-		fts_optimize_sync_iterator = 0;
-	}
-
-	slot = ib_vector_get(tables, fts_optimize_sync_iterator);
-	table = slot->table;
-
-	if (!table) {
-		return;
-	}
-
-	ut_ad(table->fts);
-
-	if (table->fts->cache) {
-		ulint	deleted = table->fts->cache->deleted;
-
-		if (table->fts->cache->added
-		    >= fts_optimize_add_threshold) {
-			fts_sync_table(table);
-		} else if (deleted >= fts_optimize_delete_threshold) {
-			fts_optimize_do_table(table);
-
-			mutex_enter(&table->fts->cache->deleted_lock);
-			table->fts->cache->deleted -= deleted;
-			mutex_exit(&table->fts->cache->deleted_lock);
-		}
-	}
-
-	fts_optimize_sync_iterator++;
-
-	return;
-}
-#endif
-
 /** Sync fts cache of a table
 @param[in]	table_id	table id */
 void
@@ -2975,7 +2840,7 @@ fts_optimize_sync_table(
 
 	if (table) {
 		if (dict_table_has_fts_index(table) && table->fts->cache) {
-			fts_sync_table(table, true, false, true);
+			fts_sync_table(table, true, false, false);
 		}
 
 		dict_table_close(table, FALSE, FALSE);
@@ -3047,8 +2912,7 @@ fts_optimize_thread(
 			fts_msg_t*	msg;
 
 			msg = static_cast<fts_msg_t*>(
-				ib_wqueue_timedwait(wq,
-						    FTS_QUEUE_WAIT_IN_USECS));
+				ib_wqueue_timedwait(wq, FTS_QUEUE_WAIT_IN_USECS));
 
 			/* Timeout ? */
 			if (msg == NULL) {
@@ -3060,12 +2924,6 @@ fts_optimize_thread(
 			}
 
 			switch (msg->type) {
-			case FTS_MSG_START:
-				break;
-
-			case FTS_MSG_PAUSE:
-				break;
-
 			case FTS_MSG_STOP:
 				done = TRUE;
 				exit_event = (os_event_t) msg->ptr;
@@ -3081,15 +2939,6 @@ fts_optimize_thread(
 				}
 				break;
 
-			case FTS_MSG_OPTIMIZE_TABLE:
-				if (!done) {
-					fts_optimize_start_table(
-						tables,
-						static_cast<dict_table_t*>(
-						msg->ptr));
-				}
-				break;
-
 			case FTS_MSG_DEL_TABLE:
 				if (fts_optimize_del_table(
 					tables, static_cast<fts_msg_del_t*>(
@@ -3104,6 +2953,10 @@ fts_optimize_thread(
 				break;
 
 			case FTS_MSG_SYNC_TABLE:
+				DBUG_EXECUTE_IF(
+					"fts_instrument_msg_sync_sleep",
+					os_thread_sleep(300000););
+
 				fts_optimize_sync_table(
 					*static_cast<table_id_t*>(msg->ptr));
 				break;
@@ -3163,7 +3016,7 @@ fts_optimize_init(void)
 	ut_ad(!srv_read_only_mode);
 
 	/* For now we only support one optimize thread. */
-	ut_a(fts_optimize_wq == NULL);
+	ut_a(!fts_optimize_is_init());
 
 	fts_optimize_wq = ib_wqueue_create();
 	ut_a(fts_optimize_wq != NULL);
diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc
index 3de46ed4f80..6a3e9575eb1 100644
--- a/storage/innobase/handler/handler0alter.cc
+++ b/storage/innobase/handler/handler0alter.cc
@@ -5710,24 +5710,24 @@ ha_innobase::commit_inplace_alter_table(
 	trx_t*		trx		= ctx0->trx;
 	bool		fail		= false;
 
-	if (new_clustered) {
-		for (inplace_alter_handler_ctx** pctx = ctx_array;
-		     *pctx; pctx++) {
-			ha_innobase_inplace_ctx*	ctx
-				= static_cast<ha_innobase_inplace_ctx*>(*pctx);
-			DBUG_ASSERT(ctx->need_rebuild());
+	/* Stop background FTS operations. */
+	for (inplace_alter_handler_ctx** pctx = ctx_array;
+			 *pctx; pctx++) {
+		ha_innobase_inplace_ctx*	ctx
+			= static_cast<ha_innobase_inplace_ctx*>(*pctx);
+
+		DBUG_ASSERT(new_clustered == ctx->need_rebuild());
 
+		if (new_clustered) {
 			if (ctx->old_table->fts) {
 				ut_ad(!ctx->old_table->fts->add_wq);
-				fts_optimize_remove_table(
-					ctx->old_table);
+				fts_optimize_remove_table(ctx->old_table);
 			}
+		}
 
-			if (ctx->new_table->fts) {
-				ut_ad(!ctx->new_table->fts->add_wq);
-				fts_optimize_remove_table(
-					ctx->new_table);
-			}
+		if (ctx->new_table->fts) {
+			ut_ad(!ctx->new_table->fts->add_wq);
+			fts_optimize_remove_table(ctx->new_table);
 		}
 	}
 
@@ -5772,41 +5772,40 @@ ha_innobase::commit_inplace_alter_table(
 
 	/* Make a concurrent Drop fts Index to wait until sync of that
 	fts index is happening in the background */
-	for (;;) {
+	for (int retry_count = 0;;) {
 		bool    retry = false;
 
 		for (inplace_alter_handler_ctx** pctx = ctx_array;
 		    *pctx; pctx++) {
-			int count =0;
 			ha_innobase_inplace_ctx*        ctx
 				= static_cast<ha_innobase_inplace_ctx*>(*pctx);
 			DBUG_ASSERT(new_clustered == ctx->need_rebuild());
 
 			if (dict_fts_index_syncing(ctx->old_table)) {
-				count++;
-				if (count == 100) {
-					fprintf(stderr,
-					"Drop index waiting for background sync"
-					"to finish\n");
-				}
 				retry = true;
+				break;
 			}
 
 			if (new_clustered && dict_fts_index_syncing(ctx->new_table)) {
-				count++;
-				if (count == 100) {
-					fprintf(stderr,
-                                        "Drop index waiting for background sync"
-                                        "to finish\n");
-				}
 				retry = true;
+				break;
 			}
 		}
 
-		 if (!retry) {
+		if (!retry) {
 			 break;
 		}
 
+		/* Print a message if waiting for a long time. */
+		if (retry_count < 100) {
+			retry_count++;
+		} else {
+			ib_logf(IB_LOG_LEVEL_INFO,
+				"Drop index waiting for background sync"
+				" to finish");
+			retry_count = 0;
+		}
+
 		DICT_BG_YIELD(trx);
 	}
 
@@ -6070,6 +6069,11 @@ foreign_fail:
 			ut_a(fts_check_cached_index(ctx->old_table));
 			DBUG_INJECT_CRASH("ib_commit_inplace_crash_fail",
 					  crash_fail_inject_count++);
+
+			/* Restart the FTS background operations. */
+			if (ctx->old_table->fts) {
+				fts_optimize_add_table(ctx->old_table);
+			}
 		}
 
 		row_mysql_unlock_data_dictionary(trx);
@@ -6118,8 +6122,6 @@ foreign_fail:
 			dict_table_autoinc_unlock(t);
 		}
 
-		bool	add_fts	= false;
-
 		/* Publish the created fulltext index, if any.
 		Note that a fulltext index can be created without
 		creating the clustered index, if there already exists
@@ -6134,14 +6136,14 @@ foreign_fail:
 				is left unset when a drop proceeds the add. */
 				DICT_TF2_FLAG_SET(ctx->new_table, DICT_TF2_FTS);
 				fts_add_index(index, ctx->new_table);
-				add_fts = true;
 			}
 		}
 
 		ut_d(dict_table_check_for_dup_indexes(
 			     ctx->new_table, CHECK_ALL_COMPLETE));
 
-		if (add_fts) {
+		/* Start/Restart the FTS background operations. */
+		if (ctx->new_table->fts) {
 			fts_optimize_add_table(ctx->new_table);
 		}
 
diff --git a/storage/innobase/include/fts0fts.h b/storage/innobase/include/fts0fts.h
index cd94956dc55..4c2d247e0a6 100644
--- a/storage/innobase/include/fts0fts.h
+++ b/storage/innobase/include/fts0fts.h
@@ -1,6 +1,6 @@
 /*****************************************************************************
 
-Copyright (c) 2011, 2016, Oracle and/or its affiliates. All Rights Reserved.
+Copyright (c) 2011, 2018, Oracle and/or its affiliates. All Rights Reserved.
 Copyright (c) 2016, 2017, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
@@ -705,6 +705,12 @@ fts_drop_index_tables(
 	dict_index_t*	index)			/*!< in: Index to drop */
 	MY_ATTRIBUTE((nonnull, warn_unused_result));
 
+/** Add the table to add to the OPTIMIZER's list.
+@param[in]	table	table to add */
+void
+fts_optimize_add_table(
+	dict_table_t*	table);
+
 /******************************************************************//**
 Remove the table from the OPTIMIZER's list. We do wait for
 acknowledgement from the consumer of the message. */
diff --git a/storage/innobase/include/fts0priv.h b/storage/innobase/include/fts0priv.h
index 2d4e9d88fd1..a3936f54a48 100644
--- a/storage/innobase/include/fts0priv.h
+++ b/storage/innobase/include/fts0priv.h
@@ -1,6 +1,6 @@
 /*****************************************************************************
 
-Copyright (c) 2011, 2016, Oracle and/or its affiliates. All Rights Reserved.
+Copyright (c) 2011, 2018, Oracle and/or its affiliates. All Rights Reserved.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -598,22 +598,6 @@ fts_get_table_id(
 					long */
 	MY_ATTRIBUTE((nonnull, warn_unused_result));
 /******************************************************************//**
-Add the table to add to the OPTIMIZER's list. */
-UNIV_INTERN
-void
-fts_optimize_add_table(
-/*===================*/
-	dict_table_t*	table)		/*!< in: table to add */
-	MY_ATTRIBUTE((nonnull));
-/******************************************************************//**
-Optimize a table. */
-UNIV_INTERN
-void
-fts_optimize_do_table(
-/*==================*/
-	dict_table_t*	table)		/*!< in: table to optimize */
-	MY_ATTRIBUTE((nonnull));
-/******************************************************************//**
 Construct the prefix name of an FTS table.
 @return own: table name, must be freed with mem_free() */
 UNIV_INTERN
diff --git a/storage/innobase/row/row0mysql.cc b/storage/innobase/row/row0mysql.cc
index bb1a27be4ba..87d40ee2ba4 100644
--- a/storage/innobase/row/row0mysql.cc
+++ b/storage/innobase/row/row0mysql.cc
@@ -3309,6 +3309,10 @@ row_truncate_table_for_mysql(
 		return(DB_TABLESPACE_NOT_FOUND);
 	}
 
+	if (table->fts) {
+		fts_optimize_remove_table(table);
+	}
+
 	trx_start_for_ddl(trx, TRX_DICT_OP_TABLE);
 
 	trx->op_info = "truncating table";
@@ -3709,6 +3713,9 @@ next_rec:
 
 		/* Reset the Doc ID in cache to 0 */
 		if (has_internal_doc_id && table->fts->cache) {
+			DBUG_EXECUTE_IF("ib_trunc_sleep_before_fts_cache_clear",
+					os_thread_sleep(10000000););
+
 			table->fts->fts_status |= TABLE_DICT_LOCKED;
 			fts_update_next_doc_id(trx, table, NULL, 0);
 			fts_cache_clear(table->fts->cache);
@@ -3732,6 +3739,11 @@ funct_exit:
                 table->memcached_sync_count = 0;
         }
 
+	/* Add the table back to FTS optimize background thread. */
+	if (table->fts) {
+		fts_optimize_add_table(table);
+	}
+
 	row_mysql_unlock_data_dictionary(trx);
 
 	dict_stats_update(table, DICT_STATS_EMPTY_TABLE);