Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add MySQL 8.4 Support #1

Merged
merged 9 commits into from
Jan 30, 2025
Merged

Add MySQL 8.4 Support #1

merged 9 commits into from
Jan 30, 2025

Conversation

chen-anders
Copy link

@chen-anders chen-anders commented Jan 29, 2025

Related issue: github#1493

Description

MySQL 8.4 deprecates several old-style master/slave commands. Unfortunately, we had upgraded our DB to 8.4.3 and then found out after the fact that gh-ost broke, so this PR aims to introduce compatibility across 8.4+.

TODO: Update bash scripts used in tests to grep for the correct strings related to master/slave status

In case this PR introduced Go code changes:

  • contributed code is using same conventions as original code
  • script/cibuild returns with no formatting errors, build errors or unit test errors.

I wasn't able to get a minimal mysql 8.4 tarball that would actually pass the CI tests with dbdeployer but did manage to get the docker version running and passing: https://github.com/chen-anders/gh-ost/actions/runs/13026574857

Review Guide

Majority of diffs come from the newly vendored vendor/github.com/hashicorp/go-version library. Actual changes involve building a map of equivalent non-deprecated terms and then subbing the terms in the relevant places where needed.

@chen-anders chen-anders force-pushed the anders/mysql-8.4-support branch from ffc60f3 to 7142b2b Compare January 29, 2025 15:52
localtests/test.sh Outdated Show resolved Hide resolved
localtests/test.sh Outdated Show resolved Hide resolved
Copy link

@michaelorr michaelorr left a comment

Choose a reason for hiding this comment

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

Looks mostly good. I saw one thing that I think is a typo and one nit about conditional structure. No major red flags on the approach in general. Its a little gross but I guess there's no avoiding a bunch of string interpolation in a library like this.

@chen-anders chen-anders merged commit 366ac02 into master Jan 30, 2025
6 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants