Skip to content

Commit 7624f97

Browse files
committed
catalog: add metamorphic testing of leased catalog views
Previously, when we enabled leased descriptors for catalog views we lost coverage for the existing default behavior fetching descriptors using KV. This patch, makes the default metamorhpic, so that 90% of the time leased descriptors are used and 10% of the time we avoid using them. Fixes: #159747 Release note: None
1 parent 0e89695 commit 7624f97

File tree

25 files changed

+39
-255
lines changed

25 files changed

+39
-255
lines changed

pkg/BUILD.bazel

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,6 @@ ALL_TESTS = [
527527
"//pkg/sql/logictest/tests/fakedist-vec-off:fakedist-vec-off_test",
528528
"//pkg/sql/logictest/tests/fakedist:fakedist_test",
529529
"//pkg/sql/logictest/tests/local-dist-merge-backfill-declarative-schema-changer:local-dist-merge-backfill-declarative-schema-changer_test",
530-
"//pkg/sql/logictest/tests/local-leased-descriptors:local-leased-descriptors_test",
531530
"//pkg/sql/logictest/tests/local-legacy-schema-changer:local-legacy-schema-changer_test",
532531
"//pkg/sql/logictest/tests/local-mixed-25.4:local-mixed-25_4_test",
533532
"//pkg/sql/logictest/tests/local-prepared:local-prepared_test",
@@ -2113,7 +2112,6 @@ GO_TARGETS = [
21132112
"//pkg/sql/logictest/tests/fakedist-vec-off:fakedist-vec-off_test",
21142113
"//pkg/sql/logictest/tests/fakedist:fakedist_test",
21152114
"//pkg/sql/logictest/tests/local-dist-merge-backfill-declarative-schema-changer:local-dist-merge-backfill-declarative-schema-changer_test",
2116-
"//pkg/sql/logictest/tests/local-leased-descriptors:local-leased-descriptors_test",
21172115
"//pkg/sql/logictest/tests/local-legacy-schema-changer:local-legacy-schema-changer_test",
21182116
"//pkg/sql/logictest/tests/local-mixed-25.4:local-mixed-25_4_test",
21192117
"//pkg/sql/logictest/tests/local-prepared:local-prepared_test",

pkg/sql/catalog/descs/collection.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ var allowLeasedDescriptorsInCatalogViews = settings.RegisterBoolSetting(
459459
settings.ApplicationLevel,
460460
"sql.catalog.allow_leased_descriptors.enabled",
461461
"if true, catalog views (crdb_internal, information_schema, pg_catalog) can use leased descriptors for improved performance",
462-
true,
462+
lease.UseLeasedDescriptorsForCatalogDefault,
463463
settings.WithPublic,
464464
)
465465

pkg/sql/catalog/lease/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ go_library(
5454
"//pkg/util/hlc",
5555
"//pkg/util/log",
5656
"//pkg/util/log/logcrash",
57+
"//pkg/util/metamorphic",
5758
"//pkg/util/metric",
5859
"//pkg/util/mon",
5960
"//pkg/util/quotapool",

pkg/sql/catalog/lease/lease.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import (
4444
"github.com/cockroachdb/cockroach/pkg/util/hlc"
4545
"github.com/cockroachdb/cockroach/pkg/util/log"
4646
"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
47+
"github.com/cockroachdb/cockroach/pkg/util/metamorphic"
4748
"github.com/cockroachdb/cockroach/pkg/util/metric"
4849
"github.com/cockroachdb/cockroach/pkg/util/mon"
4950
"github.com/cockroachdb/cockroach/pkg/util/quotapool"
@@ -97,6 +98,21 @@ var LeaseMonitorRangeFeedResetTime = settings.RegisterDurationSetting(
9798
time.Minute*25,
9899
)
99100

101+
// diableLeasedDescriptorsByDefaultThreshold any value above this is considered
102+
// disabled, making it 10% odds of disabled.
103+
const diableLeasedDescriptorsByDefaultThreshold = 90
104+
105+
// disableLeasedDescriptorThresholdDefault, by default, leased descriptors
106+
// are enabled.
107+
const disableLeasedDescriptorThresholdDefault = 0
108+
109+
// UseLeasedDescriptorsForCatalogDefault determines if leased descrptor are used for catalog
110+
// views.
111+
var UseLeasedDescriptorsForCatalogDefault = metamorphic.ConstantWithTestRange("disable-catalog-leased-descriptors-threshold",
112+
disableLeasedDescriptorThresholdDefault,
113+
0,
114+
100) < diableLeasedDescriptorsByDefaultThreshold
115+
100116
var WaitForInitialVersion = settings.RegisterBoolSetting(settings.ApplicationLevel,
101117
"sql.catalog.descriptor_wait_for_initial_version.enabled",
102118
"enables waiting for the initial version of a descriptor",
@@ -106,7 +122,7 @@ var LockedLeaseTimestamp = settings.RegisterBoolSetting(settings.ApplicationLeve
106122
"sql.catalog.descriptor_lease.use_locked_timestamps.enabled",
107123
"guarantees transactional version consistency for descriptors used by the lease manager,"+
108124
"descriptors used can be intentionally older to support this",
109-
true)
125+
UseLeasedDescriptorsForCatalogDefault)
110126

111127
var RetainOldVersionsForLocked = settings.RegisterBoolSetting(settings.ApplicationLevel,
112128
"sql.catalog.descriptor_lease.lock_old_versions.enabled",

pkg/sql/logictest/logic.go

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1843,23 +1843,6 @@ func (t *logicTest) newCluster(
18431843
t.Fatal(err)
18441844
}
18451845
}
1846-
if cfg.EnableLeasedDescriptorSupport {
1847-
if _, err := conn.Exec(
1848-
"SET CLUSTER SETTING sql.catalog.allow_leased_descriptors.enabled = true",
1849-
); err != nil {
1850-
t.Fatal(err)
1851-
}
1852-
if _, err := conn.Exec(
1853-
"SET CLUSTER SETTING sql.catalog.descriptor_lease.use_locked_timestamps.enabled = true",
1854-
); err != nil {
1855-
t.Fatal(err)
1856-
}
1857-
if _, err := conn.Exec(
1858-
"SET CLUSTER SETTING sql.catalog.allow_leased_descriptors.prefetch.enabled = true"); err != nil {
1859-
t.Fatal(err)
1860-
}
1861-
}
1862-
18631846
if cfg.UseDistributedMergeIndexBackfill {
18641847
mode := "declarative"
18651848
if cfg.DisableDeclarativeSchemaChanger {

pkg/sql/logictest/logictestbase/logictestbase.go

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,6 @@ type TestClusterConfig struct {
9595
DisableSchemaLockedByDefault bool
9696
// PrepareQueries executes queries and statements with Prepare and Execute.
9797
PrepareQueries bool
98-
// EnableLeasedDescriptorSupport enables leased descriptors for pg_catalog /
99-
// crdb_internal and locked leasing behavior.
100-
EnableLeasedDescriptorSupport bool
10198
// UseDistributedMergeIndexBackfill enables the use of distributed
10299
// merge when index backfills are preformed.
103100
UseDistributedMergeIndexBackfill bool
@@ -511,19 +508,6 @@ var LogicTestConfigs = []TestClusterConfig{
511508
DisableUpgrade: true,
512509
DeclarativeCorpusCollection: true,
513510
},
514-
{
515-
Name: "local-leased-descriptors",
516-
NumNodes: 1,
517-
OverrideDistSQLMode: "off",
518-
// local is the configuration where we run all tests which have bad
519-
// interactions with the default test tenant.
520-
//
521-
// TODO(#156124): We should review this choice. Why can't we use "Random"
522-
// here? If there are specific tests that are incompatible, we can
523-
// flag them to run only in a separate config.
524-
UseSecondaryTenant: Never,
525-
EnableLeasedDescriptorSupport: true,
526-
},
527511
{
528512
// This config runs a cluster with 3 nodes, with a separate process per
529513
// node. The nodes initially start on v25.4.

pkg/sql/logictest/testdata/logic_test/comment_on

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# LogicTest: default-configs !local-prepared local-leased-descriptors 3node-tenant
1+
# LogicTest: default-configs !local-prepared 3node-tenant
22

33
statement ok
44
CREATE DATABASE db

pkg/sql/logictest/testdata/logic_test/crdb_internal

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# 3node-tenant is blocked from running this file due to heavy reliance on
22
# unavailable node IDs in this test.
3-
# LogicTest: !3node-tenant-default-configs !local-prepared local-leased-descriptors default-configs
3+
# LogicTest: !3node-tenant-default-configs !local-prepared default-configs
44

55
query error database "crdb_internal" does not exist
66
ALTER DATABASE crdb_internal RENAME TO not_crdb_internal

pkg/sql/logictest/testdata/logic_test/event_log

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,8 @@ AND info NOT LIKE '%sql.defaults%'
480480
AND info NOT LIKE '%sql.distsql%'
481481
AND info NOT LIKE '%sql.testing%'
482482
AND info NOT LIKE '%sql.stats%'
483+
AND info NOT LIKE '%sql.catalog.allow_leased_descriptor%'
484+
AND info NOT LIKE '%sql.catalog.descriptor_lease.use_locked_timestamps.enabled%'
483485
ORDER BY "timestamp", info
484486
----
485487
1 {"ApplicationName": "$ internal-optInToDiagnosticsStatReporting", "DefaultValue": "true", "EventType": "set_cluster_setting", "SettingName": "diagnostics.reporting.enabled", "Statement": "SET CLUSTER SETTING \"diagnostics.reporting.enabled\" = true", "Tag": "SET CLUSTER SETTING", "User": "node", "Value": "true"}
@@ -500,6 +502,8 @@ AND info NOT LIKE '%sql.defaults%'
500502
AND info NOT LIKE '%sql.distsql%'
501503
AND info NOT LIKE '%sql.testing%'
502504
AND info NOT LIKE '%sql.stats%'
505+
AND info NOT LIKE '%sql.catalog.allow_leased_descriptor%'
506+
AND info NOT LIKE '%sql.catalog.descriptor_lease.use_locked_timestamps.enabled%'
503507
ORDER BY "timestamp", info
504508
----
505509
1 {"ApplicationName": "$ internal-optInToDiagnosticsStatReporting", "DefaultValue": "true", "EventType": "set_cluster_setting", "SettingName": "diagnostics.reporting.enabled", "Statement": "SET CLUSTER SETTING \"diagnostics.reporting.enabled\" = true", "Tag": "SET CLUSTER SETTING", "User": "node", "Value": "true"}

pkg/sql/logictest/testdata/logic_test/fk

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# LogicTest: !local-prepared default-configs 3node-tenant local-leased-descriptors
1+
# LogicTest: !local-prepared default-configs 3node-tenant
22
statement ok
33
SET CLUSTER SETTING sql.cross_db_fks.enabled = TRUE
44

0 commit comments

Comments
 (0)