diff --git a/pkg/ccl/kvccl/kvtenantccl/connector.go b/pkg/ccl/kvccl/kvtenantccl/connector.go index 4ee7f1a93618..a6eef86f5ecd 100644 --- a/pkg/ccl/kvccl/kvtenantccl/connector.go +++ b/pkg/ccl/kvccl/kvtenantccl/connector.go @@ -116,11 +116,6 @@ var _ rangecache.RangeDescriptorDB = (*Connector)(nil) // network. var _ config.SystemConfigProvider = (*Connector)(nil) -// Connector is capable of find the region of every node in the cluster. -// This is necessary for region validation for zone configurations and -// multi-region primitives. -var _ serverpb.RegionsServer = (*Connector)(nil) - // Connector is capable of finding debug information about the current // tenant within the cluster. This is necessary for things such as // debug zip and range reports. @@ -415,7 +410,7 @@ func (c *Connector) RangeLookup( return nil, nil, ctx.Err() } -// Regions implements the serverpb.RegionsServer interface. +// Regions implements the serverpb.TenantStatusServer interface. func (c *Connector) Regions( ctx context.Context, req *serverpb.RegionsRequest, ) (resp *serverpb.RegionsResponse, _ error) { diff --git a/pkg/cmd/roachtest/tests/admission_control_index_overload.go b/pkg/cmd/roachtest/tests/admission_control_index_overload.go index 9411df9d5269..904d49f08ca5 100644 --- a/pkg/cmd/roachtest/tests/admission_control_index_overload.go +++ b/pkg/cmd/roachtest/tests/admission_control_index_overload.go @@ -32,12 +32,9 @@ import ( // queries, but the intent is to measure the impact of the index creation. func registerIndexOverload(r registry.Registry) { r.Add(registry.TestSpec{ - Name: "admission-control/index-overload", - Owner: registry.OwnerAdmissionControl, - // TODO(baptist): After two weeks of nightly baking time, reduce - // this to a weekly cadence. This is a long-running test and serves only - // as a coarse-grained benchmark. - // Tags: []string{`weekly`}, + Name: "admission-control/index-overload", + Owner: registry.OwnerAdmissionControl, + Tags: []string{`weekly`}, Cluster: r.MakeClusterSpec(4, spec.CPU(8)), Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { crdbNodes := c.Spec().NodeCount - 1 diff --git a/pkg/jobs/jobsprotectedts/jobs_protected_ts_manager.go b/pkg/jobs/jobsprotectedts/jobs_protected_ts_manager.go index 037222de2243..65dbcd472015 100644 --- a/pkg/jobs/jobsprotectedts/jobs_protected_ts_manager.go +++ b/pkg/jobs/jobsprotectedts/jobs_protected_ts_manager.go @@ -192,6 +192,7 @@ func (p *Manager) Protect( return nil, err } return func(ctx context.Context) error { + // Remove the protected timestamp. return p.Unprotect(ctx, job) }, nil } @@ -201,19 +202,23 @@ func (p *Manager) Protect( // record. Note: This should only be used for job cleanup if is not currently, // executing. func (p *Manager) Unprotect(ctx context.Context, job *jobs.Job) error { - return p.db.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error { - // Fetch the protected timestamp UUID from the job, if one exists. - protectedtsID := getProtectedTSOnJob(job.Details()) + // Fetch the protected timestamp UUID from the job, if one exists. + if getProtectedTSOnJob(job.Details()) == nil { + return nil + } + // If we do find one then we need to clean up the protected timestamp, + // and remove it from the job. + return job.Update(ctx, nil, func(txn *kv.Txn, md jobs.JobMetadata, ju *jobs.JobUpdater) error { + // The job will get refreshed, so check one more time the protected + // timestamp still exists. The callback returned from Protect works + // on a previously cached copy. + protectedtsID := getProtectedTSOnJob(md.Payload.UnwrapDetails()) if protectedtsID == nil { return nil } - // If we do find one then we need to clean up the protected timestamp, - // and remove it from the job. - return job.Update(ctx, txn, func(txn *kv.Txn, md jobs.JobMetadata, ju *jobs.JobUpdater) error { - updatedDetails := setProtectedTSOnJob(job.Details(), nil) - md.Payload.Details = jobspb.WrapPayloadDetails(updatedDetails) - ju.UpdatePayload(md.Payload) - return p.protectedTSProvider.Release(ctx, txn, *protectedtsID) - }) + updatedDetails := setProtectedTSOnJob(job.Details(), nil) + md.Payload.Details = jobspb.WrapPayloadDetails(updatedDetails) + ju.UpdatePayload(md.Payload) + return p.protectedTSProvider.Release(ctx, txn, *protectedtsID) }) } diff --git a/pkg/kv/kvclient/kvtenant/connector.go b/pkg/kv/kvclient/kvtenant/connector.go index 891bff372e1b..e99c32621534 100644 --- a/pkg/kv/kvclient/kvtenant/connector.go +++ b/pkg/kv/kvclient/kvtenant/connector.go @@ -58,12 +58,7 @@ type Connector interface { // (e.g. is the Range being requested owned by the requesting tenant?). rangecache.RangeDescriptorDB - // RegionsServer provides access to a tenant's available regions. This is - // necessary for region validation for zone configurations and multi-region - // primitives. - serverpb.RegionsServer - - // TenantStatusServer is the subset of the serverpb.StatusInterface that is + // TenantStatusServer is the subset of the serverpb.StatusServer that is // used by the SQL system to query for debug information, such as tenant-specific // range reports. serverpb.TenantStatusServer diff --git a/pkg/kv/kvserver/client_spanconfigs_test.go b/pkg/kv/kvserver/client_spanconfigs_test.go index d5d28435f1a2..c4ed314a2ba3 100644 --- a/pkg/kv/kvserver/client_spanconfigs_test.go +++ b/pkg/kv/kvserver/client_spanconfigs_test.go @@ -100,6 +100,58 @@ func TestSpanConfigUpdateAppliedToReplica(t *testing.T) { }) } +// TestFallbackSpanConfigOverride ensures that +// spanconfig.store.fallback_config_override works as expected. +func TestFallbackSpanConfigOverride(t *testing.T) { + defer leaktest.AfterTest(t)() + + st := cluster.MakeTestingClusterSettings() + spanConfigStore := spanconfigstore.New(roachpb.TestingDefaultSpanConfig(), st, nil) + mockSubscriber := newMockSpanConfigSubscriber(spanConfigStore) + + ctx := context.Background() + args := base.TestServerArgs{ + Knobs: base.TestingKnobs{ + Store: &kvserver.StoreTestingKnobs{ + DisableMergeQueue: true, + DisableSplitQueue: true, + DisableGCQueue: true, + }, + SpanConfig: &spanconfig.TestingKnobs{ + StoreKVSubscriberOverride: mockSubscriber, + }, + }, + } + s, _, _ := serverutils.StartServer(t, args) + defer s.Stopper().Stop(context.Background()) + + _, err := s.InternalExecutor().(sqlutil.InternalExecutor).ExecEx(ctx, "inline-exec", nil, + sessiondata.InternalExecutorOverride{User: username.RootUserName()}, + `SET CLUSTER SETTING spanconfig.store.enabled = true`) + require.NoError(t, err) + + key, err := s.ScratchRange() + require.NoError(t, err) + store, err := s.GetStores().(*kvserver.Stores).GetStore(s.GetFirstStoreID()) + require.NoError(t, err) + repl := store.LookupReplica(keys.MustAddr(key)) + span := repl.Desc().RSpan().AsRawSpanWithNoLocals() + + conf := roachpb.SpanConfig{NumReplicas: 5, NumVoters: 3} + spanconfigstore.FallbackConfigOverride.Override(ctx, &st.SV, &conf) + + require.NotNil(t, mockSubscriber.callback) + mockSubscriber.callback(ctx, span) // invoke the callback + testutils.SucceedsSoon(t, func() error { + repl := store.LookupReplica(keys.MustAddr(key)) + gotConfig := repl.SpanConfig() + if !gotConfig.Equal(conf) { + return errors.Newf("expected config=%s, got config=%s", conf.String(), gotConfig.String()) + } + return nil + }) +} + type mockSpanConfigSubscriber struct { callback func(ctx context.Context, config roachpb.Span) spanconfig.Store diff --git a/pkg/kv/kvserver/store.go b/pkg/kv/kvserver/store.go index 3b1a9d7e3658..1e950873401a 100644 --- a/pkg/kv/kvserver/store.go +++ b/pkg/kv/kvserver/store.go @@ -2129,6 +2129,11 @@ func (s *Store) Start(ctx context.Context, stopper *stop.Stopper) error { } } }) + + // We also want to do it when the fallback config setting is changed. + spanconfigstore.FallbackConfigOverride.SetOnChange(&s.ClusterSettings().SV, func(ctx context.Context) { + s.applyAllFromSpanConfigStore(ctx) + }) } if !s.cfg.TestingKnobs.DisableAutomaticLeaseRenewal { diff --git a/pkg/server/server.go b/pkg/server/server.go index c25e010b4959..610da10b746d 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -896,7 +896,6 @@ func NewServer(cfg Config, stopper *stop.Stopper) (*Server, error) { protectedtsProvider: protectedtsProvider, rangeFeedFactory: rangeFeedFactory, sqlStatusServer: sStatus, - regionsServer: sStatus, tenantStatusServer: sStatus, tenantUsageServer: tenantUsage, monitorAndMetrics: sqlMonitorAndMetrics, diff --git a/pkg/server/server_sql.go b/pkg/server/server_sql.go index 40941d5d41fe..116b61090ec9 100644 --- a/pkg/server/server_sql.go +++ b/pkg/server/server_sql.go @@ -337,9 +337,6 @@ type sqlServerArgs struct { // Used to watch settings and descriptor changes. rangeFeedFactory *rangefeed.Factory - // Used to query valid regions on the server. - regionsServer serverpb.RegionsServer - // Used to query status information useful for debugging on the server. tenantStatusServer serverpb.TenantStatusServer @@ -844,7 +841,6 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) { DistSQLSrv: distSQLServer, NodesStatusServer: cfg.nodesStatusServer, SQLStatusServer: cfg.sqlStatusServer, - RegionsServer: cfg.regionsServer, SessionRegistry: cfg.sessionRegistry, ClosedSessionCache: cfg.closedSessionCache, ContentionRegistry: contentionRegistry, diff --git a/pkg/server/serverpb/status.go b/pkg/server/serverpb/status.go index 94d2ece6c2a6..6e3fbec8d63c 100644 --- a/pkg/server/serverpb/status.go +++ b/pkg/server/serverpb/status.go @@ -16,7 +16,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/util/errorutil" ) -// SQLStatusServer is a smaller version of the serverpb.StatusInterface which +// SQLStatusServer is a smaller version of the serverpb.StatusServer which // includes only the methods used by the SQL subsystem. type SQLStatusServer interface { ListSessions(context.Context, *ListSessionsRequest) (*ListSessionsResponse, error) @@ -71,20 +71,14 @@ type NodesStatusServer interface { ListNodesInternal(context.Context, *NodesRequest) (*NodesResponse, error) } -// RegionsServer is the subset of the serverpb.StatusInterface that is used -// by the SQL system to query for available regions. -// It is available for tenants. -type RegionsServer interface { - Regions(context.Context, *RegionsRequest) (*RegionsResponse, error) -} - -// TenantStatusServer is the subset of the serverpb.StatusInterface that is +// TenantStatusServer is the subset of the serverpb.StatusServer that is // used by tenants to query for debug information, such as tenant-specific // range reports. // // It is available for all tenants. type TenantStatusServer interface { TenantRanges(context.Context, *TenantRangesRequest) (*TenantRangesResponse, error) + Regions(context.Context, *RegionsRequest) (*RegionsResponse, error) } // OptionalNodesStatusServer returns the wrapped NodesStatusServer, if it is diff --git a/pkg/server/status.go b/pkg/server/status.go index b45d67feaa49..496466a38fbb 100644 --- a/pkg/server/status.go +++ b/pkg/server/status.go @@ -1374,7 +1374,7 @@ func (s *statusServer) Profile( return profileLocal(ctx, req, s.st) } -// Regions implements the serverpb.Status interface. +// Regions implements the serverpb.StatusServer interface. func (s *statusServer) Regions( ctx context.Context, req *serverpb.RegionsRequest, ) (*serverpb.RegionsResponse, error) { diff --git a/pkg/server/tenant.go b/pkg/server/tenant.go index 2e43901c0cc1..97536c0494c8 100644 --- a/pkg/server/tenant.go +++ b/pkg/server/tenant.go @@ -909,7 +909,6 @@ func makeTenantSQLServerArgs( circularJobRegistry: circularJobRegistry, protectedtsProvider: protectedTSProvider, rangeFeedFactory: rangeFeedFactory, - regionsServer: tenantConnect, tenantStatusServer: tenantConnect, costController: costController, monitorAndMetrics: monitorAndMetrics, diff --git a/pkg/settings/BUILD.bazel b/pkg/settings/BUILD.bazel index 4e5a2d27950e..afb5928c1dfc 100644 --- a/pkg/settings/BUILD.bazel +++ b/pkg/settings/BUILD.bazel @@ -16,6 +16,7 @@ go_library( "float.go", "int.go", "masked.go", + "protobuf.go", "registry.go", "setting.go", "string.go", @@ -29,10 +30,13 @@ go_library( deps = [ "//pkg/util/buildutil", "//pkg/util/humanizeutil", + "//pkg/util/protoutil", "//pkg/util/syncutil", "@com_github_cockroachdb_errors//:errors", "@com_github_cockroachdb_redact//:redact", "@com_github_cockroachdb_redact//interfaces", + "@com_github_gogo_protobuf//jsonpb", + "@com_github_gogo_protobuf//proto", ], ) diff --git a/pkg/settings/protobuf.go b/pkg/settings/protobuf.go new file mode 100644 index 000000000000..46329dbaa4bf --- /dev/null +++ b/pkg/settings/protobuf.go @@ -0,0 +1,216 @@ +// Copyright 2022 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 settings + +import ( + "context" + "reflect" + "strings" + + "github.com/cockroachdb/cockroach/pkg/util/protoutil" + "github.com/cockroachdb/errors" + "github.com/gogo/protobuf/jsonpb" + "github.com/gogo/protobuf/proto" +) + +// ProtobufSetting is the interface of a setting variable that will be updated +// automatically when the corresponding cluster-wide setting of type "protobuf" +// is updated. The proto message is held in memory, the byte representation +// stored in system.settings, and JSON presentation used when accepting input +// and rendering (just through SHOW CLUSTER SETTING , the raw form +// is visible when looking directly at system.settings). +type ProtobufSetting struct { + defaultValue protoutil.Message + validateFn func(*Values, protoutil.Message) error + common +} + +var _ internalSetting = &ProtobufSetting{} + +// String returns the string representation of the setting's current value. +func (s *ProtobufSetting) String(sv *Values) string { + p := s.Get(sv) + json, err := s.MarshalToJSON(p) + if err != nil { + panic(errors.Wrapf(err, "marshaling %s: %+v", proto.MessageName(p), p)) + } + return json +} + +// Encoded returns the encoded value of the current value of the setting. +func (s *ProtobufSetting) Encoded(sv *Values) string { + p := s.Get(sv) + return EncodeProtobuf(p) +} + +// EncodedDefault returns the encoded value of the default value of the setting. +func (s *ProtobufSetting) EncodedDefault() string { + return EncodeProtobuf(s.defaultValue) +} + +// DecodeToString decodes and renders an encoded value. +func (s *ProtobufSetting) DecodeToString(encoded string) (string, error) { + message, err := s.DecodeValue(encoded) + if err != nil { + return "", err + } + return s.MarshalToJSON(message) +} + +// Typ returns the short (1 char) string denoting the type of setting. +func (*ProtobufSetting) Typ() string { + return "p" +} + +// DecodeValue decodes the value into a protobuf. +func (s *ProtobufSetting) DecodeValue(encoded string) (protoutil.Message, error) { + p, err := newProtoMessage(proto.MessageName(s.defaultValue)) + if err != nil { + return nil, err + } + + if err := protoutil.Unmarshal([]byte(encoded), p); err != nil { + return nil, err + } + return p, nil +} + +// Default returns default value for setting. +func (s *ProtobufSetting) Default() protoutil.Message { + return s.defaultValue +} + +// Get retrieves the protobuf value in the setting. +func (s *ProtobufSetting) Get(sv *Values) protoutil.Message { + loaded := sv.getGeneric(s.slot) + if loaded == nil { + return s.defaultValue + } + return loaded.(protoutil.Message) +} + +// Validate that a value conforms with the validation function. +func (s *ProtobufSetting) Validate(sv *Values, p protoutil.Message) error { + if s.validateFn == nil { + return nil // nothing to do + } + return s.validateFn(sv, p) +} + +// Override sets the setting to the given value, assuming it passes validation. +func (s *ProtobufSetting) Override(ctx context.Context, sv *Values, p protoutil.Message) { + _ = s.set(ctx, sv, p) +} + +func (s *ProtobufSetting) set(ctx context.Context, sv *Values, p protoutil.Message) error { + if err := s.Validate(sv, p); err != nil { + return err + } + if s.Get(sv) != p { + sv.setGeneric(ctx, s.slot, p) + } + return nil +} + +func (s *ProtobufSetting) setToDefault(ctx context.Context, sv *Values) { + if err := s.set(ctx, sv, s.defaultValue); err != nil { + panic(err) + } +} + +// WithPublic sets public visibility and can be chained. +func (s *ProtobufSetting) WithPublic() *ProtobufSetting { + s.SetVisibility(Public) + return s +} + +// MarshalToJSON returns a JSON representation of the protobuf. +func (s *ProtobufSetting) MarshalToJSON(p protoutil.Message) (string, error) { + jsonEncoder := jsonpb.Marshaler{EmitDefaults: false} + return jsonEncoder.MarshalToString(p) +} + +// UnmarshalFromJSON unmarshals a protobuf from a json representation. +func (s *ProtobufSetting) UnmarshalFromJSON(jsonEncoded string) (protoutil.Message, error) { + p, err := newProtoMessage(proto.MessageName(s.defaultValue)) + if err != nil { + return nil, err + } + + json := &jsonpb.Unmarshaler{} + if err := json.Unmarshal(strings.NewReader(jsonEncoded), p); err != nil { + return nil, errors.Wrapf(err, "unmarshaling json to %s", proto.MessageName(p)) + } + return p, nil +} + +// RegisterProtobufSetting defines a new setting with type protobuf. +func RegisterProtobufSetting( + class Class, key, desc string, defaultValue protoutil.Message, +) *ProtobufSetting { + return RegisterValidatedProtobufSetting(class, key, desc, defaultValue, nil) +} + +// RegisterValidatedProtobufSetting defines a new setting with type protobuf +// with a validation function. +func RegisterValidatedProtobufSetting( + class Class, + key, desc string, + defaultValue protoutil.Message, + validateFn func(*Values, protoutil.Message) error, +) *ProtobufSetting { + if validateFn != nil { + if err := validateFn(nil, defaultValue); err != nil { + panic(errors.Wrap(err, "invalid default")) + } + } + setting := &ProtobufSetting{ + defaultValue: defaultValue, + validateFn: validateFn, + } + + // By default, all protobuf settings are considered to contain PII and are + // thus non-reportable (to exclude them from telemetry reports). + setting.SetReportable(false) + register(class, key, desc, setting) + return setting +} + +// Defeat the unused linter. +var _ = (*ProtobufSetting).Default +var _ = (*ProtobufSetting).WithPublic + +// newProtoMessage creates a new protocol message object, given its fully +// qualified name. +func newProtoMessage(name string) (protoutil.Message, error) { + // Get the reflected type of the protocol message. + rt := proto.MessageType(name) + if rt == nil { + return nil, errors.Newf("unknown proto message type %s", name) + } + + // If the message is known, we should get the pointer to our message. + if rt.Kind() != reflect.Ptr { + return nil, errors.AssertionFailedf( + "expected ptr to message, got %s instead", rt.Kind().String()) + } + + // Construct message of appropriate type, through reflection. + rv := reflect.New(rt.Elem()) + msg, ok := rv.Interface().(protoutil.Message) + if !ok { + // Just to be safe. + return nil, errors.AssertionFailedf( + "unexpected proto type for %s; expected protoutil.Message, got %T", + name, rv.Interface()) + } + return msg, nil +} diff --git a/pkg/settings/updater.go b/pkg/settings/updater.go index 8c1e1d87f952..7850247ee1ba 100644 --- a/pkg/settings/updater.go +++ b/pkg/settings/updater.go @@ -15,7 +15,9 @@ import ( "strconv" "time" + "github.com/cockroachdb/cockroach/pkg/util/protoutil" "github.com/cockroachdb/errors" + "github.com/gogo/protobuf/proto" ) // EncodeDuration encodes a duration in the format of EncodedValue.Value. @@ -38,6 +40,15 @@ func EncodeFloat(f float64) string { return strconv.FormatFloat(f, 'G', -1, 64) } +// EncodeProtobuf encodes a protobuf in the format of EncodedValue.Value. +func EncodeProtobuf(p protoutil.Message) string { + data := make([]byte, p.Size()) + if _, err := p.MarshalTo(data); err != nil { + panic(errors.Wrapf(err, "encoding %s: %+v", proto.MessageName(p), p)) + } + return string(data) +} + type updater struct { sv *Values m map[string]struct{} @@ -91,6 +102,12 @@ func (u updater) Set(ctx context.Context, key string, value EncodedValue) error switch setting := d.(type) { case *StringSetting: return setting.set(ctx, u.sv, value.Value) + case *ProtobufSetting: + p, err := setting.DecodeValue(value.Value) + if err != nil { + return err + } + return setting.set(ctx, u.sv, p) case *BoolSetting: b, err := setting.DecodeValue(value.Value) if err != nil { diff --git a/pkg/spanconfig/spanconfigstore/store.go b/pkg/spanconfig/spanconfigstore/store.go index 6beff543e175..1523f6129ce8 100644 --- a/pkg/spanconfig/spanconfigstore/store.go +++ b/pkg/spanconfig/spanconfigstore/store.go @@ -28,7 +28,8 @@ import ( // using the gossip backed system config span to instead using the span configs // infrastructure. It has no effect if COCKROACH_DISABLE_SPAN_CONFIGS // is set. -// TODO(richardjcai): We can likely remove this. +// +// TODO(irfansharif): We should remove this. var EnabledSetting = settings.RegisterBoolSetting( settings.SystemOnly, "spanconfig.store.enabled", @@ -36,6 +37,15 @@ var EnabledSetting = settings.RegisterBoolSetting( true, ) +// FallbackConfigOverride is a hidden cluster setting to override the fallback +// config used for ranges with no explicit span configs set. +var FallbackConfigOverride = settings.RegisterProtobufSetting( + settings.SystemOnly, + "spanconfig.store.fallback_config_override", + "override the fallback used for ranges with no explicit span configs set", + &roachpb.SpanConfig{}, +) + // Store is an in-memory data structure to store, retrieve, and incrementally // update the span configuration state. Internally, it makes use of an interval // btree based spanConfigStore to store non-overlapping span configurations that @@ -47,17 +57,23 @@ type Store struct { systemSpanConfigStore *systemSpanConfigStore } - // TODO(irfansharif): We're using a static fall back span config here, we - // could instead have this track the host tenant's RANGE DEFAULT config, or - // go a step further and use the tenant's own RANGE DEFAULT instead if the - // key is within the tenant's keyspace. We'd have to thread that through the - // KVAccessor interface by reserving special keys for these default configs. - settings *cluster.Settings + // fallback is the span config we'll fall back on in the absence of // something more specific. + // + // TODO(irfansharif): We're using a static[1] fallback span config here, we + // could instead have this directly track the host tenant's RANGE DEFAULT + // config, or go a step further and use the tenant's own RANGE DEFAULT + // instead if the key is within the tenant's keyspace. We'd have to thread + // that through the KVAccessor interface by reserving special keys for these + // default configs. + // + // [1]: Modulo the private spanconfig.store.fallback_config_override, which + // applies globally. fallback roachpb.SpanConfig - knobs *spanconfig.TestingKnobs + + knobs *spanconfig.TestingKnobs } var _ spanconfig.Store = &Store{} @@ -113,11 +129,18 @@ func (s *Store) getSpanConfigForKeyRLocked( ) (roachpb.SpanConfig, error) { conf, found := s.mu.spanConfigStore.getSpanConfigForKey(ctx, key) if !found { - conf = s.fallback + conf = s.getFallbackConfig() } return s.mu.systemSpanConfigStore.combine(key, conf) } +func (s *Store) getFallbackConfig() roachpb.SpanConfig { + if conf := FallbackConfigOverride.Get(&s.settings.SV).(*roachpb.SpanConfig); !conf.IsEmpty() { + return *conf + } + return s.fallback +} + // Apply is part of the spanconfig.StoreWriter interface. func (s *Store) Apply( ctx context.Context, dryrun bool, updates ...spanconfig.Update, diff --git a/pkg/sql/crdb_internal.go b/pkg/sql/crdb_internal.go index be249339f1a4..d453adbc33da 100644 --- a/pkg/sql/crdb_internal.go +++ b/pkg/sql/crdb_internal.go @@ -2817,7 +2817,7 @@ CREATE TABLE crdb_internal.create_function_statements ( tree.NewDInt(tree.DInt(fnIDToScID[fnDesc.GetID()])), // schema_id tree.NewDString(fnIDToScName[fnDesc.GetID()]), // schema_name tree.NewDInt(tree.DInt(fnDesc.GetID())), // function_id - tree.NewDString(fnDesc.GetName()), //function_name + tree.NewDString(fnDesc.GetName()), // function_name tree.NewDString(tree.AsString(treeNode)), // create_statement ) if err != nil { @@ -4594,7 +4594,7 @@ CREATE TABLE crdb_internal.regions ( ) `, populate: func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error { - resp, err := p.extendedEvalCtx.RegionsServer.Regions(ctx, &serverpb.RegionsRequest{}) + resp, err := p.extendedEvalCtx.TenantStatusServer.Regions(ctx, &serverpb.RegionsRequest{}) if err != nil { return err } diff --git a/pkg/sql/exec_util.go b/pkg/sql/exec_util.go index cd7cb016ad14..841f18b1185f 100644 --- a/pkg/sql/exec_util.go +++ b/pkg/sql/exec_util.go @@ -1177,7 +1177,6 @@ type ExecutorConfig struct { // available when not running as a system tenant. SQLStatusServer serverpb.SQLStatusServer TenantStatusServer serverpb.TenantStatusServer - RegionsServer serverpb.RegionsServer MetricsRecorder nodeStatusGenerator SessionRegistry *SessionRegistry ClosedSessionCache *ClosedSessionCache diff --git a/pkg/sql/logictest/logic.go b/pkg/sql/logictest/logic.go index 53e747e87587..667a85494fd4 100644 --- a/pkg/sql/logictest/logic.go +++ b/pkg/sql/logictest/logic.go @@ -1233,6 +1233,10 @@ func getFreePort() (int, error) { return port, err } +// Prevent a lint failure "this value is never used" in +// `(*logicTest).setup` when bazel.BuiltWithBazel returns false. +var _ = ((*logicTest)(nil)).newTestServerCluster + // newTestServerCluster creates a 3-node cluster using the cockroach-go library. // bootstrapBinaryPath is given by the config's CockroachGoBootstrapVersion. // upgradeBinaryPath is given by the config's CockroachGoUpgradeVersion, or @@ -1823,6 +1827,11 @@ func (t *logicTest) setup( if err != nil { t.Fatal(err) } + + // Prevent a lint failure "this value is never used" when + // bazel.BuiltWithBazel returns false above. + _ = bootstrapBinaryPath + localBinaryPath, found := bazel.FindBinary("pkg/cmd/cockroach-short/cockroach-short_/", "cockroach-short") if !found { t.Fatal(errors.New("cockroach binary not found")) diff --git a/pkg/sql/planner.go b/pkg/sql/planner.go index 6e899772acd9..7ca8f4180de8 100644 --- a/pkg/sql/planner.go +++ b/pkg/sql/planner.go @@ -73,10 +73,10 @@ type extendedEvalContext struct { // tenants. NodesStatusServer serverpb.OptionalNodesStatusServer - // RegionsServer gives access to valid regions in the cluster. - RegionsServer serverpb.RegionsServer + // TenantStatusServer gives access to tenant status in the cluster. + TenantStatusServer serverpb.TenantStatusServer - // SQLStatusServer gives access to a subset of the serverpb.Status service + // SQLStatusServer gives access to a subset of the serverpb.StatusServer // that is available to both system and non-system tenants. SQLStatusServer serverpb.SQLStatusServer @@ -127,7 +127,7 @@ func (evalCtx *extendedEvalContext) copyFromExecCfg(execCfg *ExecutorConfig) { evalCtx.NodeID = execCfg.NodeInfo.NodeID evalCtx.Locality = execCfg.Locality evalCtx.NodesStatusServer = execCfg.NodesStatusServer - evalCtx.RegionsServer = execCfg.RegionsServer + evalCtx.TenantStatusServer = execCfg.TenantStatusServer evalCtx.SQLStatusServer = execCfg.SQLStatusServer evalCtx.DistSQLPlanner = execCfg.DistSQLPlanner evalCtx.VirtualSchemas = execCfg.VirtualSchemas diff --git a/pkg/sql/set_cluster_setting.go b/pkg/sql/set_cluster_setting.go index 1815b908d129..5623cd09c4b5 100644 --- a/pkg/sql/set_cluster_setting.go +++ b/pkg/sql/set_cluster_setting.go @@ -178,7 +178,7 @@ func (p *planner) getAndValidateTypedClusterSetting( var dummyHelper tree.IndexedVarHelper switch setting.(type) { - case *settings.StringSetting, *settings.VersionSetting, *settings.ByteSizeSetting: + case *settings.StringSetting, *settings.VersionSetting, *settings.ByteSizeSetting, *settings.ProtobufSetting: requiredType = types.String case *settings.BoolSetting: requiredType = types.Bool @@ -662,6 +662,18 @@ func toSettingString( return string(*s), nil } return "", errors.Errorf("cannot use %s %T value for string setting", d.ResolvedType(), d) + case *settings.ProtobufSetting: + if s, ok := d.(*tree.DString); ok { + msg, err := setting.UnmarshalFromJSON(string(*s)) + if err != nil { + return "", err + } + if err := setting.Validate(&st.SV, msg); err != nil { + return "", err + } + return settings.EncodeProtobuf(msg), nil + } + return "", errors.Errorf("cannot use %s %T value for protobuf setting", d.ResolvedType(), d) case *settings.VersionSetting: if s, ok := d.(*tree.DString); ok { newRawVal, err := clusterversion.EncodingFromVersionStr(string(*s)) diff --git a/pkg/sql/set_zone_config.go b/pkg/sql/set_zone_config.go index 51274b8065d5..f9c11cb93021 100644 --- a/pkg/sql/set_zone_config.go +++ b/pkg/sql/set_zone_config.go @@ -988,7 +988,7 @@ func validateZoneAttrsAndLocalities( } return validateZoneAttrsAndLocalitiesForSystemTenant(ctx, ss.ListNodesInternal, zone) } - return validateZoneLocalitiesForSecondaryTenants(ctx, execCfg.RegionsServer.Regions, zone) + return validateZoneLocalitiesForSecondaryTenants(ctx, execCfg.TenantStatusServer.Regions, zone) } // validateZoneAttrsAndLocalitiesForSystemTenant performs all the constraint/ @@ -1046,13 +1046,13 @@ func validateZoneAttrsAndLocalitiesForSystemTenant( // validateZoneLocalitiesForSecondaryTenants performs all the constraint/lease // preferences validation for secondary tenants. Secondary tenants are only // allowed to reference locality attributes as they only have access to region -// information via the RegionServer. Even then, they're only allowed to -// reference the "region" and "zone" tiers. +// information via the serverpb.TenantStatusServer. Even then, they're only +// allowed to reference the "region" and "zone" tiers. // // Unlike the system tenant, we also validate prohibited constraints. This is // because secondary tenant must operate in the narrow view exposed via the -// RegionServer and are not allowed to configure arbitrary constraints -// (required or otherwise). +// serverpb.TenantStatusServer and are not allowed to configure arbitrary +// constraints (required or otherwise). func validateZoneLocalitiesForSecondaryTenants( ctx context.Context, getRegions regionsGetter, zone *zonepb.ZoneConfig, ) error { diff --git a/pkg/sql/show_cluster_setting.go b/pkg/sql/show_cluster_setting.go index 466b0e40e961..7b1bf6ecfff8 100644 --- a/pkg/sql/show_cluster_setting.go +++ b/pkg/sql/show_cluster_setting.go @@ -155,7 +155,7 @@ func getShowClusterSettingPlanColumns( switch val.(type) { case *settings.IntSetting: dType = types.Int - case *settings.StringSetting, *settings.ByteSizeSetting, *settings.VersionSetting, *settings.EnumSetting: + case *settings.StringSetting, *settings.ByteSizeSetting, *settings.VersionSetting, *settings.EnumSetting, *settings.ProtobufSetting: dType = types.String case *settings.BoolSetting: dType = types.Bool @@ -197,7 +197,7 @@ func planShowClusterSetting( } d = tree.NewDInt(tree.DInt(v)) case *settings.StringSetting, *settings.EnumSetting, - *settings.ByteSizeSetting, *settings.VersionSetting: + *settings.ByteSizeSetting, *settings.VersionSetting, *settings.ProtobufSetting: v, err := val.DecodeToString(encoded) if err != nil { return nil, err