Skip to content

Temp: Pin XtraBackup version used at 2.4.24 for 5.7 tests#10194

Merged
deepthi merged 12 commits intovitessio:mainfrom
planetscale:xtrabackup_2.2.24_pin
May 3, 2022
Merged

Temp: Pin XtraBackup version used at 2.4.24 for 5.7 tests#10194
deepthi merged 12 commits intovitessio:mainfrom
planetscale:xtrabackup_2.2.24_pin

Conversation

@mattlord
Copy link
Copy Markdown
Member

@mattlord mattlord commented May 2, 2022 •
edited
Loading

Description

⚠️ Note: pinning the version is temporary until XtraBackup 2.5.26 is released, at which time we will switch back to using the latest ⚠️

This pins the XtraBackup version used in our non-8.0 CI jobs to 2.4.24 as 2.4.25 crashes with the --xtrabackup_stream_mode=tar flag used with 5.7 (with 8.0 we use --xtrabackup_stream_mode=xbstream).

Open Questions

  1. Should we also pin the version for the 8.0 CI jobs for consistency?
    • Decision: NO
  2. Should we pin this permanently, even after 2.4.26 comes out with the fix in the coming weeks?
    • Decision: NO

Related Issue(s)

Checklist

  • "Backport me!" label has been added if this change should be backported
  • Tests were added or are not required
  • Documentation was added or is not required

@mattlord
Pin XtraBackup version used at 2.4.24 for 5.7 tests
4084bda 
Signed-off-by: Matt Lord <mattalord@gmail.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 2, 2022 •
edited by deepthi
Loading

Review Checklist

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

General

  • Ensure that the Pull Request has the correct release notes label. release notes none should only be used for PRs that are so trivial that they need not be 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.

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.

@deepthi
Copy link
Copy Markdown
Collaborator

deepthi commented May 2, 2022

Should we also pin the version for the 8.0 CI jobs for consistency?

Let's not do that. If they are running fine against the latest, that's good.

Should we pin this permanently, even after 2.4.26 comes out with the fix in the coming weeks?

No, we should not.

@mattlord
Explicitly define which clusters need XtraBackup
1b6d722 
Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord mattlord requested a review from deepthi May 2, 2022 23:04
mattlord added 7 commits May 2, 2022 19:07
@mattlord
Remove vtctlbackup_sharded_clustertest_heavy from needsXtraBackup list
1b02ad4 
Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord
Look for github workflow changes in paths-filter
e7c03a4 
Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord
Need to use sudo to install the deb
8ad9fef 
Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord
Correct errant quote placement
2c8960a 
Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord
Install xtrabackup deps after installing the deb file
8eac689 
Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord
Use apt install instead of dpkg
54a7ede 
Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord
Actually execute the generator 🤦
7f3716d 
Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord mattlord changed the title Pin XtraBackup version used at 2.4.24 for 5.7 tests Temp: Pin XtraBackup version used at 2.4.24 for 5.7 tests May 3, 2022
@mattlord
Improve comment
b383755 
Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord mattlord requested a review from frouioui May 3, 2022 00:27
@mattlord
Actions env var interpolation not working as expected...
701b1a4 
I suspect it may my usage of curly braces which could be impacting
their usage of ${{FOO}} for things processed before sending to the
runner. See:
  https://docs.github.com/en/actions/learn-github-actions/environment-variables#using-contexts-to-access-environment-variable-values

Signed-off-by: Matt Lord <mattalord@gmail.com>
deepthi
deepthi reviewed May 3, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@deepthi deepthi 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 for improving the workflow files. This should give us some slight improvements in CI run times too.

deepthi
deepthi reviewed May 3, 2022
View reviewed changes
@mattlord
Only include env var if xtrabackup is needed
2702706 
Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord mattlord requested a review from deepthi May 3, 2022 17:41
@deepthi
Copy link
Copy Markdown
Collaborator

deepthi commented May 3, 2022 •
edited
Loading

Things I noticed (for future PRs):

  • We should probably start running xtrabackup tests against MySQL 8.0 as well
  • Backup upgrade/downgrade tests don't seem to test xtrabackup
  • vtgate_gen4 is flaky, created Flaky vtgate_gen4 test #10208

@deepthi deepthi mentioned this pull request May 3, 2022
Merged
3 tasks
@deepthi deepthi merged commit 333fbd2 into vitessio:main May 3, 2022
@deepthi deepthi deleted the xtrabackup_2.2.24_pin branch May 3, 2022 20:44
@mattlord mattlord mentioned this pull request May 4, 2022
Merged
3 tasks
@mattlord mattlord mentioned this pull request May 9, 2022
Merged
3 tasks
@tanjinx tanjinx mentioned this pull request May 9, 2023
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@frouioui frouioui Awaiting requested review from frouioui frouioui is a code owner

1 more reviewer

@deepthi deepthi deepthi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Component: Build/CI Type: Testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Xtrabackup tests are failing

2 participants

@mattlord @deepthi