Skip to content

Commit 8f06ce7

Browse files
authored
perf: add prelimit CTE to getMapKeys query + store clickhouse settings in shared cache (#1201)
Optimize the query performance of the getMapKeys method to prevent excessive resource usage in ClickHouse, even when max_rows_to_read is specified. Ref: HDX-2411 Ref: HDX-2431
1 parent 8673f96 commit 8f06ce7

File tree

5 files changed

+213
-49
lines changed

5 files changed

+213
-49
lines changed

.changeset/strange-boats-nail.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@hyperdx/common-utils": patch
3+
"@hyperdx/app": patch
4+
---
5+
6+
perf: add prelimit CTE to getMapKeys query + store clickhouse settings in shared cache

packages/app/src/components/DBSearchPageFilters.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ import {
3333
useJsonColumns,
3434
useTableMetadata,
3535
} from '@/hooks/useMetadata';
36+
import { useMetadataWithSettings } from '@/hooks/useMetadata';
3637
import useResizable from '@/hooks/useResizable';
37-
import { getMetadata } from '@/metadata';
3838
import { FilterStateHook, usePinnedFilters } from '@/searchFilters';
3939
import { useSource } from '@/source';
4040
import { mergePath } from '@/utils';
@@ -615,6 +615,7 @@ const DBSearchPageFiltersComponent = ({
615615
keys: keysToFetch,
616616
});
617617

618+
const metadata = useMetadataWithSettings();
618619
const [extraFacets, setExtraFacets] = useState<Record<string, string[]>>({});
619620
const [loadMoreLoadingKeys, setLoadMoreLoadingKeys] = useState<Set<string>>(
620621
new Set(),
@@ -623,7 +624,6 @@ const DBSearchPageFiltersComponent = ({
623624
async (key: string) => {
624625
setLoadMoreLoadingKeys(prev => new Set(prev).add(key));
625626
try {
626-
const metadata = getMetadata();
627627
const newKeyVals = await metadata.getKeyValues({
628628
chartConfig: {
629629
...chartConfig,

packages/app/src/hooks/useMetadata.tsx

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { useEffect, useRef } from 'react';
12
import objectHash from 'object-hash';
23
import {
34
ColumnMeta,
@@ -20,6 +21,25 @@ import api from '@/api';
2021
import { getMetadata } from '@/metadata';
2122
import { toArray } from '@/utils';
2223

24+
// Hook to get metadata with proper settings applied
25+
// TODO: replace all getMetadata calls with useMetadataWithSettings
26+
export function useMetadataWithSettings() {
27+
const metadata = getMetadata();
28+
const { data: me } = api.useMe();
29+
const settingsApplied = useRef(false);
30+
31+
useEffect(() => {
32+
if (me?.team?.metadataMaxRowsToRead && !settingsApplied.current) {
33+
metadata.setClickHouseSettings({
34+
max_rows_to_read: me.team.metadataMaxRowsToRead,
35+
});
36+
settingsApplied.current = true;
37+
}
38+
}, [me?.team?.metadataMaxRowsToRead, metadata]);
39+
40+
return metadata;
41+
}
42+
2343
export function useColumns(
2444
{
2545
databaseName,
@@ -32,10 +52,10 @@ export function useColumns(
3252
},
3353
options?: Partial<UseQueryOptions<ColumnMeta[]>>,
3454
) {
55+
const metadata = useMetadataWithSettings();
3556
return useQuery<ColumnMeta[]>({
3657
queryKey: ['useMetadata.useColumns', { databaseName, tableName }],
3758
queryFn: async () => {
38-
const metadata = getMetadata();
3959
return metadata.getColumns({
4060
databaseName,
4161
tableName,
@@ -51,10 +71,10 @@ export function useJsonColumns(
5171
{ databaseName, tableName, connectionId }: TableConnection,
5272
options?: Partial<UseQueryOptions<string[]>>,
5373
) {
74+
const metadata = useMetadataWithSettings();
5475
return useQuery<string[]>({
5576
queryKey: ['useMetadata.useJsonColumns', { databaseName, tableName }],
5677
queryFn: async () => {
57-
const metadata = getMetadata();
5878
const columns = await metadata.getColumns({
5979
databaseName,
6080
tableName,
@@ -78,7 +98,7 @@ export function useAllFields(
7898
const tableConnections = Array.isArray(_tableConnections)
7999
? _tableConnections
80100
: [_tableConnections];
81-
const metadata = getMetadata();
101+
const metadata = useMetadataWithSettings();
82102
const { data: me, isFetched } = api.useMe();
83103
return useQuery<Field[]>({
84104
queryKey: [
@@ -91,13 +111,6 @@ export function useAllFields(
91111
return [];
92112
}
93113

94-
// TODO: set the settings at the top level so that it doesn't have to be set for each useQuery
95-
if (team?.metadataMaxRowsToRead) {
96-
metadata.setClickHouseSettings({
97-
max_rows_to_read: team.metadataMaxRowsToRead,
98-
});
99-
}
100-
101114
const fields2d = await Promise.all(
102115
tableConnections.map(tc => metadata.getAllFields(tc)),
103116
);
@@ -129,7 +142,7 @@ export function useTableMetadata(
129142
},
130143
options?: Omit<UseQueryOptions<any, Error>, 'queryKey'>,
131144
) {
132-
const metadata = getMetadata();
145+
const metadata = useMetadataWithSettings();
133146
return useQuery<TableMetadata>({
134147
queryKey: ['useMetadata.useTableMetadata', { databaseName, tableName }],
135148
queryFn: async () => {
@@ -159,9 +172,8 @@ export function useGetKeyValues(
159172
},
160173
options?: Omit<UseQueryOptions<any, Error>, 'queryKey'>,
161174
) {
162-
const metadata = getMetadata();
175+
const metadata = useMetadataWithSettings();
163176
const chartConfigsArr = toArray(chartConfigs);
164-
const { data: me, isFetched } = api.useMe();
165177
return useQuery<{ key: string; value: string[] }[]>({
166178
queryKey: [
167179
'useMetadata.useGetKeyValues',
@@ -170,14 +182,6 @@ export function useGetKeyValues(
170182
disableRowLimit,
171183
],
172184
queryFn: async () => {
173-
const team = me?.team;
174-
175-
// TODO: set the settings at the top level so that it doesn't have to be set for each useQuery
176-
if (team?.metadataMaxRowsToRead) {
177-
metadata.setClickHouseSettings({
178-
max_rows_to_read: team.metadataMaxRowsToRead,
179-
});
180-
}
181185
return (
182186
await Promise.all(
183187
chartConfigsArr.map(chartConfig =>
@@ -192,7 +196,7 @@ export function useGetKeyValues(
192196
).flatMap(v => v);
193197
},
194198
staleTime: 1000 * 60 * 5, // Cache every 5 min
195-
enabled: !!keys.length && isFetched,
199+
enabled: !!keys.length,
196200
placeholderData: keepPreviousData,
197201
...options,
198202
});

packages/common-utils/src/__tests__/metadata.test.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,22 @@ describe('Metadata', () => {
251251
],
252252
}),
253253
});
254+
255+
// Mock getColumns to return columns including materialized ones
256+
mockCache.getOrFetch.mockImplementation((key, queryFn) => {
257+
if (key.includes('.columns')) {
258+
return Promise.resolve([
259+
{ name: 'regular_column', type: 'String', default_type: '' },
260+
{
261+
name: 'materialized_column',
262+
type: 'String',
263+
default_type: 'MATERIALIZED',
264+
},
265+
{ name: 'default_column', type: 'String', default_type: 'DEFAULT' },
266+
]);
267+
}
268+
return queryFn();
269+
});
254270
});
255271

256272
it('should apply row limit when disableRowLimit is false', async () => {
@@ -336,5 +352,72 @@ describe('Metadata', () => {
336352

337353
expect(result).toEqual([{ key: 'column1', value: ['value1', 'value2'] }]);
338354
});
355+
356+
it('should include materialized fields when selecting all columns', async () => {
357+
const renderChartConfigSpy = jest.spyOn(
358+
renderChartConfigModule,
359+
'renderChartConfig',
360+
);
361+
362+
await metadata.getKeyValues({
363+
chartConfig: mockChartConfig,
364+
keys: ['column1'],
365+
limit: 10,
366+
});
367+
368+
// Verify that renderChartConfig was called with the expanded select list
369+
// that includes all columns by name (including materialized ones)
370+
expect(renderChartConfigSpy).toHaveBeenCalledWith(
371+
expect.objectContaining({
372+
with: [
373+
expect.objectContaining({
374+
name: 'sampledData',
375+
chartConfig: expect.objectContaining({
376+
// Should expand to all column names instead of using '*'
377+
select:
378+
'`regular_column`, `materialized_column`, `default_column`',
379+
}),
380+
}),
381+
],
382+
}),
383+
metadata,
384+
);
385+
});
386+
387+
it('should fallback to * when no columns are found', async () => {
388+
// Mock getColumns to return empty array
389+
mockCache.getOrFetch.mockImplementation((key, queryFn) => {
390+
if (key.includes('.columns')) {
391+
return Promise.resolve([]);
392+
}
393+
return queryFn();
394+
});
395+
396+
const renderChartConfigSpy = jest.spyOn(
397+
renderChartConfigModule,
398+
'renderChartConfig',
399+
);
400+
401+
await metadata.getKeyValues({
402+
chartConfig: mockChartConfig,
403+
keys: ['column1'],
404+
limit: 10,
405+
});
406+
407+
// Should fallback to '*' when no columns are found
408+
expect(renderChartConfigSpy).toHaveBeenCalledWith(
409+
expect.objectContaining({
410+
with: [
411+
expect.objectContaining({
412+
name: 'sampledData',
413+
chartConfig: expect.objectContaining({
414+
select: '*',
415+
}),
416+
}),
417+
],
418+
}),
419+
metadata,
420+
);
421+
});
339422
});
340423
});

0 commit comments

Comments
 (0)