Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ui: show data when max size reached for schema insight #97312

Merged
merged 1 commit into from
Feb 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 43 additions & 27 deletions pkg/ui/workspaces/cluster-ui/src/api/schemaInsightsApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import {
executeInternalSql,
LONG_TIMEOUT,
sqlResultsAreEmpty,
sqlApiErrorMessage,
LARGE_RESULT_SIZE,
SqlApiResponse,
formatApiResult,
} from "./sqlApi";
import {
InsightRecommendation,
Expand Down Expand Up @@ -128,23 +130,37 @@ function createIndexRecommendationsToSchemaInsight(
return results;
}

// This query have an ORDER BY for the cases where we reach the limit of the sql-api
// and want to return the most used ones as a priority.
const dropUnusedIndexQuery: SchemaInsightQuery<ClusterIndexUsageStatistic> = {
name: "DropIndex",
query: `SELECT
us.table_id,
us.index_id,
us.last_read,
ti.created_at,
ti.index_name,
t.name as table_name,
t.parent_id as database_id,
t.database_name,
t.schema_name,
(SELECT value FROM crdb_internal.cluster_settings WHERE variable = 'sql.index_recommendation.drop_unused_duration') AS unused_threshold
FROM "".crdb_internal.index_usage_statistics AS us
JOIN "".crdb_internal.table_indexes as ti ON us.index_id = ti.index_id AND us.table_id = ti.descriptor_id
JOIN "".crdb_internal.tables as t ON t.table_id = ti.descriptor_id and t.name = ti.descriptor_name
WHERE t.database_name != 'system' AND ti.index_type != 'primary';`,
query: `WITH cs AS (
SELECT value
FROM crdb_internal.cluster_settings
WHERE variable = 'sql.index_recommendation.drop_unused_duration'
)
SELECT * FROM (SELECT us.table_id,
us.index_id,
us.last_read,
us.total_reads,
ti.created_at,
ti.index_name,
t.name as table_name,
t.parent_id as database_id,
t.database_name,
t.schema_name,
cs.value as unused_threshold,
cs.value::interval as interval_threshold,
now() - COALESCE(us.last_read AT TIME ZONE 'UTC', COALESCE(ti.created_at, '0001-01-01')) as unused_interval
FROM "".crdb_internal.index_usage_statistics AS us
JOIN "".crdb_internal.table_indexes as ti
ON us.index_id = ti.index_id AND us.table_id = ti.descriptor_id
JOIN "".crdb_internal.tables as t
ON t.table_id = ti.descriptor_id and t.name = ti.descriptor_name
CROSS JOIN cs
WHERE t.database_name != 'system' AND ti.index_type != 'primary')
WHERE unused_interval > interval_threshold
ORDER BY total_reads DESC;`,
toSchemaInsight: clusterIndexUsageStatsToSchemaInsight,
};

Expand Down Expand Up @@ -181,26 +197,22 @@ const schemaInsightQueries: SchemaInsightQuery<SchemaInsightResponse>[] = [

// getSchemaInsights makes requests over the SQL API and transforms the corresponding
// SQL responses into schema insights.
export async function getSchemaInsights(): Promise<InsightRecommendation[]> {
export async function getSchemaInsights(): Promise<
SqlApiResponse<InsightRecommendation[]>
> {
const request: SqlExecutionRequest = {
statements: schemaInsightQueries.map(insightQuery => ({
sql: insightQuery.query,
})),
execute: true,
max_result_size: LARGE_RESULT_SIZE,
timeout: LONG_TIMEOUT,
};
const result = await executeInternalSql<SchemaInsightResponse>(request);
if (result.error) {
throw new Error(
`Error while retrieving insights information: ${sqlApiErrorMessage(
result.error.message,
)}`,
);
}

const results: InsightRecommendation[] = [];
if (sqlResultsAreEmpty(result)) {
// No data.
return results;
return formatApiResult([], result.error, "retrieving insights information");
}
result.execution.txn_results.map(txn_result => {
// Note: txn_result.statement values begin at 1, not 0.
Expand All @@ -210,5 +222,9 @@ export async function getSchemaInsights(): Promise<InsightRecommendation[]> {
results.push(...insightQuery.toSchemaInsight(txn_result));
}
});
return results;
return formatApiResult(
results,
result.error,
"retrieving insights information",
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,6 @@ describe("recommendDropUnusedIndex", () => {
const mockCurrentTime = moment();
const oneHourAgo: moment.Moment = moment(mockCurrentTime).subtract(1, "hour");

describe("Recently Used Index", () => {
const recentlyUsedIndex: ClusterIndexUsageStatistic = {
table_id: 1,
index_id: 1,
last_read: moment.utc(oneHourAgo, "X").format(),
created_at: null,
index_name: "recent_index",
table_name: "test_table",
database_id: 1,
database_name: "test_db",
schema_name: "public",
unused_threshold: "10h0m0s",
};
it("should not recommend index to be dropped", () => {
expect(recommendDropUnusedIndex(recentlyUsedIndex)).toEqual({
recommend: false,
reason: "",
});
});
});
describe("Never Used Index", () => {
const neverUsedIndex: ClusterIndexUsageStatistic = {
table_id: 1,
Expand Down Expand Up @@ -85,28 +65,6 @@ describe("recommendDropUnusedIndex", () => {
});
});
describe("Index Created But Never Read", () => {
describe("creation date does not exceed unuse duration", () => {
const createdNeverReadIndexNoExceed: ClusterIndexUsageStatistic = {
table_id: 1,
index_id: 1,
last_read: null,
created_at: moment.utc(oneHourAgo, "X").format(),
index_name: "recent_index",
table_name: "test_table",
database_id: 1,
database_name: "test_db",
schema_name: "public",
unused_threshold: "10h0m0s",
};
it("should not recommend index to be dropped", () => {
expect(recommendDropUnusedIndex(createdNeverReadIndexNoExceed)).toEqual(
{
recommend: false,
reason: "",
},
);
});
});
describe("creation date exceeds unuse duration", () => {
const createdNeverReadIndexExceed: ClusterIndexUsageStatistic = {
table_id: 1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,15 @@ export function recommendDropUnusedIndex(
return { recommend: true, reason: indexNeverUsedReason };
}

const duration = moment.duration(moment().diff(lastActive));
const unusedThreshold = moment.duration(
"PT" + clusterIndexUsageStat.unused_threshold.toUpperCase(),
);
if (duration >= unusedThreshold) {
return {
recommend: true,
reason: `This index has not been used in over ${formatMomentDuration(
unusedThreshold,
)} and can be removed for better write performance.`,
};
}
return { recommend: false, reason: "" };
return {
recommend: true,
reason: `This index has not been used in over ${formatMomentDuration(
unusedThreshold,
)} and can be removed for better write performance.`,
};
}

export function formatMomentDuration(duration: moment.Duration): string {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
selectSchemaInsights,
selectSchemaInsightsDatabases,
selectSchemaInsightsError,
selectSchemaInsightsMaxApiSizeReached,
selectSchemaInsightsTypes,
selectFilters,
selectSortSetting,
Expand Down Expand Up @@ -43,6 +44,7 @@ const mapStateToProps = (
filters: selectFilters(state),
sortSetting: selectSortSetting(state),
hasAdminRole: selectHasAdminRole(state),
maxSizeApiReached: selectSchemaInsightsMaxApiSizeReached(state),
});

const mapDispatchToProps = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ import { InsightsError } from "../insightsErrorComponent";
import { Pagination } from "../../pagination";
import { EmptySchemaInsightsTablePlaceholder } from "./emptySchemaInsightsTablePlaceholder";
import { CockroachCloudContext } from "../../contexts";
import { InlineAlert } from "@cockroachlabs/ui-components";
import { insights } from "src/util";
import { Anchor } from "src/anchor";
import insightTableStyles from "../../insightsTable/insightsTable.module.scss";
const cx = classNames.bind(styles);
const sortableTableCx = classNames.bind(sortableTableStyles);
Expand All @@ -51,6 +54,7 @@ export type SchemaInsightsViewStateProps = {
filters: SchemaInsightEventFilters;
sortSetting: SortSetting;
hasAdminRole: boolean;
maxSizeApiReached?: boolean;
};

export type SchemaInsightsViewDispatchProps = {
Expand All @@ -77,6 +81,7 @@ export const SchemaInsightsView: React.FC<SchemaInsightsViewProps> = ({
refreshUserSQLRoles,
onFiltersChange,
onSortChange,
maxSizeApiReached,
}: SchemaInsightsViewProps) => {
const isCockroachCloud = useContext(CockroachCloudContext);
const [pagination, setPagination] = useState<ISortedTablePagination>({
Expand Down Expand Up @@ -259,6 +264,20 @@ export const SchemaInsightsView: React.FC<SchemaInsightsViewProps> = ({
total={filteredSchemaInsights?.length}
onChange={onChangePage}
/>
{maxSizeApiReached && (
<InlineAlert
intent="info"
title={
<>
Not all insights are displayed because the maximum number of
insights was reached in the console.&nbsp;
<Anchor href={insights} target="_blank">
Learn more
</Anchor>
</>
}
/>
)}
</div>
</Loading>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ function descriptionCell(
}

function linkCell(insightRec: InsightRecommendation): React.ReactElement {
switch (insightRec.execution.execType) {
switch (insightRec.execution?.execType) {
case InsightExecEnum.STATEMENT:
return (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ import { createSlice, PayloadAction } from "@reduxjs/toolkit";
import { DOMAIN_NAME, noopReducer } from "../utils";
import moment, { Moment } from "moment";
import { InsightRecommendation } from "../../insights";
import { SqlApiResponse } from "src/api";

export type SchemaInsightsState = {
data: InsightRecommendation[];
data: SqlApiResponse<InsightRecommendation[]>;
lastUpdated: Moment;
lastError: Error;
valid: boolean;
Expand All @@ -31,7 +32,10 @@ const schemaInsightsSlice = createSlice({
name: `${DOMAIN_NAME}/schemaInsightsSlice`,
initialState,
reducers: {
received: (state, action: PayloadAction<InsightRecommendation[]>) => {
received: (
state,
action: PayloadAction<SqlApiResponse<InsightRecommendation[]>>,
) => {
state.data = action.payload;
state.valid = true;
state.lastError = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
} from "redux-saga-test-plan/providers";
import * as matchers from "redux-saga-test-plan/matchers";
import moment from "moment";
import { getSchemaInsights } from "../../api";
import { getSchemaInsights, SqlApiResponse } from "../../api";
import {
refreshSchemaInsightsSaga,
requestSchemaInsightsSaga,
Expand All @@ -40,7 +40,7 @@ describe("SchemaInsights sagas", () => {
spy.mockRestore();
});

const schemaInsightsResponse: InsightRecommendation[] = [
const schemaInsights: InsightRecommendation[] = [
{
type: "DropIndex",
database: "test_database",
Expand All @@ -55,6 +55,11 @@ describe("SchemaInsights sagas", () => {
},
];

const schemaInsightsResponse: SqlApiResponse<InsightRecommendation[]> = {
maxSizeReached: false,
results: schemaInsights,
};

const schemaInsightsAPIProvider: (EffectProviders | StaticProvider)[] = [
[matchers.call.fn(getSchemaInsights), schemaInsightsResponse],
];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const selectSchemaInsights = createSelector(
selectSchemaInsightState,
schemaInsightState => {
if (!schemaInsightState.data) return null;
return schemaInsightState.data;
return schemaInsightState.data?.results;
},
);

Expand All @@ -36,6 +36,14 @@ export const selectSchemaInsightsError = createSelector(
},
);

export const selectSchemaInsightsMaxApiSizeReached = createSelector(
selectSchemaInsightState,
schemaInsightState => {
if (!schemaInsightState.data) return false;
return schemaInsightState.data?.maxSizeReached;
},
);

export const selectSchemaInsightsDatabases = createSelector(
selectSchemaInsights,
schemaInsights => {
Expand Down
4 changes: 3 additions & 1 deletion pkg/ui/workspaces/db-console/src/redux/apiReducers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,9 @@ export interface APIReducersState {
txnInsights: CachedDataReducerState<
clusterUiApi.SqlApiResponse<TxnInsightEvent[]>
>;
schemaInsights: CachedDataReducerState<clusterUiApi.InsightRecommendation[]>;
schemaInsights: CachedDataReducerState<
clusterUiApi.SqlApiResponse<clusterUiApi.InsightRecommendation[]>
>;
statementFingerprintInsights: KeyedCachedDataReducerState<
clusterUiApi.SqlApiResponse<StmtInsightEvent[]>
>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,16 @@ export const selectSchemaInsights = createSelector(
(state: AdminUIState) => state.cachedData,
adminUiState => {
if (!adminUiState.schemaInsights) return [];
return adminUiState.schemaInsights.data;
return adminUiState.schemaInsights.data?.results;
},
);

export const selectSchemaInsightsMaxApiReached = (
state: AdminUIState,
): boolean => {
return !!state.cachedData.schemaInsights?.data?.maxSizeReached;
};

export const selectSchemaInsightsDatabases = createSelector(
selectSchemaInsights,
schemaInsights => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
schemaInsightsSortLocalSetting,
selectSchemaInsights,
selectSchemaInsightsDatabases,
selectSchemaInsightsMaxApiReached,
selectSchemaInsightsTypes,
} from "src/views/insights/insightsSelectors";
import { selectHasAdminRole } from "src/redux/user";
Expand All @@ -42,6 +43,7 @@ const mapStateToProps = (
filters: schemaInsightsFiltersLocalSetting.selector(state),
sortSetting: schemaInsightsSortLocalSetting.selector(state),
hasAdminRole: selectHasAdminRole(state),
maxSizeApiReached: selectSchemaInsightsMaxApiReached(state),
});

const mapDispatchToProps = {
Expand Down