Skip to content

cross-schema migration - #3

Open
BarShauli555 wants to merge 117 commits into
wix-playground:masterfrom
BarShauli555:migration-schema-cutover-impl
Open

cross-schema migration#3
BarShauli555 wants to merge 117 commits into
wix-playground:masterfrom
BarShauli555:migration-schema-cutover-impl

Conversation

@BarShauli555

Copy link
Copy Markdown
Collaborator

Add implementation plan for cross-schema migration via ghost_migration_schema

Spec-driven plan for the opt-in --use-migration-schema mode: creates the
ghost/changelog/checkpoint tables in a dedicated ghost_migration_schema
database and swaps tables across schemas during the atomic cut-over, so the
migrated table lands in the original schema and the old table lands in
ghost_migration_schema. Plan covers context field/accessor + flag, applier
SQL routing, cross-schema row-copy, binlog/throttler/inspector wiring,
cross-schema atomic cut-over, schema-existence validation, localtests, docs.

meiji163 and others added 30 commits February 2, 2024 09:10
Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 4 to 6.
- [Release notes](https://github.com/golangci/golangci-lint-action/releases)
- [Commits](golangci/golangci-lint-action@v4...v6)

---
updated-dependencies:
- dependency-name: golangci/golangci-lint-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: overallteach <cricis@foxmail.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Switch all unit tests to use `stretchr/testify` for assertions.
Use testcontainers to spawn MySQL server container in unit tests. github#1457
Avoid causing deadlocks when copying rows on busy tables
Improve query building routines of DML event queries, reducing time and allocations
ggilder and others added 30 commits March 9, 2026 10:05
* Replace log.Fatale with context-based error propagation

This patch modifies gh-ost to use a cancellable context instead of
log.Fatale() in listenOnPanicAbort. When using gh-ost as a library, this
allows the calling application to recover from aborts (e.g. log the
failure reason) instead of having the entire process terminate via
os.Exit(). Now we store the error and cancel a context to signal all
goroutines to stop gracefully.

* Fix shadowing

* Simplify non-blocking poll

* Simplify non-blocking poll in migrator.go

* Fix error return

* Fix hang on blocking channel send

* Add defensive fix for other potential blocking channel send deadlocks

* Add SendWithContext helper to avoid deadlocks

* Fix deadlock on PanicAbort sends

* Use checkAbort

* Fix migration abort race condition

* Remove buffer on PanicAbort channel
* Improve tests for various error scenarios

- Regex meta characters in index names should not break warning
  detection (required code fix)
- Improve tests that only checked number of rows (need to validate data
  as well)
- Test positive case allowing ignored duplicates on migration key
- Test behavior with PanicOnWarnings disabled

* Address Copilot feedback

* Add test for warnings on composite unique keys

* Add test for updating pk with duplicate

* Improve replica test debugging

- Print log excerpt on failure
- Upload full log artifacts on failure

* Reduce flakiness in update-pk test

* Revise test

* More robust test fix

* Make MySQL wait strategy less flaky

Removed the `wait.ForExposedPort()` override from test files. The tests
will now use the MySQL module's default wait strategy
(`wait.ForLog("port: 3306  MySQL Community Server")`), which properly
waits for MySQL to be ready to accept connections. Otherwise the port
may be exposed, but MySQL is still initializing and not ready to accept
connections.

* Customize update-pk integration test

Add support for test-specific execution so that we can guarantee that
we're specifically testing the DML apply phase

* Fix regression in integration test harness

* Add test timeouts and fix error propagation

Prevent indefinite test hangs by adding 120-second timeout and
duration reporting. Fix silent error drops by propagating errors from
background write goroutines to PanicAbort channel. Check for abort in
sleepWhileTrue loop and handle its error in cutOver.
* Add retry logic for instant DDL on lock wait timeout
When attempting instant DDL, a lock wait timeout (errno 1205) may occur
if a long-running transaction holds a metadata lock. Rather than failing
immediately, retry the operation up to 5 times with linear backoff.
Non-timeout errors (e.g. ALGORITHM=INSTANT not supported) still return
immediately without retrying.

* Fix int-to-Duration type mismatch in retry backoff

---------

Co-authored-by: ybs-me <yosef.bensimchon@melio.com>
* Handle warnings in middle of DML batch

* Add integration test for batch warnings

* Update expected failure message for update-pk test

---------

Co-authored-by: meiji163 <meiji163@github.com>
* Add failing test for retry + abort issue

* Fix retry after abort issue

* Skip retries for warning errors

Warning errors indicate data consistency issues that won't resolve on retry, so
attempting to retry them is futile and causes unnecessary delays. This change
detects warning errors early and aborts immediately instead of retrying.

* Fix test expectation
* Add GH_OST_INSTANT_DDL env var for hook
…ithub#1661)

* Fix Warning 1300 for varbinary columns with bytes invalid as utf8mb4

When gh-ost replays a binlog DML event, the go-mysql library returns
varbinary column values as a Go `string` (not `[]byte`). In convertArg,
the existing code only converted string → []byte when the column had a
non-empty Charset (e.g. utf8mb4 for varchar). varbinary columns have no
character set, so Charset is always "", and the string fell through
unconverted.

The Go MySQL driver sends `string` args as MYSQL_TYPE_VAR_STRING with
the connection's utf8mb4 charset metadata attached, causing MySQL to
validate the bytes. If a varbinary value (e.g. a binary UUID) contains
byte sequences that are invalid utf8mb4, MySQL emits Warning 1300. With
gh-ost's panic-on-warnings enabled, this aborts the migration.

Fix: add an else-if branch that detects binary storage types by
MySQLType (binary, varbinary, *blob) and returns []byte, so the driver
sends MYSQL_TYPE_BLOB (binary data) with no charset validation.

MySQLType is used rather than Charset == "" alone because test Column
objects built via NewColumnList leave MySQLType unset, which would have
changed the return type for all no-charset columns in existing tests.
In production, inspect.go always populates MySQLType from
information_schema.data_type.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Address PR review feedback on varbinary Warning 1300 fix

- Use MySQLType "varbinary(16)" in tests instead of bare "varbinary",
  matching the real value produced by information_schema COLUMN_TYPE
  (which includes length). Guards against future refactors that might
  switch from substring matching to exact matching.
- Correct test comment: MySQLType is populated from COLUMN_TYPE, not
  data_type.
- Broaden types.go comment to say "the connection's charset/collation
  metadata (often utf8mb4)" since gh-ost's connection charset is
  configurable via --charset.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* appease linter

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: meiji163 <meiji163@github.com>
…binlog sentinel (github#1637)

* Prevent permanent worker deadlock when cutover times out waiting for binlog sentinel

Buffer allEventsUpToLockProcessed to MaxRetries() so the applier's send always
completes immediately even after waitForEventsUpToLock has timed out and exited.

---------

Co-authored-by: meiji163 <meiji163@github.com>
…ithub#1666)

PR github#1637 buffered allEventsUpToLockProcessed to MaxRetries() to prevent
a goroutine deadlock when waitForEventsUpToLock times out during cutover.
However, when --default-retries is set to a very large value (e.g.
9999999999999), Go tries to allocate a channel with trillions of buffer
slots, causing an immediate OOM crash before the migration even starts.

Replace the MaxRetries()-sized buffer with a buffer of 1 and
overwrite-oldest (latest-wins) send semantics. When the buffer is full
(receiver timed out on a previous attempt), the stale message is drained
before sending the current sentinel. This guarantees:

- The current sentinel is always delivered (no message loss)
- The executeWriteFuncs worker is never blocked (no deadlock)
- Memory usage is constant regardless of MaxRetries() (no OOM)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ithub#1671)

* Prevent throttle() from blocking forever when context is cancelled

When abort() is called (e.g. due to heartbeat failures after a MySQL
failover), the context is cancelled and initiateThrottlerChecks exits
via ctx.Done() without calling SetThrottled(false). The throttle() loop
only checks IsThrottled(), so it would spin indefinitely, preventing the
migration from ever returning.

Replace time.Sleep with a select on ctx.Done() so throttle() unblocks
immediately on context cancellation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Address copilot feedback

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…b#1675)

* Add MigrationContext.Hooks for in-process hook implementations

gh-ost's only hook extension point is on-disk scripts globbed from
--hooks-path. Library callers that embed Migrator must either ship
scripts and their dependencies alongside their binary or maintain a
parallel Go layer that bridges script side effects back into the host
application.

Introduce a Hooks interface in go/base with one method per lifecycle
event, and an optional MigrationContext.Hooks field. NewMigrator reads
the field once at construction and falls back to the existing
HooksExecutor when unset, so CLI behavior is unchanged. A CompositeHooks
helper in go/logic lets callers run the on-disk script executor and
their own Go implementation side-by-side.

HooksExecutor's previously package-private method names are renamed
(onStartup -> OnStartup, etc.) so external types can satisfy the
interface. The struct and constructor were already exported but the
methods weren't, so no usable external API is displaced.

* Skip nil entries in CompositeHooks and self-contain doc example

Address PR review feedback:

- CompositeHooks.OnX methods skip nil members instead of panicking,
  allowing callers to conditionally append optional hooks.
- doc/hooks.md embedded-usage snippet now defines ctx and version so it
  is self-contained.
…p across 1 directory (github#1606)

* Bump golang.org/x/crypto in the go_modules group across 1 directory

Bumps the go_modules group with 1 update in the / directory: [golang.org/x/crypto](https://github.com/golang/crypto).


Updates `golang.org/x/crypto` from 0.37.0 to 0.45.0
- [Commits](golang/crypto@v0.37.0...v0.45.0)

---
updated-dependencies:
- dependency-name: golang.org/x/crypto
  dependency-version: 0.45.0
  dependency-type: indirect
  dependency-group: go_modules
...

Signed-off-by: dependabot[bot] <support@github.com>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: meiji163 <meiji163@github.com>
…locks (github#1677)

Co-authored-by: meiji163 <meiji163@github.com>
…github#1684)

* Fix resume data loss: route heartbeat coords through applyEventsQueue

onChangelogHeartbeatEvent was mutating applier.CurrentCoordinates directly
from the streamer goroutine, before any DML that preceded the heartbeat was
applied to the ghost table. The checkpoint loop reads CurrentCoordinates as
"applied through this GTID" and could persist a checkpoint whose
LastTrxCoords was ahead of what was actually applied.

If gh-ost crashed before applyEventsQueue drained, --resume read that
checkpoint and called StartSyncGTID with the persisted set; MySQL treated
the un-applied GTIDs as already-seen and never re-streamed them. The ghost
table silently lost those DMLs and cut-over produced a stale table.

Fix: enqueue a tableWriteFunc onto applyEventsQueue that performs the
coords bump. The apply goroutine executes it in order, after the DMLs the
streamer enqueued before the heartbeat, restoring the invariant.

Adds TestMigratorHeartbeatDoesNotAdvancePastUnappliedDML, which fails at
the previous HEAD and passes after the fix; also asserts queue ordering to
guard against future changes that wrap the heartbeat enqueue in a goroutine.

Co-authored-by: Bastian Bartmann <bastian.bartmann@shopify.com>

* Replace direct channel write with SendWithContext

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Bastian Bartmann <bastian.bartmann@shopify.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Add Datadog/statsd with simple client emitting startup

* Add go runtime metrics to statsd reporting

---------

Co-authored-by: meiji163 <meiji163@github.com>
Refactor newTestMigrationContext to set ServeSocketFile via os.TempDir() and remove runtime.Caller-based path derivation.
* Disable CGO for release builds

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.