MDEV-19505 Do not hold mutex while calling que_graph_free()

sym_tab_free_private(): Do not call dict_table_close(), but
simply invoke dict_table_t::release(), which we can do without
locking the whole dictionary cache. (Note: On user tables it
may still be necessary to invoke dict_table_close(), so that
InnoDB persistent statistics will be deinitialized as expected.)

fts_check_corrupt(), row_fts_merge_insert(): Invoke
aux_table->release() to simplify the code. This is never a user table.

fts_que_graph_free(), fts_que_graph_free_check_lock(): Replaced with
que_graph_free().

Reviewed by: Thirunarayanan Balathandayuthapani

storage/innobase/fts/fts0config.cc

@@ -119,7 +119,7 @@ fts_config_get_value(
 	trx->op_info = "getting FTS config value";
 
 	error = fts_eval_sql(trx, graph);
-	fts_que_graph_free(graph);
+	que_graph_free(graph);
 	return(error);
 }
 
@@ -226,7 +226,7 @@ fts_config_set_value(
 
 	error = fts_eval_sql(trx, graph);
 
-	fts_que_graph_free_check_lock(fts_table, NULL, graph);
+	que_graph_free(graph);
 
 	n_rows_updated = trx->undo_no - undo_no;
 
@@ -252,7 +252,7 @@ fts_config_set_value(
 
 		error = fts_eval_sql(trx, graph);
 
-		fts_que_graph_free_check_lock(fts_table, NULL, graph);
+		que_graph_free(graph);
 	}
 
 	return(error);

storage/innobase/fts/fts0fts.cc

@@ -184,14 +184,6 @@ struct fts_tokenize_param_t {
 	ulint		add_pos;	/*!< Added position for tokens */
 };
 
-/** Free a query graph */
-void fts_que_graph_free(que_t *graph)
-{
-  dict_sys.lock(SRW_LOCK_CALL);
-  que_graph_free(graph);
-  dict_sys.unlock();
-}
-
 /** Run SYNC on the table, i.e., write out data from the cache to the
 FTS auxiliary INDEX table and clear the cache at the end.
 @param[in,out]	sync		sync state
@@ -885,41 +877,6 @@ fts_drop_index(
 	return(err);
 }
 
-/****************************************************************//**
-Free the query graph but check whether dict_sys.latch is already
-held */
-void
-fts_que_graph_free_check_lock(
-/*==========================*/
-	fts_table_t*		fts_table,	/*!< in: FTS table */
-	const fts_index_cache_t*index_cache,	/*!< in: FTS index cache */
-	que_t*			graph)		/*!< in: query graph */
-{
-	bool	has_dict = FALSE;
-
-	if (fts_table && fts_table->table) {
-		ut_ad(fts_table->table->fts);
-
-		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->dict_locked;
-	}
-
-	if (!has_dict) {
-		dict_sys.lock(SRW_LOCK_CALL);
-	}
-
-	ut_ad(dict_sys.locked());
-
-	que_graph_free(graph);
-
-	if (!has_dict) {
-		dict_sys.unlock();
-	}
-}
-
 /****************************************************************//**
 Create an FTS index cache. */
 CHARSET_INFO*
@@ -1067,18 +1024,14 @@ fts_cache_clear(
 
 			if (index_cache->ins_graph[j] != NULL) {
 
-				fts_que_graph_free_check_lock(
-					NULL, index_cache,
-					index_cache->ins_graph[j]);
+				que_graph_free(index_cache->ins_graph[j]);
 
 				index_cache->ins_graph[j] = NULL;
 			}
 
 			if (index_cache->sel_graph[j] != NULL) {
 
-				fts_que_graph_free_check_lock(
-					NULL, index_cache,
-					index_cache->sel_graph[j]);
+				que_graph_free(index_cache->sel_graph[j]);
 
 				index_cache->sel_graph[j] = NULL;
 			}
@@ -2608,7 +2561,7 @@ fts_cmp_set_sync_doc_id(
 
 	error = fts_eval_sql(trx, graph);
 
-	fts_que_graph_free_check_lock(&fts_table, NULL, graph);
+	que_graph_free(graph);
 
 	// FIXME: We need to retry deadlock errors
 	if (error != DB_SUCCESS) {
@@ -2724,7 +2677,7 @@ fts_update_sync_doc_id(
 
 	error = fts_eval_sql(trx, graph);
 
-	fts_que_graph_free_check_lock(&fts_table, NULL, graph);
+	que_graph_free(graph);
 
 	if (local_trx) {
 		if (UNIV_LIKELY(error == DB_SUCCESS)) {
@@ -2866,7 +2819,7 @@ fts_delete(
 
 		error = fts_eval_sql(trx, graph);
 
-		fts_que_graph_free(graph);
+		que_graph_free(graph);
 	} else {
 		pars_info_free(info);
 	}
@@ -3741,7 +3694,7 @@ fts_doc_fetch_by_doc_id(
 	trx->free();
 
 	if (!get_doc) {
-		fts_que_graph_free(graph);
+		que_graph_free(graph);
 	}
 
 	return(error);
@@ -3870,7 +3823,7 @@ fts_sync_add_deleted_cache(
 		error = fts_eval_sql(sync->trx, graph);
 	}
 
-	fts_que_graph_free(graph);
+	que_graph_free(graph);
 
 	return(error);
 }
@@ -4169,18 +4122,14 @@ fts_sync_rollback(
 
 			if (index_cache->ins_graph[j] != NULL) {
 
-				fts_que_graph_free_check_lock(
-					NULL, index_cache,
-					index_cache->ins_graph[j]);
+				que_graph_free(index_cache->ins_graph[j]);
 
 				index_cache->ins_graph[j] = NULL;
 			}
 
 			if (index_cache->sel_graph[j] != NULL) {
 
-				fts_que_graph_free_check_lock(
-					NULL, index_cache,
-					index_cache->sel_graph[j]);
+				que_graph_free(index_cache->sel_graph[j]);
 
 				index_cache->sel_graph[j] = NULL;
 			}
@@ -4744,7 +4693,7 @@ fts_get_docs_clear(
 
 			ut_a(get_doc->index_cache);
 
-			fts_que_graph_free(get_doc->get_document_graph);
+			que_graph_free(get_doc->get_document_graph);
 			get_doc->get_document_graph = NULL;
 		}
 	}
@@ -4892,7 +4841,7 @@ fts_get_rows_count(
 		}
 	}
 
-	fts_que_graph_free(graph);
+	que_graph_free(graph);
 
 	trx->free();
 
@@ -4992,7 +4941,7 @@ fts_savepoint_free(
 
 		/* The default savepoint name must be NULL. */
 		if (ftt->docs_added_graph) {
-			fts_que_graph_free(ftt->docs_added_graph);
+			que_graph_free(ftt->docs_added_graph);
 		}
 
 		/* NOTE: We are responsible for free'ing the node */

storage/innobase/fts/fts0opt.cc

@@ -892,7 +892,7 @@ fts_index_fetch_words(
 			}
 		}
 
-		fts_que_graph_free(graph);
+		que_graph_free(graph);
 
 		/* Check if max word to fetch is exceeded */
 		if (optim->zip->n_words >= n_words) {
@@ -1005,7 +1005,7 @@ fts_table_fetch_doc_ids(
 
 	error = fts_eval_sql(trx, graph);
 	fts_sql_commit(trx);
-	fts_que_graph_free(graph);
+	que_graph_free(graph);
 
 	if (error == DB_SUCCESS) {
 		ib_vector_sort(doc_ids->doc_ids, fts_doc_id_cmp);
@@ -1459,7 +1459,7 @@ fts_optimize_write_word(
 			" when deleting a word from the FTS index.";
 	}
 
-	fts_que_graph_free(graph);
+	que_graph_free(graph);
 	graph = NULL;
 
 	/* Even if the operation needs to be rolled back and redone,
@@ -1491,7 +1491,7 @@ fts_optimize_write_word(
 	}
 
 	if (graph != NULL) {
-		fts_que_graph_free(graph);
+		que_graph_free(graph);
 	}
 
 	return(error);
@@ -1829,7 +1829,7 @@ fts_optimize_words(
 						charset, word->f_str,
 						word->f_len)
 					  && graph) {
-					fts_que_graph_free(graph);
+					que_graph_free(graph);
 					graph = NULL;
 				}
 			}
@@ -1848,7 +1848,7 @@ fts_optimize_words(
 	}
 
 	if (graph != NULL) {
-		fts_que_graph_free(graph);
+		que_graph_free(graph);
 	}
 }
 
@@ -2081,7 +2081,7 @@ fts_optimize_purge_deleted_doc_ids(
 		}
 	}
 
-	fts_que_graph_free(graph);
+	que_graph_free(graph);
 
 	return(error);
 }
@@ -2118,7 +2118,7 @@ fts_optimize_purge_deleted_doc_id_snapshot(
 	graph = fts_parse_sql(NULL, info, fts_end_delete_sql);
 
 	error = fts_eval_sql(optim->trx, graph);
-	fts_que_graph_free(graph);
+	que_graph_free(graph);
 
 	return(error);
 }
@@ -2186,7 +2186,7 @@ fts_optimize_create_deleted_doc_id_snapshot(
 
 	error = fts_eval_sql(optim->trx, graph);
 
-	fts_que_graph_free(graph);
+	que_graph_free(graph);
 
 	if (error != DB_SUCCESS) {
 		fts_sql_rollback(optim->trx);

storage/innobase/fts/fts0que.cc

@@ -1193,7 +1193,7 @@ fts_query_difference(
 			query->error = error;
 		}
 
-		fts_que_graph_free(graph);
+		que_graph_free(graph);
 	}
 
 	/* The size can't increase. */
@@ -1317,7 +1317,7 @@ fts_query_intersect(
 			query->error = error;
 		}
 
-		fts_que_graph_free(graph);
+		que_graph_free(graph);
 
 		if (query->error == DB_SUCCESS) {
 			/* Make the intesection (rb tree) the current doc id
@@ -1437,7 +1437,7 @@ fts_query_union(
 		query->error = error;
 	}
 
-	fts_que_graph_free(graph);
+	que_graph_free(graph);
 
 	if (query->error == DB_SUCCESS) {
 
@@ -2331,7 +2331,7 @@ fts_query_total_docs_containing_term(
 		}
 	}
 
-	fts_que_graph_free(graph);
+	que_graph_free(graph);
 
 	return(error);
 }
@@ -2413,7 +2413,7 @@ fts_query_terms_in_document(
 		}
 	}
 
-	fts_que_graph_free(graph);
+	que_graph_free(graph);
 
 	return(error);
 }
@@ -2504,7 +2504,7 @@ fts_query_is_in_proximity_range(
 
 	/* Free the prepared statement. */
 	if (get_doc.get_document_graph) {
-		fts_que_graph_free(get_doc.get_document_graph);
+		que_graph_free(get_doc.get_document_graph);
 		get_doc.get_document_graph = NULL;
 	}
 
@@ -2594,7 +2594,7 @@ fts_query_search_phrase(
 func_exit:
 	/* Free the prepared statement. */
 	if (get_doc.get_document_graph) {
-		fts_que_graph_free(get_doc.get_document_graph);
+		que_graph_free(get_doc.get_document_graph);
 		get_doc.get_document_graph = NULL;
 	}
 
@@ -2809,7 +2809,7 @@ fts_query_phrase_search(
 				query->error = error;
 			}
 
-			fts_que_graph_free(graph);
+			que_graph_free(graph);
 			graph = NULL;
 
 			fts_query_cache(query, token);
@@ -3786,7 +3786,7 @@ fts_query_free(
 {
 
 	if (query->read_nodes_graph) {
-		fts_que_graph_free(query->read_nodes_graph);
+		que_graph_free(query->read_nodes_graph);
 	}
 
 	if (query->root) {

storage/innobase/handler/i_s.cc

@@ -2964,7 +2964,7 @@ i_s_fts_index_table_fill_selected(
 		}
 	}
 
-	fts_que_graph_free(graph);
+	que_graph_free(graph);
 
 	trx->free();
 

storage/innobase/include/fts0fts.h

@@ -377,9 +377,6 @@ extern ulong		fts_min_token_size;
 need a sync to free some memory */
 extern bool		fts_need_sync;
 
-/** Free a query graph */
-void fts_que_graph_free(que_t *graph);
-
 /******************************************************************//**
 Create a FTS cache. */
 fts_cache_t*
@@ -740,16 +737,6 @@ FTS auxiliary INDEX table and clear the cache at the end.
 @return DB_SUCCESS on success, error code on failure. */
 dberr_t fts_sync_table(dict_table_t* table, bool wait = true);
 
-/****************************************************************//**
-Free the query graph but check whether dict_sys.latch is already
-held */
-void
-fts_que_graph_free_check_lock(
-/*==========================*/
-	fts_table_t*		fts_table,	/*!< in: FTS table */
-	const fts_index_cache_t*index_cache,	/*!< in: FTS index cache */
-	que_t*			graph);		/*!< in: query graph */
-
 /****************************************************************//**
 Create an FTS index cache. */
 CHARSET_INFO*

storage/innobase/pars/pars0sym.cc

@@ -67,8 +67,6 @@ sym_tab_free_private(
 	sym_node_t*	sym;
 	func_node_t*	func;
 
-	ut_ad(dict_sys.locked());
-
 	for (sym = UT_LIST_GET_FIRST(sym_tab->sym_list);
 	     sym != NULL;
 	     sym = UT_LIST_GET_NEXT(sym_list, sym)) {
@@ -76,8 +74,7 @@ sym_tab_free_private(
 		/* Close the tables opened in pars_retrieve_table_def(). */
 
 		if (sym->token_type == SYM_TABLE_REF_COUNTED) {
-
-			dict_table_close(sym->table, TRUE, FALSE);
+			sym->table->release();
 
 			sym->table = NULL;
 			sym->resolved = FALSE;

storage/innobase/row/row0ftsort.cc

@@ -1637,7 +1637,7 @@ row_fts_merge_insert(
 	aux_table = dict_table_open_on_name(aux_table_name, FALSE, FALSE,
 					    DICT_ERR_IGNORE_NONE);
 	ut_ad(aux_table != NULL);
-	dict_table_close(aux_table, FALSE, FALSE);
+	aux_table->release();
 	aux_index = dict_table_get_first_index(aux_table);
 
 	ut_ad(!aux_index->is_instant());