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

Backups: Support InnoDB Redo Log Location With 8.0.30+ #10847

Merged
merged 28 commits into from
Jul 30, 2022

Conversation

mattlord
Copy link
Contributor

@mattlord mattlord commented Jul 26, 2022

Description

The default location of the InnoDB redo logs changed to <innodb_log_group_home_dir>/#innodb_redo/ in 8.0.30. From the 8.0.30 release notes:

InnoDB: InnoDB now supports dynamic configuration of redo log capacity. The innodb_redo_log_capacity system variable can be set at runtime to increase or decrease the total amount of disk space occupied by redo log files.

With this change, the number of redo log files and their default location has also changed. From MySQL 8.0.30, InnoDB maintains 32 redo log files in an #innodb_redo directory in the data directory. Previously, InnoDB created two redo log files in the data directory by default, and the number and size of redo log files were controlled by the and innodb_log_files_in_group and innodb_log_file_size variables. These two variables are now deprecated.

When an innodb_redo_log_capacity setting is defined, innodb_log_files_in_group and innodb_log_file_size settings are ignored; otherwise, those settings are used to compute the innodb_redo_log_capacity setting (innodb_log_files_in_group * innodb_log_file_size = innodb_redo_log_capacity). If none of those variables are set, redo log capacity is set to the innodb_redo_log_capacity default value, which is 104857600 bytes (100MB).

