mysqlctl: propagate remote shutdown timeout (vitessio#20059)

Signed-off-by: Mohamed Hamza <mhamza@fastmail.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

Author: mhamza15

go/vt/mysqlctl/grpcmysqlctlclient/client.go

@@ -29,6 +29,7 @@ import (
 	"google.golang.org/grpc/credentials/insecure"
 	"google.golang.org/grpc/status"
 
+	"vitess.io/vitess/go/protoutil"
 	"vitess.io/vitess/go/vt/grpcclient"
 	"vitess.io/vitess/go/vt/mysqlctl/mysqlctlclient"
 
@@ -73,10 +74,11 @@ func (c *client) Start(ctx context.Context, mysqldArgs ...string) error {
 }
 
 // Shutdown is part of the MysqlctlClient interface.
-func (c *client) Shutdown(ctx context.Context, waitForMysqld bool) error {
+func (c *client) Shutdown(ctx context.Context, waitForMysqld bool, shutdownTimeout time.Duration) error {
 	return c.withRetry(ctx, func() error {
 		_, err := c.c.Shutdown(ctx, &mysqlctlpb.ShutdownRequest{
-			WaitForMysqld: waitForMysqld,
+			WaitForMysqld:        waitForMysqld,
+			MysqlShutdownTimeout: protoutil.DurationToProto(shutdownTimeout),
 		})
 		return err
 	})

go/vt/mysqlctl/mysqlctlclient/interface.go

@@ -22,6 +22,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"time"
 
 	"github.com/spf13/pflag"
 
@@ -47,7 +48,7 @@ type MysqlctlClient interface {
 	Start(ctx context.Context, mysqldArgs ...string) error
 
 	// Shutdown calls Mysqld.Shutdown remotely.
-	Shutdown(ctx context.Context, waitForMysqld bool) error
+	Shutdown(ctx context.Context, waitForMysqld bool, shutdownTimeout time.Duration) error
 
 	// RunMysqlUpgrade calls Mysqld.RunMysqlUpgrade remotely.
 	RunMysqlUpgrade(ctx context.Context) error

go/vt/mysqlctl/mysqld.go

@@ -632,7 +632,7 @@ func (mysqld *Mysqld) Shutdown(ctx context.Context, cnf *Mycnf, waitForMysqld bo
 			return fmt.Errorf("can't dial mysqlctld: %v", err)
 		}
 		defer client.Close()
-		return client.Shutdown(ctx, waitForMysqld)
+		return client.Shutdown(ctx, waitForMysqld, shutdownTimeout)
 	}
 
 	// We're shutting down on purpose. We no longer want to be notified when

go/vt/mysqlctl/mysqld_test.go

@@ -17,6 +17,8 @@ limitations under the License.
 package mysqlctl
 
 import (
+	"context"
+	"net"
 	"os"
 	"strconv"
 	"strings"
@@ -25,10 +27,14 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc"
 
 	"vitess.io/vitess/go/mysql/fakesqldb"
+	"vitess.io/vitess/go/protoutil"
 	"vitess.io/vitess/go/sqltypes"
 	"vitess.io/vitess/go/vt/dbconfigs"
+	_ "vitess.io/vitess/go/vt/mysqlctl/grpcmysqlctlclient"
+	mysqlctlpb "vitess.io/vitess/go/vt/proto/mysqlctl"
 )
 
 type testcase struct {
@@ -114,6 +120,70 @@ func TestParseVersionString(t *testing.T) {
 	}
 }
 
+// shutdownRecordingMysqlCtlServer records remote Shutdown requests from Mysqld.
+type shutdownRecordingMysqlCtlServer struct {
+	mysqlctlpb.UnimplementedMysqlCtlServer
+
+	// shutdownRequests carries the remote shutdown request.
+	shutdownRequests chan *mysqlctlpb.ShutdownRequest
+}
+
+// Shutdown records the request and returns a successful response.
+func (s *shutdownRecordingMysqlCtlServer) Shutdown(ctx context.Context, request *mysqlctlpb.ShutdownRequest) (*mysqlctlpb.ShutdownResponse, error) {
+	s.shutdownRequests <- request
+
+	return &mysqlctlpb.ShutdownResponse{}, nil
+}
+
+// TestMysqldShutdownForwardsTimeoutToRemoteMysqlctld verifies that the remote
+// shutdown path preserves the caller's timeout.
+func TestMysqldShutdownForwardsTimeoutToRemoteMysqlctld(t *testing.T) {
+	ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
+	t.Cleanup(cancel)
+
+	oldSocketFile := socketFile
+	t.Cleanup(func() {
+		socketFile = oldSocketFile
+	})
+
+	tempSocketFile, err := os.CreateTemp("/tmp", "mysqlctld-*.sock")
+	require.NoError(t, err)
+	require.NoError(t, tempSocketFile.Close())
+	require.NoError(t, os.Remove(tempSocketFile.Name()))
+	t.Cleanup(func() {
+		_ = os.Remove(tempSocketFile.Name())
+	})
+
+	socketPath := tempSocketFile.Name()
+	listener, err := net.Listen("unix", socketPath)
+	require.NoError(t, err)
+
+	server := grpc.NewServer()
+	t.Cleanup(server.Stop)
+
+	recordingServer := &shutdownRecordingMysqlCtlServer{
+		shutdownRequests: make(chan *mysqlctlpb.ShutdownRequest, 1),
+	}
+	mysqlctlpb.RegisterMysqlCtlServer(server, recordingServer)
+
+	go func() { _ = server.Serve(listener) }()
+
+	socketFile = socketPath
+	shutdownTimeout := 43*time.Second + 125*time.Millisecond
+
+	mysqld := &Mysqld{}
+	err = mysqld.Shutdown(ctx, &Mycnf{}, true, shutdownTimeout)
+	require.NoError(t, err)
+
+	request := <-recordingServer.shutdownRequests
+	assert.True(t, request.WaitForMysqld)
+
+	gotTimeout, ok, err := protoutil.DurationFromProto(request.MysqlShutdownTimeout)
+	require.NoError(t, err)
+	require.True(t, ok)
+	assert.Equal(t, shutdownTimeout, gotTimeout)
+}
+
 func TestRegexps(t *testing.T) {
 	{
 		submatch := binlogEntryTimestampGTIDRegexp.FindStringSubmatch(`#230608 13:14:31 server id 484362839  end_log_pos 259 CRC32 0xc07510d0 	GTID	last_committed=0	sequence_number=1	rbr_only=yes`)