Description:
When MySQL performs background automatic statistics updates, it will acquire dict_table_stats_lock(RW_X_LATCH) and call dict_stats_empty_index() to clear the statistics.
The call chain is:
dict_stats_update() → dict_stats_update_persistent() → dict_table_stats_lock(RW_X_LATCH) → dict_stats_analyze_index() → dict_stats_analyze_index_low() → dict_stats_empty_index() → dict_stats_save().
However, I found that when MySQL executes a query and retrieves the table statistics, the passed-in flag is HA_STATUS_NO_LOCK. This means it will not call dict_table_stats_lock(ib_table, RW_S_LATCH).
The call chain in this case is:
TABLE_LIST::fetch_number_of_rows() → table->file->info(HA_STATUS_VARIABLE | HA_STATUS_NO_LOCK) → ha_innobase::info(uint flag) → ha_innobase::info_low → ...
Therefore, it is possible that while the background thread is updating the table statistics, and happens to be at the point where dict_stats_empty_index() has just cleared the statistics, a query on the same table is also being executed. In this case, the query might retrieve empty statistics, which could result in the optimizer choosing a wrong execution plan, causing a query that normally runs fine to become much slower.
How to repeat:
/** Returns statistics information of the table to the MySQL interpreter, in
various fields of the handle object.
@param[in] flag what information is requested
@param[in] is_analyze True if called from "::analyze()".
@return HA_ERR_* error code or 0 */
int ha_innobase::info_low(uint flag, bool is_analyze) {
dict_table_t *ib_table;
ib_uint64_t n_rows;
DBUG_TRACE;
DEBUG_SYNC_C("ha_innobase_info_low");
/* If we are forcing recovery at a high level, we will suppress
statistics calculation on tables, because that may crash the
server if an index is badly corrupted. */
/* We do not know if MySQL can call this function before calling
external_lock(). To be safe, update the thd of the current table
handle. */
update_thd(ha_thd());
m_prebuilt->trx->op_info = (char *)"returning various info to MySQL";
ib_table = m_prebuilt->table;
assert(ib_table->n_ref_count > 0);
if (flag & HA_STATUS_TIME) {
if (is_analyze || innobase_stats_on_metadata) {
dict_stats_upd_option_t opt;
dberr_t ret;
m_prebuilt->trx->op_info = "updating table statistics";
if (dict_stats_is_persistent_enabled(ib_table)) {
if (is_analyze) {
opt = DICT_STATS_RECALC_PERSISTENT;
} else {
/* This is e.g. 'SHOW INDEXES', fetch
the persistent stats from disk. */
opt = DICT_STATS_FETCH_ONLY_IF_NOT_IN_MEMORY;
}
} else {
opt = DICT_STATS_RECALC_TRANSIENT;
}
ut_ad(!mutex_own(&dict_sys->mutex));
ret = dict_stats_update(ib_table, opt);
if (ret != DB_SUCCESS) {
m_prebuilt->trx->op_info = "";
return HA_ERR_GENERIC;
}
m_prebuilt->trx->op_info = "returning various info to MySQL";
}
stats.update_time = (ulong)ib_table->update_time;
}
if (flag & HA_STATUS_VARIABLE) {
ulint stat_clustered_index_size;
ulint stat_sum_of_other_index_sizes;
if (!(flag & HA_STATUS_NO_LOCK)) { ## here flag is HA_STATUS_VARIABLE | HA_STATUS_NO_LOCK, so dict_table_stats_lock(ib_table, RW_S_LATCH) is skipped.
dict_table_stats_lock(ib_table, RW_S_LATCH);
}
ut_a(ib_table->stat_initialized);
n_rows = ib_table->stat_n_rows;
stat_clustered_index_size = ib_table->stat_clustered_index_size;
stat_sum_of_other_index_sizes = ib_table->stat_sum_of_other_index_sizes;
if (!(flag & HA_STATUS_NO_LOCK)) {
dict_table_stats_unlock(ib_table, RW_S_LATCH);
}
Suggested fix:
When retrieving statistics, is it necessary to acquire dict_table_stats_lock(ib_table, RW_S_LATCH), or is there a better approach?
Description: When MySQL performs background automatic statistics updates, it will acquire dict_table_stats_lock(RW_X_LATCH) and call dict_stats_empty_index() to clear the statistics. The call chain is: dict_stats_update() → dict_stats_update_persistent() → dict_table_stats_lock(RW_X_LATCH) → dict_stats_analyze_index() → dict_stats_analyze_index_low() → dict_stats_empty_index() → dict_stats_save(). However, I found that when MySQL executes a query and retrieves the table statistics, the passed-in flag is HA_STATUS_NO_LOCK. This means it will not call dict_table_stats_lock(ib_table, RW_S_LATCH). The call chain in this case is: TABLE_LIST::fetch_number_of_rows() → table->file->info(HA_STATUS_VARIABLE | HA_STATUS_NO_LOCK) → ha_innobase::info(uint flag) → ha_innobase::info_low → ... Therefore, it is possible that while the background thread is updating the table statistics, and happens to be at the point where dict_stats_empty_index() has just cleared the statistics, a query on the same table is also being executed. In this case, the query might retrieve empty statistics, which could result in the optimizer choosing a wrong execution plan, causing a query that normally runs fine to become much slower. How to repeat: /** Returns statistics information of the table to the MySQL interpreter, in various fields of the handle object. @param[in] flag what information is requested @param[in] is_analyze True if called from "::analyze()". @return HA_ERR_* error code or 0 */ int ha_innobase::info_low(uint flag, bool is_analyze) { dict_table_t *ib_table; ib_uint64_t n_rows; DBUG_TRACE; DEBUG_SYNC_C("ha_innobase_info_low"); /* If we are forcing recovery at a high level, we will suppress statistics calculation on tables, because that may crash the server if an index is badly corrupted. */ /* We do not know if MySQL can call this function before calling external_lock(). To be safe, update the thd of the current table handle. */ update_thd(ha_thd()); m_prebuilt->trx->op_info = (char *)"returning various info to MySQL"; ib_table = m_prebuilt->table; assert(ib_table->n_ref_count > 0); if (flag & HA_STATUS_TIME) { if (is_analyze || innobase_stats_on_metadata) { dict_stats_upd_option_t opt; dberr_t ret; m_prebuilt->trx->op_info = "updating table statistics"; if (dict_stats_is_persistent_enabled(ib_table)) { if (is_analyze) { opt = DICT_STATS_RECALC_PERSISTENT; } else { /* This is e.g. 'SHOW INDEXES', fetch the persistent stats from disk. */ opt = DICT_STATS_FETCH_ONLY_IF_NOT_IN_MEMORY; } } else { opt = DICT_STATS_RECALC_TRANSIENT; } ut_ad(!mutex_own(&dict_sys->mutex)); ret = dict_stats_update(ib_table, opt); if (ret != DB_SUCCESS) { m_prebuilt->trx->op_info = ""; return HA_ERR_GENERIC; } m_prebuilt->trx->op_info = "returning various info to MySQL"; } stats.update_time = (ulong)ib_table->update_time; } if (flag & HA_STATUS_VARIABLE) { ulint stat_clustered_index_size; ulint stat_sum_of_other_index_sizes; if (!(flag & HA_STATUS_NO_LOCK)) { ## here flag is HA_STATUS_VARIABLE | HA_STATUS_NO_LOCK, so dict_table_stats_lock(ib_table, RW_S_LATCH) is skipped. dict_table_stats_lock(ib_table, RW_S_LATCH); } ut_a(ib_table->stat_initialized); n_rows = ib_table->stat_n_rows; stat_clustered_index_size = ib_table->stat_clustered_index_size; stat_sum_of_other_index_sizes = ib_table->stat_sum_of_other_index_sizes; if (!(flag & HA_STATUS_NO_LOCK)) { dict_table_stats_unlock(ib_table, RW_S_LATCH); } Suggested fix: When retrieving statistics, is it necessary to acquire dict_table_stats_lock(ib_table, RW_S_LATCH), or is there a better approach?