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

cluster-ui: handle partial response errors on the databases page #109245

Merged
merged 1 commit into from
Aug 31, 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
2 changes: 1 addition & 1 deletion pkg/sql/planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,7 @@ func (p *planner) GetDetailsForSpanStats(
return p.QueryIteratorEx(
ctx,
"crdb_internal.database_span_stats",
sessiondata.NoSessionDataOverride,
sessiondata.NodeUserSessionDataOverride,
query,
args...,
)
Expand Down
63 changes: 40 additions & 23 deletions pkg/ui/workspaces/cluster-ui/src/api/databaseDetailsApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
// licenses/APL.txt.

import {
combineQueryErrors,
createSqlExecutionRequest,
executeInternalSql,
formatApiResult,
Expand All @@ -17,6 +18,7 @@ import {
SqlApiResponse,
SqlExecutionErrorMessage,
SqlExecutionRequest,
sqlResultsAreEmpty,
SqlStatement,
SqlTxnResult,
txnResultIsEmpty,
Expand Down Expand Up @@ -100,7 +102,7 @@ const getDatabaseId: DatabaseDetailsQuery<DatabaseIdRow> = {
}
},
handleMaxSizeError: (_dbName, _response, _dbDetail) => {
return new Promise<boolean>(() => false);
return Promise.resolve(false);
},
};

Expand Down Expand Up @@ -138,12 +140,12 @@ const getDatabaseGrantsQuery: DatabaseDetailsQuery<DatabaseGrantsRow> = {
}
},
handleMaxSizeError: (_dbName, _response, _dbDetail) => {
return new Promise<boolean>(() => false);
return Promise.resolve(false);
},
};

// Database Tables
type DatabaseTablesResponse = {
export type DatabaseTablesResponse = {
tables: string[];
};

Expand Down Expand Up @@ -281,11 +283,11 @@ const getDatabaseZoneConfig: DatabaseDetailsQuery<DatabaseZoneConfigRow> = {
}
}
if (txn_result.error) {
resp.idResp.error = txn_result.error;
resp.zoneConfigResp.error = txn_result.error;
}
},
handleMaxSizeError: (_dbName, _response, _dbDetail) => {
return new Promise<boolean>(() => false);
return Promise.resolve(false);
},
};

Expand All @@ -296,7 +298,7 @@ type DatabaseDetailsStats = {
indexStats: SqlApiQueryResponse<DatabaseIndexUsageStatsResponse>;
};

type DatabaseSpanStatsRow = {
export type DatabaseSpanStatsRow = {
approximate_disk_bytes: number;
live_bytes: number;
total_bytes: number;
Expand Down Expand Up @@ -338,7 +340,7 @@ const getDatabaseSpanStats: DatabaseDetailsQuery<DatabaseSpanStatsRow> = {
}
},
handleMaxSizeError: (_dbName, _response, _dbDetail) => {
return new Promise<boolean>(() => false);
return Promise.resolve(false);
},
};

Expand Down Expand Up @@ -383,7 +385,7 @@ const getDatabaseReplicasAndRegions: DatabaseDetailsQuery<DatabaseReplicasRegion
}
},
handleMaxSizeError: (_dbName, _response, _dbDetail) => {
return new Promise<boolean>(() => false);
return Promise.resolve(false);
},
};

Expand Down Expand Up @@ -428,7 +430,7 @@ const getDatabaseIndexUsageStats: DatabaseDetailsQuery<IndexUsageStatistic> = {
}
},
handleMaxSizeError: (_dbName, _response, _dbDetail) => {
return new Promise<boolean>(() => false);
return Promise.resolve(false);
},
};

