Merge cockroachdb#97153 cockroachdb#97229

97153: ui: show data when max size reached r=maryliag a=maryliag Previously, when the sql api returned a max size reached error, we were just showing the error, but not the data that was also being returned. This PR creates a new function to format the return of the api calls, so when is a max size error it doesn't throw an error, but still pass that info so we can display a warning on the pages. This commit updates the Insights Workload > Statement page with the new behaviour. Following PRs will update other usages of the sql api. <img width="1252" alt="Screenshot 2023-02-16 at 11 06 23 AM" src="https://user-images.githubusercontent.com/1017486/219422728-fe21a5ab-a6e1-4c27-94d2-d1ec482cac4c.png"> https://www.loom.com/share/9d9a24c486ce466ab355e1040af095c9 Part Of: cockroachdb#96184 Release note (ui change): Still show data on the console (with a warning) for Statement Insights when we reach a "max size exceed" error from the sql api. 97229: go.mod: bump Pebble to 9b1142a5070e r=RaduBerinde a=RaduBerinde 9b1142a5 db: add ErrorIfNotPristine option fbbc39d1 sstable: add readahead for value blocks Release note: Epic: none Co-authored-by: maryliag <[email protected]> Co-authored-by: Radu Berinde <[email protected]>
ericharmeling · Feb 16, 2023 · 377fe75 · 377fe75
3 parents 142eb4e + a6fd886 + 5561bdf
commit 377fe75
Show file tree

Hide file tree

Showing 17 changed files with 101 additions and 33 deletions.
diff --git a/DEPS.bzl b/DEPS.bzl
@@ -1555,10 +1555,10 @@ def go_deps():
         patches = [
             "@com_github_cockroachdb_cockroach//build/patches:com_github_cockroachdb_pebble.patch",
         ],
-        sha256 = "827af6eb58999bd80dd1ffc82a45697d1047dc3edd8c04563dd5e2e8264cec17",
-        strip_prefix = "github.com/cockroachdb/[email protected]20230215164001-c99dee64598f",
+        sha256 = "84df44fb4599cf685813a830093a9db1c975c093fe095ec5144020aa7504d0c2",
+        strip_prefix = "github.com/cockroachdb/[email protected]20230216002728-9b1142a5070e",
         urls = [
-            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230215164001-c99dee64598f.zip",
+            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230216002728-9b1142a5070e.zip",
         ],
     )
     go_repository(

diff --git a/build/bazelutil/distdir_files.bzl b/build/bazelutil/distdir_files.bzl
@@ -202,7 +202,7 @@ DISTDIR_FILES = {
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/google-api-go-client/com_github_cockroachdb_google_api_go_client-v0.80.1-0.20221117193156-6a9f7150cb93.zip": "b3378c579f4f4340403038305907d672c86f615f8233118a8873ebe4229c4f39",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/gostdlib/com_github_cockroachdb_gostdlib-v1.19.0.zip": "c4d516bcfe8c07b6fc09b8a9a07a95065b36c2855627cb3514e40c98f872b69e",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/logtags/com_github_cockroachdb_logtags-v0.0.0-20230118201751-21c54148d20b.zip": "ca7776f47e5fecb4c495490a679036bfc29d95bd7625290cfdb9abb0baf97476",
-    "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230215164001-c99dee64598f.zip": "827af6eb58999bd80dd1ffc82a45697d1047dc3edd8c04563dd5e2e8264cec17",
+    "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230216002728-9b1142a5070e.zip": "84df44fb4599cf685813a830093a9db1c975c093fe095ec5144020aa7504d0c2",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/redact/com_github_cockroachdb_redact-v1.1.3.zip": "7778b1e4485e4f17f35e5e592d87eb99c29e173ac9507801d000ad76dd0c261e",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/returncheck/com_github_cockroachdb_returncheck-v0.0.0-20200612231554-92cdbca611dd.zip": "ce92ba4352deec995b1f2eecf16eba7f5d51f5aa245a1c362dfe24c83d31f82b",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/sentry-go/com_github_cockroachdb_sentry_go-v0.6.1-cockroachdb.2.zip": "fbb2207d02aecfdd411b1357efe1192dbb827959e36b7cab7491731ac55935c9",

diff --git a/go.mod b/go.mod
@@ -114,7 +114,7 @@ require (
 	github.com/cockroachdb/go-test-teamcity v0.0.0-20191211140407-cff980ad0a55
 	github.com/cockroachdb/gostdlib v1.19.0
 	github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b
-	github.com/cockroachdb/pebble v0.0.0-20230215164001-c99dee64598f
+	github.com/cockroachdb/pebble v0.0.0-20230216002728-9b1142a5070e
 	github.com/cockroachdb/redact v1.1.3
 	github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd
 	github.com/cockroachdb/stress v0.0.0-20220803192808-1806698b1b7b

diff --git a/go.sum b/go.sum
@@ -492,8 +492,8 @@ github.com/cockroachdb/gostdlib v1.19.0/go.mod h1:+dqqpARXbE/gRDEhCak6dm0l14AaTy
 github.com/cockroachdb/logtags v0.0.0-20211118104740-dabe8e521a4f/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs=
 github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b h1:r6VH0faHjZeQy818SGhaone5OnYfxFR/+AzdY3sf5aE=
 github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs=
-github.com/cockroachdb/pebble v0.0.0-20230215164001-c99dee64598f h1:y9LPHH46jWopzFm67GWacLdIgsiALGqPgHjD1cysZSM=
-github.com/cockroachdb/pebble v0.0.0-20230215164001-c99dee64598f/go.mod h1:9lRMC4XN3/BLPtIp6kAKwIaHu369NOf2rMucPzipz50=
+github.com/cockroachdb/pebble v0.0.0-20230216002728-9b1142a5070e h1:++hK+cy7bPIqlvaZDA3zPcwY+m//toJGKqgzWaQjVzI=
+github.com/cockroachdb/pebble v0.0.0-20230216002728-9b1142a5070e/go.mod h1:9lRMC4XN3/BLPtIp6kAKwIaHu369NOf2rMucPzipz50=
 github.com/cockroachdb/redact v1.1.3 h1:AKZds10rFSIj7qADf0g46UixK8NNLwWTNdCIGS5wfSQ=
 github.com/cockroachdb/redact v1.1.3/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=
 github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd h1:KFOt5I9nEKZgCnOSmy8r4Oykh8BYQO8bFOTgHDS8YZA=

diff --git a/pkg/ui/workspaces/cluster-ui/src/api/sqlApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/sqlApi.ts
@@ -60,6 +60,11 @@ export type SqlExecutionErrorMessage = {
   source: { file: string; line: number; function: "string" };
 };
 
+export type ApiResponse<ResultType> = {
+  maxSizeReached: boolean;
+  results: Array<ResultType>;
+};
+
 export const SQL_API_PATH = "/api/v2/sql/";
 
 /**
@@ -159,3 +164,28 @@ export function sqlApiErrorMessage(message: string): string {
 
   return message;
 }
+
+function isMaxSizeError(message: string): boolean {
+  return !!message?.includes("max result size exceeded");
+}
+
+export function formatApiResult(
+  results: Array<any>,
+  error: SqlExecutionErrorMessage,
+  errorMessageContext: string,
+): ApiResponse<any> {
+  const maxSizeError = isMaxSizeError(error?.message);
+
+  if (error && !maxSizeError) {
+    throw new Error(
+      `Error while ${errorMessageContext}: ${sqlApiErrorMessage(
+        error?.message,
+      )}`,
+    );
+  }
+
+  return {
+    maxSizeReached: maxSizeError,
+    results: results,
+  };
+}
diff --git a/pkg/ui/workspaces/cluster-ui/src/api/stmtInsightsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/stmtInsightsApi.ts
@@ -9,10 +9,11 @@
 // licenses/APL.txt.
 
 import {
+  ApiResponse,
   executeInternalSql,
+  formatApiResult,
   LARGE_RESULT_SIZE,
   LONG_TIMEOUT,
-  sqlApiErrorMessage,
   SqlExecutionRequest,
   sqlResultsAreEmpty,
   SqlTxnResult,
@@ -137,7 +138,7 @@ export const stmtInsightsByTxnExecutionQuery = (id: string): string => `
 
 export async function getStmtInsightsApi(
   req?: StmtInsightsReq,
-): Promise<StmtInsightEvent[]> {
+): Promise<ApiResponse<StmtInsightEvent>> {
   const request: SqlExecutionRequest = {
     statements: [
       {
@@ -150,21 +151,17 @@ export async function getStmtInsightsApi(
   };
 
   const result = await executeInternalSql<StmtInsightsResponseRow>(request);
-  if (result.error) {
-    throw new Error(
-      `Error while retrieving insights information: ${sqlApiErrorMessage(
-        result.error.message,
-      )}`,
-    );
-  }
 
   if (sqlResultsAreEmpty(result)) {
-    return [];
+    return formatApiResult([], result.error, "retrieving insights information");
   }
-
   const stmtInsightEvent = formatStmtInsights(result.execution?.txn_results[0]);
   await addStmtContentionInfoApi(stmtInsightEvent);
-  return stmtInsightEvent;
+  return formatApiResult(
+    stmtInsightEvent,
+    result.error,
+    "retrieving insights information",
+  );
 }
 
 async function addStmtContentionInfoApi(

diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/statementInsightDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsightDetails/statementInsightDetails.tsx
@@ -124,7 +124,7 @@ export const StatementInsightDetails: React.FC<
     getStmtInsightsApi({ stmtExecutionID: executionID, start, end })
       .then(res => {
         setInsightDetails({
-          details: res?.length ? res[0] : null,
+          details: res?.results?.length ? res.results[0] : null,
           loaded: true,
         });
       })

diff --git a/...aces/cluster-ui/src/insights/workloadInsights/statementInsights/statementInsightsView.tsx b/...aces/cluster-ui/src/insights/workloadInsights/statementInsights/statementInsightsView.tsx
@@ -56,6 +56,9 @@ import styles from "src/statementsPage/statementsPage.module.scss";
 import sortableTableStyles from "src/sortedtable/sortedtable.module.scss";
 import { commonStyles } from "../../../common";
 import { useFetchDataWithPolling } from "src/util/hooks";
+import { InlineAlert } from "@cockroachlabs/ui-components";
+import { insights } from "src/util";
+import { Anchor } from "src/anchor";
 
 const cx = classNames.bind(styles);
 const sortableTableCx = classNames.bind(sortableTableStyles);
@@ -72,6 +75,7 @@ export type StatementInsightsViewStateProps = {
   isLoading?: boolean;
   dropDownSelect?: React.ReactElement;
   timeScale?: TimeScale;
+  maxSizeApiReached?: boolean;
 };
 
 export type StatementInsightsViewDispatchProps = {
@@ -105,6 +109,7 @@ export const StatementInsightsView: React.FC<StatementInsightsViewProps> = ({
   setTimeScale,
   selectedColumnNames,
   dropDownSelect,
+  maxSizeApiReached,
 }: StatementInsightsViewProps) => {
   const [pagination, setPagination] = useState<ISortedTablePagination>({
     current: 1,
@@ -317,6 +322,20 @@ export const StatementInsightsView: React.FC<StatementInsightsViewProps> = ({
               total={filteredStatements?.length}
               onChange={onChangePage}
             />
+            {maxSizeApiReached && (
+              <InlineAlert
+                intent="info"
+                title={
+                  <>
+                    Not all insights are displayed because the maximum number of
+                    insights was reached in the console.&nbsp;
+                    <Anchor href={insights} target="_blank">
+                      Learn more
+                    </Anchor>
+                  </>
+                }
+              />
+            )}
           </div>
         </Loading>
       </div>

diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsights/workloadInsightsPageConnected.tsx b/pkg/ui/workspaces/cluster-ui/src/insights/workloadInsights/workloadInsightsPageConnected.tsx
@@ -34,6 +34,7 @@ import {
   selectStmtInsights,
   selectStmtInsightsError,
   selectStmtInsightsLoading,
+  selectStmtInsightsMaxApiReached,
 } from "src/store/insights/statementInsights";
 import {
   actions as txnInsights,
@@ -78,6 +79,7 @@ const statementMapStateToProps = (
   selectedColumnNames: selectColumns(state),
   timeScale: selectTimeScale(state),
   isLoading: selectStmtInsightsLoading(state),
+  maxSizeApiReached: selectStmtInsightsMaxApiReached(state),
 });
 
 const TransactionDispatchProps = (

diff --git a/...i/src/store/insights/statementFingerprintInsights/statementFingerprintInsights.reducer.ts b/...i/src/store/insights/statementFingerprintInsights/statementFingerprintInsights.reducer.ts
@@ -11,11 +11,11 @@
 import { createSlice, PayloadAction } from "@reduxjs/toolkit";
 import { DOMAIN_NAME } from "../../utils";
 import moment, { Moment } from "moment";
-import { ErrorWithKey, StmtInsightsReq } from "src/api";
+import { ApiResponse, ErrorWithKey, StmtInsightsReq } from "src/api";
 import { StmtInsightEvent } from "../../../insights";
 
 export type StatementFingerprintInsightsState = {
-  data: StmtInsightEvent[] | null;
+  data: ApiResponse<StmtInsightEvent> | null;
   lastUpdated: Moment | null;
   lastError: Error;
   valid: boolean;
@@ -26,7 +26,7 @@ export type StatementFingerprintInsightsCachedState = {
 };
 
 export type FingerprintInsightResponseWithKey = {
-  response: StmtInsightEvent[];
+  response: ApiResponse<StmtInsightEvent>;
   key: string;
 };
 

diff --git a/...src/store/insights/statementFingerprintInsights/statementFingerprintInsights.selectors.ts b/...src/store/insights/statementFingerprintInsights/statementFingerprintInsights.selectors.ts
@@ -20,6 +20,6 @@ export const selectStatementFingerprintInsights = createSelector(
     if (!cachedFingerprintInsights) {
       return null;
     }
-    return cachedFingerprintInsights[fingerprintID]?.data;
+    return cachedFingerprintInsights[fingerprintID]?.data?.results;
   },
 );
diff --git a/...i/workspaces/cluster-ui/src/store/insights/statementInsights/statementInsights.reducer.ts b/...i/workspaces/cluster-ui/src/store/insights/statementInsights/statementInsights.reducer.ts
@@ -11,11 +11,11 @@
 import { createSlice, PayloadAction } from "@reduxjs/toolkit";
 import { DOMAIN_NAME } from "../../utils";
 import { StmtInsightEvent } from "src/insights";
-import { StmtInsightsReq } from "src/api";
+import { ApiResponse, StmtInsightsReq } from "src/api";
 import moment from "moment";
 
 export type StmtInsightsState = {
-  data: StmtInsightEvent[];
+  data: ApiResponse<StmtInsightEvent>;
   lastError: Error;
   valid: boolean;
   inFlight: boolean;
@@ -34,7 +34,7 @@ const statementInsightsSlice = createSlice({
   name: `${DOMAIN_NAME}/statementInsightsSlice`,
   initialState,
   reducers: {
-    received: (state, action: PayloadAction<StmtInsightEvent[]>) => {
+    received: (state, action: PayloadAction<ApiResponse<StmtInsightEvent>>) => {
       state.data = action.payload;
       state.valid = true;
       state.lastError = null;

diff --git a/...workspaces/cluster-ui/src/store/insights/statementInsights/statementInsights.selectors.ts b/...workspaces/cluster-ui/src/store/insights/statementInsights/statementInsights.selectors.ts
@@ -20,11 +20,14 @@ import { selectStatementFingerprintID, selectID } from "src/selectors/common";
 import { InsightEnumToLabel, StmtInsightEvent } from "src/insights";
 
 export const selectStmtInsights = (state: AppState): StmtInsightEvent[] =>
-  state.adminUI.stmtInsights?.data;
+  state.adminUI.stmtInsights?.data?.results;
 
 export const selectStmtInsightsError = (state: AppState): Error | null =>
   state.adminUI.stmtInsights?.lastError;
 
+export const selectStmtInsightsMaxApiReached = (state: AppState): boolean =>
+  state.adminUI.stmtInsights?.data?.maxSizeReached;
+
 export const selectStmtInsightDetails = createSelector(
   selectStmtInsights,
   selectID,

diff --git a/pkg/ui/workspaces/db-console/src/redux/apiReducers.ts b/pkg/ui/workspaces/db-console/src/redux/apiReducers.ts
@@ -557,11 +557,15 @@ export interface APIReducersState {
   userSQLRoles: CachedDataReducerState<api.UserSQLRolesResponseMessage>;
   hotRanges: PaginatedCachedDataReducerState<api.HotRangesV2ResponseMessage>;
   clusterLocks: CachedDataReducerState<clusterUiApi.ClusterLocksResponse>;
-  stmtInsights: CachedDataReducerState<StmtInsightEvent[]>;
+  stmtInsights: CachedDataReducerState<
+    clusterUiApi.ApiResponse<StmtInsightEvent>
+  >;
   txnInsightDetails: KeyedCachedDataReducerState<clusterUiApi.TxnInsightDetailsResponse>;
   txnInsights: CachedDataReducerState<TxnInsightEvent[]>;
   schemaInsights: CachedDataReducerState<clusterUiApi.InsightRecommendation[]>;
-  statementFingerprintInsights: KeyedCachedDataReducerState<StmtInsightEvent[]>;
+  statementFingerprintInsights: KeyedCachedDataReducerState<
+    clusterUiApi.ApiResponse<StmtInsightEvent>
+  >;
   schedules: KeyedCachedDataReducerState<clusterUiApi.Schedules>;
   schedule: KeyedCachedDataReducerState<clusterUiApi.Schedule>;
   snapshots: KeyedCachedDataReducerState<clusterUiApi.ListTracingSnapshotsResponse>;

diff --git a/pkg/ui/workspaces/db-console/src/views/insights/insightsSelectors.ts b/pkg/ui/workspaces/db-console/src/views/insights/insightsSelectors.ts
@@ -70,8 +70,19 @@ const selectTxnInsight = createSelector(
   },
 );
 
-export const selectStmtInsights = (state: AdminUIState) =>
-  state.cachedData.stmtInsights?.data;
+export const selectStmtInsights = (state: AdminUIState) => {
+  return state.cachedData.stmtInsights?.data
+    ? state.cachedData.stmtInsights.data["results"]
+    : null;
+};
+
+export const selectStmtInsightsMaxApiReached = (
+  state: AdminUIState,
+): boolean => {
+  return state.cachedData.stmtInsights?.data
+    ? state.cachedData.stmtInsights?.data["maxSizeReached"]
+    : false;
+};
 
 export const selectTxnInsightDetails = createSelector(
   selectTxnInsight,

diff --git a/pkg/ui/workspaces/db-console/src/views/insights/workloadInsightsPage.tsx b/pkg/ui/workspaces/db-console/src/views/insights/workloadInsightsPage.tsx
@@ -30,6 +30,7 @@ import {
   selectStmtInsightsLoading,
   selectTransactionInsightsLoading,
   selectInsightTypes,
+  selectStmtInsightsMaxApiReached,
 } from "src/views/insights/insightsSelectors";
 import { bindActionCreators } from "redux";
 import { LocalSetting } from "src/redux/localsettings";
@@ -75,6 +76,7 @@ const statementMapStateToProps = (
     insightStatementColumnsLocalSetting.selectorToArray(state),
   timeScale: selectTimeScale(state),
   isLoading: selectStmtInsightsLoading(state),
+  maxSizeApiReached: selectStmtInsightsMaxApiReached(state),
 });
 
 const TransactionDispatchProps = {

diff --git a/pkg/ui/workspaces/db-console/src/views/statements/statementDetails.tsx b/pkg/ui/workspaces/db-console/src/views/statements/statementDetails.tsx
@@ -108,7 +108,7 @@ const selectStatementFingerprintInsights = createSelector(
   (_state: AdminUIState, props: RouteComponentProps): string =>
     getMatchParamByName(props.match, statementAttr),
   (cachedFingerprintInsights, fingerprintID) => {
-    return cachedFingerprintInsights[fingerprintID]?.data;
+    return cachedFingerprintInsights[fingerprintID]?.data?.results;
   },
 );