From aad0ce7e4e025cec6692847759a45b78d9744c01 Mon Sep 17 00:00:00 2001 From: maryliag Date: Fri, 11 Aug 2023 11:57:20 -0400 Subject: [PATCH] ui: user cluster settings from redux 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 fixes on api from Database pages (Database Details and Table Details). Release note: None --- .../cluster-ui/src/api/databaseDetailsApi.ts | 52 ++++++----- .../cluster-ui/src/api/tableDetailsApi.ts | 32 ++++--- .../databaseDetailsConnected.ts | 22 +++-- .../databaseDetailsPage.tsx | 23 ++++- .../databaseTablePage/databaseTablePage.tsx | 8 +- .../databaseTablePageConnected.ts | 12 ++- .../src/databasesPage/databasesPage.tsx | 11 ++- .../databasesPage/databasesPageConnected.ts | 8 +- .../clusterSettings.selectors.ts | 10 +++ .../databaseDetails.reducer.ts | 9 +- .../databaseDetails.saga.spec.ts | 6 +- .../databaseDetails/databaseDetails.saga.ts | 16 ++-- .../tableDetails.saga.spec.ts | 3 +- .../db-console/src/redux/apiReducers.spec.ts | 7 +- .../db-console/src/redux/apiReducers.ts | 4 +- .../clusterSettings.selectors.ts | 12 +++ .../db-console/src/util/api.spec.ts | 89 +++++++++++++------ .../databaseDetailsPage/redux.spec.ts | 34 ++++--- .../databases/databaseDetailsPage/redux.ts | 20 ++++- .../databases/databaseTablePage/redux.spec.ts | 5 +- .../databases/databaseTablePage/redux.ts | 9 +- .../databases/databasesPage/redux.spec.ts | 14 ++- .../views/databases/databasesPage/redux.ts | 9 +- 23 files changed, 304 insertions(+), 111 deletions(-) diff --git a/pkg/ui/workspaces/cluster-ui/src/api/databaseDetailsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/databaseDetailsApi.ts index 00dba9e743f3..0e7251254eb8 100644 --- a/pkg/ui/workspaces/cluster-ui/src/api/databaseDetailsApi.ts +++ b/pkg/ui/workspaces/cluster-ui/src/api/databaseDetailsApi.ts @@ -33,6 +33,11 @@ const { ZoneConfigurationLevel } = cockroach.server.serverpb; type ZoneConfigType = cockroach.config.zonepb.ZoneConfig; type ZoneConfigLevelType = cockroach.server.serverpb.ZoneConfigurationLevel; +export type DatabaseDetailsReqParams = { + database: string; + csIndexUnusedDuration: string; +}; + export type DatabaseDetailsResponse = { idResp: SqlApiQueryResponse; grantsResp: SqlApiQueryResponse; @@ -387,24 +392,18 @@ type DatabaseIndexUsageStatsResponse = { }; const getDatabaseIndexUsageStats: DatabaseDetailsQuery = { - createStmt: dbName => { + createStmt: (dbName: string, csIndexUnusedDuration: string) => { return { sql: Format( - `WITH cs AS ( - SELECT value - FROM crdb_internal.cluster_settings - WHERE variable = 'sql.index_recommendation.drop_unused_duration' - ) - SELECT * FROM (SELECT + `SELECT * FROM (SELECT ti.created_at, us.last_read, us.total_reads, - 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 %1.crdb_internal.index_usage_statistics AS us JOIN %1.crdb_internal.table_indexes AS ti ON (us.index_id = ti.index_id AND us.table_id = ti.descriptor_id) - CROSS JOIN cs WHERE $1 != 'system' AND ti.is_unique IS false) WHERE unused_interval > interval_threshold ORDER BY total_reads DESC;`, @@ -442,7 +441,7 @@ export type DatabaseDetailsRow = | IndexUsageStatistic; type DatabaseDetailsQuery = { - createStmt: (dbName: string) => SqlStatement; + createStmt: (dbName: string, csIndexUnusedDuration: string) => SqlStatement; addToDatabaseDetail: ( response: SqlTxnResult, dbDetail: DatabaseDetailsResponse, @@ -464,25 +463,29 @@ const databaseDetailQueries: DatabaseDetailsQuery[] = [ getDatabaseSpanStats, ]; -export function createDatabaseDetailsReq(dbName: string): SqlExecutionRequest { +export function createDatabaseDetailsReq( + params: DatabaseDetailsReqParams, +): SqlExecutionRequest { return createSqlExecutionRequest( - dbName, - databaseDetailQueries.map(query => query.createStmt(dbName)), + params.database, + databaseDetailQueries.map(query => + query.createStmt(params.database, params.csIndexUnusedDuration), + ), ); } export async function getDatabaseDetails( - databaseName: string, + params: DatabaseDetailsReqParams, timeout?: moment.Duration, ): Promise> { - return withTimeout(fetchDatabaseDetails(databaseName), timeout); + return withTimeout(fetchDatabaseDetails(params), timeout); } async function fetchDatabaseDetails( - databaseName: string, + params: DatabaseDetailsReqParams, ): Promise> { const detailsResponse: DatabaseDetailsResponse = newDatabaseDetailsResponse(); - const req: SqlExecutionRequest = createDatabaseDetailsReq(databaseName); + const req: SqlExecutionRequest = createDatabaseDetailsReq(params); const resp = await executeInternalSql(req); resp.execution.txn_results.forEach(txn_result => { if (txn_result.rows) { @@ -493,7 +496,7 @@ async function fetchDatabaseDetails( }); if (resp.error) { if (resp.error.message.includes("max result size exceeded")) { - return fetchSeparatelyDatabaseDetails(databaseName); + return fetchSeparatelyDatabaseDetails(params); } detailsResponse.error = resp.error; } @@ -505,12 +508,15 @@ async function fetchDatabaseDetails( } async function fetchSeparatelyDatabaseDetails( - databaseName: string, + params: DatabaseDetailsReqParams, ): Promise> { const detailsResponse: DatabaseDetailsResponse = newDatabaseDetailsResponse(); for (const databaseDetailQuery of databaseDetailQueries) { - const req = createSqlExecutionRequest(databaseName, [ - databaseDetailQuery.createStmt(databaseName), + const req = createSqlExecutionRequest(params.database, [ + databaseDetailQuery.createStmt( + params.database, + params.csIndexUnusedDuration, + ), ]); const resp = await executeInternalSql(req); const txn_result = resp.execution.txn_results[0]; @@ -520,7 +526,7 @@ async function fetchSeparatelyDatabaseDetails( if (resp.error) { const handleFailure = await databaseDetailQuery.handleMaxSizeError( - databaseName, + params.database, txn_result, detailsResponse, ); diff --git a/pkg/ui/workspaces/cluster-ui/src/api/tableDetailsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/tableDetailsApi.ts index 1e5cd8836818..9116685f3d3d 100644 --- a/pkg/ui/workspaces/cluster-ui/src/api/tableDetailsApi.ts +++ b/pkg/ui/workspaces/cluster-ui/src/api/tableDetailsApi.ts @@ -476,22 +476,20 @@ type TableIndexUsageStats = { }; const getTableIndexUsageStats: TableDetailsQuery = { - createStmt: (dbName, tableName) => { + createStmt: (dbName, tableName, csIndexUnusedDuration) => { const escFullTableName = Join( [new Identifier(dbName), new SQL(tableName)], new SQL("."), ); return { sql: Format( - `WITH - cs AS (SELECT value FROM crdb_internal.cluster_settings WHERE variable = 'sql.index_recommendation.drop_unused_duration'), - tableId AS (SELECT $1::regclass::int as table_id) + `WITH tableId AS (SELECT $1::regclass::int as table_id) SELECT * FROM (SELECT ti.created_at, us.last_read, us.total_reads, - 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 %1.crdb_internal.index_usage_statistics AS us JOIN tableId ON us.table_id = tableId.table_id @@ -499,7 +497,6 @@ const getTableIndexUsageStats: TableDetailsQuery = { us.index_id = ti.index_id AND tableId.table_id = ti.descriptor_id ) - CROSS JOIN cs WHERE $2 != 'system' AND ti.is_unique IS false) WHERE unused_interval > interval_threshold ORDER BY total_reads DESC`, @@ -522,7 +519,11 @@ const getTableIndexUsageStats: TableDetailsQuery = { }; type TableDetailsQuery = { - createStmt: (dbName: string, tableName: string) => SqlStatement; + createStmt: ( + dbName: string, + tableName: string, + csIndexUnusedDuration: string, + ) => SqlStatement; addToTableDetail: ( response: SqlTxnResult, tableDetail: TableDetailsResponse, @@ -557,11 +558,12 @@ const tableDetailQueries: TableDetailsQuery[] = [ export function createTableDetailsReq( dbName: string, tableName: string, + csIndexUnusedDuration: string, ): SqlExecutionRequest { return { execute: true, statements: tableDetailQueries.map(query => - query.createStmt(dbName, tableName), + query.createStmt(dbName, tableName, csIndexUnusedDuration), ), max_result_size: LARGE_RESULT_SIZE, timeout: LONG_TIMEOUT, @@ -573,23 +575,33 @@ export type TableDetailsReqParams = { database: string; // Note: table name is expected in the following format: "schemaName"."tableName" table: string; + csIndexUnusedDuration: string; }; export async function getTableDetails( params: TableDetailsReqParams, timeout?: moment.Duration, ): Promise> { - return withTimeout(fetchTableDetails(params.database, params.table), timeout); + return withTimeout( + fetchTableDetails( + params.database, + params.table, + params.csIndexUnusedDuration, + ), + timeout, + ); } async function fetchTableDetails( databaseName: string, tableName: string, + csIndexUnusedDuration: string, ): Promise> { const detailsResponse: TableDetailsResponse = newTableDetailsResponse(); const req: SqlExecutionRequest = createTableDetailsReq( databaseName, tableName, + csIndexUnusedDuration, ); const resp = await executeInternalSql(req); resp.execution.txn_results.forEach(txn_result => { diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsConnected.ts b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsConnected.ts index 2397d5488227..490a8f7d9980 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsConnected.ts +++ b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsConnected.ts @@ -38,7 +38,10 @@ import { selectDatabaseDetailsViewModeSetting, } from "../store/databaseDetails/databaseDetails.selectors"; import { combineLoadingErrors, deriveTableDetailsMemoized } from "../databases"; -import { selectIndexRecommendationsEnabled } from "../store/clusterSettings/clusterSettings.selectors"; +import { + selectDropUnusedIndexDuration, + selectIndexRecommendationsEnabled, +} from "../store/clusterSettings/clusterSettings.selectors"; const mapStateToProps = ( state: AppState, @@ -75,17 +78,26 @@ const mapStateToProps = ( isTenant, }), showIndexRecommendations: selectIndexRecommendationsEnabled(state), + csIndexUnusedDuration: selectDropUnusedIndexDuration(state), }; }; const mapDispatchToProps = ( dispatch: Dispatch, ): DatabaseDetailsPageActions => ({ - refreshDatabaseDetails: (database: string) => { - dispatch(databaseDetailsActions.refresh(database)); + refreshDatabaseDetails: (database: string, csIndexUnusedDuration: string) => { + dispatch( + databaseDetailsActions.refresh({ database, csIndexUnusedDuration }), + ); }, - refreshTableDetails: (database: string, table: string) => { - dispatch(tableDetailsActions.refresh({ database, table })); + refreshTableDetails: ( + database: string, + table: string, + csIndexUnusedDuration: string, + ) => { + dispatch( + tableDetailsActions.refresh({ database, table, csIndexUnusedDuration }), + ); }, onViewModeChange: (viewMode: ViewMode) => { dispatch( diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx index 5ad175888075..a063c3668493 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx @@ -109,6 +109,7 @@ export interface DatabaseDetailsPageData { viewMode: ViewMode; showNodeRegionsColumn?: boolean; showIndexRecommendations: boolean; + csIndexUnusedDuration: string; } export interface DatabaseDetailsPageDataTable { @@ -141,8 +142,15 @@ export interface DatabaseDetailsPageDataTableDetails { } export interface DatabaseDetailsPageActions { - refreshDatabaseDetails: (database: string) => void; - refreshTableDetails: (database: string, table: string) => void; + refreshDatabaseDetails: ( + database: string, + csIndexUnusedDuration: string, + ) => void; + refreshTableDetails: ( + database: string, + table: string, + csIndexUnusedDuration: string, + ) => void; onFilterChange?: (value: Filters) => void; onSearchComplete?: (query: string) => void; onSortingTablesChange?: (columnTitle: string, ascending: boolean) => void; @@ -245,7 +253,10 @@ export class DatabaseDetailsPage extends React.Component< componentDidMount(): void { if (!this.props.loaded && !this.props.loading && !this.props.lastError) { - this.props.refreshDatabaseDetails(this.props.name); + this.props.refreshDatabaseDetails( + this.props.name, + this.props.csIndexUnusedDuration, + ); } else { // If the props are already loaded then componentDidUpdate // will not get called so call refresh to make sure details @@ -342,7 +353,11 @@ export class DatabaseDetailsPage extends React.Component< (table.lastError === undefined || table.lastError?.name === "GetDatabaseInfoError") ) { - this.props.refreshTableDetails(this.props.name, table.name); + this.props.refreshTableDetails( + this.props.name, + table.name, + this.props.csIndexUnusedDuration, + ); } }); } diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx index 642e7cb41c0c..21ed05dc6f27 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx @@ -116,6 +116,7 @@ export interface DatabaseTablePageData { showIndexRecommendations: boolean; automaticStatsCollectionEnabled?: boolean; hasAdminRole?: UIConfigState["hasAdminRole"]; + csIndexUnusedDuration: string; } export interface DatabaseTablePageDataDetails { @@ -162,7 +163,11 @@ interface Grant { } export interface DatabaseTablePageActions { - refreshTableDetails: (database: string, table: string) => void; + refreshTableDetails: ( + database: string, + table: string, + csIndexUnusedDuration: string, + ) => void; refreshSettings: () => void; refreshIndexStats?: (database: string, table: string) => void; resetIndexUsageStats?: (database: string, table: string) => void; @@ -274,6 +279,7 @@ export class DatabaseTablePage extends React.Component< return this.props.refreshTableDetails( this.props.databaseName, this.props.name, + this.props.csIndexUnusedDuration, ); } diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePageConnected.ts b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePageConnected.ts index 5483d81bba84..fd5fdfc1d644 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePageConnected.ts +++ b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePageConnected.ts @@ -25,6 +25,7 @@ import { } from "src/store/nodes"; import { selectAutomaticStatsCollectionEnabled, + selectDropUnusedIndexDuration, selectIndexRecommendationsEnabled, selectIndexUsageStatsEnabled, } from "src/store/clusterSettings/clusterSettings.selectors"; @@ -74,6 +75,7 @@ export const mapStateToProps = ( selectAutomaticStatsCollectionEnabled(state), indexUsageStatsEnabled: selectIndexUsageStatsEnabled(state), showIndexRecommendations: selectIndexRecommendationsEnabled(state), + csIndexUnusedDuration: selectDropUnusedIndexDuration(state), hasAdminRole: selectHasAdminRole(state), indexStats: { loading: !!indexStatsState?.inFlight, @@ -88,8 +90,14 @@ export const mapStateToProps = ( export const mapDispatchToProps = ( dispatch: Dispatch, ): DatabaseTablePageActions => ({ - refreshTableDetails: (database: string, table: string) => { - dispatch(tableDetailsActions.refresh({ database, table })); + refreshTableDetails: ( + database: string, + table: string, + csIndexUnusedDuration: string, + ) => { + dispatch( + tableDetailsActions.refresh({ database, table, csIndexUnusedDuration }), + ); }, refreshIndexStats: (database: string, table: string) => { dispatch( diff --git a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx index 48c136f2f7c4..879108dde39e 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx @@ -94,6 +94,7 @@ export interface DatabasesPageData { automaticStatsCollectionEnabled?: boolean; indexRecommendationsEnabled: boolean; showNodeRegionsColumn?: boolean; + csIndexUnusedDuration: string; } export interface DatabasesPageDataDatabase { @@ -115,7 +116,10 @@ export interface DatabasesPageDataDatabase { export interface DatabasesPageActions { refreshDatabases: () => void; - refreshDatabaseDetails: (database: string) => void; + refreshDatabaseDetails: ( + database: string, + csIndexUnusedDuration: string, + ) => void; refreshSettings: () => void; refreshNodes?: () => void; onFilterChange?: (value: Filters) => void; @@ -336,7 +340,10 @@ export class DatabasesPage extends React.Component< !database.loading && database.lastError === undefined ) { - this.props.refreshDatabaseDetails(database.name); + this.props.refreshDatabaseDetails( + database.name, + this.props.csIndexUnusedDuration, + ); } }); } diff --git a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPageConnected.ts b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPageConnected.ts index 7d051d42989f..6accc08cf186 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPageConnected.ts +++ b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPageConnected.ts @@ -39,6 +39,7 @@ import { Filters } from "../queryFilter"; import { actions as analyticsActions } from "../store/analytics"; import { selectAutomaticStatsCollectionEnabled, + selectDropUnusedIndexDuration, selectIndexRecommendationsEnabled, } from "../store/clusterSettings/clusterSettings.selectors"; import { deriveDatabaseDetailsMemoized } from "../databases"; @@ -67,6 +68,7 @@ const mapStateToProps = (state: AppState): DatabasesPageData => { // Do not show node/regions columns for serverless. indexRecommendationsEnabled: selectIndexRecommendationsEnabled(state), showNodeRegionsColumn: Object.keys(nodeRegions).length > 1 && !isTenant, + csIndexUnusedDuration: selectDropUnusedIndexDuration(state), }; }; @@ -74,8 +76,10 @@ const mapDispatchToProps = (dispatch: Dispatch): DatabasesPageActions => ({ refreshDatabases: () => { dispatch(databasesListActions.refresh()); }, - refreshDatabaseDetails: (database: string) => { - dispatch(databaseDetailsActions.refresh(database)); + refreshDatabaseDetails: (database: string, csIndexUnusedDuration: string) => { + dispatch( + databaseDetailsActions.refresh({ database, csIndexUnusedDuration }), + ); }, refreshSettings: () => { dispatch(clusterSettingsActions.refresh()); 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 42e61861eb5f..826c3f55de1d 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 @@ -39,3 +39,13 @@ export const selectIndexUsageStatsEnabled = (state: AppState): boolean => { const value = settings["version"]?.value || ""; return greaterOrEqualThanVersion(value, [22, 1, 0]); }; + +export const selectDropUnusedIndexDuration = (state: AppState): string => { + const settings = state.adminUI?.clusterSettings.data?.key_values; + if (!settings) { + return "168h"; + } + return ( + settings["sql.index_recommendation.drop_unused_duration"]?.value || "168h" + ); +}; diff --git a/pkg/ui/workspaces/cluster-ui/src/store/databaseDetails/databaseDetails.reducer.ts b/pkg/ui/workspaces/cluster-ui/src/store/databaseDetails/databaseDetails.reducer.ts index 7e210b4ee97b..38e0816ab471 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/databaseDetails/databaseDetails.reducer.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/databaseDetails/databaseDetails.reducer.ts @@ -9,6 +9,7 @@ // licenses/APL.txt. import { + DatabaseDetailsReqParams, DatabaseDetailsResponse, ErrorWithKey, SqlApiResponse, @@ -55,16 +56,16 @@ const databaseDetailsReducer = createSlice({ lastError: action.payload.err, }; }, - refresh: (state, action: PayloadAction) => { - state[action.payload] = { + refresh: (state, action: PayloadAction) => { + state[action.payload.database] = { valid: false, inFlight: true, data: null, lastError: null, }; }, - request: (state, action: PayloadAction) => { - state[action.payload] = { + request: (state, action: PayloadAction) => { + state[action.payload.database] = { valid: false, inFlight: true, data: null, 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 3f794122467c..c503088d8d4c 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 @@ -18,6 +18,7 @@ import { import * as matchers from "redux-saga-test-plan/matchers"; import { expectSaga } from "redux-saga-test-plan"; import { + DatabaseDetailsReqParams, DatabaseDetailsResponse, getDatabaseDetails, SqlApiResponse, @@ -36,8 +37,9 @@ import { describe("DatabaseDetails sagas", () => { const database = "test_db"; - const requestAction: PayloadAction = { - payload: database, + const csIndexUnusedDuration = "168h"; + const requestAction: PayloadAction = { + payload: { database, csIndexUnusedDuration }, type: "request", }; const databaseDetailsResponse: SqlApiResponse = { diff --git a/pkg/ui/workspaces/cluster-ui/src/store/databaseDetails/databaseDetails.saga.ts b/pkg/ui/workspaces/cluster-ui/src/store/databaseDetails/databaseDetails.saga.ts index 71a9d9dbf50c..e6fd1cbf9a7e 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/databaseDetails/databaseDetails.saga.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/databaseDetails/databaseDetails.saga.ts @@ -11,16 +11,22 @@ import { all, call, put, takeEvery } from "redux-saga/effects"; import { actions } from "./databaseDetails.reducer"; -import { ErrorWithKey, getDatabaseDetails } from "src/api"; +import { + DatabaseDetailsReqParams, + ErrorWithKey, + getDatabaseDetails, +} from "src/api"; import moment from "moment"; import { PayloadAction } from "@reduxjs/toolkit"; -export function* refreshDatabaseDetailsSaga(action: PayloadAction) { +export function* refreshDatabaseDetailsSaga( + action: PayloadAction, +) { yield put(actions.request(action.payload)); } export function* requestDatabaseDetailsSaga( - action: PayloadAction, + action: PayloadAction, ): any { try { const result = yield call( @@ -30,14 +36,14 @@ export function* requestDatabaseDetailsSaga( ); yield put( actions.received({ - key: action.payload, + key: action.payload.database, databaseDetailsResponse: result, }), ); } catch (e) { const err: ErrorWithKey = { err: e, - key: action.payload, + key: action.payload.database, }; yield put(actions.failed(err)); } 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 1fc8158c4a88..01885368df32 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 @@ -40,9 +40,10 @@ import { generateTableID } 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 }, + payload: { database, table, csIndexUnusedDuration }, type: "request", }; const tableDetailsResponse: SqlApiResponse = { 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 a9d8284851fc..fcda692ca5d0 100644 --- a/pkg/ui/workspaces/db-console/src/redux/apiReducers.spec.ts +++ b/pkg/ui/workspaces/db-console/src/redux/apiReducers.spec.ts @@ -42,14 +42,19 @@ 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"; - expect(databaseRequestPayloadToID(database)).toEqual(database); + const csIndexUnusedDuration = "168h"; + expect( + databaseRequestPayloadToID({ database, csIndexUnusedDuration }), + ).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, }; expect(tableRequestToID(tableRequest)).toEqual( util.generateTableID(database, table), diff --git a/pkg/ui/workspaces/db-console/src/redux/apiReducers.ts b/pkg/ui/workspaces/db-console/src/redux/apiReducers.ts index b89fc635f361..3d5a07ce3578 100644 --- a/pkg/ui/workspaces/db-console/src/redux/apiReducers.ts +++ b/pkg/ui/workspaces/db-console/src/redux/apiReducers.ts @@ -109,7 +109,9 @@ const databasesReducerObj = new CachedDataReducer( ); export const refreshDatabases = databasesReducerObj.refresh; -export const databaseRequestPayloadToID = (dbName: string): string => dbName; +export const databaseRequestPayloadToID = ( + params: clusterUiApi.DatabaseDetailsReqParams, +): string => params.database; const databaseDetailsReducerObj = new KeyedCachedDataReducer( clusterUiApi.getDatabaseDetails, 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 b068dc49bde0..832bf04140e1 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 @@ -105,3 +105,15 @@ export const selectIndexUsageStatsEnabled = createSelector( return util.greaterOrEqualThanVersion(value, [22, 1, 0]); }, ); + +export const selectDropUnusedIndexDuration = createSelector( + selectClusterSettings, + (settings): string => { + if (!settings) { + return "168h"; + } + return ( + settings["sql.index_recommendation.drop_unused_duration"]?.value || "168h" + ); + }, +); 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 8b0e5d41ca25..281f26cc569d 100644 --- a/pkg/ui/workspaces/db-console/src/util/api.spec.ts +++ b/pkg/ui/workspaces/db-console/src/util/api.spec.ts @@ -103,7 +103,8 @@ describe("rest api", function () { }); describe("database details request", function () { - const dbName = "test"; + const database = "test"; + const csIndexUnusedDuration = "168h"; const mockOldDate = new Date(2023, 2, 3); const mockZoneConfig = new ZoneConfig({ inherited_constraints: true, @@ -125,7 +126,10 @@ describe("rest api", function () { it("correctly requests info about a specific database", function () { // Mock out the fetch query stubSqlApiCall( - clusterUiApi.createDatabaseDetailsReq(dbName), + clusterUiApi.createDatabaseDetailsReq({ + database, + csIndexUnusedDuration, + }), [ // Database ID query { rows: [{ database_id: "1" }] }, @@ -187,30 +191,37 @@ describe("rest api", function () { ], ); - return clusterUiApi.getDatabaseDetails(dbName).then(result => { - expect(fetchMock.calls(clusterUiApi.SQL_API_PATH).length).toBe(1); - expect(result.results.idResp.database_id).toEqual("1"); - expect(result.results.tablesResp.tables.length).toBe(1); - expect(result.results.grantsResp.grants.length).toBe(2); - expect(result.results.stats.indexStats.num_index_recommendations).toBe( - 1, - ); - expect(result.results.zoneConfigResp.zone_config).toEqual( - mockZoneConfig, - ); - expect(result.results.zoneConfigResp.zone_config_level).toBe( - ZoneConfigurationLevel.DATABASE, - ); - expect(result.results.stats.spanStats.approximate_disk_bytes).toBe(100); - expect(result.results.stats.spanStats.live_bytes).toBe(200); - expect(result.results.stats.spanStats.total_bytes).toBe(300); - expect(result.results.stats.spanStats.range_count).toBe(400); - }); + return clusterUiApi + .getDatabaseDetails({ database, csIndexUnusedDuration }) + .then(result => { + expect(fetchMock.calls(clusterUiApi.SQL_API_PATH).length).toBe(1); + expect(result.results.idResp.database_id).toEqual("1"); + expect(result.results.tablesResp.tables.length).toBe(1); + expect(result.results.grantsResp.grants.length).toBe(2); + expect( + result.results.stats.indexStats.num_index_recommendations, + ).toBe(1); + expect(result.results.zoneConfigResp.zone_config).toEqual( + mockZoneConfig, + ); + expect(result.results.zoneConfigResp.zone_config_level).toBe( + ZoneConfigurationLevel.DATABASE, + ); + expect(result.results.stats.spanStats.approximate_disk_bytes).toBe( + 100, + ); + expect(result.results.stats.spanStats.live_bytes).toBe(200); + expect(result.results.stats.spanStats.total_bytes).toBe(300); + expect(result.results.stats.spanStats.range_count).toBe(400); + }); }); it("correctly handles an error", function (done) { // Mock out the fetch query, but return a 500 status code - const req = clusterUiApi.createDatabaseDetailsReq(dbName); + const req = clusterUiApi.createDatabaseDetailsReq({ + database, + csIndexUnusedDuration, + }); fetchMock.mock({ matcher: clusterUiApi.SQL_API_PATH, method: "POST", @@ -225,7 +236,7 @@ describe("rest api", function () { }); clusterUiApi - .getDatabaseDetails(dbName) + .getDatabaseDetails({ database, csIndexUnusedDuration }) .then(_result => { done(new Error("Request unexpectedly succeeded.")); }) @@ -237,7 +248,10 @@ describe("rest api", function () { it("correctly times out", function (done) { // Mock out the fetch query, but return a promise that's never resolved to test the timeout - const req = clusterUiApi.createDatabaseDetailsReq(dbName); + const req = clusterUiApi.createDatabaseDetailsReq({ + database, + csIndexUnusedDuration, + }); fetchMock.mock({ matcher: clusterUiApi.SQL_API_PATH, method: "POST", @@ -252,7 +266,10 @@ describe("rest api", function () { }); clusterUiApi - .getDatabaseDetails(dbName, moment.duration(0)) + .getDatabaseDetails( + { database, csIndexUnusedDuration }, + moment.duration(0), + ) .then(_result => { done(new Error("Request unexpectedly succeeded.")); }) @@ -266,6 +283,7 @@ 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, @@ -288,7 +306,11 @@ describe("rest api", function () { it("correctly requests info about a specific table", function () { // Mock out the fetch query stubSqlApiCall( - clusterUiApi.createTableDetailsReq(dbName, tableName), + clusterUiApi.createTableDetailsReq( + dbName, + tableName, + csIndexUnusedDuration, + ), [ // Table ID query { rows: [{ table_id: "1" }] }, @@ -346,6 +368,7 @@ describe("rest api", function () { .getTableDetails({ database: dbName, table: tableName, + csIndexUnusedDuration, }) .then(resp => { expect(fetchMock.calls(clusterUiApi.SQL_API_PATH).length).toBe(1); @@ -391,7 +414,11 @@ describe("rest api", function () { method: "POST", response: (_url: string, requestObj: RequestInit) => { expect(JSON.parse(requestObj.body.toString())).toEqual({ - ...clusterUiApi.createTableDetailsReq(dbName, tableName), + ...clusterUiApi.createTableDetailsReq( + dbName, + tableName, + csIndexUnusedDuration, + ), application_name: clusterUiApi.INTERNAL_SQL_API_APP, }); return { throws: new Error() }; @@ -402,6 +429,7 @@ describe("rest api", function () { .getTableDetails({ database: dbName, table: tableName, + csIndexUnusedDuration, }) .then(_result => { done(new Error("Request unexpectedly succeeded.")); @@ -419,7 +447,11 @@ describe("rest api", function () { method: "POST", response: (_url: string, requestObj: RequestInit) => { expect(JSON.parse(requestObj.body.toString())).toEqual({ - ...clusterUiApi.createTableDetailsReq(dbName, tableName), + ...clusterUiApi.createTableDetailsReq( + dbName, + tableName, + csIndexUnusedDuration, + ), application_name: clusterUiApi.INTERNAL_SQL_API_APP, }); return new Promise(() => {}); @@ -431,6 +463,7 @@ describe("rest api", function () { { database: dbName, table: tableName, + csIndexUnusedDuration, }, moment.duration(0), ) 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 7ea7e3564b4b..b43a59823650 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 @@ -108,11 +108,11 @@ class TestDriver { } async refreshDatabaseDetails() { - return this.actions.refreshDatabaseDetails(this.database); + return this.actions.refreshDatabaseDetails(this.database, "168h"); } async refreshTableDetails(table: string) { - return this.actions.refreshTableDetails(this.database, table); + return this.actions.refreshTableDetails(this.database, table, "168h"); } private findTable(name: string) { @@ -150,12 +150,16 @@ describe("Database Details Page", function () { sortSettingGrants: { ascending: true, columnTitle: "name" }, tables: [], showIndexRecommendations: false, + csIndexUnusedDuration: "168h", }); }); it("makes a row for each table", async function () { fakeApi.stubSqlApiCall( - clusterUiApi.createDatabaseDetailsReq("things"), + clusterUiApi.createDatabaseDetailsReq({ + database: "things", + csUnusedDuration: "168h", + }), [ // Id { rows: [] }, @@ -183,6 +187,7 @@ describe("Database Details Page", function () { isTenant: false, showNodeRegionsColumn: false, showIndexRecommendations: false, + csIndexUnusedDuration: "168h", viewMode: ViewMode.Tables, sortSettingTables: { ascending: true, columnTitle: "name" }, sortSettingGrants: { ascending: true, columnTitle: "name" }, @@ -237,7 +242,10 @@ describe("Database Details Page", function () { it("loads table details", async function () { fakeApi.stubSqlApiCall( - clusterUiApi.createDatabaseDetailsReq("things"), + clusterUiApi.createDatabaseDetailsReq({ + database: "things", + csIndexUnusedDuration: "168h", + }), [ // Id { rows: [] }, @@ -255,7 +263,7 @@ describe("Database Details Page", function () { const mockStatsLastCreatedTimestamp = moment(); fakeApi.stubSqlApiCall( - clusterUiApi.createTableDetailsReq("things", `"public"."foo"`), + clusterUiApi.createTableDetailsReq("things", `"public"."foo"`, "168h"), [ // Table ID query { rows: [{ table_id: "1" }] }, @@ -306,7 +314,7 @@ describe("Database Details Page", function () { ); fakeApi.stubSqlApiCall( - clusterUiApi.createTableDetailsReq("things", `"public"."bar"`), + clusterUiApi.createTableDetailsReq("things", `"public"."bar"`, "168h"), [ // Table ID query { rows: [{ table_id: "2" }] }, @@ -404,7 +412,10 @@ describe("Database Details Page", function () { it("sorts roles meaningfully", async function () { fakeApi.stubSqlApiCall( - clusterUiApi.createDatabaseDetailsReq("things"), + clusterUiApi.createDatabaseDetailsReq({ + database: "things", + csIndexUnusedDuration: "168h", + }), [ // Id { rows: [] }, @@ -418,7 +429,7 @@ describe("Database Details Page", function () { ); fakeApi.stubSqlApiCall( - clusterUiApi.createTableDetailsReq("things", `"public"."foo"`), + clusterUiApi.createTableDetailsReq("things", `"public"."foo"`, "168h"), [ // Table ID query {}, @@ -451,7 +462,10 @@ describe("Database Details Page", function () { it("sorts grants meaningfully", async function () { fakeApi.stubSqlApiCall( - clusterUiApi.createDatabaseDetailsReq("things"), + clusterUiApi.createDatabaseDetailsReq({ + database: "things", + csIndexUnusedDuration: "168h", + }), [ // Id { rows: [] }, @@ -465,7 +479,7 @@ describe("Database Details Page", function () { ); fakeApi.stubSqlApiCall( - clusterUiApi.createTableDetailsReq("things", `"public"."foo"`), + clusterUiApi.createTableDetailsReq("things", `"public"."foo"`, "168h"), [ // Table ID query {}, diff --git a/pkg/ui/workspaces/db-console/src/views/databases/databaseDetailsPage/redux.ts b/pkg/ui/workspaces/db-console/src/views/databases/databaseDetailsPage/redux.ts index 4a0d86b1dd32..89bba471a1de 100644 --- a/pkg/ui/workspaces/db-console/src/views/databases/databaseDetailsPage/redux.ts +++ b/pkg/ui/workspaces/db-console/src/views/databases/databaseDetailsPage/redux.ts @@ -30,7 +30,10 @@ import { nodeRegionsByIDSelector, selectIsMoreThanOneNode, } from "src/redux/nodes"; -import { selectIndexRecommendationsEnabled } from "src/redux/clusterSettings"; +import { + selectDropUnusedIndexDuration, + selectIndexRecommendationsEnabled, +} from "src/redux/clusterSettings"; const sortSettingTablesLocalSetting = new LocalSetting( "sortSetting/DatabasesDetailsTablesPage", @@ -100,15 +103,26 @@ export const mapStateToProps = ( isTenant, }), showIndexRecommendations: selectIndexRecommendationsEnabled(state), + csIndexUnusedDuration: selectDropUnusedIndexDuration(state), }; }; export const mapDispatchToProps = { - refreshDatabaseDetails, - refreshTableDetails: (database: string, table: string) => { + refreshDatabaseDetails: (database: string, csIndexUnusedDuration: string) => { + return refreshDatabaseDetails({ + database, + csIndexUnusedDuration, + }); + }, + refreshTableDetails: ( + database: string, + table: string, + csIndexUnusedDuration: string, + ) => { return refreshTableDetails({ database, table, + csIndexUnusedDuration, }); }, onViewModeChange: (viewMode: ViewMode) => viewModeLocalSetting.set(viewMode), 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 bc2efe74922c..81cf596c43f3 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 @@ -139,7 +139,7 @@ class TestDriver { return this.actions.refreshSettings(); } async refreshTableDetails() { - return this.actions.refreshTableDetails(this.database, this.table); + return this.actions.refreshTableDetails(this.database, this.table, "168h"); } async refreshIndexStats() { @@ -196,6 +196,7 @@ describe("Database Table Page", function () { automaticStatsCollectionEnabled: true, indexUsageStatsEnabled: true, showIndexRecommendations: true, + csIndexUnusedDuration: "168h", hasAdminRole: false, indexStats: { loading: false, @@ -211,7 +212,7 @@ describe("Database Table Page", function () { const mockStatsLastCreatedTimestamp = moment(); fakeApi.stubSqlApiCall( - clusterUiApi.createTableDetailsReq("DATABASE", "TABLE"), + clusterUiApi.createTableDetailsReq("DATABASE", "TABLE", "168h"), [ // Table ID query { rows: [{ table_id: "1" }] }, diff --git a/pkg/ui/workspaces/db-console/src/views/databases/databaseTablePage/redux.ts b/pkg/ui/workspaces/db-console/src/views/databases/databaseTablePage/redux.ts index f32dacc8a620..8ff1036b3c9e 100644 --- a/pkg/ui/workspaces/db-console/src/views/databases/databaseTablePage/redux.ts +++ b/pkg/ui/workspaces/db-console/src/views/databases/databaseTablePage/redux.ts @@ -32,6 +32,7 @@ import { import { selectHasAdminRole } from "src/redux/user"; import { selectAutomaticStatsCollectionEnabled, + selectDropUnusedIndexDuration, selectIndexRecommendationsEnabled, selectIndexUsageStatsEnabled, } from "src/redux/clusterSettings"; @@ -69,6 +70,7 @@ export const mapStateToProps = ( selectAutomaticStatsCollectionEnabled(state) || false, hasAdminRole: selectHasAdminRole(state) || false, showIndexRecommendations: selectIndexRecommendationsEnabled(state), + csIndexUnusedDuration: selectDropUnusedIndexDuration(state), indexUsageStatsEnabled: selectIndexUsageStatsEnabled(state), indexStats: { loading: !!indexStats?.inFlight, @@ -81,10 +83,15 @@ export const mapStateToProps = ( }; export const mapDispatchToProps = { - refreshTableDetails: (database: string, table: string) => { + refreshTableDetails: ( + database: string, + table: string, + csIndexUnusedDuration: string, + ) => { return refreshTableDetails({ database, table, + csIndexUnusedDuration, }); }, refreshIndexStats: (database: string, table: string) => { 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 b9f31f44caef..906bc643c3bf 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 @@ -39,8 +39,11 @@ class TestDriver { return this.actions.refreshDatabases(); } - async refreshDatabaseDetails(database: string) { - return this.actions.refreshDatabaseDetails(database); + async refreshDatabaseDetails( + database: string, + csIndexUnusedDuration: string, + ) { + return this.actions.refreshDatabaseDetails(database, csIndexUnusedDuration); } async refreshNodes() { @@ -101,6 +104,7 @@ describe("Databases Page", function () { automaticStatsCollectionEnabled: true, indexRecommendationsEnabled: true, showNodeRegionsColumn: false, + csIndexUnusedDuration: "168h", }); }); @@ -168,6 +172,7 @@ describe("Databases Page", function () { sortSetting: { ascending: true, columnTitle: "name" }, showNodeRegionsColumn: false, indexRecommendationsEnabled: true, + csIndexUnusedDuration: "168h", automaticStatsCollectionEnabled: true, }); }); @@ -218,7 +223,10 @@ describe("Databases Page", function () { ); fakeApi.stubSqlApiCall( - clusterUiApi.createDatabaseDetailsReq("test"), + clusterUiApi.createDatabaseDetailsReq({ + database: "test", + csUnusedDuration: "168h", + }), [ // Id { rows: [] }, diff --git a/pkg/ui/workspaces/db-console/src/views/databases/databasesPage/redux.ts b/pkg/ui/workspaces/db-console/src/views/databases/databasesPage/redux.ts index 6dc6ea52b036..05bb02c76bb4 100644 --- a/pkg/ui/workspaces/db-console/src/views/databases/databasesPage/redux.ts +++ b/pkg/ui/workspaces/db-console/src/views/databases/databasesPage/redux.ts @@ -30,6 +30,7 @@ import { } from "src/redux/nodes"; import { selectAutomaticStatsCollectionEnabled, + selectDropUnusedIndexDuration, selectIndexRecommendationsEnabled, } from "src/redux/clusterSettings"; @@ -92,13 +93,19 @@ export const mapStateToProps = (state: AdminUIState): DatabasesPageData => { selectAutomaticStatsCollectionEnabled(state), indexRecommendationsEnabled: selectIndexRecommendationsEnabled(state), showNodeRegionsColumn: selectIsMoreThanOneNode(state), + csIndexUnusedDuration: selectDropUnusedIndexDuration(state), }; }; export const mapDispatchToProps = { refreshSettings, refreshDatabases, - refreshDatabaseDetails, + refreshDatabaseDetails: (database: string, csIndexUnusedDuration: string) => { + return refreshDatabaseDetails({ + database, + csIndexUnusedDuration, + }); + }, refreshNodes, onSortingChange: ( _tableName: string,