ui: show data when max size reached for schema insight

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 commit updates the Insights Schema page with the new behaviour. The query to retrieve the drop recommendations was returning all indexes and the ui was doing the filtering. This commit also changes the query to only return the indexes with a drop recommendation, resulting in a lot less data being sent with the sql api and causing less size limit reached. Then the ui just needs to decided the type of drop. Part Of: cockroachdb#96184 Release note (ui change): Still show data on the console (with a warning) for Schema Insights when we reach a "max size exceed" error from the sql api.
maryliag · Feb 21, 2023 · b44ec53 · b44ec53
1 parent c00dbe8
commit b44ec53
Show file tree

Hide file tree

Showing 12 changed files with 105 additions and 87 deletions.
diff --git a/pkg/ui/workspaces/cluster-ui/src/api/schemaInsightsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/schemaInsightsApi.ts
@@ -14,7 +14,9 @@ import {
   executeInternalSql,
   LONG_TIMEOUT,
   sqlResultsAreEmpty,
-  sqlApiErrorMessage,
+  LARGE_RESULT_SIZE,
+  SqlApiResponse,
+  formatApiResult,
 } from "./sqlApi";
 import {
   InsightRecommendation,
@@ -128,23 +130,37 @@ function createIndexRecommendationsToSchemaInsight(
   return results;
 }
 
+// This query have an ORDER BY for the cases where we reach the limit of the sql-api
+// and want to return the most used ones as a priority.
 const dropUnusedIndexQuery: SchemaInsightQuery<ClusterIndexUsageStatistic> = {
   name: "DropIndex",
-  query: `SELECT
-            us.table_id,
-            us.index_id,
-            us.last_read,
-            ti.created_at,
-            ti.index_name,
-            t.name as table_name,
-            t.parent_id as database_id,
-            t.database_name,
-            t.schema_name,
-            (SELECT value FROM crdb_internal.cluster_settings WHERE variable = 'sql.index_recommendation.drop_unused_duration') AS unused_threshold
-          FROM "".crdb_internal.index_usage_statistics AS us
-                 JOIN "".crdb_internal.table_indexes as ti ON us.index_id = ti.index_id AND us.table_id = ti.descriptor_id
-                 JOIN "".crdb_internal.tables as t ON t.table_id = ti.descriptor_id and t.name = ti.descriptor_name
-          WHERE t.database_name != 'system' AND ti.index_type != 'primary';`,
+  query: `WITH cs AS (
+    SELECT value 
+        FROM crdb_internal.cluster_settings 
+    WHERE variable = 'sql.index_recommendation.drop_unused_duration'
+    )
+    SELECT * FROM (SELECT us.table_id,
+                          us.index_id,
+                          us.last_read,
+                          us.total_reads,
+                          ti.created_at,
+                          ti.index_name,
+                          t.name      as table_name,
+                          t.parent_id as database_id,
+                          t.database_name,
+                          t.schema_name,
+                          cs.value as unused_threshold,
+                          cs.value::interval as interval_threshold, 
+                          now() - COALESCE(us.last_read AT TIME ZONE 'UTC', COALESCE(ti.created_at, '0001-01-01')) as unused_interval
+                   FROM "".crdb_internal.index_usage_statistics AS us
+                            JOIN "".crdb_internal.table_indexes as ti
+                                 ON us.index_id = ti.index_id AND us.table_id = ti.descriptor_id
+                            JOIN "".crdb_internal.tables as t
+                                 ON t.table_id = ti.descriptor_id and t.name = ti.descriptor_name
+                            CROSS JOIN cs
+                   WHERE t.database_name != 'system' AND ti.index_type != 'primary')
+          WHERE unused_interval > interval_threshold
+          ORDER BY total_reads DESC;`,
   toSchemaInsight: clusterIndexUsageStatsToSchemaInsight,
 };
 
@@ -181,26 +197,22 @@ const schemaInsightQueries: SchemaInsightQuery<SchemaInsightResponse>[] = [
 
 // getSchemaInsights makes requests over the SQL API and transforms the corresponding
 // SQL responses into schema insights.
-export async function getSchemaInsights(): Promise<InsightRecommendation[]> {
+export async function getSchemaInsights(): Promise<
+  SqlApiResponse<InsightRecommendation[]>
+> {
   const request: SqlExecutionRequest = {
     statements: schemaInsightQueries.map(insightQuery => ({
       sql: insightQuery.query,
     })),
     execute: true,
+    max_result_size: LARGE_RESULT_SIZE,
     timeout: LONG_TIMEOUT,
   };
   const result = await executeInternalSql<SchemaInsightResponse>(request);
-  if (result.error) {
-    throw new Error(
-      `Error while retrieving insights information: ${sqlApiErrorMessage(
-        result.error.message,
-      )}`,
-    );
-  }
+
   const results: InsightRecommendation[] = [];
   if (sqlResultsAreEmpty(result)) {
-    // No data.
-    return results;
+    return formatApiResult([], result.error, "retrieving insights information");
   }
   result.execution.txn_results.map(txn_result => {
     // Note: txn_result.statement values begin at 1, not 0.
@@ -210,5 +222,9 @@ export async function getSchemaInsights(): Promise<InsightRecommendation[]> {
       results.push(...insightQuery.toSchemaInsight(txn_result));
     }
   });
-  return results;
+  return formatApiResult(
+    results,
+    result.error,
+    "retrieving insights information",
+  );
 }
diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/indexUsageStatsRec.spec.ts b/pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/indexUsageStatsRec.spec.ts
@@ -20,26 +20,6 @@ describe("recommendDropUnusedIndex", () => {
   const mockCurrentTime = moment();
   const oneHourAgo: moment.Moment = moment(mockCurrentTime).subtract(1, "hour");
 
-  describe("Recently Used Index", () => {
-    const recentlyUsedIndex: ClusterIndexUsageStatistic = {
-      table_id: 1,
-      index_id: 1,
-      last_read: moment.utc(oneHourAgo, "X").format(),
-      created_at: null,
-      index_name: "recent_index",
-      table_name: "test_table",
-      database_id: 1,
-      database_name: "test_db",
-      schema_name: "public",
-      unused_threshold: "10h0m0s",
-    };
-    it("should not recommend index to be dropped", () => {
-      expect(recommendDropUnusedIndex(recentlyUsedIndex)).toEqual({
-        recommend: false,
-        reason: "",
-      });
-    });
-  });
   describe("Never Used Index", () => {
     const neverUsedIndex: ClusterIndexUsageStatistic = {
       table_id: 1,
@@ -85,28 +65,6 @@ describe("recommendDropUnusedIndex", () => {
     });
   });
   describe("Index Created But Never Read", () => {
-    describe("creation date does not exceed unuse duration", () => {
-      const createdNeverReadIndexNoExceed: ClusterIndexUsageStatistic = {
-        table_id: 1,
-        index_id: 1,
-        last_read: null,
-        created_at: moment.utc(oneHourAgo, "X").format(),
-        index_name: "recent_index",
-        table_name: "test_table",
-        database_id: 1,
-        database_name: "test_db",
-        schema_name: "public",
-        unused_threshold: "10h0m0s",
-      };
-      it("should not recommend index to be dropped", () => {
-        expect(recommendDropUnusedIndex(createdNeverReadIndexNoExceed)).toEqual(
-          {
-            recommend: false,
-            reason: "",
-          },
-        );
-      });
-    });
     describe("creation date exceeds unuse duration", () => {
       const createdNeverReadIndexExceed: ClusterIndexUsageStatistic = {
         table_id: 1,

diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/indexUsageStatsRec.ts b/pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/indexUsageStatsRec.ts
@@ -39,19 +39,15 @@ export function recommendDropUnusedIndex(
     return { recommend: true, reason: indexNeverUsedReason };
   }
 
-  const duration = moment.duration(moment().diff(lastActive));
   const unusedThreshold = moment.duration(
     "PT" + clusterIndexUsageStat.unused_threshold.toUpperCase(),
   );
-  if (duration >= unusedThreshold) {
-    return {
-      recommend: true,
-      reason: `This index has not been used in over ${formatMomentDuration(
-        unusedThreshold,
-      )} and can be removed for better write performance.`,
-    };
-  }
-  return { recommend: false, reason: "" };
+  return {
+    recommend: true,
+    reason: `This index has not been used in over ${formatMomentDuration(
+      unusedThreshold,
+    )} and can be removed for better write performance.`,
+  };
 }
 
 export function formatMomentDuration(duration: moment.Duration): string {

diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/schemaInsightsPageConnected.tsx b/pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/schemaInsightsPageConnected.tsx
@@ -15,6 +15,7 @@ import {
   selectSchemaInsights,
   selectSchemaInsightsDatabases,
   selectSchemaInsightsError,
+  selectSchemaInsightsMaxApiSizeReached,
   selectSchemaInsightsTypes,
   selectFilters,
   selectSortSetting,
@@ -43,6 +44,7 @@ const mapStateToProps = (
   filters: selectFilters(state),
   sortSetting: selectSortSetting(state),
   hasAdminRole: selectHasAdminRole(state),
+  maxSizeApiReached: selectSchemaInsightsMaxApiSizeReached(state),
 });
 
 const mapDispatchToProps = (

diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/schemaInsightsView.tsx b/pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/schemaInsightsView.tsx
@@ -38,6 +38,9 @@ import { InsightsError } from "../insightsErrorComponent";
 import { Pagination } from "../../pagination";
 import { EmptySchemaInsightsTablePlaceholder } from "./emptySchemaInsightsTablePlaceholder";
 import { CockroachCloudContext } from "../../contexts";
+import { InlineAlert } from "@cockroachlabs/ui-components";
+import { insights } from "src/util";
+import { Anchor } from "src/anchor";
 import insightTableStyles from "../../insightsTable/insightsTable.module.scss";
 const cx = classNames.bind(styles);
 const sortableTableCx = classNames.bind(sortableTableStyles);
@@ -51,6 +54,7 @@ export type SchemaInsightsViewStateProps = {
   filters: SchemaInsightEventFilters;
   sortSetting: SortSetting;
   hasAdminRole: boolean;
+  maxSizeApiReached?: boolean;
 };
 
 export type SchemaInsightsViewDispatchProps = {
@@ -77,6 +81,7 @@ export const SchemaInsightsView: React.FC<SchemaInsightsViewProps> = ({
   refreshUserSQLRoles,
   onFiltersChange,
   onSortChange,
+  maxSizeApiReached,
 }: SchemaInsightsViewProps) => {
   const isCockroachCloud = useContext(CockroachCloudContext);
   const [pagination, setPagination] = useState<ISortedTablePagination>({
@@ -259,6 +264,20 @@ export const SchemaInsightsView: React.FC<SchemaInsightsViewProps> = ({
               total={filteredSchemaInsights?.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/insightsTable/insightsTable.tsx b/pkg/ui/workspaces/cluster-ui/src/insightsTable/insightsTable.tsx
@@ -300,7 +300,7 @@ function descriptionCell(
 }
 
 function linkCell(insightRec: InsightRecommendation): React.ReactElement {
-  switch (insightRec.execution.execType) {
+  switch (insightRec.execution?.execType) {
     case InsightExecEnum.STATEMENT:
       return (
         <>

diff --git a/pkg/ui/workspaces/cluster-ui/src/store/schemaInsights/schemaInsights.reducer.ts b/pkg/ui/workspaces/cluster-ui/src/store/schemaInsights/schemaInsights.reducer.ts
@@ -12,9 +12,10 @@ import { createSlice, PayloadAction } from "@reduxjs/toolkit";
 import { DOMAIN_NAME, noopReducer } from "../utils";
 import moment, { Moment } from "moment";
 import { InsightRecommendation } from "../../insights";
+import { SqlApiResponse } from "src/api";
 
 export type SchemaInsightsState = {
-  data: InsightRecommendation[];
+  data: SqlApiResponse<InsightRecommendation[]>;
   lastUpdated: Moment;
   lastError: Error;
   valid: boolean;
@@ -31,7 +32,10 @@ const schemaInsightsSlice = createSlice({
   name: `${DOMAIN_NAME}/schemaInsightsSlice`,
   initialState,
   reducers: {
-    received: (state, action: PayloadAction<InsightRecommendation[]>) => {
+    received: (
+      state,
+      action: PayloadAction<SqlApiResponse<InsightRecommendation[]>>,
+    ) => {
       state.data = action.payload;
       state.valid = true;
       state.lastError = null;

diff --git a/pkg/ui/workspaces/cluster-ui/src/store/schemaInsights/schemaInsights.sagas.spec.ts b/pkg/ui/workspaces/cluster-ui/src/store/schemaInsights/schemaInsights.sagas.spec.ts
@@ -16,7 +16,7 @@ import {
 } from "redux-saga-test-plan/providers";
 import * as matchers from "redux-saga-test-plan/matchers";
 import moment from "moment";
-import { getSchemaInsights } from "../../api";
+import { getSchemaInsights, SqlApiResponse } from "../../api";
 import {
   refreshSchemaInsightsSaga,
   requestSchemaInsightsSaga,
@@ -40,7 +40,7 @@ describe("SchemaInsights sagas", () => {
     spy.mockRestore();
   });
 
-  const schemaInsightsResponse: InsightRecommendation[] = [
+  const schemaInsights: InsightRecommendation[] = [
     {
       type: "DropIndex",
       database: "test_database",
@@ -55,6 +55,11 @@ describe("SchemaInsights sagas", () => {
     },
   ];
 
+  const schemaInsightsResponse: SqlApiResponse<InsightRecommendation[]> = {
+    maxSizeReached: false,
+    results: schemaInsights,
+  };
+
   const schemaInsightsAPIProvider: (EffectProviders | StaticProvider)[] = [
     [matchers.call.fn(getSchemaInsights), schemaInsightsResponse],
   ];

diff --git a/pkg/ui/workspaces/cluster-ui/src/store/schemaInsights/schemaInsights.selectors.ts b/pkg/ui/workspaces/cluster-ui/src/store/schemaInsights/schemaInsights.selectors.ts
@@ -24,7 +24,7 @@ export const selectSchemaInsights = createSelector(
   selectSchemaInsightState,
   schemaInsightState => {
     if (!schemaInsightState.data) return null;
-    return schemaInsightState.data;
+    return schemaInsightState.data?.results;
   },
 );
 
@@ -36,6 +36,14 @@ export const selectSchemaInsightsError = createSelector(
   },
 );
 
+export const selectSchemaInsightsMaxApiSizeReached = createSelector(
+  selectSchemaInsightState,
+  schemaInsightState => {
+    if (!schemaInsightState.data) return false;
+    return schemaInsightState.data?.maxSizeReached;
+  },
+);
+
 export const selectSchemaInsightsDatabases = createSelector(
   selectSchemaInsights,
   schemaInsights => {

diff --git a/pkg/ui/workspaces/db-console/src/redux/apiReducers.ts b/pkg/ui/workspaces/db-console/src/redux/apiReducers.ts
@@ -566,7 +566,9 @@ export interface APIReducersState {
   txnInsights: CachedDataReducerState<
     clusterUiApi.SqlApiResponse<TxnInsightEvent[]>
   >;
-  schemaInsights: CachedDataReducerState<clusterUiApi.InsightRecommendation[]>;
+  schemaInsights: CachedDataReducerState<
+    clusterUiApi.SqlApiResponse<clusterUiApi.InsightRecommendation[]>
+  >;
   statementFingerprintInsights: KeyedCachedDataReducerState<
     clusterUiApi.SqlApiResponse<StmtInsightEvent[]>
   >;

diff --git a/pkg/ui/workspaces/db-console/src/views/insights/insightsSelectors.ts b/pkg/ui/workspaces/db-console/src/views/insights/insightsSelectors.ts
@@ -179,10 +179,16 @@ export const selectSchemaInsights = createSelector(
   (state: AdminUIState) => state.cachedData,
   adminUiState => {
     if (!adminUiState.schemaInsights) return [];
-    return adminUiState.schemaInsights.data;
+    return adminUiState.schemaInsights.data?.results;
   },
 );
 
+export const selectSchemaInsightsMaxApiReached = (
+  state: AdminUIState,
+): boolean => {
+  return !!state.cachedData.schemaInsights?.data?.maxSizeReached;
+};
+
 export const selectSchemaInsightsDatabases = createSelector(
   selectSchemaInsights,
   schemaInsights => {

diff --git a/pkg/ui/workspaces/db-console/src/views/insights/schemaInsightsPage.tsx b/pkg/ui/workspaces/db-console/src/views/insights/schemaInsightsPage.tsx
@@ -27,6 +27,7 @@ import {
   schemaInsightsSortLocalSetting,
   selectSchemaInsights,
   selectSchemaInsightsDatabases,
+  selectSchemaInsightsMaxApiReached,
   selectSchemaInsightsTypes,
 } from "src/views/insights/insightsSelectors";
 import { selectHasAdminRole } from "src/redux/user";
@@ -42,6 +43,7 @@ const mapStateToProps = (
   filters: schemaInsightsFiltersLocalSetting.selector(state),
   sortSetting: schemaInsightsSortLocalSetting.selector(state),
   hasAdminRole: selectHasAdminRole(state),
+  maxSizeApiReached: selectSchemaInsightsMaxApiReached(state),
 });
 
 const mapDispatchToProps = {