Skip to content

Commit

Permalink
MDEV-19505 Do not hold mutex while calling que_graph_free()
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@dr-m
dr-m committed Aug 31, 2021
1 parent 82b7c56 commit 6a2cd6f
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 103 deletions.
6 changes: 3 additions & 3 deletions storage/innobase/fts/fts0config.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;

Expand All @@ -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);
Expand Down
75 changes: 12 additions & 63 deletions storage/innobase/fts/fts0fts.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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*
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -4892,7 +4841,7 @@ fts_get_rows_count(
}
}

fts_que_graph_free(graph);
que_graph_free(graph);

trx->free();

Expand Down Expand Up @@ -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 */
Expand Down
18 changes: 9 additions & 9 deletions storage/innobase/fts/fts0opt.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1491,7 +1491,7 @@ fts_optimize_write_word(
}

if (graph != NULL) {
fts_que_graph_free(graph);
que_graph_free(graph);
}

return(error);
Expand Down Expand Up @@ -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;
}
}
Expand All @@ -1848,7 +1848,7 @@ fts_optimize_words(
}

if (graph != NULL) {
fts_que_graph_free(graph);
que_graph_free(graph);
}
}

Expand Down Expand Up @@ -2081,7 +2081,7 @@ fts_optimize_purge_deleted_doc_ids(
}
}

fts_que_graph_free(graph);
que_graph_free(graph);

return(error);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down
18 changes: 9 additions & 9 deletions storage/innobase/fts/fts0que.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1193,7 +1193,7 @@ fts_query_difference(
query->error = error;
}

fts_que_graph_free(graph);
que_graph_free(graph);
}

/* The size can't increase. */
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1437,7 +1437,7 @@ fts_query_union(
query->error = error;
}

fts_que_graph_free(graph);
que_graph_free(graph);

if (query->error == DB_SUCCESS) {

Expand Down Expand Up @@ -2331,7 +2331,7 @@ fts_query_total_docs_containing_term(
}
}

fts_que_graph_free(graph);
que_graph_free(graph);

return(error);
}
Expand Down Expand Up @@ -2413,7 +2413,7 @@ fts_query_terms_in_document(
}
}

fts_que_graph_free(graph);
que_graph_free(graph);

return(error);
}
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion storage/innobase/handler/i_s.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2964,7 +2964,7 @@ i_s_fts_index_table_fill_selected(
}
}

fts_que_graph_free(graph);
que_graph_free(graph);

trx->free();

Expand Down
13 changes: 0 additions & 13 deletions storage/innobase/include/fts0fts.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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*
Expand Down Expand Up @@ -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*
Expand Down
Loading

0 comments on commit 6a2cd6f

Please sign in to comment.