Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Emergency Reparent Refactor #6449

Merged
merged 27 commits into from
Aug 20, 2020
Merged

Emergency Reparent Refactor #6449

merged 27 commits into from
Aug 20, 2020

Conversation

PrismaPhonic
Copy link
Contributor

@PrismaPhonic PrismaPhonic commented Jul 16, 2020 •
edited by deepthi
Loading

This PR executes on all aspects of the EmergencyReparent re-design. Fixes #6206

@PrismaPhonic PrismaPhonic force-pushed the emergency-reparent-refactor branch 22 times, most recently from f84eae3 to c6c7c9d Compare July 22, 2020 20:34
@PrismaPhonic PrismaPhonic changed the title WIP: Emergency Reparent Refactor Emergency Reparent Refactor Jul 23, 2020
@PrismaPhonic PrismaPhonic marked this pull request as ready for review July 23, 2020 02:20
deepthi
deepthi reviewed Jul 23, 2020
View reviewed changes
Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend running go test -cover. Given the complexity of the design and the number of if conditions, unit test coverage seems thin.

go/test/endtoend/reparent/reparent_test.go Outdated Show resolved Hide resolved
go/test/endtoend/reparent/reparent_test.go Show resolved Hide resolved
@PrismaPhonic
Finished strategy steps 1-4 from main refactor issue.
1839e73 
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
@PrismaPhonic
Trivial fixes per review suggestions.
a69e6a0 
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
@PrismaPhonic
Refactor per review suggestion to reduce number of code paths.
6a7d62a 
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
@PrismaPhonic
Record all errors and surface them in the case we could not get a win…
80ef344 
…ning tablet in time.

Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
@PrismaPhonic
Updated waitOnNMinusOneTablets helper function to take a maximum acce…
b231de2 
…ptable number of errors as an argument to accomodate a use case where any number of errors is acceptable.

Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
@PrismaPhonic
Simplify design drastically per review suggestion to synchronously wa…
7e71b59 
…it on handleMaster.

Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
@PrismaPhonic
Fix test issue now that SetMaster calls are just best effort and done…
5f7d2cc 
… on a defer.

Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
deepthi
deepthi reviewed Aug 5, 2020
View reviewed changes
go/test/endtoend/reparent/reparent_test.go Outdated Show resolved Hide resolved
go/vt/wrangler/reparent.go Outdated Show resolved Hide resolved
go/vt/wrangler/reparent.go Outdated Show resolved Hide resolved
@PrismaPhonic
Small fixes per review suggestions.
3af3cf1 
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
@PrismaPhonic
Switched all fmt.Errorf to vterrors.Errorf or Wrapf.
1ccfab4 
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
@PrismaPhonic
Rename to ignoredTablets to match flag.
e166f94 
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
@PrismaPhonic
Significant logic re-write to make all replicas wait for relay logs t…
3a6fc89 
…o apply before running competition.

Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
@PrismaPhonic
Make sure we run competition for most caught up.
96e4843 
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
@PrismaPhonic
Fixed unit tests and refactored logic per pairing session.
fff559f 
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
@PrismaPhonic
Resolved merge conflicts with master.
519c048 
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
deepthi
deepthi previously requested changes Aug 20, 2020
View reviewed changes
Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good except for the few pending comments/changes

go/test/endtoend/reparent/reparent_test.go Outdated Show resolved Hide resolved
go/test/endtoend/reparent/reparent_test.go Outdated Show resolved Hide resolved
go/vt/wrangler/reparent.go Outdated Show resolved Hide resolved
go/vt/wrangler/reparent.go Outdated Show resolved Hide resolved
go/vt/wrangler/reparent.go Show resolved Hide resolved
go/vt/wrangler/reparent.go Show resolved Hide resolved
go/vt/wrangler/reparent.go Outdated Show resolved Hide resolved
@PrismaPhonic
Ensure we use Wrapf for ALL error returns.
d239ff5 
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
@PrismaPhonic
Get rid of unnecessary subcontext.
f445f79 
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
@PrismaPhonic
More error improvement.
dcfe4f1 
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
@PrismaPhonic
Rename per review suggestion.
04a1a60 
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
@PrismaPhonic
Remove InitTablet calls. They are unnecessary now.
c3c24f5 
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
@PrismaPhonic PrismaPhonic dismissed deepthi’s stale review August 20, 2020 20:43

Addressed all concerns

deepthi
deepthi approved these changes Aug 20, 2020
View reviewed changes
Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! LGTM.

@deepthi deepthi merged commit 3230474 into vitessio:master Aug 20, 2020
@deepthi deepthi mentioned this pull request Aug 24, 2020
Closed
@enisoc enisoc deleted the emergency-reparent-refactor branch August 25, 2020 16:31
@deepthi deepthi added this to the v8.0 milestone Sep 22, 2020
@setassociative setassociative mentioned this pull request Mar 5, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deepthi deepthi deepthi approved these changes

@sougou sougou Awaiting requested review from sougou

@systay systay Awaiting requested review from systay

@enisoc enisoc Awaiting requested review from enisoc

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v8.0
Development

Successfully merging this pull request may close these issues.

Make emergency reparents more robust.
3 participants
@PrismaPhonic @deepthi @enisoc