Skip to content

Commit

Permalink
sqlliveness: refactor out settings into slbase
Browse files Browse the repository at this point in the history
For the sqlliveness package to adopt the regionliveness package, we need
to eliminate a circular dependency between the two.  Currently, the
region liveness requires to access the default TTL and heartbeat
settings inside sqlliveness. To address this, this patch will eliminate
this cycle by moving these settings into a subpackage called slbase.

Release note: None
  • Loading branch information
fqazi committed Jan 26, 2024
1 parent 5db0b83 commit 9d04b47
Show file tree
Hide file tree
Showing 20 changed files with 81 additions and 52 deletions.
1 change: 1 addition & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -2154,6 +2154,7 @@ GO_TARGETS = [
"//pkg/sql/sqlitelogictest/tests/local-vec-off:local-vec-off_test",
"//pkg/sql/sqlitelogictest/tests/local:local_test",
"//pkg/sql/sqlitelogictest:sqlitelogictest",
"//pkg/sql/sqlliveness/slbase:slbase",
"//pkg/sql/sqlliveness/slinstance:slinstance",
"//pkg/sql/sqlliveness/slinstance:slinstance_test",
"//pkg/sql/sqlliveness/slprovider:slprovider",
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ go_test(
"//pkg/sql/sem/tree",
"//pkg/sql/sessiondata",
"//pkg/sql/sqlclustersettings",
"//pkg/sql/sqlliveness/slinstance",
"//pkg/sql/sqlliveness/slbase",
"//pkg/sql/stats",
"//pkg/storage",
"//pkg/testutils",
Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/backupccl/backup_tenant_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql"
_ "github.com/cockroachdb/cockroach/pkg/sql/importer"
"github.com/cockroachdb/cockroach/pkg/sql/sqlclustersettings"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slinstance"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slbase"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/jobutils"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
Expand Down Expand Up @@ -101,7 +101,7 @@ func TestBackupSharedProcessTenantNodeDown(t *testing.T) {
// for instance-based planning to recognize the downed node.
sv := &tenantApp.ClusterSettings().SV
padding := 10 * time.Second
timeout := slinstance.DefaultTTL.Get(sv) + slinstance.DefaultHeartBeat.Get(sv) + padding
timeout := slbase.DefaultTTL.Get(sv) + slbase.DefaultHeartBeat.Get(sv) + padding
testutils.SucceedsWithin(t, func() error {
_, err := tenantDB.Exec("BACKUP INTO 'nodelocal://1/worker-failure'")
return err
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/kvccl/kvtenantccl/upgradeccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ go_test(
"//pkg/spanconfig",
"//pkg/sql/sem/eval",
"//pkg/sql/sqlinstance/instancestorage",
"//pkg/sql/sqlliveness/slinstance",
"//pkg/sql/sqlliveness/slbase",
"//pkg/testutils/serverutils",
"//pkg/testutils/skip",
"//pkg/testutils/sqlutils",
Expand Down
6 changes: 3 additions & 3 deletions pkg/ccl/kvccl/kvtenantccl/upgradeccl/tenant_upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/spanconfig"
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
"github.com/cockroachdb/cockroach/pkg/sql/sqlinstance/instancestorage"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slinstance"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slbase"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
Expand Down Expand Up @@ -448,8 +448,8 @@ func TestTenantUpgradeFailure(t *testing.T) {
// Shorten the reclaim loop so that terminated SQL servers don't block
// the upgrade from succeeding.
instancestorage.ReclaimLoopInterval.Override(ctx, &settings.SV, 250*time.Millisecond)
slinstance.DefaultTTL.Override(ctx, &settings.SV, 15*time.Second)
slinstance.DefaultHeartBeat.Override(ctx, &settings.SV, 500*time.Millisecond)
slbase.DefaultTTL.Override(ctx, &settings.SV, 15*time.Second)
slbase.DefaultHeartBeat.Override(ctx, &settings.SV, 500*time.Millisecond)
tenantStopper := stop.NewStopper()
// Initialize the version to the minimum it could be.
require.NoError(t, clusterversion.Initialize(ctx, v0, &settings.SV))
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/kvccl/kvtenantccl/upgradeinterlockccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ go_test(
"//pkg/sql/catalog/lease",
"//pkg/sql/sessiondatapb",
"//pkg/sql/sqlinstance/instancestorage",
"//pkg/sql/sqlliveness/slinstance",
"//pkg/sql/sqlliveness/slbase",
"//pkg/sql/stats",
"//pkg/testutils/serverutils",
"//pkg/testutils/skip", # keep
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog/lease"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb"
"github.com/cockroachdb/cockroach/pkg/sql/sqlinstance/instancestorage"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slinstance"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slbase"
"github.com/cockroachdb/cockroach/pkg/sql/stats"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
Expand Down Expand Up @@ -114,8 +114,8 @@ func runTest(t *testing.T, variant sharedtestutil.TestVariant, test sharedtestut
ttlOverride *= 10
}
heartbeatOverride := ttlOverride / 10
slinstance.DefaultTTL.Override(ctx, &s.SV, ttlOverride)
slinstance.DefaultHeartBeat.Override(ctx, &s.SV, heartbeatOverride)
slbase.DefaultTTL.Override(ctx, &s.SV, ttlOverride)
slbase.DefaultHeartBeat.Override(ctx, &s.SV, heartbeatOverride)
}

// Initialize the version to the MinSupportedVersion so that we can perform
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/multitenantccl/tenantcostclient/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ go_test(
"//pkg/sql/execinfra",
"//pkg/sql/sem/eval",
"//pkg/sql/sqlliveness",
"//pkg/sql/sqlliveness/slinstance",
"//pkg/sql/sqlliveness/slbase",
"//pkg/sql/stats",
"//pkg/testutils",
"//pkg/testutils/datapathutils",
Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/multitenantccl/tenantcostclient/tenant_side_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/distsql"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slinstance"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slbase"
"github.com/cockroachdb/cockroach/pkg/sql/stats"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/datapathutils"
Expand Down Expand Up @@ -1024,7 +1024,7 @@ func TestSQLLivenessExemption(t *testing.T) {
st := cluster.MakeTestingClusterSettings()
// Make the tenant heartbeat like crazy.
ctx := context.Background()
slinstance.DefaultHeartBeat.Override(ctx, &st.SV, 10*time.Millisecond)
slbase.DefaultHeartBeat.Override(ctx, &st.SV, 10*time.Millisecond)

_, tenantDB := serverutils.StartTenant(t, hostServer, base.TestTenantArgs{
TenantID: tenantID,
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/testccl/sqlccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ go_test(
"//pkg/sql/isql",
"//pkg/sql/lexbase",
"//pkg/sql/sessiondatapb",
"//pkg/sql/sqlliveness/slinstance",
"//pkg/sql/sqlliveness/slbase",
"//pkg/sql/sqltestutils",
"//pkg/sql/tests",
"//pkg/testutils",
Expand Down
6 changes: 3 additions & 3 deletions pkg/ccl/testccl/sqlccl/temp_table_clean_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slinstance"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slbase"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
Expand Down Expand Up @@ -104,8 +104,8 @@ func TestTenantTempTableCleanup(t *testing.T) {
sql.TempObjectWaitInterval.Override(ctx, &st.SV, time.Second*0)
// Set up sessions to expire within 5 seconds of a
// nodes death.
slinstance.DefaultTTL.Override(ctx, &st.SV, 5*time.Second)
slinstance.DefaultHeartBeat.Override(ctx, &st.SV, time.Second)
slbase.DefaultTTL.Override(ctx, &st.SV, 5*time.Second)
slbase.DefaultHeartBeat.Override(ctx, &st.SV, time.Second)
return st
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/gc/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ go_test(
"//pkg/server",
"//pkg/server/serverpb",
"//pkg/settings/cluster",
"//pkg/sql/sqlliveness/slinstance",
"//pkg/sql/sqlliveness/slbase",
"//pkg/storage",
"//pkg/storage/enginepb",
"//pkg/testutils",
Expand Down
4 changes: 2 additions & 2 deletions pkg/kv/kvserver/gc/gc_int_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slinstance"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slbase"
"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
Expand Down Expand Up @@ -86,7 +86,7 @@ func TestEndToEndGC(t *testing.T) {
settings := cluster.MakeTestingClusterSettings()
// Push the TTL up to 60 hours since we emulate a 50 hours
// clock jump below.
slinstance.DefaultTTL.Override(ctx, &settings.SV, 60*time.Hour)
slbase.DefaultTTL.Override(ctx, &settings.SV, 60*time.Hour)

manualClock := hlc.NewHybridManualClock()
s, appSqlDb, appKvDb := serverutils.StartServer(t, base.TestServerArgs{
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/regionliveness/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ go_library(
"//pkg/sql/pgwire/pgerror",
"//pkg/sql/sem/tree",
"//pkg/sql/sessiondata",
"//pkg/sql/sqlliveness/slinstance",
"//pkg/sql/sqlliveness/slbase",
"//pkg/sql/types",
"//pkg/util/timeutil",
"@com_github_cockroachdb_errors//:errors",
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/regionliveness/prober.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slinstance"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slbase"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/errors"
Expand Down Expand Up @@ -138,8 +138,8 @@ SELECT count(*) FROM system.sql_instances WHERE crdb_region = $1::system.crdb_in

// Region has gone down, set the unavailable_at time on it
return l.db.Txn(ctx, func(ctx context.Context, txn isql.Txn) error {
defaultTTL := slinstance.DefaultTTL.Get(&l.settings.SV)
defaultHeartbeat := slinstance.DefaultHeartBeat.Get(&l.settings.SV)
defaultTTL := slbase.DefaultTTL.Get(&l.settings.SV)
defaultHeartbeat := slbase.DefaultHeartBeat.Get(&l.settings.SV)
// Get the read timestamp and pick a commit deadline.
readTS := txn.KV().ReadTimestamp().AddDuration(defaultHeartbeat)
txnTS := readTS.AddDuration(defaultTTL)
Expand Down
9 changes: 9 additions & 0 deletions pkg/sql/sqlliveness/slbase/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
name = "slbase",
srcs = ["slbase.go"],
importpath = "github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slbase",
visibility = ["//visibility:public"],
deps = ["//pkg/settings"],
)
36 changes: 36 additions & 0 deletions pkg/sql/sqlliveness/slbase/slbase.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright 2023 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package slbase

import (
"time"

"github.com/cockroachdb/cockroach/pkg/settings"
)

var (
// DefaultTTL specifies the time to expiration when a session is created.
DefaultTTL = settings.RegisterDurationSetting(
settings.ApplicationLevel,
"server.sqlliveness.ttl",
"default sqlliveness session ttl",
40*time.Second,
settings.NonNegativeDuration,
)
// DefaultHeartBeat specifies the period between attempts to extend a session.
DefaultHeartBeat = settings.RegisterDurationSetting(
settings.ApplicationLevel,
"server.sqlliveness.heartbeat",
"duration heart beats to push session expiration further out in time",
5*time.Second,
settings.NonNegativeDuration,
)
)
3 changes: 2 additions & 1 deletion pkg/sql/sqlliveness/slinstance/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ go_library(
importpath = "github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slinstance",
visibility = ["//visibility:public"],
deps = [
"//pkg/settings",
"//pkg/settings/cluster",
"//pkg/sql/enum",
"//pkg/sql/sqlliveness",
"//pkg/sql/sqlliveness/slbase",
"//pkg/sql/sqlliveness/slstorage",
"//pkg/util/grpcutil",
"//pkg/util/hlc",
Expand Down Expand Up @@ -38,6 +38,7 @@ go_test(
"//pkg/settings/cluster",
"//pkg/sql/enum",
"//pkg/sql/sqlliveness",
"//pkg/sql/sqlliveness/slbase",
"//pkg/sql/sqlliveness/slstorage",
"//pkg/testutils",
"//pkg/util/hlc",
Expand Down
25 changes: 3 additions & 22 deletions pkg/sql/sqlliveness/slinstance/slinstance.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ import (
"sync"
"time"

"github.com/cockroachdb/cockroach/pkg/settings"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql/enum"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slbase"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slstorage"
"github.com/cockroachdb/cockroach/pkg/util/grpcutil"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
Expand All @@ -36,25 +36,6 @@ import (
"github.com/cockroachdb/logtags"
)

var (
// DefaultTTL specifies the time to expiration when a session is created.
DefaultTTL = settings.RegisterDurationSetting(
settings.ApplicationLevel,
"server.sqlliveness.ttl",
"default sqlliveness session ttl",
40*time.Second,
settings.NonNegativeDuration,
)
// DefaultHeartBeat specifies the period between attempts to extend a session.
DefaultHeartBeat = settings.RegisterDurationSetting(
settings.ApplicationLevel,
"server.sqlliveness.heartbeat",
"duration heart beats to push session expiration further out in time",
5*time.Second,
settings.NonNegativeDuration,
)
)

// Writer provides interactions with the storage of session records.
type Writer interface {
// Insert stores the input Session.
Expand Down Expand Up @@ -413,10 +394,10 @@ func NewSQLInstance(
stopper: stopper,
sessionEvents: sessionEvents,
ttl: func() time.Duration {
return DefaultTTL.Get(&settings.SV)
return slbase.DefaultTTL.Get(&settings.SV)
},
hb: func() time.Duration {
return DefaultHeartBeat.Get(&settings.SV)
return slbase.DefaultHeartBeat.Get(&settings.SV)
},
drain: make(chan struct{}),
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/sql/sqlliveness/slinstance/slinstance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql/enum"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slbase"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slinstance"
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slstorage"
"github.com/cockroachdb/cockroach/pkg/testutils"
Expand All @@ -44,8 +45,8 @@ func TestSQLInstance(t *testing.T) {
clusterversion.Latest.Version(),
clusterversion.MinSupported.Version(),
true /* initializeVersion */)
slinstance.DefaultTTL.Override(ctx, &settings.SV, 20*time.Millisecond)
slinstance.DefaultHeartBeat.Override(ctx, &settings.SV, 10*time.Millisecond)
slbase.DefaultTTL.Override(ctx, &settings.SV, 20*time.Millisecond)
slbase.DefaultHeartBeat.Override(ctx, &settings.SV, 10*time.Millisecond)

fakeStorage := slstorage.NewFakeStorage()
sqlInstance := slinstance.NewSQLInstance(ambientCtx, stopper, clock, fakeStorage, settings, nil, nil)
Expand Down Expand Up @@ -113,8 +114,8 @@ func TestSQLInstanceRelease(t *testing.T) {
clusterversion.Latest.Version(),
clusterversion.MinSupported.Version(),
true /* initializeVersion */)
slinstance.DefaultTTL.Override(ctx, &settings.SV, 20*time.Millisecond)
slinstance.DefaultHeartBeat.Override(ctx, &settings.SV, 10*time.Millisecond)
slbase.DefaultTTL.Override(ctx, &settings.SV, 20*time.Millisecond)
slbase.DefaultHeartBeat.Override(ctx, &settings.SV, 10*time.Millisecond)

fakeStorage := slstorage.NewFakeStorage()
var ambientCtx log.AmbientContext
Expand Down

0 comments on commit 9d04b47

Please sign in to comment.