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.

Sign up for GitHub

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

Jump to bottom

MDEV-34169 Don't allow innodb_open_files to be lesser than number of non-user tablespace #3255

Open
wants to merge 1 commit into
base: 10.5
Choose a base branch
from
Open

MDEV-34169 Don't allow innodb_open_files to be lesser than number of non-user tablespace #3255

wants to merge 1 commit into from

Conversation

Thirunarayanan
Copy link
Member

@Thirunarayanan Thirunarayanan commented May 15, 2024 •
edited

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

Description

  • InnoDB only closes the user tablespace when the number of open files exceeds innodb_open_files limit. In that case, InnoDB should make sure that innodb_open_files value should be greater than number of undo tablespace, system and temporary tablespace.

How can this PR be tested?

./mtr innodb.open_files_limit

Basing the PR against the correct MariaDB version

  • This is a new feature and the PR is based against the latest MariaDB development branch.
  • This is a bug fix and the PR is based against the earliest maintained branch in which the bug can be reproduced.

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.

@Thirunarayanan Thirunarayanan requested a review from dr-m May 15, 2024 10:05
@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.

dr-m
dr-m reviewed May 20, 2024
View reviewed changes
Copy link
Contributor

@dr-m dr-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide a test script that shows what would happen when this fix is not present? The current test case does not even check that this warning is actually being issued.

storage/innobase/handler/ha_innodb.cc Outdated Show resolved Hide resolved
@Thirunarayanan Thirunarayanan changed the title MDEV-34619 Don't allow innodb_open_files to be lesser than number of non-user tablespace MDEV-34169 Don't allow innodb_open_files to be lesser than number of non-user tablespace Jun 5, 2024
@Thirunarayanan
MDEV-34169 Don't allow innodb_open_files to be lesser than
b54506b 
		number of non-user tablespace.

- InnoDB only closes the user tablespace when the number of open
files exceeds innodb_open_files limit. In that case, InnoDB should
make sure that innodb_open_files value should be greater
than number of undo tablespace, system and temporary tablespace.
dr-m
dr-m approved these changes Jun 5, 2024
View reviewed changes
Copy link
Contributor

@dr-m dr-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, this looks OK.

One more thing that occurred to me is that we could easily extend the test with a multi-file innodb_temp_data_file_path to demonstrate if the lower limit of innodb_open_files needs to take multi-file system&temporary tablespaces into account.

mysql-test/suite/innodb/t/open_files_limit.test
@@ -0,0 +1,9 @@
--source include/have_innodb.inc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be missing

--source include/not_embedded.inc

storage/innobase/handler/ha_innodb.cc
Comment on lines +3865 to +3869
"InnoDB: innodb_open_files %ld should be "
"greater than total number of undo tablespace, "
"system & temporary tablespace. So changing "
"the value of innodb_open_files to %ld",
innobase_open_files, srv_undo_tablespaces + 3);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’d suggest to use some more = in the message so that it becomes easier to understand for non-native English speakers:

InnoDB: innodb_open_files=%ld is smaller than 3+innodb_undo_tablespaces=%ld; adjusting to innodb_open_files=%ld

The message does not necessarily need to explain what the 3 is coming from.

By the way, should this take into account system or temporary tablespaces that consist of multiple files? Can you cover that this in the test case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dr-m dr-m dr-m approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
3 participants
@Thirunarayanan @CLAassistant @dr-m