Skip to content

Sync upstream 2026-11-22 - #56

Open
coding-chimp wants to merge 43 commits into
masterfrom
sync-upstream-2026-11-22
Open

coding-chimp wants to merge 43 commits into
masterfrom
sync-upstream-2026-11-22

Conversation

@coding-chimp

Copy link
Copy Markdown

Description

Sync master from upstream github/gh-ost master through f7a42f6b.

Upstream changes brought in (PR numbers refer to github/gh-ost):

Merge integration fixes

The merge had no textual conflicts, but upstream's MariaDB GTID support required adapting our Shopify-specific binlog metrics code.

  • go/binlog/gomysql_reader.go: updated the GTID metrics handler to accept the shared GTID interface. Transaction metrics flush for both MySQL and MariaDB; transaction-length and commit-group metrics remain MySQL-only, guarded by a checked type assertion.
  • go/binlog/gomysql_reader_test.go: added regression coverage for both GTID flavors, coordinate updates, counter resets, and duplicate-metric prevention.
  • dev.yml: bumped the development Go version to 1.25.12 to match upstream.

Verification

Verified commit eaffe1c3 with Go 1.25.12:

  • go test -count=1 ./go/... — success.
  • go test -race -count=1 ./go/... — success.
  • go vet ./go/... — success.
  • Branch whitespace check excluding vendor/ — success. Vendored whitespace warnings are unchanged from upstream.

conf and others added 30 commits June 28, 2026 13:18
gh-ost hard-coded the go-mysql binlog syncer to MySQLFlavor and parsed
every GTID set as MySQL, so --gtid could not be used against MariaDB.
go-mysql already speaks the MariaDB GTID dialect; this wires it up.

- Detect server flavor from the version string (IsMariaDB / FlavorFor)
  and set it on the BinlogSyncerConfig.
- Make GTIDBinlogCoordinates hold the flavor-agnostic gomysql.GTIDSet
  interface; parse via ParseGTIDSet(flavor, ...).
- Handle MariadbGTIDEvent alongside GTIDEvent in the streamer, and read
  the committed GTID set from XIDEvent.GSet without a MySQL-only cast.
- Read MariaDB GTID positions from @@global.gtid_binlog_pos,
  Gtid_IO_Pos and @@global.gtid_slave_pos (MariaDB has no
  Executed_Gtid_Set column nor gtid_mode / enforce_gtid_consistency).
- Skip the gtid_mode / enforce_gtid_consistency validation on MariaDB,
  where GTIDs are always recorded when binary logging is enabled.

localtests:
- Detect the server version once and reuse it; run gtid_mode=ON tests on
  MariaDB (normalize its current_gtid_mode to ON) and trim the GTID
  diagnostics that don't exist on MariaDB.
- Enable gtid_strict_mode on the MariaDB test servers. Because
  --test-on-replica makes gh-ost write locally on the replica, give the
  replica its own GTID domain so its writes never collide with the
  primary's domain-0 stream under strict mode.
- Add the gtid-resume case: interrupt a --gtid migration mid-copy via the
  interactive 'panic' command, then --resume, exercising the GTID
  checkpoint round-trip (WriteCheckpoint persists the GTID set,
  ReadLastCheckpoint parses it back via NewGTIDBinlogCoordinates(flavor)).
  Passes across the MySQL, MariaDB and Percona CI matrix.

Verified: full localtests suite passes on MySQL 5.7/8.0/8.4 and on the
MariaDB matrix (10.5, 10.6, 10.11, 11.4, 11.8) with gtid_strict_mode on.
Bumps the go_modules group with 1 update in the / directory: [go.opentelemetry.io/otel/sdk](https://github.com/open-telemetry/opentelemetry-go).


Updates `go.opentelemetry.io/otel/sdk` from 1.21.0 to 1.43.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.21.0...v1.43.0)

---
updated-dependencies:
- dependency-name: go.opentelemetry.io/otel/sdk
  dependency-version: 1.43.0
  dependency-type: indirect
  dependency-group: go_modules
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 7.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v4...v7)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-version: '5'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3 to 4.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@v3...v4)

---
updated-dependencies:
- dependency-name: github/codeql-action
  dependency-version: '4'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
…actions/checkout-5

Bump actions/checkout from 4 to 7
…github/codeql-action-4

Bump github/codeql-action from 3 to 4
…odules-5f729b4834

Bump go.opentelemetry.io/otel/sdk from 1.21.0 to 1.43.0 in the go_modules group across 1 directory
- Upgrade dependencies to address security vulnerabilities.
- Testcontainers v0.42.0 introduced a port API change as part of the Moby module migration which requires a code change in `test_utils.go`
Update the module, Docker build/test images, and local Go bootstrap script
to use Go 1.25.12, which includes fixes for the addressed Go CVEs.
Upgrade testcontainers to v0.43 and `x/crypto` to v0.54
go/logic/test_utils.go has no _test.go suffix, so it is part of the
ordinary build of package logic. It imports testcontainers-go, and
package logic is imported by go/cmd/gh-ost, so testcontainers and its
transitive dependencies are compiled into the released gh-ost binary
and recorded in its build info.

