Merge #154051

154051: sql: add setting to gate leased descriptors for vtables r=rafiss a=rafiss

This adds a cluster setting to control if we will use leased descriptors to populate virtual tables. It's off by default, and we plan to enable it eventually in a later release.

As part of this, we also update the remaining places where we call GetAllDescriptors or related functions to make those use leased descriptors if the setting is enabled.

fixes #154043

Release note (sql change): Added the
sql.catalog.allow_leased_descriptors.enabled cluster setting, which is false by default. When set to true, queries that access the pg_catalog or information_schema can use cached leased descriptors to populate the data in those tables, with the tradeoff that some of the data could be stale.

Co-authored-by: Rafi Shamim <[email protected]>

Author: craig[bot]

docs/generated/settings/settings-for-tenants.txt

@@ -177,6 +177,7 @@ sql.auth.change_own_password.enabled	boolean	false	controls whether a user is al
 sql.auth.grant_option_for_owner.enabled	boolean	true	determines whether the GRANT OPTION for privileges is implicitly given to the owner of an object	application
 sql.auth.grant_option_inheritance.enabled	boolean	true	determines whether the GRANT OPTION for privileges is inherited through role membership	application
 sql.auth.public_schema_create_privilege.enabled	boolean	true	determines whether to grant all users the CREATE privileges on the public schema when it is created	application
+sql.catalog.allow_leased_descriptors.enabled	boolean	false	if true, catalog views (crdb_internal, information_schema, pg_catalog) can use leased descriptors for improved performance	application
 sql.closed_session_cache.capacity	integer	1000	the maximum number of sessions in the cache	application
 sql.closed_session_cache.time_to_live	integer	3600	the maximum time to live, in seconds	application
 sql.contention.event_store.capacity	byte size	64 MiB	the in-memory storage capacity per-node of contention event store	application

docs/generated/settings/settings.html

@@ -214,6 +214,7 @@
 <tr><td><div id="setting-sql-auth-grant-option-for-owner-enabled" class="anchored"><code>sql.auth.grant_option_for_owner.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>determines whether the GRANT OPTION for privileges is implicitly given to the owner of an object</td><td>Basic/Standard/Advanced/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-auth-grant-option-inheritance-enabled" class="anchored"><code>sql.auth.grant_option_inheritance.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>determines whether the GRANT OPTION for privileges is inherited through role membership</td><td>Basic/Standard/Advanced/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-auth-public-schema-create-privilege-enabled" class="anchored"><code>sql.auth.public_schema_create_privilege.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>determines whether to grant all users the CREATE privileges on the public schema when it is created</td><td>Basic/Standard/Advanced/Self-Hosted</td></tr>
+<tr><td><div id="setting-sql-catalog-allow-leased-descriptors-enabled" class="anchored"><code>sql.catalog.allow_leased_descriptors.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>if true, catalog views (crdb_internal, information_schema, pg_catalog) can use leased descriptors for improved performance</td><td>Basic/Standard/Advanced/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-closed-session-cache-capacity" class="anchored"><code>sql.closed_session_cache.capacity</code></div></td><td>integer</td><td><code>1000</code></td><td>the maximum number of sessions in the cache</td><td>Basic/Standard/Advanced/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-closed-session-cache-time-to-live" class="anchored"><code>sql.closed_session_cache.time_to_live</code></div></td><td>integer</td><td><code>3600</code></td><td>the maximum time to live, in seconds</td><td>Basic/Standard/Advanced/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-contention-event-store-capacity" class="anchored"><code>sql.contention.event_store.capacity</code></div></td><td>byte size</td><td><code>64 MiB</code></td><td>the in-memory storage capacity per-node of contention event store</td><td>Basic/Standard/Advanced/Self-Hosted</td></tr>

pkg/cmd/roachtest/tests/large_schema_benchmark.go

