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

mysqlctld: setup a different default for onterm_timeout #15575

Merged
Merged
Show file tree
Hide file tree
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
7 changes: 7 additions & 0 deletions changelog/20.0/20.0.0/summary.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- **[Breaking changes](#breaking-changes)**
- [`shutdown_grace_period` Default Change](#shutdown-grace-period-default)
- [New `unmanaged` Flag and `disable_active_reparents` deprecation](#unmanaged-flag)
- [`mysqlctld` `onterm-timeout` Default Change](#mysqlctld-onterm-timeout)
- **[Query Compatibility](#query-compatibility)**
- [Vindex Hints](#vindex-hints)
- [Update with Limit Support](#update-limit)
Expand Down Expand Up @@ -38,6 +39,12 @@ New flag `--unmanaged` has been introduced in this release to make it easier to

Starting this release, all unmanaged tablets should specify this flag.

#### <a id="mysqlctld-onterm-timeout"/>`mysqlctld` `onterm_timeout` Default Change

The `--onterm_timeout` flag default value has changed for `mysqlctld`. It now is by default long enough to be able to wait for the default `--shutdown-wait-time` when shutting down on a `TERM` signal.

This is necessary since otherwise MySQL would never shut down cleanly with the old defaults, since `mysqlctld` would shut down already after 10 seconds by default.

### <a id="query-compatibility"/>Query Compatibility

#### <a id="vindex-hints"/> Vindex Hints
Expand Down
8 changes: 7 additions & 1 deletion go/cmd/mysqlctld/cli/mysqlctld.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,18 @@ var (
PreRunE: servenv.CobraPreRunE,
RunE: run,
}

timeouts = &servenv.TimeoutFlags{
LameduckPeriod: 50 * time.Millisecond,
OnTermTimeout: shutdownWaitTime + 10*time.Second,
OnCloseTimeout: 10 * time.Second,
}
)

func init() {
servenv.RegisterDefaultFlags()
servenv.RegisterDefaultSocketFileFlags()
servenv.RegisterFlags()
servenv.RegisterFlagsWithTimeouts(timeouts)
servenv.RegisterGRPCServerFlags()
servenv.RegisterGRPCServerAuthFlags()
servenv.RegisterServiceMapFlag()
Expand Down
2 changes: 1 addition & 1 deletion go/cmd/vtbackup/cli/vtbackup.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ var (
mysqlPort = 3306
mysqlSocket string
mysqlTimeout = 5 * time.Minute
mysqlShutdownTimeout = 5 * time.Minute
mysqlShutdownTimeout = mysqlctl.DefaultShutdownTimeout
Copy link
Member

Choose a reason for hiding this comment

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

Insane that we were defining this 5 minutes in multiple places.

initDBSQLFile string
detachedMode bool
keepAliveTimeout time.Duration
Expand Down
14 changes: 6 additions & 8 deletions go/cmd/vtcombo/cli/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,6 @@ func startMysqld(uid uint32) (mysqld *mysqlctl.Mysqld, cnf *mysqlctl.Mycnf, err
return mysqld, cnf, nil
}

const mysqlShutdownTimeout = 5 * time.Minute

func run(cmd *cobra.Command, args []string) (err error) {
// Stash away a copy of the topology that vtcombo was started with.
//
Expand Down Expand Up @@ -218,9 +216,9 @@ func run(cmd *cobra.Command, args []string) (err error) {
return err
}
servenv.OnClose(func() {
ctx, cancel := context.WithTimeout(context.Background(), mysqlShutdownTimeout+10*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), mysqlctl.DefaultShutdownTimeout+10*time.Second)
defer cancel()
mysqld.Shutdown(ctx, cnf, true, mysqlShutdownTimeout)
mysqld.Shutdown(ctx, cnf, true, mysqlctl.DefaultShutdownTimeout)
})
// We want to ensure we can write to this database
mysqld.SetReadOnly(false)
Expand All @@ -242,9 +240,9 @@ func run(cmd *cobra.Command, args []string) (err error) {
if err != nil {
// ensure we start mysql in the event we fail here
if startMysql {
ctx, cancel := context.WithTimeout(context.Background(), mysqlShutdownTimeout+10*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), mysqlctl.DefaultShutdownTimeout+10*time.Second)
defer cancel()
mysqld.Shutdown(ctx, cnf, true, mysqlShutdownTimeout)
mysqld.Shutdown(ctx, cnf, true, mysqlctl.DefaultShutdownTimeout)
}

return fmt.Errorf("initTabletMapProto failed: %w", err)
Expand Down Expand Up @@ -291,9 +289,9 @@ func run(cmd *cobra.Command, args []string) (err error) {
err := topotools.RebuildKeyspace(context.Background(), logutil.NewConsoleLogger(), ts, ks.GetName(), tpb.Cells, false)
if err != nil {
if startMysql {
ctx, cancel := context.WithTimeout(context.Background(), mysqlShutdownTimeout+10*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), mysqlctl.DefaultShutdownTimeout+10*time.Second)
defer cancel()
mysqld.Shutdown(ctx, cnf, true, mysqlShutdownTimeout)
mysqld.Shutdown(ctx, cnf, true, mysqlctl.DefaultShutdownTimeout)
}

return fmt.Errorf("Couldn't build srv keyspace for (%v: %v). Got error: %w", ks, tpb.Cells, err)
Expand Down
2 changes: 1 addition & 1 deletion go/flags/endtoend/mysqlctld.txt
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ Flags:
--mysqlctl_mycnf_template string template file to use for generating the my.cnf file during server init
--mysqlctl_socket string socket file to use for remote mysqlctl actions (empty for local actions)
--onclose_timeout duration wait no more than this for OnClose handlers before stopping (default 10s)
--onterm_timeout duration wait no more than this for OnTermSync handlers before stopping (default 10s)
--onterm_timeout duration wait no more than this for OnTermSync handlers before stopping (default 5m10s)
dbussink marked this conversation as resolved.
Show resolved Hide resolved
--pid_file string If set, the process will write its pid to the named file, and delete it on graceful shutdown.
--pool_hostname_resolve_interval duration if set force an update to all hostnames and reconnect if changed, defaults to 0 (disabled)
--port int port for the server
Expand Down
5 changes: 1 addition & 4 deletions go/vt/mysqlctl/grpcmysqlctlserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ package grpcmysqlctlserver

import (
"context"
"time"

"google.golang.org/grpc"

Expand All @@ -43,16 +42,14 @@ func (s *server) Start(ctx context.Context, request *mysqlctlpb.StartRequest) (*
return &mysqlctlpb.StartResponse{}, s.mysqld.Start(ctx, s.cnf, request.MysqldArgs...)
}

const mysqlShutdownTimeout = 5 * time.Minute

// Shutdown implements the server side of the MysqlctlClient interface.
func (s *server) Shutdown(ctx context.Context, request *mysqlctlpb.ShutdownRequest) (*mysqlctlpb.ShutdownResponse, error) {
timeout, ok, err := protoutil.DurationFromProto(request.MysqlShutdownTimeout)
if err != nil {
return nil, err
}
if !ok {
timeout = mysqlShutdownTimeout
timeout = mysqlctl.DefaultShutdownTimeout
}
return &mysqlctlpb.ShutdownResponse{}, s.mysqld.Shutdown(ctx, s.cnf, request.WaitForMysqld, timeout)
}
Expand Down
2 changes: 2 additions & 0 deletions go/vt/mysqlctl/mycnf.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import (
"time"
)

const DefaultShutdownTimeout = 5 * time.Minute

// Mycnf is a memory structure that contains a bunch of interesting
// parameters to start mysqld. It can be used to generate standard
// my.cnf files from a server id and mysql port. It can also be
Expand Down
8 changes: 4 additions & 4 deletions go/vt/servenv/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,18 @@ func Run(bindAddress string, port int) {
l.Close()

startTime := time.Now()
log.Infof("Entering lameduck mode for at least %v", lameduckPeriod)
log.Infof("Entering lameduck mode for at least %v", timeouts.LameduckPeriod)
log.Infof("Firing asynchronous OnTerm hooks")
go onTermHooks.Fire()

fireOnTermSyncHooks(onTermTimeout)
if remain := lameduckPeriod - time.Since(startTime); remain > 0 {
fireOnTermSyncHooks(timeouts.OnTermTimeout)
if remain := timeouts.LameduckPeriod - time.Since(startTime); remain > 0 {
log.Infof("Sleeping an extra %v after OnTermSync to finish lameduck period", remain)
time.Sleep(remain)
}

log.Info("Shutting down gracefully")
fireOnCloseHooks(onCloseTimeout)
fireOnCloseHooks(timeouts.OnCloseTimeout)
ListeningURL = url.URL{}
}

Expand Down
37 changes: 31 additions & 6 deletions go/vt/servenv/servenv.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,24 +75,33 @@ var (

// Flags specific to Init, Run, and RunDefault functions.
var (
lameduckPeriod = 50 * time.Millisecond
onTermTimeout = 10 * time.Second
onCloseTimeout = 10 * time.Second
catchSigpipe bool
maxStackSize = 64 * 1024 * 1024
initStartTime time.Time // time when tablet init started: for debug purposes to time how long a tablet init takes
tableRefreshInterval int
)

type TimeoutFlags struct {
LameduckPeriod time.Duration
OnTermTimeout time.Duration
OnCloseTimeout time.Duration
}

var timeouts = &TimeoutFlags{
LameduckPeriod: 50 * time.Millisecond,
OnTermTimeout: 10 * time.Second,
OnCloseTimeout: 10 * time.Second,
}

// RegisterFlags installs the flags used by Init, Run, and RunDefault.
//
// This must be called before servenv.ParseFlags if using any of those
// functions.
func RegisterFlags() {
OnParse(func(fs *pflag.FlagSet) {
fs.DurationVar(&lameduckPeriod, "lameduck-period", lameduckPeriod, "keep running at least this long after SIGTERM before stopping")
fs.DurationVar(&onTermTimeout, "onterm_timeout", onTermTimeout, "wait no more than this for OnTermSync handlers before stopping")
fs.DurationVar(&onCloseTimeout, "onclose_timeout", onCloseTimeout, "wait no more than this for OnClose handlers before stopping")
fs.DurationVar(&timeouts.LameduckPeriod, "lameduck-period", timeouts.LameduckPeriod, "keep running at least this long after SIGTERM before stopping")
fs.DurationVar(&timeouts.OnTermTimeout, "onterm_timeout", timeouts.OnTermTimeout, "wait no more than this for OnTermSync handlers before stopping")
fs.DurationVar(&timeouts.OnCloseTimeout, "onclose_timeout", timeouts.OnCloseTimeout, "wait no more than this for OnClose handlers before stopping")
fs.BoolVar(&catchSigpipe, "catch-sigpipe", catchSigpipe, "catch and ignore SIGPIPE on stdout and stderr if specified")
fs.IntVar(&maxStackSize, "max-stack-size", maxStackSize, "configure the maximum stack size in bytes")
fs.IntVar(&tableRefreshInterval, "table-refresh-interval", tableRefreshInterval, "interval in milliseconds to refresh tables in status page with refreshRequired class")
Expand All @@ -102,6 +111,22 @@ func RegisterFlags() {
})
}

func RegisterFlagsWithTimeouts(tf *TimeoutFlags) {
OnParse(func(fs *pflag.FlagSet) {
fs.DurationVar(&tf.LameduckPeriod, "lameduck-period", tf.LameduckPeriod, "keep running at least this long after SIGTERM before stopping")
fs.DurationVar(&tf.OnTermTimeout, "onterm_timeout", tf.OnTermTimeout, "wait no more than this for OnTermSync handlers before stopping")
fs.DurationVar(&tf.OnCloseTimeout, "onclose_timeout", tf.OnCloseTimeout, "wait no more than this for OnClose handlers before stopping")
fs.BoolVar(&catchSigpipe, "catch-sigpipe", catchSigpipe, "catch and ignore SIGPIPE on stdout and stderr if specified")
fs.IntVar(&maxStackSize, "max-stack-size", maxStackSize, "configure the maximum stack size in bytes")
fs.IntVar(&tableRefreshInterval, "table-refresh-interval", tableRefreshInterval, "interval in milliseconds to refresh tables in status page with refreshRequired class")

// pid_file.go
fs.StringVar(&pidFile, "pid_file", pidFile, "If set, the process will write its pid to the named file, and delete it on graceful shutdown.")

timeouts = tf
})
}

func GetInitStartTime() time.Time {
mu.Lock()
defer mu.Unlock()
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vttablet/tabletmanager/tm_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ var (
initTags flagutil.StringMapValue

initTimeout = 1 * time.Minute
mysqlShutdownTimeout = 5 * time.Minute
mysqlShutdownTimeout = mysqlctl.DefaultShutdownTimeout
)

func registerInitFlags(fs *pflag.FlagSet) {
Expand Down
Loading