diff --git a/pkg/ccl/crosscluster/physical/stream_ingestion_planning.go b/pkg/ccl/crosscluster/physical/stream_ingestion_planning.go index f3138e3fde8f..70b7defa3954 100644 --- a/pkg/ccl/crosscluster/physical/stream_ingestion_planning.go +++ b/pkg/ccl/crosscluster/physical/stream_ingestion_planning.go @@ -54,7 +54,6 @@ func streamIngestionJobDescription( ReplicationSourceTenantName: streamIngestion.ReplicationSourceTenantName, ReplicationSourceAddress: tree.NewDString(redactedSourceAddr), Options: streamIngestion.Options, - Like: streamIngestion.Like, } ann := p.ExtendedEvalContext().Annotations return tree.AsStringWithFQNames(redactedCreateStmt, ann), nil @@ -74,11 +73,6 @@ func ingestionTypeCheck( ingestionStmt.ReplicationSourceAddress, ingestionStmt.Options.Retention}, } - if ingestionStmt.Like.OtherTenant != nil { - toTypeCheck = append(toTypeCheck, - exprutil.TenantSpec{TenantSpec: ingestionStmt.Like.OtherTenant}, - ) - } if err := exprutil.TypeCheck(ctx, "INGESTION", p.SemaCtx(), toTypeCheck...); err != nil { return false, nil, err @@ -117,15 +111,6 @@ func ingestionPlanHook( return nil, nil, nil, false, err } - var likeTenantID uint64 - var likeTenantName string - if ingestionStmt.Like.OtherTenant != nil { - _, likeTenantID, likeTenantName, err = exprEval.TenantSpec(ctx, ingestionStmt.Like.OtherTenant) - if err != nil { - return nil, nil, nil, false, err - } - } - evalCtx := &p.ExtendedEvalContext().Context options, err := evalTenantReplicationOptions(ctx, ingestionStmt.Options, exprEval, evalCtx, p.SemaCtx(), createReplicationOp) if err != nil { @@ -175,12 +160,8 @@ func ingestionPlanHook( // If we don't have a resume timestamp, make a new tenant jobID := p.ExecCfg().JobRegistry.MakeJobID() var destinationTenantID roachpb.TenantID - // Determine which template will be used as config template to - // create the new tenant below. - tenantInfo, err := sql.GetTenantTemplate(ctx, p.ExecCfg().Settings, p.InternalSQLTxn(), nil, likeTenantID, likeTenantName) - if err != nil { - return err - } + + var tenantInfo mtinfopb.TenantInfoWithUsage // Create a new tenant for the replication stream. tenantInfo.PhysicalReplicationConsumerJobID = jobID @@ -197,7 +178,7 @@ func ingestionPlanHook( ctx, p.ExecCfg().Codec, p.ExecCfg().Settings, p.InternalSQLTxn(), p.ExecCfg().SpanConfigKVAccessor.WithISQLTxn(ctx, p.InternalSQLTxn()), - tenantInfo, initialTenantZoneConfig, + &tenantInfo, initialTenantZoneConfig, ingestionStmt.IfNotExists, p.ExecCfg().TenantTestingKnobs, ) diff --git a/pkg/cmd/release/update_versions.go b/pkg/cmd/release/update_versions.go index 5d3783ba006e..5f2c16356cae 100644 --- a/pkg/cmd/release/update_versions.go +++ b/pkg/cmd/release/update_versions.go @@ -289,8 +289,9 @@ func updateVersions(_ *cobra.Command, _ []string) error { for _, repo := range reposToWorkOn { repo.workOnRepoError = workOnRepo(repo) if repo.workOnRepoError != nil { - log.Printf("workOnRepo: error occurred while working on %s: %s", repo.name(), repo.workOnRepoError) - workOnRepoErrors = append(workOnRepoErrors, repo.workOnRepoError) + err = fmt.Errorf("workOnRepo: error occurred while working on repo %s: %w", repo.name(), err) + workOnRepoErrors = append(workOnRepoErrors, err) + log.Printf("%s", err) } } @@ -305,7 +306,10 @@ func updateVersions(_ *cobra.Command, _ []string) error { // run multiple times. prDesc, err := repo.prExists() if err != nil { - return fmt.Errorf("checking pr: %w", err) + err = fmt.Errorf("error while checking if pull request exists for repo %s: %w", repo.name(), err) + workOnRepoErrors = append(workOnRepoErrors, err) + log.Printf("%s", err) + continue } if prDesc != "" { log.Printf("pull request for %s already exists: %s", repo.name(), prDesc) @@ -313,19 +317,27 @@ func updateVersions(_ *cobra.Command, _ []string) error { } log.Printf("pushing changes to repo %s in %s", repo.name(), dest) if err := repo.push(); err != nil { - return fmt.Errorf("cannot push changes for %s: %w", repo.name(), err) + err = fmt.Errorf("error while pushing changes to repo %s: %w", repo.name(), err) + workOnRepoErrors = append(workOnRepoErrors, err) + log.Printf("%s", err) + continue } log.Printf("creating pull request for %s in %s", repo.name(), dest) pr, err := repo.createPullRequest() if err != nil { - return fmt.Errorf("cannot create pull request for %s: %w", repo.name(), err) + err = fmt.Errorf("error creating pull request for %s: %w", repo.name(), err) + workOnRepoErrors = append(workOnRepoErrors, err) + log.Printf("%s", err) + continue } log.Printf("Created PR: %s\n", pr) prs = append(prs, pr) } if err := sendPrReport(releasedVersion, prs, smtpPassword); err != nil { - return fmt.Errorf("cannot send email: %w", err) + err = fmt.Errorf("error sending email: %w", err) + workOnRepoErrors = append(workOnRepoErrors, err) + log.Printf("%s", err) } if len(workOnRepoErrors) > 0 { return errors.Join(workOnRepoErrors...) diff --git a/pkg/kv/kvserver/kvflowcontrol/rac2/range_controller.go b/pkg/kv/kvserver/kvflowcontrol/rac2/range_controller.go index ecd528836124..9f1a4cb5ac3d 100644 --- a/pkg/kv/kvserver/kvflowcontrol/rac2/range_controller.go +++ b/pkg/kv/kvserver/kvflowcontrol/rac2/range_controller.go @@ -78,8 +78,8 @@ type RangeController interface { // TODO(pav-kv): This interface a placeholder for the interface containing raft // methods. Replace this as part of #128019. type RaftInterface interface { - // FollowerState returns the current state of a follower. The value of - // Match, Next, Admitted are populated iff in StateReplicate. All entries >= + // FollowerStateRaftMuLocked returns the current state of a follower. The + // value of Match, Next are populated iff in StateReplicate. All entries >= // Next have not had MsgApps constructed during the lifetime of this // StateReplicate (they may have been constructed previously). // @@ -87,18 +87,8 @@ type RaftInterface interface { // StateReplicate, we start trying to send MsgApps. We should // notice such transitions both in HandleRaftEvent and SetReplicasLocked. // - // RACv1 also cared about three other cases where the follower behaved as if - // it were disconnected (a) paused follower, (b) follower is behind, (c) - // follower is inactive (see - // replicaFlowControlIntegrationImpl.notActivelyReplicatingTo). (b) and (c) - // were needed since it paced at rate of slowest replica, while for regular - // work we will in v2 pace at slowest in quorum (and we don't care about - // elastic experiencing a hiccup, given it paces at rate of slowest). For - // (a), we plan to remove follower pausing. So the v2 code will be - // simplified. - // // Requires Replica.raftMu to be held, Replica.mu is not held. - FollowerState(replicaID roachpb.ReplicaID) FollowerStateInfo + FollowerStateRaftMuLocked(replicaID roachpb.ReplicaID) FollowerStateInfo } type FollowerStateInfo struct { @@ -108,10 +98,17 @@ type FollowerStateInfo struct { // (Match, Next) is in-flight. Match uint64 Next uint64 - // TODO(kvoli): Find a better home for this, we need it for token return. - Term uint64 - // Invariant: Admitted[i] <= Match. - Admitted [raftpb.NumPriorities]uint64 +} + +// AdmittedTracker is used to retrieve the latest admitted vector for a +// replica (including the leader). +type AdmittedTracker interface { + // GetAdmitted returns the latest AdmittedVector for replicaID. It returns + // an empty struct if the replicaID is not known. NB: the + // AdmittedVector.Admitted[i] value can transiently advance past + // FollowerStateInfo.Match, since the admitted tracking subsystem is + // separate from Raft. + GetAdmitted(replicaID roachpb.ReplicaID) AdmittedVector } // RaftEvent carries a RACv2-relevant subset of raft state sent to storage. @@ -203,6 +200,7 @@ type RangeControllerOptions struct { RaftInterface RaftInterface Clock *hlc.Clock CloseTimerScheduler ProbeToCloseTimerScheduler + AdmittedTracker AdmittedTracker EvalWaitMetrics *EvalWaitMetrics } @@ -348,7 +346,7 @@ retry: func (rc *rangeController) HandleRaftEventRaftMuLocked(ctx context.Context, e RaftEvent) error { shouldWaitChange := false for r, rs := range rc.replicaMap { - info := rc.opts.RaftInterface.FollowerState(r) + info := rc.opts.RaftInterface.FollowerStateRaftMuLocked(r) shouldWaitChange = rs.handleReadyState(ctx, info) || shouldWaitChange } // If there was a quorum change, update the voter sets, triggering the @@ -504,7 +502,7 @@ func NewReplicaState( sendTokenCounter: parent.opts.SSTokenCounter.Send(stream), desc: desc, } - state := parent.opts.RaftInterface.FollowerState(desc.ReplicaID) + state := parent.opts.RaftInterface.FollowerStateRaftMuLocked(desc.ReplicaID) if state.State == tracker.StateReplicate { rs.createReplicaSendStream() } @@ -655,7 +653,7 @@ func (rs *replicaState) handleReadyState( rs.createReplicaSendStream() shouldWaitChange = true } else { - shouldWaitChange = rs.sendStream.makeConsistentInStateReplicate(ctx, info) + shouldWaitChange = rs.sendStream.makeConsistentInStateReplicate(ctx) } case tracker.StateSnapshot: @@ -692,11 +690,12 @@ func (rss *replicaState) closeSendStream(ctx context.Context) { } func (rss *replicaSendStream) makeConsistentInStateReplicate( - ctx context.Context, info FollowerStateInfo, + ctx context.Context, ) (shouldWaitChange bool) { + av := rss.parent.parent.opts.AdmittedTracker.GetAdmitted(rss.parent.desc.ReplicaID) rss.mu.Lock() defer rss.mu.Unlock() - defer rss.returnTokens(ctx, rss.mu.tracker.Untrack(info.Term, info.Admitted)) + defer rss.returnTokens(ctx, rss.mu.tracker.Untrack(av.Term, av.Admitted)) // The leader is always in state replicate. if rss.parent.parent.opts.LocalReplicaID == rss.parent.desc.ReplicaID { diff --git a/pkg/kv/kvserver/kvflowcontrol/rac2/range_controller_test.go b/pkg/kv/kvserver/kvflowcontrol/rac2/range_controller_test.go index 05bb62103457..2812192f7269 100644 --- a/pkg/kv/kvserver/kvflowcontrol/rac2/range_controller_test.go +++ b/pkg/kv/kvserver/kvflowcontrol/rac2/range_controller_test.go @@ -239,6 +239,7 @@ func (s *testingRCState) getOrInitRange(r testingRange) *testingRCRange { RaftInterface: testRC, Clock: s.clock, CloseTimerScheduler: s.probeToCloseScheduler, + AdmittedTracker: testRC, EvalWaitMetrics: s.evalMetrics, } @@ -272,7 +273,7 @@ type testingRCRange struct { } } -func (r *testingRCRange) FollowerState(replicaID roachpb.ReplicaID) FollowerStateInfo { +func (r *testingRCRange) FollowerStateRaftMuLocked(replicaID roachpb.ReplicaID) FollowerStateInfo { r.mu.Lock() defer r.mu.Unlock() @@ -283,6 +284,17 @@ func (r *testingRCRange) FollowerState(replicaID roachpb.ReplicaID) FollowerStat return replica.info } +func (r *testingRCRange) GetAdmitted(replicaID roachpb.ReplicaID) AdmittedVector { + r.mu.Lock() + defer r.mu.Unlock() + + replica, ok := r.mu.r.replicaSet[replicaID] + if !ok { + return AdmittedVector{} + } + return replica.av +} + func (r *testingRCRange) startWaitForEval(name string, pri admissionpb.WorkPriority) { r.mu.Lock() defer r.mu.Unlock() @@ -320,8 +332,8 @@ func (r *testingRCRange) admit( for _, replica := range r.mu.r.replicaSet { if replica.desc.StoreID == storeID { replica := replica - replica.info.Admitted[AdmissionToRaftPriority(pri)] = toIndex - replica.info.Term = term + replica.av.Admitted[AdmissionToRaftPriority(pri)] = toIndex + replica.av.Term = term r.mu.r.replicaSet[replica.desc.ReplicaID] = replica break } @@ -352,6 +364,7 @@ const invalidTrackerState = tracker.StateSnapshot + 1 type testingReplica struct { desc roachpb.ReplicaDescriptor info FollowerStateInfo + av AdmittedVector } func scanRanges(t *testing.T, input string) []testingRange { @@ -796,6 +809,12 @@ func TestRangeController(t *testing.T) { term, err = strconv.Atoi(parts[1]) require.NoError(t, err) + // TODO(sumeer): the test input only specifies an + // incremental change to the admitted vector, for a + // single priority. However, in practice, the whole + // vector will be updated, which also cleanly handles + // the case of an advancing term. Consider changing + // this to accept a non-incremental update. parts[2] = strings.TrimSpace(parts[2]) require.True(t, strings.HasPrefix(parts[2], "to_index=")) parts[2] = strings.TrimPrefix(strings.TrimSpace(parts[2]), "to_index=") diff --git a/pkg/kv/kvserver/kvflowcontrol/rac2/token_tracker.go b/pkg/kv/kvserver/kvflowcontrol/rac2/token_tracker.go index f66789fac666..dd7f855a036b 100644 --- a/pkg/kv/kvserver/kvflowcontrol/rac2/token_tracker.go +++ b/pkg/kv/kvserver/kvflowcontrol/rac2/token_tracker.go @@ -38,7 +38,7 @@ type tracked struct { func (dt *Tracker) Init(stream kvflowcontrol.Stream) { *dt = Tracker{ - tracked: [int(raftpb.NumPriorities)][]tracked{}, + tracked: [raftpb.NumPriorities][]tracked{}, stream: stream, } } diff --git a/pkg/kv/kvserver/kvflowcontrol/replica_rac2/processor.go b/pkg/kv/kvserver/kvflowcontrol/replica_rac2/processor.go index 97de3ebeb341..6ba05866e19d 100644 --- a/pkg/kv/kvserver/kvflowcontrol/replica_rac2/processor.go +++ b/pkg/kv/kvserver/kvflowcontrol/replica_rac2/processor.go @@ -472,6 +472,8 @@ type processorImpl struct { var _ Processor = &processorImpl{} +var _ rac2.AdmittedTracker = &processorImpl{} + func NewProcessor(opts ProcessorOptions) Processor { p := &processorImpl{opts: opts} p.mu.enabledWhenLeader = opts.EnabledWhenLeaderLevel @@ -970,6 +972,12 @@ func admittedIncreased(prev, next [raftpb.NumPriorities]uint64) bool { return false } +// GetAdmitted implements rac2.AdmittedTracker. +func (p *processorImpl) GetAdmitted(replicaID roachpb.ReplicaID) rac2.AdmittedVector { + // TODO(pav-kv): implement + return rac2.AdmittedVector{} +} + // RangeControllerFactoryImpl implements RangeControllerFactory. // // TODO(sumeer): replace with real implementation once RangeController impl is diff --git a/pkg/settings/registry.go b/pkg/settings/registry.go index 9e0d23dcac24..bc4c073e8f81 100644 --- a/pkg/settings/registry.go +++ b/pkg/settings/registry.go @@ -248,6 +248,7 @@ var retiredSettings = map[InternalKey]struct{}{ // removed as of 24.3 "bulkio.backup.split_keys_on_timestamps": {}, + "sql.create_tenant.default_template": {}, } // sqlDefaultSettings is the list of "grandfathered" existing sql.defaults diff --git a/pkg/sql/colflow/colrpc/BUILD.bazel b/pkg/sql/colflow/colrpc/BUILD.bazel index 65df3cba4a72..0df705aac5a6 100644 --- a/pkg/sql/colflow/colrpc/BUILD.bazel +++ b/pkg/sql/colflow/colrpc/BUILD.bazel @@ -61,6 +61,7 @@ go_test( "//pkg/sql/colmem", "//pkg/sql/execinfra", "//pkg/sql/execinfrapb", + "//pkg/sql/flowinfra", "//pkg/sql/types", "//pkg/testutils", "//pkg/util/cancelchecker", diff --git a/pkg/sql/colflow/colrpc/colrpc_test.go b/pkg/sql/colflow/colrpc/colrpc_test.go index 302bec0fee51..2546d0ec08a1 100644 --- a/pkg/sql/colflow/colrpc/colrpc_test.go +++ b/pkg/sql/colflow/colrpc/colrpc_test.go @@ -31,6 +31,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/colmem" "github.com/cockroachdb/cockroach/pkg/sql/execinfra" "github.com/cockroachdb/cockroach/pkg/sql/execinfrapb" + "github.com/cockroachdb/cockroach/pkg/sql/flowinfra" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/util/cancelchecker" @@ -135,7 +136,7 @@ func TestOutboxInbox(t *testing.T) { defer stopper.Stop(ctx) clock := hlc.NewClockForTesting(nil) - _, mockServer, addr, err := execinfrapb.StartMockDistSQLServer(ctx, clock, stopper, execinfra.StaticSQLInstanceID) + _, mockServer, addr, err := flowinfra.StartMockDistSQLServer(ctx, clock, stopper, execinfra.StaticSQLInstanceID) require.NoError(t, err) // Generate a random cancellation scenario. @@ -490,7 +491,7 @@ func TestInboxHostCtxCancellation(t *testing.T) { defer stopper.Stop(ctx) clock := hlc.NewClockForTesting(nil) - _, mockServer, addr, err := execinfrapb.StartMockDistSQLServer(ctx, clock, stopper, execinfra.StaticSQLInstanceID) + _, mockServer, addr, err := flowinfra.StartMockDistSQLServer(ctx, clock, stopper, execinfra.StaticSQLInstanceID) require.NoError(t, err) rng, _ := randutil.NewTestRand() @@ -578,7 +579,7 @@ func TestOutboxInboxMetadataPropagation(t *testing.T) { stopper := stop.NewStopper() defer stopper.Stop(ctx) - _, mockServer, addr, err := execinfrapb.StartMockDistSQLServer(ctx, + _, mockServer, addr, err := flowinfra.StartMockDistSQLServer(ctx, hlc.NewClockForTesting(nil), stopper, execinfra.StaticSQLInstanceID, ) require.NoError(t, err) @@ -773,7 +774,7 @@ func BenchmarkOutboxInbox(b *testing.B) { stopper := stop.NewStopper() defer stopper.Stop(ctx) - _, mockServer, addr, err := execinfrapb.StartMockDistSQLServer(ctx, + _, mockServer, addr, err := flowinfra.StartMockDistSQLServer(ctx, hlc.NewClockForTesting(nil), stopper, execinfra.StaticSQLInstanceID, ) require.NoError(b, err) @@ -848,11 +849,11 @@ func TestOutboxStreamIDPropagation(t *testing.T) { stopper := stop.NewStopper() defer stopper.Stop(ctx) - _, mockServer, addr, err := execinfrapb.StartMockDistSQLServer(ctx, + _, mockServer, addr, err := flowinfra.StartMockDistSQLServer(ctx, hlc.NewClockForTesting(nil), stopper, execinfra.StaticSQLInstanceID, ) require.NoError(t, err) - dialer := &execinfrapb.MockDialer{Addr: addr} + dialer := &flowinfra.MockDialer{Addr: addr} defer dialer.Close() typs := []*types.T{types.Int} diff --git a/pkg/sql/colflow/vectorized_flow_shutdown_test.go b/pkg/sql/colflow/vectorized_flow_shutdown_test.go index 286b99f45f07..bc38f55066f3 100644 --- a/pkg/sql/colflow/vectorized_flow_shutdown_test.go +++ b/pkg/sql/colflow/vectorized_flow_shutdown_test.go @@ -31,6 +31,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/colmem" "github.com/cockroachdb/cockroach/pkg/sql/execinfra" "github.com/cockroachdb/cockroach/pkg/sql/execinfrapb" + "github.com/cockroachdb/cockroach/pkg/sql/flowinfra" "github.com/cockroachdb/cockroach/pkg/sql/sem/eval" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/testutils" @@ -113,11 +114,11 @@ func TestVectorizedFlowShutdown(t *testing.T) { ctx := context.Background() stopper := stop.NewStopper() defer stopper.Stop(ctx) - _, mockServer, addr, err := execinfrapb.StartMockDistSQLServer(ctx, + _, mockServer, addr, err := flowinfra.StartMockDistSQLServer(ctx, hlc.NewClockForTesting(nil), stopper, execinfra.StaticSQLInstanceID, ) require.NoError(t, err) - dialer := &execinfrapb.MockDialer{Addr: addr} + dialer := &flowinfra.MockDialer{Addr: addr} defer dialer.Close() queueCfg, cleanup := colcontainerutils.NewTestingDiskQueueCfg(t, true /* inMem */) diff --git a/pkg/sql/create_tenant.go b/pkg/sql/create_tenant.go index eac03eb01e3e..63d8cd4d5ae4 100644 --- a/pkg/sql/create_tenant.go +++ b/pkg/sql/create_tenant.go @@ -13,15 +13,12 @@ package sql import ( "context" - "github.com/cockroachdb/cockroach/pkg/multitenant/mtinfopb" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" - "github.com/cockroachdb/errors" ) type createTenantNode struct { - ifNotExists bool - tenantSpec tenantSpec - likeTenantSpec tenantSpec + ifNotExists bool + tenantSpec tenantSpec } func (p *planner) CreateTenantNode(ctx context.Context, n *tree.CreateTenant) (planNode, error) { @@ -29,17 +26,9 @@ func (p *planner) CreateTenantNode(ctx context.Context, n *tree.CreateTenant) (p if err != nil { return nil, err } - var likeTenantSpec tenantSpec - if n.Like.OtherTenant != nil { - likeTenantSpec, err = p.planTenantSpec(ctx, n.Like.OtherTenant, "CREATE VIRTUAL CLUSTER LIKE") - if err != nil { - return nil, err - } - } return &createTenantNode{ - ifNotExists: n.IfNotExists, - tenantSpec: tspec, - likeTenantSpec: likeTenantSpec, + ifNotExists: n.IfNotExists, + tenantSpec: tspec, }, nil } @@ -49,20 +38,6 @@ func (n *createTenantNode) startExec(params runParams) error { return err } - var tmplInfo *mtinfopb.TenantInfo - if n.likeTenantSpec != nil { - tmplInfo, err = n.likeTenantSpec.getTenantInfo(params.ctx, params.p) - if err != nil { - return errors.Wrap(err, "retrieving record for LIKE configuration template") - } - } - configTemplate, err := GetTenantTemplate(params.ctx, - params.p.ExecCfg().Settings, params.p.InternalSQLTxn(), - tmplInfo, 0, "") - if err != nil { - return err - } - var ctcfg createTenantConfig if tenantName != "" { ctcfg.Name = (*string)(&tenantName) @@ -72,7 +47,7 @@ func (n *createTenantNode) startExec(params runParams) error { ctcfg.ID = &tenantID } ctcfg.IfNotExists = n.ifNotExists - _, err = params.p.createTenantInternal(params.ctx, ctcfg, configTemplate) + _, err = params.p.createTenantInternal(params.ctx, ctcfg) return err } diff --git a/pkg/sql/execinfrapb/BUILD.bazel b/pkg/sql/execinfrapb/BUILD.bazel index dc0a8257584f..8c5d9fc1f983 100644 --- a/pkg/sql/execinfrapb/BUILD.bazel +++ b/pkg/sql/execinfrapb/BUILD.bazel @@ -13,7 +13,6 @@ go_library( "expr.go", "flow_diagram.go", "processors.go", - "testutils.go", ], embed = [":execinfrapb_go_proto"], importpath = "github.com/cockroachdb/cockroach/pkg/sql/execinfrapb", @@ -21,9 +20,7 @@ go_library( deps = [ "//pkg/base", "//pkg/roachpb", - "//pkg/rpc", "//pkg/security/username", - "//pkg/settings/cluster", "//pkg/sql/catalog", "//pkg/sql/catalog/catalogkeys", "//pkg/sql/catalog/catenumpb", @@ -45,13 +42,9 @@ go_library( "//pkg/util/buildutil", "//pkg/util/duration", "//pkg/util/encoding", - "//pkg/util/hlc", "//pkg/util/humanizeutil", - "//pkg/util/netutil", "//pkg/util/optional", "//pkg/util/protoutil", - "//pkg/util/stop", - "//pkg/util/syncutil", "//pkg/util/tracing/tracingpb", "//pkg/util/uuid", "@com_github_cockroachdb_errors//:errors", @@ -59,7 +52,6 @@ go_library( "@com_github_cockroachdb_redact//:redact", "@com_github_dustin_go_humanize//:go-humanize", "@com_github_gogo_protobuf//types", - "@org_golang_google_grpc//:go_default_library", ], ) diff --git a/pkg/sql/flowinfra/BUILD.bazel b/pkg/sql/flowinfra/BUILD.bazel index e140daded014..4b35f4e6dfd7 100644 --- a/pkg/sql/flowinfra/BUILD.bazel +++ b/pkg/sql/flowinfra/BUILD.bazel @@ -12,6 +12,7 @@ go_library( "stream_decoder.go", "stream_encoder.go", "testing_knobs.go", + "testutils.go", ], importpath = "github.com/cockroachdb/cockroach/pkg/sql/flowinfra", visibility = ["//visibility:public"], @@ -19,7 +20,9 @@ go_library( "//pkg/base", "//pkg/kv", "//pkg/roachpb", + "//pkg/rpc", "//pkg/settings", + "//pkg/settings/cluster", "//pkg/sql/catalog/catenumpb", "//pkg/sql/catalog/colinfo", "//pkg/sql/execinfra", @@ -32,23 +35,29 @@ go_library( "//pkg/sql/rowenc", "//pkg/sql/sem/tree", "//pkg/sql/types", + "//pkg/util", "//pkg/util/admission", "//pkg/util/admission/admissionpb", "//pkg/util/buildutil", "//pkg/util/cancelchecker", "//pkg/util/ctxlog", + "//pkg/util/hlc", "//pkg/util/log", "//pkg/util/mon", + "//pkg/util/netutil", "//pkg/util/optional", "//pkg/util/stop", "//pkg/util/syncutil", "//pkg/util/timeutil", "//pkg/util/tracing", "//pkg/util/tracing/tracingpb", + "//pkg/util/uuid", "@com_github_cockroachdb_errors//:errors", + "@com_github_cockroachdb_logtags//:logtags", "@com_github_cockroachdb_redact//:redact", "@com_github_gogo_protobuf//proto", "@io_opentelemetry_go_otel//attribute", + "@org_golang_google_grpc//:go_default_library", ], ) diff --git a/pkg/sql/flowinfra/outbox_test.go b/pkg/sql/flowinfra/outbox_test.go index 7fc72e965111..7e55a7ee5d96 100644 --- a/pkg/sql/flowinfra/outbox_test.go +++ b/pkg/sql/flowinfra/outbox_test.go @@ -59,7 +59,7 @@ func TestOutbox(t *testing.T) { stopper := stop.NewStopper() defer stopper.Stop(ctx) clock := hlc.NewClockForTesting(nil) - clusterID, mockServer, addr, err := execinfrapb.StartMockDistSQLServer(ctx, clock, stopper, execinfra.StaticSQLInstanceID) + clusterID, mockServer, addr, err := flowinfra.StartMockDistSQLServer(ctx, clock, stopper, execinfra.StaticSQLInstanceID) if err != nil { t.Fatal(err) } @@ -224,7 +224,7 @@ func TestOutboxInitializesStreamBeforeReceivingAnyRows(t *testing.T) { stopper := stop.NewStopper() defer stopper.Stop(ctx) clock := hlc.NewClockForTesting(nil) - clusterID, mockServer, addr, err := execinfrapb.StartMockDistSQLServer(ctx, clock, stopper, execinfra.StaticSQLInstanceID) + clusterID, mockServer, addr, err := flowinfra.StartMockDistSQLServer(ctx, clock, stopper, execinfra.StaticSQLInstanceID) if err != nil { t.Fatal(err) } @@ -297,7 +297,7 @@ func TestOutboxClosesWhenConsumerCloses(t *testing.T) { stopper := stop.NewStopper() defer stopper.Stop(ctx) clock := hlc.NewClockForTesting(nil) - clusterID, mockServer, addr, err := execinfrapb.StartMockDistSQLServer(ctx, clock, stopper, execinfra.StaticSQLInstanceID) + clusterID, mockServer, addr, err := flowinfra.StartMockDistSQLServer(ctx, clock, stopper, execinfra.StaticSQLInstanceID) if err != nil { t.Fatal(err) } @@ -375,7 +375,7 @@ func TestOutboxCancelsFlowOnError(t *testing.T) { stopper := stop.NewStopper() defer stopper.Stop(ctx) clock := hlc.NewClockForTesting(nil) - clusterID, mockServer, addr, err := execinfrapb.StartMockDistSQLServer(ctx, clock, stopper, execinfra.StaticSQLInstanceID) + clusterID, mockServer, addr, err := flowinfra.StartMockDistSQLServer(ctx, clock, stopper, execinfra.StaticSQLInstanceID) if err != nil { t.Fatal(err) } @@ -501,7 +501,7 @@ func BenchmarkOutbox(b *testing.B) { stopper := stop.NewStopper() defer stopper.Stop(bgCtx) clock := hlc.NewClockForTesting(nil) - clusterID, mockServer, addr, err := execinfrapb.StartMockDistSQLServer(bgCtx, clock, stopper, execinfra.StaticSQLInstanceID) + clusterID, mockServer, addr, err := flowinfra.StartMockDistSQLServer(bgCtx, clock, stopper, execinfra.StaticSQLInstanceID) if err != nil { b.Fatal(err) } diff --git a/pkg/sql/execinfrapb/testutils.go b/pkg/sql/flowinfra/testutils.go similarity index 87% rename from pkg/sql/execinfrapb/testutils.go rename to pkg/sql/flowinfra/testutils.go index 402a4863a709..27c00bf7ff5f 100644 --- a/pkg/sql/execinfrapb/testutils.go +++ b/pkg/sql/flowinfra/testutils.go @@ -8,7 +8,7 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -package execinfrapb +package flowinfra import ( "context" @@ -18,6 +18,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/rpc" "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/sql/execinfrapb" "github.com/cockroachdb/cockroach/pkg/util" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/netutil" @@ -52,7 +53,7 @@ func StartMockDistSQLServer( return uuid.Nil, nil, nil, err } mock := newMockDistSQLServer() - RegisterDistSQLServer(server, mock) + execinfrapb.RegisterDistSQLServer(server, mock) ln, err := netutil.ListenAndServeGRPC(stopper, server, util.IsolatedTestAddr) if err != nil { return uuid.Nil, nil, nil, err @@ -70,12 +71,12 @@ type MockDistSQLServer struct { // that a new gRPC call has arrived and thus a stream has arrived. The rpc // handler is blocked until Donec is signaled. type InboundStreamNotification struct { - Stream DistSQL_FlowStreamServer + Stream execinfrapb.DistSQL_FlowStreamServer Donec chan<- error } // MockDistSQLServer implements the DistSQLServer interface. -var _ DistSQLServer = &MockDistSQLServer{} +var _ execinfrapb.DistSQLServer = &MockDistSQLServer{} func newMockDistSQLServer() *MockDistSQLServer { return &MockDistSQLServer{ @@ -85,20 +86,20 @@ func newMockDistSQLServer() *MockDistSQLServer { // SetupFlow is part of the DistSQLServer interface. func (ds *MockDistSQLServer) SetupFlow( - _ context.Context, req *SetupFlowRequest, -) (*SimpleResponse, error) { + _ context.Context, req *execinfrapb.SetupFlowRequest, +) (*execinfrapb.SimpleResponse, error) { return nil, nil } // CancelDeadFlows is part of the DistSQLServer interface. func (ds *MockDistSQLServer) CancelDeadFlows( - _ context.Context, req *CancelDeadFlowsRequest, -) (*SimpleResponse, error) { + _ context.Context, req *execinfrapb.CancelDeadFlowsRequest, +) (*execinfrapb.SimpleResponse, error) { return nil, nil } // FlowStream is part of the DistSQLServer interface. -func (ds *MockDistSQLServer) FlowStream(stream DistSQL_FlowStreamServer) error { +func (ds *MockDistSQLServer) FlowStream(stream execinfrapb.DistSQL_FlowStreamServer) error { donec := make(chan error) ds.InboundStreams <- InboundStreamNotification{Stream: stream, Donec: donec} return <-donec @@ -107,7 +108,7 @@ func (ds *MockDistSQLServer) FlowStream(stream DistSQL_FlowStreamServer) error { // MockDialer is a mocked implementation of the Outbox's `Dialer` interface. // Used to create a connection with a client stream. type MockDialer struct { - // Addr is assumed to be obtained from execinfrapb.StartMockDistSQLServer. + // Addr is assumed to be obtained from flowinfra.StartMockDistSQLServer. Addr net.Addr mu struct { syncutil.Mutex diff --git a/pkg/sql/flowinfra/utils_test.go b/pkg/sql/flowinfra/utils_test.go index 59579b294198..35455b1a6b71 100644 --- a/pkg/sql/flowinfra/utils_test.go +++ b/pkg/sql/flowinfra/utils_test.go @@ -43,7 +43,7 @@ func createDummyStream( stopper := stop.NewStopper() ctx := context.Background() clock := hlc.NewClockForTesting(nil) - storageClusterID, mockServer, addr, err := execinfrapb.StartMockDistSQLServer(ctx, clock, stopper, execinfra.StaticSQLInstanceID) + storageClusterID, mockServer, addr, err := StartMockDistSQLServer(ctx, clock, stopper, execinfra.StaticSQLInstanceID) if err != nil { t.Fatal(err) } diff --git a/pkg/sql/logictest/testdata/logic_test/tenant b/pkg/sql/logictest/testdata/logic_test/tenant index 6f00d23a1e7a..d60069293efd 100644 --- a/pkg/sql/logictest/testdata/logic_test/tenant +++ b/pkg/sql/logictest/testdata/logic_test/tenant @@ -378,137 +378,6 @@ DROP TENANT two statement ok DROP TENANT 'tenant-one' -subtest tenant_templates - -query T -SHOW CLUSTER SETTING sql.create_virtual_cluster.default_template ----- -ยท - -# Check we can't use the system tenant as template. -statement error using the system tenant as config template -CREATE TENANT othertenant LIKE system - -# Create some "interesting" tenant template. -statement ok -CREATE TENANT tmpl; - -let $tmplid -SELECT id FROM system.tenants WHERE name = 'tmpl' - -statement ok -ALTER TENANT tmpl GRANT CAPABILITY can_view_node_info; -- will be copied -ALTER TENANT tmpl SET CLUSTER SETTING trace.debug_http_endpoint.enabled = true; -- will be copied --- Simulate resource limits. Will be copied. --- Note: we cannot use the update_tenant_resource_limits() builtin --- directly here because it can only be used from a CCL binary. -INSERT INTO system.tenant_usage( - tenant_id, instance_id, next_instance_id, last_update, - ru_burst_limit, ru_refill_rate, ru_current, current_share_sum, total_consumption) -VALUES ($tmplid, 0, 0, now(), - 11, 22, 33, 44, ''::BYTES); - - -statement ok -ALTER TENANT tmpl START SERVICE SHARED; -- will not be copied. - -# Use it to create a new tenant. -statement ok -CREATE TENANT othertenant LIKE tmpl - -let $otherid -SELECT id FROM system.tenants WHERE name = 'othertenant' - -# Verify the service mode was not copied. -query BTTT -SELECT id = $otherid, name, data_state, service_mode FROM [SHOW TENANT othertenant] ----- -true othertenant ready none - -# Verify the new tenant has the same caps as the template -# (by showing there's no difference between the two) -query TT -SELECT capability_name, capability_value FROM [SHOW TENANT tmpl WITH CAPABILITIES] -EXCEPT SELECT capability_name, capability_value FROM [SHOW TENANT othertenant WITH CAPABILITIES]; ----- - -# Check that the setting overrides were copied. -query TTTT rowsort -SELECT variable, value, type, origin FROM [SHOW CLUSTER SETTINGS FOR TENANT othertenant] -WHERE origin != 'no-override' ----- -trace.debug_http_endpoint.enabled true b per-tenant-override - -# Check that the resource usage parameters were copied. -query IIRRRRI -SELECT instance_id, next_instance_id, - ru_burst_limit, ru_refill_rate, ru_current, - current_share_sum, length(total_consumption) -FROM system.tenant_usage WHERE tenant_id = $otherid ----- -0 0 11 22 33 0 0 - -# Clean up. -statement ok -DROP TENANT othertenant - -# Now set the default template and try again. -statement ok -SET CLUSTER SETTING sql.create_virtual_cluster.default_template = 'nonexistent'; - -statement error retrieving default tenant configuration template.*tenant "nonexistent" does not exist -CREATE TENANT othertenant - -statement ok -SET CLUSTER SETTING sql.create_virtual_cluster.default_template = 'tmpl'; - -# Create a new tenant - this should use the template implicitly now. -statement ok -CREATE TENANT othertenant - -let $otherid -SELECT id FROM system.tenants WHERE name = 'othertenant' - -# Verify the service mode was not copied. -query BTTT -SELECT id = $otherid, name, data_state, service_mode FROM [SHOW TENANT othertenant] ----- -true othertenant ready none - -query TT -SELECT capability_name, capability_value FROM [SHOW TENANT tmpl WITH CAPABILITIES] -EXCEPT SELECT capability_name, capability_value FROM [SHOW TENANT othertenant WITH CAPABILITIES]; ----- - -# Check the setting overrides were taken over. -query TTTT rowsort -SELECT variable, value, type, origin FROM [SHOW CLUSTER SETTINGS FOR TENANT othertenant] -WHERE origin != 'no-override' ----- -trace.debug_http_endpoint.enabled true b per-tenant-override - -# Check that the resource usage parameters were copied. -query IIRRRRI -SELECT instance_id, next_instance_id, - ru_burst_limit, ru_refill_rate, ru_current, - current_share_sum, length(total_consumption) -FROM system.tenant_usage WHERE tenant_id = $otherid ----- -0 0 11 22 33 0 0 - -# Clean up. -statement ok -DROP TENANT othertenant - -statement ok -ALTER TENANT tmpl STOP SERVICE - -statement ok -DROP TENANT tmpl - -statement ok -RESET CLUSTER SETTING sql.create_virtual_cluster.default_template - subtest regression_105115 statement ok diff --git a/pkg/sql/parser/sql.y b/pkg/sql/parser/sql.y index 5e634107e8e0..98776f7e6938 100644 --- a/pkg/sql/parser/sql.y +++ b/pkg/sql/parser/sql.y @@ -872,9 +872,6 @@ func (u *sqlSymUnion) showRangesOpts() *tree.ShowRangesOptions { func (u *sqlSymUnion) tenantSpec() *tree.TenantSpec { return u.val.(*tree.TenantSpec) } -func (u *sqlSymUnion) likeTenantSpec() *tree.LikeTenantSpec { - return u.val.(*tree.LikeTenantSpec) -} func (u *sqlSymUnion) cteMaterializeClause() tree.CTEMaterializeClause { return u.val.(tree.CTEMaterializeClause) } @@ -1238,7 +1235,6 @@ func (u *sqlSymUnion) triggerForEach() tree.TriggerForEach { %type create_proc_stmt %type create_trigger_stmt -%type <*tree.LikeTenantSpec> opt_like_virtual_cluster %type logical_replication_resources, logical_replication_resources_list %type <*tree.LogicalReplicationOptions> opt_logical_replication_options logical_replication_options logical_replication_options_list @@ -4744,49 +4740,45 @@ logical_replication_options: // %Help: CREATE VIRTUAL CLUSTER - create a new virtual cluster // %Category: Experimental // %Text: -// CREATE VIRTUAL CLUSTER [ IF NOT EXISTS ] name [ LIKE ] [ ] +// CREATE VIRTUAL CLUSTER [ IF NOT EXISTS ] name [ ] // // Replication option: // FROM REPLICATION OF ON [ WITH OPTIONS ... ] create_virtual_cluster_stmt: - CREATE virtual_cluster d_expr opt_like_virtual_cluster + CREATE virtual_cluster d_expr { /* SKIP DOC */ $$.val = &tree.CreateTenant{ TenantSpec: &tree.TenantSpec{IsName: true, Expr: $3.expr()}, - Like: $4.likeTenantSpec(), } } -| CREATE virtual_cluster IF NOT EXISTS d_expr opt_like_virtual_cluster +| CREATE virtual_cluster IF NOT EXISTS d_expr { /* SKIP DOC */ $$.val = &tree.CreateTenant{ IfNotExists: true, TenantSpec: &tree.TenantSpec{IsName: true, Expr: $6.expr()}, - Like: $7.likeTenantSpec(), } } -| CREATE virtual_cluster d_expr opt_like_virtual_cluster FROM REPLICATION OF d_expr ON d_expr opt_with_replication_options +| CREATE virtual_cluster d_expr FROM REPLICATION OF d_expr ON d_expr opt_with_replication_options { /* SKIP DOC */ $$.val = &tree.CreateTenantFromReplication{ TenantSpec: &tree.TenantSpec{IsName: true, Expr: $3.expr()}, - ReplicationSourceTenantName: &tree.TenantSpec{IsName: true, Expr: $8.expr()}, - ReplicationSourceAddress: $10.expr(), - Options: *$11.tenantReplicationOptions(), - Like: $4.likeTenantSpec(), + ReplicationSourceTenantName: &tree.TenantSpec{IsName: true, Expr: $7.expr()}, + ReplicationSourceAddress: $9.expr(), + Options: *$10.tenantReplicationOptions(), } } -| CREATE virtual_cluster IF NOT EXISTS d_expr opt_like_virtual_cluster FROM REPLICATION OF d_expr ON d_expr opt_with_replication_options +| CREATE virtual_cluster IF NOT EXISTS d_expr FROM REPLICATION OF d_expr ON d_expr opt_with_replication_options { /* SKIP DOC */ $$.val = &tree.CreateTenantFromReplication{ IfNotExists: true, TenantSpec: &tree.TenantSpec{IsName: true, Expr: $6.expr()}, - ReplicationSourceTenantName: &tree.TenantSpec{IsName: true, Expr: $11.expr()}, - ReplicationSourceAddress: $13.expr(), - Options: *$14.tenantReplicationOptions(), - Like: $7.likeTenantSpec(), + ReplicationSourceTenantName: &tree.TenantSpec{IsName: true, Expr: $10.expr()}, + ReplicationSourceAddress: $12.expr(), + Options: *$13.tenantReplicationOptions(), } } | CREATE virtual_cluster error // SHOW HELP: CREATE VIRTUAL CLUSTER @@ -4795,19 +4787,6 @@ virtual_cluster: TENANT { /* SKIP DOC */ } | VIRTUAL CLUSTER -// opt_like_virtual_cluster defines a LIKE clause for CREATE VIRTUAL CLUSTER. -// Eventually this can grow to support INCLUDING/EXCLUDING options -// like in CREATE TABLE. -opt_like_virtual_cluster: - /* EMPTY */ - { - $$.val = &tree.LikeTenantSpec{} - } -| LIKE virtual_cluster_spec - { - $$.val = &tree.LikeTenantSpec{OtherTenant: $2.tenantSpec()} - } - // Optional tenant replication options. opt_with_replication_options: WITH replication_options_list diff --git a/pkg/sql/parser/testdata/create_virtual_cluster b/pkg/sql/parser/testdata/create_virtual_cluster index 0d149f113b26..bcff8f337060 100644 --- a/pkg/sql/parser/testdata/create_virtual_cluster +++ b/pkg/sql/parser/testdata/create_virtual_cluster @@ -30,22 +30,6 @@ CREATE VIRTUAL CLUSTER IF NOT EXISTS (bar) -- fully parenthesized CREATE VIRTUAL CLUSTER IF NOT EXISTS bar -- literals removed CREATE VIRTUAL CLUSTER IF NOT EXISTS _ -- identifiers removed -parse -CREATE VIRTUAL CLUSTER foo LIKE bar ----- -CREATE VIRTUAL CLUSTER foo LIKE bar -CREATE VIRTUAL CLUSTER (foo) LIKE (bar) -- fully parenthesized -CREATE VIRTUAL CLUSTER foo LIKE bar -- literals removed -CREATE VIRTUAL CLUSTER _ LIKE _ -- identifiers removed - -parse -CREATE VIRTUAL CLUSTER foo LIKE [123] ----- -CREATE VIRTUAL CLUSTER foo LIKE [123] -CREATE VIRTUAL CLUSTER (foo) LIKE [(123)] -- fully parenthesized -CREATE VIRTUAL CLUSTER foo LIKE [_] -- literals removed -CREATE VIRTUAL CLUSTER _ LIKE [123] -- identifiers removed - parse CREATE VIRTUAL CLUSTER destination FROM REPLICATION OF source ON 'pgurl' ---- @@ -62,22 +46,6 @@ CREATE VIRTUAL CLUSTER IF NOT EXISTS (destination) FROM REPLICATION OF (source) CREATE VIRTUAL CLUSTER IF NOT EXISTS destination FROM REPLICATION OF source ON '_' -- literals removed CREATE VIRTUAL CLUSTER IF NOT EXISTS _ FROM REPLICATION OF _ ON 'pgurl' -- identifiers removed -parse -CREATE VIRTUAL CLUSTER destination LIKE bar FROM REPLICATION OF source ON 'pgurl' ----- -CREATE VIRTUAL CLUSTER destination LIKE bar FROM REPLICATION OF source ON 'pgurl' -CREATE VIRTUAL CLUSTER (destination) LIKE (bar) FROM REPLICATION OF (source) ON ('pgurl') -- fully parenthesized -CREATE VIRTUAL CLUSTER destination LIKE bar FROM REPLICATION OF source ON '_' -- literals removed -CREATE VIRTUAL CLUSTER _ LIKE _ FROM REPLICATION OF _ ON 'pgurl' -- identifiers removed - -parse -CREATE VIRTUAL CLUSTER destination LIKE [123] FROM REPLICATION OF source ON 'pgurl' ----- -CREATE VIRTUAL CLUSTER destination LIKE [123] FROM REPLICATION OF source ON 'pgurl' -CREATE VIRTUAL CLUSTER (destination) LIKE [(123)] FROM REPLICATION OF (source) ON ('pgurl') -- fully parenthesized -CREATE VIRTUAL CLUSTER destination LIKE [_] FROM REPLICATION OF source ON '_' -- literals removed -CREATE VIRTUAL CLUSTER _ LIKE [123] FROM REPLICATION OF _ ON 'pgurl' -- identifiers removed - parse CREATE VIRTUAL CLUSTER "destination-hyphen" FROM REPLICATION OF "source-hyphen" ON 'pgurl' ---- diff --git a/pkg/sql/sem/tree/create.go b/pkg/sql/sem/tree/create.go index 00862a601173..db8cb7e9e503 100644 --- a/pkg/sql/sem/tree/create.go +++ b/pkg/sql/sem/tree/create.go @@ -2184,7 +2184,6 @@ func (node *CreateExternalConnection) Format(ctx *FmtCtx) { type CreateTenant struct { IfNotExists bool TenantSpec *TenantSpec - Like *LikeTenantSpec } // Format implements the NodeFormatter interface. @@ -2194,20 +2193,6 @@ func (node *CreateTenant) Format(ctx *FmtCtx) { ctx.WriteString("IF NOT EXISTS ") } ctx.FormatNode(node.TenantSpec) - ctx.FormatNode(node.Like) -} - -// LikeTenantSpec represents a LIKE clause in CREATE VIRTUAL CLUSTER. -type LikeTenantSpec struct { - OtherTenant *TenantSpec -} - -func (node *LikeTenantSpec) Format(ctx *FmtCtx) { - if node.OtherTenant == nil { - return - } - ctx.WriteString(" LIKE ") - ctx.FormatNode(node.OtherTenant) } // CreateTenantFromReplication represents a CREATE VIRTUAL CLUSTER...FROM REPLICATION @@ -2228,8 +2213,6 @@ type CreateTenantFromReplication struct { ReplicationSourceAddress Expr Options TenantReplicationOptions - - Like *LikeTenantSpec } // TenantReplicationOptions options for the CREATE/ALTER VIRTUAL CLUSTER FROM REPLICATION command. @@ -2249,9 +2232,6 @@ func (node *CreateTenantFromReplication) Format(ctx *FmtCtx) { // NB: we do not anonymize the tenant name because we assume that tenant names // do not contain sensitive information. ctx.FormatNode(node.TenantSpec) - if node.Like != nil { - ctx.FormatNode(node.Like) - } if node.ReplicationSourceAddress != nil { ctx.WriteString(" FROM REPLICATION OF ") diff --git a/pkg/sql/sem/tree/walk.go b/pkg/sql/sem/tree/walk.go index c7eed182d673..26038a58e519 100644 --- a/pkg/sql/sem/tree/walk.go +++ b/pkg/sql/sem/tree/walk.go @@ -1074,31 +1074,9 @@ func (n *AlterTenantReplication) walkStmt(v Visitor) Statement { return ret } -// copyNode makes a copy of this node without recursing. -func (n *LikeTenantSpec) copyNode() *LikeTenantSpec { - nodeCopy := *n - return &nodeCopy -} - -// copyNode makes a copy of this Statement without recursing in any child Statements. -func (n *CreateTenant) copyNode() *CreateTenant { - stmtCopy := *n - return &stmtCopy -} - // walkStmt is part of the walkableStmt interface. func (n *CreateTenant) walkStmt(v Visitor) Statement { ret := n - if n.Like.OtherTenant != nil { - ts, changed := walkTenantSpec(v, n.TenantSpec) - if changed { - if ret == n { - ret = n.copyNode() - } - ret.Like = n.Like.copyNode() - ret.Like.OtherTenant = ts - } - } return ret } @@ -1143,16 +1121,6 @@ func (n *CreateTenantFromReplication) walkStmt(v Visitor) Statement { ret.Options.ExpirationWindow = e } } - if n.Like.OtherTenant != nil { - ts, changed := walkTenantSpec(v, n.TenantSpec) - if changed { - if ret == n { - ret = n.copyNode() - } - ret.Like = n.Like.copyNode() - ret.Like.OtherTenant = ts - } - } return ret } diff --git a/pkg/sql/tenant_accessors.go b/pkg/sql/tenant_accessors.go index e0eacca9631f..447c46714451 100644 --- a/pkg/sql/tenant_accessors.go +++ b/pkg/sql/tenant_accessors.go @@ -14,11 +14,9 @@ import ( "context" "github.com/cockroachdb/cockroach/pkg/keys" - "github.com/cockroachdb/cockroach/pkg/kv/kvpb" "github.com/cockroachdb/cockroach/pkg/multitenant/mtinfo" "github.com/cockroachdb/cockroach/pkg/multitenant/mtinfopb" "github.com/cockroachdb/cockroach/pkg/roachpb" - "github.com/cockroachdb/cockroach/pkg/settings" "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql/isql" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" @@ -188,99 +186,3 @@ func GetExtendedTenantInfo( return res, nil } - -var defaultTenantConfigTemplate = settings.RegisterStringSetting( - settings.ApplicationLevel, - "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 - // (i.e. empty proto, no setting overrides). - "", - settings.WithName("sql.create_virtual_cluster.default_template"), - settings.WithReportable(true), -) - -// GetTenantTemplate loads the tenant template corresponding to the -// provided origin tenant. If info is nil, likeTenantID is zero and -// likeTenantName is empty, the default template is returned. -func GetTenantTemplate( - ctx context.Context, - settings *cluster.Settings, - txn isql.Txn, - info *mtinfopb.TenantInfo, - likeTenantID uint64, - likeTenantName string, -) (res *mtinfopb.TenantInfoWithUsage, err error) { - if info != nil && (likeTenantID != 0 || likeTenantName != "") { - // Sanity check - return nil, errors.AssertionFailedf("programming error: cannot pass both default info struct and tenant reference") - } - if info == nil { - if likeTenantID == 0 && likeTenantName == "" { - // No LIKE at all. Do we have something in the cluster setting? - tmplName := defaultTenantConfigTemplate.Get(&settings.SV) - if tmplName == "" { - // No template at all - just use an empty protobuf. - return &mtinfopb.TenantInfoWithUsage{}, nil - } - // Use the template specified in the setting. - info, err = GetTenantRecordByName(ctx, settings, txn, roachpb.TenantName(tmplName)) - if err != nil { - return nil, errors.Wrapf(err, "retrieving default tenant configuration template %q", tmplName) - } - } else { - if likeTenantID != 0 && likeTenantName != "" { - return nil, errors.AssertionFailedf("programming error: conflicting input tenant spec from caller") - } - // No pre-loaded info, but we have a LIKE clause. Is it by-ID or by-Name? - if likeTenantID != 0 { - // By-ID. - tid, err := roachpb.MakeTenantID(likeTenantID) - if err != nil { - return nil, errors.Wrap(err, "invalid LIKE tenant ID") - } - info, err = GetTenantRecordByID(ctx, txn, tid, settings) - if err != nil { - return nil, errors.Wrap(err, "retrieving LIKE tenant record") - } - } else { - // By-name. - info, err = GetTenantRecordByName(ctx, settings, txn, roachpb.TenantName(likeTenantName)) - if err != nil { - return nil, errors.Wrap(err, "retrieving LIKE tenant record") - } - } - } - } - - // For now, prevent use of the record for the system tenant. The - // user may have the mistaken assumption that "LIKE system" would - // create a tenant with all the special cases of the system tenant, - // and we do not guarantee that for now. - if roachpb.MustMakeTenantID(info.ID).IsSystem() { - return nil, errors.WithHint( - pgerror.New(pgcode.WrongObjectType, "using the system tenant as config template"), - "Create another secondary tenant as template, grant it extra capabilities, and then use that as config template.") - } - - // Now we have our info field. Expand it. - tmplInfo, err := GetExtendedTenantInfo(ctx, txn, info) - if err != nil { - return nil, errors.Wrap(err, "retrieving tenant template details") - } - - // Clear out the fields we can't reuse in a fresh tenant record. - tmplInfo.ID = 0 - tmplInfo.Name = "" - tmplInfo.DataState = mtinfopb.DataStateReady - tmplInfo.ServiceMode = mtinfopb.ServiceModeNone - tmplInfo.DroppedName = "" - tmplInfo.DeprecatedID = 0 - tmplInfo.DeprecatedDataState = 0 - tmplInfo.PhysicalReplicationConsumerJobID = 0 - if tmplInfo.Usage != nil { - tmplInfo.Usage.Consumption = kvpb.TenantConsumption{} - } - - return tmplInfo, nil -} diff --git a/pkg/sql/tenant_creation.go b/pkg/sql/tenant_creation.go index 9bfa6303bf90..92eb39affb15 100644 --- a/pkg/sql/tenant_creation.go +++ b/pkg/sql/tenant_creation.go @@ -67,9 +67,7 @@ func (p *planner) CreateTenant( return tid, pgerror.Newf(pgcode.ProgramLimitExceeded, "tenant ID %d out of range", *ctcfg.ID) } - configTemplate := mtinfopb.TenantInfoWithUsage{} - - return p.createTenantInternal(ctx, ctcfg, &configTemplate) + return p.createTenantInternal(ctx, ctcfg) } type createTenantConfig struct { @@ -80,7 +78,7 @@ type createTenantConfig struct { } func (p *planner) createTenantInternal( - ctx context.Context, ctcfg createTenantConfig, configTemplate *mtinfopb.TenantInfoWithUsage, + ctx context.Context, ctcfg createTenantConfig, ) (tid roachpb.TenantID, err error) { if p.EvalContext().TxnReadOnly { return tid, readOnlyError("create_tenant()") @@ -109,11 +107,7 @@ func (p *planner) createTenantInternal( serviceMode = v } - info := configTemplate - - // Override the template fields for a fresh tenant. The other - // template fields remain unchanged (i.e. we reuse the template's - // configuration). + var info mtinfopb.TenantInfoWithUsage info.ID = tenantID info.Name = name // We synchronously initialize the tenant's keyspace below, so @@ -134,7 +128,7 @@ func (p *planner) createTenantInternal( p.ExecCfg().Settings, p.InternalSQLTxn(), p.ExecCfg().SpanConfigKVAccessor.WithISQLTxn(ctx, p.InternalSQLTxn()), - info, + &info, initialTenantZoneConfig, ctcfg.IfNotExists, p.ExecCfg().TenantTestingKnobs,