From fcf8cbc14ebd164d9f74dcada25e98a6fdd8c3a0 Mon Sep 17 00:00:00 2001 From: Steven Danna Date: Fri, 25 Nov 2022 08:13:13 +0000 Subject: [PATCH 1/2] backupccl: don't disable leases in test 399e56b3f56c0dce6c34525bebf58297be320083 introduced a bounded staleness read into the migration machinery. When `lease.TestingDisableTableLeases()` has been set, this bounded staleness read encounters an error: testcluster.go:384: migration-job-find-already-completed: cannot set fixed timestamp, txn "sql txn" meta={id=f4142488 key=/Min pri=0.01688073 epo=0 ts=1669334862.467371575,0 min=1669334862.467371575,0 seq=0} lock=false stat=PENDING rts=1669334862.467371575,0 wto=false gul=1669334862.967371575,0 already performed reads I believe that the read that was already performed in this case was the descriptor lookup. Then, when we go to execute the select, we attempt to SetFixedTimestamp in txn.NegotiateAndSend. When the testing isn't in use, on its face it looks like we don't hit this case because we don't allow a fallback to a store lookup: https://github.com/cockroachdb/cockroach/blob/b5be006bedd7d3cedc3fb3d2248df168e3d64be2/pkg/sql/catalog/descs/leased_descriptors.go#L143-L155 But, when TestingDsiableTableLeases is set, we skip right to the store lookup: https://github.com/cockroachdb/cockroach/blob/b5be006bedd7d3cedc3fb3d2248df168e3d64be2/pkg/sql/catalog/descs/descriptor.go#L489-L491 I haven't looked into why lease.TestingDisableTableLeases() was in place in the past, but it is no longer used in any other backup tests and isn't likely needed here. Fixes #92432 Fixes #92433 Fixes #92434 Fixes #92435 Release note: None --- pkg/ccl/backupccl/BUILD.bazel | 1 - pkg/ccl/backupccl/backup_cloud_test.go | 9 --------- 2 files changed, 10 deletions(-) diff --git a/pkg/ccl/backupccl/BUILD.bazel b/pkg/ccl/backupccl/BUILD.bazel index ead29afdf9a4..02ab85b2124f 100644 --- a/pkg/ccl/backupccl/BUILD.bazel +++ b/pkg/ccl/backupccl/BUILD.bazel @@ -242,7 +242,6 @@ go_test( "//pkg/sql/catalog/descpb", "//pkg/sql/catalog/descs", "//pkg/sql/catalog/desctestutils", - "//pkg/sql/catalog/lease", "//pkg/sql/catalog/systemschema", "//pkg/sql/catalog/tabledesc", "//pkg/sql/execinfra", diff --git a/pkg/ccl/backupccl/backup_cloud_test.go b/pkg/ccl/backupccl/backup_cloud_test.go index c48c38cdf48d..fea4b2b69869 100644 --- a/pkg/ccl/backupccl/backup_cloud_test.go +++ b/pkg/ccl/backupccl/backup_cloud_test.go @@ -19,7 +19,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/cloud" "github.com/cockroachdb/cockroach/pkg/cloud/amazon" "github.com/cockroachdb/cockroach/pkg/cloud/azure" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/lease" "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" @@ -51,8 +50,6 @@ func TestCloudBackupRestoreS3(t *testing.T) { defer log.Scope(t).Close(t) creds, bucket := requiredS3CredsAndBucket(t) - // TODO(dan): Actually invalidate the descriptor cache and delete this line. - defer lease.TestingDisableTableLeases()() const numAccounts = 1000 ctx := context.Background() @@ -70,8 +67,6 @@ func TestCloudBackupRestoreS3WithLegacyPut(t *testing.T) { defer log.Scope(t).Close(t) creds, bucket := requiredS3CredsAndBucket(t) - // TODO(dan): Actually invalidate the descriptor cache and delete this line. - defer lease.TestingDisableTableLeases()() const numAccounts = 1000 ctx := context.Background() @@ -131,8 +126,6 @@ func TestCloudBackupRestoreGoogleCloudStorage(t *testing.T) { skip.IgnoreLint(t, "GOOGLE_BUCKET env var must be set") } - // TODO(dan): Actually invalidate the descriptor cache and delete this line. - defer lease.TestingDisableTableLeases()() const numAccounts = 1000 ctx := context.Background() @@ -164,8 +157,6 @@ func TestCloudBackupRestoreAzure(t *testing.T) { skip.IgnoreLint(t, "AZURE_CONTAINER env var must be set") } - // TODO(dan): Actually invalidate the descriptor cache and delete this line. - defer lease.TestingDisableTableLeases()() const numAccounts = 1000 ctx := context.Background() From 1c5ddcd8efea8868113da9ab8c7bfeb1e1fc9a25 Mon Sep 17 00:00:00 2001 From: Eric Harmeling Date: Mon, 28 Nov 2022 09:59:18 -0500 Subject: [PATCH 2/2] ui: add seconds, milliseconds to insights timestamps in console This commit adds seconds and milliseconds to the timestamp values on the Insights pages in the DB Console. Fixes #91936. Release note (ui change): The Insights pages in the DB Console now show the seconds and milliseconds for all timestamp values. --- .../workspaces/cluster-ui/src/insights/utils.ts | 2 +- .../insightDetailsTables.tsx | 5 +++-- .../statementInsightDetailsOverviewTab.tsx | 10 +++++++--- .../transactionInsightDetailsOverviewTab.tsx | 7 ++++--- .../transactionInsightDetailsStmtsTab.tsx | 16 +++++++++++----- .../statementInsights/statementInsightsTable.tsx | 9 +++++++-- .../transactionInsightsTable.tsx | 6 ++++-- pkg/ui/workspaces/cluster-ui/src/util/format.ts | 6 +++++- 8 files changed, 42 insertions(+), 19 deletions(-) diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/utils.ts b/pkg/ui/workspaces/cluster-ui/src/insights/utils.ts index 82c5332c8849..bcc730064e2a 100644 --- a/pkg/ui/workspaces/cluster-ui/src/insights/utils.ts +++ b/pkg/ui/workspaces/cluster-ui/src/insights/utils.ts @@ -460,7 +460,7 @@ export function getStmtInsightRecommendations( indexRecommendations: insightDetails.indexRecommendations, databaseName: insightDetails.databaseName, elapsedTimeMillis: insightDetails.elapsedTimeMillis, - contentionTime: insightDetails.totalContentionTime.asMilliseconds(), + contentionTime: insightDetails.totalContentionTime?.asMilliseconds(), }; const recs: InsightRecommendation[] = insightDetails.insights?.map(insight => diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/insightDetailsTables.tsx b/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/insightDetailsTables.tsx index 649439e57376..37c90ca2d150 100644 --- a/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/insightDetailsTables.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/insightDetailsTables.tsx @@ -10,7 +10,7 @@ import React from "react"; import { ColumnDescriptor, SortedTable, SortSetting } from "src/sortedtable"; -import { DATE_FORMAT, Duration } from "src/util"; +import { DATE_WITH_SECONDS_AND_MILLISECONDS_FORMAT, Duration } from "src/util"; import { BlockedStatementContentionDetails, ContentionEvent, @@ -48,7 +48,8 @@ export function makeInsightDetailsColumns( { name: "contentionStartTime", title: insightsTableTitles.contentionStartTime(execType), - cell: (item: ContentionEvent) => item.startTime.format(DATE_FORMAT), + cell: (item: ContentionEvent) => + item.startTime?.format(DATE_WITH_SECONDS_AND_MILLISECONDS_FORMAT), sort: (item: ContentionEvent) => item.startTime.unix(), }, { diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/statementInsightDetailsOverviewTab.tsx b/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/statementInsightDetailsOverviewTab.tsx index 2d59ac8d716f..5e326fb30802 100644 --- a/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/statementInsightDetailsOverviewTab.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/statementInsightDetailsOverviewTab.tsx @@ -15,7 +15,7 @@ import { } from "src/insightsTable/insightsTable"; import { SummaryCard, SummaryCardItem } from "src/summaryCard"; import { capitalize, Duration } from "src/util"; -import { DATE_FORMAT_24_UTC } from "src/util/format"; +import { DATE_WITH_SECONDS_AND_MILLISECONDS_FORMAT_24_UTC } from "src/util/format"; import { FlattenedStmtInsightEvent } from "../types"; import classNames from "classnames/bind"; import { CockroachCloudContext } from "../../contexts"; @@ -99,11 +99,15 @@ export const StatementInsightDetailsOverviewTab: React.FC< = ({ diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/transactionInsightDetailsStmtsTab.tsx b/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/transactionInsightDetailsStmtsTab.tsx index 2181cfbf0433..a3809aee0b6a 100644 --- a/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/transactionInsightDetailsStmtsTab.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/transactionInsightDetailsStmtsTab.tsx @@ -13,7 +13,11 @@ import { Link } from "react-router-dom"; import { ColumnDescriptor, SortedTable } from "src/sortedtable"; import { StatementInsightEvent, TxnInsightDetails } from "../types"; import { InsightCell } from "../workloadInsights/util/insightCell"; -import { DATE_FORMAT, Duration, limitText } from "src/util"; +import { + DATE_WITH_SECONDS_AND_MILLISECONDS_FORMAT, + Duration, + limitText, +} from "src/util"; const stmtColumns: ColumnDescriptor[] = [ { @@ -46,14 +50,16 @@ const stmtColumns: ColumnDescriptor[] = [ }, { name: "startTime", - title: "Start Time", - cell: (item: StatementInsightEvent) => item.startTime.format(DATE_FORMAT), + title: "Start Time (UTC)", + cell: (item: StatementInsightEvent) => + item.startTime?.format(DATE_WITH_SECONDS_AND_MILLISECONDS_FORMAT), sort: (item: StatementInsightEvent) => item.startTime.unix(), }, { name: "endTime", - title: "End Time", - cell: (item: StatementInsightEvent) => item.endTime.format(DATE_FORMAT), + title: "End Time (UTC)", + cell: (item: StatementInsightEvent) => + item.endTime?.format(DATE_WITH_SECONDS_AND_MILLISECONDS_FORMAT), sort: (item: StatementInsightEvent) => item.endTime.unix(), }, { 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 82701203c3fc..484b1b287d31 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 @@ -15,7 +15,12 @@ import { SortedTable, SortSetting, } from "src/sortedtable"; -import { Count, DATE_FORMAT, Duration, limitText } from "src/util"; +import { + Count, + DATE_WITH_SECONDS_AND_MILLISECONDS_FORMAT, + Duration, + limitText, +} from "src/util"; import { InsightExecEnum, FlattenedStmtInsightEvent } from "src/insights"; import { InsightCell, @@ -90,7 +95,7 @@ export function makeStatementInsightsColumns( name: "startTime", title: insightsTableTitles.startTime(execType), cell: (item: FlattenedStmtInsightEvent) => - item.startTime.format(DATE_FORMAT), + item.startTime?.format(DATE_WITH_SECONDS_AND_MILLISECONDS_FORMAT), sort: (item: FlattenedStmtInsightEvent) => item.startTime.unix(), showByDefault: true, }, 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 aa768c3799de..ac33cb648301 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 @@ -15,7 +15,7 @@ import { SortedTable, SortSetting, } from "src/sortedtable"; -import { DATE_FORMAT, Duration } from "src/util"; +import { DATE_WITH_SECONDS_AND_MILLISECONDS_FORMAT, Duration } from "src/util"; import { InsightExecEnum, MergedTxnInsightEvent } from "src/insights"; import { InsightCell, @@ -79,7 +79,9 @@ export function makeTransactionInsightsColumns( { name: "startTime", title: insightsTableTitles.startTime(execType), - cell: item => item.startTime?.format(DATE_FORMAT) ?? "N/A", + cell: item => + item.startTime?.format(DATE_WITH_SECONDS_AND_MILLISECONDS_FORMAT) ?? + "N/A", sort: item => item.startTime?.unix() || 0, }, { diff --git a/pkg/ui/workspaces/cluster-ui/src/util/format.ts b/pkg/ui/workspaces/cluster-ui/src/util/format.ts index 55a227491c4c..5aaa488629f1 100644 --- a/pkg/ui/workspaces/cluster-ui/src/util/format.ts +++ b/pkg/ui/workspaces/cluster-ui/src/util/format.ts @@ -170,12 +170,16 @@ export const DurationFitScale = }; export const DATE_FORMAT = "MMM DD, YYYY [at] H:mm"; +export const DATE_WITH_SECONDS_AND_MILLISECONDS_FORMAT = + "MMM DD, YYYY [at] H:mm:ss:ms"; /** - * Alternate 24 hour UTC format + * Alternate 24 hour UTC formats */ export const DATE_FORMAT_24_UTC = "MMM DD, YYYY [at] H:mm UTC"; export const DATE_WITH_SECONDS_FORMAT_24_UTC = "MMM DD, YYYY [at] H:mm:ss UTC"; +export const DATE_WITH_SECONDS_AND_MILLISECONDS_FORMAT_24_UTC = + "MMM DD, YYYY [at] H:mm:ss:ms UTC"; export function RenderCount(yesCount: Long, totalCount: Long): string { if (longToInt(yesCount) == 0) {