Several status variables are provided for monitoring the redo log and redo log capacity r`esize operations.

As is generally required for any upgrade, this change requires a clean shutdown before upgrading.

For more information about this feature, see Redo Log.

This PR alters the backups and tests to account for this change when 8.0.30+ is being used.

ℹ️ This PR also pins the version of MySQL used in the Upgrade Downgrade Testing - Backups - Manual workflow to 8.0.29 because v14.0 binaries do not have these fixes, and ubuntu-20.04 since MySQL has no 8.0.29 packages for 22.04. This pinning can be removed once either of the following occurs:

  • The fixes in this PR are backported to vitessio/release-14
  • v15 is GA and vitessio/main has a dev version of v16.0.0-SNAPSHOT

My current tentative plan is to backport these fixes to at least v14. I would then do a follow-up PR on main to revert the changes made to the workflow (.github/workflows/upgrade_downgrade_test_backups_manual.yml).

Related Issue(s)

Checklist

@vitess-bot
Copy link
Contributor

vitess-bot bot commented Jul 26, 2022

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

  • Ensure that the Pull Request has a descriptive title.
  • If this is a change that users need to know about, please apply the release notes (needs details) label so that merging is blocked unless the summary release notes document is included.
  • If a new flag is being introduced, review whether it is really needed. The flag names should be clear and intuitive (as far as possible), and the flag's help should be descriptive. Additionally, flag names should use dashes (-) as word separators rather than underscores (_).
  • If a workflow is added or modified, each items in Jobs should be named in order to mark it as required. If the workflow should be required, the GitHub Admin should be notified.

Bug fixes

  • There should be at least one unit or end-to-end test.
  • The Pull Request description should either include a link to an issue that describes the bug OR an actual description of the bug and how to reproduce, along with a description of the fix.

Non-trivial changes

  • There should be some code comments as to why things are implemented the way they are.

New/Existing features

  • Should be documented, either by modifying the existing documentation or creating new documentation.
  • New features should have a link to a feature request issue or an RFC that documents the use cases, corner cases and test cases.

Backward compatibility

  • Protobuf changes should be wire-compatible.
  • Changes to _vt tables and RPCs need to be backward compatible.
  • vtctl command output order should be stable and awk-able.

Testing was done with:
cd examples/local
./401_teardown.sh;  ./101_initial_cluster.sh; ./backups/take_backups.sh

Signed-off-by: Matt Lord <[email protected]>
Signed-off-by: Matt Lord <[email protected]>
@mattlord mattlord changed the title Support default InnoDB Redo log location with 8.0.30+ Support InnoDB Redo log location with 8.0.30+ Jul 26, 2022
@mattlord mattlord changed the title Support InnoDB Redo log location with 8.0.30+ Backups: Support InnoDB Redo Log Location With 8.0.30+ Jul 26, 2022
The apt repo does not have older versions in it :-(

Signed-off-by: Matt Lord <[email protected]>
And we need to download more dependencies

Signed-off-by: Matt Lord <[email protected]>
@mattlord mattlord marked this pull request as ready for review July 27, 2022 15:12
@mattlord
Copy link
Contributor Author

Nice work, very clean PR.
The test cases are good. I have a couple of minor suggestions, otherwise LGTM.
We'll let the others take a look as well before approve/merge.

Thanks, @deepthi ! I've implemented both of your suggestions here: 40e9c29

Signed-off-by: Matt Lord <[email protected]>
@mattlord mattlord requested a review from deepthi July 28, 2022 14:55
@mattlord mattlord marked this pull request as draft July 28, 2022 17:30
@mattlord
Copy link
Contributor Author

mattlord commented Jul 28, 2022

Moved this back to draft as I need to confirm something and cannot seem to ATM due to: https://bugs.mysql.com/bug.php?id=108016

If there are other files in the innodb_log_group_home_dir directory — aside from the #innodb_redo subdirectory — then we need to treat that log dir in the same way that we treat the datadir for backups: backing up each file in that directory AND any immediate subdirectories (only 1 level deep).

@mattlord
Copy link
Contributor Author

mattlord commented Jul 28, 2022

Update: the issue I ran into was NOT 8.0.30 related in the end: https://bugs.mysql.com/bug.php?id=108017

I was able to confirm that the current code in the PR is correct. There are no other files in the innodb_log_group_homedir other than the new subdirectory with 8.0.30:

rm -rf /tmp/innodb /tmp/mysql; mkdir -p /tmp/innodb/data /tmp/innodb/logs /tmp/mysql  

$ mysqld --initialize-insecure --datadir=/tmp/mysql/data --innodb_log_group_home_dir=/tmp/innodb/logs

2022-07-28T18:27:13.902013Z 0 [System] [MY-013169] [Server] /usr/local/mysql-8.0.30-macos12-arm64/bin/mysqld (mysqld 8.0.30) initializing of server in progress as process 36905
2022-07-28T18:27:13.903883Z 0 [Warning] [MY-010159] [Server] Setting lower_case_table_names=2 because file system for /tmp/mysql/data/ is case insensitive
2022-07-28T18:27:13.906453Z 1 [System] [MY-013576] [InnoDB] InnoDB initialization has started.
2022-07-28T18:27:13.977380Z 1 [System] [MY-013577] [InnoDB] InnoDB initialization has ended.
2022-07-28T18:27:14.408477Z 6 [Warning] [MY-010453] [Server] root@localhost is created with an empty password ! Please consider switching off the --initialize-insecure option.
2022-07-28T18:27:14.728361Z 0 [System] [MY-013172] [Server] Received SHUTDOWN from user <via user signal>. Shutting down mysqld (Version: 8.0.30).

$ ls -l /tmp/innodb/logs                                                                                                                                                                                                                                                                                                                              
drwxr-x---  34 matt  wheel  1088 Jul 28 14:27 #innodb_redo

@mattlord mattlord marked this pull request as ready for review July 28, 2022 18:30
Copy link
Member

@GuptaManan100 GuptaManan100 left a comment

Choose a reason for hiding this comment

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

The code LGTM especially the capability check using the server version 💯

Copy link
Member

@GuptaManan100 GuptaManan100 left a comment

Choose a reason for hiding this comment

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

We need release notes changes too for v14 and v13. That however will be separate PRs right?

@deepthi
Copy link
Member

deepthi commented Jul 30, 2022

We need release notes changes too for v14 and v13. That however will be separate PRs right?

You mean documenting the 8.0.30 incompatibility as a known issue? Yeah, those have to be separate PRs on the release branches + update to the actual release content in GH releases.
We also need to write down the upgrade path for anyone on v14 and trying to move to 8.0.30. They will need to first deploy a build that includes these fixes and only then upgrade their MySQL.

@deepthi deepthi merged commit 0922dda into vitessio:main Jul 30, 2022
@deepthi deepthi deleted the 8.0.30_builtin_backups branch July 30, 2022 16:34
@vitess-bot
Copy link
Contributor

vitess-bot bot commented Jul 30, 2022

I was unable to backport this Pull Request to the following branches: release-14.0.

mattlord pushed a commit to planetscale/vitess that referenced this pull request Jul 31, 2022
…kups

... removing the temporary workflow changes on main

Backups: Support InnoDB Redo Log Location With 8.0.30+
Signed-off-by: Matt Lord <[email protected]>
mattlord added a commit to planetscale/vitess that referenced this pull request Jul 31, 2022
This should not be merged until after the backport is merged:
  vitessio#10895

Signed-off-by: Matt Lord <[email protected]>
mattlord added a commit that referenced this pull request Aug 1, 2022
This should not be merged until after the backport is merged:
  #10895

Signed-off-by: Matt Lord <[email protected]>
mattlord added a commit that referenced this pull request Aug 2, 2022
)

* Merge pull request #10847 from planetscale/8.0.30_builtin_backups

... removing the temporary workflow changes on main

Backups: Support InnoDB Redo Log Location With 8.0.30+
Signed-off-by: Matt Lord <[email protected]>

* Address linter warning:

WARN [linters context] structcheck is disabled because of go1.18. You can track the evolution of the go1.18 support by following the golangci/golangci-lint#2649.
go/mysql/conn_flaky_test.go:658:2: S1001: should use copy() instead of a loop (gosimple)
	for j, i := range t.queryPacket {
	^

Signed-off-by: Matt Lord <[email protected]>

* Pin MySQL Version at 8.0.29 for upgrade/downgrade manual e2e test

This is needed as we do not currently plan on backporting the backup
fixes to v13 and older, thus release-13.0 will not ever get the
backup fixes to support 8.0.30+.

If we do decide to backport the fixes to release-13.0 then this
workflow change can be reverted.

Signed-off-by: Matt Lord <[email protected]>

* Correct comment based on current plans

Signed-off-by: Matt Lord <[email protected]>

Co-authored-by: Deepthi Sigireddi <[email protected]>
@rsajwani
Copy link
Contributor

rsajwani commented Aug 2, 2022

Changes LGTM. Wondering we don't have similar test cases for XtrabackupEngine which can verify these changes for Xtrabackup

@mattlord
Copy link
Contributor Author

mattlord commented Aug 2, 2022

Changes LGTM. Wondering we don't have similar test cases for XtrabackupEngine which can verify these changes for Xtrabackup

Our XtraBackup tests use Percona's Apt Repo as the latest version of XtraBackup 8.0 (8.0.29 as of July 17) often does not support the latest upstream MySQL 8.0 release (8.0.30 as of today) for some time: #10691

mattlord added a commit to planetscale/vitess that referenced this pull request Aug 3, 2022
This reverts commit 06fc7a2.

The upgrade/downgrade tests use the last release tag of the
previous version (e.g. v14.0.1) instead of the previous version's
release branch (e.g. release-14.0) so we can't remove this pinning
until v14.0.2 is released.

We also update the comment to reflect the true conditions that will
allow us to remove this MySQL version pinning.

Signed-off-by: Matt Lord <[email protected]>
deepthi added a commit that referenced this pull request Aug 3, 2022
Revert: Revert temporary workflow changes made in #10847
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug Report: Builtin Backups Fail with default behaviour of MySQL 8.0.30
4 participants