From 13d7ff8d9a30bd113f6c6b9baa294c571b19c7c4 Mon Sep 17 00:00:00 2001 From: Warren <5959690+wrn14897@users.noreply.github.com> Date: Thu, 7 Aug 2025 18:10:29 -0700 Subject: [PATCH 1/3] perf: add prelimit CTE to getMapKeys query (#611) --- packages/common-utils/src/metadata.ts | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/packages/common-utils/src/metadata.ts b/packages/common-utils/src/metadata.ts index 44e01e2b1a..f93933bfd4 100644 --- a/packages/common-utils/src/metadata.ts +++ b/packages/common-utils/src/metadata.ts @@ -269,13 +269,24 @@ export class Metadata { }}) as keysArr FROM ${tableExpr({ database: databaseName, table: tableName })} ${where}`; } else { - sql = chSql`SELECT DISTINCT lowCardinalityKeys(arrayJoin(${{ - Identifier: column, - }}.keys)) as key - FROM ${tableExpr({ database: databaseName, table: tableName })} ${where} - LIMIT ${{ - Int32: maxKeys, - }}`; + sql = chSql` + WITH sampledKeys as ( + SELECT ${{ + Identifier: column, + }}.keys AS keysArr + FROM ${tableExpr({ database: databaseName, table: tableName })} ${where} + LIMIT ${{ + Int32: this.clickhouseSettings.max_rows_to_read + ? Number(this.clickhouseSettings.max_rows_to_read) + : DEFAULT_METADATA_MAX_ROWS_TO_READ, + }} + ) + SELECT DISTINCT lowCardinalityKeys(arrayJoin(keysArr)) as key + FROM sampledKeys + LIMIT ${{ + Int32: maxKeys, + }} + `; } return this.cache.getOrFetch(cacheKey, async () => { From 435bb6e9800dd2f0062a5d43ed9fe4b73db2a808 Mon Sep 17 00:00:00 2001 From: Warren <5959690+wrn14897@users.noreply.github.com> Date: Thu, 4 Sep 2025 09:19:39 -0700 Subject: [PATCH 2/3] fix + perf: store clickhouse settings in shared cache (#672) I believe the reason Anthropic was hitting the memory error is that the `metadataMaxRowsToRead` setting was applied only to a specific query (and did not include the getKeyValues method). Now, since ClickHouse settings are cached and shared across all metadata instances, every method within the Metadata class should respect these settings. This PR also adds another CTE filtering on top of queries that touch array aggregation functions like `groupUniqArray` and `groupUniqArrayArray` I will port this fix to OSS later Ref: HDX-2321 --- .../src/components/DBSearchPageFilters.tsx | 4 +- packages/app/src/hooks/useMetadata.tsx | 48 +++++----- packages/common-utils/src/metadata.ts | 88 ++++++++++++++----- 3 files changed, 96 insertions(+), 44 deletions(-) diff --git a/packages/app/src/components/DBSearchPageFilters.tsx b/packages/app/src/components/DBSearchPageFilters.tsx index f6f045d5bc..3cc53e3200 100644 --- a/packages/app/src/components/DBSearchPageFilters.tsx +++ b/packages/app/src/components/DBSearchPageFilters.tsx @@ -33,8 +33,8 @@ import { useJsonColumns, useTableMetadata, } from '@/hooks/useMetadata'; +import { useMetadataWithSettings } from '@/hooks/useMetadata'; import useResizable from '@/hooks/useResizable'; -import { getMetadata } from '@/metadata'; import { FilterStateHook, usePinnedFilters } from '@/searchFilters'; import { useSource } from '@/source'; import { mergePath } from '@/utils'; @@ -594,6 +594,7 @@ const DBSearchPageFiltersComponent = ({ keys: keysToFetch, }); + const metadata = useMetadataWithSettings(); const [extraFacets, setExtraFacets] = useState>({}); const [loadMoreLoadingKeys, setLoadMoreLoadingKeys] = useState>( new Set(), @@ -602,7 +603,6 @@ const DBSearchPageFiltersComponent = ({ async (key: string) => { setLoadMoreLoadingKeys(prev => new Set(prev).add(key)); try { - const metadata = getMetadata(); const newKeyVals = await metadata.getKeyValues({ chartConfig: { ...chartConfig, diff --git a/packages/app/src/hooks/useMetadata.tsx b/packages/app/src/hooks/useMetadata.tsx index c398be2958..7c8e2c8f13 100644 --- a/packages/app/src/hooks/useMetadata.tsx +++ b/packages/app/src/hooks/useMetadata.tsx @@ -1,3 +1,4 @@ +import { useEffect, useRef } from 'react'; import objectHash from 'object-hash'; import { ColumnMeta, @@ -20,6 +21,25 @@ import api from '@/api'; import { getMetadata } from '@/metadata'; import { toArray } from '@/utils'; +// Hook to get metadata with proper settings applied +// TODO: replace all getMetadata calls with useMetadataWithSettings +export function useMetadataWithSettings() { + const metadata = getMetadata(); + const { data: me } = api.useMe(); + const settingsApplied = useRef(false); + + useEffect(() => { + if (me?.team?.metadataMaxRowsToRead && !settingsApplied.current) { + metadata.setClickHouseSettings({ + max_rows_to_read: me.team.metadataMaxRowsToRead, + }); + settingsApplied.current = true; + } + }, [me?.team?.metadataMaxRowsToRead, metadata]); + + return metadata; +} + export function useColumns( { databaseName, @@ -32,10 +52,10 @@ export function useColumns( }, options?: Partial>, ) { + const metadata = useMetadataWithSettings(); return useQuery({ queryKey: ['useMetadata.useColumns', { databaseName, tableName }], queryFn: async () => { - const metadata = getMetadata(); return metadata.getColumns({ databaseName, tableName, @@ -59,10 +79,10 @@ export function useJsonColumns( }, options?: Partial>, ) { + const metadata = useMetadataWithSettings(); return useQuery({ queryKey: ['useMetadata.useJsonColumns', { databaseName, tableName }], queryFn: async () => { - const metadata = getMetadata(); const columns = await metadata.getColumns({ databaseName, tableName, @@ -86,7 +106,7 @@ export function useAllFields( const tableConnections = Array.isArray(_tableConnections) ? _tableConnections : [_tableConnections]; - const metadata = getMetadata(); + const metadata = useMetadataWithSettings(); const { data: me, isFetched } = api.useMe(); return useQuery({ queryKey: [ @@ -99,13 +119,6 @@ export function useAllFields( return []; } - // TODO: set the settings at the top level so that it doesn't have to be set for each useQuery - if (team?.metadataMaxRowsToRead) { - metadata.setClickHouseSettings({ - max_rows_to_read: team.metadataMaxRowsToRead, - }); - } - const fields2d = await Promise.all( tableConnections.map(tc => metadata.getAllFields(tc)), ); @@ -137,7 +150,7 @@ export function useTableMetadata( }, options?: Omit, 'queryKey'>, ) { - const metadata = getMetadata(); + const metadata = useMetadataWithSettings(); return useQuery({ queryKey: ['useMetadata.useTableMetadata', { databaseName, tableName }], queryFn: async () => { @@ -167,9 +180,8 @@ export function useGetKeyValues( }, options?: Omit, 'queryKey'>, ) { - const metadata = getMetadata(); + const metadata = useMetadataWithSettings(); const chartConfigsArr = toArray(chartConfigs); - const { data: me, isFetched } = api.useMe(); return useQuery<{ key: string; value: string[] }[]>({ queryKey: [ 'useMetadata.useGetKeyValues', @@ -178,14 +190,6 @@ export function useGetKeyValues( disableRowLimit, ], queryFn: async () => { - const team = me?.team; - - // TODO: set the settings at the top level so that it doesn't have to be set for each useQuery - if (team?.metadataMaxRowsToRead) { - metadata.setClickHouseSettings({ - max_rows_to_read: team.metadataMaxRowsToRead, - }); - } return ( await Promise.all( chartConfigsArr.map(chartConfig => @@ -200,7 +204,7 @@ export function useGetKeyValues( ).flatMap(v => v); }, staleTime: 1000 * 60 * 5, // Cache every 5 min - enabled: !!keys.length && isFetched, + enabled: !!keys.length, placeholderData: keepPreviousData, ...options, }); diff --git a/packages/common-utils/src/metadata.ts b/packages/common-utils/src/metadata.ts index f93933bfd4..5cbacdef20 100644 --- a/packages/common-utils/src/metadata.ts +++ b/packages/common-utils/src/metadata.ts @@ -94,7 +94,6 @@ export type TableMetadata = { export class Metadata { private readonly clickhouseClient: BaseClickhouseClient; private readonly cache: MetadataCache; - private readonly clickhouseSettings: ClickHouseSettings; constructor( clickhouseClient: BaseClickhouseClient, @@ -103,11 +102,19 @@ export class Metadata { ) { this.clickhouseClient = clickhouseClient; this.cache = cache; - this.clickhouseSettings = settings ?? {}; + if (settings) { + this.cache.set('clickhouse-settings', settings); + } + } + + getClickHouseSettings(): ClickHouseSettings { + return this.cache.get('clickhouse-settings') ?? {}; } setClickHouseSettings(settings: ClickHouseSettings) { - Object.assign(this.clickhouseSettings, settings); + const currentSettings = this.getClickHouseSettings(); + const updatedSettings = { ...currentSettings, ...settings }; + this.cache.set('clickhouse-settings', updatedSettings); } private async queryTableMetadata({ @@ -128,7 +135,7 @@ export class Metadata { connectionId, query: sql.sql, query_params: sql.params, - clickhouse_settings: this.clickhouseSettings, + clickhouse_settings: this.getClickHouseSettings(), }) .then(res => res.json()); return json.data[0]; @@ -153,7 +160,7 @@ export class Metadata { query: sql.sql, query_params: sql.params, connectionId, - clickhouse_settings: this.clickhouseSettings, + clickhouse_settings: this.getClickHouseSettings(), }) .then(res => res.json()) .then(d => d.data); @@ -264,10 +271,20 @@ export class Metadata { : ''; let sql: ChSql; if (strategy === 'groupUniqArrayArray') { - sql = chSql`SELECT groupUniqArrayArray(${{ Int32: maxKeys }})(${{ - Identifier: column, - }}) as keysArr - FROM ${tableExpr({ database: databaseName, table: tableName })} ${where}`; + sql = chSql` + WITH sampledKeys as ( + SELECT ${{ + Identifier: column, + }}.keys AS keys + FROM ${tableExpr({ database: databaseName, table: tableName })} ${where} + LIMIT ${{ + Int32: this.getClickHouseSettings().max_rows_to_read + ? Number(this.getClickHouseSettings().max_rows_to_read) + : DEFAULT_METADATA_MAX_ROWS_TO_READ, + }} + ) + SELECT groupUniqArrayArray(${{ Int32: maxKeys }})(keys) as keysArr + FROM sampledKeys`; } else { sql = chSql` WITH sampledKeys as ( @@ -276,8 +293,8 @@ export class Metadata { }}.keys AS keysArr FROM ${tableExpr({ database: databaseName, table: tableName })} ${where} LIMIT ${{ - Int32: this.clickhouseSettings.max_rows_to_read - ? Number(this.clickhouseSettings.max_rows_to_read) + Int32: this.getClickHouseSettings().max_rows_to_read + ? Number(this.getClickHouseSettings().max_rows_to_read) : DEFAULT_METADATA_MAX_ROWS_TO_READ, }} ) @@ -296,9 +313,12 @@ export class Metadata { query_params: sql.params, connectionId, clickhouse_settings: { - max_rows_to_read: String(DEFAULT_METADATA_MAX_ROWS_TO_READ), + max_rows_to_read: String( + this.getClickHouseSettings().max_rows_to_read ?? + DEFAULT_METADATA_MAX_ROWS_TO_READ, + ), read_overflow_mode: 'break', - ...this.clickhouseSettings, + ...this.getClickHouseSettings(), }, }) .then(res => res.json>()) @@ -353,9 +373,12 @@ export class Metadata { query_params: sql.params, connectionId, clickhouse_settings: { - max_rows_to_read: String(DEFAULT_METADATA_MAX_ROWS_TO_READ), + max_rows_to_read: String( + this.getClickHouseSettings().max_rows_to_read ?? + DEFAULT_METADATA_MAX_ROWS_TO_READ, + ), read_overflow_mode: 'break', - ...this.clickhouseSettings, + ...this.getClickHouseSettings(), }, }) .then(res => res.json<{ pathMap: Record }>()) @@ -436,9 +459,12 @@ export class Metadata { query_params: sql.params, connectionId, clickhouse_settings: { - max_rows_to_read: String(DEFAULT_METADATA_MAX_ROWS_TO_READ), + max_rows_to_read: String( + this.getClickHouseSettings().max_rows_to_read ?? + DEFAULT_METADATA_MAX_ROWS_TO_READ, + ), read_overflow_mode: 'break', - ...this.clickhouseSettings, + ...this.getClickHouseSettings(), }, }) .then(res => res.json>()) @@ -559,10 +585,29 @@ export class Metadata { async () => { const sql = await renderChartConfig( { - ...chartConfig, + with: [ + { + name: 'sampledData', + chartConfig: { + ...chartConfig, + select: keys.join(', '), + limit: { + limit: !disableRowLimit + ? this.getClickHouseSettings().max_rows_to_read + ? Number(this.getClickHouseSettings().max_rows_to_read) + : DEFAULT_METADATA_MAX_ROWS_TO_READ + : undefined, + }, + }, + isSubquery: true, + }, + ], select: keys .map((k, i) => `groupUniqArray(${limit})(${k}) AS param${i}`) .join(', '), + connection: chartConfig.connection, + from: { databaseName: '', tableName: 'sampledData' }, + where: '', }, this, ); @@ -574,9 +619,12 @@ export class Metadata { connectionId: chartConfig.connection, clickhouse_settings: !disableRowLimit ? { - max_rows_to_read: String(DEFAULT_METADATA_MAX_ROWS_TO_READ), + max_rows_to_read: String( + this.getClickHouseSettings().max_rows_to_read ?? + DEFAULT_METADATA_MAX_ROWS_TO_READ, + ), read_overflow_mode: 'break', - ...this.clickhouseSettings, + ...this.getClickHouseSettings(), } : undefined, }) From dad982abf5544c15ebb659a4678951f87e8ca1c0 Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Thu, 18 Sep 2025 19:05:29 -0700 Subject: [PATCH 3/3] docs: add changeset --- .changeset/strange-boats-nail.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/strange-boats-nail.md diff --git a/.changeset/strange-boats-nail.md b/.changeset/strange-boats-nail.md new file mode 100644 index 0000000000..7f2e62534e --- /dev/null +++ b/.changeset/strange-boats-nail.md @@ -0,0 +1,6 @@ +--- +"@hyperdx/common-utils": patch +"@hyperdx/app": patch +--- + +perf: add prelimit CTE to getMapKeys query + store clickhouse settings in shared cache