Merge cockroachdb#107563 cockroachdb#107798 cockroachdb#107807

107563: acceptance: make `TestDockerCLI_test_exec_log` wait for unambiguous l… r=abarganier a=xinhaoz …og line The `TestDockerCLI_test_exec_log` test had a section where it waits for the last executed stmt to appear in the exec_log file before proceeding to the next section. This is done by grep'ing the query string of the last stmt. This works fine most of the time, however, the 2nd last and last stmt executed happen to have the same query string, so it's possible that we are only seeing the 2nd to last stmt in the log. This can cause the test to fail as in the next section, we expect all query logs to be present in the file. This commit fixes this by issuing a distinct query as the last query so that we can grep for a distinct query string to verify all stmts should have been written. A new proc is also added to `common.tcl`: flush_and_sync_logs is a proc that takes 2 arguments, `filename` and `grep_text`. It will trigger a signal hang up to trigger a log flush and then attempt to find the `grep_text` in the specified file. Epic: none Fixes: cockroachdb#106367 107798: cluster-ui: delete unused vars r=xinhaoz a=xinhaoz Delete unused vars in cluster-ui. A following commit will turn unused vars to errors via the eslint config. Epic: None Release note: None 107807: diagnosticsccl: fix TestUsageQuantization r=zachlite a=zachlite TestUsageQuantization failed with secondary tenants because the test's server.TestingKnobs were not being passed through to test tenant creation. Resolves cockroachdb#106901 Epic: none Release note: None Co-authored-by: Xin Hao Zhang <[email protected]> Co-authored-by: zachlite <[email protected]>
rytaft · Jul 28, 2023 · 7ba8059 · 7ba8059
4 parents 32b2be3 + bb5d813 + 790d9c0 + 9e5de35
commit 7ba8059
Show file tree

Hide file tree

