diff --git a/pkg/sql/catalog/tabledesc/BUILD.bazel b/pkg/sql/catalog/tabledesc/BUILD.bazel index 5f1e15b597ca..83cbf03c972d 100644 --- a/pkg/sql/catalog/tabledesc/BUILD.bazel +++ b/pkg/sql/catalog/tabledesc/BUILD.bazel @@ -60,6 +60,7 @@ go_library( "//pkg/sql/types", "//pkg/sql/vecindex/vecpb", "//pkg/util", + "//pkg/util/buildutil", "//pkg/util/errorutil/unimplemented", "//pkg/util/hlc", "//pkg/util/interval", diff --git a/pkg/sql/catalog/tabledesc/structured.go b/pkg/sql/catalog/tabledesc/structured.go index 3bfbf1a5a9c0..cafa664162f2 100644 --- a/pkg/sql/catalog/tabledesc/structured.go +++ b/pkg/sql/catalog/tabledesc/structured.go @@ -33,6 +33,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlerrors" "github.com/cockroachdb/cockroach/pkg/sql/types" + "github.com/cockroachdb/cockroach/pkg/util/buildutil" "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/intsets" @@ -636,6 +637,14 @@ func (desc *Mutable) AllocateIDs(ctx context.Context, version clusterversion.Clu return err } + // The virtual table descriptors are entirely under our control. Since + // validating is expensive, skip it for virtual tables if this code is running + // in a test environment. This step was found to be a bottleneck in TestServer + // startup. + if desc.IsVirtualTable() && buildutil.CrdbTestBuild { + return nil + } + // This is sort of ugly. If the descriptor does not have an ID, we hack one in // to pass the table ID check. We use a non-reserved ID, reserved ones being set // before AllocateIDs. diff --git a/pkg/sql/virtual_schema.go b/pkg/sql/virtual_schema.go index a87bf186ac1f..413f062ecd32 100644 --- a/pkg/sql/virtual_schema.go +++ b/pkg/sql/virtual_schema.go @@ -41,10 +41,12 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" "github.com/cockroachdb/cockroach/pkg/sql/sqlerrors" "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" + "github.com/cockroachdb/cockroach/pkg/util/ctxgroup" "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/iterutil" "github.com/cockroachdb/cockroach/pkg/util/stop" + "github.com/cockroachdb/cockroach/pkg/util/syncutil" "github.com/cockroachdb/errors" "github.com/cockroachdb/redact" ) @@ -444,9 +446,6 @@ type VirtualSchemaHolder struct { orderedNames []string catalogCache nstree.MutableCatalog - // Needed for backward-compat on crdb_internal.ranges{_no_leases}. - // Remove in v23.2. - st *cluster.Settings } var _ VirtualTabler = (*VirtualSchemaHolder)(nil) @@ -930,10 +929,6 @@ func NewVirtualSchemaHolder( schemasByID: make(map[descpb.ID]*virtualSchemaEntry, len(virtualSchemas)), orderedNames: make([]string, len(virtualSchemas)), defsByID: make(map[descpb.ID]*virtualDefEntry, math.MaxUint32-catconstants.MinVirtualID), - - // Needed for backward-compat on crdb_internal.ranges{_no_leases}. - // Remove in v23.2. - st: st, } order := 0 @@ -985,16 +980,29 @@ func NewVirtualSchemaHolder( return tableDesc, entry, nil } + // Initialize virtual tables concurrently. This happens all at once during + // server startup, which is a bottleneck for startup time, especially in + // the TestServer used by unit tests. Adding concurrency here speeds up + // TestServer startup by about 7% in SharedTenant mode. + g := ctxgroup.WithContext(ctx) + var mu syncutil.Mutex for id, def := range schema.tableDefs { - tableDesc, entry, err := doTheWork(id, def, false /* bumpVersion */) - if err != nil { - return nil, err - } - defs[tableDesc.Name] = entry - vs.defsByID[tableDesc.ID] = entry - orderedDefNames = append(orderedDefNames, tableDesc.Name) + g.GoCtx(func(ctx context.Context) error { + tableDesc, entry, err := doTheWork(id, def, false /* bumpVersion */) + if err != nil { + return err + } + mu.Lock() + defer mu.Unlock() + defs[tableDesc.Name] = entry + vs.defsByID[tableDesc.ID] = entry + orderedDefNames = append(orderedDefNames, tableDesc.Name) + return nil + }) + } + if err := g.Wait(); err != nil { + return nil, err } - sort.Strings(orderedDefNames) vse := &virtualSchemaEntry{ diff --git a/pkg/util/ctxgroup/ctxgroup.go b/pkg/util/ctxgroup/ctxgroup.go index f2bc811b5d78..8e2d5fca2461 100644 --- a/pkg/util/ctxgroup/ctxgroup.go +++ b/pkg/util/ctxgroup/ctxgroup.go @@ -166,6 +166,16 @@ func WithContext(ctx context.Context) Group { } } +// SetLimit limits the number of active goroutines in this group to at most n. A +// negative value indicates no limit. A limit of zero will prevent any new +// goroutines from being added. Any subsequent call to the Go method will block +// until it can add an active goroutine without exceeding the configured limit. +// The limit must not be modified while any goroutines in the group are active. +// This delegates to errgroup.Group.SetLimit. +func (g Group) SetLimit(n int) { + g.wrapped.SetLimit(n) +} + // Go calls the given function in a new goroutine. func (g Group) Go(f func() error) { g.wrapped.Go(f)