From f8d7f3213ef4be4f1b19f5bd4df1fa30cab99693 Mon Sep 17 00:00:00 2001 From: Dirkjan Bussink Date: Fri, 3 Nov 2023 22:10:17 +0100 Subject: [PATCH 1/2] servenv: Remove double close() logic The servenv.Run function already runs the close hooks once it finishes. This means there's no use for deferring a `.Close()` since it double executes all close hooks. In fact, it leads to panics. For example, vttablet always panics on each shutdown because the connection to the topo is already closed. Signed-off-by: Dirkjan Bussink --- go/cmd/mysqlctld/cli/mysqlctld.go | 2 -- go/cmd/vtaclcheck/cli/vtactlcheck.go | 1 - go/cmd/vtbackup/cli/vtbackup.go | 1 - go/cmd/vtbench/cli/vtbench.go | 1 - go/cmd/vtctld/cli/cli.go | 1 - go/cmd/vtgate/cli/cli.go | 1 - go/cmd/vttablet/cli/cli.go | 1 - go/vt/servenv/run.go | 7 ------- 8 files changed, 15 deletions(-) diff --git a/go/cmd/mysqlctld/cli/mysqlctld.go b/go/cmd/mysqlctld/cli/mysqlctld.go index 4afbf510951..6ebaa5dc422 100644 --- a/go/cmd/mysqlctld/cli/mysqlctld.go +++ b/go/cmd/mysqlctld/cli/mysqlctld.go @@ -54,7 +54,6 @@ var ( Long: "`mysqlctld` is a gRPC server that can be used instead of the `mysqlctl` client tool.\n" + "If the target directories are empty when it is invoked, it automatically performs initialization operations to bootstrap the `mysqld` instance before starting it.\n" + "The `mysqlctld` process can subsequently receive gRPC commands from a `vttablet` to perform housekeeping operations like shutting down and restarting the `mysqld` instance as needed.\n\n" + - "{{< warning >}}\n" + "`mysqld_safe` is not used so the `mysqld` process will not be automatically restarted in case of a failure.\n" + "{{}}\n\n" + @@ -151,7 +150,6 @@ func run(cmd *cobra.Command, args []string) error { cancel() servenv.Init() - defer servenv.Close() // Take mysqld down with us on SIGTERM before entering lame duck. servenv.OnTermSync(func() { diff --git a/go/cmd/vtaclcheck/cli/vtactlcheck.go b/go/cmd/vtaclcheck/cli/vtactlcheck.go index d6a71a23252..ebac94131e8 100644 --- a/go/cmd/vtaclcheck/cli/vtactlcheck.go +++ b/go/cmd/vtaclcheck/cli/vtactlcheck.go @@ -44,7 +44,6 @@ var ( func run(cmd *cobra.Command, args []string) error { servenv.Init() - defer servenv.Close() opts := &vtaclcheck.Options{ ACLFile: aclFile, diff --git a/go/cmd/vtbackup/cli/vtbackup.go b/go/cmd/vtbackup/cli/vtbackup.go index 3e48b75df4d..121ba39b8c5 100644 --- a/go/cmd/vtbackup/cli/vtbackup.go +++ b/go/cmd/vtbackup/cli/vtbackup.go @@ -217,7 +217,6 @@ func init() { func run(_ *cobra.Command, args []string) error { servenv.Init() - defer servenv.Close() ctx, cancel := context.WithCancel(context.Background()) servenv.OnClose(func() { diff --git a/go/cmd/vtbench/cli/vtbench.go b/go/cmd/vtbench/cli/vtbench.go index 3c197dfc905..69b866bb60d 100644 --- a/go/cmd/vtbench/cli/vtbench.go +++ b/go/cmd/vtbench/cli/vtbench.go @@ -162,7 +162,6 @@ func run(cmd *cobra.Command, args []string) error { _ = cmd.Flags().Set("logtostderr", "true") servenv.Init() - defer servenv.Close() var clientProto vtbench.ClientProtocol switch protocol { diff --git a/go/cmd/vtctld/cli/cli.go b/go/cmd/vtctld/cli/cli.go index 42ab1cfde1e..e5124133adb 100644 --- a/go/cmd/vtctld/cli/cli.go +++ b/go/cmd/vtctld/cli/cli.go @@ -55,7 +55,6 @@ This is demonstrated in the example usage below.`, func run(cmd *cobra.Command, args []string) error { servenv.Init() - defer servenv.Close() ts = topo.Open() defer ts.Close() diff --git a/go/cmd/vtgate/cli/cli.go b/go/cmd/vtgate/cli/cli.go index 0ba24162f41..9182bfcf9a4 100644 --- a/go/cmd/vtgate/cli/cli.go +++ b/go/cmd/vtgate/cli/cli.go @@ -134,7 +134,6 @@ func run(cmd *cobra.Command, args []string) error { defer exit.Recover() servenv.Init() - defer servenv.Close() ts := topo.Open() defer ts.Close() diff --git a/go/cmd/vttablet/cli/cli.go b/go/cmd/vttablet/cli/cli.go index e967aafc164..1efa35613d7 100644 --- a/go/cmd/vttablet/cli/cli.go +++ b/go/cmd/vttablet/cli/cli.go @@ -104,7 +104,6 @@ vttablet \ func run(cmd *cobra.Command, args []string) error { servenv.Init() - defer servenv.Close() tabletAlias, err := topoproto.ParseTabletAlias(tabletPath) if err != nil { diff --git a/go/vt/servenv/run.go b/go/vt/servenv/run.go index ec0d8971e56..70b534bdcc3 100644 --- a/go/vt/servenv/run.go +++ b/go/vt/servenv/run.go @@ -18,7 +18,6 @@ package servenv import ( "net" - "net/url" "os" "os/signal" "strconv" @@ -76,12 +75,6 @@ func Run(bindAddress string, port int) { fireOnCloseHooks(onCloseTimeout) } -// Close runs any registered exit hooks in parallel. -func Close() { - onCloseHooks.Fire() - ListeningURL = url.URL{} -} - // OnClose registers f to be run at the end of the app lifecycle. // This happens after the lameduck period just before the program exits. // All hooks are run in parallel. From b731d5e5641110b891b04951b8dda46bc30d6e9b Mon Sep 17 00:00:00 2001 From: deepthi Date: Fri, 3 Nov 2023 14:51:11 -0700 Subject: [PATCH 2/2] servenv: reset ListeningURL when closing Signed-off-by: deepthi --- go/vt/servenv/run.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go/vt/servenv/run.go b/go/vt/servenv/run.go index 70b534bdcc3..6f028786eaf 100644 --- a/go/vt/servenv/run.go +++ b/go/vt/servenv/run.go @@ -18,6 +18,7 @@ package servenv import ( "net" + "net/url" "os" "os/signal" "strconv" @@ -73,6 +74,7 @@ func Run(bindAddress string, port int) { log.Info("Shutting down gracefully") fireOnCloseHooks(onCloseTimeout) + ListeningURL = url.URL{} } // OnClose registers f to be run at the end of the app lifecycle.