Skip to content

Add opt-in --analyze-ghost-table-before-cutover - #1747

Open
VandhanaSelvaprakash-at wants to merge 1 commit into
github:masterfrom
VandhanaSelvaprakash-at:vaselvap-upstream-analyze-cutover
Open

Add opt-in --analyze-ghost-table-before-cutover#1747
VandhanaSelvaprakash-at wants to merge 1 commit into
github:masterfrom
VandhanaSelvaprakash-at:vaselvap-upstream-analyze-cutover

Conversation

@VandhanaSelvaprakash-at

Copy link
Copy Markdown

Supersedes #1419 (original approach and credit to @wangzihuacool) and closes the gap discussed in #1418. Thanks @ericyan / @timvaillancourt for the go-ahead.

What

Adds an opt-in --analyze-ghost-table-before-cutover flag (default off). When set, gh-ost runs an explicit ANALYZE TABLE on the ghost table immediately before cut-over, so the freshly swapped table doesn't briefly serve traffic with near-zero InnoDB row estimates — which the optimizer can cost as a free full scan, flipping plans on hot paths until statistics recompute.

Why opt-in

Per @shaohk and @timvaillancourt on #1419: on partitioned tables ANALYZE cost grows with partition count and the statement replicates. So it's gated behind a flag, default off, intended for small non-partitioned tables — opt-in users can report on performance.

How it differs from #1419

  1. Runs after the postpone gate releases (before the source lock and before --test-on-replica stops replication) — a postponed cut-over doesn't re-stale its statistics before the swap.
  2. A failed ANALYZE aborts the migration (fatal), fail-closed. ANALYZE TABLE surfaces table-level failures (missing table, storage-engine errors) as Msg_type=Error result rows while the statement succeeds at the protocol level, so the rows are inspected and cut-over is refused unless status is OK with no Error rows.

Tests

  • TestClassifyAnalyzeTableResult — DB-free table test of the result-row classifier (status-OK, case folding, error rows, full-scan ordering, non-OK status, empty result).
  • ApplierTestSuite.TestAnalyzeGhostTable — real-MySQL test covering the happy path, the fail-open regression, and the statement-error branch.

Also folds in a separable pre-existing fix: TeardownSuiteTearDownSuite across the applier/migrator/streamer suites (testify never invoked the misspelled method, leaking a testcontainer per suite). Happy to split it into its own PR if you'd prefer.

DCO signed off.

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>
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.

1 participant