From b44ec53e767d217f0000f344d5c41df382669f84 Mon Sep 17 00:00:00 2001 From: maryliag Date: Fri, 17 Feb 2023 17:54:14 -0500 Subject: [PATCH] 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: #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. --- .../cluster-ui/src/api/schemaInsightsApi.ts | 70 ++++++++++++------- .../schemaInsights/indexUsageStatsRec.spec.ts | 42 ----------- .../schemaInsights/indexUsageStatsRec.ts | 16 ++--- .../schemaInsightsPageConnected.tsx | 2 + .../schemaInsights/schemaInsightsView.tsx | 19 +++++ .../src/insightsTable/insightsTable.tsx | 2 +- .../schemaInsights/schemaInsights.reducer.ts | 8 ++- .../schemaInsights.sagas.spec.ts | 9 ++- .../schemaInsights.selectors.ts | 10 ++- .../db-console/src/redux/apiReducers.ts | 4 +- .../src/views/insights/insightsSelectors.ts | 8 ++- .../src/views/insights/schemaInsightsPage.tsx | 2 + 12 files changed, 105 insertions(+), 87 deletions(-) diff --git a/pkg/ui/workspaces/cluster-ui/src/api/schemaInsightsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/schemaInsightsApi.ts index dcba012faf34..3a0aa1504b54 100644 --- 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 = { 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[] = [ // getSchemaInsights makes requests over the SQL API and transforms the corresponding // SQL responses into schema insights. -export async function getSchemaInsights(): Promise { +export async function getSchemaInsights(): Promise< + SqlApiResponse +> { 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(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 { 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 index 117a2be9d38a..8b37657e9636 100644 --- 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 index 516dc6e6be7c..1d34e0a1c0bb 100644 --- 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 index a7243204828f..8458a00475fb 100644 --- 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 index 6f04662dcaf2..c9a3dbae0cd9 100644 --- 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 = ({ refreshUserSQLRoles, onFiltersChange, onSortChange, + maxSizeApiReached, }: SchemaInsightsViewProps) => { const isCockroachCloud = useContext(CockroachCloudContext); const [pagination, setPagination] = useState({ @@ -259,6 +264,20 @@ export const SchemaInsightsView: React.FC = ({ total={filteredSchemaInsights?.length} onChange={onChangePage} /> + {maxSizeApiReached && ( + + Not all insights are displayed because the maximum number of + insights was reached in the console.  + + Learn more + + + } + /> + )} diff --git a/pkg/ui/workspaces/cluster-ui/src/insightsTable/insightsTable.tsx b/pkg/ui/workspaces/cluster-ui/src/insightsTable/insightsTable.tsx index 673a37d17e04..3f30b4e154e7 100644 --- 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 index 7691ae0dde45..a415afb0f844 100644 --- 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; lastUpdated: Moment; lastError: Error; valid: boolean; @@ -31,7 +32,10 @@ const schemaInsightsSlice = createSlice({ name: `${DOMAIN_NAME}/schemaInsightsSlice`, initialState, reducers: { - received: (state, action: PayloadAction) => { + received: ( + state, + action: PayloadAction>, + ) => { 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 index d4c0abfbd5d4..57bad0d0c1da 100644 --- 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 = { + 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 index e4462deea97b..58143de27850 100644 --- 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 index 0a6d0d379e98..0f1807bcfa04 100644 --- 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 >; - schemaInsights: CachedDataReducerState; + schemaInsights: CachedDataReducerState< + clusterUiApi.SqlApiResponse + >; statementFingerprintInsights: KeyedCachedDataReducerState< clusterUiApi.SqlApiResponse >; diff --git a/pkg/ui/workspaces/db-console/src/views/insights/insightsSelectors.ts b/pkg/ui/workspaces/db-console/src/views/insights/insightsSelectors.ts index e595dff73b3c..8c1b6083d21a 100644 --- 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 index 81b58393f9e0..588e02730169 100644 --- 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 = {