Merge #68431

68431: sql/tenant: remove idle detection and exit flag r=darinpp a=darinpp

We previoulsy had a flag `--idle-exit-after` that was used to monitor the
active SQL connections and terminate the server if there are no more
clients. We don't need that functionality anymore so this PR removes it.
It also fixes the issues with the related flaky tests.
Fixes #66767
Release note: None

Co-authored-by: Darin Peshev <darinp@gmail.com>

pkg/base/config.go

@@ -231,18 +231,6 @@ type Config struct {
 	// The flag exists mostly for the benefit of tests, and for
 	// `cockroach start-single-node`.
 	AutoInitializeCluster bool
-
-	// IdleExistAfter, If nonzero, will cause the server to run normally for the
-	// indicated amount of time, wait for all SQL connections to terminate,
-	// start a `defaultCountdownDuration` countdown and exit upon countdown
-	// reaching zero if no new connections occur. New connections will be
-	// accepted at all times and will effectively delay the exit (indefinitely
-	// if there is always at least one connection or there are no connection
-	// for less than `defaultCountdownDuration`. A new `defaultCountdownDuration`
-	// countdown will start when no more SQL connections exist.
-	// The interval is specified with a suffix of 's' for seconds, 'm' for
-	// minutes, and 'h' for hours.
-	IdleExitAfter time.Duration
 }
 
 // HistogramWindowInterval is used to determine the approximate length of time

pkg/base/test_server_args.go

@@ -233,9 +233,6 @@ type TestTenantArgs struct {
 	// to be created by StartTenant.
 	Existing bool
 
-	// IdleExitAfter, if set will cause the tenant process to exit if idle.
-	IdleExitAfter time.Duration
-
 	// Settings allows the caller to control the settings object used for the
 	// tenant cluster.
 	Settings *cluster.Settings

pkg/ccl/serverccl/BUILD.bazel

@@ -28,7 +28,6 @@ go_test(
         "//pkg/security/securitytest",
         "//pkg/server",
         "//pkg/server/serverpb",
-        "//pkg/settings/cluster",
         "//pkg/sql",
         "//pkg/sql/catalog/catconstants",
         "//pkg/sql/pgwire/pgcode",

pkg/ccl/serverccl/server_sql_test.go

@@ -14,12 +14,10 @@ import (
 	"io/ioutil"
 	"net/http"
 	"testing"
-	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/security"
-	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
@@ -143,71 +141,6 @@ func TestTenantHTTP(t *testing.T) {
 
 }
 
-func TestIdleExit(t *testing.T) {
-	defer leaktest.AfterTest(t)()
-	defer log.Scope(t).Close(t)
-	ctx := context.Background()
-
-	tc := serverutils.StartNewTestCluster(t, 1, base.TestClusterArgs{})
-	defer tc.Stopper().Stop(ctx)
-
-	warmupDuration := 500 * time.Millisecond
-	countdownDuration := 4000 * time.Millisecond
-	tenant, err := tc.Server(0).StartTenant(ctx,
-		base.TestTenantArgs{
-			TenantID:      serverutils.TestTenantID(),
-			IdleExitAfter: warmupDuration,
-			TestingKnobs: base.TestingKnobs{
-				TenantTestingKnobs: &sql.TenantTestingKnobs{
-					ClusterSettingsUpdater:    cluster.MakeTestingClusterSettings().MakeUpdater(),
-					IdleExitCountdownDuration: countdownDuration,
-				},
-			},
-			Stopper: tc.Stopper(),
-		})
-
-	require.NoError(t, err)
-
-	time.Sleep(warmupDuration / 2)
-	log.Infof(context.Background(), "Opening first con")
-	db := serverutils.OpenDBConn(
-		t, tenant.SQLAddr(), "", false, tc.Stopper(),
-	)
-	r := sqlutils.MakeSQLRunner(db)
-	r.QueryStr(t, `SELECT 1`)
-	require.NoError(t, db.Close())
-
-	time.Sleep(warmupDuration/2 + countdownDuration/2)
-
-	// Opening a connection in the middle of the countdown should stop the
-	// countdown timer. Closing the connection will restart the countdown.
-	log.Infof(context.Background(), "Opening second con")
-	db = serverutils.OpenDBConn(
-		t, tenant.SQLAddr(), "", false, tc.Stopper(),
-	)
-	r = sqlutils.MakeSQLRunner(db)
-	r.QueryStr(t, `SELECT 1`)
-	require.NoError(t, db.Close())
-
-	time.Sleep(countdownDuration / 2)
-
-	// If the tenant is stopped, that most likely means that the second connection
-	// didn't stop the countdown
-	select {
-	case <-tc.Stopper().IsStopped():
-		t.Error("stop on idle triggered too early")
-	default:
-	}
-
-	time.Sleep(countdownDuration * 3 / 2)
-
-	select {
-	case <-tc.Stopper().IsStopped():
-	default:
-		t.Error("stop on idle didn't trigger")
-	}
-}
-
 func TestNonExistentTenant(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)

pkg/cli/cliflags/flags.go

@@ -1532,21 +1532,6 @@ without any other details.
 `,
 	}
 
-	IdleExitAfter = FlagInfo{
-		Name: "idle-exit-after",
-		Description: `
-If nonzero, will cause the server to run normally for the 
-indicated amount of time, wait for all SQL connections to terminate, 
-start a 30s countdown and exit upon countdown reaching zero if no new 
-connections occur. New connections will be accepted at all times and 
-will effectively delay the exit (indefinitely if there is always at least 
-one connection or there are no connection for less than 30 sec.
-A new 30s countdown will start when no more SQL connections 
-exist. The interval is specified with a suffix of 's' for seconds, 
-'m' for minutes, and 'h' for hours.
-`,
-	}
-
 	ExportTableTarget = FlagInfo{
 		Name:        "table",
 		Description: `Select the table to export data from.`,

pkg/cli/flags.go

@@ -971,8 +971,6 @@ func init() {
 
 		stringSliceFlag(f, &serverCfg.SQLConfig.TenantKVAddrs, cliflags.KVAddrs)
 
-		durationFlag(f, &serverCfg.IdleExitAfter, cliflags.IdleExitAfter)
-
 		boolFlag(f, &serverCfg.ExternalIODirConfig.DisableHTTP, cliflags.ExternalIODisableHTTP)
 		boolFlag(f, &serverCfg.ExternalIODirConfig.DisableOutbound, cliflags.ExternalIODisabled)
 		boolFlag(f, &serverCfg.ExternalIODirConfig.DisableImplicitCredentials, cliflags.ExternalIODisableImplicitCredentials)

pkg/server/BUILD.bazel

@@ -19,7 +19,6 @@ go_library(
         "doc.go",
         "drain.go",
         "grpc_server.go",
-        "idle_monitor.go",
         "index_usage_stats.go",
         "init.go",
         "init_handshake.go",
@@ -267,7 +266,6 @@ go_test(
         "connectivity_test.go",
         "drain_test.go",
         "graphite_test.go",
-        "idle_monitor_test.go",
         "index_usage_stats_test.go",
         "init_handshake_test.go",
         "intent_test.go",
@@ -358,7 +356,6 @@ go_test(
         "//pkg/util/log",
         "//pkg/util/log/logpb",
         "//pkg/util/metric",
-        "//pkg/util/netutil",
         "//pkg/util/netutil/addr",
         "//pkg/util/protoutil",
         "//pkg/util/randutil",