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

PT-2389 Fix unconditional ALTER on new table with resume #919

Open
wants to merge 2 commits into
base: 3.x
Choose a base branch
from
Open

PT-2389 Fix unconditional ALTER on new table with resume #919

wants to merge 2 commits into from

Conversation

drwonky
Copy link

@drwonky drwonky commented Jan 28, 2025 •
edited
Loading

  • The contributed code is licensed under GPL v2.0
  • Contributor Licence Agreement (CLA) is signed
  • util/update-modules has been ran
  • Documentation updated
  • Test suite update

@it-percona-cla
Copy link

it-percona-cla commented Jan 28, 2025 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@drwonky
PT-2389 Fix unconditional ALTER on new table with resume
dc77289 
When running with --resume option the ALTER runs on the new table
even if the new table exists and has already been altered.

This causes a deterministic failure every time the --resume option
is used.  This fix tests if --resume is given and does not run
ALTER on the new table.  If pt-osc did not successfully alter
the new table during the previous invocation, this could cause the
ALTER to not be applied.

The right fix would be to compare the DDL of the _new table to
the proposed DDL generated by pt-osc and only run ALTER if they
do not match.
@drwonky
Copy link
Author

drwonky commented Jan 28, 2025

I'm having trouble running the test suite when following the directions in the CONTRIBUTING.md precisely. I am getting an SSL error from the PERL DBI library: SSL connection error: Enforcing SSL encryption is not supported. I noted that the resume test for PT-1717 did a null alter and that's why PT-2389 was not surfaced, so I updated that test to add a column instead of doing a null alter.

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

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

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@drwonky @it-percona-cla