Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .changeset/text-index-values-no-row-cap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
'@hyperdx/common-utils': patch
'@hyperdx/api': patch
'@hyperdx/app': patch
---

fix: show values for every key in text index value lookups

"All values" lookups that read a text index (Map key=value indexes and text
indexes on plain columns) stopped at `max_rows_to_read` with
`read_overflow_mode = 'break'`. The index is read in sorted token order, so once
a high-cardinality key used up the budget, every key that sorts after it came
back with no values and no error. These lookups no longer apply a row cap.

To keep them bounded, they now read a limited set of parts like Map key
discovery: the 20 parts closest to the median size by default, or as set by the
source's "Map key index parts" (`mapKeyIndexMaxParts`) and "Map key index part
size" (`mapKeyIndexPartSelection`) settings, which now cover values as well as
keys.
"Load more" on a single filter key still reads every part in the time range.
8 changes: 4 additions & 4 deletions packages/api/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -5206,7 +5206,7 @@
"mapKeyIndexMaxParts": {
"type": "integer",
"minimum": 1,
"description": "How many data parts Map attribute key discovery reads from a text index, chosen by mapKeyIndexPartSelection. Raise it if attribute keys are missing from filters; lower it if field loading is slow. Defaults to 20.",
"description": "How many data parts are read from text indexes to discover attribute keys and filter values, chosen by mapKeyIndexPartSelection. Raise it if keys or values are missing from filters; lower it if they load slowly. Defaults to 20.",
"nullable": true,
"example": 20
},
Expand All @@ -5217,7 +5217,7 @@
"smallest",
"largest"
],
"description": "Which data parts, by row count, Map attribute key discovery reads from a text index. \"median\" (the default) balances coverage and cost, \"smallest\" reads the newest unmerged parts, \"largest\" reads the most merged data.",
"description": "Which data parts, by row count, are read from text indexes to discover attribute keys and filter values. \"median\" (the default) balances coverage and cost, \"smallest\" reads the newest unmerged parts, \"largest\" reads the most merged data.",
"nullable": true,
"example": "median"
},
Expand Down Expand Up @@ -5478,7 +5478,7 @@
"mapKeyIndexMaxParts": {
"type": "integer",
"minimum": 1,
"description": "How many data parts Map attribute key discovery reads from a text index, chosen by mapKeyIndexPartSelection. Raise it if attribute keys are missing from filters; lower it if field loading is slow. Defaults to 20.",
"description": "How many data parts are read from text indexes to discover attribute keys and filter values, chosen by mapKeyIndexPartSelection. Raise it if keys or values are missing from filters; lower it if they load slowly. Defaults to 20.",
"nullable": true,
"example": 20
},
Expand All @@ -5489,7 +5489,7 @@
"smallest",
"largest"
],
"description": "Which data parts, by row count, Map attribute key discovery reads from a text index. \"median\" (the default) balances coverage and cost, \"smallest\" reads the newest unmerged parts, \"largest\" reads the most merged data.",
"description": "Which data parts, by row count, are read from text indexes to discover attribute keys and filter values. \"median\" (the default) balances coverage and cost, \"smallest\" reads the newest unmerged parts, \"largest\" reads the most merged data.",
"nullable": true,
"example": "median"
},
Expand Down
20 changes: 12 additions & 8 deletions packages/api/src/mcp/tools/sources/describeSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,12 @@ async function describeSourceSchema(
'metadataMaterializedViews' in source
? source.metadataMaterializedViews
: undefined;
const mapKeyIndexMaxParts =
'mapKeyIndexMaxParts' in source ? source.mapKeyIndexMaxParts : undefined;
const mapKeyIndexPartSelection =
'mapKeyIndexPartSelection' in source
? source.mapKeyIndexPartSelection
: undefined;

const now = new Date();
const dateRange: [Date, Date] = [
Expand Down Expand Up @@ -306,14 +312,8 @@ async function describeSourceSchema(
maxKeys: 50,
connectionId,
metadataMVs,
mapKeyIndexMaxParts:
'mapKeyIndexMaxParts' in source
? source.mapKeyIndexMaxParts
: undefined,
mapKeyIndexPartSelection:
'mapKeyIndexPartSelection' in source
? source.mapKeyIndexPartSelection
: undefined,
mapKeyIndexMaxParts,
mapKeyIndexPartSelection,
dateRange,
timestampValueExpression,
signal,
Expand Down Expand Up @@ -360,6 +360,8 @@ async function describeSourceSchema(
maxValuesPerKey: MAX_LC_VALUES,
connectionId,
metadataMVs,
mapKeyIndexMaxParts,
mapKeyIndexPartSelection,
dateRange,
timestampValueExpression,
signal,
Expand Down Expand Up @@ -407,6 +409,8 @@ async function describeSourceSchema(
maxValuesPerKey: MAX_MAP_KEY_VALUES,
connectionId,
metadataMVs,
mapKeyIndexMaxParts,
mapKeyIndexPartSelection,
dateRange,
timestampValueExpression,
signal,
Expand Down
12 changes: 6 additions & 6 deletions packages/api/src/mcp/tools/sources/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -454,17 +454,17 @@ const mcpSaveSourceSchema = z.object({
.min(1)
.optional()
.describe(
'Log and trace sources. How many data parts Map attribute key discovery ' +
'reads from a text index, chosen by mapKeyIndexPartSelection. Raise it ' +
'if attribute keys are missing from filters; lower it if field loading ' +
'is slow. Omit for the default of 20.',
'Log and trace sources. How many data parts are read from text indexes ' +
'to discover attribute keys and filter values, chosen by ' +
'mapKeyIndexPartSelection. Raise it if keys or values are missing from ' +
'filters; lower it if they load slowly. Omit for the default of 20.',
),
mapKeyIndexPartSelection: z
.enum(['median', 'smallest', 'largest'])
.optional()
.describe(
'Log and trace sources. Which data parts, by row count, Map attribute ' +
'key discovery reads from a text index: "median" (used when unset) ' +
'Log and trace sources. Which data parts, by row count, are read from ' +
'text indexes for attribute keys and filter values: "median" (used when unset) ' +
'balances coverage and cost, "smallest" reads the newest unmerged ' +
'parts, "largest" reads the most merged data.',
),
Expand Down
8 changes: 4 additions & 4 deletions packages/api/src/routers/external-api/v2/sources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -415,13 +415,13 @@ function formatExternalSource(source: SourceDocument) {
* mapKeyIndexMaxParts:
* type: integer
* minimum: 1
* description: How many data parts Map attribute key discovery reads from a text index, chosen by mapKeyIndexPartSelection. Raise it if attribute keys are missing from filters; lower it if field loading is slow. Defaults to 20.
* description: How many data parts are read from text indexes to discover attribute keys and filter values, chosen by mapKeyIndexPartSelection. Raise it if keys or values are missing from filters; lower it if they load slowly. Defaults to 20.
* nullable: true
* example: 20
* mapKeyIndexPartSelection:
* type: string
* enum: [median, smallest, largest]
* description: Which data parts, by row count, Map attribute key discovery reads from a text index. "median" (the default) balances coverage and cost, "smallest" reads the newest unmerged parts, "largest" reads the most merged data.
* description: Which data parts, by row count, are read from text indexes to discover attribute keys and filter values. "median" (the default) balances coverage and cost, "smallest" reads the newest unmerged parts, "largest" reads the most merged data.
* nullable: true
* example: median
* highlightedTraceAttributeExpressions:
Expand Down Expand Up @@ -628,13 +628,13 @@ function formatExternalSource(source: SourceDocument) {
* mapKeyIndexMaxParts:
* type: integer
* minimum: 1
* description: How many data parts Map attribute key discovery reads from a text index, chosen by mapKeyIndexPartSelection. Raise it if attribute keys are missing from filters; lower it if field loading is slow. Defaults to 20.
* description: How many data parts are read from text indexes to discover attribute keys and filter values, chosen by mapKeyIndexPartSelection. Raise it if keys or values are missing from filters; lower it if they load slowly. Defaults to 20.
* nullable: true
* example: 20
* mapKeyIndexPartSelection:
* type: string
* enum: [median, smallest, largest]
* description: Which data parts, by row count, Map attribute key discovery reads from a text index. "median" (the default) balances coverage and cost, "smallest" reads the newest unmerged parts, "largest" reads the most merged data.
* description: Which data parts, by row count, are read from text indexes to discover attribute keys and filter values. "median" (the default) balances coverage and cost, "smallest" reads the newest unmerged parts, "largest" reads the most merged data.
* nullable: true
* example: median
* highlightedTraceAttributeExpressions:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,9 @@ describe('useFetchFacets', () => {
});

expect(getAllKeyValues).toHaveBeenCalledTimes(1);
expect(getAllKeyValues).toHaveBeenCalledWith(
expect.objectContaining({ mapKeyIndexMaxParts: null }),
);
expect(getKeyValuesWithMVs).not.toHaveBeenCalled();
expect(result.current.data.keyValues).toEqual([
{ key: 'ServiceName', value: ['api', 'web'] },
Expand Down
2 changes: 2 additions & 0 deletions packages/app/src/components/DBSearchPageFilters/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,8 @@ function useFacets({
tableName: tableConnection.tableName,
connectionId: tableConnection.connectionId,
metadataMVs: tableConnection.metadataMVs,
// "Load more" wants every value in the time range, not a sample of parts.
mapKeyIndexMaxParts: null,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 major — "Load more" now has no bound at all: every part plus max_rows_to_read=0

Before this PR, load more read every part but stopped at a 3M-row cap. It now passes mapKeyIndexMaxParts: null, so it reads every part, and getMapTextIndexKeyValues/getTextIndexKeyValues (metadata.ts:1820, 1898) force max_rows_to_read: '0' on top of that. Nothing limits the read anymore, and until #3617 there's no per-key LIMIT either. Clicking load more on a high-cardinality key like TraceId or LogAttributes['requestId'] over a wide time range scans that key's whole text-index dictionary in every part. The sort-order problem this PR fixes only happens when several keys share one budget. Load more asks for one key, so when the part limit is null you can keep the team/default row cap with read_overflow_mode: 'break' and drop it only in the multi-key, sampled-parts path.

Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond

keyExpressions: [sqlKey],
maxValuesPerKey: SINGLE_KEY_VALUES_LIMIT,
dateRange,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export function MapKeyIndexMaxPartsFormRow({
return (
<FormRow
label="Map key index parts"
helpText={`How many data parts are read from a Map key text index to discover attribute keys; "Map key index part size" picks which ones. Raise it if keys are missing from filters; lower it if fields load slowly. Defaults to ${DEFAULT_MAP_KEY_INDEX_MAX_PARTS}.`}
helpText={`How many data parts are read from text indexes to discover attribute keys and filter values; "Map key index part size" picks which ones. Raise it if keys or values are missing from filters; lower it if they load slowly. Defaults to ${DEFAULT_MAP_KEY_INDEX_MAX_PARTS}.`}
>
<Controller
control={control}
Expand Down Expand Up @@ -46,7 +46,7 @@ export function MapKeyIndexPartSelectionFormRow({
return (
<FormRow
label="Map key index part size"
helpText='Which data parts, by row count, are read from a Map key text index. "Median" balances coverage and cost; "Smallest" reads the newest unmerged parts, which is cheap and shows new keys first; "Largest" reads the most merged data, which covers the most keys but reads the biggest indexes.'
helpText='Which data parts, by row count, are read from text indexes for attribute keys and filter values. "Median" balances coverage and cost; "Smallest" reads the newest unmerged parts, which is cheap and shows new keys first; "Largest" reads the most merged data, which covers the most keys but reads the biggest indexes.'
>
<SelectControlled
control={control}
Expand Down
48 changes: 33 additions & 15 deletions packages/app/src/hooks/useMetadata.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -577,12 +577,22 @@ export function useMultipleGetKeyValues(
const source = chartConfig.source
? sources?.find(s => s.id === chartConfig.source)
: undefined;
const isLogOrTrace =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 minor — Re-derives the log/trace source fields that tcFromSource already provides

tcFromSource in packages/common-utils/src/core/metadata.ts:3542 already returns metadataMVs and mapKeyIndexMaxParts behind the same isLogSource || isTraceSource guard. Use tcFromSource(source) here, as DBSearchPageFilters/hooks.ts does through tableConnection, so there is one place that decides which source fields reach the metadata lookups.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

!!source && (isLogSource(source) || isTraceSource(source));
const metadataMVs =
usableMVsOverride ??
(source && (isLogSource(source) || isTraceSource(source))
? source.metadataMaterializedViews
: undefined);
return { chartConfig, source, metadataMVs };
(isLogOrTrace ? source.metadataMaterializedViews : undefined);
return {
chartConfig,
source,
metadataMVs,
mapKeyIndexMaxParts: isLogOrTrace

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 minor — Values query key leaves out the source's mapKeyIndexMaxParts/PartSelection, so a settings change serves stale values

useMultipleGetKeyValues now reads source.mapKeyIndexMaxParts/mapKeyIndexPartSelection inside streamFactory, but its queryKey (lines 534-546) contains only the chart configs, keys and limits. If a user raises "Map key index parts" because values are missing, the filter panel keeps the cached values for up to the 5-minute staleTime. Keys do refresh, because useMultipleAllFields keys on tcFromSource(...), which carries both settings (common-utils/src/core/metadata.ts:3590). Add the per-lookup mapKeyIndexMaxParts/mapKeyIndexPartSelection (and metadataMVs) to the query key.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

? source.mapKeyIndexMaxParts
: undefined,
mapKeyIndexPartSelection: isLogOrTrace
? source.mapKeyIndexPartSelection
: undefined,
};
});

if (mode === 'exact') {
Expand All @@ -606,18 +616,26 @@ export function useMultipleGetKeyValues(
}

// mode === 'all'
const streams = lookups.map(({ chartConfig, metadataMVs }) =>
metadata.streamAllKeyValues({
databaseName: chartConfig.from.databaseName,
tableName: chartConfig.from.tableName,
keyExpressions: limitedKeys,
maxValuesPerKey: limit,
connectionId: chartConfig.connection,
const streams = lookups.map(
({
chartConfig,
metadataMVs,
dateRange: chartConfig.dateRange,
timestampValueExpression: chartConfig.timestampValueExpression,
signal,
}),
mapKeyIndexMaxParts,
mapKeyIndexPartSelection,
}) =>
metadata.streamAllKeyValues({
databaseName: chartConfig.from.databaseName,
tableName: chartConfig.from.tableName,
keyExpressions: limitedKeys,
maxValuesPerKey: limit,
connectionId: chartConfig.connection,
metadataMVs,
mapKeyIndexMaxParts,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 minor — No test checks that the source's part settings reach streamAllKeyValues

The existing streamAllKeyValues assertions in packages/app/src/hooks/__tests__/useMetadata.test.tsx (around lines 338-467) check metadataMVs but not the two new fields. The new hooks.test assertion only covers the null load-more path. Add a case with a log source that sets mapKeyIndexMaxParts: 5, mapKeyIndexPartSelection: 'largest' and assert both are forwarded, and that they are undefined for a non-log/trace source. A dropped field would otherwise silently fall back to the default of 20.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

mapKeyIndexPartSelection,
dateRange: chartConfig.dateRange,
timestampValueExpression: chartConfig.timestampValueExpression,
signal,
}),
);
for await (const keyValues of mergeStreams(streams)) {
yield keyValues.map(({ key, value }) => ({
Expand Down
108 changes: 108 additions & 0 deletions packages/common-utils/src/__tests__/metadata.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1403,6 +1403,114 @@ describe('Metadata', () => {
expect(sql).toContain('GROUP BY column, key');
});

it.each([
['map KV', "LogAttributes['requestId']"],
['native', 'TraceId'],
])(
'reads the %s text index without a row cap, even when one is configured',
async (_, keyExpression) => {
setupDefaultLogsSchema();
jest
.spyOn(metadata, 'getClickHouseSettings')
.mockReturnValue({ max_rows_to_read: '1000' });

await metadata.getAllKeyValues({
...baseArgs,
keyExpressions: [keyExpression],
});

const calls = (mockClickhouseClient.query as jest.Mock).mock.calls;
const textIndexCall = calls.find(([opts]) =>
opts.query.includes('mergeTreeTextIndex('),
);
expect(textIndexCall[0].clickhouse_settings).toMatchObject({
max_rows_to_read: '0',
});
},
);

it.each([
['map KV', "LogAttributes['requestId']"],
['native', 'TraceId'],
])(
'reads only the parts nearest the median size from the %s text index',
async (_, keyExpression) => {
setupDefaultLogsSchema();

await metadata.getAllKeyValues({
...baseArgs,
keyExpressions: [keyExpression],
});
await metadata.getAllKeyValues({
...baseArgs,
keyExpressions: [keyExpression],
mapKeyIndexMaxParts: 5,
});

const partsLimits = (mockClickhouseClient.query as jest.Mock).mock.calls
.filter(([opts]) => opts.query.includes('mergeTreeTextIndex('))
.map(([opts]) => {
const match = opts.query.match(
/ORDER BY abs\(rn - \(cnt \+ 1\) \/ 2\), name\s+LIMIT \{(\w+):Int32\}/,
);
expect(match).not.toBeNull();
return opts.query_params[match[1]];
});
expect(partsLimits).toEqual([20, 5]);
},
);

it.each([
['map KV', "LogAttributes['requestId']"],
['native', 'TraceId'],
])(
'reads every part in the time range from the %s text index when the part limit is null',
async (_, keyExpression) => {
setupDefaultLogsSchema();

await metadata.getAllKeyValues({
...baseArgs,
keyExpressions: [keyExpression],
mapKeyIndexMaxParts: null,
});

const textIndexCall = (
mockClickhouseClient.query as jest.Mock
).mock.calls.find(([opts]) =>
opts.query.includes('mergeTreeTextIndex('),
);
expect(textIndexCall[0].query).toMatch(
/part_name IN \(SELECT name\s+FROM system\.parts/,
);
expect(textIndexCall[0].query).not.toContain('row_number()');
},
);

it.each([
['map KV', "LogAttributes['requestId']"],
['native', 'TraceId'],
])(
'orders the %s text index parts by the configured part selection',
async (_, keyExpression) => {
setupDefaultLogsSchema();

await metadata.getAllKeyValues({
...baseArgs,
keyExpressions: [keyExpression],
mapKeyIndexPartSelection: MapKeyIndexPartSelection.Largest,
});

const textIndexCall = (
mockClickhouseClient.query as jest.Mock
).mock.calls.find(([opts]) =>
opts.query.includes('mergeTreeTextIndex('),
);
expect(textIndexCall[0].query).toMatch(
/\)\s+ORDER BY rn DESC\s+LIMIT \{\w+:Int32\}/,
);
},
);

it('routes ServiceName and SeverityText through getMetadataMVKeyValues (KV rollup MV path)', async () => {
setupDefaultLogsSchema();

Expand Down
Loading
Loading