Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MDEV-35000 dict_table_close() is a performance hog #3729

Draft
wants to merge 2 commits into
base: 10.6
Choose a base branch
from
Draft

Conversation

dr-m
Copy link
Contributor

@dr-m dr-m commented Jan 3, 2025

  • The Jira issue number for this PR is: MDEV-35000

Description

Every time dict_table_close() is decrementing the table reference count to 0, it is performing some expensive logic that aims to ensure that during FLUSH TABLES, the InnoDB persistent statistics will be deinitialized, so that during a subsequent ha_innobase::open() the statistics will be reloaded. The purpose of this is to ensure that FLUSH TABLES will make any changes to mysql.innodb_table_stats and mysql.innodb_index_stats accessible. This is an overkill, because FLUSH TABLES is executed very rarely.

There is a better way to do this: Keep track of FLUSH TABLES only, by overloading Handler_share::~Handler_share().

This also includes some preparation to simplify the initialization and clearing of the statistics:

innodb_stats_transient_sample_pages, innodb_stats_persistent_sample_pages: Change the type to UNSIGNED, because the number of pages in a table is limited to 32 bits by the InnoDB file formats.

btr_get_size_and_reserved(), fseg_get_n_frag_pages(), fseg_n_reserved_pages_low(), fseg_n_reserved_pages(): Return uint32_t. The file format limits page numbers to 32 bits.

dict_table_t::stat: An Atomic_relaxed<uint32_t> that combines a number of metadata fields.

innodb_copy_stat_flags(): Copy the statistics flags from TABLE_SHARE or HA_CREATE_INFO.

dict_table_t::stats_initialized(), dict_table_t::stats_is_persistent(): Accessors to dict_table_t::stat.

Release Notes

innodb_stats_transient_sample_pages, innodb_stats_persistent_sample_pages: Change the type to BIGINT UNSIGNED to UNSIGNED, because the number of pages in a table is limited to 32 bits by the InnoDB file formats.

How can this PR be tested?

This code is rather well covered by the regression test suite.

The FLUSH TABLES functionality with respect to InnoDB persistent staitistics is covered by the following tests:

  • innodb.innodb_stats_auto_recalc_on_nonexistent
  • innodb.innodb_stats_fetch

Basing the PR against the correct MariaDB version

  • This is a new feature or a refactoring, and the PR is based against the main branch.
  • This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.

This is a performance bug fix. The way how the statistics are initialized and updated have been largely rewritten in MariaDB Server 10.6. It is not trivial to apply these changes to 10.5, which is fairly close to its End Of Life anyway.

PR quality check

  • I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
  • For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

innodb_stats_transient_sample_pages, innodb_stats_persistent_sample_pages:
Change the type to UNSIGNED, because the number of pages in a table
is limited to 32 bits by the InnoDB file formats.

btr_get_size_and_reserved(), fseg_get_n_frag_pages(),
fseg_n_reserved_pages_low(), fseg_n_reserved_pages(): Return uint32_t.
The file format limits page numbers to 32 bits.

dict_table_t::stat: An Atomic_relaxed<uint32_t> that combines a
number of metadata fields.

innodb_copy_stat_flags(): Copy the statistics flags from
TABLE_SHARE or HA_CREATE_INFO.

dict_table_t::stats_initialized(), dict_table_t::stats_is_persistent():
Accessors to dict_table_t::stat.
@dr-m dr-m requested a review from Thirunarayanan January 3, 2025 12:31
@dr-m dr-m self-assigned this Jan 3, 2025
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants