diff --git a/pkg/ccl/backupccl/restore_job.go b/pkg/ccl/backupccl/restore_job.go index 9bdef7d2384b..88c9a2c6b942 100644 --- a/pkg/ccl/backupccl/restore_job.go +++ b/pkg/ccl/backupccl/restore_job.go @@ -2239,8 +2239,7 @@ func (r *restoreResumer) publishDescriptors( jobsKnobs, jobs.ScheduledJobTxn(txn), user, - mutTable.GetID(), - mutTable.GetRowLevelTTL(), + mutTable, ) if err != nil { return err diff --git a/pkg/ccl/backupccl/testdata/backup-restore/restore-on-fail-or-cancel-ttl b/pkg/ccl/backupccl/testdata/backup-restore/restore-on-fail-or-cancel-ttl index 89b29fdb1d3a..0c1e9b897564 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/restore-on-fail-or-cancel-ttl +++ b/pkg/ccl/backupccl/testdata/backup-restore/restore-on-fail-or-cancel-ttl @@ -34,7 +34,7 @@ job cancel=a ---- query-sql -SELECT count(1) FROM [SHOW SCHEDULES] WHERE label LIKE 'row-level-ttl-%'; +SELECT count(1) FROM [SHOW SCHEDULES] WHERE label LIKE 'row-level-ttl%'; ---- 0 @@ -75,7 +75,7 @@ job cancel=b ---- query-sql -SELECT count(1) FROM [SHOW SCHEDULES] WHERE label LIKE 'row-level-ttl-%'; +SELECT count(1) FROM [SHOW SCHEDULES] WHERE label LIKE 'row-level-ttl%'; ---- 0 diff --git a/pkg/ccl/backupccl/testdata/backup-restore/row_level_ttl b/pkg/ccl/backupccl/testdata/backup-restore/row_level_ttl index ed3389646ecc..8409720584fb 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/row_level_ttl +++ b/pkg/ccl/backupccl/testdata/backup-restore/row_level_ttl @@ -20,7 +20,7 @@ new-cluster name=s2 share-io-dir=s1 allow-implicit-access query-sql SELECT count(1) FROM [SHOW SCHEDULES] -WHERE label LIKE 'row-level-ttl-%' +WHERE label LIKE 'row-level-ttl%' ---- 0 @@ -43,7 +43,7 @@ CREATE TABLE public.t ( query-sql SELECT count(1) FROM [SHOW SCHEDULES] -WHERE label LIKE 'row-level-ttl-%' +WHERE label LIKE 'row-level-ttl%' ---- 1 @@ -53,7 +53,7 @@ DROP DATABASE d query-sql SELECT count(1) FROM [SHOW SCHEDULES] -WHERE label LIKE 'row-level-ttl-%' +WHERE label LIKE 'row-level-ttl%' ---- 0 @@ -76,7 +76,7 @@ CREATE TABLE public.t ( query-sql SELECT count(1) FROM [SHOW SCHEDULES] -WHERE label LIKE 'row-level-ttl-%' +WHERE label LIKE 'row-level-ttl%' ---- 1 @@ -86,7 +86,7 @@ DROP DATABASE d query-sql SELECT count(1) FROM [SHOW SCHEDULES] -WHERE label LIKE 'row-level-ttl-%' +WHERE label LIKE 'row-level-ttl%' ---- 0 @@ -113,7 +113,7 @@ CREATE TABLE public.t ( query-sql SELECT count(1) FROM [SHOW SCHEDULES] -WHERE label LIKE 'row-level-ttl-%' +WHERE label LIKE 'row-level-ttl%' ---- 1 @@ -123,6 +123,6 @@ DROP TABLE d.public.t query-sql SELECT count(1) FROM [SHOW SCHEDULES] -WHERE label LIKE 'row-level-ttl-%' +WHERE label LIKE 'row-level-ttl%' ---- 0 diff --git a/pkg/ccl/sqlproxyccl/tenantdirsvr/test_directory_svr.go b/pkg/ccl/sqlproxyccl/tenantdirsvr/test_directory_svr.go index 980c2d40418c..d64f2a1349cb 100644 --- a/pkg/ccl/sqlproxyccl/tenantdirsvr/test_directory_svr.go +++ b/pkg/ccl/sqlproxyccl/tenantdirsvr/test_directory_svr.go @@ -29,26 +29,12 @@ import ( var _ tenant.DirectoryServer = (*TestDirectoryServer)(nil) // NewSubStopper creates a new stopper that will be stopped when either the -// parent is stopped or its own Stop is called. The code is slightly more -// complicated that simply calling NewStopper followed by AddCloser since there -// is a possibility that between the two calls, the parent stopper completes a -// stop and then the leak detection may find a leaked stopper. +// parent is stopped or its own Stop is called. func NewSubStopper(parentStopper *stop.Stopper) *stop.Stopper { - var mu syncutil.Mutex - var subStopper *stop.Stopper + subStopper := stop.NewStopper(stop.WithTracer(parentStopper.Tracer())) parentStopper.AddCloser(stop.CloserFn(func() { - mu.Lock() - defer mu.Unlock() - if subStopper == nil { - subStopper = stop.NewStopper(stop.WithTracer(parentStopper.Tracer())) - } subStopper.Stop(context.Background()) })) - mu.Lock() - defer mu.Unlock() - if subStopper == nil { - subStopper = stop.NewStopper(stop.WithTracer(parentStopper.Tracer())) - } return subStopper } diff --git a/pkg/sql/BUILD.bazel b/pkg/sql/BUILD.bazel index 97b8876432e4..d6ad4bdfbb30 100644 --- a/pkg/sql/BUILD.bazel +++ b/pkg/sql/BUILD.bazel @@ -508,6 +508,7 @@ go_library( "//pkg/sql/storageparam/tablestorageparam", "//pkg/sql/syntheticprivilege", "//pkg/sql/syntheticprivilegecache", + "//pkg/sql/ttl/ttlbase", "//pkg/sql/types", "//pkg/sql/vtable", "//pkg/storage", diff --git a/pkg/sql/check.go b/pkg/sql/check.go index c76c1bd4ea37..a4c61bcfda30 100644 --- a/pkg/sql/check.go +++ b/pkg/sql/check.go @@ -804,8 +804,7 @@ func (p *planner) RepairTTLScheduledJobForTable(ctx context.Context, tableID int p.ExecCfg().JobsKnobs(), jobs.ScheduledJobTxn(p.InternalSQLTxn()), p.User(), - tableDesc.GetID(), - tableDesc.GetRowLevelTTL(), + tableDesc, ) if err != nil { return err diff --git a/pkg/sql/create_table.go b/pkg/sql/create_table.go index 80c929e1798a..7b3ecf9bad2a 100644 --- a/pkg/sql/create_table.go +++ b/pkg/sql/create_table.go @@ -59,6 +59,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/storageparam" "github.com/cockroachdb/cockroach/pkg/sql/storageparam/indexstorageparam" "github.com/cockroachdb/cockroach/pkg/sql/storageparam/tablestorageparam" + "github.com/cockroachdb/cockroach/pkg/sql/ttl/ttlbase" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" "github.com/cockroachdb/cockroach/pkg/util/hlc" @@ -2416,8 +2417,7 @@ func newTableDesc( params.ExecCfg().JobsKnobs(), jobs.ScheduledJobTxn(params.p.InternalSQLTxn()), params.p.User(), - ret.GetID(), - ttl, + ret, ) if err != nil { return nil, err @@ -2428,15 +2428,13 @@ func newTableDesc( } // newRowLevelTTLScheduledJob returns a *jobs.ScheduledJob for row level TTL -// for a given table. +// for a given table. newRowLevelTTLScheduledJob assumes that +// tblDesc.RowLevelTTL is not nil. func newRowLevelTTLScheduledJob( - env scheduledjobs.JobSchedulerEnv, - owner username.SQLUsername, - tblID descpb.ID, - ttl *catpb.RowLevelTTL, + env scheduledjobs.JobSchedulerEnv, owner username.SQLUsername, tblDesc *tabledesc.Mutable, ) (*jobs.ScheduledJob, error) { sj := jobs.NewScheduledJob(env) - sj.SetScheduleLabel(fmt.Sprintf("row-level-ttl-%d", tblID)) + sj.SetScheduleLabel(ttlbase.BuildScheduleLabel(tblDesc)) sj.SetOwner(owner) sj.SetScheduleDetails(jobspb.ScheduleDetails{ Wait: jobspb.ScheduleDetails_WAIT, @@ -2444,11 +2442,11 @@ func newRowLevelTTLScheduledJob( OnError: jobspb.ScheduleDetails_RETRY_SCHED, }) - if err := sj.SetSchedule(ttl.DeletionCronOrDefault()); err != nil { + if err := sj.SetSchedule(tblDesc.RowLevelTTL.DeletionCronOrDefault()); err != nil { return nil, err } args := &catpb.ScheduledRowLevelTTLArgs{ - TableID: tblID, + TableID: tblDesc.GetID(), } any, err := pbtypes.MarshalAny(args) if err != nil { @@ -2467,12 +2465,15 @@ func CreateRowLevelTTLScheduledJob( knobs *jobs.TestingKnobs, s jobs.ScheduledJobStorage, owner username.SQLUsername, - tblID descpb.ID, - ttl *catpb.RowLevelTTL, + tblDesc *tabledesc.Mutable, ) (*jobs.ScheduledJob, error) { + if !tblDesc.HasRowLevelTTL() { + return nil, errors.AssertionFailedf("CreateRowLevelTTLScheduledJob called with no .RowLevelTTL: %#v", tblDesc) + } + telemetry.Inc(sqltelemetry.RowLevelTTLCreated) env := JobSchedulerEnv(knobs) - j, err := newRowLevelTTLScheduledJob(env, owner, tblID, ttl) + j, err := newRowLevelTTLScheduledJob(env, owner, tblDesc) if err != nil { return nil, err } diff --git a/pkg/sql/descmetadata/BUILD.bazel b/pkg/sql/descmetadata/BUILD.bazel index 09368a5f89b0..58cacf05393c 100644 --- a/pkg/sql/descmetadata/BUILD.bazel +++ b/pkg/sql/descmetadata/BUILD.bazel @@ -10,10 +10,12 @@ go_library( "//pkg/settings", "//pkg/sql/catalog/descpb", "//pkg/sql/catalog/descs", + "//pkg/sql/catalog/tabledesc", "//pkg/sql/isql", "//pkg/sql/schemachanger/scexec", "//pkg/sql/sessiondata", "//pkg/sql/sessioninit", + "//pkg/sql/ttl/ttlbase", ], ) diff --git a/pkg/sql/descmetadata/metadata_updater.go b/pkg/sql/descmetadata/metadata_updater.go index f9767e8a25b4..f764571180ac 100644 --- a/pkg/sql/descmetadata/metadata_updater.go +++ b/pkg/sql/descmetadata/metadata_updater.go @@ -18,10 +18,12 @@ import ( "github.com/cockroachdb/cockroach/pkg/settings" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" "github.com/cockroachdb/cockroach/pkg/sql/isql" "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scexec" "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" "github.com/cockroachdb/cockroach/pkg/sql/sessioninit" + "github.com/cockroachdb/cockroach/pkg/sql/ttl/ttlbase" ) // metadataUpdater which implements scexec.MetaDataUpdater that is used to update @@ -94,3 +96,23 @@ func (mu metadataUpdater) DeleteSchedule(ctx context.Context, scheduleID int64) ) return err } + +// UpdateTTLScheduleLabel implement scexec.DescriptorMetadataUpdater. +func (mu metadataUpdater) UpdateTTLScheduleLabel( + ctx context.Context, tbl *tabledesc.Mutable, +) error { + if !tbl.HasRowLevelTTL() { + return nil + } + + _, err := mu.txn.ExecEx( + ctx, + "update-ttl-schedule-label", + mu.txn.KV(), + sessiondata.RootUserSessionDataOverride, + "UPDATE system.scheduled_jobs SET schedule_name = $1 WHERE schedule_id = $2", + ttlbase.BuildScheduleLabel(tbl), + tbl.RowLevelTTL.ScheduleID, + ) + return err +} diff --git a/pkg/sql/logictest/testdata/logic_test/row_level_ttl b/pkg/sql/logictest/testdata/logic_test/row_level_ttl index a51071436acd..8c390e72ef37 100644 --- a/pkg/sql/logictest/testdata/logic_test/row_level_ttl +++ b/pkg/sql/logictest/testdata/logic_test/row_level_ttl @@ -233,17 +233,17 @@ CREATE TABLE tbl_schedules ( id INT PRIMARY KEY ) WITH (ttl_expire_after = '10 minutes') -let $table_id -SELECT oid FROM pg_class WHERE relname = 'tbl_schedules' +let $label_suffix +SELECT relname || ' [' || oid || ']' FROM pg_class WHERE relname = 'tbl_schedules' query I SELECT count(1) FROM [SHOW SCHEDULES] -WHERE label = 'row-level-ttl-$table_id' +WHERE label = 'row-level-ttl: $label_suffix' ---- 1 let $schedule_id -SELECT id FROM [SHOW SCHEDULES] WHERE label = 'row-level-ttl-$table_id' +SELECT id FROM [SHOW SCHEDULES] WHERE label = 'row-level-ttl: $label_suffix' statement error cannot drop a row level TTL schedule\nHINT: use ALTER TABLE test\.public\.tbl_schedules RESET \(ttl\) instead DROP SCHEDULE $schedule_id @@ -321,12 +321,12 @@ CREATE TABLE tbl_reset_ttl ( id INT PRIMARY KEY ) WITH (ttl_expire_after = '10 minutes') -let $table_id -SELECT oid FROM pg_class WHERE relname = 'tbl_reset_ttl' +let $label_suffix +SELECT relname || ' [' || oid || ']' FROM pg_class WHERE relname = 'tbl_reset_ttl' query I SELECT count(1) FROM [SHOW SCHEDULES] -WHERE label = 'row-level-ttl-$table_id' +WHERE label = 'row-level-ttl: $label_suffix' ---- 1 @@ -351,12 +351,92 @@ SELECT crdb_internal.validate_ttl_scheduled_jobs() query I SELECT count(1) FROM [SHOW SCHEDULES] -WHERE label = 'row-level-ttl-$table_id' +WHERE label = 'row-level-ttl: $label_suffix' ---- 0 subtest end +# Ensure that SHOW SCHEDULES's label column is updated to reflect the new table +# name. +subtest rename_table + +statement ok +CREATE TABLE tbl_rename ( + id INT PRIMARY KEY +) WITH (ttl_expire_after = '10 minutes') + +let $label_suffix +SELECT relname || ' [' || oid || ']' FROM pg_class WHERE relname = 'tbl_rename' + +query I +SELECT count(1) FROM [SHOW SCHEDULES] +WHERE label = 'row-level-ttl: $label_suffix' +---- +1 + +statement ok +ALTER TABLE tbl_rename RENAME TO tbl_renamed + +query I +SELECT count(1) FROM [SHOW SCHEDULES] +WHERE label = 'row-level-ttl: $label_suffix' +---- +0 + +let $label_suffix +SELECT relname || ' [' || oid || ']' FROM pg_class WHERE relname = 'tbl_renamed' + +query I +SELECT count(1) FROM [SHOW SCHEDULES] +WHERE label = 'row-level-ttl: $label_suffix' +---- +1 + +subtest end + +subtest rename_table_legacy_label + +statement ok +CREATE TABLE tbl_rename_legacy_label ( + id INT PRIMARY KEY +) WITH (ttl_expire_after = '10 minutes') + +let $label_suffix +SELECT relname || ' [' || oid || ']' FROM pg_class WHERE relname = 'tbl_rename_legacy_label' + +query I +SELECT count(1) FROM [SHOW SCHEDULES] +WHERE label = 'row-level-ttl: $label_suffix' +---- +1 + +# Simulate a "legacy" label to ensure that it will be appropriately updated +# when the table is renamed. +statement ok +UPDATE system.scheduled_jobs SET schedule_name = 'row-level-ttl-1234' WHERE schedule_name = 'row-level-ttl: $label_suffix'; + +statement ok +ALTER TABLE tbl_rename_legacy_label RENAME TO tbl_renamed2 + +let $label_suffix +SELECT relname || ' [' || oid || ']' FROM pg_class WHERE relname = 'tbl_renamed2' + +# Legacy label no longer exists. +query I +SELECT count(1) FROM [SHOW SCHEDULES] +WHERE label = 'row-level-ttl-1234' +---- +0 + +# Renamed and new formatted label is showing up as expected. +query I +SELECT count(1) FROM [SHOW SCHEDULES] +WHERE label = 'row-level-ttl: $label_suffix' +---- +1 +subtest end + subtest drop_table # Ensure schedules are removed on DROP TABLE. @@ -365,12 +445,13 @@ CREATE TABLE tbl_drop_table ( id INT PRIMARY KEY ) WITH (ttl_expire_after = '10 minutes') -let $table_id -SELECT oid FROM pg_class WHERE relname = 'tbl_drop_table' + +let $label_suffix +SELECT relname || ' [' || oid || ']' FROM pg_class WHERE relname = 'tbl_drop_table' query I SELECT count(1) FROM [SHOW SCHEDULES] -WHERE label = 'row-level-ttl-$table_id' +WHERE label = 'row-level-ttl: $label_suffix' ---- 1 @@ -379,7 +460,7 @@ DROP TABLE tbl_drop_table query I SELECT count(1) FROM [SHOW SCHEDULES] -WHERE label = 'row-level-ttl-$table_id' +WHERE label = 'row-level-ttl: $label_suffix' ---- 0 @@ -395,15 +476,15 @@ statement ok CREATE TABLE drop_me.tbl () WITH (ttl_expire_after = '10 minutes'); CREATE TABLE drop_me.tbl2 () WITH (ttl_expire_after = '10 minutes') -let $table_id -SELECT oid FROM pg_class WHERE relname = 'tbl' +let $label_suffix +SELECT relname || ' [' || oid || ']' FROM pg_class WHERE relname = 'tbl' -let $table_id2 -SELECT oid FROM pg_class WHERE relname = 'tbl2' +let $label_suffix2 +SELECT relname || ' [' || oid || ']' FROM pg_class WHERE relname = 'tbl2' query I SELECT count(1) FROM [SHOW SCHEDULES] -WHERE label IN ('row-level-ttl-$table_id', 'row-level-ttl-$table_id2') +WHERE label IN ('row-level-ttl: $label_suffix', 'row-level-ttl: $label_suffix2') ---- 2 @@ -412,7 +493,7 @@ DROP SCHEMA drop_me CASCADE query I SELECT count(1) FROM [SHOW SCHEDULES] -WHERE label = 'row-level-ttl-$table_id' +WHERE label = 'row-level-ttl: $label_suffix' ---- 0 @@ -431,15 +512,15 @@ statement ok CREATE TABLE tbl () WITH (ttl_expire_after = '10 minutes'); CREATE TABLE tbl2 () WITH (ttl_expire_after = '10 minutes') -let $table_id -SELECT oid FROM pg_class WHERE relname = 'tbl' +let $label_suffix +SELECT relname || ' [' || oid || ']' FROM pg_class WHERE relname = 'tbl' -let $table_id2 -SELECT oid FROM pg_class WHERE relname = 'tbl2' +let $label_suffix2 +SELECT relname || ' [' || oid || ']' FROM pg_class WHERE relname = 'tbl2' query I SELECT count(1) FROM [SHOW SCHEDULES] -WHERE label IN ('row-level-ttl-$table_id', 'row-level-ttl-$table_id2') +WHERE label IN ('row-level-ttl: $label_suffix', 'row-level-ttl: $label_suffix2') ---- 2 @@ -449,7 +530,7 @@ DROP DATABASE drop_me CASCADE query I SELECT count(1) FROM [SHOW SCHEDULES] -WHERE label = 'row-level-ttl-$table_id' +WHERE label = 'row-level-ttl: $label_suffix' ---- 0 @@ -494,18 +575,18 @@ CREATE TABLE public.tbl_ttl_on_noop ( CONSTRAINT tbl_ttl_on_noop_pkey PRIMARY KEY (id ASC) ) WITH (ttl = 'on', ttl_expire_after = '00:10:00':::INTERVAL, ttl_job_cron = '@hourly') -let $table_id -SELECT oid FROM pg_class WHERE relname = 'tbl_ttl_on_noop' +let $label_suffix +SELECT relname || ' [' || oid || ']' FROM pg_class WHERE relname = 'tbl_ttl_on_noop' query TTT SELECT schedule_status, recurrence, owner FROM [SHOW SCHEDULES] -WHERE label = 'row-level-ttl-$table_id' +WHERE label = 'row-level-ttl: $label_suffix' ---- ACTIVE @hourly root let $schedule_id SELECT id FROM [SHOW SCHEDULES] -WHERE label = 'row-level-ttl-$table_id' +WHERE label = 'row-level-ttl: $label_suffix' query T SELECT create_statement FROM [SHOW CREATE SCHEDULE $schedule_id] @@ -533,12 +614,12 @@ CREATE TABLE public.tbl_ttl_job_cron ( CONSTRAINT tbl_ttl_job_cron_pkey PRIMARY KEY (id ASC) ) WITH (ttl = 'on', ttl_expire_after = '00:10:00':::INTERVAL, ttl_job_cron = '@daily') -let $table_id -SELECT oid FROM pg_class WHERE relname = 'tbl_ttl_job_cron' +let $label_suffix +SELECT relname || ' [' || oid || ']' FROM pg_class WHERE relname = 'tbl_ttl_job_cron' query TTT SELECT schedule_status, recurrence, owner FROM [SHOW SCHEDULES] -WHERE label = 'row-level-ttl-$table_id' +WHERE label = 'row-level-ttl: $label_suffix' ---- ACTIVE @daily root @@ -557,7 +638,7 @@ CREATE TABLE public.tbl_ttl_job_cron ( query TTT SELECT schedule_status, recurrence, owner FROM [SHOW SCHEDULES] -WHERE label = 'row-level-ttl-$table_id' +WHERE label = 'row-level-ttl: $label_suffix' ---- ACTIVE @weekly root @@ -575,7 +656,7 @@ CREATE TABLE public.tbl_ttl_job_cron ( query TTT SELECT schedule_status, recurrence, owner FROM [SHOW SCHEDULES] -WHERE label = 'row-level-ttl-$table_id' +WHERE label = 'row-level-ttl: $label_suffix' ---- ACTIVE @hourly root @@ -1037,12 +1118,12 @@ CREATE TABLE public.create_table_no_ttl_set_ttl_expire_after ( CONSTRAINT create_table_no_ttl_set_ttl_expire_after_pkey PRIMARY KEY (id ASC) ) WITH (ttl = 'on', ttl_expire_after = '00:10:00':::INTERVAL, ttl_job_cron = '@hourly') -let $table_id -SELECT oid FROM pg_class WHERE relname = 'create_table_no_ttl_set_ttl_expire_after' +let $label_suffix +SELECT relname || ' [' || oid || ']' FROM pg_class WHERE relname = 'create_table_no_ttl_set_ttl_expire_after' query TTT SELECT schedule_status, recurrence, owner FROM [SHOW SCHEDULES] -WHERE label = 'row-level-ttl-$table_id' +WHERE label = 'row-level-ttl: $label_suffix' ---- ACTIVE @hourly root @@ -1051,7 +1132,7 @@ ALTER TABLE create_table_no_ttl_set_ttl_expire_after RESET (ttl) query TTT SELECT schedule_status, recurrence, owner FROM [SHOW SCHEDULES] -WHERE label = 'row-level-ttl-$table_id' +WHERE label = 'row-level-ttl: $label_suffix' ---- subtest end @@ -1078,12 +1159,12 @@ CREATE TABLE public.create_table_no_ttl_set_ttl_expiration_expression ( FAMILY fam_0_id_expire_at (id, expire_at) ) WITH (ttl = 'on', ttl_expiration_expression = 'expire_at', ttl_job_cron = '@hourly') -let $table_id -SELECT oid FROM pg_class WHERE relname = 'create_table_no_ttl_set_ttl_expiration_expression' +let $label_suffix +SELECT relname || ' [' || oid || ']' FROM pg_class WHERE relname = 'create_table_no_ttl_set_ttl_expiration_expression' query TTT SELECT schedule_status, recurrence, owner FROM [SHOW SCHEDULES] -WHERE label = 'row-level-ttl-$table_id' +WHERE label = 'row-level-ttl: $label_suffix' ---- ACTIVE @hourly root @@ -1092,7 +1173,7 @@ ALTER TABLE create_table_no_ttl_set_ttl_expiration_expression RESET (ttl) query TTT SELECT schedule_status, recurrence, owner FROM [SHOW SCHEDULES] -WHERE label = 'row-level-ttl-$table_id' +WHERE label = 'row-level-ttl: $label_suffix' ---- subtest end @@ -1102,12 +1183,12 @@ subtest special_table_name statement ok CREATE TABLE "Table-Name" (id INT PRIMARY KEY) WITH (ttl_expire_after = '10 hours') -let $table_id -SELECT oid FROM pg_class WHERE relname = 'Table-Name' +let $label_suffix +SELECT relname || ' [' || oid || ']' FROM pg_class WHERE relname = 'Table-Name' let $schedule_id SELECT id FROM [SHOW SCHEDULES] -WHERE label = 'row-level-ttl-$table_id' +WHERE label = 'row-level-ttl: $label_suffix' query T SELECT create_statement FROM [SHOW CREATE SCHEDULE $schedule_id] diff --git a/pkg/sql/rename_table.go b/pkg/sql/rename_table.go index 9262d930bf00..4d5e64a11610 100644 --- a/pkg/sql/rename_table.go +++ b/pkg/sql/rename_table.go @@ -17,6 +17,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" + "github.com/cockroachdb/cockroach/pkg/sql/descmetadata" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice" @@ -232,6 +233,21 @@ func (n *renameTableNode) startExec(params runParams) error { return err } + metadataUpdater := descmetadata.NewMetadataUpdater( + ctx, + p.InternalSQLTxn(), + p.Descriptors(), + &p.ExecCfg().Settings.SV, + p.SessionData(), + ) + + // If this table has row level ttl enabled, update the schedule_name of all + // row-level-ttl jobs with the name table name. If row level TTL is not + // enabled, UpdateTTLScheduleName will no-op for us. + if err := metadataUpdater.UpdateTTLScheduleLabel(ctx, tableDesc); err != nil { + return err + } + // Log Rename Table event. This is an auditable log event and is recorded // in the same transaction as the table descriptor update. return p.logEvent(ctx, diff --git a/pkg/sql/schema_changer.go b/pkg/sql/schema_changer.go index 4144fc25f407..479546a409c7 100644 --- a/pkg/sql/schema_changer.go +++ b/pkg/sql/schema_changer.go @@ -1598,8 +1598,7 @@ func (sc *SchemaChanger) done(ctx context.Context) error { sc.execCfg.JobsKnobs(), scheduledJobs, scTable.GetPrivileges().Owner(), - scTable.GetID(), - modify.RowLevelTTL(), + scTable, ) if err != nil { return err diff --git a/pkg/sql/schema_changer_test.go b/pkg/sql/schema_changer_test.go index 81f9e01340a1..e458af695f38 100644 --- a/pkg/sql/schema_changer_test.go +++ b/pkg/sql/schema_changer_test.go @@ -7513,26 +7513,26 @@ ALTER TABLE t.test SET (ttl_expire_after = '10 hours'); "t", "test", ) + rowLevelTTL := desc.GetRowLevelTTL() if tc.expectSchedule { require.NotNil(t, rowLevelTTL) require.Greater(t, rowLevelTTL.ScheduleID, int64(0)) // Ensure there is only one schedule and that it belongs to the table. + var hasSchedule bool + require.NoError(t, sqlDB.QueryRow(`SELECT count(1) = 1 FROM [SHOW SCHEDULES] WHERE id = $1`, rowLevelTTL.ScheduleID).Scan(&hasSchedule)) + require.True(t, hasSchedule) + var numSchedules int - require.NoError(t, sqlDB.QueryRow( - `SELECT count(1) FROM [SHOW SCHEDULES] WHERE label LIKE $1`, - fmt.Sprintf("row-level-ttl-%d", rowLevelTTL.ScheduleID), - ).Scan(&numSchedules)) - require.Equal(t, 0, numSchedules) - require.NoError(t, sqlDB.QueryRow(`SELECT count(1) FROM [SHOW SCHEDULES] WHERE label LIKE 'row-level-ttl-%'`).Scan(&numSchedules)) + require.NoError(t, sqlDB.QueryRow(`SELECT count(1) FROM [SHOW SCHEDULES] WHERE label LIKE SOME ('row-level-ttl%', '%' || $1 || '%', '%' || $2 || '%')`, desc.TableDesc().ID, desc.TableDesc().Name).Scan(&numSchedules)) require.Equal(t, 1, numSchedules) } else { require.Nil(t, rowLevelTTL) // Ensure there are no schedules. var numSchedules int - require.NoError(t, sqlDB.QueryRow(`SELECT count(1) FROM [SHOW SCHEDULES] WHERE label LIKE 'row-level-ttl-%'`).Scan(&numSchedules)) + require.NoError(t, sqlDB.QueryRow(`SELECT count(1) FROM [SHOW SCHEDULES] WHERE label LIKE ANY ('row-level-ttl%', '%' || $1 || '%', '%' || $2 || '%')`, desc.TableDesc().ID, desc.TableDesc().Name).Scan(&numSchedules)) require.Equal(t, 0, numSchedules) } }) diff --git a/pkg/sql/schemachanger/scdeps/sctestdeps/test_deps.go b/pkg/sql/schemachanger/scdeps/sctestdeps/test_deps.go index b5e413a5cc76..9fba8f0782be 100644 --- a/pkg/sql/schemachanger/scdeps/sctestdeps/test_deps.go +++ b/pkg/sql/schemachanger/scdeps/sctestdeps/test_deps.go @@ -31,6 +31,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog/funcdesc" "github.com/cockroachdb/cockroach/pkg/sql/catalog/nstree" "github.com/cockroachdb/cockroach/pkg/sql/catalog/schemadesc" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" "github.com/cockroachdb/cockroach/pkg/sql/catalog/typedesc" "github.com/cockroachdb/cockroach/pkg/sql/privilege" "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scbuild" @@ -1184,6 +1185,12 @@ func (s *TestState) DeleteSchedule(ctx context.Context, id int64) error { return nil } +// UpdateTTLScheduleLabel implements scexec.DescriptorMetadataUpdater +func (s *TestState) UpdateTTLScheduleLabel(ctx context.Context, tbl *tabledesc.Mutable) error { + s.LogSideEffectf("update ttl schedule label #%d", tbl.ID) + return nil +} + // DescriptorMetadataUpdater implement scexec.Dependencies. func (s *TestState) DescriptorMetadataUpdater( ctx context.Context, diff --git a/pkg/sql/schemachanger/scexec/BUILD.bazel b/pkg/sql/schemachanger/scexec/BUILD.bazel index 58e9f2cc4d72..b8338714284a 100644 --- a/pkg/sql/schemachanger/scexec/BUILD.bazel +++ b/pkg/sql/schemachanger/scexec/BUILD.bazel @@ -28,6 +28,7 @@ go_library( "//pkg/sql/catalog/catalogkeys", "//pkg/sql/catalog/descpb", "//pkg/sql/catalog/nstree", + "//pkg/sql/catalog/tabledesc", "//pkg/sql/pgwire/pgcode", "//pkg/sql/pgwire/pgerror", "//pkg/sql/schemachanger/scerrors", diff --git a/pkg/sql/schemachanger/scexec/dependencies.go b/pkg/sql/schemachanger/scexec/dependencies.go index 5480e95f08a0..8f8612470684 100644 --- a/pkg/sql/schemachanger/scexec/dependencies.go +++ b/pkg/sql/schemachanger/scexec/dependencies.go @@ -23,6 +23,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkeys" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scexec/scmutationexec" "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" "github.com/cockroachdb/cockroach/pkg/util/hlc" @@ -348,6 +349,10 @@ type DescriptorMetadataUpdater interface { // DeleteSchedule deletes the given schedule. DeleteSchedule(ctx context.Context, id int64) error + + // UpdateTTLScheduleLabel updates the schedule_name for the TTL Scheduled Job + // of the given table. + UpdateTTLScheduleLabel(ctx context.Context, tbl *tabledesc.Mutable) error } // StatsRefreshQueue queues table for stats refreshes. diff --git a/pkg/sql/schemachanger/scexec/executor_external_test.go b/pkg/sql/schemachanger/scexec/executor_external_test.go index fae8da878a0c..9244f84a9d0f 100644 --- a/pkg/sql/schemachanger/scexec/executor_external_test.go +++ b/pkg/sql/schemachanger/scexec/executor_external_test.go @@ -518,7 +518,14 @@ func (noopMetadataUpdater) DeleteDatabaseRoleSettings(ctx context.Context, dbID return nil } -// DeleteScheduleID implements scexec.DescriptorMetadataUpdater +// DeleteScheduleID implements scexec.DescriptorMetadataUpdater. func (noopMetadataUpdater) DeleteSchedule(ctx context.Context, scheduleID int64) error { return nil } + +// UpdateTTLScheduleLabel implements scexec.DescriptorMetadataUpdater. +func (noopMetadataUpdater) UpdateTTLScheduleLabel( + ctx context.Context, tbl *tabledesc.Mutable, +) error { + return nil +} diff --git a/pkg/sql/ttl/ttlbase/BUILD.bazel b/pkg/sql/ttl/ttlbase/BUILD.bazel index 77f9ed939df6..e78de32d4222 100644 --- a/pkg/sql/ttl/ttlbase/BUILD.bazel +++ b/pkg/sql/ttl/ttlbase/BUILD.bazel @@ -11,6 +11,7 @@ go_library( deps = [ "//pkg/sql/catalog/catenumpb", "//pkg/sql/catalog/catpb", + "//pkg/sql/catalog/tabledesc", ], ) diff --git a/pkg/sql/ttl/ttlbase/ttl_helpers.go b/pkg/sql/ttl/ttlbase/ttl_helpers.go index c39e2f6cb266..6de77ee37ded 100644 --- a/pkg/sql/ttl/ttlbase/ttl_helpers.go +++ b/pkg/sql/ttl/ttlbase/ttl_helpers.go @@ -12,11 +12,13 @@ package ttlbase import ( "bytes" + "fmt" "strconv" "time" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catenumpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" ) // DefaultAOSTDuration is the default duration to use in the AS OF SYSTEM TIME @@ -32,6 +34,12 @@ var endKeyCompareOps = map[catenumpb.IndexColumn_Direction]string{ catenumpb.IndexColumn_DESC: ">", } +// BuildScheduleLabel returns a string value intended for use as the +// schedule_name/label column for the scheduled job created by row level TTL. +func BuildScheduleLabel(tbl *tabledesc.Mutable) string { + return fmt.Sprintf("row-level-ttl: %s [%d]", tbl.GetName(), tbl.ID) +} + func BuildSelectQuery( relationName string, pkColNames []string, diff --git a/pkg/sql/ttl/ttlschedule/ttlschedule.go b/pkg/sql/ttl/ttlschedule/ttlschedule.go index 08f0b576710f..64ec0502da98 100644 --- a/pkg/sql/ttl/ttlschedule/ttlschedule.go +++ b/pkg/sql/ttl/ttlschedule/ttlschedule.go @@ -136,6 +136,9 @@ func (s rowLevelTTLExecutor) ExecuteJob( return err } + // TODO(chrisseto): Should opName be updated to match schedule_name? We'll + // have to query to resolve the table name or delegate to sj.ScheduleLabel, + // which may make debugging quite confusing if the label gets out of whack. p, cleanup := cfg.PlanHookMaker( ctx, fmt.Sprintf("invoke-row-level-ttl-%d", args.TableID), diff --git a/pkg/storage/mvcc_value.go b/pkg/storage/mvcc_value.go index d671a7dec8ec..0718418b8a12 100644 --- a/pkg/storage/mvcc_value.go +++ b/pkg/storage/mvcc_value.go @@ -12,7 +12,6 @@ package storage import ( "encoding/binary" - "testing" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/storage/enginepb" @@ -147,7 +146,10 @@ var disableSimpleValueEncoding = util.ConstantWithMetamorphicTestBool( // DisableMetamorphicSimpleValueEncoding disables the disableSimpleValueEncoding // metamorphic bool for the duration of a test, resetting it at the end. -func DisableMetamorphicSimpleValueEncoding(t testing.TB) { +func DisableMetamorphicSimpleValueEncoding(t interface { + Helper() + Cleanup(func()) +}) { t.Helper() if disableSimpleValueEncoding { disableSimpleValueEncoding = false diff --git a/pkg/testutils/release/releases_test.go b/pkg/testutils/release/releases_test.go index 8bfbb91ff00c..f5073d3182c0 100644 --- a/pkg/testutils/release/releases_test.go +++ b/pkg/testutils/release/releases_test.go @@ -40,10 +40,17 @@ var ( }, } - // Results rely on this constant seed - rng = rand.New(rand.NewSource(1)) + // Results rely on this constant seed. + seed = int64(1) + + // Tests must call `resetRNG` if they use this global rng. + rng *rand.Rand ) +func resetRNG() { + rng = rand.New(rand.NewSource(seed)) +} + func TestLatestAndRandomPredecessor(t *testing.T) { testCases := []struct { name string @@ -72,16 +79,17 @@ func TestLatestAndRandomPredecessor(t *testing.T) { name: "latest is withdrawn", v: "v22.2.3", expectedLatest: "22.1.12", - expectedRandom: "22.1.8", + expectedRandom: "22.1.10", }, } - for _, tc := range testCases { - oldReleaseData := releaseData - releaseData = testReleaseData - defer func() { releaseData = oldReleaseData }() + oldReleaseData := releaseData + releaseData = testReleaseData + defer func() { releaseData = oldReleaseData }() + for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + resetRNG() // deterministic results latestPred, latestErr := LatestPredecessor(version.MustParse(tc.v)) randomPred, randomErr := RandomPredecessor(rng, version.MustParse(tc.v)) if tc.expectedErr == "" { @@ -119,23 +127,24 @@ func TestLatestPredecessorHistory(t *testing.T) { v: "v23.1.1", k: 3, expectedLatest: []string{"19.2.0", "22.1.12", "22.2.8"}, - expectedRandom: []string{"19.2.0", "22.1.12", "22.2.1"}, + expectedRandom: []string{"19.2.0", "22.1.8", "22.2.8"}, }, { name: "with pre-release", v: "v23.1.1-beta.1", k: 2, expectedLatest: []string{"22.1.12", "22.2.8"}, - expectedRandom: []string{"22.1.0", "22.2.5"}, + expectedRandom: []string{"22.1.8", "22.2.8"}, }, } - for _, tc := range testCases { - oldReleaseData := releaseData - releaseData = testReleaseData - defer func() { releaseData = oldReleaseData }() + oldReleaseData := releaseData + releaseData = testReleaseData + defer func() { releaseData = oldReleaseData }() + for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + resetRNG() // deterministic results latestHistory, latestErr := LatestPredecessorHistory(version.MustParse(tc.v), tc.k) randomHistory, randomErr := RandomPredecessorHistory(rng, version.MustParse(tc.v), tc.k) if tc.expectedErr == "" {