Expand Down Expand Up @@ -467,12 +469,15 @@ const databaseDetailQueries: DatabaseDetailsQuery<DatabaseDetailsRow>[] = [
export function createDatabaseDetailsReq(
params: DatabaseDetailsReqParams,
): SqlExecutionRequest {
return createSqlExecutionRequest(
params.database,
databaseDetailQueries.map(query =>
query.createStmt(params.database, params.csIndexUnusedDuration),
return {
...createSqlExecutionRequest(
params.database,
databaseDetailQueries.map(query =>
query.createStmt(params.database, params.csIndexUnusedDuration),
),
),
);
separate_txns: true,
};
}

export async function getDatabaseDetails(
Expand All @@ -488,30 +493,36 @@ async function fetchDatabaseDetails(
const detailsResponse: DatabaseDetailsResponse = newDatabaseDetailsResponse();
const req: SqlExecutionRequest = createDatabaseDetailsReq(params);
const resp = await executeInternalSql<DatabaseDetailsRow>(req);
const errs: Error[] = [];
resp.execution.txn_results.forEach(txn_result => {
if (txn_result.rows) {
const query: DatabaseDetailsQuery<DatabaseDetailsRow> =
databaseDetailQueries[txn_result.statement - 1];
query.addToDatabaseDetail(txn_result, detailsResponse);
if (txn_result.error) {
errs.push(txn_result.error);
}
const query: DatabaseDetailsQuery<DatabaseDetailsRow> =
databaseDetailQueries[txn_result.statement - 1];
query.addToDatabaseDetail(txn_result, detailsResponse);
});
if (resp.error) {
if (resp.error.message.includes("max result size exceeded")) {
if (isMaxSizeError(resp.error.message)) {
return fetchSeparatelyDatabaseDetails(params);
}
detailsResponse.error = resp.error;
}

detailsResponse.error = combineQueryErrors(errs, detailsResponse.error);
return formatApiResult<DatabaseDetailsResponse>(
detailsResponse,
detailsResponse.error,
"retrieving database details information",
`retrieving database details information for database '${params.database}'`,
false,
);
}

async function fetchSeparatelyDatabaseDetails(
params: DatabaseDetailsReqParams,
): Promise<SqlApiResponse<DatabaseDetailsResponse>> {
const detailsResponse: DatabaseDetailsResponse = newDatabaseDetailsResponse();
const errs: Error[] = [];
for (const databaseDetailQuery of databaseDetailQueries) {
const req = createSqlExecutionRequest(params.database, [
databaseDetailQuery.createStmt(
Expand All @@ -520,10 +531,14 @@ async function fetchSeparatelyDatabaseDetails(
),
]);
const resp = await executeInternalSql<DatabaseDetailsRow>(req);
if (sqlResultsAreEmpty(resp)) {
continue;
}
const txn_result = resp.execution.txn_results[0];
if (txn_result.rows) {
databaseDetailQuery.addToDatabaseDetail(txn_result, detailsResponse);
if (txn_result.error) {
errs.push(txn_result.error);
}
databaseDetailQuery.addToDatabaseDetail(txn_result, detailsResponse);

if (resp.error) {
const handleFailure = await databaseDetailQuery.handleMaxSizeError(
Expand All @@ -537,9 +552,11 @@ async function fetchSeparatelyDatabaseDetails(
}
}

detailsResponse.error = combineQueryErrors(errs, detailsResponse.error);
return formatApiResult<DatabaseDetailsResponse>(
detailsResponse,
detailsResponse.error,
"retrieving database details information",
`retrieving database details information for database '${params.database}'`,
false,
);
}
35 changes: 15 additions & 20 deletions pkg/ui/workspaces/cluster-ui/src/api/databasesApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,30 +42,25 @@ export const databasesRequest: SqlExecutionRequest = {
// getDatabasesList from cluster-ui will need to pass a timeout argument for
// promise timeout handling (callers from db-console already have promise
// timeout handling as part of the cacheDataReducer).
export function getDatabasesList(
export async function getDatabasesList(
timeout?: moment.Duration,
): Promise<DatabasesListResponse> {
return withTimeout(
const resp = await withTimeout(
executeInternalSql<DatabasesColumns>(databasesRequest),
timeout,
).then(result => {
// If there is an error and there are no result throw error.
const noTxnResultsExist = result?.execution?.txn_results?.length === 0;
if (
result.error &&
(noTxnResultsExist || result.execution.txn_results[0].rows.length === 0)
) {
throw result.error;
}
);
// Encountered a response level error, or empty response.
if (resp.error || sqlResultsAreEmpty(resp)) {
return { databases: [], error: resp.error };
}

if (sqlResultsAreEmpty(result)) {
return { databases: [], error: result.error };
}
// Get database names.
const dbNames: string[] = resp.execution.txn_results[0].rows.map(
row => row.database_name,
);

const dbNames: string[] = result.execution.txn_results[0].rows.map(
row => row.database_name,
);

return { databases: dbNames, error: result.error };
});
// Note that we do not surface the txn_result error in the returned payload.
// This request only contains a single txn_result, any error encountered by the txn_result
// will be surfaced at the response level by the sql api.
return { databases: dbNames, error: resp.error };
}
53 changes: 47 additions & 6 deletions pkg/ui/workspaces/cluster-ui/src/api/sqlApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
// licenses/APL.txt.

import { fetchDataJSON } from "./fetchData";
import { getLogger } from "../util";

export type SqlExecutionRequest = {
statements: SqlStatement[];
Expand All @@ -17,6 +18,7 @@ export type SqlExecutionRequest = {
application_name?: string; // Defaults to '$ api-v2-sql'
database?: string; // Defaults to system
max_result_size?: number; // Default 10kib
separate_txns?: boolean;
};

export type SqlStatement = {
Expand Down Expand Up @@ -165,7 +167,8 @@ export function sqlApiErrorMessage(message: string): string {

message = message.replace("run-query-via-api: ", "");
if (message.includes(":")) {
return message.split(":")[1];
const idx = message.indexOf(":") + 1;
return idx < message.length ? message.substring(idx) : message;
}

return message;
Expand All @@ -184,23 +187,43 @@ export function createSqlExecutionRequest(
};
}

export function isSeparateTxnError(message: string): boolean {
return !!message?.includes(
"separate transaction payload encountered transaction error",
);
}

export function isMaxSizeError(message: string): boolean {
return !!message?.includes("max result size exceeded");
}

export function isPrivilegeError(code: string): boolean {
return code === "42501";
}

export function formatApiResult<ResultType>(
results: ResultType,
error: SqlExecutionErrorMessage,
errorMessageContext: string,
shouldThrowOnQueryError = true,
): SqlApiResponse<ResultType> {
const maxSizeError = isMaxSizeError(error?.message);

if (error && !maxSizeError) {
throw new Error(
`Error while ${errorMessageContext}: ${sqlApiErrorMessage(
error?.message,
)}`,
);
if (shouldThrowOnQueryError) {
throw new Error(
`Error while ${errorMessageContext}: ${sqlApiErrorMessage(
error?.message,
)}`,
);
} else {
// Otherwise, just log.
getLogger().warn(
`Error while ${errorMessageContext}: ${sqlApiErrorMessage(
error?.message,
)}`,
);
}
}

return {
Expand All @@ -209,6 +232,24 @@ export function formatApiResult<ResultType>(
};
}

export function combineQueryErrors(
errs: Error[],
sqlError?: SqlExecutionErrorMessage,
): SqlExecutionErrorMessage {
if (errs.length === 0 && !sqlError) {
return;
}
const errMsgs = errs.map(err => `\n-` + sqlApiErrorMessage(err.message));
let sqlErrMsg = sqlError.message;
if (isSeparateTxnError(sqlErrMsg)) {
sqlErrMsg = "Encountered query error(s) fetching data:";
}
return {
...sqlError,
message: [sqlErrMsg, ...errMsgs].join(``),
};
}

export function txnResultIsEmpty(txn_result: SqlTxnResult<unknown>): boolean {
return !txn_result.rows || txn_result.rows?.length === 0;
}
17 changes: 4 additions & 13 deletions pkg/ui/workspaces/cluster-ui/src/databases/combiners.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { DatabasesListResponse, SqlExecutionErrorMessage } from "../api";
import { DatabasesPageDataDatabase } from "../databasesPage";
import {
buildIndexStatToRecommendationsMap,
combineLoadingErrors,
getNodesByRegionString,
normalizePrivileges,
normalizeRoles,
Expand Down Expand Up @@ -70,8 +69,6 @@ const deriveDatabaseDetails = (
isTenant: boolean,
): DatabasesPageDataDatabase => {
const dbStats = dbDetails?.data?.results.stats;
const sizeInBytes = dbStats?.spanStats?.approximate_disk_bytes || 0;
const rangeCount = dbStats?.spanStats.range_count || 0;
const nodes = dbStats?.replicaData.replicas || [];
const nodesByRegionString = getNodesByRegionString(
nodes,
Expand All @@ -81,20 +78,14 @@ const deriveDatabaseDetails = (
const numIndexRecommendations =
dbStats?.indexStats.num_index_recommendations || 0;

const combinedErr = combineLoadingErrors(
dbDetails?.lastError,
dbDetails?.data?.maxSizeReached,
dbListError?.message,
);

return {
loading: !!dbDetails?.inFlight,
loaded: !!dbDetails?.valid,
lastError: combinedErr,
requestError: dbDetails?.lastError,
queryError: dbDetails?.data?.results?.error,
name: database,
sizeInBytes: sizeInBytes,
tableCount: dbDetails?.data?.results.tablesResp.tables?.length || 0,
rangeCount: rangeCount,
spanStats: dbStats?.spanStats,
tables: dbDetails?.data?.results.tablesResp,
nodes: nodes,
nodesByRegionString,
numIndexRecommendations,
Expand Down
Loading