From f60777823dbef332427f842f3acd7bbd49b0e55f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Fri, 17 May 2019 10:29:00 +0300 Subject: [PATCH] MDEV-19505 WIP: Do not require mutex in que_graph_free() FIXME: sym_tab_free_private() is calling dict_table_close(). --- storage/innobase/fts/fts0config.cc | 6 +- storage/innobase/fts/fts0fts.cc | 139 +++++++++-------------------- storage/innobase/fts/fts0opt.cc | 20 ++--- storage/innobase/fts/fts0que.cc | 18 ++-- storage/innobase/handler/i_s.cc | 2 - storage/innobase/include/fts0fts.h | 17 ---- storage/innobase/que/que0que.cc | 8 -- storage/innobase/row/row0mysql.cc | 7 +- 8 files changed, 62 insertions(+), 155 deletions(-) diff --git a/storage/innobase/fts/fts0config.cc b/storage/innobase/fts/fts0config.cc index 6b6042dee66..dbd2ce47491 100644 --- a/storage/innobase/fts/fts0config.cc +++ b/storage/innobase/fts/fts0config.cc @@ -120,9 +120,7 @@ fts_config_get_value( error = fts_eval_sql(trx, graph); - mutex_enter(&dict_sys->mutex); que_graph_free(graph); - mutex_exit(&dict_sys->mutex); return(error); } @@ -230,7 +228,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; @@ -256,7 +254,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); diff --git a/storage/innobase/fts/fts0fts.cc b/storage/innobase/fts/fts0fts.cc index 128518d7433..aaabbde177c 100644 --- a/storage/innobase/fts/fts0fts.cc +++ b/storage/innobase/fts/fts0fts.cc @@ -462,6 +462,23 @@ fts_load_user_stopword( name */ fts_stopword_t* stopword_info) /*!< in: Stopword info */ { + /* Validate the user table existence and in the right + format */ + stopword_info->charset = fts_valid_stopword_table(stopword_table_name); + + if (!stopword_info->charset) { + return FALSE; + } + + if (!stopword_info->cached_stopword) { + /* Create the stopword RB tree with the stopword column + charset. All comparison will use this charset */ + stopword_info->cached_stopword = rbt_create_arg_cmp( + sizeof(fts_tokenizer_word_t), innobase_fts_text_cmp, + (void*)stopword_info->charset); + + } + pars_info_t* info; que_t* graph; dberr_t error = DB_SUCCESS; @@ -476,21 +493,6 @@ fts_load_user_stopword( mutex_enter(&dict_sys->mutex); } - /* Validate the user table existence and in the right - format */ - stopword_info->charset = fts_valid_stopword_table(stopword_table_name); - if (!stopword_info->charset) { - ret = FALSE; - goto cleanup; - } else if (!stopword_info->cached_stopword) { - /* Create the stopword RB tree with the stopword column - charset. All comparison will use this charset */ - stopword_info->cached_stopword = rbt_create_arg_cmp( - sizeof(fts_tokenizer_word_t), innobase_fts_text_cmp, - (void*)stopword_info->charset); - - } - info = pars_info_create(); pars_info_bind_id(info, TRUE, "table_stopword", stopword_table_name); @@ -541,13 +543,12 @@ fts_load_user_stopword( } } - que_graph_free(graph); - -cleanup: if (!has_lock) { mutex_exit(&dict_sys->mutex); } + que_graph_free(graph); + trx_free(trx); return(ret); } @@ -915,43 +916,6 @@ fts_drop_index( return(err); } -/****************************************************************//** -Free the query graph but check whether dict_sys->mutex 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 */ -{ - ibool 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; - } else if (index_cache) { - ut_ad(index_cache->index->table->fts); - - has_dict = index_cache->index->table->fts->fts_status - & TABLE_DICT_LOCKED; - } - - if (!has_dict) { - mutex_enter(&dict_sys->mutex); - } - - ut_ad(mutex_own(&dict_sys->mutex)); - - que_graph_free(graph); - - if (!has_dict) { - mutex_exit(&dict_sys->mutex); - } -} - /****************************************************************//** Create an FTS index cache. */ CHARSET_INFO* @@ -1083,7 +1047,6 @@ fts_cache_clear( ulint i; for (i = 0; i < ib_vector_size(cache->indexes); ++i) { - ulint j; fts_index_cache_t* index_cache; index_cache = static_cast( @@ -1095,24 +1058,15 @@ fts_cache_clear( index_cache->words = NULL; - for (j = 0; j < FTS_NUM_AUX_INDEX; ++j) { - - if (index_cache->ins_graph[j] != NULL) { - - fts_que_graph_free_check_lock( - NULL, index_cache, - index_cache->ins_graph[j]); - - index_cache->ins_graph[j] = NULL; + for (ulint j = 0; j < FTS_NUM_AUX_INDEX; ++j) { + if (que_t*& graph = index_cache->ins_graph[j]) { + que_graph_free(graph); + graph = NULL; } - if (index_cache->sel_graph[j] != NULL) { - - fts_que_graph_free_check_lock( - NULL, index_cache, - index_cache->sel_graph[j]); - - index_cache->sel_graph[j] = NULL; + if (que_t*& graph = index_cache->sel_graph[j]) { + que_graph_free(graph); + graph = NULL; } } @@ -2698,7 +2652,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) { @@ -2815,7 +2769,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 (error == DB_SUCCESS) { @@ -2972,7 +2926,7 @@ fts_delete( error = fts_eval_sql(trx, graph); - fts_que_graph_free(graph); + que_graph_free(graph); } else { pars_info_free(info); } @@ -3842,7 +3796,7 @@ fts_doc_fetch_by_doc_id( trx_free(trx); if (!get_doc) { - fts_que_graph_free(graph); + que_graph_free(graph); } return(error); @@ -3971,7 +3925,7 @@ fts_sync_add_deleted_cache( error = fts_eval_sql(sync->trx, graph); } - fts_que_graph_free(graph); + que_graph_free(graph); return(error); } @@ -4254,7 +4208,6 @@ fts_sync_rollback( fts_cache_t* cache = sync->table->fts->cache; for (ulint i = 0; i < ib_vector_size(cache->indexes); ++i) { - ulint j; fts_index_cache_t* index_cache; index_cache = static_cast( @@ -4264,24 +4217,14 @@ fts_sync_rollback( in the next sync, see fts_sync_write_words(). */ fts_sync_index_reset(index_cache); - for (j = 0; fts_index_selector[j].value; ++j) { - - if (index_cache->ins_graph[j] != NULL) { - - fts_que_graph_free_check_lock( - NULL, index_cache, - index_cache->ins_graph[j]); - - index_cache->ins_graph[j] = NULL; + for (ulint j = 0; j < FTS_NUM_AUX_INDEX; ++j) { + if (que_t*& graph = index_cache->ins_graph[j]) { + que_graph_free(graph); + graph = NULL; } - - if (index_cache->sel_graph[j] != NULL) { - - fts_que_graph_free_check_lock( - NULL, index_cache, - index_cache->sel_graph[j]); - - index_cache->sel_graph[j] = NULL; + if (que_t*& graph = index_cache->sel_graph[j]) { + que_graph_free(graph); + graph = NULL; } } } @@ -4871,7 +4814,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; } } @@ -5018,7 +4961,7 @@ fts_get_rows_count( } } - fts_que_graph_free(graph); + que_graph_free(graph); trx_free(trx); @@ -5118,7 +5061,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 */ diff --git a/storage/innobase/fts/fts0opt.cc b/storage/innobase/fts/fts0opt.cc index f91ebcbf2a6..59f42958a2f 100644 --- a/storage/innobase/fts/fts0opt.cc +++ b/storage/innobase/fts/fts0opt.cc @@ -894,7 +894,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) { @@ -1009,9 +1009,7 @@ fts_table_fetch_doc_ids( error = fts_eval_sql(trx, graph); fts_sql_commit(trx); - mutex_enter(&dict_sys->mutex); que_graph_free(graph); - mutex_exit(&dict_sys->mutex); if (error == DB_SUCCESS) { ib_vector_sort(doc_ids->doc_ids, fts_update_doc_id_cmp); @@ -1476,7 +1474,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, @@ -1508,7 +1506,7 @@ fts_optimize_write_word( } if (graph != NULL) { - fts_que_graph_free(graph); + que_graph_free(graph); } return(error); @@ -1832,8 +1830,8 @@ fts_optimize_words( != fts_select_index( charset, word->f_str, word->f_len) - && graph) { - fts_que_graph_free(graph); + && graph) { + que_graph_free(graph); graph = NULL; } } @@ -1852,7 +1850,7 @@ fts_optimize_words( } if (graph != NULL) { - fts_que_graph_free(graph); + que_graph_free(graph); } } @@ -2086,7 +2084,7 @@ fts_optimize_purge_deleted_doc_ids( } } - fts_que_graph_free(graph); + que_graph_free(graph); return(error); } @@ -2124,7 +2122,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); } @@ -2193,7 +2191,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); diff --git a/storage/innobase/fts/fts0que.cc b/storage/innobase/fts/fts0que.cc index bf7e262ac98..4e5f45c4c4d 100644 --- a/storage/innobase/fts/fts0que.cc +++ b/storage/innobase/fts/fts0que.cc @@ -1190,7 +1190,7 @@ fts_query_difference( query->error = error; } - fts_que_graph_free(graph); + que_graph_free(graph); } /* The size can't increase. */ @@ -1314,7 +1314,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 @@ -1434,7 +1434,7 @@ fts_query_union( query->error = error; } - fts_que_graph_free(graph); + que_graph_free(graph); if (query->error == DB_SUCCESS) { @@ -2328,7 +2328,7 @@ fts_query_total_docs_containing_term( } } - fts_que_graph_free(graph); + que_graph_free(graph); return(error); } @@ -2410,7 +2410,7 @@ fts_query_terms_in_document( } } - fts_que_graph_free(graph); + que_graph_free(graph); return(error); } @@ -2502,7 +2502,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; } @@ -2592,7 +2592,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; } @@ -2807,7 +2807,7 @@ fts_query_phrase_search( query->error = error; } - fts_que_graph_free(graph); + que_graph_free(graph); graph = NULL; fts_query_cache(query, token); @@ -3780,7 +3780,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) { diff --git a/storage/innobase/handler/i_s.cc b/storage/innobase/handler/i_s.cc index 4016b11387e..e2978d7ee4d 100644 --- a/storage/innobase/handler/i_s.cc +++ b/storage/innobase/handler/i_s.cc @@ -3474,9 +3474,7 @@ i_s_fts_index_table_fill_selected( } } - mutex_enter(&dict_sys->mutex); que_graph_free(graph); - mutex_exit(&dict_sys->mutex); trx_free(trx); diff --git a/storage/innobase/include/fts0fts.h b/storage/innobase/include/fts0fts.h index 9a10375759c..001e8730d1b 100644 --- a/storage/innobase/include/fts0fts.h +++ b/storage/innobase/include/fts0fts.h @@ -407,13 +407,6 @@ content through information schema table */ extern char* fts_internal_tbl_name; extern char* fts_internal_tbl_name2; -#define fts_que_graph_free(graph) \ -do { \ - mutex_enter(&dict_sys->mutex); \ - que_graph_free(graph); \ - mutex_exit(&dict_sys->mutex); \ -} while (0) - /******************************************************************//** Create a FTS cache. */ fts_cache_t* @@ -803,16 +796,6 @@ fts_sync_table( bool wait, bool has_dict); -/****************************************************************//** -Free the query graph but check whether dict_sys->mutex 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* diff --git a/storage/innobase/que/que0que.cc b/storage/innobase/que/que0que.cc index ceb48c4b298..e77ef5be476 100644 --- a/storage/innobase/que/que0que.cc +++ b/storage/innobase/que/que0que.cc @@ -1220,15 +1220,7 @@ que_eval_sql( que_run_threads(thr); - if (reserve_dict_mutex) { - mutex_enter(&dict_sys->mutex); - } - que_graph_free(graph); - if (reserve_dict_mutex) { - mutex_exit(&dict_sys->mutex); - } - DBUG_RETURN(trx->error_state); } diff --git a/storage/innobase/row/row0mysql.cc b/storage/innobase/row/row0mysql.cc index afbcdfe4423..aee659aad4a 100644 --- a/storage/innobase/row/row0mysql.cc +++ b/storage/innobase/row/row0mysql.cc @@ -3249,12 +3249,7 @@ row_drop_ancillary_fts_tables( the cluster index is being rebuilt. Such table might not have DICT_TF2_FTS flag set. So keep this out of above dict_table_has_fts_index condition */ - if (table->fts != NULL) { - /* 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; - + if (table->fts) { fts_free(table); } -- 2.20.1