Skip to content

vtctl: add new BackupShard cmd#4320

Merged
sougou merged 2 commits intovitessio:masterfrom
derekperkins:backup-shard
Oct 29, 2018
Merged

vtctl: add new BackupShard cmd#4320
sougou merged 2 commits intovitessio:masterfrom
derekperkins:backup-shard

Conversation

@derekperkins
Copy link
Copy Markdown
Member

@derekperkins derekperkins commented Oct 28, 2018

This chooses the most up to date tablet in a shard to run a backup on.

@derekperkins
vtctl: add new BackupShard cmd
d665159 
chooses the most up to date tablet to run the backup on

Signed-off-by: Derek Perkins <derek@derekperkins.com>
sougou
sougou reviewed Oct 28, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@sougou sougou left a comment

Choose a reason for hiding this comment

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

I think this makes sense. Let's have one other set of eyeballs. @rafael?

@derekperkins derekperkins requested a review from rafael October 28, 2018 23:29
@rafael
Copy link
Copy Markdown
Member

rafael commented Oct 29, 2018 •
edited
Loading

One question. Now that:

{"Backup", commandBackup,
  "[-concurrency=4] <tablet alias>",
  "Stops mysqld and uses the BackupStorage service to store a new backup. This function also remembers if the tablet was replicating so that it can restore the same state after the backup completes."},

Is registered in go/vt/vtctl/backup.go, should we remove from go/vt/vtctl/vtctl.go?

@derekperkins
Copy link
Copy Markdown
Member Author

derekperkins commented Oct 29, 2018

I moved it in the second commit.

@derekperkins
Copy link
Copy Markdown
Member Author

derekperkins commented Oct 29, 2018

... or at least I thought I did. Just a sec

@derekperkins
vtctl: move commandBackup to backup.go
8c0ec16 
Signed-off-by: Derek Perkins <derek@derekperkins.com>
@derekperkins
Copy link
Copy Markdown
Member Author

derekperkins commented Oct 29, 2018

Ok, it's now not double registered anymore.

rafael
rafael approved these changes Oct 29, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@rafael rafael left a comment

Choose a reason for hiding this comment

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

LGTM

@sougou
Copy link
Copy Markdown
Contributor

sougou commented Oct 29, 2018

The more I thought about this, the more I like this.

@sougou sougou merged commit ee89e01 into vitessio:master Oct 29, 2018
@derekperkins derekperkins deleted the backup-shard branch November 9, 2018 18:47
@rafael rafael mentioned this pull request Dec 3, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rafael rafael rafael approved these changes

+1 more reviewer

@sougou sougou sougou left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@derekperkins @rafael @sougou