ccl/sqlproxyccl: remove the idle monitor component

This commit removes the idle monitor component from the sqlproxy as there's no
strong use case for it, as discussed. This is currently not being used anywhere
on CockroachCloud, and with the connection migration feature, it's unlikely
that we will need this. When a pod goes into the draining state, all
connections are migrated away to other running pods. Even though the idle
monitor may benefit us by terminating idle connections earlier for connections
that cannot be migrated, the complexity of maintaining this outweighs the
benefits that we get (i.e. being able to shut down a pod earlier; between 1 to
10 minutes rather than waiting until 10 minutes for forceful termination). At
the same time, the --drain-timeout flag has been removed from the
`mt start-proxy` command as well.

Release note: None

pkg/BUILD.bazel

@@ -45,7 +45,6 @@ ALL_TESTS = [
     "//pkg/ccl/spanconfigccl/spanconfigsqlwatcherccl:spanconfigsqlwatcherccl_test",
     "//pkg/ccl/sqlproxyccl/balancer:balancer_test",
     "//pkg/ccl/sqlproxyccl/denylist:denylist_test",
-    "//pkg/ccl/sqlproxyccl/idle:idle_test",
     "//pkg/ccl/sqlproxyccl/interceptor:interceptor_test",
     "//pkg/ccl/sqlproxyccl/tenant:tenant_test",
     "//pkg/ccl/sqlproxyccl/throttler:throttler_test",

pkg/ccl/sqlproxyccl/BUILD.bazel

@@ -23,7 +23,6 @@ go_library(
         "//pkg/base",
         "//pkg/ccl/sqlproxyccl/balancer",
         "//pkg/ccl/sqlproxyccl/denylist",
-        "//pkg/ccl/sqlproxyccl/idle",
         "//pkg/ccl/sqlproxyccl/interceptor",
         "//pkg/ccl/sqlproxyccl/tenant",
         "//pkg/ccl/sqlproxyccl/tenantdirsvr",

pkg/ccl/sqlproxyccl/connector.go

@@ -72,16 +72,6 @@ type connector struct {
 	// NOTE: This field is optional.
 	TLSConfig *tls.Config
 
-	// IdleMonitorWrapperFn is used to wrap the connection to the SQL pod with
-	// an idle monitor. If not specified, the raw connection to the SQL pod
-	// will be returned.
-	//
-	// In the case of connecting with an authentication phase, the connection
-	// will be wrapped before starting the authentication.
-	//
-	// NOTE: This field is optional.
-	IdleMonitorWrapperFn func(serverConn net.Conn) net.Conn
-
 	// Testing knobs for internal connector calls. If specified, these will
 	// be called instead of the actual logic.
 	testingKnobs struct {
@@ -112,10 +102,6 @@ func (c *connector) OpenTenantConnWithToken(
 		}
 	}()
 
-	if c.IdleMonitorWrapperFn != nil {
-		serverConn = c.IdleMonitorWrapperFn(serverConn)
-	}
-
 	// When we use token-based authentication, we will still get the initial
 	// connection data messages (e.g. ParameterStatus and BackendKeyData).
 	// Since this method is only used during connection migration (i.e. proxy
@@ -161,10 +147,6 @@ func (c *connector) OpenTenantConnWithAuth(
 		}
 	}()
 
-	if c.IdleMonitorWrapperFn != nil {
-		serverConn = c.IdleMonitorWrapperFn(serverConn)
-	}
-
 	// Perform user authentication for non-token-based auth methods. This will
 	// block until the server has authenticated the client.
 	if err := authenticate(clientConn, serverConn, throttleHook); err != nil {

pkg/ccl/sqlproxyccl/connector_test.go

@@ -152,59 +152,6 @@ func TestConnector_OpenTenantConnWithToken(t *testing.T) {
 		_, ok := c.StartupMsg.Parameters[sessionRevivalTokenStartupParam]
 		require.False(t, ok)
 	})
-
-	t.Run("idle monitor wrapper is called", func(t *testing.T) {
-		var wrapperCalled bool
-		f := &forwarder{}
-		c := &connector{
-			StartupMsg: &pgproto3.StartupMessage{
-				Parameters: make(map[string]string),
-			},
-			IdleMonitorWrapperFn: func(crdbConn net.Conn) net.Conn {
-				wrapperCalled = true
-				return crdbConn
-			},
-		}
-
-		conn, _ := net.Pipe()
-		defer conn.Close()
-
-		var openCalled bool
-		c.testingKnobs.dialTenantCluster = func(
-			ctx context.Context, requester balancer.ConnectionHandle,
-		) (net.Conn, error) {
-			require.Equal(t, f, requester)
-			openCalled = true
-
-			// Validate that token is set.
-			str, ok := c.StartupMsg.Parameters[sessionRevivalTokenStartupParam]
-			require.True(t, ok)
-			require.Equal(t, token, str)
-
-			return conn, nil
-		}
-
-		var authCalled bool
-		defer testutils.TestingHook(
-			&readTokenAuthResult,
-			func(serverConn net.Conn) error {
-				authCalled = true
-				require.Equal(t, conn, serverConn)
-				return nil
-			},
-		)()
-
-		crdbConn, err := c.OpenTenantConnWithToken(ctx, f, token)
-		require.True(t, wrapperCalled)
-		require.True(t, openCalled)
-		require.True(t, authCalled)
-		require.NoError(t, err)
-		require.Equal(t, conn, crdbConn)
-
-		// Ensure that token is deleted.
-		_, ok := c.StartupMsg.Parameters[sessionRevivalTokenStartupParam]
-		require.False(t, ok)
-	})
 }
 
 func TestConnector_OpenTenantConnWithAuth(t *testing.T) {
@@ -334,68 +281,6 @@ func TestConnector_OpenTenantConnWithAuth(t *testing.T) {
 		require.False(t, sentToClient)
 		require.Equal(t, serverConn, crdbConn)
 	})
-
-	t.Run("idle monitor wrapper is called", func(t *testing.T) {
-		clientConn, _ := net.Pipe()
-		defer clientConn.Close()
-
-		serverConn, _ := net.Pipe()
-		defer serverConn.Close()
-
-		var wrapperCalled bool
-		f := &forwarder{}
-		c := &connector{
-			StartupMsg: &pgproto3.StartupMessage{
-				Parameters: map[string]string{
-					// Passing in a token should have no effect.
-					sessionRevivalTokenStartupParam: "foo",
-				},
-			},
-			IdleMonitorWrapperFn: func(crdbConn net.Conn) net.Conn {
-				wrapperCalled = true
-				return crdbConn
-			},
-		}
-
-		var openCalled bool
-		c.testingKnobs.dialTenantCluster = func(
-			ctx context.Context, requester balancer.ConnectionHandle,
-		) (net.Conn, error) {
-			require.Equal(t, f, requester)
-			openCalled = true
-
-			// Validate that token is not set.
-			_, ok := c.StartupMsg.Parameters[sessionRevivalTokenStartupParam]
-			require.False(t, ok)
-
-			return serverConn, nil
-		}
-
-		var authCalled bool
-		defer testutils.TestingHook(
-			&authenticate,
-			func(
-				client net.Conn,
-				server net.Conn,
-				throttleHook func(status throttler.AttemptStatus) error,
-			) error {
-				authCalled = true
-				require.Equal(t, clientConn, client)
-				require.NotNil(t, server)
-				require.Equal(t, reflect.ValueOf(dummyHook).Pointer(),
-					reflect.ValueOf(throttleHook).Pointer())
-				return nil
-			},
-		)()
-
-		crdbConn, sentToClient, err := c.OpenTenantConnWithAuth(ctx, f, clientConn, dummyHook)
-		require.True(t, openCalled)
-		require.True(t, wrapperCalled)
-		require.True(t, authCalled)
-		require.NoError(t, err)
-		require.False(t, sentToClient)
-		require.Equal(t, serverConn, crdbConn)
-	})
 }
 
 func TestConnector_dialTenantCluster(t *testing.T) {

pkg/ccl/sqlproxyccl/error.go

@@ -41,10 +41,6 @@ const (
 	// or vice versa.
 	codeUnexpectedInsecureStartupMessage
 
-	// codeSNIRoutingFailed indicates an error choosing a backend address based on
-	// the client's SNI header.
-	codeSNIRoutingFailed
-
 	// codeUnexpectedStartupMessage indicates an unexpected startup message
 	// received from the client after TLS negotiation.
 	codeUnexpectedStartupMessage
@@ -77,10 +73,6 @@ const (
 	// has expired and should be closed.
 	codeExpiredClientConnection
 
-	// codeIdleDisconnect indicates that the connection was disconnected for
-	// being idle for longer than the specified timeout.
-	codeIdleDisconnect
-
 	// codeUnavailable indicates that the backend SQL server exists but is not
 	// accepting connections. For example, a tenant cluster that has maxPods set to 0.
 	codeUnavailable