From 85c1b990b16274f505249617e5181f22b4a17f20 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Wed, 21 Oct 2026 20:17:51 -0700 Subject: [PATCH] shell: bound the initial master connection wait behind -connectTimeout (#11922) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * shell: bound the initial master connection wait behind -connectTimeout weed shell waited on WaitUntilConnected with a background context, so an unreachable master hung the command forever. The wait now runs on a 30s (configurable) timeout and reports the failure instead. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * shell: stop the master reconnect loop when RunShell returns KeepConnectedToMaster ran on context.Background(), so once RunShell returned — including the new connect-timeout early return — the reconnect goroutine kept dialing forever, and repeated RunShell calls accumulated one each. Give it its own cancellable context, separate from connectCtx whose deadline must not cut reconnects under a live shell. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * shell: bound filer discovery by -connectTimeout too The discovery WithClient and ListClusterNodes ran on the unbounded background context, so a master disconnect after the initial connect — or a persistently failing ListClusterNodes — could block startup past the deadline. Reusing connectCtx bounds the whole startup phase. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * shell: report filer discovery failures instead of silently losing the filer target * shell: give filer discovery its own budget after master connects Discovery shared connectCtx, so a connection that landed near the -connectTimeout deadline left ListClusterNodes almost no time and no room for WithClient to retry. A fresh timeout keeps discovery bounded without starving it. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * shell: cover the -connectTimeout master wait with a test Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * shell: note filer discovery in the -connectTimeout help text Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- weed/command/shell.go | 12 ++++++++---- weed/shell/commands.go | 13 +++++++------ weed/shell/shell_liner.go | 25 +++++++++++++++++++++---- weed/shell/shell_liner_test.go | 19 +++++++++++++++++++ 4 files changed, 55 insertions(+), 14 deletions(-) diff --git a/weed/command/shell.go b/weed/command/shell.go index 0162d15015..f0aa1a30a7 100644 --- a/weed/command/shell.go +++ b/weed/command/shell.go @@ -3,6 +3,7 @@ package command import ( "fmt" "os" + "time" "github.com/seaweedfs/seaweedfs/weed/pb" @@ -12,10 +13,11 @@ import ( ) var ( - shellOptions shell.ShellOptions - shellInitialFiler *string - shellCluster *string - shellDebug *bool + shellOptions shell.ShellOptions + shellInitialFiler *string + shellCluster *string + shellDebug *bool + shellConnectTimeout *time.Duration ) func init() { @@ -25,6 +27,7 @@ func init() { shellInitialFiler = cmdShell.Flag.String("filer", "", "filer host and port for initial connection, e.g. localhost:8888") shellCluster = cmdShell.Flag.String("cluster", "", "cluster defined in shell.toml") shellDebug = cmdShell.Flag.Bool("debug", false, "print informational logs to stderr") + shellConnectTimeout = cmdShell.Flag.Duration("connectTimeout", 30*time.Second, "timeout for the initial master connection and filer discovery") } var cmdShell = &Command{ @@ -64,6 +67,7 @@ func runShell(command *Command, args []string) bool { } shellOptions.FilerAddress = pb.ServerAddress(filerAddress) shellOptions.Debug = *shellDebug + shellOptions.ConnectTimeout = *shellConnectTimeout if shellOptions.Debug { fmt.Fprintf(os.Stderr, "master: %s filer: %s\n", *shellOptions.Masters, shellOptions.FilerAddress) } diff --git a/weed/shell/commands.go b/weed/shell/commands.go index 315dbcb585..3bbb363b18 100644 --- a/weed/shell/commands.go +++ b/weed/shell/commands.go @@ -26,12 +26,13 @@ type ShellOptions struct { Masters *string GrpcDialOption grpc.DialOption // shell transient context - FilerHost string - FilerPort int64 - FilerGroup *string - FilerAddress pb.ServerAddress - Directory string - Debug bool + FilerHost string + FilerPort int64 + FilerGroup *string + FilerAddress pb.ServerAddress + Directory string + Debug bool + ConnectTimeout time.Duration } type CommandEnv struct { diff --git a/weed/shell/shell_liner.go b/weed/shell/shell_liner.go index c3e0d86d7f..6aae477b0f 100644 --- a/weed/shell/shell_liner.go +++ b/weed/shell/shell_liner.go @@ -10,6 +10,7 @@ import ( "path" "slices" "strings" + "time" "github.com/seaweedfs/seaweedfs/weed/cluster" "github.com/seaweedfs/seaweedfs/weed/pb" @@ -57,14 +58,28 @@ func RunShell(options ShellOptions) error { commandEnv := NewCommandEnv(&options) + connectTimeout := options.ConnectTimeout + if connectTimeout <= 0 { + connectTimeout = 30 * time.Second + } ctx := context.Background() - go commandEnv.MasterClient.KeepConnectedToMaster(ctx) - commandEnv.MasterClient.WaitUntilConnected(ctx) + connectCtx, cancel := context.WithTimeout(ctx, connectTimeout) + defer cancel() + reconnectCtx, cancelReconnect := context.WithCancel(ctx) + defer cancelReconnect() + go commandEnv.MasterClient.KeepConnectedToMaster(reconnectCtx) + if commandEnv.MasterClient.GetMaster(connectCtx) == "" { + err := fmt.Errorf("cannot connect to master %s within %s", *options.Masters, connectTimeout) + fmt.Fprintln(os.Stderr, err) + return err + } if commandEnv.option.FilerAddress == "" { var filers []pb.ServerAddress - commandEnv.MasterClient.WithClient(ctx, false, func(client master_pb.SeaweedClient) error { - resp, err := client.ListClusterNodes(context.Background(), &master_pb.ListClusterNodesRequest{ + discoverCtx, cancelDiscover := context.WithTimeout(ctx, connectTimeout) + defer cancelDiscover() + listErr := commandEnv.MasterClient.WithClient(discoverCtx, false, func(client master_pb.SeaweedClient) error { + resp, err := client.ListClusterNodes(discoverCtx, &master_pb.ListClusterNodesRequest{ ClientType: cluster.FilerType, FilerGroup: *options.FilerGroup, }) @@ -79,6 +94,8 @@ func RunShell(options ShellOptions) error { }) if len(filers) > 0 { commandEnv.option.FilerAddress = filers[rand.IntN(len(filers))] + } else if listErr != nil { + fmt.Fprintf(os.Stderr, "cannot list filers from master %s: %v\n", *options.Masters, listErr) } if options.Debug { if len(filers) > 0 { diff --git a/weed/shell/shell_liner_test.go b/weed/shell/shell_liner_test.go index 6d9bda9e7c..21a707ac8a 100644 --- a/weed/shell/shell_liner_test.go +++ b/weed/shell/shell_liner_test.go @@ -5,6 +5,7 @@ import ( "io" "strings" "testing" + "time" ) type fakeCommand struct { @@ -44,6 +45,24 @@ func TestProcessEachCmdReturnsErrors(t *testing.T) { } } +func TestRunShellConnectTimeout(t *testing.T) { + masters := "127.0.0.1:1" + filerGroup := "" + + start := time.Now() + err := RunShell(ShellOptions{ + Masters: &masters, + FilerGroup: &filerGroup, + ConnectTimeout: 200 * time.Millisecond, + }) + if err == nil || !strings.Contains(err.Error(), "cannot connect to master") { + t.Fatalf("expected a master connection timeout, got: %v", err) + } + if elapsed := time.Since(start); elapsed > 10*time.Second { + t.Fatalf("RunShell should give up near the connect timeout, took %v", elapsed) + } +} + func TestProcessEachCmdPreservesRegisteredCommandError(t *testing.T) { doErr := errors.New("daily_run: shard=3: recovery walk failed") failing := &fakeCommand{name: "test.failing.command", err: doErr}