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.

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

Flag for specifying the source tablet type of the binlog player #2822

Merged
merged 1 commit into from
May 19, 2017

Conversation

tirsen
Copy link
Collaborator

@tirsen tirsen commented May 5, 2017

No description provided.

Copy link
Contributor

@michael-berlin michael-berlin left a comment

Choose a reason for hiding this comment

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

LGTM after comment is addressed.

@tirsen
Copy link
Collaborator Author

tirsen commented May 5, 2017

I can't see a comment... :-)

@@ -299,19 +300,24 @@ func (bpc *BinlogPlayerController) Iteration() (err error) {
return fmt.Errorf("not starting because flag '%v' is set", binlogplayer.BlpFlagDontStart)
}

sourceTabletType, err := topoproto.ParseTabletType(*sourceTabletTypeStr)
if err != nil {
log.Fatalf("unknown tablet type: %v", *sourceTabletTypeStr)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use return fmt.Errorf e.g. as in a couple lines above.

We don't want to crash tablets when they're already running.

@tirsen tirsen force-pushed the binlog-player-source-tablet-flag branch from 90c7caf to 8d6a758 Compare May 9, 2017 09:05
@tirsen tirsen force-pushed the binlog-player-source-tablet-flag branch from 8d6a758 to 98a7270 Compare May 9, 2017 09:06
@sougou sougou merged commit 47cbd4a into vitessio:master May 19, 2017
@tirsen tirsen deleted the binlog-player-source-tablet-flag branch June 12, 2017 13:09
frouioui pushed a commit to planetscale/vitess that referenced this pull request Mar 26, 2024
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.

4 participants