diff --git a/docs/generated/settings/settings-for-tenants.txt b/docs/generated/settings/settings-for-tenants.txt index be3464124cb1..a72454410acc 100644 --- a/docs/generated/settings/settings-for-tenants.txt +++ b/docs/generated/settings/settings-for-tenants.txt @@ -299,4 +299,4 @@ trace.opentelemetry.collector string address of an OpenTelemetry trace collecto trace.snapshot.rate duration 0s if non-zero, interval at which background trace snapshots are captured 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-78 set the active cluster version in the format '.' +version version 1000022.2-80 set the active cluster version in the format '.' diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html index 4b2960fc0186..1aa4da7f7e7e 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -240,6 +240,6 @@
trace.snapshot.rate
duration0sif non-zero, interval at which background trace snapshots are captured
trace.span_registry.enabled
booleantrueif set, ongoing traces can be seen at https://<ui>/#/debug/tracez
trace.zipkin.collector
stringthe address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used. -
version
version1000022.2-78set the active cluster version in the format '<major>.<minor>' +
version
version1000022.2-80set the active cluster version in the format '<major>.<minor>' diff --git a/pkg/cli/testdata/declarative-rules/deprules b/pkg/cli/testdata/declarative-rules/deprules index 474a59c6a24a..c9e4be873845 100644 --- a/pkg/cli/testdata/declarative-rules/deprules +++ b/pkg/cli/testdata/declarative-rules/deprules @@ -1,6 +1,6 @@ dep ---- -debug declarative-print-rules 1000022.2-78 dep +debug declarative-print-rules 1000022.2-80 dep deprules ---- - name: 'CheckConstraint transitions to ABSENT uphold 2-version invariant: PUBLIC->VALIDATED' diff --git a/pkg/cli/testdata/declarative-rules/oprules b/pkg/cli/testdata/declarative-rules/oprules index 7c72140937fd..01262e55df2f 100644 --- a/pkg/cli/testdata/declarative-rules/oprules +++ b/pkg/cli/testdata/declarative-rules/oprules @@ -1,6 +1,6 @@ op ---- -debug declarative-print-rules 1000022.2-78 op +debug declarative-print-rules 1000022.2-80 op rules ---- [] diff --git a/pkg/clusterversion/cockroach_versions.go b/pkg/clusterversion/cockroach_versions.go index ad623e6110ab..a7529e5aa7e7 100644 --- a/pkg/clusterversion/cockroach_versions.go +++ b/pkg/clusterversion/cockroach_versions.go @@ -479,6 +479,11 @@ const ( // since we've made mixed-version clusters tolerate new privileges. V23_1AllowNewSystemPrivileges + // V23_1JobInfoTableIsBackfilled is a version gate after which the + // system.jobs_info table has been backfilled with rows for the payload and + // progress of each job in the system.jobs table. + V23_1JobInfoTableIsBackfilled + // ************************************************* // Step (1): Add new versions here. // Do not add new versions to a patch release. @@ -828,6 +833,10 @@ var rawVersionsSingleton = keyedVersions{ Key: V23_1AllowNewSystemPrivileges, Version: roachpb.Version{Major: 22, Minor: 2, Internal: 78}, }, + { + Key: V23_1JobInfoTableIsBackfilled, + Version: roachpb.Version{Major: 22, Minor: 2, Internal: 80}, + }, // ************************************************* // Step (2): Add new versions here. diff --git a/pkg/cmd/roachtest/tests/restore.go b/pkg/cmd/roachtest/tests/restore.go index 9b318ba4ddd0..13bfe8b012bf 100644 --- a/pkg/cmd/roachtest/tests/restore.go +++ b/pkg/cmd/roachtest/tests/restore.go @@ -39,7 +39,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/util/httputil" "github.com/cockroachdb/cockroach/pkg/util/humanizeutil" "github.com/cockroachdb/cockroach/pkg/util/log" - "github.com/cockroachdb/cockroach/pkg/util/retry" "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/cockroachdb/cockroach/pkg/workload/histogram" "github.com/cockroachdb/errors" @@ -235,80 +234,37 @@ func (dul *DiskUsageLogger) Runner(ctx context.Context) error { } } func registerRestoreNodeShutdown(r registry.Registry) { + sp := restoreSpecs{ + hardware: makeHardwareSpecs(hardwareSpecs{}), + backup: makeBackupSpecs( + backupSpecs{workload: tpceRestore{customers: 5000}, + version: "v22.2.1"}), + timeout: 1 * time.Hour, + } + makeRestoreStarter := func(ctx context.Context, t test.Test, c cluster.Cluster, gatewayNode int) jobStarter { return func(c cluster.Cluster, t test.Test) (string, error) { - t.L().Printf("connecting to gateway") - gatewayDB := c.Conn(ctx, t.L(), gatewayNode) - defer gatewayDB.Close() - - t.L().Printf("creating bank database") - if _, err := gatewayDB.Exec("CREATE DATABASE bank"); err != nil { - return "", err - } - - errCh := make(chan error, 1) - go func() { - defer close(errCh) - - // 10 GiB restore. - restoreQuery := `RESTORE bank.bank FROM - 'gs://cockroach-fixtures/workload/bank/version=1.0.0,payload-bytes=100,ranges=10,rows=10000000,seed=1/bank?AUTH=implicit'` - - t.L().Printf("starting to run the restore job") - if _, err := gatewayDB.Exec(restoreQuery); err != nil { - errCh <- err - } - t.L().Printf("done running restore job") - }() - - // Wait for the job. - retryOpts := retry.Options{ - MaxRetries: 50, - InitialBackoff: 1 * time.Second, - MaxBackoff: 5 * time.Second, - } - for r := retry.StartWithCtx(ctx, retryOpts); r.Next(); { - var jobCount int - if err := gatewayDB.QueryRowContext(ctx, "SELECT count(*) FROM [SHOW JOBS] WHERE job_type = 'RESTORE'").Scan(&jobCount); err != nil { - return "", err - } - - select { - case err := <-errCh: - // We got an error when starting the job. - return "", err - default: - } - - if jobCount == 0 { - t.L().Printf("waiting for restore job") - } else if jobCount == 1 { - t.L().Printf("found restore job") - break - } else { - t.L().Printf("found multiple restore jobs -- erroring") - return "", errors.New("unexpectedly found multiple restore jobs") - } - } - - var jobID string - if err := gatewayDB.QueryRowContext(ctx, "SELECT job_id FROM [SHOW JOBS] WHERE job_type = 'RESTORE'").Scan(&jobID); err != nil { - return "", errors.Wrap(err, "querying the job ID") - } - return jobID, nil + sp.getRuntimeSpecs(ctx, t, c) + jobID, err := sp.runDetached(ctx, "DATABASE tpce", gatewayNode) + return fmt.Sprintf("%d", jobID), err } } r.Add(registry.TestSpec{ Name: "restore/nodeShutdown/worker", Owner: registry.OwnerDisasterRecovery, - Cluster: r.MakeClusterSpec(4), + Cluster: sp.hardware.makeClusterSpecs(r), + Timeout: sp.timeout, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { gatewayNode := 2 nodeToShutdown := 3 + + if c.Spec().Cloud != sp.backup.cloud { + // For now, only run the test on the cloud provider that also stores the backup. + t.Skip("test configured to run on %s", sp.backup.cloud) + } c.Put(ctx, t.Cockroach(), "./cockroach") c.Start(ctx, t.L(), option.DefaultStartOptsNoBackups(), install.MakeClusterSettings()) - jobSurvivesNodeShutdown(ctx, t, c, nodeToShutdown, makeRestoreStarter(ctx, t, c, gatewayNode)) }, }) @@ -316,10 +272,18 @@ func registerRestoreNodeShutdown(r registry.Registry) { r.Add(registry.TestSpec{ Name: "restore/nodeShutdown/coordinator", Owner: registry.OwnerDisasterRecovery, - Cluster: r.MakeClusterSpec(4), + Cluster: sp.hardware.makeClusterSpecs(r), + Timeout: sp.timeout, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { + gatewayNode := 2 nodeToShutdown := 2 + + if c.Spec().Cloud != sp.backup.cloud { + // For now, only run the test on the cloud provider that also stores the backup. + t.Skip("test configured to run on %s", sp.backup.cloud) + } + c.Put(ctx, t.Cockroach(), "./cockroach") c.Start(ctx, t.L(), option.DefaultStartOptsNoBackups(), install.MakeClusterSettings()) @@ -446,7 +410,7 @@ func registerRestore(r registry.Registry) { defer close(jobIDCh) t.Status(`running restore`) metricCollector := withPauseSpecs.initRestorePerfMetrics(ctx, durationGauge) - jobID, err := withPauseSpecs.runDetached(ctx, "DATABASE tpce") + jobID, err := withPauseSpecs.runDetached(ctx, "DATABASE tpce", 1) require.NoError(t, err) jobIDCh <- jobID @@ -840,14 +804,16 @@ func (sp *restoreSpecs) run(ctx context.Context, target string) error { return sp.c.RunE(ctx, sp.c.Node(1), sp.restoreCmd(target, "")) } -func (sp *restoreSpecs) runDetached(ctx context.Context, target string) (jobspb.JobID, error) { - if err := sp.c.RunE(ctx, sp.c.Node(1), sp.restoreCmd(target, "WITH DETACHED")); err != nil { +func (sp *restoreSpecs) runDetached( + ctx context.Context, target string, node int, +) (jobspb.JobID, error) { + if err := sp.c.RunE(ctx, sp.c.Node(node), sp.restoreCmd(target, "WITH DETACHED")); err != nil { return 0, err } - db, err := sp.c.ConnE(ctx, sp.t.L(), sp.c.Node(1)[0]) + db, err := sp.c.ConnE(ctx, sp.t.L(), sp.c.Node(node)[0]) if err != nil { - return 0, errors.Wrap(err, "failed to connect to node 1; running restore detached") + return 0, errors.Wrapf(err, "failed to connect to node %d; running restore detached", node) } var jobID jobspb.JobID if err := db.QueryRow(`SELECT job_id FROM [SHOW JOBS] WHERE job_type = 'RESTORE'`).Scan(&jobID); err != nil { diff --git a/pkg/jobs/job_info_storage.go b/pkg/jobs/job_info_storage.go index 918d773eef65..d6f6aef58ef0 100644 --- a/pkg/jobs/job_info_storage.go +++ b/pkg/jobs/job_info_storage.go @@ -109,7 +109,7 @@ func (i InfoStorage) write(ctx context.Context, infoKey, value []byte) error { // First clear out any older revisions of this info. _, err := i.txn.ExecEx( - ctx, "job-info-write", i.txn.KV(), + ctx, "write-job-info-delete", i.txn.KV(), sessiondata.NodeUserSessionDataOverride, "DELETE FROM system.job_info WHERE job_id = $1 AND info_key = $2", j.ID(), infoKey, @@ -120,7 +120,7 @@ func (i InfoStorage) write(ctx context.Context, infoKey, value []byte) error { // Write the new info, using the same transaction. _, err = i.txn.ExecEx( - ctx, "job-info-write", i.txn.KV(), + ctx, "write-job-info-insert", i.txn.KV(), sessiondata.NodeUserSessionDataOverride, `INSERT INTO system.job_info (job_id, info_key, written, value) VALUES ($1, $2, now(), $3)`, j.ID(), infoKey, value, @@ -206,6 +206,18 @@ const ( legacyProgressKey = "legacy_progress" ) +// GetLegacyPayloadKey returns the info_key whose value is the jobspb.Payload of +// the job. +func GetLegacyPayloadKey() []byte { + return []byte(legacyPayloadKey) +} + +// GetLegacyProgressKey returns the info_key whose value is the jobspb.Progress +// of the job. +func GetLegacyProgressKey() []byte { + return []byte(legacyProgressKey) +} + // GetLegacyPayload returns the job's Payload from the system.jobs_info table. func (i InfoStorage) GetLegacyPayload(ctx context.Context) ([]byte, bool, error) { return i.Get(ctx, []byte(legacyPayloadKey)) diff --git a/pkg/ui/workspaces/cluster-ui/src/api/stmtInsightsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/stmtInsightsApi.ts index 2bdf2a885406..e605a1390432 100644 --- a/pkg/ui/workspaces/cluster-ui/src/api/stmtInsightsApi.ts +++ b/pkg/ui/workspaces/cluster-ui/src/api/stmtInsightsApi.ts @@ -22,6 +22,7 @@ import { ContentionDetails, getInsightsFromProblemsAndCauses, InsightExecEnum, + StatementStatus, StmtInsightEvent, } from "src/insights"; import moment from "moment"; @@ -63,6 +64,8 @@ export type StmtInsightsResponseRow = { index_recommendations: string[]; plan_gist: string; cpu_sql_nanos: number; + error_code: string; + status: StatementStatus; }; const stmtColumns = ` @@ -90,7 +93,9 @@ causes, problem, index_recommendations, plan_gist, -cpu_sql_nanos +cpu_sql_nanos, +error_code, +status `; const stmtInsightsOverviewQuery = (filters?: StmtInsightsReq): string => { @@ -232,6 +237,8 @@ export function formatStmtInsights( ), planGist: row.plan_gist, cpuSQLNanos: row.cpu_sql_nanos, + errorCode: row.error_code, + status: row.status, } as StmtInsightEvent; }); } diff --git a/pkg/ui/workspaces/cluster-ui/src/api/txnInsightsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/txnInsightsApi.ts index 445f1dcbe5c9..93153be881b1 100644 --- a/pkg/ui/workspaces/cluster-ui/src/api/txnInsightsApi.ts +++ b/pkg/ui/workspaces/cluster-ui/src/api/txnInsightsApi.ts @@ -26,6 +26,7 @@ import { getInsightsFromProblemsAndCauses, InsightExecEnum, InsightNameEnum, + TransactionStatus, TxnContentionInsightDetails, TxnInsightDetails, TxnInsightEvent, @@ -293,6 +294,8 @@ type TxnInsightsResponseRow = { causes: string[]; stmt_execution_ids: string[]; cpu_sql_nanos: number; + last_error_code: string; + status: TransactionStatus; }; type TxnQueryFilters = { @@ -326,7 +329,9 @@ last_retry_reason, problems, causes, stmt_execution_ids, -cpu_sql_nanos`; +cpu_sql_nanos, +last_error_code, +status`; if (filters?.execID) { return ` @@ -394,6 +399,8 @@ function formatTxnInsightsRow(row: TxnInsightsResponseRow): TxnInsightEvent { insights, stmtExecutionIDs: row.stmt_execution_ids, cpuSQLNanos: row.cpu_sql_nanos, + errorCode: row.last_error_code, + status: row.status, }; } diff --git a/pkg/ui/workspaces/cluster-ui/src/indexDetailsPage/indexDetailsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/indexDetailsPage/indexDetailsPage.tsx index 5b2ab2c28fc9..c365a129397a 100644 --- a/pkg/ui/workspaces/cluster-ui/src/indexDetailsPage/indexDetailsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/indexDetailsPage/indexDetailsPage.tsx @@ -10,6 +10,7 @@ import React from "react"; import classNames from "classnames/bind"; +import { flatMap } from "lodash"; import { ISortedTablePagination, SortedTable, @@ -462,7 +463,9 @@ export class IndexDetailsPage extends React.Component< .map(n => Number(n)) .sort(); const regions = unique( - nodes.map(node => nodeRegions[node.toString()]), + isTenant + ? flatMap(statements, statement => statement.stats.regions) + : nodes.map(node => nodeRegions[node.toString()]), ).sort(); const filteredStmts = this.filteredStatements(); diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/types.ts b/pkg/ui/workspaces/cluster-ui/src/insights/types.ts index 620db1424abc..64fc4ac6584f 100644 --- a/pkg/ui/workspaces/cluster-ui/src/insights/types.ts +++ b/pkg/ui/workspaces/cluster-ui/src/insights/types.ts @@ -26,6 +26,18 @@ export enum InsightExecEnum { STATEMENT = "statement", } +export enum StatementStatus { + COMPLETED = "Completed", + FAILED = "Failed", +} + +export enum TransactionStatus { + COMPLETED = "Completed", + FAILED = "Failed", + // Unimplemented, see https://github.com/cockroachdb/cockroach/issues/98219/. + CANCELLED = "Cancelled", +} + // Common fields for both txn and stmt insights. export type InsightEventBase = { application: string; @@ -46,9 +58,11 @@ export type InsightEventBase = { transactionExecutionID: string; transactionFingerprintID: string; username: string; + errorCode: string; }; export type TxnInsightEvent = InsightEventBase & { + status: TransactionStatus; stmtExecutionIDs: string[]; }; @@ -99,6 +113,7 @@ export type StmtInsightEvent = InsightEventBase & { planGist: string; databaseName: string; execType?: InsightExecEnum; + status: StatementStatus; }; export type Insight = { @@ -327,6 +342,8 @@ export interface ExecutionDetails { statementExecutionID?: string; transactionExecutionID?: string; execType?: InsightExecEnum; + errorCode?: string; + status?: string; } export interface insightDetails { diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/utils.spec.ts b/pkg/ui/workspaces/cluster-ui/src/insights/utils.spec.ts index bb8c55a19eaf..63d5a20703ba 100644 --- a/pkg/ui/workspaces/cluster-ui/src/insights/utils.spec.ts +++ b/pkg/ui/workspaces/cluster-ui/src/insights/utils.spec.ts @@ -26,8 +26,10 @@ import { InsightNameEnum, planRegressionInsight, slowExecutionInsight, + StatementStatus, StmtInsightEvent, suboptimalPlanInsight, + TransactionStatus, TxnInsightDetails, TxnInsightEvent, } from "./types"; @@ -76,6 +78,8 @@ const statementInsightMock: StmtInsightEvent = { indexRecommendations: [], planGist: "gist", cpuSQLNanos: 50, + errorCode: "", + status: StatementStatus.COMPLETED, }; function mockStmtInsightEvent( @@ -116,6 +120,8 @@ const txnInsightEventMock: TxnInsightEvent = { elapsedTimeMillis: 1, stmtExecutionIDs: [statementInsightMock.statementExecutionID], cpuSQLNanos: 50, + errorCode: "", + status: TransactionStatus.COMPLETED, }; function mockTxnInsightEvent( diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/utils.ts b/pkg/ui/workspaces/cluster-ui/src/insights/utils.ts index e4ee7e3b4aac..ab26aadc11a6 100644 --- a/pkg/ui/workspaces/cluster-ui/src/insights/utils.ts +++ b/pkg/ui/workspaces/cluster-ui/src/insights/utils.ts @@ -392,6 +392,8 @@ export function getStmtInsightRecommendations( statementExecutionID: insightDetails.statementExecutionID, transactionExecutionID: insightDetails.transactionExecutionID, execType: InsightExecEnum.STATEMENT, + errorCode: insightDetails.errorCode, + status: insightDetails.status, }; const recs: InsightRecommendation[] = insightDetails.insights?.map(insight => @@ -412,6 +414,7 @@ export function getTxnInsightRecommendations( contentionTimeMs: insightDetails.contentionTime.asMilliseconds(), elapsedTimeMillis: insightDetails.elapsedTimeMillis, execType: InsightExecEnum.TRANSACTION, + errorCode: insightDetails.errorCode, }; const recs: InsightRecommendation[] = []; diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsights/statementInsights/statementInsightsTable.tsx b/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsights/statementInsights/statementInsightsTable.tsx index 35a7d6d206aa..e0b6130788e2 100644 --- a/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsights/statementInsights/statementInsightsTable.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsights/statementInsights/statementInsightsTable.tsx @@ -22,7 +22,11 @@ import { limitText, NO_SAMPLES_FOUND, } from "src/util"; -import { InsightExecEnum, StmtInsightEvent } from "src/insights"; +import { + InsightExecEnum, + StatementStatus, + StmtInsightEvent, +} from "src/insights"; import { InsightCell, insightsTableTitles, @@ -33,9 +37,21 @@ import { Link } from "react-router-dom"; import classNames from "classnames/bind"; import styles from "../util/workloadInsights.module.scss"; import { TimeScale } from "../../../timeScaleDropdown"; +import { Badge } from "src/badge"; const cx = classNames.bind(styles); +function stmtStatusToString(status: StatementStatus) { + switch (status) { + case StatementStatus.COMPLETED: + return "success"; + case StatementStatus.FAILED: + return "danger"; + default: + return "info"; + } +} + interface StatementInsightsTable { data: StmtInsightEvent[]; sortSetting: SortSetting; @@ -77,6 +93,15 @@ export function makeStatementInsightsColumns(): ColumnDescriptor item.query, showByDefault: true, }, + { + name: "status", + title: insightsTableTitles.status(execType), + cell: (item: StmtInsightEvent) => ( + + ), + sort: (item: StmtInsightEvent) => item.status, + showByDefault: true, + }, { name: "insights", title: insightsTableTitles.insights(execType), diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsights/transactionInsights/transactionInsightsTable.tsx b/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsights/transactionInsights/transactionInsightsTable.tsx index 2d031945a7ca..d939bc07fed0 100644 --- a/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsights/transactionInsights/transactionInsightsTable.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsights/transactionInsights/transactionInsightsTable.tsx @@ -16,7 +16,11 @@ import { SortSetting, } from "src/sortedtable"; import { DATE_WITH_SECONDS_AND_MILLISECONDS_FORMAT, Duration } from "src/util"; -import { InsightExecEnum, TxnInsightEvent } from "src/insights"; +import { + InsightExecEnum, + TransactionStatus, + TxnInsightEvent, +} from "src/insights"; import { InsightCell, insightsTableTitles, @@ -25,6 +29,18 @@ import { } from "../util"; import { Link } from "react-router-dom"; import { TimeScale } from "../../../timeScaleDropdown"; +import { Badge } from "src/badge"; + +function txnStatusToString(status: TransactionStatus) { + switch (status) { + case TransactionStatus.COMPLETED: + return "success"; + case TransactionStatus.FAILED: + return "danger"; + case TransactionStatus.CANCELLED: + return "info"; + } +} interface TransactionInsightsTable { data: TxnInsightEvent[]; @@ -60,6 +76,15 @@ export function makeTransactionInsightsColumns(): ColumnDescriptor QueriesCell([item.query], 50), sort: item => item.query, }, + { + name: "status", + title: insightsTableTitles.status(execType), + cell: item => ( + + ), + sort: item => item.status, + showByDefault: true, + }, { name: "insights", title: insightsTableTitles.insights(execType), diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsights/util/insightsColumns.tsx b/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsights/util/insightsColumns.tsx index d6bf4f7f3625..a71f009712ee 100644 --- a/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsights/util/insightsColumns.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsights/util/insightsColumns.tsx @@ -21,6 +21,7 @@ export const insightsColumnLabels = { waitingID: "Waiting Execution ID", waitingFingerprintID: "Waiting Fingerprint ID", query: "Execution", + status: "Status", insights: "Insights", startTime: "Start Time (UTC)", elapsedTime: "Elapsed Time", @@ -123,6 +124,10 @@ export const insightsTableTitles: InsightsTableTitleType = { } return makeToolTip(

{tooltipText}

, "query", execType); }, + status: (execType: InsightExecEnum) => { + const tooltipText = `The ${execType} status`; + return makeToolTip(

{tooltipText}

, "status"); + }, insights: (execType: InsightExecEnum) => { return makeToolTip(

The category of insight identified for the {execType} execution.

, diff --git a/pkg/ui/workspaces/cluster-ui/src/insightsTable/insightsTable.module.scss b/pkg/ui/workspaces/cluster-ui/src/insightsTable/insightsTable.module.scss index 2e8970417605..14d1d9f5044b 100644 --- a/pkg/ui/workspaces/cluster-ui/src/insightsTable/insightsTable.module.scss +++ b/pkg/ui/workspaces/cluster-ui/src/insightsTable/insightsTable.module.scss @@ -5,6 +5,11 @@ font-weight: $font-weight--bold; } +.insights-type-failed { + color: $colors--functional-red-4; + font-weight: $font-weight--bold; +} + .label-bold { font-family: $font-family--semi-bold; font-size: $font-size--medium; diff --git a/pkg/ui/workspaces/cluster-ui/src/insightsTable/insightsTable.tsx b/pkg/ui/workspaces/cluster-ui/src/insightsTable/insightsTable.tsx index 4b3841c7db82..8df6cd27a9c3 100644 --- a/pkg/ui/workspaces/cluster-ui/src/insightsTable/insightsTable.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/insightsTable/insightsTable.tsx @@ -31,6 +31,7 @@ import { Link } from "react-router-dom"; import { InsightExecEnum, InsightRecommendation, + InsightType, insightType, } from "../insights"; @@ -93,8 +94,12 @@ export const insightsTableTitles: InsightsTableTitleType = { }, }; -function typeCell(value: string): React.ReactElement { - return
{value}
; +function TypeCell(value: InsightType): React.ReactElement { + const className = + insightType(value) === "FailedExecution" + ? "insight-type-failed" + : "insight-type"; + return
{value}
; } const StatementExecution = ({ @@ -276,7 +281,8 @@ function descriptionCell( return ( <>
- This execution has failed. + Error Code: {" "} + {insightRec.execution.errorCode}
); @@ -409,7 +415,7 @@ export function makeInsightsColumns( { name: "insights", title: insightsTableTitles.insights(), - cell: (item: InsightRecommendation) => typeCell(insightType(item.type)), + cell: (item: InsightRecommendation) => TypeCell(item.type), sort: (item: InsightRecommendation) => item.type, }, { diff --git a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx index 750d97df64e4..a4b82c830437 100644 --- a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx @@ -102,8 +102,8 @@ export class SessionDetails extends React.Component { terminateQueryRef: React.RefObject; componentDidMount(): void { - this.props.refreshNodes(); if (!this.props.isTenant) { + this.props.refreshNodes(); this.props.refreshNodesLiveness(); } this.props.refreshSessions(); diff --git a/pkg/ui/workspaces/cluster-ui/src/sqlActivity/util.tsx b/pkg/ui/workspaces/cluster-ui/src/sqlActivity/util.tsx index 80c132182394..5f0bb539cc4e 100644 --- a/pkg/ui/workspaces/cluster-ui/src/sqlActivity/util.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/sqlActivity/util.tsx @@ -61,14 +61,7 @@ export function filteredStatementsData( // list if the list is not empty. statement => regions.length == 0 || - (statement.stats.nodes && - containAny( - statement.stats.nodes.map( - node => nodeRegions[node.toString()], - regions, - ), - regions, - )), + statement.stats.regions?.some(region => regions.includes(region)), ) .filter( // The statement must contain at least one value from the selected nodes diff --git a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx index 29413afb5570..0688b4d5cf13 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx @@ -316,8 +316,8 @@ export class StatementDetails extends React.Component< ); } this.props.refreshUserSQLRoles(); - this.props.refreshNodes(); if (!this.props.isTenant) { + this.props.refreshNodes(); this.props.refreshNodesLiveness(); if (!this.props.hasViewActivityRedactedRole) { this.props.refreshStatementDiagnosticsRequests(); @@ -335,8 +335,8 @@ export class StatementDetails extends React.Component< this.refreshStatementDetails(); } - this.props.refreshNodes(); if (!this.props.isTenant) { + this.props.refreshNodes(); this.props.refreshNodesLiveness(); if (!this.props.hasViewActivityRedactedRole) { this.props.refreshStatementDiagnosticsRequests(); @@ -571,9 +571,9 @@ export class StatementDetails extends React.Component< (stats.nodes || []).map(node => node.toString()), ).sort(); const regions = unique( - (stats.nodes || []) - .map(node => nodeRegions[node.toString()]) - .filter(r => r), // Remove undefined / unknown regions. + isTenant + ? stats.regions || [] + : nodes.map(node => nodeRegions[node]).filter(r => r), // Remove undefined / unknown regions. ).sort(); const lastExec = diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx index 10576973d9d5..17c5c8dc081f 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx @@ -10,7 +10,7 @@ import React from "react"; import { RouteComponentProps } from "react-router-dom"; -import { isNil, merge } from "lodash"; +import { flatMap, isNil, merge } from "lodash"; import classNames from "classnames/bind"; import { getValidErrorsList, Loading } from "src/loading"; import { Delayed } from "src/delayed"; @@ -352,9 +352,11 @@ export class StatementsPage extends React.Component< this.refreshDatabases(); this.props.refreshUserSQLRoles(); - this.props.refreshNodes(); - if (!this.props.isTenant && !this.props.hasViewActivityRedactedRole) { - this.props.refreshStatementDiagnosticsRequests(); + if (!this.props.isTenant) { + this.props.refreshNodes(); + if (!this.props.hasViewActivityRedactedRole) { + this.props.refreshStatementDiagnosticsRequests(); + } } } @@ -392,9 +394,11 @@ export class StatementsPage extends React.Component< componentDidUpdate = (): void => { this.updateQueryParams(); - this.props.refreshNodes(); - if (!this.props.isTenant && !this.props.hasViewActivityRedactedRole) { - this.props.refreshStatementDiagnosticsRequests(); + if (!this.props.isTenant) { + this.props.refreshNodes(); + if (!this.props.hasViewActivityRedactedRole) { + this.props.refreshStatementDiagnosticsRequests(); + } } }; @@ -603,6 +607,7 @@ export class StatementsPage extends React.Component< onDiagnosticsModalOpen, apps, databases, + statements, search, isTenant, nodeRegions, @@ -614,7 +619,9 @@ export class StatementsPage extends React.Component< .sort(); const regions = unique( - nodes.map(node => nodeRegions[node.toString()]), + isTenant + ? flatMap(statements, statement => statement.stats.regions) + : nodes.map(node => nodeRegions[node.toString()]), ).sort(); const { filters, activeFilters } = this.state; diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.spec.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.spec.tsx deleted file mode 100644 index e4e9de24803c..000000000000 --- a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.spec.tsx +++ /dev/null @@ -1,59 +0,0 @@ -// 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. - -import { assert } from "chai"; -import Long from "long"; -import { - AggregateStatistics, - populateRegionNodeForStatements, -} from "./statementsTable"; - -describe("populateRegionNodeForStatements", () => { - function statementWithNodeIDs(...nodeIDs: number[]): AggregateStatistics { - return { - aggregatedFingerprintID: "", - aggregatedFingerprintHexID: "", - label: "", - summary: "", - aggregatedTs: 0, - implicitTxn: false, - fullScan: false, - database: "", - applicationName: "", - stats: { nodes: nodeIDs.map(id => Long.fromInt(id)) }, - }; - } - - it("maps nodes to regions, sorted", () => { - const statement = statementWithNodeIDs(1, 2); - populateRegionNodeForStatements([statement], { - "1": "gcp-us-west1", - "2": "gcp-us-east1", - }); - assert.deepEqual(["gcp-us-east1", "gcp-us-west1"], statement.regions); - }); - - it("handles statements without nodes", () => { - const statement = statementWithNodeIDs(); - populateRegionNodeForStatements([statement], { - "1": "gcp-us-west1", - "2": "gcp-us-east1", - }); - assert.deepEqual(statement.regions, []); - }); - - it("excludes nodes whose region we don't know", () => { - const statement = statementWithNodeIDs(1, 2); - populateRegionNodeForStatements([statement], { - "1": "gcp-us-west1", - }); - assert.deepEqual(statement.regions, ["gcp-us-west1"]); - }); -}); diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx index adb6adbf5e0c..8964473ce5c2 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx @@ -73,7 +73,6 @@ export interface AggregateStatistics { diagnosticsReports?: StatementDiagnosticsReport[]; // totalWorkload is the sum of service latency of all statements listed on the table. totalWorkload?: Long; - regions?: string[]; regionNodes?: string[]; } @@ -356,9 +355,9 @@ function makeRegionsColumn( title: statisticsTableTitles.regions(statType), className: cx("statements-table__col-regions"), cell: (stmt: AggregateStatistics) => { - return longListWithTooltip(stmt.regions.sort().join(", "), 50); + return longListWithTooltip(stmt.stats.regions.sort().join(", "), 50); }, - sort: (stmt: AggregateStatistics) => stmt.regions.sort().join(", "), + sort: (stmt: AggregateStatistics) => stmt.stats.regions.sort().join(", "), }; } else { return { @@ -417,7 +416,6 @@ export function populateRegionNodeForStatements( ")", ); }); - stmt.regions = Object.keys(regions).sort(); stmt.regionNodes = regionNodes; }); } diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx index 5c2982033a45..3385d03c8e27 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx @@ -260,7 +260,9 @@ export class TransactionDetails extends React.Component< ); } this.props.refreshUserSQLRoles(); - this.props.refreshNodes(); + if (!this.props.isTenant) { + this.props.refreshNodes(); + } } componentWillUnmount(): void { @@ -269,7 +271,9 @@ export class TransactionDetails extends React.Component< componentDidUpdate(prevProps: TransactionDetailsProps): void { this.getTransactionStateInfo(prevProps.transactionFingerprintId); - this.props.refreshNodes(); + if (!this.props.isTenant) { + this.props.refreshNodes(); + } } onChangeSortSetting = (ss: SortSetting): void => { diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactions.fixture.ts b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactions.fixture.ts index ec88fe77a466..96f096e82e43 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactions.fixture.ts +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactions.fixture.ts @@ -88,6 +88,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { stats: { count: Long.fromInt(557), nodes: [Long.fromNumber(1), Long.fromNumber(2)], + regions: ["gcp-us-east1"], first_attempt_count: Long.fromInt(557), max_retries: Long.fromInt(0), legacy_last_err: "", @@ -167,6 +168,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { stats: { count: Long.fromInt(70), nodes: [Long.fromNumber(1), Long.fromNumber(3)], + regions: ["gcp-us-east1", "gcp-us-west1"], first_attempt_count: Long.fromInt(70), max_retries: Long.fromInt(0), legacy_last_err: "", @@ -233,6 +235,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { stats: { count: Long.fromInt(1), nodes: [Long.fromNumber(1), Long.fromNumber(3)], + regions: ["gcp-us-east1", "gcp-us-west1"], first_attempt_count: Long.fromInt(1), max_retries: Long.fromInt(0), legacy_last_err: "", @@ -290,6 +293,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { stats: { count: Long.fromInt(280), nodes: [Long.fromNumber(3), Long.fromNumber(4)], + regions: ["gcp-us-west1", "gcp-europe-west1"], first_attempt_count: Long.fromInt(280), max_retries: Long.fromInt(0), legacy_last_err: "", @@ -391,6 +395,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { stats: { count: Long.fromInt(1), nodes: [Long.fromNumber(2), Long.fromNumber(4)], + regions: ["gcp-us-east1", "gcp-europe-west1"], first_attempt_count: Long.fromInt(1), max_retries: Long.fromInt(0), legacy_last_err: "", @@ -442,6 +447,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { stats: { count: Long.fromInt(1), nodes: [Long.fromNumber(1)], + regions: ["gcp-us-east1"], first_attempt_count: Long.fromInt(1), max_retries: Long.fromInt(0), legacy_last_err: "", @@ -482,6 +488,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { stats: { count: Long.fromInt(1), nodes: [Long.fromNumber(3), Long.fromNumber(4)], + regions: ["gcp-us-west1", "gcp-europe-west1"], first_attempt_count: Long.fromInt(1), max_retries: Long.fromInt(0), legacy_last_err: "", @@ -533,6 +540,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { stats: { count: Long.fromInt(24), nodes: [Long.fromNumber(2), Long.fromNumber(3)], + regions: ["gcp-us-east1", "gcp-us-west1"], first_attempt_count: Long.fromInt(24), max_retries: Long.fromInt(0), legacy_last_err: "", @@ -622,6 +630,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { stats: { count: Long.fromInt(141), nodes: [Long.fromNumber(1), Long.fromNumber(2), Long.fromNumber(3)], + regions: ["gcp-us-east1", "gcp-us-west1"], first_attempt_count: Long.fromInt(141), max_retries: Long.fromInt(0), legacy_last_err: "", @@ -740,6 +749,7 @@ export const data: cockroach.server.serverpb.IStatementsResponse = { Long.fromNumber(3), Long.fromNumber(4), ], + regions: ["gcp-us-east1", "gcp-us-west1", "gcp-europe-west1"], first_attempt_count: Long.fromInt(1), max_retries: Long.fromInt(0), legacy_last_err: "", diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx index d5102531cea4..f1495ca0ffdc 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx @@ -36,7 +36,7 @@ import { filterTransactions, } from "./utils"; import Long from "long"; -import { merge } from "lodash"; +import { flatMap, merge } from "lodash"; import { unique, syncHistory } from "src/util"; import { EmptyTransactionsPlaceholder } from "./emptyTransactionsPlaceholder"; import { Loading } from "../loading"; @@ -251,7 +251,10 @@ export class TransactionsPage extends React.Component< ); } - this.props.refreshNodes(); + if (!this.props.isTenant) { + this.props.refreshNodes(); + } + this.props.refreshUserSQLRoles(); } @@ -293,7 +296,9 @@ export class TransactionsPage extends React.Component< componentDidUpdate(): void { this.updateQueryParams(); - this.props.refreshNodes(); + if (!this.props.isTenant) { + this.props.refreshNodes(); + } } onChangeSortSetting = (ss: SortSetting): void => { @@ -425,7 +430,9 @@ export class TransactionsPage extends React.Component< .sort(); const regions = unique( - nodes.map(node => nodeRegions[node.toString()]), + isTenant + ? flatMap(statements, statement => statement.stats.regions) + : nodes.map(node => nodeRegions[node.toString()]), ).sort(); // We apply the search filters and app name filters prior to aggregating across Node IDs @@ -508,7 +515,7 @@ export class TransactionsPage extends React.Component< t => ({ stats_data: t.stats_data, node_id: t.node_id, - regions: generateRegion(t, statements, nodeRegions), + regions: generateRegion(t, statements), regionNodes: generateRegionNode(t, statements, nodeRegions), }), ); diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.spec.ts b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.spec.ts index 1056c9275dfd..b7a415707c89 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.spec.ts +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.spec.ts @@ -293,9 +293,7 @@ SELECT _`, }); describe("generateRegion", () => { - function transactionWithStatementFingerprintIDs( - ...ids: number[] - ): Transaction { + function transaction(...ids: number[]): Transaction { return { stats_data: { statement_fingerprint_ids: ids.map(id => Long.fromInt(id)), @@ -303,35 +301,18 @@ describe("generateRegion", () => { }; } - function statementWithFingerprintAndNodeIDs( - id: number, - ...nodeIDs: number[] - ): Statement { - return { - id: Long.fromInt(id), - stats: { nodes: nodeIDs.map(id => Long.fromInt(id)) }, - }; + function statement(id: number, ...regions: string[]): Statement { + return { id: Long.fromInt(id), stats: { regions } }; } it("gathers up the list of regions for the transaction, sorted", () => { assert.deepEqual( - generateRegion( - transactionWithStatementFingerprintIDs(42), - [statementWithFingerprintAndNodeIDs(42, 1, 2)], - { "1": "gcp-us-west1", "2": "gcp-us-east1" }, - ), - ["gcp-us-east1", "gcp-us-west1"], - ); - }); - - it("skips over nodes with unknown regions", () => { - assert.deepEqual( - generateRegion( - transactionWithStatementFingerprintIDs(42), - [statementWithFingerprintAndNodeIDs(42, 1, 2)], - { "1": "gcp-us-west1" }, - ), - ["gcp-us-west1"], + generateRegion(transaction(42, 43, 44), [ + statement(42, "gcp-us-west1", "gcp-us-east1"), + statement(43, "gcp-us-west1"), + statement(44, "gcp-us-central1"), + ]), + ["gcp-us-central1", "gcp-us-east1", "gcp-us-west1"], ); }); }); diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts index 01316df3552f..49459d7e099f 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts @@ -156,6 +156,7 @@ export const searchTransactionsData = ( ); }; +// TODO(todd): Remove unused nodeRegions parameter. export const filterTransactions = ( data: Transaction[], filters: Filters, @@ -210,31 +211,31 @@ export const filterTransactions = ( timeValue === "empty", ) .filter((t: Transaction) => { - // The transaction must contain at least one value of the nodes - // and regions list (if the list is not empty). - if (regions.length == 0 && nodes.length == 0) return true; - // If the cluster is a tenant cluster we don't care - // about nodes. - let foundRegion: boolean = regions.length == 0; - let foundNode: boolean = isTenant || nodes.length == 0; - - getStatementsByFingerprintId( + // The transaction must contain at least one value of the regions list + // (if the list is not empty). + if (regions.length == 0) return true; + + return getStatementsByFingerprintId( t.stats_data.statement_fingerprint_ids, statements, - ).some(stmt => { - stmt.stats.nodes && - stmt.stats.nodes.some(node => { - if (foundRegion || regions.includes(nodeRegions[node.toString()])) { - foundRegion = true; - } - if (foundNode || nodes.includes("n" + node)) { - foundNode = true; - } - if (foundNode && foundRegion) return true; - }); - }); + ).some(stmt => + stmt.stats.regions?.some(region => regions.includes(region)), + ); + }) + .filter((t: Transaction) => { + // The transaction must contain at least one value of the nodes list + // (if the list is not empty). + if (nodes.length == 0) return true; + + // If the cluster is a tenant cluster we don't care about nodes. + if (isTenant) return true; - return foundRegion && foundNode; + return getStatementsByFingerprintId( + t.stats_data.statement_fingerprint_ids, + statements, + ).some(stmt => + stmt.stats.nodes?.some(node => nodes.includes("n" + node)), + ); }); return { @@ -249,32 +250,21 @@ export const filterTransactions = ( * E.g. of one element of the list: `gcp-us-east1` * @param transaction: list of transactions. * @param statements: list of all statements collected. - * @param nodeRegions: object with keys being the node id and the value - * which region it belongs to. */ export const generateRegion = ( transaction: Transaction, statements: Statement[], - nodeRegions: { [p: string]: string }, ): string[] => { const regions: Set = new Set(); - // Get the list of statements that were executed on the transaction. Combine all - // nodes and regions of all the statements to a single list of `region: nodes` - // for the transaction. - // E.g. {"gcp-us-east1" : [1,3,4]} + getStatementsByFingerprintId( transaction.stats_data.statement_fingerprint_ids, statements, ).forEach(stmt => { - stmt.stats.nodes && - stmt.stats.nodes.forEach(n => { - regions.add(nodeRegions[n.toString()]); - }); + stmt.stats.regions?.forEach(region => regions.add(region)); }); - return Array.from(regions) - .filter(r => r) // Remove undefined / unknown regions. - .sort(); + return Array.from(regions).sort(); }; /** diff --git a/pkg/upgrade/upgrademanager/manager.go b/pkg/upgrade/upgrademanager/manager.go index 2a6d601b3ce1..4c3afba6010a 100644 --- a/pkg/upgrade/upgrademanager/manager.go +++ b/pkg/upgrade/upgrademanager/manager.go @@ -550,6 +550,7 @@ func (m *Manager) runMigration( // The TenantDeps used here are incomplete, but enough for the "permanent // upgrades" that run under this testing knob. if err := upg.Run(ctx, mig.Version(), upgrade.TenantDeps{ + KVDB: m.deps.DB.KV(), DB: m.deps.DB, Codec: m.codec, Settings: m.settings, diff --git a/pkg/upgrade/upgrades/BUILD.bazel b/pkg/upgrade/upgrades/BUILD.bazel index 99b178582be9..cfba95b260aa 100644 --- a/pkg/upgrade/upgrades/BUILD.bazel +++ b/pkg/upgrade/upgrades/BUILD.bazel @@ -7,6 +7,7 @@ go_library( "alter_jobs_add_job_type.go", "alter_sql_instances_sql_addr.go", "alter_table_statistics_partial_predicate_and_id.go", + "backfill_jobs_info_table_migration.go", "create_index_usage_statement_statistics.go", "create_jobs_metrics_polling_job.go", "database_role_settings_table_user_id_migration.go", @@ -86,6 +87,7 @@ go_test( "alter_jobs_add_job_type_test.go", "alter_sql_instances_sql_addr_test.go", "alter_table_statistics_partial_predicate_and_id_test.go", + "backfill_jobs_info_table_migration_test.go", "builtins_test.go", "create_index_usage_statement_statistics_test.go", "create_jobs_metrics_polling_job_test.go", @@ -126,6 +128,7 @@ go_test( "//pkg/jobs/jobspb", "//pkg/jobs/jobstest", "//pkg/keys", + "//pkg/keyvisualizer", "//pkg/kv", "//pkg/roachpb", "//pkg/security/securityassets", diff --git a/pkg/upgrade/upgrades/backfill_jobs_info_table_migration.go b/pkg/upgrade/upgrades/backfill_jobs_info_table_migration.go new file mode 100644 index 000000000000..08baeb468aec --- /dev/null +++ b/pkg/upgrade/upgrades/backfill_jobs_info_table_migration.go @@ -0,0 +1,48 @@ +// Copyright 2023 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 + +import ( + "context" + + "github.com/cockroachdb/cockroach/pkg/clusterversion" + "github.com/cockroachdb/cockroach/pkg/jobs" + "github.com/cockroachdb/cockroach/pkg/kv" + "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" + "github.com/cockroachdb/cockroach/pkg/upgrade" + "github.com/cockroachdb/errors" +) + +const backfillJobInfoTableWithPayloadStmt = ` +INSERT INTO system.job_info (job_id, info_key, value) +SELECT id, $1, payload FROM system.jobs +` + +const backfillJobInfoTableWithProgressStmt = ` +INSERT INTO system.job_info (job_id, info_key, value) +SELECT id, $1, progress FROM system.jobs +` + +func backfillJobInfoTable( + ctx context.Context, cs clusterversion.ClusterVersion, d upgrade.TenantDeps, +) error { + return d.DB.KV().Txn(ctx, func(ctx context.Context, txn *kv.Txn) error { + _, err := d.InternalExecutor.ExecEx(ctx, "backfill-job-info-payload", txn, + sessiondata.NodeUserSessionDataOverride, backfillJobInfoTableWithPayloadStmt, jobs.GetLegacyPayloadKey()) + if err != nil { + return errors.Wrap(err, "failed to backfill legacy payload") + } + + _, err = d.InternalExecutor.ExecEx(ctx, "backfill-job-info-progress", txn, + sessiondata.NodeUserSessionDataOverride, backfillJobInfoTableWithProgressStmt, jobs.GetLegacyProgressKey()) + return errors.Wrap(err, "failed to backfill legacy progress") + }) +} diff --git a/pkg/upgrade/upgrades/backfill_jobs_info_table_migration_test.go b/pkg/upgrade/upgrades/backfill_jobs_info_table_migration_test.go new file mode 100644 index 000000000000..4246ea49e9fd --- /dev/null +++ b/pkg/upgrade/upgrades/backfill_jobs_info_table_migration_test.go @@ -0,0 +1,112 @@ +// Copyright 2023 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_test + +import ( + "context" + "testing" + + "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/clusterversion" + "github.com/cockroachdb/cockroach/pkg/jobs" + "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" + "github.com/cockroachdb/cockroach/pkg/keyvisualizer" + "github.com/cockroachdb/cockroach/pkg/security/username" + "github.com/cockroachdb/cockroach/pkg/server" + "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" + "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" + "github.com/cockroachdb/cockroach/pkg/upgrade/upgradebase" + "github.com/cockroachdb/cockroach/pkg/upgrade/upgrades" + "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/stretchr/testify/require" +) + +// TestBackfillJobsInfoTable tests that the `system.job_info` table is +// backfilled with the progress and payload of each job in the `system.jobs` +// table. +func TestBackfillJobsInfoTable(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + clusterArgs := base.TestClusterArgs{ + // Disable all automatic jobs creation and adoption. + ServerArgs: base.TestServerArgs{ + DisableSpanConfigs: true, + Knobs: base.TestingKnobs{ + // Avoiding jobs to be adopted. + JobsTestingKnobs: &jobs.TestingKnobs{ + DisableAdoptions: true, + }, + // DisableAdoptions needs this. + UpgradeManager: &upgradebase.TestingKnobs{ + DontUseJobs: true, + SkipJobMetricsPollingJobBootstrap: true, + }, + KeyVisualizer: &keyvisualizer.TestingKnobs{ + SkipJobBootstrap: true, + }, + Server: &server.TestingKnobs{ + DisableAutomaticVersionUpgrade: make(chan struct{}), + BinaryVersionOverride: clusterversion.ByKey(clusterversion.V22_2), + }, + }, + }, + } + + ctx := context.Background() + tc := testcluster.StartTestCluster(t, 1, clusterArgs) + r := tc.Server(0).JobRegistry().(*jobs.Registry) + sqlDB := sqlutils.MakeSQLRunner(tc.ServerConn(0)) + defer tc.Stopper().Stop(ctx) + + createJob := func(id jobspb.JobID, details jobspb.Details, progress jobspb.ProgressDetails) *jobs.Job { + defaultRecord := jobs.Record{ + Details: details, + Progress: progress, + Username: username.TestUserName(), + } + + job, err := r.CreateJobWithTxn(ctx, defaultRecord, id, nil /* txn */) + require.NoError(t, err) + return job + } + + // Create a few different types of jobs. + createJob(1, jobspb.BackupDetails{}, jobspb.BackupProgress{}) + createJob(2, jobspb.RestoreDetails{}, jobspb.RestoreProgress{}) + createJob(3, jobspb.ChangefeedDetails{}, jobspb.ChangefeedProgress{}) + + // Validate that there are no rows in the system.job_info table. + sqlDB.CheckQueryResults(t, `SELECT count(*) FROM system.job_info`, [][]string{{"0"}}) + + upgrades.Upgrade(t, tc.ServerConn(0), clusterversion.V23_1CreateSystemJobInfoTable, nil, false) + + // Create two more jobs that we should see written to both system.jobs and + // system.job_info. + createJob(4, jobspb.ImportDetails{}, jobspb.ImportProgress{}) + createJob(5, jobspb.SchemaChangeDetails{}, jobspb.SchemaChangeProgress{}) + + // Validate that we see 2 rows (payload and progress) in the system.job_info + // table for each row written in the system.jobs table since the last upgrade. + sqlDB.CheckQueryResults(t, `SELECT count(*) FROM system.jobs AS j, system.job_info AS i WHERE j.id = i.job_id AND (j.payload = i.value OR j.progress = i.value)`, + [][]string{{"4"}}) + + upgrades.Upgrade(t, tc.ServerConn(0), clusterversion.V23_1JobInfoTableIsBackfilled, nil, false) + + // At this point the entire system.jobs table should be backfilled into the + // system.job_info table. + // We expect to see 14 rows because of: + // - 4 rows from before + // - 10 rows (payload + progress) for the 5 jobs in system.jobs + sqlDB.CheckQueryResults(t, `SELECT count(*) FROM system.jobs AS j, system.job_info AS i WHERE j.id = i.job_id AND (j.payload = i.value OR j.progress = i.value)`, + [][]string{{"14"}}) +} diff --git a/pkg/upgrade/upgrades/schemachanger_elements_test.go b/pkg/upgrade/upgrades/schemachanger_elements_test.go index a6d5b5c8980b..a4ef9502a815 100644 --- a/pkg/upgrade/upgrades/schemachanger_elements_test.go +++ b/pkg/upgrade/upgrades/schemachanger_elements_test.go @@ -26,6 +26,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/tests" "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" + "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/util/ctxgroup" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/log" @@ -34,6 +35,7 @@ import ( func TestUpgradeSchemaChangerElements(t *testing.T) { defer leaktest.AfterTest(t)() + skip.WithIssue(t, 98062, "flaky test") defer log.Scope(t).Close(t) ctx := context.Background() diff --git a/pkg/upgrade/upgrades/upgrades.go b/pkg/upgrade/upgrades/upgrades.go index e52760725278..8a3c7b7d1df2 100644 --- a/pkg/upgrade/upgrades/upgrades.go +++ b/pkg/upgrade/upgrades/upgrades.go @@ -276,6 +276,12 @@ var upgrades = []upgradebase.Upgrade{ upgrade.NoPrecondition, backfillExternalConnectionsTableOwnerIDColumn, ), + upgrade.NewTenantUpgrade( + "backfill the system.jobs_info table with the payload and progress of each job in the system.jobs table", + toCV(clusterversion.V23_1JobInfoTableIsBackfilled), + upgrade.NoPrecondition, + backfillJobInfoTable, + ), } func init() {