MDEV-19529 InnoDB hang on DROP FULLTEXT INDEX

Problem:
=======
  During dropping of fts index, InnoDB waits for fts_optimize_remove_table()
and it holds dict_sys->mutex and dict_operaiton_lock even though the
table id is not present in the queue. But fts_optimize_thread does wait
for dict_sys->mutex to process the unrelated table id from the slot.

Solution:
========
  Whenever table is added to fts_optimize_wq, update the fts_status
of in-memory fts subsystem to TABLE_IN_QUEUE. Whenever drop index
wants to remove table from the queue, it can check the fts_status
to decide whether it should send the MSG_DELETE_TABLE to the queue.

Removed the following functions because these are all deadcode.
dict_table_wait_for_bg_threads_to_exit(),
fts_wait_for_background_thread_to_start(),fts_start_shutdown(), fts_shudown().

12 files changed, 60 insertions, 245 deletions

diff --git a/storage/innobase/dict/dict0dict.cc b/storage/innobase/dict/dict0dict.cc
index c6ccc37d589..f9295e74d7c 100644
--- a/storage/innobase/dict/dict0dict.cc
+++ b/storage/innobase/dict/dict0dict.cc
@@ -2911,33 +2911,6 @@ dict_table_copy_types(
 	}
 }
 
-/********************************************************************
-Wait until all the background threads of the given table have exited, i.e.,
-bg_threads == 0. Note: bg_threads_mutex must be reserved when
-calling this. */
-UNIV_INTERN
-void
-dict_table_wait_for_bg_threads_to_exit(
-/*===================================*/
-	dict_table_t*	table,	/*< in: table */
-	ulint		delay)	/*< in: time in microseconds to wait between
-				checks of bg_threads. */
-{
-	fts_t*		fts = table->fts;
-
-#ifdef UNIV_SYNC_DEBUG
-	ut_ad(mutex_own(&fts->bg_threads_mutex));
-#endif /* UNIV_SYNC_DEBUG */
-
-	while (fts->bg_threads > 0) {
-		mutex_exit(&fts->bg_threads_mutex);
-
-		os_thread_sleep(delay);
-
-		mutex_enter(&fts->bg_threads_mutex);
-	}
-}
-
 /*******************************************************************//**
 Builds the internal dictionary cache representation for a clustered
 index, containing also system fields not defined by the user.
diff --git a/storage/innobase/fts/fts0config.cc b/storage/innobase/fts/fts0config.cc
index 940c944a436..b39d568575c 100644
--- a/storage/innobase/fts/fts0config.cc
+++ b/storage/innobase/fts/fts0config.cc
@@ -224,8 +224,7 @@ fts_config_set_value(
 	pars_info_bind_varchar_literal(info, "value",
 				       value->f_str, value->f_len);
 
-	const bool dict_locked = fts_table->table->fts->fts_status
-		& TABLE_DICT_LOCKED;
+	const bool dict_locked = fts_table->table->fts->dict_locked;
 
 	fts_table->suffix = "CONFIG";
 	fts_get_table_name(fts_table, table_name, dict_locked);
diff --git a/storage/innobase/fts/fts0fts.cc b/storage/innobase/fts/fts0fts.cc
index 264c520bb1e..1cf5c818a9e 100644
--- a/storage/innobase/fts/fts0fts.cc
+++ b/storage/innobase/fts/fts0fts.cc
@@ -458,7 +458,7 @@ fts_load_user_stopword(
 	dberr_t		error = DB_SUCCESS;
 	ibool		ret = TRUE;
 	trx_t*		trx;
-	ibool		has_lock = fts->fts_status & TABLE_DICT_LOCKED;
+	ibool		has_lock = fts->dict_locked;
 
 	trx = trx_allocate_for_background();
 	trx->op_info = "Load user stopword table into FTS cache";
@@ -933,18 +933,16 @@ fts_que_graph_free_check_lock(
 	const fts_index_cache_t*index_cache,	/*!< in: FTS index cache */
 	que_t*			graph)		/*!< in: query graph */
 {
-	ibool	has_dict = FALSE;
+	bool	has_dict = FALSE;
 
 	if (fts_table && fts_table->table) {
 		ut_ad(fts_table->table->fts);
 
-		has_dict = fts_table->table->fts->fts_status
-			 & TABLE_DICT_LOCKED;
+		has_dict = fts_table->table->fts->dict_locked;
 	} else if (index_cache) {
 		ut_ad(index_cache->index->table->fts);
 
-		has_dict = index_cache->index->table->fts->fts_status
-			 & TABLE_DICT_LOCKED;
+		has_dict = index_cache->index->table->fts->dict_locked;
 	}
 
 	if (!has_dict) {
@@ -2831,7 +2829,7 @@ fts_update_sync_doc_id(
 	pars_info_bind_varchar_literal(info, "doc_id", id, id_len);
 
 	fts_get_table_name(&fts_table, fts_name,
-			   table->fts->fts_status & TABLE_DICT_LOCKED);
+			   table->fts->dict_locked);
 	pars_info_bind_id(info, true, "table_name", fts_name);
 
 	graph = fts_parse_sql(
@@ -2947,7 +2945,7 @@ fts_delete(
 	into cache from last crash (delete Doc will not initialize the
 	sync). Avoid any added counter accounting until the FTS cache
 	is re-established and sync-ed */
-	if (table->fts->fts_status & ADDED_TABLE_SYNCED
+	if (table->fts->added_synced
 	    && doc_id > cache->synced_doc_id) {
 		mutex_enter(&table->fts->cache->deleted_lock);
 
@@ -3385,7 +3383,7 @@ fts_add_doc_by_id(
 	/* If Doc ID has been supplied by the user, then the table
 	might not yet be sync-ed */
 
-	if (!(ftt->table->fts->fts_status & ADDED_TABLE_SYNCED)) {
+	if (!ftt->table->fts->added_synced) {
 		fts_init_index(ftt->table, FALSE);
 	}
 
@@ -4933,7 +4931,7 @@ fts_init_doc_id(
 		fts_init_index((dict_table_t*) table, TRUE);
 	}
 
-	table->fts->fts_status |= ADDED_TABLE_SYNCED;
+	table->fts->added_synced = true;
 
 	table->fts->cache->first_doc_id = max_doc_id;
 
@@ -5387,69 +5385,6 @@ fts_cache_append_deleted_doc_ids(
 }
 
 /*********************************************************************//**
-Wait for the background thread to start. We poll to detect change
-of state, which is acceptable, since the wait should happen only
-once during startup.
-@return true if the thread started else FALSE (i.e timed out) */
-UNIV_INTERN
-ibool
-fts_wait_for_background_thread_to_start(
-/*====================================*/
-	dict_table_t*		table,		/*!< in: table to which the thread
-						is attached */
-	ulint			max_wait)	/*!< in: time in microseconds, if
-						set to 0 then it disables
-						timeout checking */
-{
-	ulint			count = 0;
-	ibool			done = FALSE;
-
-	ut_a(max_wait == 0 || max_wait >= FTS_MAX_BACKGROUND_THREAD_WAIT);
-
-	for (;;) {
-		fts_t*		fts = table->fts;
-
-		mutex_enter(&fts->bg_threads_mutex);
-
-		if (fts->fts_status & BG_THREAD_READY) {
-
-			done = TRUE;
-		}
-
-		mutex_exit(&fts->bg_threads_mutex);
-
-		if (!done) {
-			os_thread_sleep(FTS_MAX_BACKGROUND_THREAD_WAIT);
-
-			if (max_wait > 0) {
-
-				max_wait -= FTS_MAX_BACKGROUND_THREAD_WAIT;
-
-				/* We ignore the residual value. */
-				if (max_wait < FTS_MAX_BACKGROUND_THREAD_WAIT) {
-					break;
-				}
-			}
-
-			++count;
-		} else {
-			break;
-		}
-
-		if (count >= FTS_BACKGROUND_THREAD_WAIT_COUNT) {
-			ut_print_timestamp(stderr);
-			fprintf(stderr, " InnoDB: Error the background thread "
-				"for the FTS table %s refuses to start\n",
-				table->name);
-
-			count = 0;
-		}
-	}
-
-	return(done);
-}
-
-/*********************************************************************//**
 Add the FTS document id hidden column. */
 UNIV_INTERN
 void
@@ -5590,42 +5525,6 @@ fts_free(
 }
 
 /*********************************************************************//**
-Signal FTS threads to initiate shutdown. */
-UNIV_INTERN
-void
-fts_start_shutdown(
-/*===============*/
-	dict_table_t*	table,		/*!< in: table with FTS indexes */
-	fts_t*		fts)		/*!< in: fts instance that needs
-					to be informed about shutdown */
-{
-	mutex_enter(&fts->bg_threads_mutex);
-
-	fts->fts_status |= BG_THREAD_STOP;
-
-	mutex_exit(&fts->bg_threads_mutex);
-
-}
-
-/*********************************************************************//**
-Wait for FTS threads to shutdown. */
-UNIV_INTERN
-void
-fts_shutdown(
-/*=========*/
-	dict_table_t*	table,		/*!< in: table with FTS indexes */
-	fts_t*		fts)		/*!< in: fts instance to shutdown */
-{
-	mutex_enter(&fts->bg_threads_mutex);
-
-	ut_a(fts->fts_status & BG_THREAD_STOP);
-
-	dict_table_wait_for_bg_threads_to_exit(table, 20000);
-
-	mutex_exit(&fts->bg_threads_mutex);
-}
-
-/*********************************************************************//**
 Take a FTS savepoint. */
 UNIV_INLINE
 void
@@ -7639,7 +7538,7 @@ fts_init_index(
 	}
 	rw_lock_x_unlock(&cache->init_lock);
 
-	if (table->fts->fts_status & ADDED_TABLE_SYNCED) {
+	if (table->fts->added_synced) {
 		goto func_exit;
 	}
 
@@ -7681,7 +7580,7 @@ fts_init_index(
 		}
 	}
 
-	table->fts->fts_status |= ADDED_TABLE_SYNCED;
+	table->fts->added_synced = true;
 
 	fts_get_docs_clear(cache->get_docs);
 
diff --git a/storage/innobase/fts/fts0opt.cc b/storage/innobase/fts/fts0opt.cc
index 553c6ff392b..3c15b514102 100644
--- a/storage/innobase/fts/fts0opt.cc
+++ b/storage/innobase/fts/fts0opt.cc
@@ -2628,6 +2628,10 @@ UNIV_INTERN void fts_optimize_add_table(dict_table_t* table)
 	msg = fts_optimize_create_msg(FTS_MSG_ADD_TABLE, table);
 
 	ib_wqueue_add(fts_optimize_wq, msg, msg->heap);
+
+	mutex_enter(&table->fts->bg_threads_mutex);
+	table->fts->in_queue = true;
+	mutex_exit(&table->fts->bg_threads_mutex);
 }
 
 /**********************************************************************//**
@@ -2656,6 +2660,15 @@ fts_optimize_remove_table(
 		return;
 	}
 
+	fts_t*	fts = table->fts;
+	mutex_enter(&fts->bg_threads_mutex);
+	bool is_in_optimize_queue = fts->in_queue;
+	mutex_exit(&fts->bg_threads_mutex);
+
+	if (!is_in_optimize_queue) {
+		return;
+	}
+
 	msg = fts_optimize_create_msg(FTS_MSG_DEL_TABLE, NULL);
 
 	/* We will wait on this event until signalled by the consumer. */
@@ -2673,6 +2686,10 @@ fts_optimize_remove_table(
 	os_event_wait(event);
 
 	os_event_free(event);
+
+	mutex_enter(&fts->bg_threads_mutex);
+	fts->in_queue = false;
+	mutex_exit(&fts->bg_threads_mutex);
 }
 
 /** Send sync fts cache for the table.
@@ -2706,6 +2723,10 @@ fts_optimize_request_sync_table(
 	msg->ptr = table_id;
 
 	ib_wqueue_add(fts_optimize_wq, msg, msg->heap);
+
+	mutex_enter(&table->fts->bg_threads_mutex);
+	table->fts->in_queue = true;
+	mutex_exit(&table->fts->bg_threads_mutex);
 }
 
 /** Add a table to fts_slots if it doesn't already exist. */
@@ -2850,6 +2871,10 @@ static void fts_optimize_sync_table(table_id_t table_id)
 			fts_sync_table(table, true, false, false);
 		}
 
+		DBUG_EXECUTE_IF(
+			"ib_optimize_wq_hang",
+			os_thread_sleep(6000000););
+
 		dict_table_close(table, FALSE, FALSE);
 	}
 }
diff --git a/storage/innobase/fts/fts0sql.cc b/storage/innobase/fts/fts0sql.cc
index 258164415f4..2c91115281b 100644
--- a/storage/innobase/fts/fts0sql.cc
+++ b/storage/innobase/fts/fts0sql.cc
@@ -165,8 +165,7 @@ fts_parse_sql(
 	str = ut_str3cat(fts_sql_begin, sql, fts_sql_end);
 
 	dict_locked = (fts_table && fts_table->table->fts
-		       && (fts_table->table->fts->fts_status
-			   & TABLE_DICT_LOCKED));
+		       && fts_table->table->fts->dict_locked);
 
 	if (!dict_locked) {
 		ut_ad(!mutex_own(&(dict_sys->mutex)));
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
index 5dfe240631b..fd8f0d3787f 100644
--- a/storage/innobase/handler/ha_innodb.cc
+++ b/storage/innobase/handler/ha_innodb.cc
@@ -10094,10 +10094,10 @@ ha_innobase::ft_init_ext(
 		return(NULL);
 	}
 
-	if (!(ft_table->fts->fts_status & ADDED_TABLE_SYNCED)) {
+	if (!(ft_table->fts->added_synced)) {
 		fts_init_index(ft_table, FALSE);
 
-		ft_table->fts->fts_status |= ADDED_TABLE_SYNCED;
+		ft_table->fts->added_synced = true;
 	}
 
 	error = fts_query(trx, index, flags, query, query_len, &result);
diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc
index 24db706d0bc..682f09992a4 100644
--- a/storage/innobase/handler/handler0alter.cc
+++ b/storage/innobase/handler/handler0alter.cc
@@ -3245,15 +3245,13 @@ op_ok:
 				goto error_handling;
 			}
 
-			ctx->new_table->fts->fts_status
-				|= TABLE_DICT_LOCKED;
+			ctx->new_table->fts->dict_locked = true;
 
 			error = innobase_fts_load_stopword(
 				ctx->new_table, ctx->trx,
 				ctx->prebuilt->trx->mysql_thd)
 				? DB_SUCCESS : DB_ERROR;
-			ctx->new_table->fts->fts_status
-				&= ~TABLE_DICT_LOCKED;
+			ctx->new_table->fts->dict_locked = false;
 
 			if (error != DB_SUCCESS) {
 				goto error_handling;
@@ -5624,6 +5622,7 @@ commit_cache_norebuild(
 					    || (index->type
 						& DICT_CORRUPT));
 				DBUG_ASSERT(index->table->fts);
+				DEBUG_SYNC_C("norebuild_fts_drop");
 				fts_drop_index(index->table, index, trx);
 			}
 
diff --git a/storage/innobase/include/dict0dict.h b/storage/innobase/include/dict0dict.h
index 37f2a4e2b23..93ca3692141 100644
--- a/storage/innobase/include/dict0dict.h
+++ b/storage/innobase/include/dict0dict.h
@@ -1016,18 +1016,6 @@ dict_table_copy_types(
 	dtuple_t*		tuple,	/*!< in/out: data tuple */
 	const dict_table_t*	table)	/*!< in: table */
 	MY_ATTRIBUTE((nonnull));
-/********************************************************************
-Wait until all the background threads of the given table have exited, i.e.,
-bg_threads == 0. Note: bg_threads_mutex must be reserved when
-calling this. */
-UNIV_INTERN
-void
-dict_table_wait_for_bg_threads_to_exit(
-/*===================================*/
-	dict_table_t*	table,	/* in: table */
-	ulint		delay)	/* in: time in microseconds to wait between
-				checks of bg_threads. */
-	MY_ATTRIBUTE((nonnull));
 /**********************************************************************//**
 Looks for an index with the given id. NOTE that we do not reserve
 the dictionary mutex: this function is for emergency purposes like
diff --git a/storage/innobase/include/fts0fts.h b/storage/innobase/include/fts0fts.h
index a1fab659732..5c50e381f91 100644
--- a/storage/innobase/include/fts0fts.h
+++ b/storage/innobase/include/fts0fts.h
@@ -279,40 +279,21 @@ struct fts_table_t {
 					index auxiliary table */
 };
 
-enum	fts_status {
-	BG_THREAD_STOP = 1,	 	/*!< TRUE if the FTS background thread
-					has finished reading the ADDED table,
-					meaning more items can be added to
-					the table. */
-
-	BG_THREAD_READY = 2,		/*!< TRUE if the FTS background thread
-					is ready */
-
-	ADD_THREAD_STARTED = 4,		/*!< TRUE if the FTS add thread
-					has started */
-
-	ADDED_TABLE_SYNCED = 8,		/*!< TRUE if the ADDED table record is
-					sync-ed after crash recovery */
-
-	TABLE_DICT_LOCKED = 16		/*!< Set if the table has
-					dict_sys->mutex */
-};
-
-typedef	enum fts_status	fts_status_t;
-
 /** The state of the FTS sub system. */
 struct fts_t {
 					/*!< mutex protecting bg_threads* and
 					fts_add_wq. */
 	ib_mutex_t		bg_threads_mutex;
 
-	ulint		bg_threads;	/*!< number of background threads
-					accessing this table */
-
-					/*!< TRUE if background threads running
-					should stop themselves */
-	ulint		fts_status;	/*!< Status bit regarding fts
-					running state */
+	/* Wheter the table was added to fts_optimize_wq();
+	protected by bg_threads mutex */
+	unsigned	in_queue:1;
+	/* Whether the ADDED table record sync-ed after
+	crash recovery; protected by bg_threads mutex */
+	unsigned	added_synced:1;
+	/* Whether the table hold dict_sys->mutex;
+	protected by bg_threads mutex */
+	unsigned	dict_locked:1;
 
 	ib_wqueue_t*	add_wq;		/*!< Work queue for scheduling jobs
 					for the FTS 'Add' thread, or NULL
@@ -615,28 +596,6 @@ fts_startup(void);
 /*==============*/
 
 /******************************************************************//**
-Signal FTS threads to initiate shutdown. */
-UNIV_INTERN
-void
-fts_start_shutdown(
-/*===============*/
-	dict_table_t*	table,			/*!< in: table with FTS
-						indexes */
-	fts_t*		fts);			/*!< in: fts instance to
-						shutdown */
-
-/******************************************************************//**
-Wait for FTS threads to shutdown. */
-UNIV_INTERN
-void
-fts_shutdown(
-/*=========*/
-	dict_table_t*	table,			/*!< in: table with FTS
-						indexes */
-	fts_t*		fts);			/*!< in: fts instance to
-						shutdown */
-
-/******************************************************************//**
 Create an instance of fts_t.
 @return instance of fts_t */
 UNIV_INTERN
diff --git a/storage/innobase/include/fts0priv.h b/storage/innobase/include/fts0priv.h
index 266534c9511..1622eaa07ae 100644
--- a/storage/innobase/include/fts0priv.h
+++ b/storage/innobase/include/fts0priv.h
@@ -521,20 +521,6 @@ fts_cache_append_deleted_doc_ids(
 	const fts_cache_t*
 			cache,		/*!< in: cache to use */
 	ib_vector_t*	vector);	/*!< in: append to this vector */
-/******************************************************************//**
-Wait for the background thread to start. We poll to detect change
-of state, which is acceptable, since the wait should happen only
-once during startup.
-@return true if the thread started else FALSE (i.e timed out) */
-UNIV_INTERN
-ibool
-fts_wait_for_background_thread_to_start(
-/*====================================*/
-	dict_table_t*	table,		/*!< in: table to which the thread
-					is attached */
-	ulint		max_wait);	/*!< in: time in microseconds, if set
-					to 0 then it disables timeout
-					checking */
 #ifdef FTS_DOC_STATS_DEBUG
 /******************************************************************//**
 Get the total number of words in the FTS for a particular FTS index.
diff --git a/storage/innobase/row/row0mysql.cc b/storage/innobase/row/row0mysql.cc
index 52aa43384f7..c6d9909a6fc 100644
--- a/storage/innobase/row/row0mysql.cc
+++ b/storage/innobase/row/row0mysql.cc
@@ -3829,11 +3829,11 @@ next_rec:
 			DBUG_EXECUTE_IF("ib_trunc_sleep_before_fts_cache_clear",
 					os_thread_sleep(10000000););
 
-			table->fts->fts_status |= TABLE_DICT_LOCKED;
+			table->fts->dict_locked = true;
 			fts_update_next_doc_id(trx, table, 0);
 			fts_cache_clear(table->fts->cache);
 			fts_cache_init(table->fts->cache);
-			table->fts->fts_status &= ~TABLE_DICT_LOCKED;
+			table->fts->dict_locked = false;
 		}
 	}
 
@@ -4434,8 +4434,7 @@ do_drop:
 			/* Need to set TABLE_DICT_LOCKED bit, since
 			fts_que_graph_free_check_lock would try to acquire
 			dict mutex lock */
-			table->fts->fts_status |= TABLE_DICT_LOCKED;
-
+			table->fts->dict_locked = true;
 			fts_free(table);
 		}
 
diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc
index 06de737a5b3..76d501aad53 100644
--- a/storage/innobase/trx/trx0trx.cc
+++ b/storage/innobase/trx/trx0trx.cc
@@ -1062,27 +1062,16 @@ trx_finalize_for_fts_table(
 {
 	fts_t*                  fts = ftt->table->fts;
 	fts_doc_ids_t*          doc_ids = ftt->added_doc_ids;
+	mem_heap_t*		heap;
 
-	mutex_enter(&fts->bg_threads_mutex);
+	ut_a(fts->add_wq);
 
-	if (fts->fts_status & BG_THREAD_STOP) {
-		/* The table is about to be dropped, no use
-		adding anything to its work queue. */
+	heap = static_cast<mem_heap_t*>(doc_ids->self_heap->arg);
 
-		mutex_exit(&fts->bg_threads_mutex);
-	} else {
-		mem_heap_t*     heap;
-		mutex_exit(&fts->bg_threads_mutex);
-
-		ut_a(fts->add_wq);
-
-		heap = static_cast<mem_heap_t*>(doc_ids->self_heap->arg);
+	ib_wqueue_add(fts->add_wq, doc_ids, heap);
 
-		ib_wqueue_add(fts->add_wq, doc_ids, heap);
-
-		/* fts_trx_table_t no longer owns the list. */
-		ftt->added_doc_ids = NULL;
-	}
+	/* fts_trx_table_t no longer owns the list. */
+	ftt->added_doc_ids = NULL;
 }
 
 /******************************************************************//**