Skip to content

Commit 1cd4e7c

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

File tree

5 files changed

+199
-61
lines changed

5 files changed

+199
-61
lines changed

‎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/components/SearchTotalCountChart.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export function useSearchTotalCount(
1010
config: ChartConfigWithDateRange,
1111
queryKeyPrefix: string,
1212
) {
13-
// copied from DBTimeChart
13+
// queriedConfig, queryKey, and enableQueryChunking match DBTimeChart so that react query can de-dupe these queries.
1414
const { granularity } = useTimeChartSettings(config);
1515
const queriedConfig = {
1616
...config,
@@ -22,10 +22,11 @@ export function useSearchTotalCount(
2222
isLoading,
2323
isError,
2424
} = useQueriedChartConfig(queriedConfig, {
25-
queryKey: [queryKeyPrefix, queriedConfig],
25+
queryKey: [queryKeyPrefix, queriedConfig, 'chunked'],
2626
staleTime: 1000 * 60 * 5,
2727
refetchOnWindowFocus: false,
2828
placeholderData: keepPreviousData, // no need to flash loading state when in live tail
29+
enableQueryChunking: true,
2930
});
3031

3132
const isTotalCountComplete = !!totalCountData?.isComplete;

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

Lines changed: 161 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
},
@@ -862,6 +936,54 @@ describe('useChartConfig', () => {
862936
expect(result.current.error).toBe(mockError);
863937
});
864938

939+
it('has an error status with partial data after the second chunk fails to fetch', async () => {
940+
const mockError = new Error('Query failed');
941+
const mockSuccess = createMockQueryResponse([
942+
{
943+
'count()': '71',
944+
__hdx_time_bucket: '2025-10-01T18:00:00Z',
945+
},
946+
]);
947+
948+
mockClickhouseClient.queryChartConfig
949+
.mockResolvedValueOnce(mockSuccess)
950+
.mockRejectedValueOnce(mockError);
951+
952+
const onError = jest.fn();
953+
const config = createMockChartConfig({
954+
dateRange: [
955+
new Date('2025-10-01 00:00:00Z'),
956+
new Date('2025-10-02 00:00:00Z'),
957+
],
958+
granularity: '3 hour',
959+
});
960+
961+
const { result } = renderHook(
962+
() =>
963+
useQueriedChartConfig(config, {
964+
onError,
965+
retry: false,
966+
enableQueryChunking: true,
967+
}),
968+
{
969+
wrapper,
970+
},
971+
);
972+
973+
await waitFor(() => expect(result.current.isError).toBe(true));
974+
expect(result.current.isFetching).toBe(false);
975+
expect(result.current.isSuccess).toBe(false);
976+
977+
expect(result.current.error).toBe(mockError);
978+
expect(onError).toHaveBeenCalledWith(mockError);
979+
expect(result.current.data).toEqual({
980+
data: mockSuccess.data,
981+
meta: mockSuccess.meta,
982+
rows: mockSuccess.rows,
983+
isComplete: false,
984+
});
985+
});
986+
865987
it('does not make requests if it is disabled', async () => {
866988
const config = createMockChartConfig({
867989
dateRange: [
@@ -881,7 +1003,11 @@ describe('useChartConfig', () => {
8811003
mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse);
8821004

8831005
const { result } = renderHook(
884-
() => useQueriedChartConfig(config, { enabled: false }),
1006+
() =>
1007+
useQueriedChartConfig(config, {
1008+
enabled: false,
1009+
enableQueryChunking: true,
1010+
}),
8851011
{
8861012
wrapper,
8871013
},
@@ -895,7 +1021,7 @@ describe('useChartConfig', () => {
8951021
expect(result.current.data).toBeUndefined();
8961022
});
8971023

898-
it('uses different query keys for the same config when one sets disableQueryChunking', async () => {
1024+
it('uses different query keys for the same config when one sets enableQueryChunking', async () => {
8991025
const config = createMockChartConfig({
9001026
dateRange: [
9011027
new Date('2025-10-01 00:00:00Z'),
@@ -923,7 +1049,7 @@ describe('useChartConfig', () => {
9231049
);
9241050

9251051
const { result: result1 } = renderHook(
926-
() => useQueriedChartConfig(config),
1052+
() => useQueriedChartConfig(config, { enableQueryChunking: true }),
9271053
{
9281054
wrapper,
9291055
},
@@ -938,13 +1064,13 @@ describe('useChartConfig', () => {
9381064
expect(chunkedCallCount).toBeGreaterThan(1);
9391065
expect(result1.current.data?.rows).toBeGreaterThan(1);
9401066

941-
// Second render with same config but disableQueryChunking=true
1067+
// Second render with same config but without query chunking enabled
9421068
mockClickhouseClient.queryChartConfig.mockResolvedValue(
9431069
mockResponseNonChunked,
9441070
);
9451071

9461072
const { result: result2 } = renderHook(
947-
() => useQueriedChartConfig(config, { disableQueryChunking: true }),
1073+
() => useQueriedChartConfig(config),
9481074
{
9491075
wrapper,
9501076
},
@@ -975,9 +1101,12 @@ describe('useChartConfig', () => {
9751101
const mockResponse = createMockQueryResponse([]);
9761102
mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse);
9771103

978-
const { result } = renderHook(() => useQueriedChartConfig(config), {
979-
wrapper,
980-
});
1104+
const { result } = renderHook(
1105+
() => useQueriedChartConfig(config, { enableQueryChunking: true }),
1106+
{
1107+
wrapper,
1108+
},
1109+
);
9811110

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

0 commit comments

Comments
 (0)