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

PXB-3079 : Prepare skips rollback on encrypted tables and completes s… #1480

Merged
merged 3 commits into from
Aug 14, 2023

Conversation

satya-bodapati
Copy link
Contributor

@satya-bodapati satya-bodapati commented Aug 1, 2023

…uccessfully if keyring plugin is not loaded

https://jira.percona.com/browse/PXB-3079

Problem:

If the backup contains an encrypted table and xtrabackup during prepare is not configured correctly, i.e. either keyring_file plugin is not passed to xtrabackup, or the keyring_file_data is not pointing to a valid keyring file, xtrabackup doesn't abort.

If there is a rollback on such encrypted tables, it skips rollback. The table is considered as dropped because it was not loaded to the cache. Encryption errors are ignored when the tablespace is loaded into the cache.

Fix:

If xtrabackup cannot load an encrypted tablespace during prepare, abort with an error.

wangbincmss and others added 2 commits August 1, 2023 06:53
https://jira.percona.com/browse/PXB-3059

Added an option to set innodb_redo_log_archive_dirs. xtrabackup
only sets the option if the server innodb_redo_log_archive_dirs is
currently set to NULL. We always restore it back to NULL in the end of
the backup.

Thanks wangbincmss for the patch.
Implemented PXB-3059 - Add an option to set innodb_redo_log archive.
@satya-bodapati
Copy link
Contributor Author

@altmannmarcelo
Copy link
Contributor

altmannmarcelo commented Aug 3, 2023

@satya-bodapati, what about single table backups?
We might have a lot of missing tables.
This is meant for fixing an issue with encryption but the fix is on general code path.
Will this patch break single table backup functionality?

@satya-bodapati
Copy link
Contributor Author

@satya-bodapati, what about single table backups? We might have a lot of missing tables. Will this patch break single table backup functionality?

No marce, for the IBDs that are missing, we will not see them. so no issues.

@altmannmarcelo
Copy link
Contributor

@satya-bodapati Do you have an example of an xtrabackup output exploring the new code path? I would like to have a look on the output.

@satya-bodapati
Copy link
Contributor Author

satya-bodapati commented Aug 3, 2023

Although fil_open_for_xtrabackup() is used by backup and prepare, only the prepare caller aborts.

@satya-bodapati
Copy link
Contributor Author

@satya-bodapati Do you have an example of an xtrabackup output exploring the new code path? I would like to have a look on the output.

023-08-03T18:05:03.074433+01:00 0 [ERROR] [MY-012226] [InnoDB] Encryption information in datafile: ./test/t1.ibd can't be decrypted, please confirm that keyring is loaded.
2023-08-03T18:05:03.074467+01:00 0 [Note] [MY-011825] [Xtrabackup] Failed to decrypt table ./test/t1.ibd with space id 8. Will check if encrytion key has been parsed at the end of backup.
2023-08-03T18:05:03.074500+01:00 0 [ERROR] [MY-011825] [Xtrabackup] Unable to open tablespace 'test/t1.ibd'. InnoDB DB_ error code is 113

@altmannmarcelo
Copy link
Contributor

@satya-bodapati As per slack conversation, we should try to only abort the backup if undo entries belong to missing encryption tables.

@altmannmarcelo
Copy link
Contributor

@satya-bodapati, please also change the target branch to be the release branch if you want this on 8.0.34

@altmannmarcelo
Copy link
Contributor

@satya-bodapati FYI: clang format is broken on your commit

@satya-bodapati
Copy link
Contributor Author

Yes @altmannmarcelo , I fixed it locally. Will push it along with other changes

@satya-bodapati satya-bodapati force-pushed the PXB-8.0-3079 branch 2 times, most recently from 78b673b to 5ff4c13 Compare August 9, 2023 13:47
@satya-bodapati
Copy link
Contributor Author

@altmannmarcelo updated the patch and added new testcase

Copy link
Contributor

@altmannmarcelo altmannmarcelo left a comment

Choose a reason for hiding this comment

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

Thanks for re-working it to only abort if we have rollback on the said tablespace.
Few minor comments. Since we basically re-worked a good part of the logic, can we have another Jenkins run?

share/messages_to_error_log.txt Outdated Show resolved Hide resolved
storage/innobase/fil/fil0fil.cc Outdated Show resolved Hide resolved
storage/innobase/fil/fil0fil.cc Outdated Show resolved Hide resolved
storage/innobase/xtrabackup/src/xb_dict.cc Outdated Show resolved Hide resolved
storage/innobase/fil/fil0fil.cc Outdated Show resolved Hide resolved
storage/innobase/fsp/fsp0file.cc Show resolved Hide resolved
storage/innobase/xtrabackup/src/xb_dict.cc Show resolved Hide resolved
Copy link
Contributor

@altmannmarcelo altmannmarcelo left a comment

Choose a reason for hiding this comment

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

LGTM. Please get the last round of jenkins and squash the commits

@altmannmarcelo
Copy link
Contributor

@satya-bodapati Don't forget to target this PR to RB.

@satya-bodapati
Copy link
Contributor Author

@satya-bodapati Don't forget to target this PR to RB.

Yeah, will rebase to release branch

@satya-bodapati
Copy link
Contributor Author

LGTM. Please get the last round of jenkins and squash the commits

Will squash and submit to Jenkins

…uccessfully if keyring plugin is not loaded

https://jira.percona.com/browse/PXB-3079

Problem:
--------
If backup contains encrypted table and xtrabackup is not configured correctly, i.e. either keyring_file plugin is
not passed to xtrabackup or the keyring_file_data is not pointing to valid keyring file, xtrabackup doesnt abort.

If there is a rollback on such encryped tables, it skips rollback. the table is considered as dropped because it was
not loaded to cache. Encryption errors are ignored when tablespace are loaded into cache.

Fix:
----
If there is a rollback on an encrypted table and it cannot be loaded,
abort with an error

If the backup contains encrypted table but there is no redo or rollback
on the table, and no keyring configuration is passed to xtrbackup, it
will continue to prepare successfully.
@satya-bodapati
Copy link
Contributor Author

@satya-bodapati satya-bodapati changed the base branch from 8.0 to release-8.0.34-29 August 14, 2023 09:53
@satya-bodapati satya-bodapati merged commit 0e52a5c into percona:release-8.0.34-29 Aug 14, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants