Skip to content

Rebase #3450 Use destination master for comparison during split clone#4331

Merged
sougou merged 2 commits intovitessio:masterfrom
planetscale:tirsen-use-dest-master-for-split-clone
Nov 9, 2018
Merged

Rebase #3450 Use destination master for comparison during split clone#4331
sougou merged 2 commits intovitessio:masterfrom
planetscale:tirsen-use-dest-master-for-split-clone

Conversation

@deepthi
Copy link
Copy Markdown
Collaborator

@deepthi deepthi commented Oct 31, 2018

After rebase, made the following changes:

  1. set attempt := 1 in nextWithRetries
  2. change worker test to stop master while the copy is running instead of before starting SplitClone
  3. Fix bug in cancelVerticalResharding that was causing vertical_split tests to fail with this change

@tirsen Please review

Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
@deepthi deepthi requested a review from sougou October 31, 2018 23:18
@sougou
Copy link
Copy Markdown
Contributor

sougou commented Nov 4, 2018

@rafael @tirsen: @deepthi has fixed up this old PR.
Can you eyeball this before I merge?

@tirsen
Copy link
Copy Markdown
Collaborator

tirsen commented Nov 5, 2018

@mpawliszyn Can you have a look too. I think you've been working on the retries here as well right?

@tirsen
Copy link
Copy Markdown
Collaborator

tirsen commented Nov 5, 2018

I'm guessing you've only changed test code not production code? In that case LGTM, otherwise could you point to any production code you changed?

@deepthi
Copy link
Copy Markdown
Collaborator Author

deepthi commented Nov 5, 2018

@tirsen
here are the changes from your PR:

  1. restartable_result_reader.go:nextWithRetries
    attempt := 1. You had changed this to attempt := 0
  2. executor.go:checkError
    - case errNo == "2002" || errNo == "2006" || errNo == "2013":
    + case errNo == "2002" || errNo == "2006" || errNo == "2013" || errNo == "1053":
    This is to handle a destination master mysql going down and coming back up while SplitClone is running

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.

3 participants