mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
perf: cap count queries and emit native UUID comparisons for audit/connection logs (#23835)
Audit and connection log pages were timing out due to expensive COUNT(*) queries over large tables. This commit adds opt-in count capping: requests can return a `count_cap` field signaling that the count was truncated at a threshold, avoiding full table scans that caused page timeouts. Text-cast UUID comparisons in regosql-generated authorization queries also contributed to the slowdown by preventing index usage for connection and audit log queries. These now emit native UUID operators. Frontend changes handle the capped state in usePaginatedQuery and PaginationWidget, optionally displaying a capped count in the pagination UI (e.g. "Showing 2,076 to 2,100 of 2,000+ logs") Related to: https://linear.app/codercom/issue/PLAT-31/connectionaudit-log-performance-issue
This commit is contained in:
Generated
+2
@@ -913,6 +913,7 @@ export interface AuditLog {
|
||||
export interface AuditLogResponse {
|
||||
readonly audit_logs: readonly AuditLog[];
|
||||
readonly count: number;
|
||||
readonly count_cap: number;
|
||||
}
|
||||
|
||||
// From codersdk/audit.go
|
||||
@@ -2269,6 +2270,7 @@ export interface ConnectionLog {
|
||||
export interface ConnectionLogResponse {
|
||||
readonly connection_logs: readonly ConnectionLog[];
|
||||
readonly count: number;
|
||||
readonly count_cap: number;
|
||||
}
|
||||
|
||||
// From codersdk/connectionlog.go
|
||||
|
||||
@@ -7,6 +7,7 @@ type PaginationHeaderProps = {
|
||||
limit: number;
|
||||
totalRecords: number | undefined;
|
||||
currentOffsetStart: number | undefined;
|
||||
countIsCapped?: boolean;
|
||||
|
||||
// Temporary escape hatch until Workspaces can be switched over to using
|
||||
// PaginationContainer
|
||||
@@ -18,6 +19,7 @@ export const PaginationAmount: FC<PaginationHeaderProps> = ({
|
||||
limit,
|
||||
totalRecords,
|
||||
currentOffsetStart,
|
||||
countIsCapped,
|
||||
className,
|
||||
}) => {
|
||||
const theme = useTheme();
|
||||
@@ -52,10 +54,16 @@ export const PaginationAmount: FC<PaginationHeaderProps> = ({
|
||||
<strong>
|
||||
{(
|
||||
currentOffsetStart +
|
||||
Math.min(limit - 1, totalRecords - currentOffsetStart)
|
||||
(countIsCapped
|
||||
? limit - 1
|
||||
: Math.min(limit - 1, totalRecords - currentOffsetStart))
|
||||
).toLocaleString()}
|
||||
</strong>{" "}
|
||||
of <strong>{totalRecords.toLocaleString()}</strong>{" "}
|
||||
of{" "}
|
||||
<strong>
|
||||
{totalRecords.toLocaleString()}
|
||||
{countIsCapped && "+"}
|
||||
</strong>{" "}
|
||||
{paginationUnitLabel}
|
||||
</div>
|
||||
)}
|
||||
|
||||
@@ -18,6 +18,7 @@ export const mockPaginationResultBase: ResultBase = {
|
||||
limit: 25,
|
||||
hasNextPage: false,
|
||||
hasPreviousPage: false,
|
||||
countIsCapped: false,
|
||||
goToPreviousPage: () => {},
|
||||
goToNextPage: () => {},
|
||||
goToFirstPage: () => {},
|
||||
@@ -33,6 +34,7 @@ export const mockInitialRenderResult: PaginationResult = {
|
||||
hasPreviousPage: false,
|
||||
totalRecords: undefined,
|
||||
totalPages: undefined,
|
||||
countIsCapped: false,
|
||||
};
|
||||
|
||||
export const mockSuccessResult: PaginationResult = {
|
||||
|
||||
@@ -94,7 +94,7 @@ export const FirstPageWithTonsOfData: Story = {
|
||||
currentPage: 2,
|
||||
currentOffsetStart: 1000,
|
||||
totalRecords: 123_456,
|
||||
totalPages: 1235,
|
||||
totalPages: 4939,
|
||||
hasPreviousPage: false,
|
||||
hasNextPage: true,
|
||||
isPlaceholderData: false,
|
||||
@@ -135,3 +135,54 @@ export const SecondPageWithData: Story = {
|
||||
children: <div>New data for page 2</div>,
|
||||
},
|
||||
};
|
||||
|
||||
export const CappedCountFirstPage: Story = {
|
||||
args: {
|
||||
query: {
|
||||
...mockPaginationResultBase,
|
||||
isSuccess: true,
|
||||
currentPage: 1,
|
||||
currentOffsetStart: 1,
|
||||
totalRecords: 2000,
|
||||
totalPages: 80,
|
||||
hasPreviousPage: false,
|
||||
hasNextPage: true,
|
||||
isPlaceholderData: false,
|
||||
countIsCapped: true,
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
export const CappedCountMiddlePage: Story = {
|
||||
args: {
|
||||
query: {
|
||||
...mockPaginationResultBase,
|
||||
isSuccess: true,
|
||||
currentPage: 3,
|
||||
currentOffsetStart: 51,
|
||||
totalRecords: 2000,
|
||||
totalPages: 80,
|
||||
hasPreviousPage: true,
|
||||
hasNextPage: true,
|
||||
isPlaceholderData: false,
|
||||
countIsCapped: true,
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
export const CappedCountBeyondKnownPages: Story = {
|
||||
args: {
|
||||
query: {
|
||||
...mockPaginationResultBase,
|
||||
isSuccess: true,
|
||||
currentPage: 85,
|
||||
currentOffsetStart: 2101,
|
||||
totalRecords: 2000,
|
||||
totalPages: 85,
|
||||
hasPreviousPage: true,
|
||||
hasNextPage: true,
|
||||
isPlaceholderData: false,
|
||||
countIsCapped: true,
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
@@ -27,12 +27,14 @@ export const PaginationContainer: FC<PaginationProps> = ({
|
||||
totalRecords={query.totalRecords}
|
||||
currentOffsetStart={query.currentOffsetStart}
|
||||
paginationUnitLabel={paginationUnitLabel}
|
||||
countIsCapped={query.countIsCapped}
|
||||
className="justify-end"
|
||||
/>
|
||||
|
||||
{query.isSuccess && (
|
||||
<PaginationWidgetBase
|
||||
totalRecords={query.totalRecords}
|
||||
totalPages={query.totalPages}
|
||||
currentPage={query.currentPage}
|
||||
pageSize={query.limit}
|
||||
onPageChange={query.onPageChange}
|
||||
|
||||
@@ -12,6 +12,10 @@ export type PaginationWidgetBaseProps = {
|
||||
|
||||
hasPreviousPage?: boolean;
|
||||
hasNextPage?: boolean;
|
||||
/** Override the computed totalPages.
|
||||
* Used when, e.g., the row count is capped and the user navigates beyond
|
||||
* the known range, so totalPages stays at least as high as currentPage. */
|
||||
totalPages?: number;
|
||||
};
|
||||
|
||||
export const PaginationWidgetBase: FC<PaginationWidgetBaseProps> = ({
|
||||
@@ -21,8 +25,9 @@ export const PaginationWidgetBase: FC<PaginationWidgetBaseProps> = ({
|
||||
onPageChange,
|
||||
hasPreviousPage,
|
||||
hasNextPage,
|
||||
totalPages: totalPagesProp,
|
||||
}) => {
|
||||
const totalPages = Math.ceil(totalRecords / pageSize);
|
||||
const totalPages = totalPagesProp ?? Math.ceil(totalRecords / pageSize);
|
||||
|
||||
if (totalPages < 2) {
|
||||
return null;
|
||||
|
||||
@@ -258,6 +258,78 @@ describe(usePaginatedQuery.name, () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("Capped count behavior", () => {
|
||||
const mockQueryKey = vi.fn(() => ["mock"]);
|
||||
|
||||
// Returns count 2001 (capped) with items on pages up to page 84
|
||||
// (84 * 25 = 2100 items total).
|
||||
const mockCappedQueryFn = vi.fn(({ pageNumber, limit }) => {
|
||||
const totalItems = 2100;
|
||||
const offset = (pageNumber - 1) * limit;
|
||||
// Returns 0 items when the requested page is past the end, simulating
|
||||
// an empty server response.
|
||||
const itemsOnPage = Math.max(0, Math.min(limit, totalItems - offset));
|
||||
return Promise.resolve({
|
||||
data: new Array(itemsOnPage).fill(pageNumber),
|
||||
count: 2001,
|
||||
count_cap: 2000,
|
||||
});
|
||||
});
|
||||
|
||||
it("Caps totalRecords at 2000 when count exceeds cap", async () => {
|
||||
const { result } = await render({
|
||||
queryKey: mockQueryKey,
|
||||
queryFn: mockCappedQueryFn,
|
||||
});
|
||||
|
||||
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
||||
expect(result.current.totalRecords).toBe(2000);
|
||||
});
|
||||
|
||||
it("hasNextPage is true when count is capped", async () => {
|
||||
const { result } = await render(
|
||||
{ queryKey: mockQueryKey, queryFn: mockCappedQueryFn },
|
||||
"/?page=80",
|
||||
);
|
||||
|
||||
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
||||
expect(result.current.hasNextPage).toBe(true);
|
||||
});
|
||||
|
||||
it("hasPreviousPage is true when count is capped and page is beyond cap", async () => {
|
||||
const { result } = await render(
|
||||
{ queryKey: mockQueryKey, queryFn: mockCappedQueryFn },
|
||||
"/?page=83",
|
||||
);
|
||||
|
||||
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
||||
expect(result.current.hasPreviousPage).toBe(true);
|
||||
});
|
||||
|
||||
it("Does not redirect to last page when count is capped and page is valid", async () => {
|
||||
const { result } = await render(
|
||||
{ queryKey: mockQueryKey, queryFn: mockCappedQueryFn },
|
||||
"/?page=83",
|
||||
);
|
||||
|
||||
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
||||
// Should stay on page 83 — not redirect to page 80.
|
||||
expect(result.current.currentPage).toBe(83);
|
||||
});
|
||||
|
||||
it("Redirects to last known page when navigating beyond actual data", async () => {
|
||||
const { result } = await render(
|
||||
{ queryKey: mockQueryKey, queryFn: mockCappedQueryFn },
|
||||
"/?page=999",
|
||||
);
|
||||
|
||||
// Page 999 has no items. Should redirect to page 81
|
||||
// (ceil(2001 / 25) = 81), the last page guaranteed to
|
||||
// have data.
|
||||
await waitFor(() => expect(result.current.currentPage).toBe(81));
|
||||
});
|
||||
});
|
||||
|
||||
describe("Passing in searchParams property", () => {
|
||||
const mockQueryKey = vi.fn(() => ["mock"]);
|
||||
const mockQueryFn = vi.fn(({ pageNumber, limit }) =>
|
||||
|
||||
@@ -144,16 +144,44 @@ export function usePaginatedQuery<
|
||||
placeholderData: keepPreviousData,
|
||||
});
|
||||
|
||||
const totalRecords = query.data?.count;
|
||||
const totalPages =
|
||||
totalRecords !== undefined ? Math.ceil(totalRecords / limit) : undefined;
|
||||
const count = query.data?.count;
|
||||
const countCap = query.data?.count_cap;
|
||||
const countIsCapped =
|
||||
countCap !== undefined &&
|
||||
countCap > 0 &&
|
||||
count !== undefined &&
|
||||
count > countCap;
|
||||
const totalRecords = countIsCapped ? countCap : count;
|
||||
let totalPages =
|
||||
totalRecords !== undefined
|
||||
? Math.max(
|
||||
Math.ceil(totalRecords / limit),
|
||||
// True count is not known; let them navigate forward
|
||||
// until they hit an empty page (checked below).
|
||||
countIsCapped ? currentPage : 0,
|
||||
)
|
||||
: undefined;
|
||||
|
||||
// When the true count is unknown, the user can navigate past
|
||||
// all actual data. If that happens, we need to redirect (via
|
||||
// updatePageIfInvalid) to the last page guaranteed to be not
|
||||
// empty.
|
||||
const pageIsEmpty =
|
||||
query.data != null &&
|
||||
!Object.values(query.data).some((v) => Array.isArray(v) && v.length > 0);
|
||||
if (pageIsEmpty) {
|
||||
totalPages = count !== undefined ? Math.ceil(count / limit) : 1;
|
||||
}
|
||||
|
||||
const hasNextPage =
|
||||
totalRecords !== undefined && limit + currentPageOffset < totalRecords;
|
||||
totalRecords !== undefined &&
|
||||
((countIsCapped && !pageIsEmpty) ||
|
||||
limit + currentPageOffset < totalRecords);
|
||||
const hasPreviousPage =
|
||||
totalRecords !== undefined &&
|
||||
currentPage > 1 &&
|
||||
currentPageOffset - limit < totalRecords;
|
||||
((countIsCapped && !pageIsEmpty) ||
|
||||
currentPageOffset - limit < totalRecords);
|
||||
|
||||
const queryClient = useQueryClient();
|
||||
const prefetchPage = useEffectEvent((newPage: number) => {
|
||||
@@ -224,10 +252,14 @@ export function usePaginatedQuery<
|
||||
});
|
||||
|
||||
useEffect(() => {
|
||||
if (!query.isFetching && totalPages !== undefined) {
|
||||
if (
|
||||
!query.isFetching &&
|
||||
totalPages !== undefined &&
|
||||
currentPage > totalPages
|
||||
) {
|
||||
void updatePageIfInvalid(totalPages);
|
||||
}
|
||||
}, [updatePageIfInvalid, query.isFetching, totalPages]);
|
||||
}, [updatePageIfInvalid, query.isFetching, totalPages, currentPage]);
|
||||
|
||||
const onPageChange = (newPage: number) => {
|
||||
// Page 1 is the only page that can be safely navigated to without knowing
|
||||
@@ -236,7 +268,12 @@ export function usePaginatedQuery<
|
||||
return;
|
||||
}
|
||||
|
||||
const cleanedInput = clamp(Math.trunc(newPage), 1, totalPages ?? 1);
|
||||
// If the true count is unknown, we allow navigating past the
|
||||
// known page range.
|
||||
const upperBound = countIsCapped
|
||||
? Number.MAX_SAFE_INTEGER
|
||||
: (totalPages ?? 1);
|
||||
const cleanedInput = clamp(Math.trunc(newPage), 1, upperBound);
|
||||
if (Number.isNaN(cleanedInput)) {
|
||||
return;
|
||||
}
|
||||
@@ -274,6 +311,7 @@ export function usePaginatedQuery<
|
||||
totalRecords: totalRecords as number,
|
||||
totalPages: totalPages as number,
|
||||
currentOffsetStart: currentPageOffset + 1,
|
||||
countIsCapped,
|
||||
}
|
||||
: {
|
||||
isSuccess: false,
|
||||
@@ -282,6 +320,7 @@ export function usePaginatedQuery<
|
||||
totalRecords: undefined,
|
||||
totalPages: undefined,
|
||||
currentOffsetStart: undefined,
|
||||
countIsCapped: false as const,
|
||||
}),
|
||||
};
|
||||
|
||||
@@ -323,6 +362,7 @@ export type PaginationResultInfo = {
|
||||
totalRecords: undefined;
|
||||
totalPages: undefined;
|
||||
currentOffsetStart: undefined;
|
||||
countIsCapped: false;
|
||||
}
|
||||
| {
|
||||
isSuccess: true;
|
||||
@@ -331,6 +371,7 @@ export type PaginationResultInfo = {
|
||||
totalRecords: number;
|
||||
totalPages: number;
|
||||
currentOffsetStart: number;
|
||||
countIsCapped: boolean;
|
||||
}
|
||||
);
|
||||
|
||||
@@ -417,6 +458,7 @@ type QueryPageParamsWithPayload<TPayload = never> = QueryPageParams & {
|
||||
*/
|
||||
export type PaginatedData = {
|
||||
count: number;
|
||||
count_cap?: number;
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -71,6 +71,7 @@ describe("AuditPage", () => {
|
||||
const getAuditLogsSpy = vi.spyOn(API, "getAuditLogs").mockResolvedValue({
|
||||
audit_logs: [MockAuditLog, MockAuditLog2],
|
||||
count: 2,
|
||||
count_cap: 0,
|
||||
});
|
||||
|
||||
// When
|
||||
@@ -90,6 +91,7 @@ describe("AuditPage", () => {
|
||||
vi.spyOn(API, "getAuditLogs").mockResolvedValue({
|
||||
audit_logs: [MockAuditLog],
|
||||
count: 1,
|
||||
count_cap: 0,
|
||||
});
|
||||
|
||||
await renderPage();
|
||||
@@ -114,6 +116,7 @@ describe("AuditPage", () => {
|
||||
vi.spyOn(API, "getAuditLogs").mockResolvedValue({
|
||||
audit_logs: [MockAuditLog],
|
||||
count: 1,
|
||||
count_cap: 0,
|
||||
});
|
||||
|
||||
await renderPage();
|
||||
@@ -140,9 +143,11 @@ describe("AuditPage", () => {
|
||||
|
||||
describe("Filtering", () => {
|
||||
it("filters by URL", async () => {
|
||||
const getAuditLogsSpy = vi
|
||||
.spyOn(API, "getAuditLogs")
|
||||
.mockResolvedValue({ audit_logs: [MockAuditLog], count: 1 });
|
||||
const getAuditLogsSpy = vi.spyOn(API, "getAuditLogs").mockResolvedValue({
|
||||
audit_logs: [MockAuditLog],
|
||||
count: 1,
|
||||
count_cap: 0,
|
||||
});
|
||||
|
||||
const query = "resource_type:workspace action:create";
|
||||
await renderPage({ filter: query });
|
||||
@@ -173,4 +178,29 @@ describe("AuditPage", () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("Capped count", () => {
|
||||
it("shows capped count indicator and navigates to next page with correct offset", async () => {
|
||||
vi.spyOn(API, "getAuditLogs").mockResolvedValue({
|
||||
audit_logs: [MockAuditLog, MockAuditLog2],
|
||||
count: 2001,
|
||||
count_cap: 2000,
|
||||
});
|
||||
|
||||
const user = userEvent.setup();
|
||||
await renderPage();
|
||||
|
||||
await screen.findByText(/2,000\+/);
|
||||
|
||||
await user.click(screen.getByRole("button", { name: /next page/i }));
|
||||
|
||||
await waitFor(() =>
|
||||
expect(API.getAuditLogs).toHaveBeenLastCalledWith<[AuditLogsRequest]>({
|
||||
limit: DEFAULT_RECORDS_PER_PAGE,
|
||||
offset: DEFAULT_RECORDS_PER_PAGE,
|
||||
q: "",
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -69,6 +69,7 @@ describe("ConnectionLogPage", () => {
|
||||
MockDisconnectedSSHConnectionLog,
|
||||
],
|
||||
count: 2,
|
||||
count_cap: 0,
|
||||
});
|
||||
|
||||
// When
|
||||
@@ -95,6 +96,7 @@ describe("ConnectionLogPage", () => {
|
||||
.mockResolvedValue({
|
||||
connection_logs: [MockConnectedSSHConnectionLog],
|
||||
count: 1,
|
||||
count_cap: 0,
|
||||
});
|
||||
|
||||
const query = "type:ssh status:ongoing";
|
||||
|
||||
Reference in New Issue
Block a user