Skip to content

Commit 4d67322

Browse files
pulpdrewpulpdrew
committed
feat: Disable chart chunking by default
1 parent 6ee53de commit 4d67322

File tree

5 files changed

+146
-56
lines changed

5 files changed

+146
-56
lines changed

‎packages/app/src/ServicesDashboardPage.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,9 @@ function ServiceSelectControlled({
120120

121121
const { data, isLoading, isError } = useQueriedChartConfig(queriedConfig, {
122122
placeholderData: (prev: any) => prev,
123-
queryKey: ['service-select', queriedConfig],
123+
queryKey: ['service-select', queriedConfig, 'chunked'],
124124
enabled: !!source,
125+
enableQueryChunking: true,
125126
});
126127

127128
const values = useMemo(() => {

‎packages/app/src/components/DBTimeChart.tsx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,12 @@ function DBTimeChartComponent({
6262
limit: { limit: 100000 },
6363
};
6464

65-
const { data, isLoading, isError, error, isSuccess, isFetching } =
65+
const { data, isLoading, isError, error, isPlaceholderData, isSuccess } =
6666
useQueriedChartConfig(queriedConfig, {
6767
placeholderData: (prev: any) => prev,
68-
queryKey: [queryKeyPrefix, queriedConfig],
68+
queryKey: [queryKeyPrefix, queriedConfig, 'chunked'],
6969
enabled,
70+
enableQueryChunking: true,
7071
});
7172

7273
useEffect(() => {
@@ -75,6 +76,8 @@ function DBTimeChartComponent({
7576
}
7677
}, [isError, isErrorExpanded, errorExpansion]);
7778

79+
const isLoadingOrPlaceholder =
80+
isLoading || !data?.isComplete || isPlaceholderData;
7881
const { data: source } = useSource({ id: sourceId });
7982

8083
const { graphResults, timestampColumn, groupKeys, lineNames, lineColors } =
@@ -337,7 +340,7 @@ function DBTimeChartComponent({
337340
graphResults={graphResults}
338341
groupKeys={groupKeys}
339342
isClickActive={false}
340-
isLoading={isFetching}
343+
isLoading={isLoadingOrPlaceholder}
341344
lineColors={lineColors}
342345
lineNames={lineNames}
343346
logReferenceTimestamp={logReferenceTimestamp}

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

Lines changed: 113 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -364,9 +364,12 @@ describe('useChartConfig', () => {
364364

365365
mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse);
366366

367-
const { result } = renderHook(() => useQueriedChartConfig(config), {
368-
wrapper,
369-
});
367+
const { result } = renderHook(
368+
() => useQueriedChartConfig(config, { enableQueryChunking: true }),
369+
{
370+
wrapper,
371+
},
372+
);
370373

371374
await waitFor(() => expect(result.current.isSuccess).toBe(true));
372375
await waitFor(() => expect(result.current.isFetching).toBe(false));
@@ -410,9 +413,12 @@ describe('useChartConfig', () => {
410413

411414
mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse);
412415

413-
const { result } = renderHook(() => useQueriedChartConfig(config), {
414-
wrapper,
415-
});
416+
const { result } = renderHook(
417+
() => useQueriedChartConfig(config, { enableQueryChunking: true }),
418+
{
419+
wrapper,
420+
},
421+
);
416422

417423
await waitFor(() => expect(result.current.isSuccess).toBe(true));
418424
await waitFor(() => expect(result.current.isFetching).toBe(false));
@@ -460,9 +466,12 @@ describe('useChartConfig', () => {
460466

461467
mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse);
462468

463-
const { result } = renderHook(() => useQueriedChartConfig(config), {
464-
wrapper,
465-
});
469+
const { result } = renderHook(
470+
() => useQueriedChartConfig(config, { enableQueryChunking: true }),
471+
{
472+
wrapper,
473+
},
474+
);
466475

467476
await waitFor(() => expect(result.current.isSuccess).toBe(true));
468477
await waitFor(() => expect(result.current.isFetching).toBe(false));
@@ -536,9 +545,12 @@ describe('useChartConfig', () => {
536545

537546
mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse);
538547

539-
const { result } = renderHook(() => useQueriedChartConfig(config), {
540-
wrapper,
541-
});
548+
const { result } = renderHook(
549+
() => useQueriedChartConfig(config, { enableQueryChunking: true }),
550+
{
551+
wrapper,
552+
},
553+
);
542554

543555
await waitFor(() => expect(result.current.isSuccess).toBe(true));
544556
await waitFor(() => expect(result.current.isFetching).toBe(false));
@@ -560,7 +572,7 @@ describe('useChartConfig', () => {
560572
});
561573
});
562574

563-
it('fetches data without chunking when disableQueryChunking is true', async () => {
575+
it('fetches data without chunking when enableQueryChunking is false', async () => {
564576
const config = createMockChartConfig({
565577
dateRange: [
566578
new Date('2025-10-01 00:00:00Z'),
@@ -585,7 +597,7 @@ describe('useChartConfig', () => {
585597
mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse);
586598

587599
const { result } = renderHook(
588-
() => useQueriedChartConfig(config, { disableQueryChunking: true }),
600+
() => useQueriedChartConfig(config, { enableQueryChunking: false }),
589601
{
590602
wrapper,
591603
},
@@ -611,6 +623,54 @@ describe('useChartConfig', () => {
611623
});
612624
});
613625

626+
it('fetches data without chunking when enableQueryChunking is not provided', async () => {
627+
const config = createMockChartConfig({
628+
dateRange: [
629+
new Date('2025-10-01 00:00:00Z'),
630+
new Date('2025-10-02 00:00:00Z'),
631+
],
632+
granularity: '1 hour',
633+
});
634+
635+
const mockResponse = createMockQueryResponse([
636+
{
637+
'count()': '71',
638+
SeverityText: 'info',
639+
__hdx_time_bucket: '2025-10-01T00:00:00Z',
640+
},
641+
{
642+
'count()': '73',
643+
SeverityText: 'info',
644+
__hdx_time_bucket: '2025-10-02T00:00:00Z',
645+
},
646+
]);
647+
648+
mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse);
649+
650+
const { result } = renderHook(() => useQueriedChartConfig(config), {
651+
wrapper,
652+
});
653+
654+
await waitFor(() => expect(result.current.isSuccess).toBe(true));
655+
await waitFor(() => expect(result.current.isFetching).toBe(false));
656+
657+
// Should only be called once since chunking is explicitly disabled
658+
expect(mockClickhouseClient.queryChartConfig).toHaveBeenCalledTimes(1);
659+
expect(mockClickhouseClient.queryChartConfig).toHaveBeenCalledWith({
660+
config,
661+
metadata: expect.any(Object),
662+
opts: {
663+
abort_signal: expect.any(AbortSignal),
664+
},
665+
});
666+
expect(result.current.data).toEqual({
667+
data: mockResponse.data,
668+
meta: mockResponse.meta,
669+
rows: mockResponse.rows,
670+
isComplete: true,
671+
});
672+
});
673+
614674
it('fetches data with chunking when granularity and date range are provided', async () => {
615675
const config = createMockChartConfig({
616676
dateRange: [
@@ -654,9 +714,12 @@ describe('useChartConfig', () => {
654714
.mockResolvedValueOnce(mockResponse2)
655715
.mockResolvedValueOnce(mockResponse3);
656716

657-
const { result } = renderHook(() => useQueriedChartConfig(config), {
658-
wrapper,
659-
});
717+
const { result } = renderHook(
718+
() => useQueriedChartConfig(config, { enableQueryChunking: true }),
719+
{
720+
wrapper,
721+
},
722+
);
660723

661724
await waitFor(() => expect(result.current.isSuccess).toBe(true));
662725
await waitFor(() => expect(result.current.isFetching).toBe(false));
@@ -746,9 +809,12 @@ describe('useChartConfig', () => {
746809
.mockResolvedValueOnce(mockResponse2)
747810
.mockResolvedValueOnce(mockResponse3);
748811

749-
const { result } = renderHook(() => useQueriedChartConfig(config), {
750-
wrapper,
751-
});
812+
const { result } = renderHook(
813+
() => useQueriedChartConfig(config, { enableQueryChunking: true }),
814+
{
815+
wrapper,
816+
},
817+
);
752818

753819
await waitFor(() => expect(result.current.isSuccess).toBe(true));
754820
await waitFor(() => expect(result.current.isPending).toBe(false));
@@ -806,9 +872,12 @@ describe('useChartConfig', () => {
806872
mockResponse1Promise,
807873
);
808874

809-
const { result } = renderHook(() => useQueriedChartConfig(config), {
810-
wrapper,
811-
});
875+
const { result } = renderHook(
876+
() => useQueriedChartConfig(config, { enableQueryChunking: true }),
877+
{
878+
wrapper,
879+
},
880+
);
812881

813882
// Should be in loading state before first chunk
814883
expect(result.current.isLoading).toBe(true);
@@ -850,7 +919,12 @@ describe('useChartConfig', () => {
850919
});
851920

852921
const { result } = renderHook(
853-
() => useQueriedChartConfig(config, { onError, retry: false }),
922+
() =>
923+
useQueriedChartConfig(config, {
924+
onError,
925+
retry: false,
926+
enableQueryChunking: true,
927+
}),
854928
{
855929
wrapper,
856930
},
@@ -881,7 +955,11 @@ describe('useChartConfig', () => {
881955
mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse);
882956

883957
const { result } = renderHook(
884-
() => useQueriedChartConfig(config, { enabled: false }),
958+
() =>
959+
useQueriedChartConfig(config, {
960+
enabled: false,
961+
enableQueryChunking: true,
962+
}),
885963
{
886964
wrapper,
887965
},
@@ -895,7 +973,7 @@ describe('useChartConfig', () => {
895973
expect(result.current.data).toBeUndefined();
896974
});
897975

898-
it('uses different query keys for the same config when one sets disableQueryChunking', async () => {
976+
it('uses different query keys for the same config when one sets enableQueryChunking', async () => {
899977
const config = createMockChartConfig({
900978
dateRange: [
901979
new Date('2025-10-01 00:00:00Z'),
@@ -923,7 +1001,7 @@ describe('useChartConfig', () => {
9231001
);
9241002

9251003
const { result: result1 } = renderHook(
926-
() => useQueriedChartConfig(config),
1004+
() => useQueriedChartConfig(config, { enableQueryChunking: true }),
9271005
{
9281006
wrapper,
9291007
},
@@ -938,13 +1016,13 @@ describe('useChartConfig', () => {
9381016
expect(chunkedCallCount).toBeGreaterThan(1);
9391017
expect(result1.current.data?.rows).toBeGreaterThan(1);
9401018

941-
// Second render with same config but disableQueryChunking=true
1019+
// Second render with same config but without query chunking enabled
9421020
mockClickhouseClient.queryChartConfig.mockResolvedValue(
9431021
mockResponseNonChunked,
9441022
);
9451023

9461024
const { result: result2 } = renderHook(
947-
() => useQueriedChartConfig(config, { disableQueryChunking: true }),
1025+
() => useQueriedChartConfig(config),
9481026
{
9491027
wrapper,
9501028
},
@@ -975,9 +1053,12 @@ describe('useChartConfig', () => {
9751053
const mockResponse = createMockQueryResponse([]);
9761054
mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse);
9771055

978-
const { result } = renderHook(() => useQueriedChartConfig(config), {
979-
wrapper,
980-
});
1056+
const { result } = renderHook(
1057+
() => useQueriedChartConfig(config, { enableQueryChunking: true }),
1058+
{
1059+
wrapper,
1060+
},
1061+
);
9811062

9821063
await waitFor(() => expect(result.current.isSuccess).toBe(true));
9831064
await waitFor(() => expect(result.current.isFetching).toBe(false));

‎packages/app/src/hooks/useChartConfig.tsx‎

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,12 @@ import { generateTimeWindowsDescending } from '@/utils/searchWindows';
3535
interface AdditionalUseQueriedChartConfigOptions {
3636
onError?: (error: Error | ClickHouseQueryError) => void;
3737
/**
38-
* By default, queries with large date ranges are split into multiple smaller queries to
38+
* Queries with large date ranges can be split into multiple smaller queries to
3939
* avoid overloading the ClickHouse server and running into timeouts. In some cases, such
4040
* as when data is being sampled across the entire range, this chunking is not desirable
41-
* and can be disabled.
41+
* and should be disabled.
4242
*/
43-
disableQueryChunking?: boolean;
43+
enableQueryChunking?: boolean;
4444
}
4545

4646
type TimeWindow = {
@@ -121,16 +121,21 @@ export const getGranularityAlignedTimeWindows = (
121121
return windows;
122122
};
123123

124-
async function* fetchDataInChunks(
125-
config: ChartConfigWithOptDateRange,
126-
clickhouseClient: ClickhouseClient,
127-
signal: AbortSignal,
128-
disableQueryChunking: boolean = false,
129-
) {
124+
async function* fetchDataInChunks({
125+
config,
126+
clickhouseClient,
127+
signal,
128+
enableQueryChunking = false,
129+
}: {
130+
config: ChartConfigWithOptDateRange;
131+
clickhouseClient: ClickhouseClient;
132+
signal: AbortSignal;
133+
enableQueryChunking?: boolean;
134+
}) {
130135
const windows =
131-
!disableQueryChunking && shouldUseChunking(config)
136+
enableQueryChunking && shouldUseChunking(config)
132137
? getGranularityAlignedTimeWindows(config)
133-
: ([undefined] as const);
138+
: [undefined];
134139

135140
if (IS_MTVIEWS_ENABLED) {
136141
const { dataTableDDL, mtViewDDL, renderMTViewConfig } =
@@ -181,7 +186,7 @@ function appendChunk(
181186
*
182187
* If all of the following are true, the query will be chunked into multiple smaller queries:
183188
* - The config includes a dateRange, granularity, and timestampValueExpression
184-
* - `options.disableQueryChunking` is falsy
189+
* - `options.enableQueryChunking` is true
185190
*
186191
* For chunked queries, note the following:
187192
* - `config.limit`, if provided, is applied to each chunk, so the total number
@@ -202,9 +207,9 @@ export function useQueriedChartConfig(
202207
const queryClient = useQueryClient();
203208

204209
const query = useQuery<TQueryFnData, ClickHouseQueryError | Error>({
205-
// Include disableQueryChunking in the query key to ensure that queries with the
206-
// same config but different disableQueryChunking values do not share a query
207-
queryKey: [config, options?.disableQueryChunking ?? false],
210+
// Include enableQueryChunking in the query key to ensure that queries with the
211+
// same config but different enableQueryChunking values do not share a query
212+
queryKey: [config, options?.enableQueryChunking ?? false],
208213
// TODO: Replace this with `streamedQuery` when it is no longer experimental. Use 'replace' refetch mode.
209214
// https://tanstack.com/query/latest/docs/reference/streamedQuery
210215
queryFn: async context => {
@@ -220,12 +225,12 @@ export function useQueriedChartConfig(
220225
isComplete: false,
221226
};
222227

223-
const chunks = fetchDataInChunks(
228+
const chunks = fetchDataInChunks({
224229
config,
225230
clickhouseClient,
226-
context.signal,
227-
options?.disableQueryChunking,
228-
);
231+
signal: context.signal,
232+
enableQueryChunking: options?.enableQueryChunking,
233+
});
229234

230235
let accumulatedChunks: TQueryFnData = emptyValue;
231236
for await (const chunk of chunks) {

‎packages/app/src/hooks/usePatterns.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ function usePatterns({
147147
{
148148
enabled: configWithPrimaryAndPartitionKey != null && enabled,
149149
// Disable chunking to ensure we get the desired sample size
150-
disableQueryChunking: true,
150+
enableQueryChunking: false,
151151
},
152152
);
153153

0 commit comments

Comments
 (0)