From f6283937629f449e283463975ee595693cb9a0ef Mon Sep 17 00:00:00 2001 From: maryliag Date: Tue, 15 Aug 2023 10:25:18 -0400 Subject: [PATCH] ui: use cluster setting from redux on schema insights Part Of #108373 Use the value of the cluster setting `sql.index_recommendation.drop_unused_duration` from redux, instead of adding as part of the select. With this change, now users with VIEWACTIVITY or VIEWACTIVITYREDACTED can see index recommendations on the console, without the need the view cluster settings permission. This commit changes the Schema Insights Api. Release note (ui change): Users without `VIEWCLUSTERSETTINGS` permission but with `VIEWACTIVITY` or `VIEWACTIVITYREDACTED` can now see index recommendations. --- .../cluster-ui/src/api/databaseDetailsApi.ts | 3 +- .../cluster-ui/src/api/schemaInsightsApi.ts | 43 ++++++++++------ .../cluster-ui/src/api/tableDetailsApi.ts | 3 +- .../databaseDetailsPage.stories.tsx | 7 +-- .../databaseTablePage.stories.tsx | 5 +- .../databasesPage/databasesPage.stories.tsx | 7 +-- .../schemaInsights/schemaInsights.fixture.ts | 2 + .../schemaInsightsPageConnected.tsx | 6 ++- .../schemaInsights/schemaInsightsView.tsx | 14 ++++-- .../clusterSettings.selectors.ts | 7 +-- .../databaseDetails.saga.spec.ts | 3 +- .../tableDetails.saga.spec.ts | 5 +- .../schemaInsights/schemaInsights.reducer.ts | 9 ++-- .../schemaInsights/schemaInsights.sagas.ts | 15 ++++-- .../cluster-ui/src/util/constants.ts | 3 ++ .../db-console/src/redux/apiReducers.spec.ts | 10 ++-- .../clusterSettings.selectors.ts | 6 ++- .../db-console/src/util/api.spec.ts | 37 ++++++++------ .../db-console/src/util/constants.ts | 1 + .../databaseDetailsPage/redux.spec.ts | 49 ++++++++++++++----- .../databases/databaseTablePage/redux.spec.ts | 20 ++++++-- .../databases/databasesPage/redux.spec.ts | 9 ++-- .../src/views/insights/schemaInsightsPage.tsx | 6 ++- 23 files changed, 178 insertions(+), 92 deletions(-) diff --git a/pkg/ui/workspaces/cluster-ui/src/api/databaseDetailsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/databaseDetailsApi.ts index 0e7251254eb8..722b3973a23a 100644 --- a/pkg/ui/workspaces/cluster-ui/src/api/databaseDetailsApi.ts +++ b/pkg/ui/workspaces/cluster-ui/src/api/databaseDetailsApi.ts @@ -26,7 +26,7 @@ import { Format, Identifier, QualifiedIdentifier } from "./safesql"; import moment from "moment-timezone"; import { fromHexString, withTimeout } from "./util"; import { cockroach } from "@cockroachlabs/crdb-protobuf-client"; -import { getLogger } from "../util"; +import { getLogger, indexUnusedDuration } from "../util"; const { ZoneConfig } = cockroach.config.zonepb; const { ZoneConfigurationLevel } = cockroach.server.serverpb; @@ -393,6 +393,7 @@ type DatabaseIndexUsageStatsResponse = { const getDatabaseIndexUsageStats: DatabaseDetailsQuery = { createStmt: (dbName: string, csIndexUnusedDuration: string) => { + csIndexUnusedDuration = csIndexUnusedDuration ?? indexUnusedDuration; return { sql: Format( `SELECT * FROM (SELECT diff --git a/pkg/ui/workspaces/cluster-ui/src/api/schemaInsightsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/schemaInsightsApi.ts index 6b688b53fea8..3e8e63351ad9 100644 --- a/pkg/ui/workspaces/cluster-ui/src/api/schemaInsightsApi.ts +++ b/pkg/ui/workspaces/cluster-ui/src/api/schemaInsightsApi.ts @@ -23,7 +23,7 @@ import { InsightType, recommendDropUnusedIndex, } from "../insights"; -import { HexStringToInt64String } from "../util"; +import { HexStringToInt64String, indexUnusedDuration } from "../util"; import { QuoteIdentifier } from "./safesql"; // Export for db-console import from clusterUiApi. @@ -51,12 +51,16 @@ type CreateIndexRecommendationsResponse = { index_recommendations: string[]; }; +export type SchemaInsightReqParams = { + csIndexUnusedDuration: string; +}; + type SchemaInsightResponse = | ClusterIndexUsageStatistic | CreateIndexRecommendationsResponse; type SchemaInsightQuery = { name: InsightType; - query: string; + query: string | ((csIndexUnusedDuration: string) => string); toSchemaInsight: (response: SqlTxnResult) => InsightRecommendation[]; }; @@ -142,12 +146,9 @@ function createIndexRecommendationsToSchemaInsight( // and want to return the most used ones as a priority. const dropUnusedIndexQuery: SchemaInsightQuery = { name: "DropIndex", - 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, + query: (csIndexUnusedDuration: string) => { + csIndexUnusedDuration = csIndexUnusedDuration ?? indexUnusedDuration; + return `SELECT * FROM (SELECT us.table_id, us.index_id, us.last_read, us.total_reads, @@ -157,18 +158,18 @@ const dropUnusedIndexQuery: SchemaInsightQuery = { t.parent_id as database_id, t.database_name, t.schema_name, - cs.value as unused_threshold, - cs.value::interval as interval_threshold, + '${csIndexUnusedDuration}' as unused_threshold, + '${csIndexUnusedDuration}'::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.is_unique IS false) WHERE unused_interval > interval_threshold - ORDER BY total_reads DESC;`, + ORDER BY total_reads DESC;`; + }, toSchemaInsight: clusterIndexUsageStatsToSchemaInsight, }; @@ -211,14 +212,24 @@ const schemaInsightQueries: SchemaInsightQuery[] = [ createIndexRecommendationsQuery, ]; +function getQuery( + csIndexUnusedDuration: string, + query: string | ((csIndexUnusedDuration: string) => string), +): string { + if (typeof query == "string") { + return query; + } + return query(csIndexUnusedDuration); +} + // getSchemaInsights makes requests over the SQL API and transforms the corresponding // SQL responses into schema insights. -export async function getSchemaInsights(): Promise< - SqlApiResponse -> { +export async function getSchemaInsights( + params: SchemaInsightReqParams, +): Promise> { const request: SqlExecutionRequest = { statements: schemaInsightQueries.map(insightQuery => ({ - sql: insightQuery.query, + sql: getQuery(params.csIndexUnusedDuration, insightQuery.query), })), execute: true, max_result_size: LARGE_RESULT_SIZE, diff --git a/pkg/ui/workspaces/cluster-ui/src/api/tableDetailsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/tableDetailsApi.ts index 9116685f3d3d..f4f20d7f1ec2 100644 --- a/pkg/ui/workspaces/cluster-ui/src/api/tableDetailsApi.ts +++ b/pkg/ui/workspaces/cluster-ui/src/api/tableDetailsApi.ts @@ -26,7 +26,7 @@ import { fromHexString, withTimeout } from "./util"; import { Format, Identifier, Join, SQL } from "./safesql"; import { cockroach } from "@cockroachlabs/crdb-protobuf-client"; import { IndexUsageStatistic, recommendDropUnusedIndex } from "../insights"; -import { getLogger } from "../util"; +import { getLogger, indexUnusedDuration } from "../util"; const { ZoneConfig } = cockroach.config.zonepb; const { ZoneConfigurationLevel } = cockroach.server.serverpb; @@ -481,6 +481,7 @@ const getTableIndexUsageStats: TableDetailsQuery = { [new Identifier(dbName), new SQL(tableName)], new SQL("."), ); + csIndexUnusedDuration = csIndexUnusedDuration ?? indexUnusedDuration; return { sql: Format( `WITH tableId AS (SELECT $1::regclass::int as table_id) diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.stories.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.stories.tsx index a46ee949b20f..10b683a6ceab 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.stories.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.stories.tsx @@ -28,6 +28,7 @@ import { import * as H from "history"; import moment from "moment-timezone"; import { defaultFilters } from "src/queryFilter"; +import { indexUnusedDuration } from "src/util/constants"; const history = H.createHashHistory(); const withLoadingIndicator: DatabaseDetailsPageProps = { @@ -35,7 +36,7 @@ const withLoadingIndicator: DatabaseDetailsPageProps = { loaded: false, lastError: undefined, showIndexRecommendations: false, - csIndexUnusedDuration: "168h", + csIndexUnusedDuration: indexUnusedDuration, name: randomName(), tables: [], viewMode: ViewMode.Tables, @@ -69,7 +70,7 @@ const withoutData: DatabaseDetailsPageProps = { loaded: true, lastError: null, showIndexRecommendations: false, - csIndexUnusedDuration: "168h", + csIndexUnusedDuration: indexUnusedDuration, name: randomName(), tables: [], viewMode: ViewMode.Tables, @@ -131,7 +132,7 @@ const withData: DatabaseDetailsPageProps = { loaded: true, lastError: null, showIndexRecommendations: true, - csIndexUnusedDuration: "168h", + csIndexUnusedDuration: indexUnusedDuration, name: randomName(), tables: [createTable()], viewMode: ViewMode.Tables, diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.stories.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.stories.tsx index e9721f37830a..30825fcb6377 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.stories.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.stories.tsx @@ -21,6 +21,7 @@ import { import { DatabaseTablePage, DatabaseTablePageProps } from "./databaseTablePage"; import moment from "moment-timezone"; import * as H from "history"; +import { indexUnusedDuration } from "src/util/constants"; const history = H.createHashHistory(); const withLoadingIndicator: DatabaseTablePageProps = { @@ -30,7 +31,7 @@ const withLoadingIndicator: DatabaseTablePageProps = { schemaName: randomName(), indexUsageStatsEnabled: false, showIndexRecommendations: false, - csIndexUnusedDuration: "168h", + csIndexUnusedDuration: indexUnusedDuration, details: { loading: true, loaded: false, @@ -77,7 +78,7 @@ const withData: DatabaseTablePageProps = { schemaName: randomName(), indexUsageStatsEnabled: true, showIndexRecommendations: true, - csIndexUnusedDuration: "168h", + csIndexUnusedDuration: indexUnusedDuration, details: { loading: false, loaded: true, diff --git a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.stories.tsx b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.stories.tsx index 1eb7e037799c..1435333a7d94 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.stories.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.stories.tsx @@ -18,6 +18,7 @@ import { DatabasesPage, DatabasesPageProps } from "./databasesPage"; import * as H from "history"; import { defaultFilters } from "src/queryFilter"; +import { indexUnusedDuration } from "src/util/constants"; const history = H.createHashHistory(); const withLoadingIndicator: DatabasesPageProps = { @@ -26,7 +27,7 @@ const withLoadingIndicator: DatabasesPageProps = { lastError: undefined, automaticStatsCollectionEnabled: true, indexRecommendationsEnabled: false, - csIndexUnusedDuration: "168h", + csIndexUnusedDuration: indexUnusedDuration, databases: [], sortSetting: { ascending: false, @@ -55,7 +56,7 @@ const withoutData: DatabasesPageProps = { lastError: null, automaticStatsCollectionEnabled: true, indexRecommendationsEnabled: false, - csIndexUnusedDuration: "168h", + csIndexUnusedDuration: indexUnusedDuration, databases: [], sortSetting: { ascending: false, @@ -85,7 +86,7 @@ const withData: DatabasesPageProps = { showNodeRegionsColumn: true, automaticStatsCollectionEnabled: true, indexRecommendationsEnabled: true, - csIndexUnusedDuration: "168h", + csIndexUnusedDuration: indexUnusedDuration, sortSetting: { ascending: false, columnTitle: "name", diff --git a/pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/schemaInsights.fixture.ts b/pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/schemaInsights.fixture.ts index a774d897b26a..8af5f8b8d85a 100644 --- a/pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/schemaInsights.fixture.ts +++ b/pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/schemaInsights.fixture.ts @@ -8,6 +8,7 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. +import { indexUnusedDuration } from "src/util/constants"; import { SchemaInsightsViewProps } from "./schemaInsightsView"; export const SchemaInsightsPropsFixture: SchemaInsightsViewProps = { @@ -71,6 +72,7 @@ export const SchemaInsightsPropsFixture: SchemaInsightsViewProps = { schemaInsightType: "", }, hasAdminRole: true, + csIndexUnusedDuration: indexUnusedDuration, refreshSchemaInsights: () => {}, onSortChange: () => {}, onFiltersChange: () => {}, 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 8458a00475fb..49ebf928d9c5 100644 --- a/pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/schemaInsightsPageConnected.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/schemaInsightsPageConnected.tsx @@ -32,6 +32,7 @@ import { actions as localStorageActions } from "../../store/localStorage"; import { Dispatch } from "redux"; import { selectHasAdminRole } from "../../store/uiConfig"; import { actions as analyticsActions } from "../../store/analytics"; +import { selectDropUnusedIndexDuration } from "src/store/clusterSettings/clusterSettings.selectors"; const mapStateToProps = ( state: AppState, @@ -45,6 +46,7 @@ const mapStateToProps = ( sortSetting: selectSortSetting(state), hasAdminRole: selectHasAdminRole(state), maxSizeApiReached: selectSchemaInsightsMaxApiSizeReached(state), + csIndexUnusedDuration: selectDropUnusedIndexDuration(state), }); const mapDispatchToProps = ( @@ -82,8 +84,8 @@ const mapDispatchToProps = ( }), ); }, - refreshSchemaInsights: () => { - dispatch(actions.refresh()); + refreshSchemaInsights: (csIndexUnusedDuration: string) => { + dispatch(actions.refresh({ csIndexUnusedDuration })); }, refreshUserSQLRoles: () => dispatch(uiConfigActions.refreshUserSQLRoles()), }); 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 4b49858e46e0..3ac96dd838ed 100644 --- a/pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/schemaInsightsView.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/schemaInsightsView.tsx @@ -55,13 +55,14 @@ export type SchemaInsightsViewStateProps = { filters: SchemaInsightEventFilters; sortSetting: SortSetting; hasAdminRole: boolean; + csIndexUnusedDuration: string; maxSizeApiReached?: boolean; }; export type SchemaInsightsViewDispatchProps = { onFiltersChange: (filters: SchemaInsightEventFilters) => void; onSortChange: (ss: SortSetting) => void; - refreshSchemaInsights: () => void; + refreshSchemaInsights: (csIndexUnusedDuration: string) => void; refreshUserSQLRoles: () => void; }; @@ -83,6 +84,7 @@ export const SchemaInsightsView: React.FC = ({ onFiltersChange, onSortChange, maxSizeApiReached, + csIndexUnusedDuration, }: SchemaInsightsViewProps) => { const isCockroachCloud = useContext(CockroachCloudContext); const [pagination, setPagination] = useState({ @@ -95,13 +97,17 @@ export const SchemaInsightsView: React.FC = ({ ); useEffect(() => { + const refreshSchema = (): void => { + refreshSchemaInsights(csIndexUnusedDuration); + }; + // Refresh every 1 minute. - refreshSchemaInsights(); - const interval = setInterval(refreshSchemaInsights, 60 * 1000); + refreshSchema(); + const interval = setInterval(refreshSchema, 60 * 1000); return () => { clearInterval(interval); }; - }, [refreshSchemaInsights]); + }, [refreshSchemaInsights, csIndexUnusedDuration]); useEffect(() => { // Refresh every 5 minutes. diff --git a/pkg/ui/workspaces/cluster-ui/src/store/clusterSettings/clusterSettings.selectors.ts b/pkg/ui/workspaces/cluster-ui/src/store/clusterSettings/clusterSettings.selectors.ts index 826c3f55de1d..937ef0418c65 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/clusterSettings/clusterSettings.selectors.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/clusterSettings/clusterSettings.selectors.ts @@ -9,7 +9,7 @@ // licenses/APL.txt. import { AppState } from "../reducers"; -import { greaterOrEqualThanVersion } from "../../util"; +import { greaterOrEqualThanVersion, indexUnusedDuration } from "../../util"; export const selectAutomaticStatsCollectionEnabled = ( state: AppState, @@ -43,9 +43,10 @@ export const selectIndexUsageStatsEnabled = (state: AppState): boolean => { export const selectDropUnusedIndexDuration = (state: AppState): string => { const settings = state.adminUI?.clusterSettings.data?.key_values; if (!settings) { - return "168h"; + return indexUnusedDuration; } return ( - settings["sql.index_recommendation.drop_unused_duration"]?.value || "168h" + settings["sql.index_recommendation.drop_unused_duration"]?.value || + indexUnusedDuration ); }; diff --git a/pkg/ui/workspaces/cluster-ui/src/store/databaseDetails/databaseDetails.saga.spec.ts b/pkg/ui/workspaces/cluster-ui/src/store/databaseDetails/databaseDetails.saga.spec.ts index c503088d8d4c..37660d9f7cf3 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/databaseDetails/databaseDetails.saga.spec.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/databaseDetails/databaseDetails.saga.spec.ts @@ -34,10 +34,11 @@ import { KeyedDatabaseDetailsState, reducer, } from "./databaseDetails.reducer"; +import { indexUnusedDuration } from "src/util/constants"; describe("DatabaseDetails sagas", () => { const database = "test_db"; - const csIndexUnusedDuration = "168h"; + const csIndexUnusedDuration = indexUnusedDuration; const requestAction: PayloadAction = { payload: { database, csIndexUnusedDuration }, type: "request", diff --git a/pkg/ui/workspaces/cluster-ui/src/store/databaseTableDetails/tableDetails.saga.spec.ts b/pkg/ui/workspaces/cluster-ui/src/store/databaseTableDetails/tableDetails.saga.spec.ts index 01885368df32..ad7a9d6557fd 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/databaseTableDetails/tableDetails.saga.spec.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/databaseTableDetails/tableDetails.saga.spec.ts @@ -35,15 +35,14 @@ import { reducer, } from "./tableDetails.reducer"; import moment from "moment"; -import { generateTableID } from "../../util"; +import { generateTableID, indexUnusedDuration } from "../../util"; describe("TableDetails sagas", () => { const database = "test_db"; const table = "test_table"; - const csIndexUnusedDuration = "168h"; const key = generateTableID(database, table); const requestAction: PayloadAction = { - payload: { database, table, csIndexUnusedDuration }, + payload: { database, table, csIndexUnusedDuration: indexUnusedDuration }, type: "request", }; const tableDetailsResponse: SqlApiResponse = { 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 3aa0cb1ffe2a..63dd27f4ab4a 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 @@ -9,10 +9,10 @@ // licenses/APL.txt. import { createSlice, PayloadAction } from "@reduxjs/toolkit"; -import { DOMAIN_NAME, noopReducer } from "../utils"; +import { DOMAIN_NAME } from "../utils"; import moment, { Moment } from "moment-timezone"; import { InsightRecommendation } from "../../insights"; -import { SqlApiResponse } from "src/api"; +import { SchemaInsightReqParams, SqlApiResponse } from "src/api"; export type SchemaInsightsState = { data: SqlApiResponse; @@ -50,9 +50,8 @@ const schemaInsightsSlice = createSlice({ state.valid = false; state.lastUpdated = moment.utc(); }, - // Define actions that don't change state. - refresh: noopReducer, - request: noopReducer, + refresh: (_, _action: PayloadAction) => {}, + request: (_, _action: PayloadAction) => {}, }, }); diff --git a/pkg/ui/workspaces/cluster-ui/src/store/schemaInsights/schemaInsights.sagas.ts b/pkg/ui/workspaces/cluster-ui/src/store/schemaInsights/schemaInsights.sagas.ts index 60cb09693409..bd1548b40efe 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/schemaInsights/schemaInsights.sagas.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/schemaInsights/schemaInsights.sagas.ts @@ -11,15 +11,20 @@ import { all, call, put, takeLatest } from "redux-saga/effects"; import { actions } from "./schemaInsights.reducer"; -import { getSchemaInsights } from "../../api"; +import { SchemaInsightReqParams, getSchemaInsights } from "../../api"; +import { PayloadAction } from "@reduxjs/toolkit"; -export function* refreshSchemaInsightsSaga() { - yield put(actions.request()); +export function* refreshSchemaInsightsSaga( + action: PayloadAction, +) { + yield put(actions.request(action.payload)); } -export function* requestSchemaInsightsSaga(): any { +export function* requestSchemaInsightsSaga( + action: PayloadAction, +): any { try { - const result = yield call(getSchemaInsights); + const result = yield call(getSchemaInsights, action.payload); yield put(actions.received(result)); } catch (e) { yield put(actions.failed(e)); diff --git a/pkg/ui/workspaces/cluster-ui/src/util/constants.ts b/pkg/ui/workspaces/cluster-ui/src/util/constants.ts index 24fc0f28485d..dd7665a7d1ca 100644 --- a/pkg/ui/workspaces/cluster-ui/src/util/constants.ts +++ b/pkg/ui/workspaces/cluster-ui/src/util/constants.ts @@ -36,6 +36,9 @@ export const unset = "(unset)"; export const viewAttr = "view"; export const idAttr = "id"; +// Default value for cluster settings +export const indexUnusedDuration = "168h"; + export const REMOTE_DEBUGGING_ERROR_TEXT = "This information is not available due to the current value of the 'server.remote_debugging.mode' setting."; diff --git a/pkg/ui/workspaces/db-console/src/redux/apiReducers.spec.ts b/pkg/ui/workspaces/db-console/src/redux/apiReducers.spec.ts index fcda692ca5d0..68649be44454 100644 --- a/pkg/ui/workspaces/db-console/src/redux/apiReducers.spec.ts +++ b/pkg/ui/workspaces/db-console/src/redux/apiReducers.spec.ts @@ -22,6 +22,7 @@ import moment from "moment-timezone"; import { cockroach } from "@cockroachlabs/crdb-protobuf-client"; import { RouteComponentProps } from "react-router"; import { queryByName } from "src/util/query"; +import { indexUnusedDuration } from "../util/constants"; describe("table id generator", function () { it("generates encoded db/table id", function () { @@ -42,19 +43,20 @@ describe("table id generator", function () { describe("request to string functions", function () { it("correctly generates a string from a database details request", function () { const database = "testDatabase"; - const csIndexUnusedDuration = "168h"; expect( - databaseRequestPayloadToID({ database, csIndexUnusedDuration }), + databaseRequestPayloadToID({ + database, + csIndexUnusedDuration: indexUnusedDuration, + }), ).toEqual(database); }); it("correctly generates a string from a table details request", function () { const database = "testDatabase"; const table = "testTable"; - const csIndexUnusedDuration = "168h"; const tableRequest: clusterUiApi.TableDetailsReqParams = { database, table, - csIndexUnusedDuration, + csIndexUnusedDuration: indexUnusedDuration, }; expect(tableRequestToID(tableRequest)).toEqual( util.generateTableID(database, table), diff --git a/pkg/ui/workspaces/db-console/src/redux/clusterSettings/clusterSettings.selectors.ts b/pkg/ui/workspaces/db-console/src/redux/clusterSettings/clusterSettings.selectors.ts index 832bf04140e1..8a611303589d 100644 --- a/pkg/ui/workspaces/db-console/src/redux/clusterSettings/clusterSettings.selectors.ts +++ b/pkg/ui/workspaces/db-console/src/redux/clusterSettings/clusterSettings.selectors.ts @@ -13,6 +13,7 @@ import { AdminUIState } from "src/redux/state"; import { cockroach } from "src/js/protos"; import moment from "moment-timezone"; import { CoordinatedUniversalTime, util } from "@cockroachlabs/cluster-ui"; +import { indexUnusedDuration } from "src/util/constants"; export const selectClusterSettings = createSelector( (state: AdminUIState) => state.cachedData.settings?.data, @@ -110,10 +111,11 @@ export const selectDropUnusedIndexDuration = createSelector( selectClusterSettings, (settings): string => { if (!settings) { - return "168h"; + return indexUnusedDuration; } return ( - settings["sql.index_recommendation.drop_unused_duration"]?.value || "168h" + settings["sql.index_recommendation.drop_unused_duration"]?.value || + indexUnusedDuration ); }, ); diff --git a/pkg/ui/workspaces/db-console/src/util/api.spec.ts b/pkg/ui/workspaces/db-console/src/util/api.spec.ts index 281f26cc569d..98c44f362a0a 100644 --- a/pkg/ui/workspaces/db-console/src/util/api.spec.ts +++ b/pkg/ui/workspaces/db-console/src/util/api.spec.ts @@ -18,7 +18,10 @@ import * as protos from "@cockroachlabs/crdb-protobuf-client"; const cockroach = protos.cockroach; import * as api from "./api"; import { api as clusterUiApi } from "@cockroachlabs/cluster-ui"; -import { REMOTE_DEBUGGING_ERROR_TEXT } from "src/util/constants"; +import { + REMOTE_DEBUGGING_ERROR_TEXT, + indexUnusedDuration, +} from "src/util/constants"; import Severity = protos.cockroach.util.log.Severity; import { stubSqlApiCall } from "src/util/fakeApi"; @@ -104,7 +107,6 @@ describe("rest api", function () { describe("database details request", function () { const database = "test"; - const csIndexUnusedDuration = "168h"; const mockOldDate = new Date(2023, 2, 3); const mockZoneConfig = new ZoneConfig({ inherited_constraints: true, @@ -128,7 +130,7 @@ describe("rest api", function () { stubSqlApiCall( clusterUiApi.createDatabaseDetailsReq({ database, - csIndexUnusedDuration, + csIndexUnusedDuration: indexUnusedDuration, }), [ // Database ID query @@ -192,7 +194,10 @@ describe("rest api", function () { ); return clusterUiApi - .getDatabaseDetails({ database, csIndexUnusedDuration }) + .getDatabaseDetails({ + database, + csIndexUnusedDuration: indexUnusedDuration, + }) .then(result => { expect(fetchMock.calls(clusterUiApi.SQL_API_PATH).length).toBe(1); expect(result.results.idResp.database_id).toEqual("1"); @@ -220,7 +225,7 @@ describe("rest api", function () { // Mock out the fetch query, but return a 500 status code const req = clusterUiApi.createDatabaseDetailsReq({ database, - csIndexUnusedDuration, + csIndexUnusedDuration: indexUnusedDuration, }); fetchMock.mock({ matcher: clusterUiApi.SQL_API_PATH, @@ -236,7 +241,10 @@ describe("rest api", function () { }); clusterUiApi - .getDatabaseDetails({ database, csIndexUnusedDuration }) + .getDatabaseDetails({ + database, + csIndexUnusedDuration: indexUnusedDuration, + }) .then(_result => { done(new Error("Request unexpectedly succeeded.")); }) @@ -250,7 +258,7 @@ describe("rest api", function () { // Mock out the fetch query, but return a promise that's never resolved to test the timeout const req = clusterUiApi.createDatabaseDetailsReq({ database, - csIndexUnusedDuration, + csIndexUnusedDuration: indexUnusedDuration, }); fetchMock.mock({ matcher: clusterUiApi.SQL_API_PATH, @@ -267,7 +275,7 @@ describe("rest api", function () { clusterUiApi .getDatabaseDetails( - { database, csIndexUnusedDuration }, + { database, csIndexUnusedDuration: indexUnusedDuration }, moment.duration(0), ) .then(_result => { @@ -283,7 +291,6 @@ describe("rest api", function () { describe("table details request", function () { const dbName = "testDB"; const tableName = "testTable"; - const csIndexUnusedDuration = "168h"; const mockOldDate = new Date(2023, 2, 3); const mockZoneConfig = new ZoneConfig({ inherited_constraints: true, @@ -309,7 +316,7 @@ describe("rest api", function () { clusterUiApi.createTableDetailsReq( dbName, tableName, - csIndexUnusedDuration, + indexUnusedDuration, ), [ // Table ID query @@ -368,7 +375,7 @@ describe("rest api", function () { .getTableDetails({ database: dbName, table: tableName, - csIndexUnusedDuration, + csIndexUnusedDuration: indexUnusedDuration, }) .then(resp => { expect(fetchMock.calls(clusterUiApi.SQL_API_PATH).length).toBe(1); @@ -417,7 +424,7 @@ describe("rest api", function () { ...clusterUiApi.createTableDetailsReq( dbName, tableName, - csIndexUnusedDuration, + indexUnusedDuration, ), application_name: clusterUiApi.INTERNAL_SQL_API_APP, }); @@ -429,7 +436,7 @@ describe("rest api", function () { .getTableDetails({ database: dbName, table: tableName, - csIndexUnusedDuration, + csIndexUnusedDuration: indexUnusedDuration, }) .then(_result => { done(new Error("Request unexpectedly succeeded.")); @@ -450,7 +457,7 @@ describe("rest api", function () { ...clusterUiApi.createTableDetailsReq( dbName, tableName, - csIndexUnusedDuration, + indexUnusedDuration, ), application_name: clusterUiApi.INTERNAL_SQL_API_APP, }); @@ -463,7 +470,7 @@ describe("rest api", function () { { database: dbName, table: tableName, - csIndexUnusedDuration, + csIndexUnusedDuration: indexUnusedDuration, }, moment.duration(0), ) diff --git a/pkg/ui/workspaces/db-console/src/util/constants.ts b/pkg/ui/workspaces/db-console/src/util/constants.ts index 1fe7f2c4da38..bd07938c01ce 100644 --- a/pkg/ui/workspaces/db-console/src/util/constants.ts +++ b/pkg/ui/workspaces/db-console/src/util/constants.ts @@ -36,4 +36,5 @@ export const { viewAttr, REMOTE_DEBUGGING_ERROR_TEXT, idAttr, + indexUnusedDuration, } = util; diff --git a/pkg/ui/workspaces/db-console/src/views/databases/databaseDetailsPage/redux.spec.ts b/pkg/ui/workspaces/db-console/src/views/databases/databaseDetailsPage/redux.spec.ts index 4737740c3b57..f181f4c64e75 100644 --- a/pkg/ui/workspaces/db-console/src/views/databases/databaseDetailsPage/redux.spec.ts +++ b/pkg/ui/workspaces/db-console/src/views/databases/databaseDetailsPage/redux.spec.ts @@ -22,7 +22,7 @@ import { } from "@cockroachlabs/cluster-ui"; import { AdminUIState, createAdminUIStore } from "src/redux/state"; -import { databaseNameAttr } from "src/util/constants"; +import { databaseNameAttr, indexUnusedDuration } from "src/util/constants"; import * as fakeApi from "src/util/fakeApi"; import { mapStateToProps, mapDispatchToProps } from "./redux"; import moment from "moment-timezone"; @@ -108,11 +108,18 @@ class TestDriver { } async refreshDatabaseDetails() { - return this.actions.refreshDatabaseDetails(this.database, "168h"); + return this.actions.refreshDatabaseDetails( + this.database, + indexUnusedDuration, + ); } async refreshTableDetails(table: string) { - return this.actions.refreshTableDetails(this.database, table, "168h"); + return this.actions.refreshTableDetails( + this.database, + table, + indexUnusedDuration, + ); } private findTable(name: string) { @@ -150,7 +157,7 @@ describe("Database Details Page", function () { sortSettingGrants: { ascending: true, columnTitle: "name" }, tables: [], showIndexRecommendations: false, - csIndexUnusedDuration: "168h", + csIndexUnusedDuration: indexUnusedDuration, }); }); @@ -158,7 +165,7 @@ describe("Database Details Page", function () { fakeApi.stubSqlApiCall( clusterUiApi.createDatabaseDetailsReq({ database: "things", - csIndexUnusedDuration: "168h", + csIndexUnusedDuration: indexUnusedDuration, }), [ // Id @@ -187,7 +194,7 @@ describe("Database Details Page", function () { isTenant: false, showNodeRegionsColumn: false, showIndexRecommendations: false, - csIndexUnusedDuration: "168h", + csIndexUnusedDuration: indexUnusedDuration, viewMode: ViewMode.Tables, sortSettingTables: { ascending: true, columnTitle: "name" }, sortSettingGrants: { ascending: true, columnTitle: "name" }, @@ -244,7 +251,7 @@ describe("Database Details Page", function () { fakeApi.stubSqlApiCall( clusterUiApi.createDatabaseDetailsReq({ database: "things", - csIndexUnusedDuration: "168h", + csIndexUnusedDuration: indexUnusedDuration, }), [ // Id @@ -263,7 +270,11 @@ describe("Database Details Page", function () { const mockStatsLastCreatedTimestamp = moment(); fakeApi.stubSqlApiCall( - clusterUiApi.createTableDetailsReq("things", `"public"."foo"`, "168h"), + clusterUiApi.createTableDetailsReq( + "things", + `"public"."foo"`, + indexUnusedDuration, + ), [ // Table ID query { rows: [{ table_id: "1" }] }, @@ -314,7 +325,11 @@ describe("Database Details Page", function () { ); fakeApi.stubSqlApiCall( - clusterUiApi.createTableDetailsReq("things", `"public"."bar"`, "168h"), + clusterUiApi.createTableDetailsReq( + "things", + `"public"."bar"`, + indexUnusedDuration, + ), [ // Table ID query { rows: [{ table_id: "2" }] }, @@ -414,7 +429,7 @@ describe("Database Details Page", function () { fakeApi.stubSqlApiCall( clusterUiApi.createDatabaseDetailsReq({ database: "things", - csIndexUnusedDuration: "168h", + csIndexUnusedDuration: indexUnusedDuration, }), [ // Id @@ -429,7 +444,11 @@ describe("Database Details Page", function () { ); fakeApi.stubSqlApiCall( - clusterUiApi.createTableDetailsReq("things", `"public"."foo"`, "168h"), + clusterUiApi.createTableDetailsReq( + "things", + `"public"."foo"`, + indexUnusedDuration, + ), [ // Table ID query {}, @@ -464,7 +483,7 @@ describe("Database Details Page", function () { fakeApi.stubSqlApiCall( clusterUiApi.createDatabaseDetailsReq({ database: "things", - csIndexUnusedDuration: "168h", + csIndexUnusedDuration: indexUnusedDuration, }), [ // Id @@ -479,7 +498,11 @@ describe("Database Details Page", function () { ); fakeApi.stubSqlApiCall( - clusterUiApi.createTableDetailsReq("things", `"public"."foo"`, "168h"), + clusterUiApi.createTableDetailsReq( + "things", + `"public"."foo"`, + indexUnusedDuration, + ), [ // Table ID query {}, diff --git a/pkg/ui/workspaces/db-console/src/views/databases/databaseTablePage/redux.spec.ts b/pkg/ui/workspaces/db-console/src/views/databases/databaseTablePage/redux.spec.ts index 81cf596c43f3..b482367f61e2 100644 --- a/pkg/ui/workspaces/db-console/src/views/databases/databaseTablePage/redux.spec.ts +++ b/pkg/ui/workspaces/db-console/src/views/databases/databaseTablePage/redux.spec.ts @@ -22,7 +22,11 @@ import { } from "@cockroachlabs/cluster-ui"; import { AdminUIState, createAdminUIStore } from "src/redux/state"; -import { databaseNameAttr, tableNameAttr } from "src/util/constants"; +import { + databaseNameAttr, + indexUnusedDuration, + tableNameAttr, +} from "src/util/constants"; import * as fakeApi from "src/util/fakeApi"; import { mapStateToProps, mapDispatchToProps } from "./redux"; import moment from "moment-timezone"; @@ -139,7 +143,11 @@ class TestDriver { return this.actions.refreshSettings(); } async refreshTableDetails() { - return this.actions.refreshTableDetails(this.database, this.table, "168h"); + return this.actions.refreshTableDetails( + this.database, + this.table, + indexUnusedDuration, + ); } async refreshIndexStats() { @@ -196,7 +204,7 @@ describe("Database Table Page", function () { automaticStatsCollectionEnabled: true, indexUsageStatsEnabled: true, showIndexRecommendations: true, - csIndexUnusedDuration: "168h", + csIndexUnusedDuration: indexUnusedDuration, hasAdminRole: false, indexStats: { loading: false, @@ -212,7 +220,11 @@ describe("Database Table Page", function () { const mockStatsLastCreatedTimestamp = moment(); fakeApi.stubSqlApiCall( - clusterUiApi.createTableDetailsReq("DATABASE", "TABLE", "168h"), + clusterUiApi.createTableDetailsReq( + "DATABASE", + "TABLE", + indexUnusedDuration, + ), [ // Table ID query { rows: [{ table_id: "1" }] }, diff --git a/pkg/ui/workspaces/db-console/src/views/databases/databasesPage/redux.spec.ts b/pkg/ui/workspaces/db-console/src/views/databases/databasesPage/redux.spec.ts index 83a68fff39ea..fb7ace249fdf 100644 --- a/pkg/ui/workspaces/db-console/src/views/databases/databasesPage/redux.spec.ts +++ b/pkg/ui/workspaces/db-console/src/views/databases/databasesPage/redux.spec.ts @@ -22,6 +22,7 @@ import { import { AdminUIState, createAdminUIStore } from "src/redux/state"; import * as fakeApi from "src/util/fakeApi"; import { mapDispatchToProps, mapStateToProps } from "./redux"; +import { indexUnusedDuration } from "src/util/constants"; class TestDriver { private readonly actions: DatabasesPageActions; @@ -104,7 +105,7 @@ describe("Databases Page", function () { automaticStatsCollectionEnabled: true, indexRecommendationsEnabled: true, showNodeRegionsColumn: false, - csIndexUnusedDuration: "168h", + csIndexUnusedDuration: indexUnusedDuration, }); }); @@ -172,7 +173,7 @@ describe("Databases Page", function () { sortSetting: { ascending: true, columnTitle: "name" }, showNodeRegionsColumn: false, indexRecommendationsEnabled: true, - csIndexUnusedDuration: "168h", + csIndexUnusedDuration: indexUnusedDuration, automaticStatsCollectionEnabled: true, }); }); @@ -225,7 +226,7 @@ describe("Databases Page", function () { fakeApi.stubSqlApiCall( clusterUiApi.createDatabaseDetailsReq({ database: "test", - csIndexUnusedDuration: "168h", + csIndexUnusedDuration: indexUnusedDuration, }), [ // Id @@ -287,7 +288,7 @@ describe("Databases Page", function () { await driver.refreshNodes(); await driver.refreshDatabases(); - await driver.refreshDatabaseDetails("test", "168h"); + await driver.refreshDatabaseDetails("test", indexUnusedDuration); driver.assertDatabaseProperties("test", { loading: false, 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 588e02730169..6394b14ba3dc 100644 --- a/pkg/ui/workspaces/db-console/src/views/insights/schemaInsightsPage.tsx +++ b/pkg/ui/workspaces/db-console/src/views/insights/schemaInsightsPage.tsx @@ -31,6 +31,7 @@ import { selectSchemaInsightsTypes, } from "src/views/insights/insightsSelectors"; import { selectHasAdminRole } from "src/redux/user"; +import { selectDropUnusedIndexDuration } from "src/redux/clusterSettings"; const mapStateToProps = ( state: AdminUIState, @@ -44,13 +45,16 @@ const mapStateToProps = ( sortSetting: schemaInsightsSortLocalSetting.selector(state), hasAdminRole: selectHasAdminRole(state), maxSizeApiReached: selectSchemaInsightsMaxApiReached(state), + csIndexUnusedDuration: selectDropUnusedIndexDuration(state), }); const mapDispatchToProps = { onFiltersChange: (filters: SchemaInsightEventFilters) => schemaInsightsFiltersLocalSetting.set(filters), onSortChange: (ss: SortSetting) => schemaInsightsSortLocalSetting.set(ss), - refreshSchemaInsights: refreshSchemaInsights, + refreshSchemaInsights: (csIndexUnusedDuration: string) => { + return refreshSchemaInsights({ csIndexUnusedDuration }); + }, refreshUserSQLRoles: refreshUserSQLRoles, };