Skip to content

Commit

Permalink
WIP MDEV-35000: dict_table_close() is a performance hog
Browse files Browse the repository at this point in the history 
dict_table_close(table): Replaced with table->release().

dict_table_close(table, thd, mdl): Remove the parameter "dict_locked".
Do not try to invalidate the statistics.

FIXME: innodb.innodb_stats_fetch, innodb.innodb_stats_auto_recalc_on_nonexistent
  • Loading branch information
@dr-m
dr-m committed Jan 3, 2025
1 parent 4263181 commit b83054b
Show file tree
Hide file tree
Showing 17 changed files with 95 additions and 158 deletions.
8 changes: 4 additions & 4 deletions mysql-test/suite/innodb/r/innodb_stats_auto_recalc_on_nonexistent.result
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ SELECT COUNT(*) FROM mysql.innodb_index_stats WHERE table_name = 't';
COUNT(*) 0
SELECT * FROM t;
SELECT COUNT(*) FROM mysql.innodb_table_stats WHERE table_name = 't';
COUNT(*) 1
COUNT(*) 0
SELECT COUNT(*) FROM mysql.innodb_index_stats WHERE table_name = 't';
COUNT(*) 3
COUNT(*) 0
DROP TABLE t;
Test with explicit enable
CREATE TABLE t (a INT, PRIMARY KEY (a)) ENGINE=INNODB STATS_AUTO_RECALC=1;
Expand All @@ -34,9 +34,9 @@ SELECT COUNT(*) FROM mysql.innodb_index_stats WHERE table_name = 't';
COUNT(*) 0
SELECT * FROM t;
SELECT COUNT(*) FROM mysql.innodb_table_stats WHERE table_name = 't';
COUNT(*) 1
COUNT(*) 0
SELECT COUNT(*) FROM mysql.innodb_index_stats WHERE table_name = 't';
COUNT(*) 3
COUNT(*) 0
DROP TABLE t;
Test with explicit disable
CREATE TABLE t (a INT, PRIMARY KEY (a)) ENGINE=INNODB STATS_AUTO_RECALC=0;
Expand Down
12 changes: 6 additions & 6 deletions mysql-test/suite/innodb/r/innodb_stats_fetch.result
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,21 +131,21 @@ FROM information_schema.statistics WHERE table_name = 'test_ps_fetch'
ORDER BY index_name, seq_in_index;
seq_in_index 1
column_name c
cardinality 6
cardinality 0
seq_in_index 2
column_name d
cardinality 22
cardinality 0
seq_in_index 1
column_name a
cardinality 40
cardinality 0
seq_in_index 2
column_name b
cardinality 200
cardinality 0
SELECT
table_rows, avg_row_length, max_data_length, index_length
FROM information_schema.tables WHERE table_name = 'test_ps_fetch';
table_rows 1000
avg_row_length 81
table_rows 0
avg_row_length 0
max_data_length 0
index_length 16384
DROP TABLE test_ps_fetch;
Expand Down
18 changes: 9 additions & 9 deletions storage/innobase/dict/dict0defrag_bg.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ static void dict_stats_process_entry_from_defrag_pool(THD *thd)
? dict_table_find_index_on_id(table, index_id) : nullptr)
if (index->is_btree())
dict_stats_save_defrag_stats(index);
dict_table_close(table, false, thd, mdl);
dict_table_close(table, thd, mdl);
}
}

Expand Down Expand Up @@ -230,7 +230,7 @@ dberr_t dict_stats_save_defrag_summary(dict_index_t *index, THD *thd)
{
release_and_exit:
if (table_stats)
dict_table_close(table_stats, false, thd, mdl_table);
dict_table_close(table_stats, thd, mdl_table);
return DB_STATS_DO_NOT_EXIST;
}

Expand All @@ -246,7 +246,7 @@ dberr_t dict_stats_save_defrag_summary(dict_index_t *index, THD *thd)
goto release_and_exit;
if (strcmp(index_stats->name.m_name, INDEX_STATS_NAME))
{
dict_table_close(index_stats, false, thd, mdl_index);
dict_table_close(index_stats, thd, mdl_index);
goto release_and_exit;
}

