Skip to content

Commit

Permalink
ui: user cluster settings from redux
Browse files Browse the repository at this point in the history
Part Of cockroachdb#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
  • Loading branch information
maryliag committed Aug 11, 2023
1 parent d9d421e commit aad0ce7
Show file tree
Hide file tree
Showing 23 changed files with 304 additions and 111 deletions.
52 changes: 29 additions & 23 deletions pkg/ui/workspaces/cluster-ui/src/api/databaseDetailsApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<DatabaseIdRow>;
grantsResp: SqlApiQueryResponse<DatabaseGrantsResponse>;
Expand Down Expand Up @@ -387,24 +392,18 @@ type DatabaseIndexUsageStatsResponse = {
};

const getDatabaseIndexUsageStats: DatabaseDetailsQuery<IndexUsageStatistic> = {
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;`,
Expand Down Expand Up @@ -442,7 +441,7 @@ export type DatabaseDetailsRow =
| IndexUsageStatistic;

type DatabaseDetailsQuery<RowType> = {
createStmt: (dbName: string) => SqlStatement;
createStmt: (dbName: string, csIndexUnusedDuration: string) => SqlStatement;
addToDatabaseDetail: (
response: SqlTxnResult<RowType>,
dbDetail: DatabaseDetailsResponse,
Expand All @@ -464,25 +463,29 @@ const databaseDetailQueries: DatabaseDetailsQuery<DatabaseDetailsRow>[] = [
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<SqlApiResponse<DatabaseDetailsResponse>> {
return withTimeout(fetchDatabaseDetails(databaseName), timeout);
return withTimeout(fetchDatabaseDetails(params), timeout);
}

async function fetchDatabaseDetails(
databaseName: string,
params: DatabaseDetailsReqParams,
): Promise<SqlApiResponse<DatabaseDetailsResponse>> {
const detailsResponse: DatabaseDetailsResponse = newDatabaseDetailsResponse();
const req: SqlExecutionRequest = createDatabaseDetailsReq(databaseName);
const req: SqlExecutionRequest = createDatabaseDetailsReq(params);
const resp = await executeInternalSql<DatabaseDetailsRow>(req);
resp.execution.txn_results.forEach(txn_result => {
if (txn_result.rows) {
Expand All @@ -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;
}
Expand All @@ -505,12 +508,15 @@ async function fetchDatabaseDetails(
}

async function fetchSeparatelyDatabaseDetails(
databaseName: string,
params: DatabaseDetailsReqParams,
): Promise<SqlApiResponse<DatabaseDetailsResponse>> {
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<DatabaseDetailsRow>(req);
const txn_result = resp.execution.txn_results[0];
Expand All @@ -520,7 +526,7 @@ async function fetchSeparatelyDatabaseDetails(

if (resp.error) {
const handleFailure = await databaseDetailQuery.handleMaxSizeError(
databaseName,
params.database,
txn_result,
detailsResponse,
);
Expand Down
32 changes: 22 additions & 10 deletions pkg/ui/workspaces/cluster-ui/src/api/tableDetailsApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -476,30 +476,27 @@ type TableIndexUsageStats = {
};

const getTableIndexUsageStats: TableDetailsQuery<IndexUsageStatistic> = {
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
JOIN %1.crdb_internal.table_indexes AS ti ON (
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`,
Expand All @@ -522,7 +519,11 @@ const getTableIndexUsageStats: TableDetailsQuery<IndexUsageStatistic> = {
};

type TableDetailsQuery<RowType> = {
createStmt: (dbName: string, tableName: string) => SqlStatement;
createStmt: (
dbName: string,
tableName: string,
csIndexUnusedDuration: string,
) => SqlStatement;
addToTableDetail: (
response: SqlTxnResult<RowType>,
tableDetail: TableDetailsResponse,
Expand Down Expand Up @@ -557,11 +558,12 @@ const tableDetailQueries: TableDetailsQuery<TableDetailsRow>[] = [
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,
Expand All @@ -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<SqlApiResponse<TableDetailsResponse>> {
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<SqlApiResponse<TableDetailsResponse>> {
const detailsResponse: TableDetailsResponse = newTableDetailsResponse();
const req: SqlExecutionRequest = createTableDetailsReq(
databaseName,
tableName,
csIndexUnusedDuration,
);
const resp = await executeInternalSql<TableDetailsRow>(req);
resp.execution.txn_results.forEach(txn_result => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export interface DatabaseDetailsPageData {
viewMode: ViewMode;
showNodeRegionsColumn?: boolean;
showIndexRecommendations: boolean;
csIndexUnusedDuration: string;
}

export interface DatabaseDetailsPageDataTable {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export interface DatabaseTablePageData {
showIndexRecommendations: boolean;
automaticStatsCollectionEnabled?: boolean;
hasAdminRole?: UIConfigState["hasAdminRole"];
csIndexUnusedDuration: string;
}

export interface DatabaseTablePageDataDetails {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -274,6 +279,7 @@ export class DatabaseTablePage extends React.Component<
return this.props.refreshTableDetails(
this.props.databaseName,
this.props.name,
this.props.csIndexUnusedDuration,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
} from "src/store/nodes";
import {
selectAutomaticStatsCollectionEnabled,
selectDropUnusedIndexDuration,
selectIndexRecommendationsEnabled,
selectIndexUsageStatsEnabled,
} from "src/store/clusterSettings/clusterSettings.selectors";
Expand Down Expand Up @@ -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,
Expand All @@ -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(
Expand Down
Loading

0 comments on commit aad0ce7

Please sign in to comment.