From 9707e5e8a5251ae6c302fd970f52d8424b0b269f Mon Sep 17 00:00:00 2001 From: Jeff Date: Sun, 16 Oct 2022 21:22:30 +0000 Subject: [PATCH] slstorage: migrate system.sqlliveness to an rbr table Migrate the sqlliveness table to a format that is compatible with regional by row tables. Multi region serverless is the motiviation for this change. When a sql server starts up, it must write its session to the sqlliveness table. The remote session write can add ~400ms to a servers startup time. Part of #85736 Release note: None --- .../settings/settings-for-tenants.txt | 2 +- docs/generated/settings/settings.html | 2 +- pkg/ccl/multiregionccl/BUILD.bazel | 7 ++ .../multiregion_system_table_test.go | 115 ++++++++++++++++++ .../tenantcostclient/tenant_side_test.go | 2 +- pkg/clusterversion/cockroach_versions.go | 9 ++ pkg/clusterversion/key_string.go | 5 +- pkg/sql/catalog/systemschema/system.go | 27 ++-- .../systemschema_test/testdata/bootstrap | 8 +- .../testdata/logic_test/information_schema | 12 +- .../logictest/testdata/logic_test/pg_catalog | 5 +- pkg/sql/sqlliveness/slstorage/BUILD.bazel | 3 + .../sqlliveness/slstorage/sessionid_test.go | 1 + pkg/sql/sqlliveness/slstorage/slstorage.go | 2 +- .../sqlliveness/slstorage/slstorage_test.go | 31 +++-- pkg/sql/sqlliveness/slstorage/table.go | 112 ++++++++++++++--- pkg/sql/sqlliveness/slstorage/table_test.go | 78 ++++++++++-- pkg/upgrade/upgrades/BUILD.bazel | 1 + pkg/upgrade/upgrades/rbr_sqlliveness.go | 27 ++++ 19 files changed, 387 insertions(+), 62 deletions(-) create mode 100644 pkg/ccl/multiregionccl/multiregion_system_table_test.go create mode 100644 pkg/upgrade/upgrades/rbr_sqlliveness.go diff --git a/docs/generated/settings/settings-for-tenants.txt b/docs/generated/settings/settings-for-tenants.txt index c41cba90ea85..452adb1054e1 100644 --- a/docs/generated/settings/settings-for-tenants.txt +++ b/docs/generated/settings/settings-for-tenants.txt @@ -297,4 +297,4 @@ trace.jaeger.agent string the address of a Jaeger agent to receive traces using trace.opentelemetry.collector string address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as :. If no port is specified, 4317 will be used. trace.span_registry.enabled boolean true if set, ongoing traces can be seen at https:///#/debug/tracez trace.zipkin.collector string the address of a Zipkin instance to receive traces, as :. If no port is specified, 9411 will be used. -version version 1000022.2-4 set the active cluster version in the format '.' +version version 1000022.2-6 set the active cluster version in the format '.' diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html index 9b9e362c525c..70de428b4019 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -233,6 +233,6 @@ trace.opentelemetry.collectorstringaddress of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as :. If no port is specified, 4317 will be used. trace.span_registry.enabledbooleantrueif set, ongoing traces can be seen at https:///#/debug/tracez trace.zipkin.collectorstringthe address of a Zipkin instance to receive traces, as :. If no port is specified, 9411 will be used. -versionversion1000022.2-4set the active cluster version in the format '.' +versionversion1000022.2-6set the active cluster version in the format '.' diff --git a/pkg/ccl/multiregionccl/BUILD.bazel b/pkg/ccl/multiregionccl/BUILD.bazel index 4ffc2f706a8b..623f7c3833d0 100644 --- a/pkg/ccl/multiregionccl/BUILD.bazel +++ b/pkg/ccl/multiregionccl/BUILD.bazel @@ -28,6 +28,7 @@ go_test( srcs = [ "datadriven_test.go", "main_test.go", + "multiregion_system_table_test.go", "multiregion_test.go", "region_test.go", "regional_by_row_test.go", @@ -62,10 +63,12 @@ go_test( "//pkg/sql/catalog/descpb", "//pkg/sql/catalog/descs", "//pkg/sql/catalog/desctestutils", + "//pkg/sql/enum", "//pkg/sql/execinfra", "//pkg/sql/parser", "//pkg/sql/rowenc", "//pkg/sql/sem/tree", + "//pkg/sql/sqlliveness/slstorage", "//pkg/sql/sqltestutils", "//pkg/sql/tests", "//pkg/testutils", @@ -74,12 +77,16 @@ go_test( "//pkg/testutils/sqlutils", "//pkg/testutils/testcluster", "//pkg/util", + "//pkg/util/hlc", "//pkg/util/leaktest", "//pkg/util/log", "//pkg/util/randutil", "//pkg/util/syncutil", + "//pkg/util/timeutil", "//pkg/util/tracing", "//pkg/util/tracing/tracingpb", + "//pkg/util/uuid", + "@com_github_cockroachdb_apd_v3//:apd", "@com_github_cockroachdb_datadriven//:datadriven", "@com_github_cockroachdb_errors//:errors", "@com_github_stretchr_testify//require", diff --git a/pkg/ccl/multiregionccl/multiregion_system_table_test.go b/pkg/ccl/multiregionccl/multiregion_system_table_test.go new file mode 100644 index 000000000000..09c2ca0921ec --- /dev/null +++ b/pkg/ccl/multiregionccl/multiregion_system_table_test.go @@ -0,0 +1,115 @@ +// Copyright 2022 The Cockroach Authors. +// +// Licensed as a CockroachDB Enterprise file under the Cockroach Community +// License (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt + +package multiregionccl + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/cockroachdb/apd/v3" + "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/ccl/multiregionccl/multiregionccltestutils" + "github.com/cockroachdb/cockroach/pkg/keys" + "github.com/cockroachdb/cockroach/pkg/kv" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" + "github.com/cockroachdb/cockroach/pkg/sql/enum" + "github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slstorage" + "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" + "github.com/cockroachdb/cockroach/pkg/util/hlc" + "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/cockroachdb/cockroach/pkg/util/timeutil" + "github.com/cockroachdb/cockroach/pkg/util/uuid" + "github.com/stretchr/testify/require" +) + +func createSqllivenessTable( + t *testing.T, db *sqlutils.SQLRunner, dbName string, +) (tableID descpb.ID) { + t.Helper() + db.Exec(t, fmt.Sprintf(` + CREATE DATABASE IF NOT EXISTS "%s" + WITH PRIMARY REGION "us-east1" + REGIONS "us-east1", "us-east2", "us-east3" + `, dbName)) + + // expiration needs to be column 2. slstorage.Table assumes the column id. + // session_uuid and crdb_region are identified by their location in the + // primary key. + db.Exec(t, fmt.Sprintf(` + CREATE TABLE "%s".sqlliveness ( + session_uuid BYTES NOT NULL, + expiration DECIMAL NOT NULL, + crdb_region "%s".public.crdb_internal_region, + PRIMARY KEY(crdb_region, session_uuid) + ) LOCALITY REGIONAL BY ROW; + `, dbName, dbName)) + db.QueryRow(t, ` + select u.id + from system.namespace t + join system.namespace u + on t.id = u."parentID" + where t.name = $1 and u.name = $2`, + dbName, "sqlliveness").Scan(&tableID) + return tableID +} + +func TestRbrSqllivenessTable(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + ctx := context.Background() + + cluster, sqlDB, cleanup := multiregionccltestutils.TestingCreateMultiRegionCluster(t, 3, base.TestingKnobs{}) + defer cleanup() + settings := cluster.Servers[0].Cfg.Settings + kvDB := cluster.Servers[0].DB() + + tDB := sqlutils.MakeSQLRunner(sqlDB) + + t0 := time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC) + timeSource := timeutil.NewManualTime(t0) + clock := hlc.NewClock(timeSource, base.DefaultMaxClockOffset) + + setup := func(t *testing.T) slstorage.Table { + dbName := t.Name() + tableID := createSqllivenessTable(t, tDB, dbName) + return slstorage.MakeTestTable(settings, keys.SystemSQLCodec, tableID, 1, 2) + } + + t.Run("SqlRead", func(t *testing.T) { + table := setup(t) + + initialUUID := uuid.MakeV4() + session, err := slstorage.MakeSessionID(enum.One, initialUUID) + require.NoError(t, err) + + writeExpiration := clock.Now().Add(10, 00) + require.NoError(t, kvDB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error { + return table.SetExpiration(ctx, txn, session, writeExpiration) + })) + + var sessionUUID string + var crdbRegion string + var rawExpiration apd.Decimal + + row := tDB.QueryRow(t, fmt.Sprintf(`SELECT crdb_region, session_uuid, expiration FROM "%s".sqlliveness`, t.Name())) + row.Scan(&crdbRegion, &sessionUUID, &rawExpiration) + + require.Contains(t, []string{"us-east1", "us-east2", "us-east3"}, crdbRegion) + require.Equal(t, sessionUUID, string(initialUUID.GetBytes())) + + readExpiration, err := hlc.DecimalToHLC(&rawExpiration) + require.NoError(t, err) + + require.Equal(t, writeExpiration, readExpiration) + }) +} diff --git a/pkg/ccl/multitenantccl/tenantcostclient/tenant_side_test.go b/pkg/ccl/multitenantccl/tenantcostclient/tenant_side_test.go index be1618c69f37..66afe13d71a5 100644 --- a/pkg/ccl/multitenantccl/tenantcostclient/tenant_side_test.go +++ b/pkg/ccl/multitenantccl/tenantcostclient/tenant_side_test.go @@ -941,7 +941,7 @@ func TestSQLLivenessExemption(t *testing.T) { _ = r codec := keys.MakeSQLCodec(tenantID) - key := codec.IndexPrefix(keys.SqllivenessID, 1) + key := codec.TablePrefix(keys.SqllivenessID) // livenessValue returns the KV value for the one row in the // system.sqlliveness table. The value contains the session expiration time diff --git a/pkg/clusterversion/cockroach_versions.go b/pkg/clusterversion/cockroach_versions.go index fc78a1adc363..009529185ddf 100644 --- a/pkg/clusterversion/cockroach_versions.go +++ b/pkg/clusterversion/cockroach_versions.go @@ -321,6 +321,11 @@ const ( // Step (1): Add new versions here. // Do not add new versions to a patch release. // ************************************************* + + // RbrSqlliveness switches the system.sqlliveness descriptor to the new + // regional by row compatible format. It also causes the slstorage package + // to stop writing to the legacy rbt index. + RbrSqlliveness ) // TODOPreV22_1 is an alias for V22_1 for use in any version gate/check that @@ -519,6 +524,10 @@ var rawVersionsSingleton = keyedVersions{ Key: TenantNames, Version: roachpb.Version{Major: 22, Minor: 2, Internal: 4}, }, + { + Key: RbrSqlliveness, + Version: roachpb.Version{Major: 22, Minor: 2, Internal: 6}, + }, // ************************************************* // Step (2): Add new versions here. diff --git a/pkg/clusterversion/key_string.go b/pkg/clusterversion/key_string.go index 75e43e8a75ac..87feb6342667 100644 --- a/pkg/clusterversion/key_string.go +++ b/pkg/clusterversion/key_string.go @@ -51,11 +51,12 @@ func _() { _ = x[V22_2-39] _ = x[V23_1Start-40] _ = x[TenantNames-41] + _ = x[RbrSqlliveness-42] } -const _Key_name = "invalidVersionKeyV22_1Start22_2LocalTimestampsPebbleFormatSplitUserKeysMarkedCompactedEnsurePebbleFormatVersionRangeKeysEnablePebbleFormatVersionRangeKeysTrigramInvertedIndexesRemoveGrantPrivilegeMVCCRangeTombstonesUpgradeSequenceToBeReferencedByIDSampledStmtDiagReqsAddSSTableTombstonesSystemPrivilegesTableEnablePredicateProjectionChangefeedAlterSystemSQLInstancesAddLocalitySystemExternalConnectionsTableAlterSystemStatementStatisticsAddIndexRecommendationsRoleIDSequenceAddSystemUserIDColumnSystemUsersIDColumnIsBackfilledSetSystemUsersUserIDColumnNotNullSQLSchemaTelemetryScheduledJobsSchemaChangeSupportsCreateFunctionDeleteRequestReturnKeyPebbleFormatPrePebblev1MarkedRoleOptionsTableHasIDColumnRoleOptionsIDColumnIsBackfilledSetRoleOptionsUserIDColumnNotNullUseDelRangeInGCJobWaitedForDelRangeInGCJobRangefeedUseOneStreamPerNodeNoNonMVCCAddSSTableGCHintInReplicaStateUpdateInvalidColumnIDsInSequenceBackReferencesTTLDistSQLPrioritizeSnapshotsEnableLeaseUpgradeSupportAssumeRoleAuthFixUserfileRelatedDescriptorCorruptionV22_2V23_1StartTenantNames" +const _Key_name = "invalidVersionKeyV22_1Start22_2LocalTimestampsPebbleFormatSplitUserKeysMarkedCompactedEnsurePebbleFormatVersionRangeKeysEnablePebbleFormatVersionRangeKeysTrigramInvertedIndexesRemoveGrantPrivilegeMVCCRangeTombstonesUpgradeSequenceToBeReferencedByIDSampledStmtDiagReqsAddSSTableTombstonesSystemPrivilegesTableEnablePredicateProjectionChangefeedAlterSystemSQLInstancesAddLocalitySystemExternalConnectionsTableAlterSystemStatementStatisticsAddIndexRecommendationsRoleIDSequenceAddSystemUserIDColumnSystemUsersIDColumnIsBackfilledSetSystemUsersUserIDColumnNotNullSQLSchemaTelemetryScheduledJobsSchemaChangeSupportsCreateFunctionDeleteRequestReturnKeyPebbleFormatPrePebblev1MarkedRoleOptionsTableHasIDColumnRoleOptionsIDColumnIsBackfilledSetRoleOptionsUserIDColumnNotNullUseDelRangeInGCJobWaitedForDelRangeInGCJobRangefeedUseOneStreamPerNodeNoNonMVCCAddSSTableGCHintInReplicaStateUpdateInvalidColumnIDsInSequenceBackReferencesTTLDistSQLPrioritizeSnapshotsEnableLeaseUpgradeSupportAssumeRoleAuthFixUserfileRelatedDescriptorCorruptionV22_2V23_1StartTenantNamesRbrSqlliveness" -var _Key_index = [...]uint16{0, 17, 22, 31, 46, 86, 120, 154, 176, 196, 215, 248, 267, 287, 308, 343, 377, 407, 460, 474, 495, 526, 559, 590, 624, 646, 675, 702, 733, 766, 784, 808, 836, 855, 875, 921, 931, 950, 968, 989, 1027, 1032, 1042, 1053} +var _Key_index = [...]uint16{0, 17, 22, 31, 46, 86, 120, 154, 176, 196, 215, 248, 267, 287, 308, 343, 377, 407, 460, 474, 495, 526, 559, 590, 624, 646, 675, 702, 733, 766, 784, 808, 836, 855, 875, 921, 931, 950, 968, 989, 1027, 1032, 1042, 1053, 1067} func (i Key) String() string { i -= -1 diff --git a/pkg/sql/catalog/systemschema/system.go b/pkg/sql/catalog/systemschema/system.go index c400a3f70aed..bd3a1bf9825b 100644 --- a/pkg/sql/catalog/systemschema/system.go +++ b/pkg/sql/catalog/systemschema/system.go @@ -457,10 +457,11 @@ CREATE TABLE system.scheduled_jobs ( SqllivenessTableSchema = ` CREATE TABLE system.sqlliveness ( - session_id BYTES NOT NULL, - expiration DECIMAL NOT NULL, - CONSTRAINT "primary" PRIMARY KEY (session_id), - FAMILY fam0_session_id_expiration (session_id, expiration) + crdb_region BYTES NOT NULL, + session_uuid BYTES NOT NULL, + expiration DECIMAL NOT NULL, + CONSTRAINT "primary" PRIMARY KEY (crdb_region, session_uuid), + FAMILY "primary" (crdb_region, session_uuid, expiration) )` MigrationsTableSchema = ` @@ -2002,19 +2003,27 @@ var ( catconstants.SqllivenessTableName, keys.SqllivenessID, []descpb.ColumnDescriptor{ - {Name: "session_id", ID: 1, Type: types.Bytes, Nullable: false}, + {Name: "crdb_region", ID: 4, Type: types.Bytes, Nullable: false}, + {Name: "session_uuid", ID: 3, Type: types.Bytes, Nullable: false}, {Name: "expiration", ID: 2, Type: types.Decimal, Nullable: false}, }, []descpb.ColumnFamilyDescriptor{ { - Name: "fam0_session_id_expiration", + Name: "primary", ID: 0, - ColumnNames: []string{"session_id", "expiration"}, - ColumnIDs: []descpb.ColumnID{1, 2}, + ColumnNames: []string{"crdb_region", "session_uuid", "expiration"}, + ColumnIDs: []descpb.ColumnID{4, 3, 2}, DefaultColumnID: 2, }, }, - pk("session_id"), + descpb.IndexDescriptor{ + Name: "primary", + ID: 2, + Unique: true, + KeyColumnNames: []string{"crdb_region", "session_uuid"}, + KeyColumnDirections: []catpb.IndexColumn_Direction{catpb.IndexColumn_ASC, catpb.IndexColumn_ASC}, + KeyColumnIDs: []descpb.ColumnID{4, 3}, + }, )) // MigrationsTable is the descriptor for the migrations table. It stores facts diff --git a/pkg/sql/catalog/systemschema_test/testdata/bootstrap b/pkg/sql/catalog/systemschema_test/testdata/bootstrap index 50f9d4df7cf0..9b8df5f9ca23 100644 --- a/pkg/sql/catalog/systemschema_test/testdata/bootstrap +++ b/pkg/sql/catalog/systemschema_test/testdata/bootstrap @@ -281,10 +281,10 @@ CREATE TABLE public.scheduled_jobs ( FAMILY other (schedule_name, created, owner, schedule_expr, schedule_details, executor_type, execution_args) ); CREATE TABLE public.sqlliveness ( - session_id BYTES NOT NULL, + crdb_region BYTES NOT NULL, + session_uuid BYTES NOT NULL, expiration DECIMAL NOT NULL, - CONSTRAINT "primary" PRIMARY KEY (session_id ASC), - FAMILY fam0_session_id_expiration (session_id, expiration) + CONSTRAINT "primary" PRIMARY KEY (crdb_region ASC, session_uuid ASC) ); CREATE TABLE public.migrations ( major INT8 NOT NULL, @@ -422,7 +422,7 @@ schema_telemetry {"table":{"name":"settings","id":6,"version":"1","modificationTime":{"wallTime":"0"},"parentId":1,"unexposedParentSchemaId":29,"columns":[{"name":"name","id":1,"type":{"family":"StringFamily","oid":25}},{"name":"value","id":2,"type":{"family":"StringFamily","oid":25}},{"name":"lastUpdated","id":3,"type":{"family":"TimestampFamily","oid":1114},"defaultExpr":"now():::TIMESTAMP"},{"name":"valueType","id":4,"type":{"family":"StringFamily","oid":25},"nullable":true}],"nextColumnId":5,"families":[{"name":"fam_0_name_value_lastUpdated_valueType","columnNames":["name","value","lastUpdated","valueType"],"columnIds":[1,2,3,4]}],"nextFamilyId":1,"primaryIndex":{"name":"primary","id":1,"unique":true,"version":4,"keyColumnNames":["name"],"keyColumnDirections":["ASC"],"storeColumnNames":["value","lastUpdated","valueType"],"keyColumnIds":[1],"storeColumnIds":[2,3,4],"foreignKey":{},"interleave":{},"partitioning":{},"encodingType":1,"sharded":{},"geoConfig":{},"constraintId":1},"nextIndexId":2,"privileges":{"users":[{"userProto":"admin","privileges":480,"withGrantOption":480},{"userProto":"root","privileges":480,"withGrantOption":480}],"ownerProto":"node","version":2},"nextMutationId":1,"formatVersion":3,"replacementOf":{"time":{}},"createAsOfTime":{"wallTime":"0"},"nextConstraintId":2}} {"table":{"name":"span_configurations","id":47,"version":"1","modificationTime":{"wallTime":"0"},"parentId":1,"unexposedParentSchemaId":29,"columns":[{"name":"start_key","id":1,"type":{"family":"BytesFamily","oid":17}},{"name":"end_key","id":2,"type":{"family":"BytesFamily","oid":17}},{"name":"config","id":3,"type":{"family":"BytesFamily","oid":17}}],"nextColumnId":4,"families":[{"name":"primary","columnNames":["start_key","end_key","config"],"columnIds":[1,2,3]}],"nextFamilyId":1,"primaryIndex":{"name":"primary","id":1,"unique":true,"version":4,"keyColumnNames":["start_key"],"keyColumnDirections":["ASC"],"storeColumnNames":["end_key","config"],"keyColumnIds":[1],"storeColumnIds":[2,3],"foreignKey":{},"interleave":{},"partitioning":{},"encodingType":1,"sharded":{},"geoConfig":{},"constraintId":1},"nextIndexId":2,"privileges":{"users":[{"userProto":"admin","privileges":480,"withGrantOption":480},{"userProto":"root","privileges":480,"withGrantOption":480}],"ownerProto":"node","version":2},"nextMutationId":1,"formatVersion":3,"checks":[{"expr":"start_key \u003c end_key","name":"check_bounds","columnIds":[1,2],"constraintId":2}],"replacementOf":{"time":{}},"createAsOfTime":{"wallTime":"0"},"nextConstraintId":3}} {"table":{"name":"sql_instances","id":46,"version":"1","modificationTime":{"wallTime":"0"},"parentId":1,"unexposedParentSchemaId":29,"columns":[{"name":"id","id":1,"type":{"family":"IntFamily","width":64,"oid":20}},{"name":"addr","id":2,"type":{"family":"StringFamily","oid":25},"nullable":true},{"name":"session_id","id":3,"type":{"family":"BytesFamily","oid":17},"nullable":true},{"name":"locality","id":4,"type":{"family":"JsonFamily","oid":3802},"nullable":true}],"nextColumnId":5,"families":[{"name":"primary","columnNames":["id","addr","session_id","locality"],"columnIds":[1,2,3,4]}],"nextFamilyId":1,"primaryIndex":{"name":"primary","id":1,"unique":true,"version":4,"keyColumnNames":["id"],"keyColumnDirections":["ASC"],"storeColumnNames":["addr","session_id","locality"],"keyColumnIds":[1],"storeColumnIds":[2,3,4],"foreignKey":{},"interleave":{},"partitioning":{},"encodingType":1,"sharded":{},"geoConfig":{},"constraintId":1},"nextIndexId":2,"privileges":{"users":[{"userProto":"admin","privileges":480,"withGrantOption":480},{"userProto":"root","privileges":480,"withGrantOption":480}],"ownerProto":"node","version":2},"nextMutationId":1,"formatVersion":3,"replacementOf":{"time":{}},"createAsOfTime":{"wallTime":"0"},"nextConstraintId":2}} -{"table":{"name":"sqlliveness","id":39,"version":"1","modificationTime":{"wallTime":"0"},"parentId":1,"unexposedParentSchemaId":29,"columns":[{"name":"session_id","id":1,"type":{"family":"BytesFamily","oid":17}},{"name":"expiration","id":2,"type":{"family":"DecimalFamily","oid":1700}}],"nextColumnId":3,"families":[{"name":"fam0_session_id_expiration","columnNames":["session_id","expiration"],"columnIds":[1,2],"defaultColumnId":2}],"nextFamilyId":1,"primaryIndex":{"name":"primary","id":1,"unique":true,"version":4,"keyColumnNames":["session_id"],"keyColumnDirections":["ASC"],"storeColumnNames":["expiration"],"keyColumnIds":[1],"storeColumnIds":[2],"foreignKey":{},"interleave":{},"partitioning":{},"encodingType":1,"sharded":{},"geoConfig":{},"constraintId":1},"nextIndexId":2,"privileges":{"users":[{"userProto":"admin","privileges":480,"withGrantOption":480},{"userProto":"root","privileges":480,"withGrantOption":480}],"ownerProto":"node","version":2},"nextMutationId":1,"formatVersion":3,"replacementOf":{"time":{}},"createAsOfTime":{"wallTime":"0"},"nextConstraintId":2}} +{"table":{"name":"sqlliveness","id":39,"version":"1","modificationTime":{"wallTime":"0"},"parentId":1,"unexposedParentSchemaId":29,"columns":[{"name":"crdb_region","id":4,"type":{"family":"BytesFamily","oid":17}},{"name":"session_uuid","id":3,"type":{"family":"BytesFamily","oid":17}},{"name":"expiration","id":2,"type":{"family":"DecimalFamily","oid":1700}}],"nextColumnId":5,"families":[{"name":"primary","columnNames":["crdb_region","session_uuid","expiration"],"columnIds":[4,3,2],"defaultColumnId":2}],"nextFamilyId":1,"primaryIndex":{"name":"primary","id":2,"unique":true,"version":4,"keyColumnNames":["crdb_region","session_uuid"],"keyColumnDirections":["ASC","ASC"],"storeColumnNames":["expiration"],"keyColumnIds":[4,3],"storeColumnIds":[2],"foreignKey":{},"interleave":{},"partitioning":{},"encodingType":1,"sharded":{},"geoConfig":{},"constraintId":1},"nextIndexId":3,"privileges":{"users":[{"userProto":"admin","privileges":480,"withGrantOption":480},{"userProto":"root","privileges":480,"withGrantOption":480}],"ownerProto":"node","version":2},"nextMutationId":1,"formatVersion":3,"replacementOf":{"time":{}},"createAsOfTime":{"wallTime":"0"},"nextConstraintId":2}} {"table":{"name":"statement_bundle_chunks","id":34,"version":"1","modificationTime":{"wallTime":"0"},"parentId":1,"unexposedParentSchemaId":29,"columns":[{"name":"id","id":1,"type":{"family":"IntFamily","width":64,"oid":20},"defaultExpr":"unique_rowid()"},{"name":"description","id":2,"type":{"family":"StringFamily","oid":25},"nullable":true},{"name":"data","id":3,"type":{"family":"BytesFamily","oid":17}}],"nextColumnId":4,"families":[{"name":"primary","columnNames":["id","description","data"],"columnIds":[1,2,3]}],"nextFamilyId":1,"primaryIndex":{"name":"primary","id":1,"unique":true,"version":4,"keyColumnNames":["id"],"keyColumnDirections":["ASC"],"storeColumnNames":["description","data"],"keyColumnIds":[1],"storeColumnIds":[2,3],"foreignKey":{},"interleave":{},"partitioning":{},"encodingType":1,"sharded":{},"geoConfig":{},"constraintId":1},"nextIndexId":2,"privileges":{"users":[{"userProto":"admin","privileges":480,"withGrantOption":480},{"userProto":"root","privileges":480,"withGrantOption":480}],"ownerProto":"node","version":2},"nextMutationId":1,"formatVersion":3,"replacementOf":{"time":{}},"createAsOfTime":{"wallTime":"0"},"nextConstraintId":2}} {"table":{"name":"statement_diagnostics","id":36,"version":"1","modificationTime":{"wallTime":"0"},"parentId":1,"unexposedParentSchemaId":29,"columns":[{"name":"id","id":1,"type":{"family":"IntFamily","width":64,"oid":20},"defaultExpr":"unique_rowid()"},{"name":"statement_fingerprint","id":2,"type":{"family":"StringFamily","oid":25}},{"name":"statement","id":3,"type":{"family":"StringFamily","oid":25}},{"name":"collected_at","id":4,"type":{"family":"TimestampTZFamily","oid":1184}},{"name":"trace","id":5,"type":{"family":"JsonFamily","oid":3802},"nullable":true},{"name":"bundle_chunks","id":6,"type":{"family":"ArrayFamily","width":64,"arrayElemType":"IntFamily","oid":1016,"arrayContents":{"family":"IntFamily","width":64,"oid":20}},"nullable":true},{"name":"error","id":7,"type":{"family":"StringFamily","oid":25},"nullable":true}],"nextColumnId":8,"families":[{"name":"primary","columnNames":["id","statement_fingerprint","statement","collected_at","trace","bundle_chunks","error"],"columnIds":[1,2,3,4,5,6,7]}],"nextFamilyId":1,"primaryIndex":{"name":"primary","id":1,"unique":true,"version":4,"keyColumnNames":["id"],"keyColumnDirections":["ASC"],"storeColumnNames":["statement_fingerprint","statement","collected_at","trace","bundle_chunks","error"],"keyColumnIds":[1],"storeColumnIds":[2,3,4,5,6,7],"foreignKey":{},"interleave":{},"partitioning":{},"encodingType":1,"sharded":{},"geoConfig":{},"constraintId":1},"nextIndexId":2,"privileges":{"users":[{"userProto":"admin","privileges":480,"withGrantOption":480},{"userProto":"root","privileges":480,"withGrantOption":480}],"ownerProto":"node","version":2},"nextMutationId":1,"formatVersion":3,"replacementOf":{"time":{}},"createAsOfTime":{"wallTime":"0"},"nextConstraintId":2}} {"table":{"name":"statement_diagnostics_requests","id":35,"version":"1","modificationTime":{"wallTime":"0"},"parentId":1,"unexposedParentSchemaId":29,"columns":[{"name":"id","id":1,"type":{"family":"IntFamily","width":64,"oid":20},"defaultExpr":"unique_rowid()"},{"name":"completed","id":2,"type":{"oid":16},"defaultExpr":"false"},{"name":"statement_fingerprint","id":3,"type":{"family":"StringFamily","oid":25}},{"name":"statement_diagnostics_id","id":4,"type":{"family":"IntFamily","width":64,"oid":20},"nullable":true},{"name":"requested_at","id":5,"type":{"family":"TimestampTZFamily","oid":1184}},{"name":"min_execution_latency","id":6,"type":{"family":"IntervalFamily","oid":1186,"intervalDurationField":{}},"nullable":true},{"name":"expires_at","id":7,"type":{"family":"TimestampTZFamily","oid":1184},"nullable":true},{"name":"sampling_probability","id":8,"type":{"family":"FloatFamily","width":64,"oid":701},"nullable":true}],"nextColumnId":9,"families":[{"name":"primary","columnNames":["id","completed","statement_fingerprint","statement_diagnostics_id","requested_at","min_execution_latency","expires_at","sampling_probability"],"columnIds":[1,2,3,4,5,6,7,8]}],"nextFamilyId":1,"primaryIndex":{"name":"primary","id":1,"unique":true,"version":4,"keyColumnNames":["id"],"keyColumnDirections":["ASC"],"storeColumnNames":["completed","statement_fingerprint","statement_diagnostics_id","requested_at","min_execution_latency","expires_at","sampling_probability"],"keyColumnIds":[1],"storeColumnIds":[2,3,4,5,6,7,8],"foreignKey":{},"interleave":{},"partitioning":{},"encodingType":1,"sharded":{},"geoConfig":{},"constraintId":1},"indexes":[{"name":"completed_idx","id":2,"version":3,"keyColumnNames":["completed","id"],"keyColumnDirections":["ASC","ASC"],"storeColumnNames":["statement_fingerprint","min_execution_latency","expires_at","sampling_probability"],"keyColumnIds":[2,1],"storeColumnIds":[3,6,7,8],"foreignKey":{},"interleave":{},"partitioning":{},"sharded":{},"geoConfig":{}}],"nextIndexId":3,"privileges":{"users":[{"userProto":"admin","privileges":480,"withGrantOption":480},{"userProto":"root","privileges":480,"withGrantOption":480}],"ownerProto":"node","version":2},"nextMutationId":1,"formatVersion":3,"checks":[{"expr":"sampling_probability BETWEEN _:::FLOAT8 AND _:::FLOAT8","name":"check_sampling_probability","columnIds":[8],"constraintId":2}],"replacementOf":{"time":{}},"createAsOfTime":{"wallTime":"0"},"nextConstraintId":3}} diff --git a/pkg/sql/logictest/testdata/logic_test/information_schema b/pkg/sql/logictest/testdata/logic_test/information_schema index e42abb7d43ba..ef3ccb869d8b 100644 --- a/pkg/sql/logictest/testdata/logic_test/information_schema +++ b/pkg/sql/logictest/testdata/logic_test/information_schema @@ -1602,6 +1602,7 @@ system public 630200280_46_1_not_null system public primary system public sql_instances PRIMARY KEY NO NO system public 630200280_39_1_not_null system public sqlliveness CHECK NO NO system public 630200280_39_2_not_null system public sqlliveness CHECK NO NO +system public 630200280_39_3_not_null system public sqlliveness CHECK NO NO system public primary system public sqlliveness PRIMARY KEY NO NO system public 630200280_34_1_not_null system public statement_bundle_chunks CHECK NO NO system public 630200280_34_3_not_null system public statement_bundle_chunks CHECK NO NO @@ -1798,8 +1799,9 @@ system public 630200280_37_2_not_null system public 630200280_37_3_not_null created IS NOT NULL system public 630200280_37_4_not_null owner IS NOT NULL system public 630200280_37_9_not_null executor_type IS NOT NULL -system public 630200280_39_1_not_null session_id IS NOT NULL -system public 630200280_39_2_not_null expiration IS NOT NULL +system public 630200280_39_1_not_null crdb_region IS NOT NULL +system public 630200280_39_2_not_null session_uuid IS NOT NULL +system public 630200280_39_3_not_null expiration IS NOT NULL system public 630200280_3_1_not_null id IS NOT NULL system public 630200280_40_1_not_null major IS NOT NULL system public 630200280_40_2_not_null minor IS NOT NULL @@ -1927,7 +1929,8 @@ system public span_configurations end_key system public span_configurations start_key system public check_bounds system public span_configurations start_key system public primary system public sql_instances id system public primary -system public sqlliveness session_id system public primary +system public sqlliveness crdb_region system public primary +system public sqlliveness session_uuid system public primary system public statement_bundle_chunks id system public primary system public statement_diagnostics id system public primary system public statement_diagnostics_requests id system public primary @@ -2190,8 +2193,9 @@ system public sql_instances addr system public sql_instances id 1 system public sql_instances locality 4 system public sql_instances session_id 3 +system public sqlliveness crdb_region 4 system public sqlliveness expiration 2 -system public sqlliveness session_id 1 +system public sqlliveness session_uuid 3 system public statement_bundle_chunks data 3 system public statement_bundle_chunks description 2 system public statement_bundle_chunks id 1 diff --git a/pkg/sql/logictest/testdata/logic_test/pg_catalog b/pkg/sql/logictest/testdata/logic_test/pg_catalog index e33124e68028..ce5420aa4bbb 100644 --- a/pkg/sql/logictest/testdata/logic_test/pg_catalog +++ b/pkg/sql/logictest/testdata/logic_test/pg_catalog @@ -1108,7 +1108,7 @@ ORDER BY indexrelid indexrelid indrelid indnatts indisunique indisprimary indisexclusion indimmediate indisclustered indisvalid indcheckxmin indisready indislive indisreplident indkey indcollation indclass indoption indexprs indpred indnkeyatts 144368028 32 1 true true false true false true false false true false 1 0 0 2 NULL NULL 1 190763692 48 1 false true false false false true false false true false 1 0 0 2 NULL NULL 1 -404104299 39 1 true true false true false true false false true false 1 0 0 2 NULL NULL 1 +404104296 39 2 true true false true false true false false true false 4 3 0 0 0 0 2 2 NULL NULL 2 543291288 23 1 false false false false false true false false true false 1 3403232968 0 2 NULL NULL 1 543291289 23 1 false false false false false true false false true false 2 3403232968 0 2 NULL NULL 1 543291291 23 2 true true false true false true false false true false 1 2 3403232968 3403232968 0 0 2 2 NULL NULL 2 @@ -1175,7 +1175,8 @@ ORDER BY indexrelid, operator_argument_position indexrelid operator_argument_type_oid operator_argument_position 144368028 0 1 190763692 0 1 -404104299 0 1 +404104296 0 1 +404104296 0 2 543291288 0 1 543291289 0 1 543291291 0 1 diff --git a/pkg/sql/sqlliveness/slstorage/BUILD.bazel b/pkg/sql/sqlliveness/slstorage/BUILD.bazel index c8cea0b8aa99..b796721a50af 100644 --- a/pkg/sql/sqlliveness/slstorage/BUILD.bazel +++ b/pkg/sql/sqlliveness/slstorage/BUILD.bazel @@ -13,6 +13,7 @@ go_library( importpath = "github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slstorage", visibility = ["//visibility:public"], deps = [ + "//pkg/clusterversion", "//pkg/keys", "//pkg/kv", "//pkg/multitenant", @@ -53,6 +54,7 @@ go_test( deps = [ ":slstorage", "//pkg/base", + "//pkg/clusterversion", "//pkg/keys", "//pkg/kv", "//pkg/kv/kvserver", @@ -64,6 +66,7 @@ go_test( "//pkg/sql/catalog/descpb", "//pkg/sql/catalog/systemschema", "//pkg/sql/enum", + "//pkg/sql/sem/eval", "//pkg/sql/sqlliveness", "//pkg/testutils", "//pkg/testutils/serverutils", diff --git a/pkg/sql/sqlliveness/slstorage/sessionid_test.go b/pkg/sql/sqlliveness/slstorage/sessionid_test.go index 9b4669080e79..8a46cb008738 100644 --- a/pkg/sql/sqlliveness/slstorage/sessionid_test.go +++ b/pkg/sql/sqlliveness/slstorage/sessionid_test.go @@ -151,6 +151,7 @@ func TestSessionIDEncoding(t *testing.T) { require.Error(t, err) require.Contains(t, err.Error(), tc.err) } else { + require.NoError(t, err) require.Equal(t, region, tc.region) require.Equal(t, uuid, tc.id.GetBytes()) } diff --git a/pkg/sql/sqlliveness/slstorage/slstorage.go b/pkg/sql/sqlliveness/slstorage/slstorage.go index 7ae54f982204..968e7067b0c2 100644 --- a/pkg/sql/sqlliveness/slstorage/slstorage.go +++ b/pkg/sql/sqlliveness/slstorage/slstorage.go @@ -131,7 +131,7 @@ func NewTestingStorage( return time.Duration(frac * float64(baseInterval.Nanoseconds())) }, metrics: makeMetrics(), - table: MakeTable(codec, sqllivenessTableID), + table: MakeTable(settings, codec, sqllivenessTableID), } cacheConfig := cache.Config{ Policy: cache.CacheLRU, diff --git a/pkg/sql/sqlliveness/slstorage/slstorage_test.go b/pkg/sql/sqlliveness/slstorage/slstorage_test.go index 53015ddc4cf0..08190cc1b30f 100644 --- a/pkg/sql/sqlliveness/slstorage/slstorage_test.go +++ b/pkg/sql/sqlliveness/slstorage/slstorage_test.go @@ -28,6 +28,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/systemschema" + "github.com/cockroachdb/cockroach/pkg/sql/enum" "github.com/cockroachdb/cockroach/pkg/sql/sqlliveness" "github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slstorage" "github.com/cockroachdb/cockroach/pkg/testutils" @@ -79,7 +80,8 @@ func TestStorage(t *testing.T) { defer stopper.Stop(ctx) exp := clock.Now().Add(time.Second.Nanoseconds(), 0) - const id = "asdf" + id, err := slstorage.MakeSessionID(enum.One, uuid.MakeV4()) + require.NoError(t, err) metrics := storage.Metrics() { @@ -123,8 +125,10 @@ func TestStorage(t *testing.T) { // Create two records which will expire before nextGC. exp := clock.Now().Add(gcInterval.Nanoseconds()-1, 0) - const id1 = "asdf" - const id2 = "ghjk" + id1, err := slstorage.MakeSessionID(enum.One, uuid.MakeV4()) + require.NoError(t, err) + id2, err := slstorage.MakeSessionID(enum.One, uuid.MakeV4()) + require.NoError(t, err) { require.NoError(t, storage.Insert(ctx, id1, exp)) require.NoError(t, storage.Insert(ctx, id2, exp)) @@ -224,7 +228,8 @@ func TestStorage(t *testing.T) { storage.Start(ctx) exp := clock.Now().Add(time.Second.Nanoseconds(), 0) - const id = "asdf" + id, err := slstorage.MakeSessionID(enum.One, uuid.MakeV4()) + require.NoError(t, err) metrics := storage.Metrics() { @@ -346,8 +351,11 @@ func TestConcurrentAccessesAndEvictions(t *testing.T) { t.Helper() state.Lock() defer state.Unlock() + + sid, err := slstorage.MakeSessionID(enum.One, uuid.MakeV4()) + require.NoError(t, err) s := session{ - id: sqlliveness.SessionID(uuid.MakeV4().String()), + id: sqlliveness.SessionID(sid), expiration: clock.Now().Add(expiration.Nanoseconds(), 0), } require.NoError(t, storage.Insert(ctx, s.id, s.expiration)) @@ -518,7 +526,8 @@ func TestConcurrentAccessSynchronization(t *testing.T) { cached := storage.CachedReader() var alive bool var g errgroup.Group - sid := sqlliveness.SessionID(t.Name()) + sid, err := slstorage.MakeSessionID(enum.One, uuid.MakeV4()) + require.NoError(t, err) g.Go(func() (err error) { alive, err = cached.IsAlive(ctx, sid) return err @@ -547,7 +556,8 @@ func TestConcurrentAccessSynchronization(t *testing.T) { cached := storage.CachedReader() var alive bool var g errgroup.Group - sid := sqlliveness.SessionID(t.Name()) + sid, err := slstorage.MakeSessionID(enum.One, uuid.MakeV4()) + require.NoError(t, err) toCancel, cancel := context.WithCancel(ctx) before := storage.Metrics().IsAliveCacheMisses.Count() @@ -595,7 +605,8 @@ func TestConcurrentAccessSynchronization(t *testing.T) { cached := storage.CachedReader() var alive bool var g errgroup.Group - sid := sqlliveness.SessionID(t.Name()) + sid, err := slstorage.MakeSessionID(enum.One, uuid.MakeV4()) + require.NoError(t, err) g.Go(func() (err error) { alive, err = cached.IsAlive(ctx, sid) return err @@ -671,8 +682,8 @@ func TestDeleteMidUpdateFails(t *testing.T) { ) // Insert a session. - ID := sqlliveness.SessionID("foo") - require.NoError(t, storage.Insert(ctx, ID, s.Clock().Now())) + ID, err := slstorage.MakeSessionID(enum.One, uuid.MakeV4()) + require.NoError(t, err) // Install a filter which will send on this channel when we attempt // to perform an update after the get has evaluated. diff --git a/pkg/sql/sqlliveness/slstorage/table.go b/pkg/sql/sqlliveness/slstorage/table.go index 1ec0540700e6..d631d986cf94 100644 --- a/pkg/sql/sqlliveness/slstorage/table.go +++ b/pkg/sql/sqlliveness/slstorage/table.go @@ -13,9 +13,11 @@ package slstorage import ( "context" + "github.com/cockroachdb/cockroach/pkg/clusterversion" "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql/sem/catid" "github.com/cockroachdb/cockroach/pkg/sql/sem/eval" "github.com/cockroachdb/cockroach/pkg/sql/sqlliveness" @@ -25,15 +27,50 @@ import ( "github.com/cockroachdb/redact" ) -// Table is an interface to system.sqlliveness table. +// Table is an interface to system.sqlliveness table. The api was created to +// encapsulate the migration to a RBR sqlliveness table. +// +// # Migrating to Rbr +// When the binary is upgraded, it creates a session that encodes the region in +// the session id. sessions with an encoded region are dual written to the RBT +// and RBR index. +// +// When an upgraded binary encounters a new session id, it consults the RBR +// index to see if it is valid. If a legacy binary encounters an RBR session, +// it treats the id as a black box and consults the RBT index. +// +// Before the RbrSqlliveness version gate is flipped the sqlliveness +// descriptor is upgraded to the new format. This is safe because the version +// can only advance after all servers are running the new binary and that +// implies all legacy sessions are inactive. +// +// After the version gate is flipped, servers stop dual writing. The legacy RBT +// index is eventually cleaned up by deleteExpiredSessions. type Table struct { rbtIndex roachpb.Key + rbrIndex roachpb.Key + settings *cluster.Settings } // MakeTable constructs a typed interface to the system.sqlliveness table. -func MakeTable(codec keys.SQLCodec, tableID catid.DescID) Table { +func MakeTable(settings *cluster.Settings, codec keys.SQLCodec, tableID catid.DescID) Table { return Table{ rbtIndex: codec.IndexPrefix(uint32(tableID), 1), + rbrIndex: codec.IndexPrefix(uint32(tableID), 2), + settings: settings, + } +} + +// MakeTestTable constructs a table with specific index ids. This is needed to +// test the interface with a RBR table created via sql, since the index id will +// be wrong. +func MakeTestTable( + settings *cluster.Settings, codec keys.SQLCodec, tableID catid.DescID, rbrIndex, rbtIndex uint32, +) Table { + return Table{ + rbtIndex: codec.IndexPrefix(uint32(tableID), rbtIndex), + rbrIndex: codec.IndexPrefix(uint32(tableID), rbrIndex), + settings: settings, } } @@ -42,12 +79,21 @@ func MakeTable(codec keys.SQLCodec, tableID catid.DescID) Table { func (t *Table) GetExpiration( ctx context.Context, txn *kv.Txn, sid sqlliveness.SessionID, ) (exists bool, expiration hlc.Timestamp, err error) { - k := t.makeSessionKey(sid) - kv, err := txn.Get(ctx, k) + region, uuid, err := UnsafeDecodeSessionID(sid) + if err != nil { + return false, hlc.Timestamp{}, err + } + + var key roachpb.Key + if 0 < len(region) { + key = t.makeRbrSessionKey(region, uuid) + } else { + key = t.makeRbtSessionKey(sid) + } + kv, err := txn.Get(ctx, key) if err != nil { return false, hlc.Timestamp{}, err } - // The session is not alive. if kv.Value == nil { return false, hlc.Timestamp{}, err } @@ -55,27 +101,59 @@ func (t *Table) GetExpiration( if err != nil { return false, hlc.Timestamp{}, errors.Wrapf(err, "failed to decode expiration for %s", redact.SafeString(sid.String())) } - return true, expiration, nil } // SetExpiration upserts the session with the given expiration. func (t *Table) SetExpiration( - ctx context.Context, txn *kv.Txn, id sqlliveness.SessionID, expiration hlc.Timestamp, + ctx context.Context, txn *kv.Txn, sid sqlliveness.SessionID, expiration hlc.Timestamp, ) error { - k := t.makeSessionKey(id) - v := encodeValue(expiration) - return txn.Put(ctx, k, &v) + region, uuid, err := UnsafeDecodeSessionID(sid) + if err != nil { + return err + } + if len(region) == 0 && t.settings.Version.IsActive(ctx, clusterversion.RbrSqlliveness) { + // SetExpiration is only used to create and heartbeat a node's own + // session. It should never be called with a session that does not + // include a region. + return errors.Newf("attempted to set the expiration of a legacy session: '%s'", sid.String()) + } + + batch := txn.NewBatch() + if 0 < len(region) { + batch.Put(t.makeRbrSessionKey(region, uuid), encodeValue(expiration)) + } + if !t.settings.Version.IsActive(ctx, clusterversion.RbrSqlliveness) { + batch.Put(t.makeRbtSessionKey(sid), encodeValue(expiration)) + } + return txn.Run(ctx, batch) } // Delete deletes the session. -func (t *Table) Delete(ctx context.Context, txn *kv.Txn, id sqlliveness.SessionID) error { - key := t.makeSessionKey(id) - _, err := txn.Del(ctx, key) - return err +func (t *Table) Delete(ctx context.Context, txn *kv.Txn, sid sqlliveness.SessionID) error { + region, uuid, err := UnsafeDecodeSessionID(sid) + if err != nil { + return err + } + batch := txn.NewBatch() + if 0 < len(region) { + batch.Del(t.makeRbrSessionKey(region, uuid)) + } + if len(region) == 0 || !t.settings.Version.IsActive(ctx, clusterversion.RbrSqlliveness) { + batch.Del(t.makeRbtSessionKey(sid)) + } + return txn.Run(ctx, batch) +} + +func (t *Table) makeRbrSessionKey(region []byte, uuid []byte) roachpb.Key { + key := t.rbrIndex.Clone() + key = encoding.EncodeBytesAscending(key, region) + key = encoding.EncodeBytesAscending(key, uuid) + key = keys.MakeFamilyKey(key, 0) + return key } -func (t *Table) makeSessionKey(id sqlliveness.SessionID) roachpb.Key { +func (t *Table) makeRbtSessionKey(id sqlliveness.SessionID) roachpb.Key { return keys.MakeFamilyKey(encoding.EncodeBytesAscending(t.rbtIndex.Clone(), id.UnsafeBytes()), 0) } @@ -93,9 +171,9 @@ func decodeValue(kv kv.KeyValue) (hlc.Timestamp, error) { return hlc.DecimalToHLC(&dec) } -func encodeValue(expiration hlc.Timestamp) roachpb.Value { +func encodeValue(expiration hlc.Timestamp) *roachpb.Value { var v roachpb.Value dec := eval.TimestampToDecimal(expiration) v.SetTuple(encoding.EncodeDecimalValue(nil, 2, &dec)) - return v + return &v } diff --git a/pkg/sql/sqlliveness/slstorage/table_test.go b/pkg/sql/sqlliveness/slstorage/table_test.go index 1b2f92012d97..3178e6ab7076 100644 --- a/pkg/sql/sqlliveness/slstorage/table_test.go +++ b/pkg/sql/sqlliveness/slstorage/table_test.go @@ -18,10 +18,13 @@ import ( "github.com/cockroachdb/apd/v3" "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/clusterversion" "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/kv" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql/catalog/systemschema" "github.com/cockroachdb/cockroach/pkg/sql/enum" + "github.com/cockroachdb/cockroach/pkg/sql/sem/eval" "github.com/cockroachdb/cockroach/pkg/sql/sqlliveness" "github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slstorage" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" @@ -34,15 +37,30 @@ import ( "github.com/stretchr/testify/require" ) -// test random read/writes to the table while stepping through the migration state -// ensure the table can be converted to regional by row in the final migration state +const rbtSqllivenessTable = ` +CREATE TABLE system.sqlliveness ( + session_id BYTES NOT NULL, + expiration DECIMAL NOT NULL, + CONSTRAINT "primary" PRIMARY KEY (session_id), + FAMILY "primary" (session_id) +); +` func TestSqlLivenessTable(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) ctx := context.Background() - s, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{}) + settings := cluster.MakeTestingClusterSettingsWithVersions( + clusterversion.TestingBinaryVersion, + clusterversion.TestingBinaryMinSupportedVersion, + false, + ) + require.NoError(t, clusterversion.Initialize( + context.Background(), clusterversion.TestingBinaryMinSupportedVersion, &settings.SV, + )) + + s, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{Settings: settings}) defer s.Stopper().Stop(ctx) tDB := sqlutils.MakeSQLRunner(sqlDB) @@ -51,14 +69,24 @@ func TestSqlLivenessTable(t *testing.T) { timeSource := timeutil.NewManualTime(t0) clock := hlc.NewClock(timeSource, base.DefaultMaxClockOffset) - setup := func(t *testing.T) slstorage.Table { + setup := func(t *testing.T, schema string) slstorage.Table { dbName := t.Name() - tableID := newSystemTable(t, tDB, dbName, "sqlliveness", systemschema.SqllivenessTableSchema) - return slstorage.MakeTable(keys.SystemSQLCodec, tableID) + tableID := newSystemTable(t, tDB, dbName, "sqlliveness", schema) + + settings := cluster.MakeTestingClusterSettingsWithVersions( + clusterversion.TestingBinaryVersion, + clusterversion.TestingBinaryMinSupportedVersion, + false, + ) + require.NoError(t, clusterversion.Initialize( + context.Background(), clusterversion.TestingBinaryMinSupportedVersion, &settings.SV, + )) + + return slstorage.MakeTable(settings, keys.SystemSQLCodec, tableID) } t.Run("NotFound", func(t *testing.T) { - table := setup(t) + table := setup(t, systemschema.SqllivenessTableSchema) session, err := slstorage.MakeSessionID(enum.One, uuid.MakeV4()) require.NoError(t, err) require.NoError(t, kvDB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error { @@ -72,7 +100,7 @@ func TestSqlLivenessTable(t *testing.T) { }) t.Run("CreateAndUpdate", func(t *testing.T) { - table := setup(t) + table := setup(t, systemschema.SqllivenessTableSchema) session, err := slstorage.MakeSessionID(enum.One, uuid.MakeV4()) require.NoError(t, err) @@ -97,7 +125,7 @@ func TestSqlLivenessTable(t *testing.T) { }) t.Run("DeleteSession", func(t *testing.T) { - table := setup(t) + table := setup(t, systemschema.SqllivenessTableSchema) session, err := slstorage.MakeSessionID(enum.One, uuid.MakeV4()) require.NoError(t, err) @@ -118,8 +146,38 @@ func TestSqlLivenessTable(t *testing.T) { })) }) + t.Run("LegacySession", func(t *testing.T) { + table := setup(t, rbtSqllivenessTable) + legacySession := sqlliveness.SessionID(uuid.MakeV4().GetBytes()) + writeExpiration := clock.Now().Add(10, 00) + + tDB.Exec(t, + fmt.Sprintf(`INSERT INTO "%s".sqlliveness (session_id, expiration) VALUES ($1, $2)`, t.Name()), + legacySession, + eval.TimestampToDecimal(writeExpiration)) + + require.NoError(t, kvDB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error { + exists, expiration, err := table.GetExpiration(ctx, txn, legacySession) + if err != nil { + return err + } + require.True(t, exists) + require.Equal(t, expiration, writeExpiration) + return nil + })) + + require.NoError(t, kvDB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error { + return table.Delete(ctx, txn, legacySession) + })) + + row := tDB.QueryRow(t, fmt.Sprintf(`SELECT count(*) FROM "%s".sqlliveness`, t.Name())) + var count int + row.Scan(&count) + require.Equal(t, count, 0) + }) + t.Run("RbtSql", func(t *testing.T) { - table := setup(t) + table := setup(t, rbtSqllivenessTable) session, err := slstorage.MakeSessionID(enum.One, uuid.MakeV4()) require.NoError(t, err) diff --git a/pkg/upgrade/upgrades/BUILD.bazel b/pkg/upgrade/upgrades/BUILD.bazel index daef003781ca..b8b69769a88b 100644 --- a/pkg/upgrade/upgrades/BUILD.bazel +++ b/pkg/upgrade/upgrades/BUILD.bazel @@ -10,6 +10,7 @@ go_library( "ensure_sql_schema_telemetry_schedule.go", "fix_userfile_descriptor_corruption.go", "precondition_before_starting_an_upgrade.go", + "rbr_sqlliveness.go", "remove_grant_migration.go", "role_id_sequence_migration.go", "role_options_table_migration.go", diff --git a/pkg/upgrade/upgrades/rbr_sqlliveness.go b/pkg/upgrade/upgrades/rbr_sqlliveness.go new file mode 100644 index 000000000000..8635a34791b5 --- /dev/null +++ b/pkg/upgrade/upgrades/rbr_sqlliveness.go @@ -0,0 +1,27 @@ +// 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 upgrades + +// useRbrSqllivenessDescriptor is a precondition for the +// CleanupRbtSqllivenessIndex version gate. +func useRbrSqllivenessDescriptor() error { + // DO NOT SUBMIT(jeffswenson): implement the migration + + // Step 1: Wait for all non-regional sessions to expire. The non-regional + // sessions have no representation in the Rbr index. + + // Step 2: Replace the RBT sqlliveness descriptor with the RBR sqlliveness + // descriptor. At this point the RBT descriptor is valid for reads. + // slstorage.Table dual writes regional sessions to the RBT and RBR + // indexes. + + return nil +}