Expand All @@ -272,9 +272,9 @@ dberr_t dict_stats_save_defrag_summary(dict_index_t *index, THD *thd)
trx->rollback();

if (table_stats)
dict_table_close(table_stats, true, thd, mdl_table);
dict_table_close(table_stats, thd, mdl_table);
if (index_stats)
dict_table_close(index_stats, true, thd, mdl_index);
dict_table_close(index_stats, thd, mdl_index);

row_mysql_unlock_data_dictionary(trx);
trx->free();
Expand Down Expand Up @@ -367,7 +367,7 @@ dict_stats_save_defrag_stats(
{
release_and_exit:
if (table_stats)
dict_table_close(table_stats, false, thd, mdl_table);
dict_table_close(table_stats, thd, mdl_table);
return DB_STATS_DO_NOT_EXIST;
}

Expand All @@ -384,7 +384,7 @@ dict_stats_save_defrag_stats(

if (strcmp(index_stats->name.m_name, INDEX_STATS_NAME))
{
dict_table_close(index_stats, false, thd, mdl_index);
dict_table_close(index_stats, thd, mdl_index);
goto release_and_exit;
}

Expand Down Expand Up @@ -424,9 +424,9 @@ dict_stats_save_defrag_stats(
trx->rollback();

if (table_stats)
dict_table_close(table_stats, true, thd, mdl_table);
dict_table_close(table_stats, thd, mdl_table);
if (index_stats)
dict_table_close(index_stats, true, thd, mdl_index);
dict_table_close(index_stats, thd, mdl_index);
row_mysql_unlock_data_dictionary(trx);
trx->free();

Expand Down
72 changes: 8 additions & 64 deletions storage/innobase/dict/dict0dict.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,70 +195,6 @@ dict_tables_have_same_db(
return(FALSE);
}

/** Decrement the count of open handles */
void dict_table_close(dict_table_t *table)
{
if (table->get_ref_count() == 1 && table->stats_is_persistent() &&
strchr(table->name.m_name, '/'))
{
/* It looks like we are closing the last handle. The user could
have executed FLUSH TABLES in order to have the statistics reloaded
from the InnoDB persistent statistics tables. We must acquire
exclusive dict_sys.latch to prevent a race condition with another
thread concurrently acquiring a handle on the table. */
dict_sys.lock(SRW_LOCK_CALL);
if (table->release())
{
table->stats_mutex_lock();
if (table->get_ref_count() == 0)
dict_stats_deinit(table);
table->stats_mutex_unlock();
}
dict_sys.unlock();
}
else
table->release();
}

/** Decrements the count of open handles of a table.
@param[in,out] table table
@param[in] dict_locked whether dict_sys.latch is being held
@param[in] thd thread to release MDL
@param[in] mdl metadata lock or NULL if the thread
is a foreground one. */
void
dict_table_close(
dict_table_t* table,
bool dict_locked,
THD* thd,
MDL_ticket* mdl)
{
if (!dict_locked)
dict_table_close(table);
else
{
if (table->release() && table->stats_is_persistent() &&
strchr(table->name.m_name, '/'))
{
/* Force persistent stats re-read upon next open of the table so
that FLUSH TABLE can be used to forcibly fetch stats from disk if
they have been manually modified. */
table->stats_mutex_lock();
if (table->get_ref_count() == 0)
dict_stats_deinit(table);
table->stats_mutex_unlock();
}

ut_ad(dict_lru_validate());
ut_ad(dict_sys.find(table));
}

if (!thd || !mdl);
else if (MDL_context *mdl_context= static_cast<MDL_context*>
(thd_mdl_context(thd)))
mdl_context->release_lock(mdl);
}

/** Check if the table has a given (non_virtual) column.
@param[in] table table object
@param[in] col_name column name
Expand Down Expand Up @@ -585,6 +521,14 @@ dict_index_get_nth_field_pos(
return(ULINT_UNDEFINED);
}

void mdl_release(THD *thd, MDL_ticket *mdl) noexcept
{
if (!thd || !mdl);
else if (MDL_context *mdl_context= static_cast<MDL_context*>
(thd_mdl_context(thd)))
mdl_context->release_lock(mdl);
}

/** Parse the table file name into table name and database name.
@tparam dict_frozen whether the caller holds dict_sys.latch
@param[in,out] db_name database name buffer
Expand Down
16 changes: 8 additions & 8 deletions storage/innobase/dict/dict0stats.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2928,7 +2928,7 @@ dict_stats_save(
|| strcmp(table_stats->name.m_name, TABLE_STATS_NAME)) {
release_and_exit:
if (table_stats) {
dict_table_close(table_stats, false, thd, mdl_table);
dict_table_close(table_stats, thd, mdl_table);
}
return DB_STATS_DO_NOT_EXIST;
}
Expand All @@ -2945,7 +2945,7 @@ dict_stats_save(
goto release_and_exit;
}
if (strcmp(index_stats->name.m_name, INDEX_STATS_NAME)) {
dict_table_close(index_stats, false, thd, mdl_index);
dict_table_close(index_stats, thd, mdl_index);
goto release_and_exit;
}

Expand Down Expand Up @@ -3014,8 +3014,8 @@ dict_stats_save(
dict_sys.unlock();
unlocked_free_and_exit:
trx->free();
dict_table_close(table_stats, false, thd, mdl_table);
dict_table_close(index_stats, false, thd, mdl_index);
dict_table_close(table_stats, thd, mdl_table);
dict_table_close(index_stats, thd, mdl_index);
return ret;
}

Expand Down Expand Up @@ -3499,7 +3499,7 @@ dict_stats_fetch_from_ps(
|| strcmp(table_stats->name.m_name, TABLE_STATS_NAME)) {
release_and_exit:
if (table_stats) {
dict_table_close(table_stats, false, thd, mdl_table);
dict_table_close(table_stats, thd, mdl_table);
}
return DB_STATS_DO_NOT_EXIST;
}
Expand All @@ -3516,7 +3516,7 @@ dict_stats_fetch_from_ps(
goto release_and_exit;
}
if (strcmp(index_stats->name.m_name, INDEX_STATS_NAME)) {
dict_table_close(index_stats, false, thd, mdl_index);
dict_table_close(index_stats, thd, mdl_index);
goto release_and_exit;
}

Expand Down Expand Up @@ -3606,8 +3606,8 @@ dict_stats_fetch_from_ps(
/* pinfo is freed by que_eval_sql() */
dict_sys.unlock();

dict_table_close(table_stats, false, thd, mdl_table);
dict_table_close(index_stats, false, thd, mdl_index);
dict_table_close(table_stats, thd, mdl_table);
dict_table_close(index_stats, thd, mdl_index);

trx_commit_for_mysql(trx);

Expand Down
4 changes: 2 additions & 2 deletions storage/innobase/dict/dict0stats_bg.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ static bool dict_stats_process_entry_from_recalc_pool(THD *thd)

if (!mdl || !table->is_accessible())
{
dict_table_close(table, false, thd, mdl);
dict_table_close(table, thd, mdl);
goto invalid_table_id;
}

Expand All @@ -346,7 +346,7 @@ static bool dict_stats_process_entry_from_recalc_pool(THD *thd)
? dict_stats_update(table, DICT_STATS_RECALC_PERSISTENT)
: DB_SUCCESS_LOCKED_REC;

dict_table_close(table, false, thd, mdl);
dict_table_close(table, thd, mdl);

mysql_mutex_lock(&recalc_pool_mutex);
auto i= std::find_if(recalc_pool.begin(), recalc_pool.end(),
Expand Down
2 changes: 1 addition & 1 deletion storage/innobase/fts/fts0opt.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2809,7 +2809,7 @@ static void fts_optimize_sync_table(dict_table_t *table,
std::this_thread::sleep_for(std::chrono::seconds(6)););

if (mdl_ticket)
dict_table_close(sync_table, false, fts_opt_thd, mdl_ticket);
dict_table_close(sync_table, fts_opt_thd, mdl_ticket);
}

/**********************************************************************//**
Expand Down
Loading

0 comments on commit b83054b

Please sign in to comment.