Skip to content

[move tables] single table integration test support#1714

Merged
chriskirkland merged 10 commits into
feature-move-tablesfrom
chriskirkland/move-tables-integration-tests
Jun 19, 2026
Merged

[move tables] single table integration test support#1714
chriskirkland merged 10 commits into
feature-move-tablesfrom
chriskirkland/move-tables-integration-tests

Conversation

@chriskirkland

@chriskirkland chriskirkland commented Jun 18, 2026

Copy link
Copy Markdown

Part of https://github.com/github/database-infrastructure/issues/8260

A Pull Request should be associated with an Issue.

We wish to have discussions in Issues. A single issue may be targeted by multiple PRs.
If you're offering a new feature or fixing anything, we'd like to know beforehand in Issues,
and potentially we'll be able to point development in a particular direction.

Related issue (internal): https://github.com/github/database-infrastructure/issues/8260
Related issue (public): #1681

Further notes in https://github.com/github/gh-ost/blob/master/.github/CONTRIBUTING.md
Thank you! We are open to PRs, but please understand if for technical reasons we are unable to accept each and any PR

Description

This PR introduces new tests into localtests to test functionality in --move-tables mode, building upon the existing harness but utilizing a separate 2 cluster setup necessary for e2e validation.

Summary:

  • .github/workflows/move-tables-tests.yml - new workflow which invokes move tables tests in CI
  • script/docker-gh-ost-move-tables-tests- entrypoint for managing dual cluster setup and executing move-tables tests
  • localtests/move-tables-test.sh - harness for running one or more move-tables tests
  • new tests to exercise simple e2e flows1
    • single - migrate a single, pre-populated table with no DML events
    • single-concurrent-writes - migrate a single, pre-populated table with DMLs events running concurrent to write cutover.

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.

Example

Try out these changes locally:

./script/docker-gh-ost-move-tables-tests run --rm

Footnotes

  1. I'm planning more sophisticated tests to validate cutover state (e.g. crash safety), throttling, etc. in a separate PR.

@chriskirkland chriskirkland force-pushed the chriskirkland/move-tables-integration-tests branch from 6ebaafe to fe7e22f Compare June 18, 2026 20:46
@zacharysierakowski zacharysierakowski added the feature-move-tables PRs that are associated with the new move-tables feature label Jun 19, 2026
Base automatically changed from move-tables-1.6-crash-safe-resume to feature-move-tables June 19, 2026 16:55
@chriskirkland chriskirkland force-pushed the chriskirkland/move-tables-integration-tests branch from fe7e22f to 8413d45 Compare June 19, 2026 17:09
@chriskirkland chriskirkland marked this pull request as ready for review June 19, 2026 18:34
@chriskirkland chriskirkland requested a review from meiji163 as a code owner June 19, 2026 18:34
Copilot AI review requested due to automatic review settings June 19, 2026 18:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a dedicated local integration-test harness for --move-tables mode, including CI coverage via a new GitHub Actions workflow and a Docker-based dual-cluster (source/target) MySQL test environment.

Changes:

  • Introduces a new localtests/move-tables-test.sh harness plus initial end-to-end move-tables test cases (single, single-concurrent-writes).
  • Adds script/docker-gh-ost-move-tables-tests and updates move-tables helper scripts to support the dual-cluster setup and configurable database name.
  • Adds a new CI workflow to run move-tables tests across a MySQL/Percona version matrix, and excludes move-tables from the existing localtests/test.sh runner.
Show a summary per file
File Description
script/move-tables/setup Aligns the setup DB name and seeds from the new move-tables test SQL.
script/move-tables/insert-source-primary-loop Adds support for configurable DB selection for write-load generation.
script/docker-gh-ost-move-tables-tests New Docker entrypoint to bring up/down a dual-cluster environment and run move-tables tests.
localtests/test.sh Excludes move-tables from the legacy localtests runner.
localtests/move-tables/single/tables.txt Declares tables involved in the “single” move-tables test.
localtests/move-tables/single/create.sql Creates/seeds the “single” test table.
localtests/move-tables/single-concurrent-writes/tables.txt Declares tables involved in the concurrent-writes test.
localtests/move-tables/single-concurrent-writes/on_test.sh Starts background writes during the move-tables run.
localtests/move-tables/single-concurrent-writes/create.sql Creates/seeds the concurrent-writes test table.
localtests/move-tables-test.sh New move-tables test harness that runs against source/target clusters and validates results.
.github/workflows/move-tables-tests.yml New CI workflow running move-tables tests in a containerized environment and uploading logs on failure.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 10/11 changed files
  • Comments generated: 7

Comment thread script/docker-gh-ost-move-tables-tests
Comment thread script/docker-gh-ost-move-tables-tests
Comment thread localtests/move-tables-test.sh
Comment thread localtests/move-tables-test.sh Outdated
Comment thread localtests/move-tables-test.sh Outdated
Comment thread localtests/move-tables-test.sh Outdated
Comment thread script/docker-gh-ost-move-tables-tests

@zacharysierakowski zacharysierakowski left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

still reviewing localtests/move-tables-test.sh closer, but just sending this off for now

Comment thread localtests/move-tables-test.sh Outdated
Comment thread localtests/move-tables-test.sh

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is this only running single right now? Quick pass I can't tell where single-concurrent-writes is called

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

oh i see in the CI output that it's running it, so maybe i'm just missing how that's populated. Assuming it's reading the move-tables directory

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yeah, test_all looks for all of the directories at the configured path

tests_path=$(dirname $0)/move-tables

Comment thread localtests/move-tables-test.sh Outdated
chriskirkland and others added 3 commits June 19, 2026 14:18
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@chriskirkland chriskirkland merged commit f028cbd into feature-move-tables Jun 19, 2026
12 of 13 checks passed
@chriskirkland chriskirkland deleted the chriskirkland/move-tables-integration-tests branch June 19, 2026 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature-move-tables PRs that are associated with the new move-tables feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants