Online DDL: resume vreplication after cut-over/RENAME failure #18428

shlomi-noach · 2025-07-08T07:39:27Z

Description

In Online DDL cut-over, and once vreplication is stopped just before renaming the tables, ensure to restart vreplication in case the cut-over ends up failing (specifically there is one remaining operation, the RENAME).

Another thing in this PR: when canceling a vreplication migration (can be due to user's explicit CANCEL, or due to failure), do not delete the vreplication entry, as it is useful for debugging/analysis purposes.

Backport

This is a bug that needs to be backported to supported versions.

Related Issue(s)

Fixes #18427

Checklist

"Backport to:" labels have been added if this change should be back-ported to release branches
If this change is to be back-ported to previous releases, a justification is included in the PR description
Tests were added or are not required
Did the new or modified tests pass consistently locally and on CI?
Documentation was added or is not required

Deployment Notes

Signed-off-by: Shlomi Noach <[email protected]>

vitess-bot · 2025-07-08T07:39:30Z

shlomi-noach · 2025-07-08T07:40:09Z

go/vt/vttablet/onlineddl/executor.go

 					cancellable = append(cancellable, newCancellableMigration(uuid, s.message))
 				}
 				if !s.isRunning() {
+					log.Infof("migration %s in 'running' state but vreplication state is '%s'", uuid, s.state.String())


Just some added visibility.

shlomi-noach · 2025-07-08T08:08:12Z

~~onlineddl_scheduler failing tests are related, looking into.~~ solved

codecov · 2025-07-08T08:11:36Z

Codecov Report

Attention: Patch coverage is 0% with 26 lines in your changes missing coverage. Please review.

Project coverage is 67.50%. Comparing base (4fbab4f) to head (b180f6f).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
go/vt/vttablet/onlineddl/executor.go	0.00%	26 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #18428      +/-   ##
==========================================
+ Coverage   67.49%   67.50%   +0.01%     
==========================================
  Files        1607     1607              
  Lines      262741   262791      +50     
==========================================
+ Hits       177339   177407      +68     
+ Misses      85402    85384      -18

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…entry can exist on RETRYing a failed migration) Signed-off-by: Shlomi Noach <[email protected]>

Signed-off-by: Shlomi Noach <[email protected]>

harshit-gangal · 2025-07-09T14:41:12Z

Add the backport labels

harshit-gangal · 2025-07-09T14:43:13Z

go/vt/vttablet/onlineddl/executor.go

+			if err := e.startVreplication(ctx, tablet.Tablet, s.workflow); err != nil {
+				log.Errorf("cutOverVReplMigration %v: failed restarting vreplication after cutover failure: %v", s.workflow, err)
+			}
+			go log.Infof("cutOverVReplMigration %v: started vreplication after cutover failure", s.workflow)


why to start a goroutine for calling in log function?

because in this critical section I don't want to block on writing a log line, as silly as it may be.

@shlomi-noach Is it then essential to log at all? Starting a goroutine also has some cost 😄.

@dbussink , yeah, I'd like this entry to appear in the logs. If it appears, it means vreplication has definitely started, which is an important info

This will not be the only go log.Infof call in the cutOverVReplMigration function. There's a bunch of other such calls.

harshit-gangal

one comment. rest looks good

go/vt/vttablet/onlineddl/executor.go

Co-authored-by: Noble Mittal <[email protected]> Signed-off-by: Shlomi Noach <[email protected]>

Signed-off-by: Shlomi Noach <[email protected]>

Signed-off-by: Shlomi Noach <[email protected]> Co-authored-by: Noble Mittal <[email protected]>

…failure (#18428) (#18437) Signed-off-by: Shlomi Noach <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Noble Mittal <[email protected]>

…failure (#18428) (#18436) Signed-off-by: Shlomi Noach <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Noble Mittal <[email protected]>

* origin/master: bugfix: Fix impossible query for UNION (vitessio#18463) fix topo use in local_example (vitessio#18357) fix: update go-upgrade tool to check patch number (vitessio#18252) (vitessio#18402) Update MAINTAINERS.md and CODEOWNERS (vitessio#18462) Add logging to binlog watcher actions (vitessio#18264) `schemadiff`: `RelatedForeignKeyTables()` (vitessio#18195) `vtorc`: allow recoveries to be disabled from startup (vitessio#18005) Fix `vttablet` not being marked as not serving when MySQL stalls (vitessio#17883) make xtrabackup ShouldDrainForBackup configurable (vitessio#18431) Reset in-memory sequence info on vttablet on UpdateSequenceTables request (vitessio#18415) Fix watcher storm during topo outages (vitessio#18434) Online DDL: resume vreplication after cut-over/RENAME failure (vitessio#18428) Online DDL cutover enhancements (vitessio#18423) VStreamer: change in filter logic (vitessio#18319) Online DDL metrics: `OnlineDDLStaleMigrationMinutes` (vitessio#18417) Signed-off-by: Morgan Tocker <[email protected]>

…failure (vitessio#18428) (vitessio#18437) Signed-off-by: Shlomi Noach <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Noble Mittal <[email protected]>

Online DDL: resume vreplication after cut-over/RENAME failure

474fb29

Signed-off-by: Shlomi Noach <[email protected]>

shlomi-noach requested a review from a team July 8, 2025 07:39

shlomi-noach requested review from mattlord, notfelineit and rohit-nayak-ps as code owners July 8, 2025 07:39

shlomi-noach added Type: Bug Component: Online DDL Online DDL (vitess/native/gh-ost/pt-osc) labels Jul 8, 2025

github-actions bot added this to the v23.0.0 milestone Jul 8, 2025

shlomi-noach commented Jul 8, 2025

View reviewed changes

shlomi-noach added 2 commits July 8, 2025 12:14

ensure to remvoe vreplication entry when starting a vrepl migration (…

67de091

…entry can exist on RETRYing a failed migration) Signed-off-by: Shlomi Noach <[email protected]>

typo

b180f6f

Signed-off-by: Shlomi Noach <[email protected]>

rohit-nayak-ps requested a review from beingnoble03 July 8, 2025 12:14

harshit-gangal reviewed Jul 9, 2025

View reviewed changes

harshit-gangal approved these changes Jul 9, 2025

View reviewed changes

shlomi-noach added Backport to: release-21.0 Backport to: release-22.0 Needs to be backport to release-22.0 labels Jul 9, 2025

beingnoble03 approved these changes Jul 9, 2025

View reviewed changes

go/vt/vttablet/onlineddl/executor.go Outdated Show resolved Hide resolved

Update go/vt/vttablet/onlineddl/executor.go

b337478

Co-authored-by: Noble Mittal <[email protected]> Signed-off-by: Shlomi Noach <[email protected]>

fix referencing call

bffd76f

Signed-off-by: Shlomi Noach <[email protected]>

shlomi-noach merged commit 3baf4bc into vitessio:main Jul 10, 2025
101 of 106 checks passed

shlomi-noach deleted the onlineddl-cutover-start-vrepl branch July 10, 2025 08:10

vitess-bot pushed a commit that referenced this pull request Jul 10, 2025

Online DDL: resume vreplication after cut-over/RENAME failure (#18428)

c3004ce

Signed-off-by: Shlomi Noach <[email protected]> Co-authored-by: Noble Mittal <[email protected]>

vitess-bot bot mentioned this pull request Jul 10, 2025

[release-21.0] Online DDL: resume vreplication after cut-over/RENAME failure (#18428) #18436

Merged

vitess-bot pushed a commit that referenced this pull request Jul 10, 2025

Online DDL: resume vreplication after cut-over/RENAME failure (#18428)

deb0020

Signed-off-by: Shlomi Noach <[email protected]> Co-authored-by: Noble Mittal <[email protected]>

vitess-bot bot mentioned this pull request Jul 10, 2025

[release-22.0] Online DDL: resume vreplication after cut-over/RENAME failure (#18428) #18437

Merged

Online DDL: resume vreplication after cut-over/RENAME failure #18428

Online DDL: resume vreplication after cut-over/RENAME failure #18428

Uh oh!

Conversation

shlomi-noach commented Jul 8, 2025

Description

Backport

Related Issue(s)

Checklist

Deployment Notes

Uh oh!

vitess-bot bot commented Jul 8, 2025

Review Checklist

General

Tests

Documentation

New flags

If a workflow is added or modified:

Backward compatibility

Uh oh!

shlomi-noach Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

shlomi-noach commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

harshit-gangal commented Jul 9, 2025

Uh oh!

harshit-gangal Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

shlomi-noach Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

dbussink Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

shlomi-noach Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

shlomi-noach Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

harshit-gangal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

shlomi-noach commented Jul 8, 2025 •

edited

Loading

codecov bot commented Jul 8, 2025 •

edited

Loading