From 5b95da825c9cea95be063974d6a210d9974ce3d0 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 22 Aug 2024 13:03:22 +0300 Subject: [PATCH 1/2] Online DDL: negative value for --force-cut-over-after DDL strategy flag means 'immediately' Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schema/ddl_strategy_test.go | 7 +++++++ go/vt/vttablet/onlineddl/executor.go | 2 +- go/vt/vttablet/onlineddl/executor_test.go | 24 +++++++++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/go/vt/schema/ddl_strategy_test.go b/go/vt/schema/ddl_strategy_test.go index f27f0963e80..c394907b98a 100644 --- a/go/vt/schema/ddl_strategy_test.go +++ b/go/vt/schema/ddl_strategy_test.go @@ -328,6 +328,13 @@ func TestParseDDLStrategy(t *testing.T) { runtimeOptions: "", forceCutOverAfter: 3 * time.Minute, }, + { + strategyVariable: "vitess --force-cut-over-after=-1ms", + strategy: DDLStrategyVitess, + options: "--force-cut-over-after=-1ms", + runtimeOptions: "", + forceCutOverAfter: -1 * time.Millisecond, + }, { strategyVariable: "vitess --force-cut-over-after=r3m", strategy: DDLStrategyVitess, diff --git a/go/vt/vttablet/onlineddl/executor.go b/go/vt/vttablet/onlineddl/executor.go index db73f67ed64..9ea805bcca4 100644 --- a/go/vt/vttablet/onlineddl/executor.go +++ b/go/vt/vttablet/onlineddl/executor.go @@ -3794,7 +3794,7 @@ func shouldCutOverAccordingToBackoff( // is beyond the --force-cut-over-after setting, or the column `force_cutover` is "1", and this means: // - we do not want to backoff, we want to cutover asap // - we agree to brute-force KILL any pending queries on the migrated table so as to ensure it's unlocked. - if forceCutOverAfter > 0 && sinceReadyToComplete > forceCutOverAfter { + if forceCutOverAfter != 0 && sinceReadyToComplete > forceCutOverAfter { // time since migration was ready to complete is beyond the --force-cut-over-after setting return true, true } diff --git a/go/vt/vttablet/onlineddl/executor_test.go b/go/vt/vttablet/onlineddl/executor_test.go index 1dc5447bbb9..10799a7de0e 100644 --- a/go/vt/vttablet/onlineddl/executor_test.go +++ b/go/vt/vttablet/onlineddl/executor_test.go @@ -481,6 +481,30 @@ func TestShouldCutOverAccordingToBackoff(t *testing.T) { expectShouldCutOver: false, expectShouldForceCutOver: false, }, + { + name: "backoff; cutover-after still not in effect yet", + cutoverAttempts: 3, + forceCutOverAfter: time.Second, + sinceReadyToComplete: 0, + expectShouldCutOver: false, + expectShouldForceCutOver: false, + }, + { + name: "backoff; cutover-after still not in effect yet", + cutoverAttempts: 3, + forceCutOverAfter: 0, + sinceReadyToComplete: 0, + expectShouldCutOver: false, + expectShouldForceCutOver: false, + }, + { + name: "backoff; cutover-after still not in effect yet", + cutoverAttempts: 3, + forceCutOverAfter: -time.Millisecond, + sinceReadyToComplete: 0, + expectShouldCutOver: true, + expectShouldForceCutOver: true, + }, { name: "cutover-after overrides backoff", cutoverAttempts: 3, From 5e369651dff982f0148c55a1cf3b3cdb87f0dd63 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 22 Aug 2024 13:26:49 +0300 Subject: [PATCH 2/2] Evaluate microseconds_since_ready_to_complete Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/vttablet/onlineddl/executor.go | 4 ++-- go/vt/vttablet/onlineddl/executor_test.go | 18 +++++++++++++----- go/vt/vttablet/onlineddl/schema.go | 2 +- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/go/vt/vttablet/onlineddl/executor.go b/go/vt/vttablet/onlineddl/executor.go index 9ea805bcca4..a432581b3cd 100644 --- a/go/vt/vttablet/onlineddl/executor.go +++ b/go/vt/vttablet/onlineddl/executor.go @@ -3794,7 +3794,7 @@ func shouldCutOverAccordingToBackoff( // is beyond the --force-cut-over-after setting, or the column `force_cutover` is "1", and this means: // - we do not want to backoff, we want to cutover asap // - we agree to brute-force KILL any pending queries on the migrated table so as to ensure it's unlocked. - if forceCutOverAfter != 0 && sinceReadyToComplete > forceCutOverAfter { + if forceCutOverAfter > 0 && sinceReadyToComplete > forceCutOverAfter { // time since migration was ready to complete is beyond the --force-cut-over-after setting return true, true } @@ -3845,7 +3845,7 @@ func (e *Executor) reviewRunningMigrations(ctx context.Context) (countRunnning i uuid := row["migration_uuid"].ToString() cutoverAttempts := row.AsInt64("cutover_attempts", 0) sinceLastCutoverAttempt := time.Second * time.Duration(row.AsInt64("seconds_since_last_cutover_attempt", 0)) - sinceReadyToComplete := time.Second * time.Duration(row.AsInt64("seconds_since_ready_to_complete", 0)) + sinceReadyToComplete := time.Microsecond * time.Duration(row.AsInt64("microseconds_since_ready_to_complete", 0)) onlineDDL, migrationRow, err := e.readMigration(ctx, uuid) if err != nil { return countRunnning, cancellable, err diff --git a/go/vt/vttablet/onlineddl/executor_test.go b/go/vt/vttablet/onlineddl/executor_test.go index 10799a7de0e..d5e7f635a19 100644 --- a/go/vt/vttablet/onlineddl/executor_test.go +++ b/go/vt/vttablet/onlineddl/executor_test.go @@ -482,7 +482,7 @@ func TestShouldCutOverAccordingToBackoff(t *testing.T) { expectShouldForceCutOver: false, }, { - name: "backoff; cutover-after still not in effect yet", + name: "zero since ready", cutoverAttempts: 3, forceCutOverAfter: time.Second, sinceReadyToComplete: 0, @@ -490,7 +490,7 @@ func TestShouldCutOverAccordingToBackoff(t *testing.T) { expectShouldForceCutOver: false, }, { - name: "backoff; cutover-after still not in effect yet", + name: "zero since read, zero cut-over-after", cutoverAttempts: 3, forceCutOverAfter: 0, sinceReadyToComplete: 0, @@ -498,13 +498,21 @@ func TestShouldCutOverAccordingToBackoff(t *testing.T) { expectShouldForceCutOver: false, }, { - name: "backoff; cutover-after still not in effect yet", + name: "microsecond", cutoverAttempts: 3, - forceCutOverAfter: -time.Millisecond, - sinceReadyToComplete: 0, + forceCutOverAfter: time.Microsecond, + sinceReadyToComplete: time.Millisecond, expectShouldCutOver: true, expectShouldForceCutOver: true, }, + { + name: "microsecond, not ready", + cutoverAttempts: 3, + forceCutOverAfter: time.Millisecond, + sinceReadyToComplete: time.Microsecond, + expectShouldCutOver: false, + expectShouldForceCutOver: false, + }, { name: "cutover-after overrides backoff", cutoverAttempts: 3, diff --git a/go/vt/vttablet/onlineddl/schema.go b/go/vt/vttablet/onlineddl/schema.go index 28e32e7dab4..c4c26aa52fd 100644 --- a/go/vt/vttablet/onlineddl/schema.go +++ b/go/vt/vttablet/onlineddl/schema.go @@ -304,7 +304,7 @@ const ( postpone_completion, force_cutover, cutover_attempts, - ifnull(timestampdiff(second, ready_to_complete_timestamp, now()), 0) as seconds_since_ready_to_complete, + ifnull(timestampdiff(microsecond, ready_to_complete_timestamp, now(6)), 0) as microseconds_since_ready_to_complete, ifnull(timestampdiff(second, last_cutover_attempt_timestamp, now()), 0) as seconds_since_last_cutover_attempt, timestampdiff(second, started_timestamp, now()) as elapsed_seconds FROM _vt.schema_migrations