diff --git a/docs/generated/settings/settings-for-tenants.txt b/docs/generated/settings/settings-for-tenants.txt index 34655bc34f31..0a87591cd000 100644 --- a/docs/generated/settings/settings-for-tenants.txt +++ b/docs/generated/settings/settings-for-tenants.txt @@ -44,12 +44,14 @@ feature.restore.enabled boolean true set to true to enable restore, false to dis feature.schema_change.enabled boolean true set to true to enable schema changes, false to disable; default is true tenant-rw feature.stats.enabled boolean true set to true to enable CREATE STATISTICS/ANALYZE, false to disable; default is true tenant-rw jobs.retention_time duration 336h0m0s the amount of time for which records for completed jobs are retained tenant-rw +kv.bulk_sst.target_size byte size 16 MiB target size for SSTs emitted from export requests; export requests (i.e. BACKUP) may buffer up to the sum of kv.bulk_sst.target_size and kv.bulk_sst.max_allowed_overage in memory tenant-ro +kv.closed_timestamp.follower_reads.enabled boolean true allow (all) replicas to serve consistent historical reads based on closed timestamp information tenant-ro kv.closed_timestamp.lead_for_global_reads_override duration 0s if nonzero, overrides the lead time that global_read ranges use to publish closed timestamps tenant-ro kv.closed_timestamp.side_transport_interval duration 200ms the interval at which the closed timestamp side-transport attempts to advance each range's closed timestamp; set to 0 to disable the side-transport tenant-ro kv.closed_timestamp.target_duration duration 3s if nonzero, attempt to provide closed timestamp notifications for timestamps trailing cluster time by approximately this duration tenant-ro kv.protectedts.reconciliation.interval duration 5m0s the frequency for reconciling jobs with protected timestamp records tenant-ro kv.rangefeed.closed_timestamp_refresh_interval duration 3s the interval at which closed-timestamp updatesare delivered to rangefeeds; set to 0 to use kv.closed_timestamp.side_transport_interval tenant-ro -kv.rangefeed.enabled boolean false if set, rangefeed registration is enabled tenant-rw +kv.rangefeed.enabled boolean false if set, rangefeed registration is enabled tenant-ro kv.rangefeed.range_stuck_threshold duration 1m0s restart rangefeeds if they don't emit anything for the specified threshold; 0 disables (kv.rangefeed.closed_timestamp_refresh_interval takes precedence) tenant-rw kv.transaction.max_intents_bytes integer 4194304 maximum number of bytes used to track locks in transactions tenant-rw kv.transaction.max_refresh_spans_bytes integer 4194304 maximum number of bytes used to track refresh spans in serializable transactions tenant-rw @@ -238,6 +240,7 @@ This session variable default should now be configured using ALTER ROLE... SET: sql.distsql.temp_storage.workmem byte size 64 MiB maximum amount of memory in bytes a processor can use before falling back to temp storage tenant-rw sql.guardrails.max_row_size_err byte size 512 MiB maximum size of row (or column family if multiple column families are in use) that SQL can write to the database, above which an error is returned; use 0 to disable tenant-rw sql.guardrails.max_row_size_log byte size 64 MiB maximum size of row (or column family if multiple column families are in use) that SQL can write to the database, above which an event is logged to SQL_PERF (or SQL_INTERNAL_PERF if the mutating statement was internal); use 0 to disable tenant-rw +sql.hash_sharded_range_pre_split.max integer 16 max pre-split ranges to have when adding hash sharded index to an existing table tenant-rw sql.insights.anomaly_detection.enabled boolean true enable per-fingerprint latency recording and anomaly detection tenant-rw sql.insights.anomaly_detection.latency_threshold duration 50ms statements must surpass this threshold to trigger anomaly detection and identification tenant-rw sql.insights.anomaly_detection.memory_limit byte size 1.0 MiB the maximum amount of memory allowed for tracking statement latencies tenant-rw @@ -304,7 +307,8 @@ sql.ttl.job.enabled boolean true whether the TTL job is enabled tenant-rw sql.txn.read_committed_syntax.enabled boolean false set to true to allow transactions to use the READ COMMITTED isolation level if specified by BEGIN/SET commands tenant-rw sql.txn_fingerprint_id_cache.capacity integer 100 the maximum number of txn fingerprint IDs stored tenant-rw storage.max_sync_duration duration 20s maximum duration for disk operations; any operations that take longer than this setting trigger a warning log entry or process crash tenant-ro -storage.max_sync_duration.fatal.enabled boolean true if true, fatal the process when a disk operation exceeds storage.max_sync_duration tenant-ro +storage.max_sync_duration.fatal.enabled boolean true if true, fatal the process when a disk operation exceeds storage.max_sync_duration tenant-rw +storage.value_blocks.enabled boolean true set to true to enable writing of value blocks in sstables tenant-rw timeseries.storage.enabled boolean true if set, periodic timeseries data is stored within the cluster; disabling is not recommended unless you are storing the data elsewhere tenant-rw timeseries.storage.resolution_10s.ttl duration 240h0m0s the maximum age of time series data stored at the 10 second resolution. Data older than this is subject to rollup and deletion. tenant-rw timeseries.storage.resolution_30m.ttl duration 2160h0m0s the maximum age of time series data stored at the 30 minute resolution. Data older than this is subject to deletion. tenant-rw @@ -314,4 +318,5 @@ trace.opentelemetry.collector string address of an OpenTelemetry trace collecto trace.snapshot.rate duration 0s if non-zero, interval at which background trace snapshots are captured tenant-rw trace.span_registry.enabled boolean true if set, ongoing traces can be seen at https:///#/debug/tracez tenant-rw trace.zipkin.collector string the address of a Zipkin instance to receive traces, as :. If no port is specified, 9411 will be used. tenant-rw +ui.display_timezone enumeration etc/utc the timezone used to format timestamps in the ui [etc/utc = 0, america/new_york = 1] tenant-rw version version 1000023.1-26 set the active cluster version in the format '.' tenant-rw diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html index b562f15bbad7..68665ebcf41b 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -59,8 +59,8 @@
kv.allocator.store_cpu_rebalance_threshold
float0.1minimum fraction away from the mean a store's cpu usage can be before it is considered overfull or underfullDedicated/Self-Hosted
kv.bulk_io_write.max_rate
byte size1.0 TiBthe rate limit (bytes/sec) to use for writes to disk on behalf of bulk io opsDedicated/Self-Hosted
kv.bulk_sst.max_allowed_overage
byte size64 MiBif positive, allowed size in excess of target size for SSTs from export requests; export requests (i.e. BACKUP) may buffer up to the sum of kv.bulk_sst.target_size and kv.bulk_sst.max_allowed_overage in memoryDedicated/Self-Hosted -
kv.bulk_sst.target_size
byte size16 MiBtarget size for SSTs emitted from export requests; export requests (i.e. BACKUP) may buffer up to the sum of kv.bulk_sst.target_size and kv.bulk_sst.max_allowed_overage in memoryDedicated/Self-Hosted -
kv.closed_timestamp.follower_reads.enabled
booleantrueallow (all) replicas to serve consistent historical reads based on closed timestamp informationDedicated/Self-Hosted +
kv.bulk_sst.target_size
byte size16 MiBtarget size for SSTs emitted from export requests; export requests (i.e. BACKUP) may buffer up to the sum of kv.bulk_sst.target_size and kv.bulk_sst.max_allowed_overage in memoryServerless/Dedicated/Self-Hosted (read-only) +
kv.closed_timestamp.follower_reads.enabled
booleantrueallow (all) replicas to serve consistent historical reads based on closed timestamp informationServerless/Dedicated/Self-Hosted (read-only)
kv.closed_timestamp.lead_for_global_reads_override
duration0sif nonzero, overrides the lead time that global_read ranges use to publish closed timestampsServerless/Dedicated/Self-Hosted (read-only)
kv.closed_timestamp.side_transport_interval
duration200msthe interval at which the closed timestamp side-transport attempts to advance each range's closed timestamp; set to 0 to disable the side-transportServerless/Dedicated/Self-Hosted (read-only)
kv.closed_timestamp.target_duration
duration3sif nonzero, attempt to provide closed timestamp notifications for timestamps trailing cluster time by approximately this durationServerless/Dedicated/Self-Hosted (read-only) @@ -70,7 +70,7 @@
kv.range_split.load_cpu_threshold
duration500msthe CPU use per second over which, the range becomes a candidate for load based splittingDedicated/Self-Hosted
kv.range_split.load_qps_threshold
integer2500the QPS over which, the range becomes a candidate for load based splittingDedicated/Self-Hosted
kv.rangefeed.closed_timestamp_refresh_interval
duration3sthe interval at which closed-timestamp updatesare delivered to rangefeeds; set to 0 to use kv.closed_timestamp.side_transport_intervalServerless/Dedicated/Self-Hosted (read-only) -
kv.rangefeed.enabled
booleanfalseif set, rangefeed registration is enabledServerless/Dedicated/Self-Hosted +
kv.rangefeed.enabled
booleanfalseif set, rangefeed registration is enabledServerless/Dedicated/Self-Hosted (read-only)
kv.rangefeed.range_stuck_threshold
duration1m0srestart rangefeeds if they don't emit anything for the specified threshold; 0 disables (kv.rangefeed.closed_timestamp_refresh_interval takes precedence)Serverless/Dedicated/Self-Hosted
kv.replica_circuit_breaker.slow_replication_threshold
duration1m0sduration after which slow proposals trip the per-Replica circuit breaker (zero duration disables breakers)Dedicated/Self-Hosted
kv.replica_stats.addsst_request_size_factor
integer50000the divisor that is applied to addsstable request sizes, then recorded in a leaseholders QPS; 0 means all requests are treated as cost 1Dedicated/Self-Hosted @@ -187,7 +187,7 @@
sql.distsql.temp_storage.workmem
byte size64 MiBmaximum amount of memory in bytes a processor can use before falling back to temp storageServerless/Dedicated/Self-Hosted
sql.guardrails.max_row_size_err
byte size512 MiBmaximum size of row (or column family if multiple column families are in use) that SQL can write to the database, above which an error is returned; use 0 to disableServerless/Dedicated/Self-Hosted
sql.guardrails.max_row_size_log
byte size64 MiBmaximum size of row (or column family if multiple column families are in use) that SQL can write to the database, above which an event is logged to SQL_PERF (or SQL_INTERNAL_PERF if the mutating statement was internal); use 0 to disableServerless/Dedicated/Self-Hosted -
sql.hash_sharded_range_pre_split.max
integer16max pre-split ranges to have when adding hash sharded index to an existing tableDedicated/Self-Hosted +
sql.hash_sharded_range_pre_split.max
integer16max pre-split ranges to have when adding hash sharded index to an existing tableServerless/Dedicated/Self-Hosted
sql.insights.anomaly_detection.enabled
booleantrueenable per-fingerprint latency recording and anomaly detectionServerless/Dedicated/Self-Hosted
sql.insights.anomaly_detection.latency_threshold
duration50msstatements must surpass this threshold to trigger anomaly detection and identificationServerless/Dedicated/Self-Hosted
sql.insights.anomaly_detection.memory_limit
byte size1.0 MiBthe maximum amount of memory allowed for tracking statement latenciesServerless/Dedicated/Self-Hosted @@ -255,8 +255,8 @@
sql.txn_fingerprint_id_cache.capacity
integer100the maximum number of txn fingerprint IDs storedServerless/Dedicated/Self-Hosted
storage.experimental.eventually_file_only_snapshots.enabled
booleanfalseset to true to use eventually-file-only-snapshotsDedicated/Self-Hosted
storage.max_sync_duration
duration20smaximum duration for disk operations; any operations that take longer than this setting trigger a warning log entry or process crashServerless/Dedicated/Self-Hosted (read-only) -
storage.max_sync_duration.fatal.enabled
booleantrueif true, fatal the process when a disk operation exceeds storage.max_sync_durationServerless/Dedicated/Self-Hosted (read-only) -
storage.value_blocks.enabled
booleantrueset to true to enable writing of value blocks in sstablesDedicated/Self-Hosted +
storage.max_sync_duration.fatal.enabled
booleantrueif true, fatal the process when a disk operation exceeds storage.max_sync_durationServerless/Dedicated/Self-Hosted +
storage.value_blocks.enabled
booleantrueset to true to enable writing of value blocks in sstablesServerless/Dedicated/Self-Hosted
timeseries.storage.enabled
booleantrueif set, periodic timeseries data is stored within the cluster; disabling is not recommended unless you are storing the data elsewhereServerless/Dedicated/Self-Hosted
timeseries.storage.resolution_10s.ttl
duration240h0m0sthe maximum age of time series data stored at the 10 second resolution. Data older than this is subject to rollup and deletion.Serverless/Dedicated/Self-Hosted
timeseries.storage.resolution_30m.ttl
duration2160h0m0sthe maximum age of time series data stored at the 30 minute resolution. Data older than this is subject to deletion.Serverless/Dedicated/Self-Hosted @@ -266,7 +266,7 @@
trace.snapshot.rate
duration0sif non-zero, interval at which background trace snapshots are capturedServerless/Dedicated/Self-Hosted
trace.span_registry.enabled
booleantrueif set, ongoing traces can be seen at https://<ui>/#/debug/tracezServerless/Dedicated/Self-Hosted
trace.zipkin.collector
stringthe address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.Serverless/Dedicated/Self-Hosted -
ui.display_timezone
enumerationetc/utcthe timezone used to format timestamps in the ui [etc/utc = 0, america/new_york = 1]Dedicated/Self-Hosted +
ui.display_timezone
enumerationetc/utcthe timezone used to format timestamps in the ui [etc/utc = 0, america/new_york = 1]Serverless/Dedicated/Self-Hosted
version
version1000023.1-26set the active cluster version in the format '<major>.<minor>'Serverless/Dedicated/Self-Hosted diff --git a/pkg/base/test_server_args.go b/pkg/base/test_server_args.go index 30b2dd52ba20..f4a4d2f06b06 100644 --- a/pkg/base/test_server_args.go +++ b/pkg/base/test_server_args.go @@ -410,6 +410,29 @@ func DefaultTestTempStorageConfigWithSize( } } +// InheritTestTempStorageConfig is the associated temp storage for +// a secondary tenant. +func InheritTestTempStorageConfig( + st *cluster.Settings, parentConfig TempStorageConfig, +) TempStorageConfig { + monitor := mon.NewMonitor( + "in-mem temp storage", + mon.DiskResource, + nil, /* curCount */ + nil, /* maxHist */ + 1024*1024, /* increment */ + DefaultInMemTempStorageMaxSizeBytes/10, /* noteworthy */ + st, + ) + monitor.Start(context.Background(), parentConfig.Mon, nil /* reserved */) + return TempStorageConfig{ + InMemory: parentConfig.InMemory, + Path: parentConfig.Path, + Mon: monitor, + Settings: st, + } +} + // TestSharedProcessTenantArgs are the arguments to // TestServer.StartSharedProcessTenant. type TestSharedProcessTenantArgs struct { diff --git a/pkg/ccl/changefeedccl/alter_changefeed_test.go b/pkg/ccl/changefeedccl/alter_changefeed_test.go index 3d710b812e4f..28c451b31c3c 100644 --- a/pkg/ccl/changefeedccl/alter_changefeed_test.go +++ b/pkg/ccl/changefeedccl/alter_changefeed_test.go @@ -25,6 +25,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/kv" + "github.com/cockroachdb/cockroach/pkg/kv/kvserver" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/server/telemetry" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" @@ -664,11 +665,17 @@ func TestAlterChangefeedPersistSinkURI(t *testing.T) { const unredactedSinkURI = "null://blah?AWS_ACCESS_KEY_ID=the_secret" - s, rawSQLDB, _ := serverutils.StartServer(t, base.TestServerArgs{}) - sqlDB := sqlutils.MakeSQLRunner(rawSQLDB) - registry := s.ApplicationLayer().JobRegistry().(*jobs.Registry) ctx := context.Background() - defer s.Stopper().Stop(ctx) + srv, rawSQLDB, _ := serverutils.StartServer(t, base.TestServerArgs{}) + defer srv.Stopper().Stop(ctx) + + s := srv.ApplicationLayer() + sqlDB := sqlutils.MakeSQLRunner(rawSQLDB) + registry := s.JobRegistry().(*jobs.Registry) + + for _, l := range []serverutils.ApplicationLayerInterface{s, srv.SystemLayer()} { + kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) + } query := `CREATE TABLE foo (a string)` sqlDB.Exec(t, query) @@ -676,9 +683,6 @@ func TestAlterChangefeedPersistSinkURI(t *testing.T) { query = `CREATE TABLE bar (b string)` sqlDB.Exec(t, query) - query = `SET CLUSTER SETTING kv.rangefeed.enabled = true` - sqlDB.Exec(t, query) - var changefeedID jobspb.JobID doneCh := make(chan struct{}) diff --git a/pkg/ccl/changefeedccl/cdceval/BUILD.bazel b/pkg/ccl/changefeedccl/cdceval/BUILD.bazel index 9a8a86911755..db566987ab17 100644 --- a/pkg/ccl/changefeedccl/cdceval/BUILD.bazel +++ b/pkg/ccl/changefeedccl/cdceval/BUILD.bazel @@ -72,6 +72,7 @@ go_test( "//pkg/jobs/jobspb", "//pkg/keys", "//pkg/kv/kvpb", + "//pkg/kv/kvserver", "//pkg/roachpb", "//pkg/security/securityassets", "//pkg/security/securitytest", diff --git a/pkg/ccl/changefeedccl/cdceval/expr_eval_test.go b/pkg/ccl/changefeedccl/cdceval/expr_eval_test.go index 1e98c7da0ef6..98d3647e9546 100644 --- a/pkg/ccl/changefeedccl/cdceval/expr_eval_test.go +++ b/pkg/ccl/changefeedccl/cdceval/expr_eval_test.go @@ -22,6 +22,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/ccl/changefeedccl/changefeedbase" "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" "github.com/cockroachdb/cockroach/pkg/kv/kvpb" + "github.com/cockroachdb/cockroach/pkg/kv/kvserver" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/cockroachdb/cockroach/pkg/sql" @@ -43,12 +44,17 @@ func TestEvaluator(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) + ctx := context.Background() + srv, db, _ := serverutils.StartServer(t, base.TestServerArgs{}) - defer srv.Stopper().Stop(context.Background()) + defer srv.Stopper().Stop(ctx) s := srv.ApplicationLayer() + for _, l := range []serverutils.ApplicationLayerInterface{s, srv.SystemLayer()} { + kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) + } + sqlDB := sqlutils.MakeSQLRunner(db) - sqlDB.Exec(t, "SET CLUSTER SETTING kv.rangefeed.enabled = true") sqlDB.Exec(t, `CREATE TYPE status AS ENUM ('open', 'closed', 'inactive')`) sqlDB.Exec(t, ` CREATE TABLE foo ( diff --git a/pkg/ccl/changefeedccl/cdcevent/BUILD.bazel b/pkg/ccl/changefeedccl/cdcevent/BUILD.bazel index 4d3aa6efa3e6..0c5a38cd0c93 100644 --- a/pkg/ccl/changefeedccl/cdcevent/BUILD.bazel +++ b/pkg/ccl/changefeedccl/cdcevent/BUILD.bazel @@ -58,6 +58,7 @@ go_test( "//pkg/ccl/changefeedccl/cdctest", "//pkg/ccl/changefeedccl/changefeedbase", "//pkg/jobs/jobspb", + "//pkg/kv/kvserver", "//pkg/roachpb", "//pkg/security/securityassets", "//pkg/security/securitytest", diff --git a/pkg/ccl/changefeedccl/cdcevent/event_test.go b/pkg/ccl/changefeedccl/cdcevent/event_test.go index 18f18818526e..3608ceebd0e8 100644 --- a/pkg/ccl/changefeedccl/cdcevent/event_test.go +++ b/pkg/ccl/changefeedccl/cdcevent/event_test.go @@ -17,6 +17,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/ccl/changefeedccl/cdctest" "github.com/cockroachdb/cockroach/pkg/ccl/changefeedccl/changefeedbase" "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" + "github.com/cockroachdb/cockroach/pkg/kv/kvserver" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql" @@ -137,13 +138,18 @@ func TestEventDecoder(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) + ctx := context.Background() + srv, db, _ := serverutils.StartServer(t, base.TestServerArgs{}) - defer srv.Stopper().Stop(context.Background()) + defer srv.Stopper().Stop(ctx) s := srv.ApplicationLayer() + for _, l := range []serverutils.ApplicationLayerInterface{s, srv.SystemLayer()} { + kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) + } + sqlDB := sqlutils.MakeSQLRunner(db) - sqlDB.Exec(t, "SET CLUSTER SETTING kv.rangefeed.enabled = true") sqlDB.Exec(t, `CREATE TYPE status AS ENUM ('open', 'closed', 'inactive')`) sqlDB.Exec(t, ` CREATE TABLE foo ( @@ -419,12 +425,17 @@ func TestEventColumnOrderingWithSchemaChanges(t *testing.T) { skip.UnderRace(t) skip.UnderStress(t) + ctx := context.Background() + srv, db, _ := serverutils.StartServer(t, base.TestServerArgs{}) - defer srv.Stopper().Stop(context.Background()) + defer srv.Stopper().Stop(ctx) s := srv.ApplicationLayer() + for _, l := range []serverutils.ApplicationLayerInterface{s, srv.SystemLayer()} { + kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) + } + sqlDB := sqlutils.MakeSQLRunner(db) - sqlDB.Exec(t, "SET CLUSTER SETTING kv.rangefeed.enabled = true") // Use alter column type to force column reordering. sqlDB.Exec(t, `SET enable_experimental_alter_column_type_general = true`) diff --git a/pkg/ccl/changefeedccl/changefeed_test.go b/pkg/ccl/changefeedccl/changefeed_test.go index 399c5391a8ed..4870efe431cc 100644 --- a/pkg/ccl/changefeedccl/changefeed_test.go +++ b/pkg/ccl/changefeedccl/changefeed_test.go @@ -4840,7 +4840,7 @@ func TestChangefeedErrors(t *testing.T) { defer log.Scope(t).Close(t) ctx := context.Background() - s, db, _ := serverutils.StartServer(t, base.TestServerArgs{ + srv, db, _ := serverutils.StartServer(t, base.TestServerArgs{ Locality: roachpb.Locality{ Tiers: []roachpb.Tier{{ Key: "region", @@ -4848,10 +4848,13 @@ func TestChangefeedErrors(t *testing.T) { }}, }, }) + defer srv.Stopper().Stop(ctx) + + s := srv.ApplicationLayer() + schemaReg := cdctest.StartTestSchemaRegistry() defer schemaReg.Close() - defer s.Stopper().Stop(ctx) sqlDB := sqlutils.MakeSQLRunner(db) sqlDB.SucceedsSoonDuration = 5 * time.Second @@ -4860,22 +4863,28 @@ func TestChangefeedErrors(t *testing.T) { // Changefeeds default to rangefeed, but for now, rangefeed defaults to off. // Verify that this produces a useful error. - sqlDB.Exec(t, `SET CLUSTER SETTING kv.rangefeed.enabled = false`) + for _, l := range []serverutils.ApplicationLayerInterface{s, srv.SystemLayer()} { + kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, false) + } + sqlDB.Exec(t, `CREATE TABLE rangefeed_off (a INT PRIMARY KEY)`) sqlDB.ExpectErrWithTimeout( t, `rangefeeds require the kv.rangefeed.enabled setting`, `EXPERIMENTAL CHANGEFEED FOR rangefeed_off`, ) - sqlDB.Exec(t, `SET CLUSTER SETTING kv.rangefeed.enabled = true`) + + for _, l := range []serverutils.ApplicationLayerInterface{s, srv.SystemLayer()} { + kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) + } // Feature flag for changefeeds is off — test that CREATE CHANGEFEED and // EXPERIMENTAL CHANGEFEED FOR surface error. - featureChangefeedEnabled.Override(ctx, &s.ApplicationLayer().ClusterSettings().SV, false) + featureChangefeedEnabled.Override(ctx, &s.ClusterSettings().SV, false) sqlDB.ExpectErrWithTimeout(t, `feature CHANGEFEED was disabled by the database administrator`, `CREATE CHANGEFEED FOR foo`) sqlDB.ExpectErrWithTimeout(t, `feature CHANGEFEED was disabled by the database administrator`, `EXPERIMENTAL CHANGEFEED FOR foo`) - featureChangefeedEnabled.Override(ctx, &s.ApplicationLayer().ClusterSettings().SV, true) + featureChangefeedEnabled.Override(ctx, &s.ClusterSettings().SV, true) sqlDB.ExpectErrWithTimeout( t, `unknown format: nope`, diff --git a/pkg/ccl/changefeedccl/show_changefeed_jobs_test.go b/pkg/ccl/changefeedccl/show_changefeed_jobs_test.go index 2da7bdbf0c66..12f01f3d8c41 100644 --- a/pkg/ccl/changefeedccl/show_changefeed_jobs_test.go +++ b/pkg/ccl/changefeedccl/show_changefeed_jobs_test.go @@ -22,6 +22,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/ccl/changefeedccl/changefeedbase" "github.com/cockroachdb/cockroach/pkg/jobs" "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" + "github.com/cockroachdb/cockroach/pkg/kv/kvserver" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" @@ -238,12 +239,21 @@ func TestShowChangefeedJobsStatusChange(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) + ctx := context.Background() + var params base.TestServerArgs params.Knobs.JobsTestingKnobs = jobs.NewTestingKnobsWithShortIntervals() - s, rawSQLDB, _ := serverutils.StartServer(t, params) - registry := s.ApplicationLayer().JobRegistry().(*jobs.Registry) + srv, rawSQLDB, _ := serverutils.StartServer(t, params) + defer srv.Stopper().Stop(ctx) + + s := srv.ApplicationLayer() + + for _, l := range []serverutils.ApplicationLayerInterface{s, srv.SystemLayer()} { + kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) + } + + registry := s.JobRegistry().(*jobs.Registry) sqlDB := sqlutils.MakeSQLRunner(rawSQLDB) - defer s.Stopper().Stop(context.Background()) query := `CREATE TABLE foo (a string)` sqlDB.Exec(t, query) @@ -259,9 +269,6 @@ func TestShowChangefeedJobsStatusChange(t *testing.T) { return &r }) - query = `SET CLUSTER SETTING kv.rangefeed.enabled = true` - sqlDB.Exec(t, query) - var changefeedID jobspb.JobID query = `CREATE CHANGEFEED FOR TABLE foo INTO diff --git a/pkg/ccl/multiregionccl/multiregion_system_table_test.go b/pkg/ccl/multiregionccl/multiregion_system_table_test.go index 1c0b6c308d4e..7dc48c1a32b3 100644 --- a/pkg/ccl/multiregionccl/multiregion_system_table_test.go +++ b/pkg/ccl/multiregionccl/multiregion_system_table_test.go @@ -40,21 +40,26 @@ func TestMrSystemDatabase(t *testing.T) { ctx := context.Background() // Enable settings required for configuring a tenant's system database as multi-region. - cs := cluster.MakeTestingClusterSettings() - instancestorage.ReclaimLoopInterval.Override(ctx, &cs.SV, 150*time.Millisecond) + makeSettings := func() *cluster.Settings { + cs := cluster.MakeTestingClusterSettings() + instancestorage.ReclaimLoopInterval.Override(ctx, &cs.SV, 150*time.Millisecond) + return cs + } - cluster, _, cleanup := multiregionccltestutils.TestingCreateMultiRegionCluster(t, 3, base.TestingKnobs{}, multiregionccltestutils.WithSettings(cs)) + cluster, _, cleanup := multiregionccltestutils.TestingCreateMultiRegionCluster(t, 3, + base.TestingKnobs{}, + multiregionccltestutils.WithSettings(makeSettings())) defer cleanup() id, err := roachpb.MakeTenantID(11) require.NoError(t, err) tenantArgs := base.TestTenantArgs{ - Settings: cs, + Settings: makeSettings(), TenantID: id, Locality: cluster.Servers[0].Locality(), } - _, tenantSQL := serverutils.StartTenant(t, cluster.Servers[0], tenantArgs) + ts, tenantSQL := serverutils.StartTenant(t, cluster.Servers[0], tenantArgs) tDB := sqlutils.MakeSQLRunner(tenantSQL) @@ -143,7 +148,7 @@ func TestMrSystemDatabase(t *testing.T) { FROM system.sql_instances WHERE session_id IS NULL GROUP BY crdb_region ` - preallocatedCount := instancestorage.PreallocatedCount.Get(&cs.SV) + preallocatedCount := instancestorage.PreallocatedCount.Get(&ts.ClusterSettings().SV) testutils.SucceedsSoon(t, func() error { rows := tDB.Query(t, query) require.True(t, rows.Next()) diff --git a/pkg/ccl/multitenantccl/tenantcostclient/BUILD.bazel b/pkg/ccl/multitenantccl/tenantcostclient/BUILD.bazel index 83358e311ecd..5084115ad0c3 100644 --- a/pkg/ccl/multitenantccl/tenantcostclient/BUILD.bazel +++ b/pkg/ccl/multitenantccl/tenantcostclient/BUILD.bazel @@ -64,6 +64,7 @@ go_test( "//pkg/keys", "//pkg/kv/kvclient/kvtenant", "//pkg/kv/kvpb", + "//pkg/kv/kvserver", "//pkg/multitenant", "//pkg/multitenant/multitenantio", "//pkg/multitenant/tenantcostmodel", diff --git a/pkg/ccl/multitenantccl/tenantcostclient/tenant_side_test.go b/pkg/ccl/multitenantccl/tenantcostclient/tenant_side_test.go index 7b7d60478a49..c0f409f78e20 100644 --- a/pkg/ccl/multitenantccl/tenantcostclient/tenant_side_test.go +++ b/pkg/ccl/multitenantccl/tenantcostclient/tenant_side_test.go @@ -34,6 +34,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvtenant" "github.com/cockroachdb/cockroach/pkg/kv/kvpb" + "github.com/cockroachdb/cockroach/pkg/kv/kvserver" "github.com/cockroachdb/cockroach/pkg/multitenant" "github.com/cockroachdb/cockroach/pkg/multitenant/multitenantio" "github.com/cockroachdb/cockroach/pkg/multitenant/tenantcostmodel" @@ -1016,15 +1017,19 @@ func TestScheduledJobsConsumption(t *testing.T) { defer log.Scope(t).Close(t) ctx := context.Background() - st := cluster.MakeTestingClusterSettings() - stats.AutomaticStatisticsClusterMode.Override(ctx, &st.SV, false) - stats.UseStatisticsOnSystemTables.Override(ctx, &st.SV, false) - stats.AutomaticStatisticsOnSystemTables.Override(ctx, &st.SV, false) - tenantcostclient.TargetPeriodSetting.Override(ctx, &st.SV, time.Millisecond*20) + + makeSettings := func() *cluster.Settings { + st := cluster.MakeTestingClusterSettings() + stats.AutomaticStatisticsClusterMode.Override(ctx, &st.SV, false) + stats.UseStatisticsOnSystemTables.Override(ctx, &st.SV, false) + stats.AutomaticStatisticsOnSystemTables.Override(ctx, &st.SV, false) + tenantcostclient.TargetPeriodSetting.Override(ctx, &st.SV, time.Millisecond*20) + return st + } hostServer := serverutils.StartServerOnly(t, base.TestServerArgs{ DefaultTestTenant: base.TestControlsTenantsExplicitly, - Settings: st, + Settings: makeSettings(), }) defer hostServer.Stopper().Stop(ctx) @@ -1037,7 +1042,7 @@ func TestScheduledJobsConsumption(t *testing.T) { var tenantDB *gosql.DB tenantServer, tenantDB = serverutils.StartTenant(t, hostServer, base.TestTenantArgs{ TenantID: serverutils.TestTenantID(), - Settings: st, + Settings: makeSettings(), TestingKnobs: base.TestingKnobs{ TenantTestingKnobs: &sql.TenantTestingKnobs{ OverrideTokenBucketProvider: func(kvtenant.TokenBucketProvider) kvtenant.TokenBucketProvider { @@ -1109,17 +1114,19 @@ func TestConsumptionChangefeeds(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - hostServer, hostDB, _ := serverutils.StartServer(t, base.TestServerArgs{ + ctx := context.Background() + + hostServer := serverutils.StartServerOnly(t, base.TestServerArgs{ DefaultTestTenant: base.TestControlsTenantsExplicitly, }) - defer hostServer.Stopper().Stop(context.Background()) - if _, err := hostDB.Exec("SET CLUSTER SETTING kv.rangefeed.enabled = true"); err != nil { - t.Fatalf("changefeed setup failed: %s", err.Error()) - } + defer hostServer.Stopper().Stop(ctx) + + kvserver.RangefeedEnabled.Override(ctx, &hostServer.ClusterSettings().SV, true) st := cluster.MakeTestingClusterSettings() - tenantcostclient.TargetPeriodSetting.Override(context.Background(), &st.SV, time.Millisecond*20) - tenantcostclient.CPUUsageAllowance.Override(context.Background(), &st.SV, 0) + tenantcostclient.TargetPeriodSetting.Override(ctx, &st.SV, time.Millisecond*20) + tenantcostclient.CPUUsageAllowance.Override(ctx, &st.SV, 0) + kvserver.RangefeedEnabled.Override(ctx, &st.SV, true) testProvider := newTestProvider() @@ -1143,7 +1150,6 @@ func TestConsumptionChangefeeds(t *testing.T) { r.Exec(t, "CREATE TABLE t (v STRING)") r.Exec(t, "INSERT INTO t SELECT repeat('1234567890', 1024) FROM generate_series(1, 10) AS g(i)") beforeChangefeed := testProvider.waitForConsumption(t) - r.Exec(t, "SET CLUSTER SETTING kv.rangefeed.enabled = true") r.Exec(t, "CREATE CHANGEFEED FOR t INTO 'null://'") // Make sure some external io usage is reported. diff --git a/pkg/ccl/serverccl/diagnosticsccl/reporter_test.go b/pkg/ccl/serverccl/diagnosticsccl/reporter_test.go index 33c8e816a3da..cf8c02673e91 100644 --- a/pkg/ccl/serverccl/diagnosticsccl/reporter_test.go +++ b/pkg/ccl/serverccl/diagnosticsccl/reporter_test.go @@ -284,12 +284,10 @@ func TestUsageQuantization(t *testing.T) { r := diagutils.NewServer() defer r.Close() - st := cluster.MakeTestingClusterSettings() ctx := context.Background() url := r.URL() s, db, _ := serverutils.StartServer(t, base.TestServerArgs{ - Settings: st, Knobs: base.TestingKnobs{ Server: &server.TestingKnobs{ DiagnosticsTestingKnobs: diagnostics.TestingKnobs{ @@ -341,7 +339,7 @@ func TestUsageQuantization(t *testing.T) { // The stats "hide" the application name by hashing it. To find the // test app name, we need to hash the ref string too prior to the // comparison. - clusterSecret := sql.ClusterSecret.Get(&st.SV) + clusterSecret := sql.ClusterSecret.Get(&ts.ClusterSettings().SV) hashedAppName := sql.HashForReporting(clusterSecret, "test") require.NotEqual(t, sql.FailedHashedValue, hashedAppName, "expected hashedAppName to not be 'unknown'") diff --git a/pkg/ccl/serverccl/server_sql_test.go b/pkg/ccl/serverccl/server_sql_test.go index 30aef3fa29e2..506bc82ddca3 100644 --- a/pkg/ccl/serverccl/server_sql_test.go +++ b/pkg/ccl/serverccl/server_sql_test.go @@ -383,18 +383,24 @@ func TestTenantInstanceIDReclaimLoop(t *testing.T) { defer log.Scope(t).Close(t) ctx := context.Background() - clusterSettings := cluster.MakeTestingClusterSettings() + clusterSettings := func() *cluster.Settings { + cs := cluster.MakeTestingClusterSettings() + instancestorage.ReclaimLoopInterval.Override(ctx, &cs.SV, 250*time.Millisecond) + instancestorage.PreallocatedCount.Override(ctx, &cs.SV, 5) + return cs + } + s := serverutils.StartServerOnly(t, base.TestServerArgs{ - Settings: clusterSettings, + Settings: clusterSettings(), DefaultTestTenant: base.TestControlsTenantsExplicitly, }) defer s.Stopper().Stop(ctx) - instancestorage.ReclaimLoopInterval.Override(ctx, &clusterSettings.SV, 250*time.Millisecond) - instancestorage.PreallocatedCount.Override(ctx, &clusterSettings.SV, 5) - _, db := serverutils.StartTenant( - t, s, base.TestTenantArgs{TenantID: serverutils.TestTenantID(), Settings: clusterSettings}, + t, s, base.TestTenantArgs{ + TenantID: serverutils.TestTenantID(), + Settings: clusterSettings(), + }, ) defer db.Close() sqlDB := sqlutils.MakeSQLRunner(db) diff --git a/pkg/ccl/serverccl/tenant_migration_test.go b/pkg/ccl/serverccl/tenant_migration_test.go index 515bfc8aea22..387d6814bd7b 100644 --- a/pkg/ccl/serverccl/tenant_migration_test.go +++ b/pkg/ccl/serverccl/tenant_migration_test.go @@ -77,15 +77,18 @@ func TestValidateTargetTenantClusterVersion(t *testing.T) { t.Run(fmt.Sprintf("test %d", i), func(t *testing.T) { defer log.Scope(t).Close(t) - st := cluster.MakeTestingClusterSettingsWithVersions( - test.binaryVersion, - test.binaryMinSupportedVersion, - false, /* initializeVersion */ - ) + makeSettings := func() *cluster.Settings { + st := cluster.MakeTestingClusterSettingsWithVersions( + test.binaryVersion, + test.binaryMinSupportedVersion, + false, /* initializeVersion */ + ) + return st + } s := serverutils.StartServerOnly(t, base.TestServerArgs{ DefaultTestTenant: base.TestControlsTenantsExplicitly, - Settings: st, + Settings: makeSettings(), Knobs: base.TestingKnobs{ Server: &server.TestingKnobs{ BinaryVersionOverride: test.binaryVersion, @@ -100,7 +103,7 @@ func TestValidateTargetTenantClusterVersion(t *testing.T) { ctx := context.Background() upgradePod, err := s.StartTenant(ctx, base.TestTenantArgs{ - Settings: st, + Settings: makeSettings(), TenantID: serverutils.TestTenantID(), TestingKnobs: base.TestingKnobs{ Server: &server.TestingKnobs{ @@ -187,15 +190,18 @@ func TestBumpTenantClusterVersion(t *testing.T) { t.Run(fmt.Sprintf("config=%d", i), func(t *testing.T) { defer log.Scope(t).Close(t) - st := cluster.MakeTestingClusterSettingsWithVersions( - test.binaryVersion.Version, - test.minSupportedVersion.Version, - false, /* initializeVersion */ - ) + makeSettings := func() *cluster.Settings { + st := cluster.MakeTestingClusterSettingsWithVersions( + test.binaryVersion.Version, + test.minSupportedVersion.Version, + false, /* initializeVersion */ + ) + return st + } s := serverutils.StartServerOnly(t, base.TestServerArgs{ DefaultTestTenant: base.TestControlsTenantsExplicitly, - Settings: st, + Settings: makeSettings(), Knobs: base.TestingKnobs{ Server: &server.TestingKnobs{ // This test wants to bootstrap at the previously active @@ -210,9 +216,9 @@ func TestBumpTenantClusterVersion(t *testing.T) { }) defer s.Stopper().Stop(context.Background()) - tenant, err := s.StartTenant(ctx, + tenant, err := s.TenantController().StartTenant(ctx, base.TestTenantArgs{ - Settings: st, + Settings: makeSettings(), TenantID: serverutils.TestTenantID(), TestingKnobs: base.TestingKnobs{ Server: &server.TestingKnobs{ diff --git a/pkg/ccl/telemetryccl/telemetry_logging_test.go b/pkg/ccl/telemetryccl/telemetry_logging_test.go index a77ab7579542..c2be4adfdaba 100644 --- a/pkg/ccl/telemetryccl/telemetry_logging_test.go +++ b/pkg/ccl/telemetryccl/telemetry_logging_test.go @@ -291,7 +291,7 @@ func TestBulkJobTelemetryLogging(t *testing.T) { }, } - sql.TelemetryMaxEventFrequency.Override(context.Background(), &testCluster.Server(0).ClusterSettings().SV, 10) + sql.TelemetryMaxEventFrequency.Override(context.Background(), &testCluster.Server(0).ApplicationLayer().ClusterSettings().SV, 10) // Run all the queries, one after the previous one is finished. var jobID int diff --git a/pkg/ccl/testccl/sqlccl/temp_table_clean_test.go b/pkg/ccl/testccl/sqlccl/temp_table_clean_test.go index ecb3a3dd8885..c10eb5ab1c7d 100644 --- a/pkg/ccl/testccl/sqlccl/temp_table_clean_test.go +++ b/pkg/ccl/testccl/sqlccl/temp_table_clean_test.go @@ -31,13 +31,6 @@ func TestTenantTempTableCleanup(t *testing.T) { ctx := context.Background() t.Helper() - settings := cluster.MakeTestingClusterSettings() - sql.TempObjectCleanupInterval.Override(ctx, &settings.SV, time.Second) - sql.TempObjectWaitInterval.Override(ctx, &settings.SV, time.Second*0) - // Set up sessions to expire within 5 seconds of a - // nodes death. - slinstance.DefaultTTL.Override(ctx, &settings.SV, 5*time.Second) - slinstance.DefaultHeartBeat.Override(ctx, &settings.SV, time.Second) // Knob state is used to track when temporary object clean up // is executed. var ( @@ -98,15 +91,23 @@ func TestTenantTempTableCleanup(t *testing.T) { t, 3 /* numNodes */, base.TestClusterArgs{ReplicationMode: base.ReplicationManual, ServerArgs: base.TestServerArgs{ DefaultTestTenant: base.TestControlsTenantsExplicitly, - Settings: settings, }, }, ) tenantStoppers := []*stop.Stopper{stop.NewStopper(), stop.NewStopper()} + + tenantSettings := cluster.MakeTestingClusterSettings() + sql.TempObjectCleanupInterval.Override(ctx, &tenantSettings.SV, time.Second) + sql.TempObjectWaitInterval.Override(ctx, &tenantSettings.SV, time.Second*0) + // Set up sessions to expire within 5 seconds of a + // nodes death. + slinstance.DefaultTTL.Override(ctx, &tenantSettings.SV, 5*time.Second) + slinstance.DefaultHeartBeat.Override(ctx, &tenantSettings.SV, time.Second) + _, tenantPrimaryDB := serverutils.StartTenant(t, tc.Server(0), base.TestTenantArgs{ TenantID: serverutils.TestTenantID(), - Settings: settings, + Settings: tenantSettings, TestingKnobs: tenantTempKnobSettings, Stopper: tenantStoppers[0], }) @@ -123,7 +124,7 @@ func TestTenantTempTableCleanup(t *testing.T) { _, tenantSecondDB := serverutils.StartTenant(t, tc.Server(1), base.TestTenantArgs{ TenantID: serverutils.TestTenantID(), - Settings: settings, + Settings: tenantSettings, Stopper: tenantStoppers[1], }) tenantSecondSQL := sqlutils.MakeSQLRunner(tenantSecondDB) @@ -164,7 +165,7 @@ func TestTenantTempTableCleanup(t *testing.T) { _, tenantPrimaryDB = serverutils.StartTenant(t, tc.Server(0), base.TestTenantArgs{ TenantID: serverutils.TestTenantID(), - Settings: settings, + Settings: tenantSettings, TestingKnobs: tenantTempKnobSettings, Stopper: tenantStoppers[0]}) tenantSQL = sqlutils.MakeSQLRunner(tenantPrimaryDB) diff --git a/pkg/cmd/roachprod-microbench/google/service.go b/pkg/cmd/roachprod-microbench/google/service.go index b3e4a214d1d2..ec3b46b0e802 100644 --- a/pkg/cmd/roachprod-microbench/google/service.go +++ b/pkg/cmd/roachprod-microbench/google/service.go @@ -84,12 +84,13 @@ func (srv *Service) CreateSheet( var s sheets.Spreadsheet s.Properties = &sheets.SpreadsheetProperties{Title: name} - // Sort sheets by name. + // Sort sheets by name in reverse order. This ensures `sec/op` is the first + // sheet and metric in the summary. sheetNames := make([]string, 0, len(metricMap)) for sheetName := range metricMap { sheetNames = append(sheetNames, sheetName) } - sort.Strings(sheetNames) + sort.Sort(sort.Reverse(sort.StringSlice(sheetNames))) // Raw data sheets. sheetInfos := make([]rawSheetInfo, len(metricMap)) diff --git a/pkg/jobs/jobsprofiler/BUILD.bazel b/pkg/jobs/jobsprofiler/BUILD.bazel index c14958351927..7a41084f0a78 100644 --- a/pkg/jobs/jobsprofiler/BUILD.bazel +++ b/pkg/jobs/jobsprofiler/BUILD.bazel @@ -32,6 +32,7 @@ go_test( "//pkg/jobs", "//pkg/jobs/jobspb", "//pkg/jobs/jobsprofiler/profilerconstants", + "//pkg/kv/kvserver", "//pkg/security/securityassets", "//pkg/security/securitytest", "//pkg/server", diff --git a/pkg/jobs/jobsprofiler/profiler_test.go b/pkg/jobs/jobsprofiler/profiler_test.go index 4054108683ba..161b9f30021f 100644 --- a/pkg/jobs/jobsprofiler/profiler_test.go +++ b/pkg/jobs/jobsprofiler/profiler_test.go @@ -23,6 +23,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" "github.com/cockroachdb/cockroach/pkg/jobs/jobsprofiler" "github.com/cockroachdb/cockroach/pkg/jobs/jobsprofiler/profilerconstants" + "github.com/cockroachdb/cockroach/pkg/kv/kvserver" "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/sql/execinfrapb" "github.com/cockroachdb/cockroach/pkg/sql/isql" @@ -56,8 +57,10 @@ func TestProfilerStorePlanDiagram(t *testing.T) { require.NoError(t, err) _, err = sqlDB.Exec(`INSERT INTO foo VALUES (1), (2)`) require.NoError(t, err) - _, err = sqlDB.Exec(`SET CLUSTER SETTING kv.rangefeed.enabled = true`) - require.NoError(t, err) + + for _, l := range []serverutils.ApplicationLayerInterface{s.ApplicationLayer(), s.SystemLayer()} { + kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) + } for _, tc := range []struct { name string diff --git a/pkg/kv/kvclient/rangefeed/rangefeed_external_test.go b/pkg/kv/kvclient/rangefeed/rangefeed_external_test.go index 6ad4b1d57c64..4d14e9744abd 100644 --- a/pkg/kv/kvclient/rangefeed/rangefeed_external_test.go +++ b/pkg/kv/kvclient/rangefeed/rangefeed_external_test.go @@ -122,9 +122,12 @@ func TestRangeFeedIntegration(t *testing.T) { // Enable rangefeeds, otherwise the thing will retry until they are enabled. for _, l := range []serverutils.ApplicationLayerInterface{ts, srv.SystemLayer()} { + // Whether rangefeeds are enabled is a property of both the + // storage layer and the application layer. kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) - kvserver.RangeFeedUseScheduler.Override(ctx, &l.ClusterSettings().SV, s.useScheduler) } + // Scheduler use is a property of the storage layer. + kvserver.RangeFeedUseScheduler.Override(ctx, &srv.SystemLayer().ClusterSettings().SV, s.useScheduler) f, err := rangefeed.NewFactory(ts.AppStopper(), db, ts.ClusterSettings(), nil) require.NoError(t, err) @@ -211,10 +214,11 @@ func TestWithOnFrontierAdvance(t *testing.T) { for _, l := range []serverutils.ApplicationLayerInterface{ts, srv.SystemLayer()} { // Enable rangefeeds, otherwise the thing will retry until they are enabled. kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) - kvserver.RangeFeedUseScheduler.Override(ctx, &l.ClusterSettings().SV, s.useScheduler) // Lower the closed timestamp target duration to speed up the test. closedts.TargetDuration.Override(ctx, &l.ClusterSettings().SV, 100*time.Millisecond) } + // Scheduler use is a property of the storage layer. + kvserver.RangeFeedUseScheduler.Override(ctx, &srv.SystemLayer().ClusterSettings().SV, s.useScheduler) // Split the range into two so we know the frontier has more than one span to // track for certain. We later write to both these ranges. @@ -333,10 +337,11 @@ func TestWithOnCheckpoint(t *testing.T) { for _, l := range []serverutils.ApplicationLayerInterface{ts, srv.SystemLayer()} { // Enable rangefeeds, otherwise the thing will retry until they are enabled. kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) - kvserver.RangeFeedUseScheduler.Override(ctx, &l.ClusterSettings().SV, s.useScheduler) // Lower the closed timestamp target duration to speed up the test. closedts.TargetDuration.Override(ctx, &l.ClusterSettings().SV, 100*time.Millisecond) } + // Scheduler use is a property of the storage layer. + kvserver.RangeFeedUseScheduler.Override(ctx, &srv.SystemLayer().ClusterSettings().SV, s.useScheduler) f, err := rangefeed.NewFactory(ts.AppStopper(), db, ts.ClusterSettings(), nil) require.NoError(t, err) @@ -434,10 +439,11 @@ func TestRangefeedValueTimestamps(t *testing.T) { for _, l := range []serverutils.ApplicationLayerInterface{ts, srv.SystemLayer()} { // Enable rangefeeds, otherwise the thing will retry until they are enabled. kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) - kvserver.RangeFeedUseScheduler.Override(ctx, &l.ClusterSettings().SV, s.useScheduler) // Lower the closed timestamp target duration to speed up the test. closedts.TargetDuration.Override(ctx, &l.ClusterSettings().SV, 100*time.Millisecond) } + // Scheduler use is a property of the storage layer. + kvserver.RangeFeedUseScheduler.Override(ctx, &srv.SystemLayer().ClusterSettings().SV, s.useScheduler) f, err := rangefeed.NewFactory(ts.AppStopper(), db, ts.ClusterSettings(), nil) require.NoError(t, err) @@ -542,8 +548,10 @@ func TestWithOnSSTable(t *testing.T) { for _, l := range []serverutils.ApplicationLayerInterface{tsrv, srv.SystemLayer()} { // Enable rangefeeds, otherwise the thing will retry until they are enabled. kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) - kvserver.RangeFeedUseScheduler.Override(ctx, &l.ClusterSettings().SV, s.useScheduler) } + // Scheduler use is a property of the storage layer. + kvserver.RangeFeedUseScheduler.Override(ctx, &srv.SystemLayer().ClusterSettings().SV, s.useScheduler) + f, err := rangefeed.NewFactory(tsrv.AppStopper(), db, tsrv.ClusterSettings(), nil) require.NoError(t, err) @@ -648,11 +656,12 @@ func TestWithOnSSTableCatchesUpIfNotSet(t *testing.T) { require.NoError(t, err) require.NoError(t, tc.WaitForFullReplication()) - for _, l := range []serverutils.ApplicationLayerInterface{tsrv, tsrv.SystemLayer()} { + for _, l := range []serverutils.ApplicationLayerInterface{srv, tsrv.SystemLayer()} { // Enable rangefeeds, otherwise the thing will retry until they are enabled. kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) - kvserver.RangeFeedUseScheduler.Override(ctx, &l.ClusterSettings().SV, s.useScheduler) } + // Scheduler use is a property of the storage layer. + kvserver.RangeFeedUseScheduler.Override(ctx, &tsrv.SystemLayer().ClusterSettings().SV, s.useScheduler) f, err := rangefeed.NewFactory(srv.AppStopper(), db, srv.ClusterSettings(), nil) require.NoError(t, err) @@ -755,11 +764,12 @@ func TestWithOnDeleteRange(t *testing.T) { require.NoError(t, err) require.NoError(t, tc.WaitForFullReplication()) - for _, l := range []serverutils.ApplicationLayerInterface{tsrv, tsrv.SystemLayer()} { + for _, l := range []serverutils.ApplicationLayerInterface{srv, tsrv.SystemLayer()} { // Enable rangefeeds, otherwise the thing will retry until they are enabled. kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) - kvserver.RangeFeedUseScheduler.Override(ctx, &l.ClusterSettings().SV, s.useScheduler) } + // Scheduler use is a property of the storage layer. + kvserver.RangeFeedUseScheduler.Override(ctx, &tsrv.SystemLayer().ClusterSettings().SV, s.useScheduler) f, err := rangefeed.NewFactory(srv.AppStopper(), db, srv.ClusterSettings(), nil) require.NoError(t, err) @@ -944,10 +954,11 @@ func TestUnrecoverableErrors(t *testing.T) { for _, l := range []serverutils.ApplicationLayerInterface{ts, srv.SystemLayer()} { // Enable rangefeeds, otherwise the thing will retry until they are enabled. kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) - kvserver.RangeFeedUseScheduler.Override(ctx, &l.ClusterSettings().SV, s.useScheduler) // Lower the closed timestamp target duration to speed up the test. closedts.TargetDuration.Override(ctx, &l.ClusterSettings().SV, 100*time.Millisecond) } + // Scheduler use is a property of the storage layer. + kvserver.RangeFeedUseScheduler.Override(ctx, &srv.SystemLayer().ClusterSettings().SV, s.useScheduler) store, err := srv.GetStores().(*kvserver.Stores).GetStore(srv.GetFirstStoreID()) require.NoError(t, err) @@ -1033,10 +1044,11 @@ func TestMVCCHistoryMutationError(t *testing.T) { for _, l := range []serverutils.ApplicationLayerInterface{ts, srv.SystemLayer()} { // Enable rangefeeds, otherwise the thing will retry until they are enabled. kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) - kvserver.RangeFeedUseScheduler.Override(ctx, &l.ClusterSettings().SV, s.useScheduler) // Lower the closed timestamp target duration to speed up the test. closedts.TargetDuration.Override(ctx, &l.ClusterSettings().SV, 100*time.Millisecond) } + // Scheduler use is a property of the storage layer. + kvserver.RangeFeedUseScheduler.Override(ctx, &srv.SystemLayer().ClusterSettings().SV, s.useScheduler) // Set up a rangefeed. f, err := rangefeed.NewFactory(ts.AppStopper(), db, ts.ClusterSettings(), nil) @@ -1131,8 +1143,9 @@ func TestRangefeedWithLabelsOption(t *testing.T) { for _, l := range []serverutils.ApplicationLayerInterface{ts, srv.SystemLayer()} { // Enable rangefeeds, otherwise the thing will retry until they are enabled. kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) - kvserver.RangeFeedUseScheduler.Override(ctx, &l.ClusterSettings().SV, s.useScheduler) } + // Scheduler use is a property of the storage layer. + kvserver.RangeFeedUseScheduler.Override(ctx, &srv.SystemLayer().ClusterSettings().SV, s.useScheduler) const rangefeedName = "test-feed" type label struct { @@ -1239,8 +1252,9 @@ func TestRangeFeedStartTimeExclusive(t *testing.T) { for _, l := range []serverutils.ApplicationLayerInterface{ts, srv.SystemLayer()} { // Enable rangefeeds, otherwise the thing will retry until they are enabled. kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) - kvserver.RangeFeedUseScheduler.Override(ctx, &l.ClusterSettings().SV, s.useScheduler) } + // Scheduler use is a property of the storage layer. + kvserver.RangeFeedUseScheduler.Override(ctx, &srv.SystemLayer().ClusterSettings().SV, s.useScheduler) f, err := rangefeed.NewFactory(ts.AppStopper(), db, ts.ClusterSettings(), nil) require.NoError(t, err) diff --git a/pkg/kv/kvserver/batcheval/cmd_export.go b/pkg/kv/kvserver/batcheval/cmd_export.go index 1ed47e7c3c2d..3145dfb1468d 100644 --- a/pkg/kv/kvserver/batcheval/cmd_export.go +++ b/pkg/kv/kvserver/batcheval/cmd_export.go @@ -38,7 +38,7 @@ const SSTTargetSizeSetting = "kv.bulk_sst.target_size" // ExportRequestTargetFileSize controls the target file size for SSTs created // during backups. var ExportRequestTargetFileSize = settings.RegisterByteSizeSetting( - settings.SystemOnly, + settings.TenantReadOnly, // used by BACKUP SSTTargetSizeSetting, fmt.Sprintf("target size for SSTs emitted from export requests; "+ "export requests (i.e. BACKUP) may buffer up to the sum of %s and %s in memory", diff --git a/pkg/kv/kvserver/kvserverbase/base.go b/pkg/kv/kvserver/kvserverbase/base.go index 88ff413b10f2..8fce50282cb6 100644 --- a/pkg/kv/kvserver/kvserverbase/base.go +++ b/pkg/kv/kvserver/kvserverbase/base.go @@ -279,7 +279,7 @@ func IntersectSpan( // SplitByLoadMergeDelay wraps "kv.range_split.by_load_merge_delay". var SplitByLoadMergeDelay = settings.RegisterDurationSetting( - settings.SystemOnly, + settings.TenantReadOnly, // used by TRUNCATE in SQL "kv.range_split.by_load_merge_delay", "the delay that range splits created due to load will wait before considering being merged away", 5*time.Minute, diff --git a/pkg/kv/kvserver/protectedts/settings.go b/pkg/kv/kvserver/protectedts/settings.go index 92780de471a7..fe9f63555655 100644 --- a/pkg/kv/kvserver/protectedts/settings.go +++ b/pkg/kv/kvserver/protectedts/settings.go @@ -33,7 +33,7 @@ var MaxBytes = settings.RegisterIntSetting( // MaxSpans controls the maximum number of spans which can be protected // by all protected timestamp records. var MaxSpans = settings.RegisterIntSetting( - settings.SystemOnly, + settings.TenantWritable, "kv.protectedts.max_spans", "if non-zero the limit of the number of spans which can be protected", 32768, diff --git a/pkg/kv/kvserver/replica_follower_read.go b/pkg/kv/kvserver/replica_follower_read.go index 6aae45e780f3..7fac760c443d 100644 --- a/pkg/kv/kvserver/replica_follower_read.go +++ b/pkg/kv/kvserver/replica_follower_read.go @@ -27,7 +27,7 @@ import ( // information is collected and passed around, regardless of the value of this // setting. var FollowerReadsEnabled = settings.RegisterBoolSetting( - settings.SystemOnly, + settings.TenantReadOnly, // needed for planning in SQL "kv.closed_timestamp.follower_reads_enabled", "allow (all) replicas to serve consistent historical reads based on closed timestamp information", true, diff --git a/pkg/kv/kvserver/replica_rangefeed.go b/pkg/kv/kvserver/replica_rangefeed.go index 5a59c2a494bd..f00c5e952b3b 100644 --- a/pkg/kv/kvserver/replica_rangefeed.go +++ b/pkg/kv/kvserver/replica_rangefeed.go @@ -47,7 +47,7 @@ import ( // ranges and ranges covering tables in the system database); this setting // covers everything else. var RangefeedEnabled = settings.RegisterBoolSetting( - settings.TenantWritable, + settings.TenantReadOnly, "kv.rangefeed.enabled", "if set, rangefeed registration is enabled", false, diff --git a/pkg/kv/kvserver/store.go b/pkg/kv/kvserver/store.go index b9cb1f36ae70..007a779fe65b 100644 --- a/pkg/kv/kvserver/store.go +++ b/pkg/kv/kvserver/store.go @@ -276,7 +276,7 @@ const leaseTransferWaitSettingName = "server.shutdown.lease_transfer_wait" // here since we check it in the caller to limit generated requests as well // to prevent excessive queuing. var ExportRequestsLimit = settings.RegisterIntSetting( - settings.SystemOnly, + settings.TenantReadOnly, // used in backup processor "kv.bulk_io_write.concurrent_export_requests", "number of export requests a store will handle concurrently before queuing", 3, diff --git a/pkg/server/admin.go b/pkg/server/admin.go index c203ea8f4500..ca92fd2dde68 100644 --- a/pkg/server/admin.go +++ b/pkg/server/admin.go @@ -2017,6 +2017,12 @@ func (s *adminServer) Settings( } } + showSystem := s.sqlServer.execCfg.Codec.ForSystemTenant() + target := settings.ForVirtualCluster + if showSystem { + target = settings.ForSystemTenant + } + // settingsKeys is the list of setting keys to retrieve. settingsKeys := make([]settings.InternalKey, 0, len(req.Keys)) for _, desiredSetting := range req.Keys { @@ -2030,7 +2036,7 @@ func (s *adminServer) Settings( } if !consoleSettingsOnly { if len(settingsKeys) == 0 { - settingsKeys = settings.Keys(settings.ForSystemTenant) + settingsKeys = settings.Keys(target) } } else { if len(settingsKeys) == 0 { @@ -2076,13 +2082,14 @@ func (s *adminServer) Settings( var v settings.Setting var ok bool if redactValues { - v, ok = settings.LookupForReportingByKey(k, settings.ForSystemTenant) + v, ok = settings.LookupForReportingByKey(k, target) } else { - v, ok = settings.LookupForLocalAccessByKey(k, settings.ForSystemTenant) + v, ok = settings.LookupForLocalAccessByKey(k, target) } if !ok { continue } + var altered *time.Time if val, ok := alteredSettings[k]; ok { altered = val diff --git a/pkg/server/application_api/config_test.go b/pkg/server/application_api/config_test.go index fa7d33c4e6ab..744027359ef1 100644 --- a/pkg/server/application_api/config_test.go +++ b/pkg/server/application_api/config_test.go @@ -42,21 +42,21 @@ func TestAdminAPISettings(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - srv, conn, _ := serverutils.StartServer(t, base.TestServerArgs{ - // Disable the default test tenant for now as this tests fails - // with it enabled. Tracked with #81590. - // DefaultTestTenant: base.TODOTestTenantDisabled, - }) + srv, conn, _ := serverutils.StartServer(t, base.TestServerArgs{}) defer srv.Stopper().Stop(context.Background()) s := srv.ApplicationLayer() // Any bool that defaults to true will work here. const settingKey = "sql.metrics.statement_details.enabled" st := s.ClusterSettings() - allKeys := settings.Keys(settings.ForSystemTenant) + target := settings.ForSystemTenant + if srv.TenantController().StartedDefaultTestTenant() { + target = settings.ForVirtualCluster + } + allKeys := settings.Keys(target) checkSetting := func(t *testing.T, k settings.InternalKey, v serverpb.SettingsResponse_Value) { - ref, ok := settings.LookupForReportingByKey(k, settings.ForSystemTenant) + ref, ok := settings.LookupForReportingByKey(k, target) if !ok { t.Fatalf("%s: not found after initial lookup", k) } @@ -105,7 +105,7 @@ func TestAdminAPISettings(t *testing.T) { t.Fatal(err) } - // Check that all expected keys were returned + // Check that all expected keys were returned. if len(allKeys) != len(resp.KeyValues) { t.Fatalf("expected %d keys, got %d", len(allKeys), len(resp.KeyValues)) } @@ -161,7 +161,13 @@ func TestAdminAPISettings(t *testing.T) { t.Run("different-permissions", func(t *testing.T) { var resp serverpb.SettingsResponse nonAdminUser := apiconstants.TestingUserNameNoAdmin().Normalized() - consoleKeys := append([]settings.InternalKey(nil), settings.ConsoleKeys()...) + var consoleKeys []settings.InternalKey + for _, k := range settings.ConsoleKeys() { + if _, ok := settings.LookupForLocalAccessByKey(k, target); !ok { + continue + } + consoleKeys = append(consoleKeys, k) + } sort.Slice(consoleKeys, func(i, j int) bool { return consoleKeys[i] < consoleKeys[j] }) // Admin should return all cluster settings. diff --git a/pkg/server/server.go b/pkg/server/server.go index c27416b1b039..54e4dcda11e2 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -240,6 +240,10 @@ func NewServer(cfg Config, stopper *stop.Stopper) (serverctl.ServerStartupInterf st := cfg.Settings + // Ensure that we don't mistakenly reuse the same Values container + // across servers (e.g. misuse of TestServer API). + st.SV.SpecializeForSystemInterface() + if cfg.AmbientCtx.Tracer == nil { panic(errors.New("no tracer set in AmbientCtx")) } diff --git a/pkg/server/settingswatcher/row_decoder_external_test.go b/pkg/server/settingswatcher/row_decoder_external_test.go index c814a6016834..aaf3ef811a76 100644 --- a/pkg/server/settingswatcher/row_decoder_external_test.go +++ b/pkg/server/settingswatcher/row_decoder_external_test.go @@ -45,12 +45,12 @@ func TestRowDecoder(t *testing.T) { expStr string expValType string }{ - "kv.rangefeed.enabled": { + "trace.span_registry.enabled": { val: true, expStr: "true", expValType: "b", }, - "kv.queue.process.guaranteed_time_budget": { + "sql.trace.txn.enable_threshold": { val: "17s", expStr: "17s", expValType: "d", diff --git a/pkg/server/settingswatcher/settings_watcher_external_test.go b/pkg/server/settingswatcher/settings_watcher_external_test.go index 7fc65a730f6e..f88b6b757373 100644 --- a/pkg/server/settingswatcher/settings_watcher_external_test.go +++ b/pkg/server/settingswatcher/settings_watcher_external_test.go @@ -152,7 +152,7 @@ func TestSettingWatcherOnTenant(t *testing.T) { copySettingsFromSystemToFakeTenant() tenantSettings := cluster.MakeTestingClusterSettings() - tenantSettings.SV.SetNonSystemTenant() + tenantSettings.SV.SpecializeForVirtualCluster() // Needed for backward-compat on crdb_internal.ranges{_no_leases}. // Remove in v23.2. diff --git a/pkg/server/structlogging/hot_ranges_log_test.go b/pkg/server/structlogging/hot_ranges_log_test.go index a1d669963280..3ee3b907bf9b 100644 --- a/pkg/server/structlogging/hot_ranges_log_test.go +++ b/pkg/server/structlogging/hot_ranges_log_test.go @@ -60,18 +60,17 @@ func TestHotRangesStats(t *testing.T) { }) defer s.Stopper().Stop(ctx) - logcrash.DiagnosticsReportingEnabled.Override(ctx, &s.ClusterSettings().SV, true) - structlogging.TelemetryHotRangesStatsEnabled.Override(ctx, &s.ClusterSettings().SV, true) - structlogging.TelemetryHotRangesStatsInterval.Override(ctx, &s.ClusterSettings().SV, 500*time.Millisecond) - structlogging.TelemetryHotRangesStatsLoggingDelay.Override(ctx, &s.ClusterSettings().SV, 10*time.Millisecond) - tenantID := roachpb.MustMakeTenantID(2) - tt, err := s.StartTenant(ctx, base.TestTenantArgs{ + tt, err := s.TenantController().StartTenant(ctx, base.TestTenantArgs{ TenantID: tenantID, - Settings: s.ClusterSettings(), }) require.NoError(t, err) + logcrash.DiagnosticsReportingEnabled.Override(ctx, &tt.ClusterSettings().SV, true) + structlogging.TelemetryHotRangesStatsEnabled.Override(ctx, &tt.ClusterSettings().SV, true) + structlogging.TelemetryHotRangesStatsInterval.Override(ctx, &tt.ClusterSettings().SV, 500*time.Millisecond) + structlogging.TelemetryHotRangesStatsLoggingDelay.Override(ctx, &tt.ClusterSettings().SV, 10*time.Millisecond) + testutils.SucceedsSoon(t, func() error { ss := tt.TenantStatusServer().(serverpb.TenantStatusServer) resp, err := ss.HotRangesV2(ctx, &serverpb.HotRangesRequest{TenantID: tenantID.String()}) diff --git a/pkg/server/tenant.go b/pkg/server/tenant.go index 47c9076b3c4f..a6d3d1255d0e 100644 --- a/pkg/server/tenant.go +++ b/pkg/server/tenant.go @@ -1020,6 +1020,10 @@ func makeTenantSQLServerArgs( ) (sqlServerArgs, error) { st := baseCfg.Settings + // Ensure that the settings accessor bark if an attempt is made + // to use a SystemOnly setting. + st.SV.SpecializeForVirtualCluster() + // We want all log messages issued on behalf of this SQL instance to report // the instance ID (once known) as a tag. startupCtx = baseCfg.AmbientCtx.AnnotateCtx(startupCtx) diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go index 332b57bd25e8..e09ca2424f52 100644 --- a/pkg/server/testserver.go +++ b/pkg/server/testserver.go @@ -589,14 +589,24 @@ func (ts *testServer) maybeStartDefaultTestTenant(ctx context.Context) error { return nil } - tenantSettings := ts.params.Settings - if tenantSettings == nil { - tenantSettings = cluster.MakeTestingClusterSettings() - } - tempStorageConfig := ts.params.TempStorageConfig - if tempStorageConfig.Settings == nil { + tenantSettings := cluster.MakeTestingClusterSettings() + if st := ts.params.Settings; st != nil { + // Copy overrides and other test-specific configuration, + // as a convenience for test writers that do the following: + // - create a new Settings + // - add some overrides + // - call serverutils.StartServer + // - expect the overrides to propagate to the application layer. + tenantSettings.SV.TestingCopyForVirtualCluster(&st.SV) + } + + var tempStorageConfig base.TempStorageConfig + if tsc := ts.params.TempStorageConfig; tsc.Settings != nil { + tempStorageConfig = base.InheritTestTempStorageConfig(tenantSettings, tsc) + } else { tempStorageConfig = base.DefaultTestTempStorageConfig(tenantSettings) } + params := base.TestTenantArgs{ // Currently, all the servers leverage the same tenant ID. We may // want to change this down the road, for more elaborate testing. diff --git a/pkg/settings/registry.go b/pkg/settings/registry.go index 08dc56505ab3..9ca76245666e 100644 --- a/pkg/settings/registry.go +++ b/pkg/settings/registry.go @@ -395,6 +395,10 @@ func LookupForReportingByKey(key InternalKey, forSystemTenant bool) (Setting, bo // tenant. const ForSystemTenant = true +// ForVirtualCluster can be passed to Lookup for code that runs on +// virtual clusters. +const ForVirtualCluster = false + // ReadableTypes maps our short type identifiers to friendlier names. var ReadableTypes = map[string]string{ "s": "string", diff --git a/pkg/settings/settings_test.go b/pkg/settings/settings_test.go index d06a2375d476..e5b1af3445fd 100644 --- a/pkg/settings/settings_test.go +++ b/pkg/settings/settings_test.go @@ -802,13 +802,13 @@ func TestOverride(t *testing.T) { require.Equal(t, 42.0, overrideFloat.Get(sv)) } -func TestSystemOnlyDisallowedOnTenant(t *testing.T) { +func TestSystemOnlyDisallowedOnVirtualCluster(t *testing.T) { skip.UnderNonTestBuild(t) ctx := context.Background() sv := &settings.Values{} sv.Init(ctx, settings.TestOpaque) - sv.SetNonSystemTenant() + sv.SpecializeForVirtualCluster() // Check that we can still read non-SystemOnly settings. if expected, actual := "", strFooA.Get(sv); expected != actual { @@ -819,7 +819,7 @@ func TestSystemOnlyDisallowedOnTenant(t *testing.T) { defer func() { if r := recover(); r == nil { t.Error("Get did not panic") - } else if !strings.Contains(fmt.Sprint(r), "attempted to set forbidden setting") { + } else if !strings.Contains(fmt.Sprint(r), "invalid access to SystemOnly") { t.Errorf("received unexpected panic: %v", r) } }() diff --git a/pkg/settings/updater.go b/pkg/settings/updater.go index a051d702a851..e5023381573a 100644 --- a/pkg/settings/updater.go +++ b/pkg/settings/updater.go @@ -175,7 +175,7 @@ func (u updater) ResetRemaining(ctx context.Context) { u.sv.setValueOrigin(ctx, v.getSlot(), OriginDefault) } - if u.sv.NonSystemTenant() && v.Class() == SystemOnly { + if u.sv.SpecializedToVirtualCluster() && v.Class() == SystemOnly { // Don't try to reset system settings on a non-system tenant. continue } diff --git a/pkg/settings/values.go b/pkg/settings/values.go index 8e9bd0c7ac6a..3967730e1d72 100644 --- a/pkg/settings/values.go +++ b/pkg/settings/values.go @@ -30,7 +30,7 @@ const MaxSettings = 1023 type Values struct { container valuesContainer - nonSystemTenant bool + classCheck classCheck defaultOverridesMu struct { syncutil.Mutex @@ -51,6 +51,22 @@ type Values struct { opaque interface{} } +type classCheck int8 + +const ( + // classCheckUndefined is used when the settings.Values hasn't been + // specialized yet. + classCheckUndefined classCheck = iota + // classCheckSystemInterface is used when the settings.Values is + // specialized for the system interface and SystemOnly settings can + // be used. + classCheckSystemInterface + // classCheckVirtualCluster is used when the settings.Values is + // specialized for a virtual cluster and SystemOnly settings cannot + // be used. + classCheckVirtualCluster +) + const numSlots = MaxSettings + 1 type valuesContainer struct { @@ -95,7 +111,14 @@ func (c *valuesContainer) getGeneric(slot slotIdx) interface{} { func (c *valuesContainer) checkForbidden(slot slotIdx) bool { if c.forbidden[slot] { if buildutil.CrdbTestBuild { - panic(errors.AssertionFailedf("attempted to set forbidden setting %s", slotTable[slot].Name())) + const msg = `programming error: invalid access to SystemOnly setting %s from a virtual cluster! + +TIP: use class TenantWritable for settings that configure just 1 +virtual cluster; SystemOnly for settings that affect only the shared +storage layer; and TenantReadOnly for settings that affect the storage +layer and also must be visible to all virtual clusters. +` + panic(errors.AssertionFailedf(msg, slotTable[slot].Name())) } return false } @@ -119,10 +142,28 @@ func (sv *Values) Init(ctx context.Context, opaque interface{}) { } } -// SetNonSystemTenant marks this container as pertaining to a non-system tenant, -// after which use of SystemOnly values is disallowed. -func (sv *Values) SetNonSystemTenant() { - sv.nonSystemTenant = true +const alreadySpecializedError = `programming error: setting value container is already specialized! + +TIP: avoid using the same cluster.Settings or settings.Value object across multiple servers. +` + +// SpecializeForSystemInterface marks the values container as +// pertaining to the system interface. +func (sv *Values) SpecializeForSystemInterface() { + if sv.classCheck != classCheckUndefined && sv.classCheck != classCheckSystemInterface { + panic(errors.AssertionFailedf(alreadySpecializedError)) + } + sv.classCheck = classCheckSystemInterface +} + +// SpecializeForVirtualCluster marks this container as pertaining to +// a virtual cluster, after which use of SystemOnly values is +// disallowed. +func (sv *Values) SpecializeForVirtualCluster() { + if sv.classCheck != classCheckUndefined && sv.classCheck != classCheckVirtualCluster { + panic(errors.AssertionFailedf(alreadySpecializedError)) + } + sv.classCheck = classCheckVirtualCluster for slot, setting := range slotTable { if setting != nil && setting.Class() == SystemOnly { sv.container.forbidden[slot] = true @@ -130,10 +171,10 @@ func (sv *Values) SetNonSystemTenant() { } } -// NonSystemTenant returns true if this container is for a non-system tenant -// (i.e. SetNonSystemTenant() was called). -func (sv *Values) NonSystemTenant() bool { - return sv.nonSystemTenant +// SpecializedToVirtualCluster returns true if this container is for a +// virtual cluster (i.e. SpecializeToVirtualCluster() was called). +func (sv *Values) SpecializedToVirtualCluster() bool { + return sv.classCheck == classCheckVirtualCluster } // Opaque returns the argument passed to Init. @@ -204,3 +245,31 @@ func (sv *Values) setOnChange(slot slotIdx, fn func(ctx context.Context)) { sv.changeMu.onChange[slot] = append(sv.changeMu.onChange[slot], fn) sv.changeMu.Unlock() } + +// TestingCopyForVirtualCluster makes a copy of the input Values in +// the target Values for use when initializing a server for a virtual +// cluster in tests. This is meant to propagate overrides +// to TenantWritable settings. +func (sv *Values) TestingCopyForVirtualCluster(input *Values) { + for slot := slotIdx(0); slot < slotIdx(len(registry)); slot++ { + s := slotTable[slot] + if s.Class() != TenantWritable && s.Class() != TenantReadOnly { + continue + } + + // Copy the value. + sv.container.intVals[slot] = input.container.intVals[slot] + if v := input.container.genericVals[slot].Load(); v != nil { + sv.container.genericVals[slot].Store(v) + } + + // Copy the default. + input.defaultOverridesMu.Lock() + v, hasVal := input.defaultOverridesMu.defaultOverrides[slot] + input.defaultOverridesMu.Unlock() + if !hasVal { + continue + } + sv.setDefaultOverride(slot, v) + } +} diff --git a/pkg/sql/colflow/draining_test.go b/pkg/sql/colflow/draining_test.go index 15fa6c2760b7..cc31bfc1a29e 100644 --- a/pkg/sql/colflow/draining_test.go +++ b/pkg/sql/colflow/draining_test.go @@ -56,6 +56,7 @@ func TestDrainingAfterRemoteError(t *testing.T) { tempStorageConfig := base.TempStorageConfig{InMemory: true, Mon: diskMonitor, Settings: st} args := base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ + Settings: st, TempStorageConfig: tempStorageConfig, }, ReplicationMode: base.ReplicationManual, diff --git a/pkg/sql/exec_util.go b/pkg/sql/exec_util.go index c5207d153595..a22d9e3a0155 100644 --- a/pkg/sql/exec_util.go +++ b/pkg/sql/exec_util.go @@ -353,7 +353,7 @@ var overrideMultiRegionZoneConfigClusterMode = settings.RegisterBoolSetting( settings.WithPublic) var maxHashShardedIndexRangePreSplit = settings.RegisterIntSetting( - settings.SystemOnly, + settings.TenantWritable, "sql.hash_sharded_range_pre_split.max", "max pre-split ranges to have when adding hash sharded index to an existing table", 16, diff --git a/pkg/sql/sqlinstance/instancestorage/BUILD.bazel b/pkg/sql/sqlinstance/instancestorage/BUILD.bazel index d0258f6789ee..d33e5710a138 100644 --- a/pkg/sql/sqlinstance/instancestorage/BUILD.bazel +++ b/pkg/sql/sqlinstance/instancestorage/BUILD.bazel @@ -69,6 +69,7 @@ go_test( "//pkg/kv", "//pkg/kv/kvclient/kvtenant", "//pkg/kv/kvclient/rangefeed", + "//pkg/kv/kvserver", "//pkg/roachpb", "//pkg/security/securityassets", "//pkg/security/securitytest", diff --git a/pkg/sql/sqlinstance/instancestorage/instancereader_test.go b/pkg/sql/sqlinstance/instancestorage/instancereader_test.go index ae85f7b1b138..062a79c799ce 100644 --- a/pkg/sql/sqlinstance/instancestorage/instancereader_test.go +++ b/pkg/sql/sqlinstance/instancestorage/instancereader_test.go @@ -18,6 +18,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangefeed" + "github.com/cockroachdb/cockroach/pkg/kv/kvserver" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/server/settingswatcher" "github.com/cockroachdb/cockroach/pkg/sql/catalog/desctestutils" @@ -46,7 +47,10 @@ func TestReader(t *testing.T) { s := srv.ApplicationLayer() tDB := sqlutils.MakeSQLRunner(sqlDB) // Enable rangefeed for the test. - tDB.Exec(t, `SET CLUSTER SETTING kv.rangefeed.enabled = true`) + for _, l := range []serverutils.ApplicationLayerInterface{s, srv.SystemLayer()} { + kvserver.RangefeedEnabled.Override(ctx, &l.ClusterSettings().SV, true) + } + setup := func(t *testing.T) ( *instancestorage.Storage, *slstorage.FakeStorage, *hlc.Clock, *instancestorage.Reader, ) { diff --git a/pkg/sql/sqlstats/insights/integration/insights_test.go b/pkg/sql/sqlstats/insights/integration/insights_test.go index 6ad78314cd7e..16cb6637cbe9 100644 --- a/pkg/sql/sqlstats/insights/integration/insights_test.go +++ b/pkg/sql/sqlstats/insights/integration/insights_test.go @@ -667,11 +667,12 @@ func TestInsightsIndexRecommendationIntegration(t *testing.T) { tc := testcluster.StartTestCluster(t, 1, args) defer tc.Stopper().Stop(ctx) + ts := tc.ApplicationLayer(0) + sqlConn := tc.ServerConn(0) + // Enable detection by setting a latencyThreshold > 0. latencyThreshold := 30 * time.Millisecond - insights.LatencyThreshold.Override(ctx, &settings.SV, latencyThreshold) - - sqlConn := tc.ServerConn(0) + insights.LatencyThreshold.Override(ctx, &ts.ClusterSettings().SV, latencyThreshold) _, err := sqlConn.ExecContext(ctx, "CREATE TABLE t1 (k INT, i INT, f FLOAT, s STRING)") require.NoError(t, err) diff --git a/pkg/sql/sqlstats/persistedsqlstats/BUILD.bazel b/pkg/sql/sqlstats/persistedsqlstats/BUILD.bazel index ed85b3a762e6..7777724f9db0 100644 --- a/pkg/sql/sqlstats/persistedsqlstats/BUILD.bazel +++ b/pkg/sql/sqlstats/persistedsqlstats/BUILD.bazel @@ -87,7 +87,6 @@ go_test( "//pkg/security/securitytest", "//pkg/security/username", "//pkg/server", - "//pkg/settings/cluster", "//pkg/sql", "//pkg/sql/appstatspb", "//pkg/sql/catalog", diff --git a/pkg/sql/sqlstats/persistedsqlstats/controller_test.go b/pkg/sql/sqlstats/persistedsqlstats/controller_test.go index c05f65e40c0a..62f4ec6b305c 100644 --- a/pkg/sql/sqlstats/persistedsqlstats/controller_test.go +++ b/pkg/sql/sqlstats/persistedsqlstats/controller_test.go @@ -18,7 +18,6 @@ import ( "testing" "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/sqlstats/persistedsqlstats" "github.com/cockroachdb/cockroach/pkg/testutils" @@ -263,9 +262,7 @@ func TestStmtStatsEnable(t *testing.T) { // Start the cluster. (One node is sufficient; the outliers system is currently in-memory only.) ctx := context.Background() - settings := cluster.MakeTestingClusterSettings() - args := base.TestClusterArgs{ServerArgs: base.TestServerArgs{Settings: settings}} - tc := testcluster.StartTestCluster(t, 1, args) + tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{}) defer tc.Stopper().Stop(ctx) serverutils.SetClusterSetting(t, tc, "sql.metrics.statement_details.enabled", "false") diff --git a/pkg/sql/tenant_accessors.go b/pkg/sql/tenant_accessors.go index 264eba4dc60d..d34ab1b4a493 100644 --- a/pkg/sql/tenant_accessors.go +++ b/pkg/sql/tenant_accessors.go @@ -197,7 +197,7 @@ func GetExtendedTenantInfo( } var defaultTenantConfigTemplate = settings.RegisterStringSetting( - settings.SystemOnly, + settings.TenantWritable, "sql.create_tenant.default_template", "tenant to use as configuration template when LIKE is not specified in CREATE VIRTUAL CLUSTER", // We use the empty string so that no template is used by default diff --git a/pkg/sql/tests/autocommit_extended_protocol_test.go b/pkg/sql/tests/autocommit_extended_protocol_test.go index 282ba1cd8bb3..4f3630cadd9d 100644 --- a/pkg/sql/tests/autocommit_extended_protocol_test.go +++ b/pkg/sql/tests/autocommit_extended_protocol_test.go @@ -19,7 +19,6 @@ import ( "testing" "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/catalog/descs" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" @@ -172,7 +171,6 @@ func TestErrorDuringExtendedProtocolCommit(t *testing.T) { return nil }, } - params.Settings = cluster.MakeTestingClusterSettings() s, db, _ := serverutils.StartServer(t, params) defer s.Stopper().Stop(ctx) diff --git a/pkg/sql/tests/monotonic_insert_test.go b/pkg/sql/tests/monotonic_insert_test.go index 607b434c0770..58a303f5e1ae 100644 --- a/pkg/sql/tests/monotonic_insert_test.go +++ b/pkg/sql/tests/monotonic_insert_test.go @@ -108,13 +108,14 @@ func testMonotonicInserts(t *testing.T, distSQLMode sessiondatapb.DistSQLExecMod defer tc.Stopper().Stop(ctx) for _, server := range tc.Servers { - st := server.ClusterSettings() + st := server.ApplicationLayer().ClusterSettings() st.Manual.Store(true) sql.DistSQLClusterExecMode.Override(ctx, &st.SV, int64(distSQLMode)) // Let transactions push immediately to detect deadlocks. The test creates a // large amount of contention and dependency cycles, and could take a long // time to complete without this. - concurrency.LockTableDeadlockDetectionPushDelay.Override(ctx, &st.SV, 0) + concurrency.LockTableDeadlockDetectionPushDelay.Override(ctx, + &server.SystemLayer().ClusterSettings().SV, 0) } var clients []mtClient diff --git a/pkg/sql/vars.go b/pkg/sql/vars.go index b5d2a4ed97ae..e04c01411646 100644 --- a/pkg/sql/vars.go +++ b/pkg/sql/vars.go @@ -1619,7 +1619,8 @@ var varGen = map[string]sessionVar{ // - The session var is named "disable_" because we want the Go // default value (false) to mean that tenant deletion is enabled. // This is needed for backward-compatibility with Cockroach Cloud. - return formatBoolAsPostgresSetting(!enableDropVirtualCluster.Get(sv)) + enabled := !sv.SpecializedToVirtualCluster() && !enableDropVirtualCluster.Get(sv) + return formatBoolAsPostgresSetting(enabled) }, }, diff --git a/pkg/storage/pebble.go b/pkg/storage/pebble.go index c7c2a00ac7f9..d975e3628a20 100644 --- a/pkg/storage/pebble.go +++ b/pkg/storage/pebble.go @@ -80,7 +80,7 @@ var MaxSyncDuration = settings.RegisterDurationSetting( // MaxSyncDurationFatalOnExceeded governs whether disk stalls longer than // MaxSyncDuration fatal the Cockroach process. Defaults to true. var MaxSyncDurationFatalOnExceeded = settings.RegisterBoolSetting( - settings.TenantReadOnly, + settings.TenantWritable, // used for temp storage in virtual cluster servers "storage.max_sync_duration.fatal.enabled", "if true, fatal the process when a disk operation exceeds storage.max_sync_duration", true, @@ -92,7 +92,7 @@ var MaxSyncDurationFatalOnExceeded = settings.RegisterBoolSetting( // compactions, and does not eagerly change the encoding of existing sstables. // Reads can correctly read both kinds of sstables. var ValueBlocksEnabled = settings.RegisterBoolSetting( - settings.SystemOnly, + settings.TenantWritable, // used for temp storage in virtual cluster servers "storage.value_blocks.enabled", "set to true to enable writing of value blocks in sstables", util.ConstantWithMetamorphicTestBool( @@ -123,7 +123,7 @@ var UseEFOS = settings.RegisterBoolSetting( // // This cluster setting will be removed in a subsequent release. var IngestAsFlushable = settings.RegisterBoolSetting( - settings.SystemOnly, + settings.TenantWritable, // used to init temp storage in virtual cluster servers "storage.ingest_as_flushable.enabled", "set to true to enable lazy ingestion of sstables", util.ConstantWithMetamorphicTestBool( diff --git a/pkg/ui/ui.go b/pkg/ui/ui.go index bdb063a53864..ec918b7d2f1b 100644 --- a/pkg/ui/ui.go +++ b/pkg/ui/ui.go @@ -39,7 +39,7 @@ const ( ) var _ = settings.RegisterEnumSetting( - settings.SystemOnly, + settings.TenantWritable, "ui.display_timezone", "the timezone used to format timestamps in the ui", "Etc/UTC", diff --git a/pkg/util/schedulerlatency/sampler.go b/pkg/util/schedulerlatency/sampler.go index f0cbc5668681..77fb34e17dbd 100644 --- a/pkg/util/schedulerlatency/sampler.go +++ b/pkg/util/schedulerlatency/sampler.go @@ -34,7 +34,7 @@ import ( // requests in work queues as part of this tick). Might be worth checking for // grantees more frequently independent of this sample period. var samplePeriod = settings.RegisterDurationSetting( - settings.SystemOnly, + settings.TenantWritable, // used in virtual clusters "scheduler_latency.sample_period", "controls the duration between consecutive scheduler latency samples", 100*time.Millisecond, @@ -47,7 +47,7 @@ var samplePeriod = settings.RegisterDurationSetting( ) var sampleDuration = settings.RegisterDurationSetting( - settings.SystemOnly, + settings.TenantWritable, // used in virtual clusters "scheduler_latency.sample_duration", "controls the duration over which each scheduler latency sample is a measurement over", 2500*time.Millisecond, diff --git a/pkg/util/tracing/BUILD.bazel b/pkg/util/tracing/BUILD.bazel index 1452ce55cea2..cc3fbfb2c49a 100644 --- a/pkg/util/tracing/BUILD.bazel +++ b/pkg/util/tracing/BUILD.bazel @@ -75,7 +75,6 @@ go_test( "//pkg/security/securitytest", "//pkg/server", "//pkg/settings", - "//pkg/settings/cluster", "//pkg/sql", "//pkg/testutils/serverutils", "//pkg/testutils/skip", diff --git a/pkg/util/tracing/tracer_external_test.go b/pkg/util/tracing/tracer_external_test.go index 18ac78834604..1ab3498fa30b 100644 --- a/pkg/util/tracing/tracer_external_test.go +++ b/pkg/util/tracing/tracer_external_test.go @@ -20,7 +20,6 @@ import ( _ "github.com/cockroachdb/cockroach/pkg/ccl" // For tenant functionality. "github.com/cockroachdb/cockroach/pkg/kv/kvbase" "github.com/cockroachdb/cockroach/pkg/server" - "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" "github.com/cockroachdb/cockroach/pkg/testutils/skip" @@ -106,12 +105,11 @@ func TestTraceForTenantWithLocalKVServer(t *testing.T) { defer log.Scope(t).Close(t) ctx := context.Background() - st := cluster.MakeTestingClusterSettings() - server.RedactServerTracesForSecondaryTenants.Override(ctx, &st.SV, false) - args := base.TestServerArgs{Settings: st} - s := serverutils.StartServerOnly(t, args) + s := serverutils.StartServerOnly(t, base.TestServerArgs{}) defer s.Stopper().Stop(ctx) + server.RedactServerTracesForSecondaryTenants.Override(ctx, &s.SystemLayer().ClusterSettings().SV, false) + // Create our own test tenant with a known name. const tenantName = "test-tenant"