Container-testing infrastructure has no role at runtime. Shipping it
enlarges the binary and widens the dependency surface that scanners
report against, which is how the vendored Docker client came to
account for 3 HIGH CVEs in the v1.1.10 binary (github#1738). Upgrading
testcontainers in github#1741 cleared those particular findings, but the
structural issue remains: a future advisory anywhere in the
container-testing tree would again surface in scans of a binary that
never calls into it.

Rename the file to test_utils_test.go so it stays available to the
tests in the same package while being excluded from the ordinary
build. Every identifier it declares is referenced only from
applier_test.go, streamer_test.go and migrator_test.go, so this is a
pure rename with no content change.

Effect on the linux/amd64 binary, built with go1.25.12:
  - modules recorded in build info: 63 -> 22
  - binary size: 18.64 MB -> 16.99 MB
  - Trivy HIGH/CRITICAL: 0 -> 0, unchanged; master is already clean
…ess-high-cves

refactor(logic): scope testcontainers helpers to the test build
Co-authored-by: Eric Yan <ericyan@github.com>
docs: clarify replica selection requirements for --throttle-control-replicas
Add --analyze-ghost-table-before-cutover. When set, cutOver() runs an
explicit ANALYZE TABLE on the ghost table after the postpone gate releases
— before atomicCutOver() takes the source lock and before --test-on-replica
stops replication — logs the elapsed milliseconds on success, and aborts
the migration (fatal) if the ANALYZE fails, rather than swap in a table
with stale InnoDB statistics.

The abort exits synchronously (Log.Fatale), not via a retriable return — a
plain return re-runs cutOver() and the ANALYZE up to --default-retries.
Because ANALYZE TABLE reports table-level failures (missing table,
storage-engine errors) as Msg_type Error rows in its result set while
succeeding at the protocol level, the result rows are inspected and
cut-over is refused unless ANALYZE reports status OK with no Error rows;
privilege-style failures surface as statement errors on the same abort
path.

Without this, a freshly swapped table can briefly serve traffic with a
near-zero row estimate, which the optimizer may cost as a free full scan on
hot query paths, flipping plans until statistics are recomputed. Issue
github#1418 / PR github#1419 propose an ANALYZE for the same reason; this variant
corrects two defects there — the ANALYZE runs after the postpone gate (so a
postponed cut-over still gets fresh statistics) and a failed ANALYZE aborts
instead of being ignored. Opt-in, matching the maintainers' ask on github#1419
(ANALYZE cost grows with partition count, and the statement replicates).

The result-row inspection is extracted as classifyAnalyzeTableResult, a
pure, DB-free function, and covered by:
- TestClassifyAnalyzeTableResult: a table test over status-OK, case
  folding, an error row (alone and alongside a status-OK row), a status-OK
  row followed by a later error row (rows are scanned fully, not
  short-circuited), a non-OK status, and an empty result. Each refusal
  asserts the underlying cause via ErrorContains.
- ApplierTestSuite.TestAnalyzeGhostTable (real MySQL): happy path; the
  fail-open regression (dropping the ghost table makes ANALYZE return an
  Error row with no statement error, which the row inspection must refuse);
  and the statement-error branch (a closed connection is refused via the
  distinct error path).

Also fixes a pre-existing suite bug surfaced while adding the test above:
testify's suite runner calls TearDownSuite() (capital D), but the applier,
migrator, and streamer suites all spelled it TeardownSuite(), so the method
never matched the interface and the MySQL testcontainer was never
terminated. Renamed in all three suites.

Co-authored-by: wangzihuacool <wangzihuacool@163.com>
Signed-off-by: Vandhana Selvaprakash <vandhana.selvaprakash@airtable.com>
- applier: name the AnalyzeGhostTable receiver `apl`, consistent with the
  rest of applier.go (staticcheck ST1016) — fixes the golangci-lint failure.
- migrator: extract the pre-cut-over ANALYZE gating into
  analyzeGhostTableBeforeCutOver(analyze func() error), a narrow injectable
  seam (behavior unchanged), so the orchestration contract is unit-testable
  without a live applier or the process-exiting Log.Fatale path.
- migrator test: TestAnalyzeGhostTableBeforeCutOver covers the flag gating,
  the happy path (ANALYZE runs once), and fail-closed (a failed ANALYZE
  propagates so cut-over aborts before replica-stop / cut-over locking).
- localtest: analyze-ghost-table-before-cutover exercises the flag end-to-end
  against live MySQL with ongoing DML.

Signed-off-by: Vandhana Selvaprakash <vandhana.selvaprakash@airtable.com>
…upstream-analyze-cutover

Add opt-in --analyze-ghost-table-before-cutover
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4 to 7.
- [Release notes](https://github.com/actions/upload-artifact/releases)
- [Commits](actions/upload-artifact@v4...v7)

---
updated-dependencies:
- dependency-name: actions/upload-artifact
  dependency-version: '5'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
…actions/upload-artifact-5

Bump actions/upload-artifact from 4 to 7
Signed-off-by: vladdoster <vladislav.doster@icloud.com>
ericyan and others added 13 commits August 12, 2026 13:43
The binlog reader updates its current coordinates while streaming, while go-mysql's BinlogSyncer retains and mutates the GTID set passed to StartSyncGTID.

Clone the caller-provided coordinates for the reader and clone the GTID set again for the syncer. This prevents both components from mutating the caller's reconnect coordinates and eliminates concurrent access to the same MysqlGTIDSet.
The retry-hook test redirected os.Stdout and os.Stderr to capture status
and hook output. Migrate has background status and logging goroutines,
so reassigning those process-global streams races with their output.

Inject a status writer into Migrator, retaining stdout as the default.
The test supplies thread-safe buffers for the outputs, preserving the
separate stdout and stderr assertions without mutating global state.
Fix data races and enable race detector in CI
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…sion-topology

Fix mixed version topology discovery
…te-migration-key

Force migration key for composite chunk boundaries
@coding-chimp coding-chimp self-assigned this Sep 22, 2026
@coding-chimp
coding-chimp added this pull request to stack #58 September 22, 2026 07:59
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.

9 participants