Merge #92481 #92571

92481: backupccl: don't disable leases in test r=adityamaru a=stevendanna 399e56b 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-L159 But, when TestingDisableTableLeases 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 92571: ui: add seconds, milliseconds to insights timestamps in console r=ericharmeling a=ericharmeling This commit adds seconds and milliseconds to the timestamp values on the Insights pages in the DB Console. Fixes #91936. Loom: https://www.loom.com/share/1aa7e5ca81984b1190f67a1748506aae Release note (ui change): The Insights pages in the DB Console now show the seconds and milliseconds for all timestamp values. Co-authored-by: Steven Danna <[email protected]> Co-authored-by: Eric Harmeling <[email protected]>
cockroachdb · Nov 29, 2022 · a16106e · a16106e
3 parents 8e5d672 + fcf8cbc + 1c5ddcd
commit a16106e
Show file tree

Hide file tree

Showing 10 changed files with 42 additions and 29 deletions.
diff --git 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
@@ -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()

diff --git 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
@@ -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/...ces/cluster-ui/src/insights/workloadInsightDetails/statementInsightDetailsOverviewTab.tsx b/...ces/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<
           <SummaryCard>
             <SummaryCardItem
               label="Start Time"
-              value={insightDetails.startTime.format(DATE_FORMAT_24_UTC)}
+              value={insightDetails.startTime.format(
+                DATE_WITH_SECONDS_AND_MILLISECONDS_FORMAT_24_UTC,
+              )}
             />
             <SummaryCardItem
               label="End Time"
-              value={insightDetails.endTime.format(DATE_FORMAT_24_UTC)}
+              value={insightDetails.endTime.format(
+                DATE_WITH_SECONDS_AND_MILLISECONDS_FORMAT_24_UTC,
+              )}
             />
             <SummaryCardItem
               label="Elapsed Time"

diff --git a/...s/cluster-ui/src/insights/workloadInsightDetails/transactionInsightDetailsOverviewTab.tsx b/...s/cluster-ui/src/insights/workloadInsightDetails/transactionInsightDetailsOverviewTab.tsx
@@ -15,7 +15,7 @@ import "antd/lib/col/style";
 import "antd/lib/row/style";
 import { SqlBox, SqlBoxSize } from "src/sql";
 import { SummaryCard, SummaryCardItem } from "src/summaryCard";
-import { DATE_FORMAT_24_UTC } from "src/util/format";
+import { DATE_WITH_SECONDS_AND_MILLISECONDS_FORMAT_24_UTC } from "src/util/format";
 import { WaitTimeInsightsLabels } from "src/detailsPanels/waitTimeInsightsPanel";
 import { TxnContentionInsightDetailsRequest } from "src/api";
 import {
@@ -118,8 +118,9 @@ export const TransactionInsightDetailsOverviewTab: React.FC<Props> = ({
                   <SummaryCardItem
                     label="Start Time"
                     value={
-                      insightDetails.startTime?.format(DATE_FORMAT_24_UTC) ??
-                      "no samples"
+                      insightDetails.startTime?.format(
+                        DATE_WITH_SECONDS_AND_MILLISECONDS_FORMAT_24_UTC,
+                      ) ?? "no samples"
                     }
                   />
                   <SummaryCardItem label="Rows Read" value={rowsRead} />

diff --git a/...aces/cluster-ui/src/insights/workloadInsightDetails/transactionInsightDetailsStmtsTab.tsx b/...aces/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<StatementInsightEvent>[] = [
   {
@@ -46,14 +50,16 @@ const stmtColumns: ColumnDescriptor<StatementInsightEvent>[] = [
   },
   {
     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/...ces/cluster-ui/src/insights/workloadInsights/statementInsights/statementInsightsTable.tsx b/...ces/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/...cluster-ui/src/insights/workloadInsights/transactionInsights/transactionInsightsTable.tsx b/...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
@@ -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) {