perf: share one Map key read between concurrent callers

.changeset/dedupe-map-keys-reads.md

@@ -0,0 +1,10 @@
+---
+'@hyperdx/common-utils': patch
+---
+
+perf: share one Map key read between concurrent callers
+
+The search page's filter panel, search bar and group-by each load the same Map
+keys when the page opens. Each sent its own text index read, which on large
+tables can use several GiB of memory. Concurrent requests for the same keys now
+share one query.

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

@@ -3146,6 +3146,39 @@ describe('Metadata', () => {
 
       expect(mockClickhouseClient.query).toHaveBeenCalledTimes(2);
     });
+
+    it('shares one scan between concurrent callers when no text index or rollup applies', async () => {
+      const md = buildMetadata();
+      jest.spyOn(console, 'warn').mockImplementation(() => {});
+      jest
+        .spyOn(md, 'getMapColumnTextIndexes')
+        .mockResolvedValue(new Map() as TextIndexInfoLookup);
+      jest.spyOn(md, 'getColumn').mockResolvedValue({
+        name: 'LogAttributes',
+        type: 'Map(String, String)',
+        default_type: '',
+        default_expression: '',
+        comment: '',
+        codec_expression: '',
+        ttl_expression: '',
+      });
+      (mockClickhouseClient.query as jest.Mock).mockResolvedValue({
+        json: () => Promise.resolve({ data: [{ keysArr: ['raw.key'] }] }),
+      });
+
+      const {
+        timestampValueExpression: _timestampValueExpression,
+        metadataMVs: _metadataMVs,
+        ...scanArgs
+      } = baseArgs;
+      const results = await Promise.all([
+        md.getMapKeys(scanArgs),
+        md.getMapKeys(scanArgs),
+      ]);
+
+      expect(results).toEqual([['raw.key'], ['raw.key']]);
+      expect(mockClickhouseClient.query).toHaveBeenCalledTimes(1);
+    });
   });
 
   describe('getMapKeys (text index paths)', () => {
@@ -3224,6 +3257,39 @@ describe('Metadata', () => {
         );
       },
     );
+
+    it.each(textIndexLookups)(
+      'shares one read between concurrent callers on the %s text index path',
+      async (_, textIndexLookup) => {
+        const md = new Metadata(mockClickhouseClient, new MetadataCache());
+        jest.spyOn(md, 'getServerVersion').mockResolvedValue([26, 3, 0, 0]);
+        jest
+          .spyOn(md, 'getMapColumnTextIndexes')
+          .mockResolvedValue(textIndexLookup);
+        (mockClickhouseClient.query as jest.Mock).mockResolvedValue({
+          json: () => Promise.resolve({ data: [{ key: 'http.method' }] }),
+        });
+        const args = {
+          databaseName: 'default',
+          tableName: 'otel_logs',
+          column: 'LogAttributes',
+          connectionId: 'test_connection',
+          dateRange: [
+            new Date('2024-01-01T00:00:00Z'),
+            new Date('2024-01-01T01:00:00Z'),
+          ] as [Date, Date],
+          timestampValueExpression: 'Timestamp',
+        };
+
+        const results = await Promise.all([
+          md.getMapKeys(args),
+          md.getMapKeys(args),
+        ]);
+
+        expect(results).toEqual([['http.method'], ['http.method']]);
+        expect(mockClickhouseClient.query).toHaveBeenCalledTimes(1);
+      },
+    );
   });
 
   describe('getMapValues', () => {

packages/common-utils/src/core/metadata.ts

@@ -1117,12 +1117,62 @@ export class Metadata {
     const cacheKey = conditionSql
       ? `${unscopedCacheKey}.${objectHash(conditionSql)}.condition`
       : unscopedCacheKey;
-    const cachedKeys = this.cache.get<string[]>(cacheKey);
 
-    if (cachedKeys != null) {
-      return cachedKeys;
-    }
+    // getOrFetch shares one in-flight read between concurrent callers (the
+    // filter panel, search bar and group-by all load fields at once); a text
+    // index read can take GiBs of memory. A condition changes with variable
+    // selections, so its entries expire instead of piling up.
+    return this.cache.getOrFetch<string[]>(
+      cacheKey,
+      () =>
+        this.queryMapKeys({
+          databaseName,
+          tableName,
+          column,
+          maxKeys,
+          connectionId,
+          metricName,
+          metadataMVs,
+          dateRange,
+          alignedDateRange,
+          timestampValueExpression,
+          conditionSql,
+          cacheKey,
+          signal,
+        }),
+      { ttlMs: conditionSql ? CONDITION_KEYS_CACHE_TTL_MS : undefined },
+    );
+  }
 
+  private async queryMapKeys({
+    databaseName,
+    tableName,
+    column,
+    maxKeys,
+    connectionId,
+    metricName,
+    metadataMVs,
+    dateRange,
+    alignedDateRange,
+    timestampValueExpression,
+    conditionSql,
+    cacheKey,
+    signal,
+  }: {
+    databaseName: string;
+    tableName: string;
+    column: string;
+    maxKeys: number;
+    connectionId: string;
+    metricName?: string;
+    metadataMVs?: MetadataMaterializedViews;
+    dateRange: [Date, Date];
+    alignedDateRange?: [Date, Date];
+    timestampValueExpression?: string;
+    conditionSql?: string;
+    cacheKey: string;
+    signal?: AbortSignal;
+  }): Promise<string[]> {
     const textIndexInfoLookup = await this.getMapColumnTextIndexes({
       databaseName,
       tableName,
@@ -1176,10 +1226,7 @@ export class Metadata {
           })
           .then(r => r.json<{ key: string }>())
           .then(d => d.data.map(r => r.key).filter(Boolean));
-        if (keys.length > 0) {
-          this.cache.set(cacheKey, keys);
-          return keys;
-        }
+        if (keys.length > 0) return keys;
       } catch (e) {
         // The caller gave up; don't start the fallback with a dead signal and
         // cache its instant rejection under the rollup key.
@@ -1225,10 +1272,7 @@ export class Metadata {
           })
           .then(r => r.json<{ key: string }>())
           .then(d => d.data.map(r => r.key).filter(Boolean));
-        if (keys.length > 0) {
-          this.cache.set(cacheKey, keys);
-          return keys;
-        }
+        if (keys.length > 0) return keys;
       } catch (e) {
         // See the key text index catch above.
         if (signal?.aborted) throw e;
@@ -1403,42 +1447,34 @@ export class Metadata {
       `;
     }
 
-    const fetchKeys = async () => {
-      const keys = await this.clickhouseClient
-        .query<'JSON'>({
-          query: sql.sql,
-          query_params: sql.params,
-          connectionId,
-          clickhouse_settings: {
-            ...this.getClickHouseSettings(),
-            // Max 15 seconds to get keys
-            timeout_overflow_mode: 'break',
-            max_execution_time: 15,
-            // Set the value to 0 (unlimited) so that the LIMIT is used instead
-            max_rows_to_read: '0',
-          },
-          abort_signal: signal,
-        })
-        .then(res => res.json<{ keysArr?: string[]; key?: string }>())
-        .then(d => {
-          let output: string[];
-          if (strategy === 'groupUniqArrayArray') {
-            output = d.data[0].keysArr ?? [];
-          } else {
-            output = d.data
-              .map(row => row.key)
-              .filter((k): k is string => Boolean(k));
-          }
+    return this.clickhouseClient
+      .query<'JSON'>({
+        query: sql.sql,
+        query_params: sql.params,
+        connectionId,
+        clickhouse_settings: {
+          ...this.getClickHouseSettings(),
+          // Max 15 seconds to get keys
+          timeout_overflow_mode: 'break',
+          max_execution_time: 15,
+          // Set the value to 0 (unlimited) so that the LIMIT is used instead
+          max_rows_to_read: '0',
+        },
+        abort_signal: signal,
+      })
+      .then(res => res.json<{ keysArr?: string[]; key?: string }>())
+      .then(d => {
+        let output: string[];
+        if (strategy === 'groupUniqArrayArray') {
+          output = d.data[0].keysArr ?? [];
+        } else {
+          output = d.data
+            .map(row => row.key)
+            .filter((k): k is string => Boolean(k));
+        }
 
-          return output.filter(r => r);
-        });
-      return keys;
-    };
-    // A condition changes with variable selections; expire its entries so
-    // they don't pile up.
-    return this.cache.getOrFetch<string[]>(cacheKey, fetchKeys, {
-      ttlMs: conditionSql ? CONDITION_KEYS_CACHE_TTL_MS : undefined,
-    });
+        return output.filter(r => r);
+      });
   }
 
   async getJSONKeys({