Showing 37 changed files with 69 additions and 132 deletions.
diff --git a/pkg/ccl/serverccl/diagnosticsccl/reporter_test.go b/pkg/ccl/serverccl/diagnosticsccl/reporter_test.go
@@ -288,8 +288,6 @@ func TestUsageQuantization(t *testing.T) {
 
 	url := r.URL()
 	s, db, _ := serverutils.StartServer(t, base.TestServerArgs{
-		DefaultTestTenant: base.TestDoesNotWorkWithSecondaryTenantsButWeDontKnowWhyYet(106901),
-
 		Settings: st,
 		Knobs: base.TestingKnobs{
 			Server: &server.TestingKnobs{

diff --git a/pkg/cli/interactive_tests/common.tcl b/pkg/cli/interactive_tests/common.tcl
@@ -144,6 +144,20 @@ proc flush_server_logs {} {
     report "END FLUSH LOGS"
 }
 
+proc flush_and_sync_logs {filename grep_text} {
+    report "BEGIN FLUSH LOGS $filename"
+    system "kill -HUP `cat server_pid` 2>/dev/null"
+    # Wait for flush to occur.
+    system "for i in `seq 1 5`; do
+              grep '$grep_text' $filename && exit 0;
+              echo still waiting
+              sleep 1
+            done
+            echo 'server failed to flush logs?'
+            exit 1"
+    report "END FLUSH LOGS"
+}
+
 proc force_stop_server {argv} {
     report "BEGIN FORCE STOP SERVER"
     system "kill -KILL `cat server_pid`"

diff --git a/pkg/cli/interactive_tests/test_exec_log.tcl b/pkg/cli/interactive_tests/test_exec_log.tcl
@@ -97,23 +97,15 @@ send "SELECT 550+5;\r"
 eexpect 555
 eexpect root@
 
-flush_server_logs
+# Use this distinct query to be the boundary - if we see this stmt in the
+# exec log, then we should expect all the previous statements in the log too.
+
+send "SELECT 111;\r"
+eexpect 111
+eexpect root@
+
+flush_and_sync_logs $logfile "SELECT ..*111..*"
 
-# Now check the items are there in the log file. We need to iterate
-# because flush_server_logs only syncs on flush of cockroach.log, not
-# the exec log.
-#
-# We also check the last statement first, this ensures that every
-# previous statement is also in the log file after this check
-# succeeds.
-system "for i in `seq 1 3`; do
-  grep 'SELECT ..*550..* +' $logfile && exit 0;
-  echo still waiting;
-  sleep 1;
-done;
-echo 'not finding two separate txn counter values?';
-grep 'SELECT ..*550..* +' $logfile;
-exit 1;"
 
 # Two separate single-stmt txns.
 system "n=`grep 'SELECT ..*550..* +' $logfile | sed -e 's/.*TxnCounter.:\\(\[0-9\]*\\).*/\\1/g' | uniq | wc -l`; if test \$n -ne 2; then echo unexpected \$n; exit 1; fi"

diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go
@@ -617,9 +617,13 @@ func (ts *TestServer) maybeStartDefaultTestTenant(ctx context.Context) error {
 	// Since we're creating a tenant, it doesn't make sense to pass through the
 	// Server testing knobs, since the bulk of them only apply to the system
 	// tenant. Any remaining knobs which are required by the tenant should be
-	// setup in StartTenant below.
+	// passed through here.
 	params.TestingKnobs.Server = &TestingKnobs{}
 
+	if ts.params.Knobs.Server != nil {
+		params.TestingKnobs.Server.(*TestingKnobs).DiagnosticsTestingKnobs = ts.params.Knobs.Server.(*TestingKnobs).DiagnosticsTestingKnobs
+	}
+
 	// Temporarily disable the error that is returned if a tenant should not be started manually,
 	// so that we can start the default test tenant internally here.
 	disableStartTenantError := ts.disableStartTenantError

diff --git a/pkg/ui/workspaces/cluster-ui/src/api/databaseDetailsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/databaseDetailsApi.ts
@@ -93,11 +93,7 @@ const getDatabaseId: DatabaseDetailsQuery<DatabaseIdRow> = {
       resp.idResp.error = txn_result.error;
     }
   },
-  handleMaxSizeError: (
-    dbName: string,
-    response: SqlTxnResult<DatabaseIdRow>,
-    dbDetail: DatabaseDetailsResponse,
-  ) => {
+  handleMaxSizeError: (_dbName, _response, _dbDetail) => {
     return new Promise<boolean>(() => false);
   },
 };
@@ -135,11 +131,7 @@ const getDatabaseGrantsQuery: DatabaseDetailsQuery<DatabaseGrantsRow> = {
       }
     }
   },
-  handleMaxSizeError: (
-    dbName: string,
-    response: SqlTxnResult<DatabaseGrantsRow>,
-    dbDetail: DatabaseDetailsResponse,
-  ) => {
+  handleMaxSizeError: (_dbName, _response, _dbDetail) => {
     return new Promise<boolean>(() => false);
   },
 };
@@ -284,11 +276,7 @@ const getDatabaseZoneConfig: DatabaseDetailsQuery<DatabaseZoneConfigRow> = {
       resp.idResp.error = txn_result.error;
     }
   },
-  handleMaxSizeError: (
-    dbName: string,
-    response: SqlTxnResult<DatabaseZoneConfigRow>,
-    dbDetail: DatabaseDetailsResponse,
-  ) => {
+  handleMaxSizeError: (_dbName, _response, _dbDetail) => {
     return new Promise<boolean>(() => false);
   },
 };
@@ -341,11 +329,7 @@ const getDatabaseSpanStats: DatabaseDetailsQuery<DatabaseSpanStatsRow> = {
       );
     }
   },
-  handleMaxSizeError: (
-    dbName: string,
-    response: SqlTxnResult<DatabaseSpanStatsRow>,
-    dbDetail: DatabaseDetailsResponse,
-  ) => {
+  handleMaxSizeError: (_dbName, _response, _dbDetail) => {
     return new Promise<boolean>(() => false);
   },
 };
@@ -390,11 +374,7 @@ const getDatabaseReplicasAndRegions: DatabaseDetailsQuery<DatabaseReplicasRegion
         resp.stats.replicaData.error = txn_result.error;
       }
     },
-    handleMaxSizeError: (
-      dbName: string,
-      response: SqlTxnResult<DatabaseReplicasRegionsRow>,
-      dbDetail: DatabaseDetailsResponse,
-    ) => {
+    handleMaxSizeError: (_dbName, _response, _dbDetail) => {
       return new Promise<boolean>(() => false);
     },
   };
@@ -444,11 +424,7 @@ const getDatabaseIndexUsageStats: DatabaseDetailsQuery<IndexUsageStatistic> = {
       resp.stats.indexStats.error = txn_result.error;
     }
   },
-  handleMaxSizeError: (
-    dbName: string,
-    response: SqlTxnResult<IndexUsageStatistic>,
-    dbDetail: DatabaseDetailsResponse,
-  ) => {
+  handleMaxSizeError: (_dbName, _response, _dbDetail) => {
     return new Promise<boolean>(() => false);
   },
 };