@@ -156,6 +156,10 @@ func registerLargeSchemaBenchmark(r registry.Registry, numTables int, isMultiReg
 						// of time. In multi-region this latency can be substantial.
 						_, err := conn.Exec("SET CLUSTER SETTING sql.defaults.autocommit_before_ddl.enabled = 'false'")
 						require.NoError(t, err)
+						// Allow optimizations to use leased descriptors when querying
+						// pg_catalog and information_schema.
+						_, err = conn.Exec("SET CLUSTER SETTING sql.catalog.allow_leased_descriptors.enabled = 'true'")
+						require.NoError(t, err)
 						// Since we will be making a large number of databases / tables
 						// quickly,on MR the job retention can slow things down. Let's
 						// minimize how long jobs are kept, so that the creation / ingest

pkg/sql/catalog/descs/BUILD.bazel

@@ -36,6 +36,7 @@ go_library(
         "//pkg/kv",
         "//pkg/roachpb",
         "//pkg/server/serverpb",
+        "//pkg/settings",
         "//pkg/settings/cluster",
         "//pkg/spanconfig",
         "//pkg/sql/catalog",

pkg/sql/catalog/descs/collection.go

@@ -16,6 +16,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/kv"
 	"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/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/bootstrap"
@@ -369,6 +370,63 @@ func WithOnlyVersionBump() WriteDescOption {
 	return onlyVersionBumpOption(true)
 }
 
+type getAllOptions struct {
+	allowLeased bool
+}
+
+// GetAllOption defines functional options for GetAll* methods.
+type GetAllOption interface {
+	apply(*getAllOptions)
+}
+
+type allowLeasedOption bool
+
+func (c allowLeasedOption) apply(opts *getAllOptions) {
+	opts.allowLeased = bool(c)
+}
+
+// WithAllowLeased configures GetAll* methods to allow leased descriptors.
+func WithAllowLeased() GetAllOption {
+	return allowLeasedOption(true)
+}
+
+// applyGetAllOptions applies the provided functional options to a getAllOptions struct.
+func applyGetAllOptions(opts []GetAllOption) getAllOptions {
+	options := getAllOptions{}
+	for _, opt := range opts {
+		opt.apply(&options)
+	}
+	return options
+}
+
+var allowLeasedDescriptorsInCatalogViews = settings.RegisterBoolSetting(
+	settings.ApplicationLevel,
+	"sql.catalog.allow_leased_descriptors.enabled",
+	"if true, catalog views (crdb_internal, information_schema, pg_catalog) can use leased descriptors for improved performance",
+	false,
+	settings.WithPublic,
+)
+
+// GetCatalogGetAllOptions returns the functional options for GetAll* methods
+// based on the cluster setting for allowing leased descriptors in catalog views.
+func GetCatalogGetAllOptions(sv *settings.Values) []GetAllOption {
+	if allowLeasedDescriptorsInCatalogViews.Get(sv) {
+		return []GetAllOption{WithAllowLeased()}
+	}
+	return nil
+}
+
+// GetCatalogDescriptorGetter returns the appropriate descriptor getter for
+// catalog views based on the cluster setting.
+func GetCatalogDescriptorGetter(
+	descriptors *Collection, txn *kv.Txn, sv *settings.Values,
+) ByIDGetterBuilder {
+	if allowLeasedDescriptorsInCatalogViews.Get(sv) {
+		return descriptors.ByIDWithLeased(txn)
+	}
+	return descriptors.ByIDWithoutLeased(txn)
+}
+
 // maybeMarkVersionBumpOnly updates the version bump only flag for the
 // descriptor so it reflects previous mutations to the descriptor along with the
 // current mutation.
@@ -830,12 +888,15 @@ func newMutableSyntheticDescriptorAssertionError(id descpb.ID) error {
 
 // GetAll returns all descriptors, namespace entries, comments and
 // zone configs visible by the transaction.
-func (tc *Collection) GetAll(ctx context.Context, txn *kv.Txn) (nstree.Catalog, error) {
+func (tc *Collection) GetAll(
+	ctx context.Context, txn *kv.Txn, opts ...GetAllOption,
+) (nstree.Catalog, error) {
+	options := applyGetAllOptions(opts)
 	stored, err := tc.cr.ScanAll(ctx, txn)
 	if err != nil {
 		return nstree.Catalog{}, err
 	}
-	ret, err := tc.aggregateAllLayers(ctx, txn, stored)
+	ret, err := tc.aggregateAllLayers(ctx, txn, options.allowLeased, stored)
 	if err != nil {
 		return nstree.Catalog{}, err
 	}
@@ -860,7 +921,8 @@ func (tc *Collection) GetAllComments(
 	if err != nil {
 		return nil, err
 	}
-	comments, err := tc.aggregateAllLayers(ctx, txn, kvComments)
+	const allowLeased = false
+	comments, err := tc.aggregateAllLayers(ctx, txn, allowLeased, kvComments)
 	if err != nil {
 		return nil, err
 	}
@@ -878,12 +940,15 @@ func (tc *Collection) GetAllFromStorageUnvalidated(
 }
 
 // GetAllDatabases is like GetAll but filtered to non-dropped databases.
-func (tc *Collection) GetAllDatabases(ctx context.Context, txn *kv.Txn) (nstree.Catalog, error) {
+func (tc *Collection) GetAllDatabases(
+	ctx context.Context, txn *kv.Txn, opts ...GetAllOption,
+) (nstree.Catalog, error) {
+	options := applyGetAllOptions(opts)
 	stored, err := tc.cr.ScanNamespaceForDatabases(ctx, txn)
 	if err != nil {
 		return nstree.Catalog{}, err
 	}
-	ret, err := tc.aggregateAllLayers(ctx, txn, stored)
+	ret, err := tc.aggregateAllLayers(ctx, txn, options.allowLeased, stored)
 	if err != nil {
 		return nstree.Catalog{}, err
 	}
@@ -901,17 +966,18 @@ func (tc *Collection) GetAllDatabases(ctx context.Context, txn *kv.Txn) (nstree.
 // GetAllSchemasInDatabase is like GetAll but filtered to the schemas with
 // the specified parent database. Includes virtual schemas.
 func (tc *Collection) GetAllSchemasInDatabase(
-	ctx context.Context, txn *kv.Txn, db catalog.DatabaseDescriptor,
+	ctx context.Context, txn *kv.Txn, db catalog.DatabaseDescriptor, opts ...GetAllOption,
 ) (nstree.Catalog, error) {
+	options := applyGetAllOptions(opts)
 	stored, err := tc.cr.ScanNamespaceForDatabaseSchemas(ctx, txn, db)
 	if err != nil {
 		return nstree.Catalog{}, err
 	}
 	var ret nstree.MutableCatalog
 	if db.HasPublicSchemaWithDescriptor() {
-		ret, err = tc.aggregateAllLayers(ctx, txn, stored)
+		ret, err = tc.aggregateAllLayers(ctx, txn, options.allowLeased, stored)
 	} else {
-		ret, err = tc.aggregateAllLayers(ctx, txn, stored, schemadesc.GetPublicSchema())
+		ret, err = tc.aggregateAllLayers(ctx, txn, options.allowLeased, stored, schemadesc.GetPublicSchema())
 	}
 	if err != nil {
 		return nstree.Catalog{}, err
@@ -938,8 +1004,13 @@ func (tc *Collection) GetAllSchemasInDatabase(
 // the specified parent schema. Includes virtual objects. Does not include
 // dropped objects.
 func (tc *Collection) GetAllObjectsInSchema(
-	ctx context.Context, txn *kv.Txn, db catalog.DatabaseDescriptor, sc catalog.SchemaDescriptor,
+	ctx context.Context,
+	txn *kv.Txn,
+	db catalog.DatabaseDescriptor,
+	sc catalog.SchemaDescriptor,
+	opts ...GetAllOption,
 ) (nstree.Catalog, error) {
+	options := applyGetAllOptions(opts)
 	var ret nstree.MutableCatalog
 	if sc.SchemaKind() == catalog.SchemaVirtual {
 		tc.virtual.addAllToCatalog(ret)
@@ -948,7 +1019,7 @@ func (tc *Collection) GetAllObjectsInSchema(
 		if err != nil {
 			return nstree.Catalog{}, err
 		}
-		ret, err = tc.aggregateAllLayers(ctx, txn, stored, sc)
+		ret, err = tc.aggregateAllLayers(ctx, txn, options.allowLeased, stored, sc)
 		if err != nil {
 			return nstree.Catalog{}, err
 		}
@@ -967,13 +1038,14 @@ func (tc *Collection) GetAllObjectsInSchema(
 // GetAllObjectsInSchema applied to each of those schemas.
 // Includes virtual objects. Does not include dropped objects.
 func (tc *Collection) GetAllInDatabase(
-	ctx context.Context, txn *kv.Txn, db catalog.DatabaseDescriptor,
+	ctx context.Context, txn *kv.Txn, db catalog.DatabaseDescriptor, opts ...GetAllOption,
 ) (nstree.Catalog, error) {
+	options := applyGetAllOptions(opts)
 	stored, err := tc.cr.ScanNamespaceForDatabaseSchemasAndObjects(ctx, txn, db)
 	if err != nil {
 		return nstree.Catalog{}, err
 	}
-	schemas, err := tc.GetAllSchemasInDatabase(ctx, txn, db)
+	schemas, err := tc.GetAllSchemasInDatabase(ctx, txn, db, opts...)
 	if err != nil {
 		return nstree.Catalog{}, err
 	}
@@ -985,7 +1057,7 @@ func (tc *Collection) GetAllInDatabase(
 	}); err != nil {
 		return nstree.Catalog{}, err
 	}
-	ret, err := tc.aggregateAllLayers(ctx, txn, stored, schemasSlice...)
+	ret, err := tc.aggregateAllLayers(ctx, txn, options.allowLeased, stored, schemasSlice...)
 	if err != nil {
 		return nstree.Catalog{}, err
 	}
@@ -1013,17 +1085,18 @@ func (tc *Collection) GetAllInDatabase(
 // GetAllTablesInDatabase is like GetAllInDatabase but filtered to tables.
 // Includes virtual objects. Does not include dropped objects.
 func (tc *Collection) GetAllTablesInDatabase(
-	ctx context.Context, txn *kv.Txn, db catalog.DatabaseDescriptor,
+	ctx context.Context, txn *kv.Txn, db catalog.DatabaseDescriptor, opts ...GetAllOption,
 ) (nstree.Catalog, error) {
+	options := applyGetAllOptions(opts)
 	stored, err := tc.cr.ScanNamespaceForDatabaseSchemasAndObjects(ctx, txn, db)
 	if err != nil {
 		return nstree.Catalog{}, err
 	}
 	var ret nstree.MutableCatalog
 	if db.HasPublicSchemaWithDescriptor() {
-		ret, err = tc.aggregateAllLayers(ctx, txn, stored)
+		ret, err = tc.aggregateAllLayers(ctx, txn, options.allowLeased, stored)
 	} else {
-		ret, err = tc.aggregateAllLayers(ctx, txn, stored, schemadesc.GetPublicSchema())
+		ret, err = tc.aggregateAllLayers(ctx, txn, options.allowLeased, stored, schemadesc.GetPublicSchema())
 	}
 	if err != nil {
 		return nstree.Catalog{}, err
@@ -1046,7 +1119,11 @@ func (tc *Collection) GetAllTablesInDatabase(
 // takes care to stack all of the Collection's layer appropriately and ensures
 // that the returned descriptors are properly hydrated and validated.
 func (tc *Collection) aggregateAllLayers(
-	ctx context.Context, txn *kv.Txn, stored nstree.Catalog, schemas ...catalog.SchemaDescriptor,
+	ctx context.Context,
+	txn *kv.Txn,
+	allowLeased bool,
+	stored nstree.Catalog,
+	schemas ...catalog.SchemaDescriptor,
 ) (ret nstree.MutableCatalog, _ error) {
 	// Descriptors need to be re-read to ensure proper validation hydration etc.
 	// We collect their IDs for this purpose and we'll re-add them later.
@@ -1142,8 +1219,12 @@ func (tc *Collection) aggregateAllLayers(
 	// Remove deleted descriptors from consideration, re-read and add the rest.
 	tc.deletedDescs.ForEach(descIDs.Remove)
 	allDescs := make([]catalog.Descriptor, descIDs.Len())
+	flags := defaultUnleasedFlags()
+	if allowLeased {
+		flags.layerFilters.withoutLeased = false
+	}
 	if err := getDescriptorsByID(
-		ctx, tc, txn, defaultUnleasedFlags(), allDescs, descIDs.Ordered()...,
+		ctx, tc, txn, flags, allDescs, descIDs.Ordered()...,
 	); err != nil {
 		return nstree.MutableCatalog{}, err
 	}
@@ -1159,7 +1240,7 @@ func (tc *Collection) aggregateAllLayers(
 // in the requested database.
 // Deprecated: prefer GetAllInDatabase.
 func (tc *Collection) GetAllDescriptorsForDatabase(
-	ctx context.Context, txn *kv.Txn, db catalog.DatabaseDescriptor,
+	ctx context.Context, txn *kv.Txn, db catalog.DatabaseDescriptor, opts ...GetAllOption,
 ) (nstree.Catalog, error) {
 	// Re-read database descriptor to have the freshest version.
 	{
@@ -1169,7 +1250,7 @@ func (tc *Collection) GetAllDescriptorsForDatabase(
 			return nstree.Catalog{}, err
 		}
 	}
-	c, err := tc.GetAllInDatabase(ctx, txn, db)
+	c, err := tc.GetAllInDatabase(ctx, txn, db, opts...)
 	if err != nil {
 		return nstree.Catalog{}, err
 	}
@@ -1185,8 +1266,10 @@ func (tc *Collection) GetAllDescriptorsForDatabase(
 // GetAllDescriptors returns all physical descriptors visible by the
 // transaction.
 // Deprecated: prefer GetAll.
-func (tc *Collection) GetAllDescriptors(ctx context.Context, txn *kv.Txn) (nstree.Catalog, error) {
-	all, err := tc.GetAll(ctx, txn)
+func (tc *Collection) GetAllDescriptors(
+	ctx context.Context, txn *kv.Txn, opts ...GetAllOption,
+) (nstree.Catalog, error) {
+	all, err := tc.GetAll(ctx, txn, opts...)
 	if err != nil {
 		return nstree.Catalog{}, err
 	}
@@ -1213,9 +1296,9 @@ func (tc *Collection) GetAllDescriptors(ctx context.Context, txn *kv.Txn) (nstre
 // transaction, ordered by name.
 // Deprecated: prefer GetAllDatabases.
 func (tc *Collection) GetAllDatabaseDescriptors(
-	ctx context.Context, txn *kv.Txn,
+	ctx context.Context, txn *kv.Txn, opts ...GetAllOption,
 ) (ret []catalog.DatabaseDescriptor, _ error) {
-	c, err := tc.GetAllDatabases(ctx, txn)
+	c, err := tc.GetAllDatabases(ctx, txn, opts...)
 	if err != nil {
 		return nil, err
 	}