diff --git a/docs/RFCS/20220120_row_level_ttl.md b/docs/RFCS/20220120_row_level_ttl.md index e2f2bb6ce4a4..335b0c264b40 100644 --- a/docs/RFCS/20220120_row_level_ttl.md +++ b/docs/RFCS/20220120_row_level_ttl.md @@ -13,7 +13,7 @@ This has been a [feature commonly asked for](#20239). This RFC proposes a CockroachDB level mechanism to support row-level TTL, where rows will be deleted after a certain period of time. As a further extension in a -later release, rows rows will be automatically hidden after they've expired +later release, rows will be automatically hidden after they've expired their TTL and before they've been physically deleted. The following declarative syntaxes will initialize a table with row-level TTL: @@ -378,8 +378,8 @@ a few problems: process. This adds further complexity to CDC. As row-level TTL is a "SQL level" feature, it makes sense that something in the -SQL layer would be most appropriate to handle it. See [comparison -doc](comparison doc) for other observations. +SQL layer would be most appropriate to handle it. See [comparison doc](comparison doc) +for other observations. ### Alternative TTL columns Another proposal for TTL columns was to have two columns: @@ -465,7 +465,7 @@ has two benefits, both of which are very significant: This is predicated on filtering out expired rows working as otherwise users could miss entries when querying the secondary index as opposed to the primary. -### Improve the deletion loopl +### Improve the deletion loop We can speed up the deletion by using indexes if one was created on the TTL column for the table. diff --git a/docs/generated/sql/functions.md b/docs/generated/sql/functions.md index b682dbab3bce..bf423a9418dd 100644 --- a/docs/generated/sql/functions.md +++ b/docs/generated/sql/functions.md @@ -3005,6 +3005,8 @@ SELECT * FROM crdb_internal.check_consistency(true, ‘\x02’, ‘\x04’)

crdb_internal.range_stats(key: bytes) → jsonb

This function is used to retrieve range statistics information as a JSON object.

+crdb_internal.repair_ttl_table_scheduled_job(oid: oid) → void

Repairs the scheduled job for a TTL table if it is missing.

+
crdb_internal.reset_index_usage_stats() → bool

This function is used to clear the collected index usage statistics.

crdb_internal.reset_sql_stats() → bool

This function is used to clear the collected SQL statistics.

@@ -3034,6 +3036,8 @@ table. Returns an error if validation fails.

crdb_internal.validate_session_revival_token(token: bytes) → bool

Validate a token that was created by create_session_revival_token. Intended for testing.

+crdb_internal.validate_ttl_scheduled_jobs() → void

Validate all TTL tables have a valid scheduled job attached.

+
crdb_internal.void_func() → void

This function is used only by CockroachDB’s developers for testing purposes.

current_database() → string

Returns the current database.

diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 202ba8f1fbea..6df843bb9f7a 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -343,6 +343,7 @@ ALL_TESTS = [ "//pkg/sql/sem/tree/eval_test:eval_test_test", "//pkg/sql/sem/tree:tree_test", "//pkg/sql/sessiondata:sessiondata_test", + "//pkg/sql/sessioninit:sessioninit_test", "//pkg/sql/span:span_test", "//pkg/sql/sqlinstance/instanceprovider:instanceprovider_test", "//pkg/sql/sqlinstance/instancestorage:instancestorage_test", diff --git a/pkg/ccl/backupccl/schedule_exec.go b/pkg/ccl/backupccl/schedule_exec.go index 66e4733a41bd..e7c492880a8e 100644 --- a/pkg/ccl/backupccl/schedule_exec.go +++ b/pkg/ccl/backupccl/schedule_exec.go @@ -17,6 +17,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/scheduledjobs" "github.com/cockroachdb/cockroach/pkg/sql" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" "github.com/cockroachdb/cockroach/pkg/sql/parser" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlutil" @@ -448,6 +449,7 @@ func (e *scheduledBackupExecutor) OnDrop( env scheduledjobs.JobSchedulerEnv, sj *jobs.ScheduledJob, txn *kv.Txn, + descsCol *descs.Collection, ) error { args := &ScheduledBackupExecutionArgs{} if err := pbtypes.UnmarshalAny(sj.ExecutionArgs().Args, args); err != nil { diff --git a/pkg/ccl/backupccl/testdata/backup-restore/row_level_ttl b/pkg/ccl/backupccl/testdata/backup-restore/row_level_ttl index 94d8571a77a6..0e49fa0fec4f 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/row_level_ttl +++ b/pkg/ccl/backupccl/testdata/backup-restore/row_level_ttl @@ -28,6 +28,10 @@ exec-sql RESTORE FROM LATEST IN 'nodelocal://0/full_cluster_backup/' ---- +exec-sql +SELECT crdb_internal.validate_ttl_scheduled_jobs() +---- + query-sql SELECT create_statement FROM [SHOW CREATE TABLE d.public.t] ---- @@ -57,6 +61,10 @@ exec-sql RESTORE DATABASE d FROM LATEST IN 'nodelocal://0/database_backup/' ---- +exec-sql +SELECT crdb_internal.validate_ttl_scheduled_jobs() +---- + query-sql SELECT create_statement FROM [SHOW CREATE TABLE d.public.t] ---- @@ -90,6 +98,10 @@ exec-sql RESTORE TABLE d.public.t FROM LATEST IN 'nodelocal://0/database_backup/' ---- +exec-sql +SELECT crdb_internal.validate_ttl_scheduled_jobs() +---- + query-sql SELECT create_statement FROM [SHOW CREATE TABLE d.public.t] ---- diff --git a/pkg/cli/clisqlexec/format_table.go b/pkg/cli/clisqlexec/format_table.go index c7a9f8a2f474..f2e8d20bec96 100644 --- a/pkg/cli/clisqlexec/format_table.go +++ b/pkg/cli/clisqlexec/format_table.go @@ -12,6 +12,7 @@ package clisqlexec import ( "bytes" + "encoding/json" "fmt" "html" "io" @@ -584,6 +585,45 @@ func (p *recordReporter) doneRows(w io.Writer, seenRows int) error { func (p *recordReporter) beforeFirstRow(_ io.Writer, _ RowStrIter) error { return nil } func (p *recordReporter) doneNoRows(_ io.Writer) error { return nil } +type ndjsonReporter struct { + cols []string +} + +func (n *ndjsonReporter) describe(w io.Writer, cols []string) error { + n.cols = cols + return nil +} + +func (n *ndjsonReporter) beforeFirstRow(w io.Writer, allRows RowStrIter) error { + return nil +} + +func (n *ndjsonReporter) iter(w, ew io.Writer, rowIdx int, row []string) error { + retMap := make(map[string]string, len(row)) + for i := range row { + retMap[n.cols[i]] = row[i] + } + out, err := json.Marshal(retMap) + if err != nil { + return err + } + if _, err := ew.Write(out); err != nil { + return err + } + if _, err := ew.Write([]byte("\n")); err != nil { + return err + } + return nil +} + +func (n *ndjsonReporter) doneRows(w io.Writer, seenRows int) error { + return nil +} + +func (n *ndjsonReporter) doneNoRows(w io.Writer) error { + return nil +} + type sqlReporter struct { noColumns bool } @@ -644,6 +684,9 @@ func (sqlExecCtx *Context) makeReporter(w io.Writer) (rowReporter, func(), error reporter, cleanup := makeCSVReporter(w, sqlExecCtx.TableDisplayFormat) return reporter, cleanup, nil + case TableDisplayNDJSON: + return &ndjsonReporter{}, nil, nil + case TableDisplayRaw: return &rawReporter{}, nil, nil diff --git a/pkg/cli/clisqlexec/format_table_test.go b/pkg/cli/clisqlexec/format_table_test.go index 0dbb957251c1..92a27b0a6c7b 100644 --- a/pkg/cli/clisqlexec/format_table_test.go +++ b/pkg/cli/clisqlexec/format_table_test.go @@ -203,6 +203,7 @@ func Example_sql_empty_table() { // ----- // (0 rows) // sql --format=records -e select * from t.norows + // sql --format=ndjson -e select * from t.norows // sql --format=sql -e select * from t.norows // CREATE TABLE results ( // x STRING @@ -237,6 +238,10 @@ func Example_sql_empty_table() { // (3 rows) // sql --format=records -e select * from t.nocols // (3 rows) + // sql --format=ndjson -e select * from t.nocols + // {} + // {} + // {} // sql --format=sql -e select * from t.nocols // CREATE TABLE results ( // ); @@ -278,6 +283,7 @@ func Example_sql_empty_table() { // (0 rows) // sql --format=records -e select * from t.nocolsnorows // (0 rows) + // sql --format=ndjson -e select * from t.nocolsnorows // sql --format=sql -e select * from t.nocolsnorows // CREATE TABLE results ( // ); @@ -593,6 +599,16 @@ func Example_sql_table() { // s | a b c+ // | 12 123123213 12313 // d | tabs + // sql --format=ndjson -e select * from t.t + // {"d":"printable ASCII","s":"foo"} + // {"d":"printable ASCII with quotes","s":"\"foo"} + // {"d":"printable ASCII with backslash","s":"\\foo"} + // {"d":"non-printable ASCII","s":"foo\nbar"} + // {"d":"printable UTF8","s":"κόσμε"} + // {"d":"printable UTF8 using escapes","s":"ñ"} + // {"d":"non-printable UTF8 string","s":"\\x01"} + // {"d":"UTF8 string with RTL char","s":"܈85"} + // {"d":"tabs","s":"a\tb\tc\n12\t123123213\t12313"} // sql --format=sql -e select * from t.t // CREATE TABLE results ( // s STRING, diff --git a/pkg/cli/clisqlexec/table_display_format.go b/pkg/cli/clisqlexec/table_display_format.go index aac5ae5f88c1..c089cf523d85 100644 --- a/pkg/cli/clisqlexec/table_display_format.go +++ b/pkg/cli/clisqlexec/table_display_format.go @@ -35,6 +35,9 @@ const ( // TableDisplayRecords is a record-oriented format. It is somewhat // compatible with 'psql' "expanded display" mode. TableDisplayRecords + // TableDisplayNDJSON reports results in an nd-json format + // (https://github.com/ndjson/ndjson-spec). + TableDisplayNDJSON // TableDisplaySQL reports results using SQL statements that mimic // the creation of a SQL table containing the result values. TableDisplaySQL @@ -71,6 +74,8 @@ func (f *TableDisplayFormat) String() string { return "records" case TableDisplaySQL: return "sql" + case TableDisplayNDJSON: + return "ndjson" case TableDisplayHTML: return "html" case TableDisplayRawHTML: @@ -94,6 +99,8 @@ func (f *TableDisplayFormat) Set(s string) error { *f = TableDisplayRecords case "sql": *f = TableDisplaySQL + case "ndjson": + *f = TableDisplayNDJSON case "html": *f = TableDisplayHTML case "rawhtml": diff --git a/pkg/cli/clisqlshell/sql.go b/pkg/cli/clisqlshell/sql.go index 19bd732c688a..f9c5f8ff6317 100644 --- a/pkg/cli/clisqlshell/sql.go +++ b/pkg/cli/clisqlshell/sql.go @@ -359,7 +359,7 @@ var options = map[string]struct { display: func(c *cliState) string { return strconv.Itoa(c.sqlExecCtx.TableBorderMode) }, }, `display_format`: { - description: "the output format for tabular data (table, csv, tsv, html, sql, records, raw)", + description: "the output format for tabular data (table, csv, tsv, html, ndjson, sql, records, raw)", isBoolean: false, validDuringMultilineEntry: true, set: func(c *cliState, val string) error { diff --git a/pkg/cli/debug.go b/pkg/cli/debug.go index 2797bd981c18..7eec0fc0d107 100644 --- a/pkg/cli/debug.go +++ b/pkg/cli/debug.go @@ -129,6 +129,40 @@ func parsePositiveDuration(arg string) (time.Duration, error) { return duration, nil } +type keyFormat int + +const ( + hexKey = iota + base64Key +) + +func (f *keyFormat) Set(value string) error { + switch value { + case "hex": + *f = hexKey + case "base64": + *f = base64Key + default: + return errors.Errorf("unsupported format %s", value) + } + return nil +} + +func (f *keyFormat) String() string { + switch *f { + case hexKey: + return "hex" + case base64Key: + return "base64" + default: + panic(errors.AssertionFailedf("invalid format value %d", *f)) + } +} + +func (f *keyFormat) Type() string { + return "hex|base64" +} + // OpenEngineOptions tunes the behavior of OpenEngine. type OpenEngineOptions struct { ReadOnly bool @@ -528,19 +562,34 @@ func runDebugRangeDescriptors(cmd *cobra.Command, args []string) error { }) } +var decodeKeyOptions struct { + encoding keyFormat +} + var debugDecodeKeyCmd = &cobra.Command{ Use: "decode-key", Short: "decode ", Long: ` -Decode a hexadecimal-encoded key and pretty-print it. For example: +Decode encoded keys provided as command arguments and pretty-print them. +Key encoding type could be changed using encoding flag. +For example: - $ decode-key BB89F902ADB43000151C2D1ED07DE6C009 + $ cockroach debug decode-key BB89F902ADB43000151C2D1ED07DE6C009 /Table/51/1/44938288/1521140384.514565824,0 `, Args: cobra.ArbitraryArgs, RunE: func(cmd *cobra.Command, args []string) error { for _, arg := range args { - b, err := gohex.DecodeString(arg) + var b []byte + var err error + switch decodeKeyOptions.encoding { + case hexKey: + b, err = gohex.DecodeString(arg) + case base64Key: + b, err = base64.StdEncoding.DecodeString(arg) + default: + return errors.Errorf("unsupported key format %d", decodeKeyOptions.encoding) + } if err != nil { return err } @@ -1697,6 +1746,9 @@ func init() { f.Var(&debugMergeLogsOpts.useColor, "color", "force use of TTY escape codes to colorize the output") + f = debugDecodeKeyCmd.Flags() + f.Var(&decodeKeyOptions.encoding, "encoding", "key argument encoding") + f = debugDecodeProtoCmd.Flags() f.StringVar(&debugDecodeProtoName, "schema", "cockroach.sql.sqlbase.Descriptor", "fully qualified name of the proto to decode") diff --git a/pkg/cli/interactive_tests/test_pretty.tcl b/pkg/cli/interactive_tests/test_pretty.tcl index 68b4fe02f9d8..5b1106798ec6 100644 --- a/pkg/cli/interactive_tests/test_pretty.tcl +++ b/pkg/cli/interactive_tests/test_pretty.tcl @@ -19,6 +19,13 @@ eexpect "1 row" eexpect ":/# " end_test + +start_test "Check that tables are pretty-printed when output is not a terminal but --format=ndjson is specified." +send "echo 'select 1 as WOO;' | $argv sql --format=ndjson | cat\r" +eexpect "{\"woo\":\"1\"}" +eexpect ":/# " +end_test + start_test "Check that tables are pretty-printed when input is not a terminal and --format=table is not specified, but output is a terminal." send "echo begin; echo 'select 1 as WOO;' | $argv sql\r" eexpect "woo" diff --git a/pkg/jobs/BUILD.bazel b/pkg/jobs/BUILD.bazel index 11bd5b90e704..6d4b29342ab9 100644 --- a/pkg/jobs/BUILD.bazel +++ b/pkg/jobs/BUILD.bazel @@ -40,6 +40,7 @@ go_library( "//pkg/sql/catalog", "//pkg/sql/catalog/colinfo", "//pkg/sql/catalog/descpb", + "//pkg/sql/catalog/descs", "//pkg/sql/protoreflect", "//pkg/sql/sem/builtins", "//pkg/sql/sem/tree", diff --git a/pkg/jobs/scheduled_job_executor.go b/pkg/jobs/scheduled_job_executor.go index fd54b56b5007..fdff4a33a0d2 100644 --- a/pkg/jobs/scheduled_job_executor.go +++ b/pkg/jobs/scheduled_job_executor.go @@ -16,6 +16,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/scheduledjobs" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" "github.com/cockroachdb/cockroach/pkg/sql/sqlutil" "github.com/cockroachdb/cockroach/pkg/util/metric" "github.com/cockroachdb/cockroach/pkg/util/syncutil" @@ -63,8 +64,14 @@ type ScheduledJobExecutor interface { type ScheduledJobController interface { // OnDrop runs before the passed in `schedule` is dropped as part of a `DROP // SCHEDULE` query. - OnDrop(ctx context.Context, scheduleControllerEnv scheduledjobs.ScheduleControllerEnv, - env scheduledjobs.JobSchedulerEnv, schedule *ScheduledJob, txn *kv.Txn) error + OnDrop( + ctx context.Context, + scheduleControllerEnv scheduledjobs.ScheduleControllerEnv, + env scheduledjobs.JobSchedulerEnv, + schedule *ScheduledJob, + txn *kv.Txn, + descsCol *descs.Collection, + ) error } // ScheduledJobExecutorFactory is a callback to create a ScheduledJobExecutor. diff --git a/pkg/kv/kvserver/replica_protected_timestamp.go b/pkg/kv/kvserver/replica_protected_timestamp.go index fb86e671987f..827627a15a67 100644 --- a/pkg/kv/kvserver/replica_protected_timestamp.go +++ b/pkg/kv/kvserver/replica_protected_timestamp.go @@ -132,6 +132,17 @@ func (r *Replica) checkProtectedTimestampsForGC( if err != nil { return false, hlc.Timestamp{}, hlc.Timestamp{}, hlc.Timestamp{}, hlc.Timestamp{}, err } + + if read.readAt.IsEmpty() { + // We don't want to allow GC to proceed if no protected timestamp + // information is available. This can happen if the initial scan of the + // rangefeed established by the spanconfig.KVSubscriber hasn't completed + // yet. + log.VEventf(ctx, 1, + "not gc'ing replica %v because protected timestamp information is unavailable", r) + return false, hlc.Timestamp{}, hlc.Timestamp{}, hlc.Timestamp{}, hlc.Timestamp{}, nil + } + gcTimestamp = read.readAt if !read.earliestProtectionTimestamp.IsEmpty() { // NB: we want to allow GC up to the timestamp preceding the earliest valid diff --git a/pkg/kv/kvserver/replica_protected_timestamp_test.go b/pkg/kv/kvserver/replica_protected_timestamp_test.go index f02ca7af3ead..fb54c671ab32 100644 --- a/pkg/kv/kvserver/replica_protected_timestamp_test.go +++ b/pkg/kv/kvserver/replica_protected_timestamp_test.go @@ -53,6 +53,16 @@ func TestCheckProtectedTimestampsForGC(t *testing.T) { require.Zero(t, gcTimestamp) }, }, + { + name: "no PTS information is available", + test: func(t *testing.T, r *Replica, mp *manualPTSReader) { + mp.asOf = hlc.Timestamp{} + canGC, _, gcTimestamp, _, _, err := r.checkProtectedTimestampsForGC(ctx, makeTTLDuration(10)) + require.NoError(t, err) + require.False(t, canGC) + require.Zero(t, gcTimestamp) + }, + }, { name: "have overlapping but new enough that it's okay", test: func(t *testing.T, r *Replica, mp *manualPTSReader) { diff --git a/pkg/sql/BUILD.bazel b/pkg/sql/BUILD.bazel index e66c79b0cc45..980bc5b9d870 100644 --- a/pkg/sql/BUILD.bazel +++ b/pkg/sql/BUILD.bazel @@ -468,6 +468,7 @@ go_test( "backfill_test.go", "builtin_mem_usage_test.go", "builtin_test.go", + "check_test.go", "comment_on_column_test.go", "comment_on_constraint_test.go", "comment_on_database_test.go", @@ -583,6 +584,7 @@ go_test( "//pkg/gossip", "//pkg/jobs", "//pkg/jobs/jobspb", + "//pkg/jobs/jobstest", "//pkg/keys", "//pkg/kv", "//pkg/kv/kvclient", @@ -651,6 +653,7 @@ go_test( "//pkg/sql/sqlliveness", "//pkg/sql/sqlstats", "//pkg/sql/sqltestutils", + "//pkg/sql/sqlutil", "//pkg/sql/stats", "//pkg/sql/stmtdiagnostics", "//pkg/sql/tests", @@ -707,6 +710,7 @@ go_test( "@com_github_cockroachdb_logtags//:logtags", "@com_github_cockroachdb_redact//:redact", "@com_github_gogo_protobuf//proto", + "@com_github_gogo_protobuf//types", "@com_github_jackc_pgconn//:pgconn", "@com_github_jackc_pgtype//:pgtype", "@com_github_jackc_pgx_v4//:pgx", diff --git a/pkg/sql/catalog/descs/BUILD.bazel b/pkg/sql/catalog/descs/BUILD.bazel index c9c7ea73a1ae..d68b6be84499 100644 --- a/pkg/sql/catalog/descs/BUILD.bazel +++ b/pkg/sql/catalog/descs/BUILD.bazel @@ -18,6 +18,7 @@ go_library( "system_database_namespace_cache.go", "system_table.go", "table.go", + "table_name.go", "temporary_descriptors.go", "txn.go", "type.go", diff --git a/pkg/sql/catalog/descs/table_name.go b/pkg/sql/catalog/descs/table_name.go new file mode 100644 index 000000000000..84d45e1ce34c --- /dev/null +++ b/pkg/sql/catalog/descs/table_name.go @@ -0,0 +1,50 @@ +// 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 descs + +import ( + "context" + + "github.com/cockroachdb/cockroach/pkg/kv" + "github.com/cockroachdb/cockroach/pkg/sql/catalog" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" + "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/errors" +) + +// GetTableNameByID fetches the full tree table name by the given ID. +func GetTableNameByID( + ctx context.Context, txn *kv.Txn, tc *Collection, tableID descpb.ID, +) (*tree.TableName, error) { + tbl, err := tc.GetImmutableTableByID(ctx, txn, tableID, tree.ObjectLookupFlagsWithRequired()) + if err != nil { + return nil, err + } + return GetTableNameByDesc(ctx, txn, tc, tbl) +} + +// GetTableNameByDesc fetches the full tree table name by the given table descriptor. +func GetTableNameByDesc( + ctx context.Context, txn *kv.Txn, tc *Collection, tbl catalog.TableDescriptor, +) (*tree.TableName, error) { + sc, err := tc.GetImmutableSchemaByID(ctx, txn, tbl.GetParentSchemaID(), tree.SchemaLookupFlags{Required: true}) + if err != nil { + return nil, err + } + found, db, err := tc.GetImmutableDatabaseByID(ctx, txn, tbl.GetParentID(), tree.DatabaseLookupFlags{Required: true}) + if err != nil { + return nil, err + } + if !found { + return nil, errors.AssertionFailedf("expected database %d to exist", tbl.GetParentID()) + } + return tree.NewTableNameWithSchema(tree.Name(db.GetName()), tree.Name(sc.GetName()), tree.Name(tbl.GetName())), nil +} diff --git a/pkg/sql/check.go b/pkg/sql/check.go index 322f66a0d281..61b6305c1077 100644 --- a/pkg/sql/check.go +++ b/pkg/sql/check.go @@ -16,8 +16,10 @@ import ( "fmt" "strings" + "github.com/cockroachdb/cockroach/pkg/jobs" "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/sql/catalog" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/schemaexpr" "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" @@ -29,6 +31,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/util" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/errors" + pbtypes "github.com/gogo/protobuf/types" ) // validateCheckExpr verifies that the given CHECK expression returns true @@ -578,6 +581,130 @@ func validateUniqueConstraint( return nil } +// ValidateTTLScheduledJobsInCurrentDB is part of the EvalPlanner interface. +func (p *planner) ValidateTTLScheduledJobsInCurrentDB(ctx context.Context) error { + dbName := p.CurrentDatabase() + log.Infof(ctx, "validating scheduled jobs in database %s", dbName) + db, err := p.Descriptors().GetImmutableDatabaseByName( + ctx, p.Txn(), dbName, tree.DatabaseLookupFlags{Required: true}, + ) + if err != nil { + return err + } + tableDescs, err := p.Descriptors().GetAllTableDescriptorsInDatabase(ctx, p.Txn(), db.GetID()) + if err != nil { + return err + } + + for _, tableDesc := range tableDescs { + if err = p.validateTTLScheduledJobInTable(ctx, tableDesc); err != nil { + return err + } + } + return nil +} + +var invalidTableTTLScheduledJobError = errors.Newf("invalid scheduled job for table") + +// validateTTLScheduledJobsInCurrentDB is part of the EvalPlanner interface. +func (p *planner) validateTTLScheduledJobInTable( + ctx context.Context, tableDesc catalog.TableDescriptor, +) error { + if !tableDesc.HasRowLevelTTL() { + return nil + } + ttl := tableDesc.GetRowLevelTTL() + + execCfg := p.ExecCfg() + env := JobSchedulerEnv(execCfg) + + wrapError := func(origErr error) error { + return errors.WithHintf( + errors.Mark(origErr, invalidTableTTLScheduledJobError), + `use crdb_internal.repair_ttl_table_scheduled_job(%d) to repair the missing job`, + tableDesc.GetID(), + ) + } + + sj, err := jobs.LoadScheduledJob( + ctx, + env, + ttl.ScheduleID, + execCfg.InternalExecutor, + p.txn, + ) + if err != nil { + if jobs.HasScheduledJobNotFoundError(err) { + return wrapError( + pgerror.Newf( + pgcode.Internal, + "table id %d maps to a non-existent schedule id %d", + tableDesc.GetID(), + ttl.ScheduleID, + ), + ) + } + return errors.Wrapf(err, "error fetching schedule id %d for table id %d", ttl.ScheduleID, tableDesc.GetID()) + } + + var args catpb.ScheduledRowLevelTTLArgs + if err := pbtypes.UnmarshalAny(sj.ExecutionArgs().Args, &args); err != nil { + return wrapError( + pgerror.Wrapf( + err, + pgcode.Internal, + "error unmarshalling scheduled jobs args for table id %d, schedule id %d", + tableDesc.GetID(), + ttl.ScheduleID, + ), + ) + } + + if args.TableID != tableDesc.GetID() { + return wrapError( + pgerror.Newf( + pgcode.Internal, + "schedule id %d points to table id %d instead of table id %d", + ttl.ScheduleID, + args.TableID, + tableDesc.GetID(), + ), + ) + } + + return nil +} + +// RepairTTLScheduledJobForTable is part of the EvalPlanner interface. +func (p *planner) RepairTTLScheduledJobForTable(ctx context.Context, tableID int64) error { + tableDesc, err := p.Descriptors().GetMutableTableByID(ctx, p.txn, descpb.ID(tableID), tree.ObjectLookupFlagsWithRequired()) + if err != nil { + return err + } + validateErr := p.validateTTLScheduledJobInTable(ctx, tableDesc) + if validateErr == nil { + return nil + } + if !errors.HasType(validateErr, invalidTableTTLScheduledJobError) { + return errors.Wrap(validateErr, "error validating TTL on table") + } + sj, err := CreateRowLevelTTLScheduledJob( + ctx, + p.ExecCfg(), + p.txn, + p.User(), + tableDesc.GetID(), + tableDesc.GetRowLevelTTL(), + ) + if err != nil { + return err + } + tableDesc.RowLevelTTL.ScheduleID = sj.ScheduleID() + return p.Descriptors().WriteDesc( + ctx, false /* kvTrace */, tableDesc, p.txn, + ) +} + func formatValues(colNames []string, values tree.Datums) string { var pairs bytes.Buffer for i := range values { diff --git a/pkg/sql/check_test.go b/pkg/sql/check_test.go new file mode 100644 index 000000000000..abdf0d95add6 --- /dev/null +++ b/pkg/sql/check_test.go @@ -0,0 +1,137 @@ +// 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 sql_test + +import ( + "context" + gosql "database/sql" + "fmt" + "testing" + + "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/jobs" + "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" + "github.com/cockroachdb/cockroach/pkg/jobs/jobstest" + "github.com/cockroachdb/cockroach/pkg/keys" + "github.com/cockroachdb/cockroach/pkg/kv" + "github.com/cockroachdb/cockroach/pkg/sql" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/desctestutils" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" + "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/cockroach/pkg/sql/sqlutil" + "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" + "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/cockroachdb/cockroach/pkg/util/timeutil" + "github.com/cockroachdb/errors" + pbtypes "github.com/gogo/protobuf/types" + "github.com/lib/pq" + "github.com/stretchr/testify/require" +) + +func TestValidateTTLScheduledJobs(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + ctx := context.Background() + + testCases := []struct { + desc string + setup func(t *testing.T, sqlDB *gosql.DB, kvDB *kv.DB, s serverutils.TestServerInterface, tableDesc *tabledesc.Mutable, scheduleID int64) + expectedErrRe func(tableID descpb.ID, scheduleID int64) string + }{ + { + desc: "not pointing at a valid scheduled job", + setup: func(t *testing.T, sqlDB *gosql.DB, kvDB *kv.DB, s serverutils.TestServerInterface, tableDesc *tabledesc.Mutable, scheduleID int64) { + tableDesc.RowLevelTTL.ScheduleID = 0 + require.NoError(t, sql.TestingDescsTxn(ctx, s, func(ctx context.Context, txn *kv.Txn, col *descs.Collection) error { + return col.WriteDesc(ctx, false /* kvBatch */, tableDesc, txn) + })) + }, + expectedErrRe: func(tableID descpb.ID, scheduleID int64) string { + return fmt.Sprintf(`table id %d maps to a non-existent schedule id 0`, tableID) + }, + }, + { + desc: "scheduled job points at an different table", + setup: func(t *testing.T, sqlDB *gosql.DB, kvDB *kv.DB, s serverutils.TestServerInterface, tableDesc *tabledesc.Mutable, scheduleID int64) { + ie := s.InternalExecutor().(sqlutil.InternalExecutor) + require.NoError(t, kvDB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error { + sj, err := jobs.LoadScheduledJob( + ctx, + jobstest.NewJobSchedulerTestEnv( + jobstest.UseSystemTables, + timeutil.Now(), + tree.ScheduledBackupExecutor, + ), + scheduleID, + ie, + txn, + ) + if err != nil { + return err + } + var args catpb.ScheduledRowLevelTTLArgs + if err := pbtypes.UnmarshalAny(sj.ExecutionArgs().Args, &args); err != nil { + return err + } + args.TableID = 0 + any, err := pbtypes.MarshalAny(&args) + if err != nil { + return err + } + sj.SetExecutionDetails(sj.ExecutorType(), jobspb.ExecutionArguments{Args: any}) + return sj.Update(ctx, ie, txn) + })) + }, + expectedErrRe: func(tableID descpb.ID, scheduleID int64) string { + return fmt.Sprintf(`schedule id %d points to table id 0 instead of table id %d`, scheduleID, tableID) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + s, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{}) + defer s.Stopper().Stop(ctx) + + _, err := sqlDB.Exec(`CREATE TABLE t () WITH (ttl_expire_after = '10 mins')`) + require.NoError(t, err) + + tableDesc := desctestutils.TestingGetMutableExistingTableDescriptor(kvDB, keys.SystemSQLCodec, "defaultdb", "t") + require.NotNil(t, tableDesc.GetRowLevelTTL()) + scheduleID := tableDesc.GetRowLevelTTL().ScheduleID + + tc.setup(t, sqlDB, kvDB, s, tableDesc, scheduleID) + + _, err = sqlDB.Exec(`SELECT crdb_internal.validate_ttl_scheduled_jobs()`) + require.Error(t, err) + require.Regexp(t, tc.expectedErrRe(tableDesc.GetID(), scheduleID), err) + var pgxErr *pq.Error + require.True(t, errors.As(err, &pgxErr)) + require.Regexp( + t, + fmt.Sprintf(`use crdb_internal.repair_ttl_table_scheduled_job\(%d\) to repair the missing job`, tableDesc.GetID()), + pgxErr.Hint, + ) + + // Repair and check jobs are valid. + _, err = sqlDB.Exec(`DROP SCHEDULE $1`, scheduleID) + require.NoError(t, err) + _, err = sqlDB.Exec(`SELECT crdb_internal.repair_ttl_table_scheduled_job($1)`, tableDesc.GetID()) + require.NoError(t, err) + _, err = sqlDB.Exec(`SELECT crdb_internal.validate_ttl_scheduled_jobs()`) + require.NoError(t, err) + }) + } +} diff --git a/pkg/sql/compact_sql_stats.go b/pkg/sql/compact_sql_stats.go index 497f456d60a9..d485293c4e2f 100644 --- a/pkg/sql/compact_sql_stats.go +++ b/pkg/sql/compact_sql_stats.go @@ -20,6 +20,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/scheduledjobs" "github.com/cockroachdb/cockroach/pkg/security" "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" "github.com/cockroachdb/cockroach/pkg/sql/sqlstats/persistedsqlstats" @@ -172,6 +173,7 @@ func (e *scheduledSQLStatsCompactionExecutor) OnDrop( env scheduledjobs.JobSchedulerEnv, schedule *jobs.ScheduledJob, txn *kv.Txn, + descsCol *descs.Collection, ) error { return persistedsqlstats.ErrScheduleUndroppable } diff --git a/pkg/sql/control_schedules.go b/pkg/sql/control_schedules.go index 3d9ce3265455..c64896c2a74b 100644 --- a/pkg/sql/control_schedules.go +++ b/pkg/sql/control_schedules.go @@ -158,9 +158,14 @@ func (n *controlSchedulesNode) startExec(params runParams) error { if controller, ok := ex.(jobs.ScheduledJobController); ok { scheduleControllerEnv := scheduledjobs.MakeProdScheduleControllerEnv( params.ExecCfg().ProtectedTimestampProvider, params.ExecCfg().InternalExecutor) - if err := controller.OnDrop(params.ctx, scheduleControllerEnv, - scheduledjobs.ProdJobSchedulerEnv, schedule, - params.extendedEvalCtx.Txn); err != nil { + if err := controller.OnDrop( + params.ctx, + scheduleControllerEnv, + scheduledjobs.ProdJobSchedulerEnv, + schedule, + params.extendedEvalCtx.Txn, + params.p.Descriptors(), + ); err != nil { return errors.Wrap(err, "failed to run OnDrop") } } diff --git a/pkg/sql/descmetadata/metadata_updater.go b/pkg/sql/descmetadata/metadata_updater.go index cca4aafc617b..54e67ea779e7 100644 --- a/pkg/sql/descmetadata/metadata_updater.go +++ b/pkg/sql/descmetadata/metadata_updater.go @@ -126,9 +126,9 @@ func (mu metadataUpdater) DeleteDatabaseRoleSettings(ctx context.Context, dbID d if err != nil { return err } - // If system table updates should be minimized, avoid bumping up the version - // number of the table below. - if mu.cacheEnabled || rowsDeleted == 0 { + // If the cache is off or if no rows changed, there's no need to bump the + // table version. + if !mu.cacheEnabled || rowsDeleted == 0 { return nil } // Bump the table version for the role settings table when we modify it. diff --git a/pkg/sql/faketreeeval/evalctx.go b/pkg/sql/faketreeeval/evalctx.go index b9643f7f5a6b..916a702fb3b4 100644 --- a/pkg/sql/faketreeeval/evalctx.go +++ b/pkg/sql/faketreeeval/evalctx.go @@ -278,6 +278,16 @@ func (*DummyEvalPlanner) RevalidateUniqueConstraint( return errors.WithStack(errEvalPlanner) } +// ValidateTTLScheduledJobsInCurrentDB is part of the EvalPlanner interface. +func (*DummyEvalPlanner) ValidateTTLScheduledJobsInCurrentDB(ctx context.Context) error { + return errors.WithStack(errEvalPlanner) +} + +// RepairTTLScheduledJobForTable is part of the EvalPlanner interface. +func (*DummyEvalPlanner) RepairTTLScheduledJobForTable(ctx context.Context, tableID int64) error { + return errors.WithStack(errEvalPlanner) +} + // ExecutorConfig is part of the EvalPlanner interface. func (*DummyEvalPlanner) ExecutorConfig() interface{} { return nil diff --git a/pkg/sql/logictest/testdata/logic_test/row_level_ttl b/pkg/sql/logictest/testdata/logic_test/row_level_ttl index ad42e95a6d32..86ce05639f4a 100644 --- a/pkg/sql/logictest/testdata/logic_test/row_level_ttl +++ b/pkg/sql/logictest/testdata/logic_test/row_level_ttl @@ -44,6 +44,15 @@ CREATE TABLE public.tbl ( FAMILY fam_0_id_text_crdb_internal_expiration (id, text, crdb_internal_expiration) ) WITH (ttl = 'on', ttl_automatic_column = 'on', ttl_expire_after = '00:10:00':::INTERVAL) +statement ok +SELECT crdb_internal.validate_ttl_scheduled_jobs() + +statement ok +SELECT crdb_internal.repair_ttl_table_scheduled_job('tbl'::regclass::oid) + +statement ok +SELECT crdb_internal.validate_ttl_scheduled_jobs() + statement error resetting "ttl_expire_after" is not permitted\nHINT: use `RESET \(ttl\)` to remove TTL from the table ALTER TABLE tbl RESET (ttl_expire_after) @@ -67,6 +76,12 @@ WHERE label LIKE 'row-level-ttl-%' ---- 1 +let $schedule_id +SELECT id FROM [SHOW SCHEDULES] WHERE label LIKE 'row-level-ttl-%' + +statement error cannot drop a row level TTL schedule\nHINT: use ALTER TABLE test\.public\.tbl RESET \(ttl\) instead +DROP SCHEDULE $schedule_id + statement ok ALTER TABLE tbl SET (ttl_expire_after = '10 days') @@ -114,6 +129,9 @@ CREATE TABLE public.tbl ( FAMILY fam_0_id_text_crdb_internal_expiration (id, text) ) +statement ok +SELECT crdb_internal.validate_ttl_scheduled_jobs() + query I SELECT count(1) FROM [SHOW SCHEDULES] WHERE label LIKE 'row-level-ttl-%' diff --git a/pkg/sql/sem/builtins/builtins.go b/pkg/sql/sem/builtins/builtins.go index 959ca50fd812..08fd5929fd64 100644 --- a/pkg/sql/sem/builtins/builtins.go +++ b/pkg/sql/sem/builtins/builtins.go @@ -6473,6 +6473,40 @@ table's zone configuration this will return NULL.`, }, ), + "crdb_internal.validate_ttl_scheduled_jobs": makeBuiltin( + tree.FunctionProperties{ + Category: categorySystemInfo, + }, + tree.Overload{ + Types: tree.ArgTypes{}, + ReturnType: tree.FixedReturnType(types.Void), + Fn: func(evalCtx *tree.EvalContext, args tree.Datums) (tree.Datum, error) { + return tree.DVoidDatum, evalCtx.Planner.ValidateTTLScheduledJobsInCurrentDB(evalCtx.Context) + }, + Info: `Validate all TTL tables have a valid scheduled job attached.`, + Volatility: tree.VolatilityVolatile, + }, + ), + + "crdb_internal.repair_ttl_table_scheduled_job": makeBuiltin( + tree.FunctionProperties{ + Category: categorySystemInfo, + }, + tree.Overload{ + Types: tree.ArgTypes{{"oid", types.Oid}}, + ReturnType: tree.FixedReturnType(types.Void), + Fn: func(evalCtx *tree.EvalContext, args tree.Datums) (tree.Datum, error) { + oid := tree.MustBeDOid(args[0]) + if err := evalCtx.Planner.RepairTTLScheduledJobForTable(evalCtx.Ctx(), int64(oid.DInt)); err != nil { + return nil, err + } + return tree.DVoidDatum, nil + }, + Info: `Repairs the scheduled job for a TTL table if it is missing.`, + Volatility: tree.VolatilityVolatile, + }, + ), + "crdb_internal.check_password_hash_format": makeBuiltin( tree.FunctionProperties{ Category: categorySystemInfo, diff --git a/pkg/sql/sem/tree/eval.go b/pkg/sql/sem/tree/eval.go index af20082f4131..6ff44c92c689 100644 --- a/pkg/sql/sem/tree/eval.go +++ b/pkg/sql/sem/tree/eval.go @@ -3304,6 +3304,13 @@ type EvalPlanner interface { // constraint on the table. RevalidateUniqueConstraint(ctx context.Context, tableID int, constraintName string) error + // ValidateTTLScheduledJobsInCurrentDB checks scheduled jobs for each table + // in the database maps to a scheduled job. + ValidateTTLScheduledJobsInCurrentDB(ctx context.Context) error + // RepairTTLScheduledJob repairs the scheduled job for the given table if + // it is invalid. + RepairTTLScheduledJobForTable(ctx context.Context, tableID int64) error + // QueryRowEx executes the supplied SQL statement and returns a single row, or // nil if no row is found, or an error if more that one row is returned. // diff --git a/pkg/sql/sessioninit/BUILD.bazel b/pkg/sql/sessioninit/BUILD.bazel index ae52b325478b..b123cf775d43 100644 --- a/pkg/sql/sessioninit/BUILD.bazel +++ b/pkg/sql/sessioninit/BUILD.bazel @@ -1,4 +1,4 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "sessioninit", @@ -25,3 +25,30 @@ go_library( "@com_github_cockroachdb_logtags//:logtags", ], ) + +go_test( + name = "sessioninit_test", + srcs = [ + "cache_test.go", + "main_test.go", + ], + deps = [ + ":sessioninit", + "//pkg/base", + "//pkg/kv", + "//pkg/security", + "//pkg/security/securitytest", + "//pkg/server", + "//pkg/sql", + "//pkg/sql/catalog/descpb", + "//pkg/sql/catalog/descs", + "//pkg/sql/sqlutil", + "//pkg/testutils/serverutils", + "//pkg/testutils/sqlutils", + "//pkg/testutils/testcluster", + "//pkg/util/leaktest", + "//pkg/util/log", + "//pkg/util/randutil", + "@com_github_stretchr_testify//require", + ], +) diff --git a/pkg/sql/sessioninit/cache_test.go b/pkg/sql/sessioninit/cache_test.go new file mode 100644 index 000000000000..e1b33dc8d4fe --- /dev/null +++ b/pkg/sql/sessioninit/cache_test.go @@ -0,0 +1,196 @@ +// 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 sessioninit_test + +import ( + "context" + gosql "database/sql" + "net/url" + "testing" + + "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/kv" + "github.com/cockroachdb/cockroach/pkg/security" + "github.com/cockroachdb/cockroach/pkg/sql" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" + "github.com/cockroachdb/cockroach/pkg/sql/sessioninit" + "github.com/cockroachdb/cockroach/pkg/sql/sqlutil" + "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" + "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" + "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/stretchr/testify/require" +) + +func TestCacheInvalidation(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + ctx := context.Background() + s, db, _ := serverutils.StartServer(t, base.TestServerArgs{Insecure: false}) + defer s.Stopper().Stop(ctx) + defer db.Close() + + pgURL, cleanupFunc := sqlutils.PGUrl( + t, s.ServingSQLAddr(), "TestCacheInvalidation" /* prefix */, url.UserPassword("testuser", "abc"), + ) + defer cleanupFunc() + + // Extract login as a function so that we can call it to populate the cache + // with real information. + login := func() { + thisDB, err := gosql.Open("postgres", pgURL.String()) + require.NoError(t, err) + var i int + err = thisDB.QueryRow("SELECT 1").Scan(&i) + require.NoError(t, err) + _ = thisDB.Close() + } + + execCfg := s.ExecutorConfig().(sql.ExecutorConfig) + getSettingsFromCache := func() ([]sessioninit.SettingsCacheEntry, bool, error) { + didReadFromSystemTable := false + settings, err := execCfg.SessionInitCache.GetDefaultSettings( + ctx, + s.ClusterSettings(), + s.InternalExecutor().(sqlutil.InternalExecutor), + s.DB(), + s.CollectionFactory().(*descs.CollectionFactory), + security.TestUserName(), + "defaultdb", + func(ctx context.Context, txn *kv.Txn, ie sqlutil.InternalExecutor, username security.SQLUsername, databaseID descpb.ID) ([]sessioninit.SettingsCacheEntry, error) { + didReadFromSystemTable = true + return nil, nil + }) + return settings, didReadFromSystemTable, err + } + getAuthInfoFromCache := func() (sessioninit.AuthInfo, bool, error) { + didReadFromSystemTable := false + aInfo, err := execCfg.SessionInitCache.GetAuthInfo( + ctx, + s.ClusterSettings(), + s.InternalExecutor().(sqlutil.InternalExecutor), + s.DB(), + s.CollectionFactory().(*descs.CollectionFactory), + security.TestUserName(), + func(ctx context.Context, txn *kv.Txn, ie sqlutil.InternalExecutor, username security.SQLUsername) (sessioninit.AuthInfo, error) { + didReadFromSystemTable = true + return sessioninit.AuthInfo{}, nil + }) + return aInfo, didReadFromSystemTable, err + } + + // Create user and warm the cache. + _, err := db.ExecContext(ctx, "CREATE USER testuser WITH PASSWORD 'abc'") + require.NoError(t, err) + login() + + t.Run("default settings cache", func(t *testing.T) { + for _, stmt := range []string{ + `ALTER ROLE ALL IN DATABASE postgres SET search_path = 'a'`, + `ALTER ROLE testuser SET search_path = 'b'`, + } { + _, err := db.ExecContext(ctx, stmt) + require.NoError(t, err) + } + + // Check that the cache initially contains the default settings for testuser. + login() + settings, didReadFromSystemTable, err := getSettingsFromCache() + require.NoError(t, err) + require.False(t, didReadFromSystemTable) + require.Contains(t, settings, sessioninit.SettingsCacheEntry{ + SettingsCacheKey: sessioninit.SettingsCacheKey{ + DatabaseID: 0, + Username: security.TestUserName(), + }, + Settings: []string{"search_path=b"}, + }) + + // Verify that dropping a database referenced in the default settings table + // causes the cache to be invalidated. + _, err = db.ExecContext(ctx, "DROP DATABASE postgres") + require.NoError(t, err) + settings, didReadFromSystemTable, err = getSettingsFromCache() + require.NoError(t, err) + require.True(t, didReadFromSystemTable) + require.Empty(t, settings) + + // Verify that adding a new default setting causes the cache to be + // invalidated. We need to use login() to load "real" data. + _, err = db.ExecContext(ctx, `ALTER ROLE ALL SET search_path = 'c'`) + require.NoError(t, err) + login() + settings, didReadFromSystemTable, err = getSettingsFromCache() + require.NoError(t, err) + require.False(t, didReadFromSystemTable) + require.Contains(t, settings, sessioninit.SettingsCacheEntry{ + SettingsCacheKey: sessioninit.SettingsCacheKey{ + DatabaseID: 0, + Username: security.MakeSQLUsernameFromPreNormalizedString(""), + }, + Settings: []string{"search_path=c"}, + }) + + // Verify that dropping a user referenced in the default settings table + // causes the cache to be invalidated. + _, err = db.ExecContext(ctx, "DROP USER testuser") + require.NoError(t, err) + settings, didReadFromSystemTable, err = getSettingsFromCache() + require.NoError(t, err) + require.True(t, didReadFromSystemTable) + require.Empty(t, settings) + + // Re-create the user and warm the cache for the next test. + _, err = db.ExecContext(ctx, "CREATE USER testuser WITH PASSWORD 'abc'") + require.NoError(t, err) + login() + }) + + t.Run("auth info cache", func(t *testing.T) { + // Check that the cache initially contains info for testuser. + login() + aInfo, didReadFromSystemTable, err := getAuthInfoFromCache() + require.NoError(t, err) + require.False(t, didReadFromSystemTable) + require.True(t, aInfo.UserExists) + require.True(t, aInfo.CanLoginSQL) + + // Verify that creating a different user invalidates the cache. + _, err = db.ExecContext(ctx, "CREATE USER testuser2") + require.NoError(t, err) + aInfo, didReadFromSystemTable, err = getAuthInfoFromCache() + require.NoError(t, err) + require.True(t, didReadFromSystemTable) + + // Verify that dropping a user invalidates the cache + _, err = db.ExecContext(ctx, "DROP USER testuser2") + require.NoError(t, err) + aInfo, didReadFromSystemTable, err = getAuthInfoFromCache() + require.NoError(t, err) + require.True(t, didReadFromSystemTable) + + // Verify that altering VALID UNTIL invalidates the cache + _, err = db.ExecContext(ctx, "ALTER USER testuser VALID UNTIL '2099-01-01'") + require.NoError(t, err) + aInfo, didReadFromSystemTable, err = getAuthInfoFromCache() + require.NoError(t, err) + require.True(t, didReadFromSystemTable) + + // Sanity check to make sure the cache is used. + _, err = db.ExecContext(ctx, "SELECT 1") + require.NoError(t, err) + aInfo, didReadFromSystemTable, err = getAuthInfoFromCache() + require.NoError(t, err) + require.False(t, didReadFromSystemTable) + }) +} diff --git a/pkg/sql/sessioninit/main_test.go b/pkg/sql/sessioninit/main_test.go new file mode 100644 index 000000000000..597b8fd13429 --- /dev/null +++ b/pkg/sql/sessioninit/main_test.go @@ -0,0 +1,33 @@ +// Copyright 2015 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 sessioninit_test + +import ( + "os" + "testing" + + "github.com/cockroachdb/cockroach/pkg/security" + "github.com/cockroachdb/cockroach/pkg/security/securitytest" + "github.com/cockroachdb/cockroach/pkg/server" + "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" + "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" + "github.com/cockroachdb/cockroach/pkg/util/randutil" +) + +//go:generate ../../util/leaktest/add-leaktest.sh *_test.go + +func TestMain(m *testing.M) { + security.SetAssetLoader(securitytest.EmbeddedAssets) + randutil.SeedForTests() + serverutils.InitTestServerFactory(server.TestServerFactory) + serverutils.InitTestClusterFactory(testcluster.TestClusterFactory) + os.Exit(m.Run()) +} diff --git a/pkg/sql/ttl/ttljob/ttljob.go b/pkg/sql/ttl/ttljob/ttljob.go index 62741040081e..71eebc5f4095 100644 --- a/pkg/sql/ttl/ttljob/ttljob.go +++ b/pkg/sql/ttl/ttljob/ttljob.go @@ -255,7 +255,7 @@ var _ jobs.Resumer = (*rowLevelTTLResumer)(nil) func (t rowLevelTTLResumer) Resume(ctx context.Context, execCtx interface{}) error { p := execCtx.(sql.JobExecContext) db := p.ExecCfg().DB - descs := p.ExtendedEvalContext().Descs + descsCol := p.ExtendedEvalContext().Descs if enabled := jobEnabled.Get(p.ExecCfg().SV()); !enabled { return errors.Newf( @@ -291,7 +291,7 @@ func (t rowLevelTTLResumer) Resume(ctx context.Context, execCtx interface{}) err var name string var rangeSpan roachpb.Span if err := db.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error { - desc, err := descs.GetImmutableTableByID( + desc, err := descsCol.GetImmutableTableByID( ctx, txn, details.TableID, @@ -327,34 +327,11 @@ func (t rowLevelTTLResumer) Resume(ctx context.Context, execCtx interface{}) err return errors.Newf("ttl jobs on table %s are currently paused", tree.Name(desc.GetName())) } - _, dbDesc, err := descs.GetImmutableDatabaseByID( - ctx, - txn, - desc.GetParentID(), - tree.CommonLookupFlags{ - Required: true, - }, - ) + tn, err := descs.GetTableNameByDesc(ctx, txn, descsCol, desc) if err != nil { - return err - } - schemaDesc, err := descs.GetImmutableSchemaByID( - ctx, - txn, - desc.GetParentSchemaID(), - tree.CommonLookupFlags{ - Required: true, - }, - ) - if err != nil { - return err + return errors.Wrapf(err, "error fetching table name for TTL") } - tn := tree.MakeTableNameWithSchema( - tree.Name(dbDesc.GetName()), - tree.Name(schemaDesc.GetName()), - tree.Name(desc.GetName()), - ) name = tn.FQString() rangeSpan = desc.TableSpan(p.ExecCfg().Codec) ttlSettings = *ttl diff --git a/pkg/sql/ttl/ttlschedule/BUILD.bazel b/pkg/sql/ttl/ttlschedule/BUILD.bazel index 7c130ad7e8e4..953b2c21317b 100644 --- a/pkg/sql/ttl/ttlschedule/BUILD.bazel +++ b/pkg/sql/ttl/ttlschedule/BUILD.bazel @@ -13,9 +13,11 @@ go_library( "//pkg/security", "//pkg/sql", "//pkg/sql/catalog/catpb", + "//pkg/sql/catalog/descs", "//pkg/sql/pgwire/pgcode", "//pkg/sql/pgwire/pgerror", "//pkg/sql/sem/tree", + "//pkg/sql/sqlerrors", "//pkg/sql/sqlutil", "//pkg/util/metric", "//pkg/util/timeutil", diff --git a/pkg/sql/ttl/ttlschedule/ttlschedule.go b/pkg/sql/ttl/ttlschedule/ttlschedule.go index 476df185d952..d801e90db1b1 100644 --- a/pkg/sql/ttl/ttlschedule/ttlschedule.go +++ b/pkg/sql/ttl/ttlschedule/ttlschedule.go @@ -21,9 +21,11 @@ import ( "github.com/cockroachdb/cockroach/pkg/security" "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/cockroach/pkg/sql/sqlerrors" "github.com/cockroachdb/cockroach/pkg/sql/sqlutil" "github.com/cockroachdb/cockroach/pkg/util/metric" "github.com/cockroachdb/cockroach/pkg/util/timeutil" @@ -38,7 +40,6 @@ type rowLevelTTLExecutor struct { var _ jobs.ScheduledJobController = (*rowLevelTTLExecutor)(nil) type rowLevelTTLMetrics struct { - // TODO(#75189): add more useful metrics here *jobs.ExecutorMetrics } @@ -54,14 +55,68 @@ func (s rowLevelTTLExecutor) OnDrop( env scheduledjobs.JobSchedulerEnv, schedule *jobs.ScheduledJob, txn *kv.Txn, + descsCol *descs.Collection, ) error { - return errors.WithHint( - pgerror.Newf( - pgcode.InvalidTableDefinition, - "cannot drop row level TTL schedule", - ), - `use ALTER TABLE ... RESET (expire_after) instead`, - ) + + var args catpb.ScheduledRowLevelTTLArgs + if err := pbtypes.UnmarshalAny(schedule.ExecutionArgs().Args, &args); err != nil { + return err + } + + canDrop, err := canDropTTLSchedule(ctx, txn, descsCol, schedule, args) + if err != nil { + return err + } + + if !canDrop { + tn, err := descs.GetTableNameByID(ctx, txn, descsCol, args.TableID) + if err != nil { + return err + } + f := tree.NewFmtCtx(tree.FmtSimple) + tn.Format(f) + return errors.WithHintf( + pgerror.Newf( + pgcode.InvalidTableDefinition, + "cannot drop a row level TTL schedule", + ), + `use ALTER TABLE %s RESET (ttl) instead`, + f.CloseAndGetString(), + ) + } + return nil +} + +// canDropTTLSchedule determines whether we can drop a given row-level TTL +// schedule. This is intended to only be permitted for schedules which are not +// valid. +func canDropTTLSchedule( + ctx context.Context, + txn *kv.Txn, + descsCol *descs.Collection, + schedule *jobs.ScheduledJob, + args catpb.ScheduledRowLevelTTLArgs, +) (bool, error) { + desc, err := descsCol.GetImmutableTableByID(ctx, txn, args.TableID, tree.ObjectLookupFlags{}) + if err != nil { + // If the descriptor does not exist we can drop this schedule. + if sqlerrors.IsUndefinedRelationError(err) { + return true, nil + } + return false, err + } + if desc == nil { + return true, nil + } + // If there is no row-level TTL on the table we can drop this schedule. + if !desc.HasRowLevelTTL() { + return true, nil + } + // If there is a schedule id mismatch we can drop this schedule. + if desc.GetRowLevelTTL().ScheduleID != schedule.ScheduleID() { + return true, nil + } + return false, nil } // ExecuteJob implements the jobs.ScheduledJobController interface. diff --git a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionsPage.tsx index bd3adc7d2dc0..2c5dd1b56730 100644 --- a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionsPage.tsx @@ -109,7 +109,7 @@ function getSessionAppFilterOptions(sessions: SessionInfo[]): string[] { ), ); - return Array.from(uniqueAppNames); + return Array.from(uniqueAppNames).sort(); } export class SessionsPage extends React.Component< diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts index ef420ab3e130..4a8eea6d07e6 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts @@ -81,7 +81,9 @@ export const selectApps = createSelector(sqlStatsSelector, sqlStatsState => { } }, ); - return [].concat(sawBlank ? ["(unset)"] : []).concat(Object.keys(apps)); + return [] + .concat(sawBlank ? ["(unset)"] : []) + .concat(Object.keys(apps).sort()); }); // selectDatabases returns the array of all databases with statement statistics present @@ -99,7 +101,9 @@ export const selectDatabases = createSelector( s.key.key_data.database ? s.key.key_data.database : "(unset)", ), ), - ).filter((dbName: string) => dbName !== null && dbName.length > 0); + ) + .filter((dbName: string) => dbName !== null && dbName.length > 0) + .sort(); }, ); diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts index ab5ee29a4bc5..4c8d296cccf8 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts @@ -45,7 +45,7 @@ export const getTrxAppFilterOptions = ( .map(t => (t.stats_data.app ? t.stats_data.app : "(unset)")), ); - return Array.from(uniqueAppNames); + return Array.from(uniqueAppNames).sort(); }; export const collectStatementsText = (statements: Statement[]): string => diff --git a/pkg/ui/workspaces/db-console/src/views/statements/statements.spec.tsx b/pkg/ui/workspaces/db-console/src/views/statements/statements.spec.tsx index 3b4e28ee0082..a0641bbc00b2 100644 --- a/pkg/ui/workspaces/db-console/src/views/statements/statements.spec.tsx +++ b/pkg/ui/workspaces/db-console/src/views/statements/statements.spec.tsx @@ -214,7 +214,7 @@ describe("selectApps", () => { const result = selectApps(state); - assert.deepEqual(result, ["(unset)", "foobar", "cockroach sql"]); + assert.deepEqual(result, ["(unset)", "cockroach sql", "foobar"]); }); }); diff --git a/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx b/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx index 3312d5836034..412f2aab9bed 100644 --- a/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx +++ b/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx @@ -188,7 +188,10 @@ export const selectApps = createSelector( } }, ); - return [].concat(sawBlank ? ["(unset)"] : []).concat(Object.keys(apps)); + return [] + .concat(sawBlank ? ["(unset)"] : []) + .concat(Object.keys(apps)) + .sort(); }, ); @@ -206,7 +209,9 @@ export const selectDatabases = createSelector( s.key.key_data.database ? s.key.key_data.database : "(unset)", ), ), - ).filter((dbName: string) => dbName !== null && dbName.length > 0); + ) + .filter((dbName: string) => dbName !== null && dbName.length > 0) + .sort(); }, );