From 93dedde365455ac271a2fe7b96fb331e44a8f6dc Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Sun, 10 Nov 2024 13:44:38 +0200 Subject: [PATCH] only ANALYZE once per migration Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- .../scheduler/onlineddl_scheduler_test.go | 11 +++++++++ .../onlineddl/vrepl/onlineddl_vrepl_test.go | 1 + .../schema/onlineddl/schema_migrations.sql | 1 + go/vt/vttablet/onlineddl/executor.go | 23 +++++++++++-------- go/vt/vttablet/onlineddl/schema.go | 3 +++ 5 files changed, 30 insertions(+), 9 deletions(-) diff --git a/go/test/endtoend/onlineddl/scheduler/onlineddl_scheduler_test.go b/go/test/endtoend/onlineddl/scheduler/onlineddl_scheduler_test.go index b7e18e9af7c..815db8de056 100644 --- a/go/test/endtoend/onlineddl/scheduler/onlineddl_scheduler_test.go +++ b/go/test/endtoend/onlineddl/scheduler/onlineddl_scheduler_test.go @@ -562,6 +562,16 @@ func testScheduler(t *testing.T) { status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, t1uuid, normalWaitTime, schema.OnlineDDLStatusRunning) fmt.Printf("# Migration status (for debug purposes): <%s>\n", status) }) + + t.Run("wait for ready_to_complete", func(t *testing.T) { + waitForReadyToComplete(t, t1uuid, true) + rs := onlineddl.ReadMigrations(t, &vtParams, t1uuid) + require.NotNil(t, rs) + for _, row := range rs.Named().Rows { + assert.True(t, row["vrepl_analyzed_timestamp"].IsNull()) + } + }) + t.Run("check postpone_completion", func(t *testing.T) { rs := onlineddl.ReadMigrations(t, &vtParams, t1uuid) require.NotNil(t, rs) @@ -582,6 +592,7 @@ func testScheduler(t *testing.T) { for _, row := range rs.Named().Rows { postponeCompletion := row.AsInt64("postpone_completion", 0) assert.Equal(t, int64(0), postponeCompletion) + assert.False(t, row["vrepl_analyzed_timestamp"].IsNull()) } }) }) diff --git a/go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go b/go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go index 161b1566680..59850fb25c8 100644 --- a/go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go +++ b/go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go @@ -304,6 +304,7 @@ func TestVreplSchemaChanges(t *testing.T) { for _, row := range rs.Named().Rows { retainArtifactSeconds := row.AsInt64("retain_artifacts_seconds", 0) assert.Equal(t, int64(-1), retainArtifactSeconds) + assert.False(t, row["vrepl_analyzed_timestamp"].IsNull()) } }) t.Run("successful online alter, vtctl", func(t *testing.T) { diff --git a/go/vt/sidecardb/schema/onlineddl/schema_migrations.sql b/go/vt/sidecardb/schema/onlineddl/schema_migrations.sql index 85d87edefc2..64225935ed4 100644 --- a/go/vt/sidecardb/schema/onlineddl/schema_migrations.sql +++ b/go/vt/sidecardb/schema/onlineddl/schema_migrations.sql @@ -72,6 +72,7 @@ CREATE TABLE IF NOT EXISTS schema_migrations `is_immediate_operation` tinyint unsigned NOT NULL DEFAULT '0', `reviewed_timestamp` timestamp NULL DEFAULT NULL, `ready_to_complete_timestamp` timestamp NULL DEFAULT NULL, + `vrepl_analyzed_timestamp` timestamp NULL DEFAULT NULL, `removed_foreign_key_names` text NOT NULL, `last_cutover_attempt_timestamp` timestamp NULL DEFAULT NULL, `force_cutover` tinyint unsigned NOT NULL DEFAULT '0', diff --git a/go/vt/vttablet/onlineddl/executor.go b/go/vt/vttablet/onlineddl/executor.go index 5fb173798d6..a4b13669ff0 100644 --- a/go/vt/vttablet/onlineddl/executor.go +++ b/go/vt/vttablet/onlineddl/executor.go @@ -880,10 +880,11 @@ func (e *Executor) cutOverVReplMigration(ctx context.Context, s *VReplStream, sh } // information about source tablet - onlineDDL, _, err := e.readMigration(ctx, s.workflow) + onlineDDL, row, err := e.readMigration(ctx, s.workflow) if err != nil { return vterrors.Wrapf(err, "cutover: failed reading migration") } + needsVreplTableAnalysis := row["vrepl_analyzed_timestamp"].IsNull() isVreplicationTestSuite := onlineDDL.StrategySetting().IsVreplicationTestSuite() e.updateMigrationStage(ctx, onlineDDL.UUID, "starting cut-over") @@ -957,15 +958,19 @@ func (e *Executor) cutOverVReplMigration(ctx context.Context, s *VReplStream, sh } defer preparationConnRestoreLockWaitTimeout() - // Run `ANALYZE TABLE` on the vreplication table so that it has up-to-date statistics at cut-over - parsed := sqlparser.BuildParsedQuery(sqlAnalyzeTable, vreplTable) - if _, err := preparationsConn.Conn.Exec(ctx, parsed.Query, -1, false); err != nil { - return vterrors.Wrapf(err, "failed ANALYZE shadow table") + if needsVreplTableAnalysis { + // Run `ANALYZE TABLE` on the vreplication table so that it has up-to-date statistics at cut-over + parsed := sqlparser.BuildParsedQuery(sqlAnalyzeTable, vreplTable) + if _, err := preparationsConn.Conn.Exec(ctx, parsed.Query, -1, false); err != nil { + // Best effort only. Do not fail the mgiration if this fails. + _ = e.updateMigrationMessage(ctx, "failed ANALYZE shadow table", s.workflow) + } else { + _ = e.updateMigrationTimestamp(ctx, "vrepl_analyzed_timestamp", s.workflow) + } + // This command will have blocked the table for writes, presumably only for a brief time. But this can cause + // vreplication to now lag. Thankfully we're gonna create the sentry table and waitForPos. } - // This command will have blocked the table for writes, presumably only for a brief time. But this can cause - // vreplication to now lag. Thankfully we're gonna create the sentry table and waitForPos. - - parsed = sqlparser.BuildParsedQuery(sqlCreateSentryTable, sentryTableName) + parsed := sqlparser.BuildParsedQuery(sqlCreateSentryTable, sentryTableName) if _, err := preparationsConn.Conn.Exec(ctx, parsed.Query, 1, false); err != nil { return vterrors.Wrapf(err, "failed creating sentry table") } diff --git a/go/vt/vttablet/onlineddl/schema.go b/go/vt/vttablet/onlineddl/schema.go index a30ab6b3ed9..905940f6f75 100644 --- a/go/vt/vttablet/onlineddl/schema.go +++ b/go/vt/vttablet/onlineddl/schema.go @@ -270,6 +270,7 @@ const ( cancelled_timestamp=NULL, completed_timestamp=NULL, last_cutover_attempt_timestamp=NULL, + vrepl_analyzed_timestamp=NULL, cleanup_timestamp=NULL WHERE migration_status IN ('failed', 'cancelled') @@ -291,6 +292,7 @@ const ( cancelled_timestamp=NULL, completed_timestamp=NULL, last_cutover_attempt_timestamp=NULL, + vrepl_analyzed_timestamp=NULL, cleanup_timestamp=NULL WHERE migration_status IN ('failed', 'cancelled') @@ -441,6 +443,7 @@ const ( postpone_launch, postpone_completion, is_immediate_operation, + vrepl_analyzed_timestamp, reviewed_timestamp FROM _vt.schema_migrations WHERE