diff --git a/pkg/ui/workspaces/cluster-ui/src/columnsSelector/columnsSelector.tsx b/pkg/ui/workspaces/cluster-ui/src/columnsSelector/columnsSelector.tsx
@@ -86,7 +86,7 @@ const customStyles = {
     ...provided,
     maxHeight: "310px",
   }),
-  option: (provided: any, state: any) => ({
+  option: (provided: any, _state: any) => ({
     ...provided,
     backgroundColor: "white",
     color: "#475872",

diff --git a/pkg/ui/workspaces/cluster-ui/src/databases/combiners.ts b/pkg/ui/workspaces/cluster-ui/src/databases/combiners.ts
@@ -26,7 +26,6 @@ import { DatabaseTablePageDataDetails, IndexStat } from "../databaseTablePage";
 import { IndexStatsState } from "../store/indexStats";
 import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
 import { RecommendationType as RecType } from "../indexDetailsPage";
-import { TableIndexStatsResponse } from "../api/indexDetailsApi";
 type IndexUsageStatistic =
   cockroach.server.serverpb.TableIndexStatsResponse.IExtendedCollectedIndexUsageStatistics;
 const { RecommendationType } = cockroach.sql.IndexRecommendation;

diff --git a/pkg/ui/workspaces/cluster-ui/src/databases/util.spec.ts b/pkg/ui/workspaces/cluster-ui/src/databases/util.spec.ts
@@ -8,7 +8,6 @@
 // by the Apache License, Version 2.0, included in the file
 // licenses/APL.txt.
 
-import { assert } from "chai";
 import {
   getNodesByRegionString,
   normalizePrivileges,

diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/insightDetailsTables.tsx b/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/insightDetailsTables.tsx
@@ -19,7 +19,7 @@ import {
   TransactionDetailsLink,
 } from "../workloadInsights/util";
 import { TimeScale } from "../../timeScaleDropdown";
-import { Timestamp, Timezone } from "../../timestamp";
+import { Timestamp } from "../../timestamp";
 
 interface InsightDetailsTableProps {
   data: ContentionEvent[];

diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/statementInsightDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/statementInsightDetails.tsx
@@ -76,7 +76,6 @@ export const StatementInsightDetails: React.FC<
   isTenant,
   timeScale,
   hasAdminRole,
-  setTimeScale,
   refreshUserSQLRoles,
 }) => {
   const [explainPlanState, setExplainPlanState] = useState<ExplainPlanState>({

diff --git a/...ces/cluster-ui/src/insights/workloadInsightDetails/statementInsightDetailsOverviewTab.tsx b/...ces/cluster-ui/src/insights/workloadInsightDetails/statementInsightDetailsOverviewTab.tsx
@@ -33,7 +33,6 @@ import {
   StatementDetailsLink,
   TransactionDetailsLink,
 } from "../workloadInsights/util";
-import { TimeScale } from "../../timeScaleDropdown";
 import { getStmtInsightRecommendations } from "../utils";
 import { ContentionStatementDetailsTable } from "./insightDetailsTables";
 import { WaitTimeInsightsLabels } from "../../detailsPanels/waitTimeInsightsPanel";

diff --git a/...ces/cluster-ui/src/insights/workloadInsights/statementInsights/statementInsightsTable.tsx b/...ces/cluster-ui/src/insights/workloadInsights/statementInsights/statementInsightsTable.tsx
@@ -36,9 +36,8 @@ import { Tooltip } from "@cockroachlabs/ui-components";
 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";
-import { Timestamp, Timezone } from "../../../timestamp";
+import { Timestamp } from "../../../timestamp";
 
 const cx = classNames.bind(styles);
 

diff --git a/...aces/cluster-ui/src/insights/workloadInsights/statementInsights/statementInsightsView.tsx b/...aces/cluster-ui/src/insights/workloadInsights/statementInsights/statementInsightsView.tsx
@@ -112,7 +112,6 @@ export const StatementInsightsView: React.FC<StatementInsightsViewProps> = ({
   selectedColumnNames,
   dropDownSelect,
   maxSizeApiReached,
-  isTenant,
 }: StatementInsightsViewProps) => {
   const [pagination, setPagination] = useState<ISortedTablePagination>({
     current: 1,

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
@@ -166,19 +166,19 @@ export const insightsTableTitles: InsightsTableTitleType = {
       "username",
     );
   },
-  schemaName: (execType: InsightExecEnum) => {
+  schemaName: (_execType: InsightExecEnum) => {
     return makeToolTip(<p>The name of the contended schema.</p>, "schemaName");
   },
-  databaseName: (execType: InsightExecEnum) => {
+  databaseName: (_execType: InsightExecEnum) => {
     return makeToolTip(
       <p>The name of the contended database.</p>,
       "databaseName",
     );
   },
-  tableName: (execType: InsightExecEnum) => {
+  tableName: (_execType: InsightExecEnum) => {
     return makeToolTip(<p>The name of the contended table.</p>, "tableName");
   },
-  indexName: (execType: InsightExecEnum) => {
+  indexName: (_execType: InsightExecEnum) => {
     return makeToolTip(<p>The name of the contended index.</p>, "indexName");
   },
   applicationName: (execType: InsightExecEnum) => {

diff --git a/pkg/ui/workspaces/cluster-ui/src/jobs/jobsPage/jobsPage.spec.tsx b/pkg/ui/workspaces/cluster-ui/src/jobs/jobsPage/jobsPage.spec.tsx
@@ -13,13 +13,12 @@ import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
 import { JobsPage, JobsPageProps } from "./jobsPage";
 import { formatDuration } from "../util/duration";
 import { allJobsFixture, earliestRetainedTime } from "./jobsPage.fixture";
-import { prettyDOM, prettyFormat, render } from "@testing-library/react";
+import { render } from "@testing-library/react";
 import React from "react";
 import { MemoryRouter } from "react-router-dom";
 import * as H from "history";
 
 import Job = cockroach.server.serverpb.IJobResponse;
-import { CoordinatedUniversalTime, TimezoneContext } from "src/contexts";
 
 const getMockJobsPageProps = (jobs: Array<Job>): JobsPageProps => {
   const history = H.createHashHistory();

diff --git a/pkg/ui/workspaces/cluster-ui/src/schedules/schedulesPage/schedulesPage.fixture.tsx b/pkg/ui/workspaces/cluster-ui/src/schedules/schedulesPage/schedulesPage.fixture.tsx
@@ -7,7 +7,6 @@
 // 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 * as protos from "@cockroachlabs/crdb-protobuf-client";
 import { createMemoryHistory } from "history";
 import Long from "long";
 import moment from "moment-timezone";

diff --git a/pkg/ui/workspaces/cluster-ui/src/schedules/schedulesPage/schedulesPage.tsx b/pkg/ui/workspaces/cluster-ui/src/schedules/schedulesPage/schedulesPage.tsx
@@ -14,7 +14,7 @@ import { Helmet } from "react-helmet";
 import { RouteComponentProps } from "react-router-dom";
 import { Schedules } from "src/api/schedulesApi";
 import { Delayed } from "src/delayed";
-import { Dropdown, DropdownOption } from "src/dropdown";
+import { Dropdown } from "src/dropdown";
 import { Loading } from "src/loading";
 import { PageConfig, PageConfigItem } from "src/pageConfig";
 import { SortSetting } from "src/sortedtable";

diff --git a/pkg/ui/workspaces/cluster-ui/src/selectors/activeExecutions.selectors.ts b/pkg/ui/workspaces/cluster-ui/src/selectors/activeExecutions.selectors.ts
@@ -9,12 +9,7 @@
 // licenses/APL.txt.
 
 import { createSelector } from "reselect";
-import {
-  ActiveExecutions,
-  ActiveTransaction,
-  ExecutionStatus,
-  ExecutionType,
-} from "src/activeExecutions/types";
+import { ActiveExecutions } from "src/activeExecutions/types";
 import { AppState } from "src/store";
 import { selectActiveExecutionsCombiner } from "src/selectors/activeExecutionsCommon.selectors";
 import { selectExecutionID } from "src/selectors/common";

diff --git a/pkg/ui/workspaces/cluster-ui/src/selectors/common.ts b/pkg/ui/workspaces/cluster-ui/src/selectors/common.ts
@@ -15,17 +15,7 @@ import {
   idAttr,
   statementAttr,
   txnFingerprintIdAttr,
-  unset,
-  ExecutionStatistics,
-  queryByName,
-  appAttr,
-  flattenStatementStats,
-  FixFingerprintHexValue,
 } from "src/util";
-import { createSelector } from "@reduxjs/toolkit";
-import { SqlStatsResponse } from "../api";
-import { AggregateStatistics } from "src/statementsTable";
-import { StatementDiagnosticsDictionary } from "src/store/statementDiagnostics";
 
 // The functions in this file are agnostic to the different shape of each
 // state in db-console and cluster-ui. This file contains selector functions

diff --git a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetailsPage.fixture.ts b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetailsPage.fixture.ts
@@ -46,8 +46,8 @@ const sessionDetailsPropsBase: SessionDetailsProps = {
   },
   setTimeScale: () => {},
   refreshSessions: () => {},
-  cancelSession: (req: CancelSessionRequestMessage) => {},
-  cancelQuery: (req: CancelQueryRequestMessage) => {},
+  cancelSession: (_req: CancelSessionRequestMessage) => {},
+  cancelQuery: (_req: CancelQueryRequestMessage) => {},
   refreshNodes: () => {},
   refreshNodesLiveness: () => {},
   uiConfig: {

diff --git a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionsPage.fixture.ts b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionsPage.fixture.ts
@@ -210,8 +210,8 @@ export const sessionsPagePropsFixture: SessionsPageProps = {
   columns: null,
   internalAppNamePrefix: "$ internal",
   refreshSessions: () => {},
-  cancelSession: (req: CancelSessionRequestMessage) => {},
-  cancelQuery: (req: CancelQueryRequestMessage) => {},
+  cancelSession: (_req: CancelSessionRequestMessage) => {},
+  cancelQuery: (_req: CancelQueryRequestMessage) => {},
   onSortingChange: () => {},
 };
 
@@ -239,7 +239,7 @@ export const sessionsPagePropsEmptyFixture: SessionsPageProps = {
   columns: null,
   internalAppNamePrefix: "$ internal",
   refreshSessions: () => {},
-  cancelSession: (req: CancelSessionRequestMessage) => {},
-  cancelQuery: (req: CancelQueryRequestMessage) => {},
+  cancelSession: (_req: CancelSessionRequestMessage) => {},
+  cancelQuery: (_req: CancelQueryRequestMessage) => {},
   onSortingChange: () => {},
 };
diff --git a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionsPageConnected.tsx b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionsPageConnected.tsx
@@ -75,7 +75,7 @@ export const selectFilters = createSelector(
 
 export const SessionsPageConnected = withRouter(
   connect(
-    (state: AppState, props: RouteComponentProps) => ({
+    (state: AppState, _props: RouteComponentProps) => ({
       sessions: selectSessions(state),
       internalAppNamePrefix: selectAppName(state),
       sessionsError: state.adminUI?.sessions.lastError,

diff --git a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionsTable.tsx b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionsTable.tsx
@@ -16,12 +16,7 @@ import {
   DurationToNumber,
   TimestampToMoment,
 } from "src/util/convert";
-import {
-  BytesWithPrecision,
-  Count,
-  DATE_FORMAT,
-  DATE_FORMAT_24_TZ,
-} from "src/util/format";
+import { BytesWithPrecision, Count, DATE_FORMAT } from "src/util/format";
 import { Link } from "react-router-dom";
 import React from "react";
 

diff --git a/pkg/ui/workspaces/cluster-ui/src/sortedtable/sortedtable.tsx b/pkg/ui/workspaces/cluster-ui/src/sortedtable/sortedtable.tsx
@@ -221,7 +221,7 @@ export class SortedTable<T> extends React.Component<
       data: T[],
       sortSetting: SortSetting,
       columns: ColumnDescriptor<T>[],
-      pagination?: ISortedTablePagination,
+      _pagination?: ISortedTablePagination,
     ): T[] => {
       if (!sortSetting) {
         return this.paginatedData();

diff --git a/pkg/ui/workspaces/cluster-ui/src/sqlActivity/errorComponent.tsx b/pkg/ui/workspaces/cluster-ui/src/sqlActivity/errorComponent.tsx
@@ -11,7 +11,7 @@
 import React from "react";
 import classNames from "classnames/bind";
 import styles from "./sqlActivity.module.scss";
-import moment, { Moment } from "moment-timezone";
+import moment from "moment-timezone";
 
 const cx = classNames.bind(styles);
 
@@ -50,7 +50,7 @@ export function mergeErrors(errs: Error | Error[]): Error {
   };
 
   errors.forEach(
-    (x, i, arr) => (
+    (x, i) => (
       (mergedError.name += ` ${i}: ${x.name};`),
       (mergedError.message += ` ${i}: ${x.message};`)
     ),