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

release-2.0: cli: fix cockroach quit #26163

Merged
merged 1 commit into from
Jun 8, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions pkg/cli/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -1042,6 +1042,9 @@ func doShutdown(ctx context.Context, c serverpb.AdminClient, onModes []int32) er
// out, or perhaps drops the connection while waiting). To that end, we first
// run a noop DrainRequest. If that fails, we give up.
if err := checkNodeRunning(ctx, c); err != nil {
if grpcutil.IsClosedConnection(err) {
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

I think if you try to run cockroach quit against a node that's not running, we still want to fail instead of swallowing the error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The thing still fails. IsClosedConnection does not return true on ECONNREFUSED.

}
return err
}
// Send a drain request and continue reading until the connection drops (which
Expand Down Expand Up @@ -1113,16 +1116,21 @@ func runQuit(cmd *cobra.Command, args []string) (err error) {
case err := <-errChan:
if err != nil {
if _, ok := err.(errTryHardShutdown); ok {
fmt.Printf("graceful shutdown failed: %s; proceeding with hard shutdown\n", err)
log.Warningf(ctx, "graceful shutdown failed: %s; proceeding with hard shutdown\n", err)
break
}
return err
}
return nil
case <-time.After(time.Minute):
fmt.Println("timed out; proceeding with hard shutdown")
log.Warningf(ctx, "timed out; proceeding with hard shutdown")
}
// Not passing drain modes tells the server to not bother and go
// straight to shutdown.
// straight to shutdown. We try two times just in case there is a transient error.
err = doShutdown(ctx, c, nil)
if err != nil {
log.Warningf(ctx, "hard shutdown attempt failed, retrying: %v", err)
err = doShutdown(ctx, c, nil)
}
return errors.Wrap(doShutdown(ctx, c, nil), "hard shutdown failed")
}