Skip to content

Commit 76d2753

Browse files
committed
fix: disable query chunking for total count hook in patterns table
1 parent 473ecb2 commit 76d2753

File tree

4 files changed

+87
-43
lines changed

4 files changed

+87
-43
lines changed

packages/app/src/components/PatternTable.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export default function PatternTable({
3232
const { totalCount, isLoading: isTotalCountLoading } = useSearchTotalCount(
3333
totalCountConfig,
3434
totalCountQueryKeyPrefix,
35+
true, // disable chunked queries for total count to avoid estimated counts flickering
3536
);
3637

3738
const {

packages/app/src/components/SearchTotalCountChart.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { useQueriedChartConfig } from '@/hooks/useChartConfig';
99
export function useSearchTotalCount(
1010
config: ChartConfigWithDateRange,
1111
queryKeyPrefix: string,
12+
disableQueryChunking: boolean = false,
1213
) {
1314
// copied from DBTimeChart
1415
const { granularity } = useTimeChartSettings(config);
@@ -22,10 +23,11 @@ export function useSearchTotalCount(
2223
isLoading,
2324
isError,
2425
} = useQueriedChartConfig(queriedConfig, {
25-
queryKey: [queryKeyPrefix, queriedConfig],
26+
queryKey: [queryKeyPrefix, queriedConfig, disableQueryChunking],
2627
staleTime: 1000 * 60 * 5,
2728
refetchOnWindowFocus: false,
2829
placeholderData: keepPreviousData, // no need to flash loading state when in live tail
30+
disableQueryChunking,
2931
});
3032

3133
const totalCount = useMemo(() => {

packages/app/src/hooks/__tests__/useChartConfig.test.tsx

Lines changed: 74 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import React from 'react';
22
import { ResponseJSON } from '@hyperdx/common-utils/dist/clickhouse';
33
import { ClickhouseClient } from '@hyperdx/common-utils/dist/clickhouse/browser';
44
import {
5+
ChartConfigWithDateRange,
56
ChartConfigWithOptDateRange,
67
MetricsDataType,
78
} from '@hyperdx/common-utils/dist/types';
@@ -73,33 +74,6 @@ const createMockQueryResponse = (data: any[]): ResponseJSON<any> => {
7374

7475
describe('useChartConfig', () => {
7576
describe('getGranularityAlignedTimeWindows', () => {
76-
it('returns [undefined] if no dateRange is provided', () => {
77-
expect(
78-
getGranularityAlignedTimeWindows({
79-
granularity: '1 hour',
80-
timestampValueExpression: 'TimestampTime',
81-
} as ChartConfigWithOptDateRange),
82-
).toEqual([undefined]);
83-
});
84-
85-
it('returns [undefined] if no granularity is provided', () => {
86-
expect(
87-
getGranularityAlignedTimeWindows({
88-
dateRange: [new Date('2023-01-01'), new Date('2023-01-02')],
89-
timestampValueExpression: 'TimestampTime',
90-
} as ChartConfigWithOptDateRange),
91-
).toEqual([undefined]);
92-
});
93-
94-
it('returns [undefined] if no timestampValueExpression is provided', () => {
95-
expect(
96-
getGranularityAlignedTimeWindows({
97-
dateRange: [new Date('2023-01-01'), new Date('2023-01-02')],
98-
granularity: '1 hour',
99-
} as ChartConfigWithOptDateRange),
100-
).toEqual([undefined]);
101-
});
102-
10377
it('returns windows aligned to the granularity if the granularity is auto', () => {
10478
expect(
10579
getGranularityAlignedTimeWindows(
@@ -110,7 +84,7 @@ describe('useChartConfig', () => {
11084
],
11185
granularity: 'auto', // will be 1 minute
11286
timestampValueExpression: 'TimestampTime',
113-
} as ChartConfigWithOptDateRange,
87+
} as ChartConfigWithDateRange & { granularity: string },
11488
[
11589
30, // 30s
11690
5 * 60, // 5m
@@ -152,7 +126,7 @@ describe('useChartConfig', () => {
152126
],
153127
granularity: '1 minute',
154128
timestampValueExpression: 'TimestampTime',
155-
} as ChartConfigWithOptDateRange,
129+
} as ChartConfigWithDateRange & { granularity: string },
156130
[
157131
30, // 30s
158132
60, // 1m
@@ -201,7 +175,7 @@ describe('useChartConfig', () => {
201175
],
202176
granularity: '1 minute',
203177
timestampValueExpression: 'TimestampTime',
204-
} as ChartConfigWithOptDateRange,
178+
} as ChartConfigWithDateRange & { granularity: string },
205179
[
206180
15, // 15s
207181
],
@@ -235,7 +209,7 @@ describe('useChartConfig', () => {
235209
granularity: '1 minute',
236210
timestampValueExpression: 'TimestampTime',
237211
dateRangeEndInclusive: true,
238-
} as ChartConfigWithOptDateRange,
212+
} as ChartConfigWithDateRange & { granularity: string },
239213
[
240214
15 * 60, // 15m
241215
30 * 60, // 30m
@@ -290,7 +264,7 @@ describe('useChartConfig', () => {
290264
],
291265
granularity: '1 minute',
292266
timestampValueExpression: 'TimestampTime',
293-
} as ChartConfigWithOptDateRange,
267+
} as ChartConfigWithDateRange & { granularity: string },
294268
[
295269
60, // 1m
296270
],
@@ -885,5 +859,73 @@ describe('useChartConfig', () => {
885859
expect(result.current.isPending).toBe(true);
886860
expect(result.current.data).toBeUndefined();
887861
});
862+
863+
it('uses different query keys for the same config when one sets disableQueryChunking', async () => {
864+
const config = createMockChartConfig({
865+
dateRange: [
866+
new Date('2025-10-01 00:00:00Z'),
867+
new Date('2025-10-02 00:00:00Z'),
868+
],
869+
granularity: '3 hour',
870+
});
871+
872+
const mockResponseChunked = createMockQueryResponse([
873+
{
874+
'count()': '50',
875+
__hdx_time_bucket: '2025-10-01T18:00:00Z',
876+
},
877+
]);
878+
879+
const mockResponseNonChunked = createMockQueryResponse([
880+
{
881+
'count()': '100',
882+
__hdx_time_bucket: '2025-10-01T12:00:00Z',
883+
},
884+
]);
885+
886+
mockClickhouseClient.queryChartConfig.mockResolvedValue(
887+
mockResponseChunked,
888+
);
889+
890+
const { result: result1 } = renderHook(
891+
() => useQueriedChartConfig(config),
892+
{
893+
wrapper,
894+
},
895+
);
896+
897+
await waitFor(() => expect(result1.current.isSuccess).toBe(true));
898+
await waitFor(() => expect(result1.current.isFetching).toBe(false));
899+
900+
// Should have been called multiple times for chunked query
901+
const chunkedCallCount =
902+
mockClickhouseClient.queryChartConfig.mock.calls.length;
903+
expect(chunkedCallCount).toBeGreaterThan(1);
904+
expect(result1.current.data?.rows).toBeGreaterThan(1);
905+
906+
// Second render with same config but disableQueryChunking=true
907+
mockClickhouseClient.queryChartConfig.mockResolvedValue(
908+
mockResponseNonChunked,
909+
);
910+
911+
const { result: result2 } = renderHook(
912+
() => useQueriedChartConfig(config, { disableQueryChunking: true }),
913+
{
914+
wrapper,
915+
},
916+
);
917+
918+
await waitFor(() => expect(result2.current.isSuccess).toBe(true));
919+
await waitFor(() => expect(result2.current.isFetching).toBe(false));
920+
921+
// Should have made a new request (not using cached chunked data)
922+
expect(mockClickhouseClient.queryChartConfig).toHaveBeenCalledTimes(
923+
chunkedCallCount + 1,
924+
);
925+
expect(result2.current.data?.rows).toBe(1);
926+
927+
// The original query should still have its chunked data
928+
expect(result1.current.data?.rows).toBeGreaterThan(1);
929+
});
888930
});
889931
});

packages/app/src/hooks/useChartConfig.tsx

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,17 +65,13 @@ const shouldUseChunking = (
6565
// complex metric queries.
6666
if (isMetricChartConfig(config)) return false;
6767

68-
console.log('Chunking enabled for chart config:', config);
69-
7068
return true;
7169
};
7270

7371
export const getGranularityAlignedTimeWindows = (
74-
config: ChartConfigWithOptDateRange,
72+
config: ChartConfigWithDateRange & { granularity: string },
7573
windowDurationsSeconds?: number[],
76-
): TimeWindow[] | [undefined] => {
77-
if (!shouldUseChunking(config)) return [undefined];
78-
74+
): TimeWindow[] => {
7975
const [startDate, endDate] = config.dateRange;
8076
const windowsUnaligned = generateTimeWindowsDescending(
8177
startDate,
@@ -124,9 +120,10 @@ async function* fetchDataInChunks(
124120
signal: AbortSignal,
125121
disableQueryChunking: boolean = false,
126122
) {
127-
const windows = disableQueryChunking
128-
? ([undefined] as const)
129-
: getGranularityAlignedTimeWindows(config);
123+
const windows =
124+
!disableQueryChunking && shouldUseChunking(config)
125+
? getGranularityAlignedTimeWindows(config)
126+
: ([undefined] as const);
130127

131128
let query = null;
132129
if (IS_MTVIEWS_ENABLED) {
@@ -179,7 +176,9 @@ export function useQueriedChartConfig(
179176
const clickhouseClient = useClickhouseClient();
180177

181178
const query = useQuery<TQueryFnData, ClickHouseQueryError | Error>({
182-
queryKey: [config],
179+
// Include disableQueryChunking in the query key to ensure that queries with the
180+
// same config but different disableQueryChunking values do not share a query
181+
queryKey: [config, options?.disableQueryChunking ?? false],
183182
queryFn: streamedQuery({
184183
streamFn: context =>
185184
fetchDataInChunks(

0 commit comments

Comments
 (0)