From b732027be99a4a63ca2380a07eae925810be4e41 Mon Sep 17 00:00:00 2001 From: Bilal Atique Date: Wed, 28 Oct 2026 12:19:40 +0500 Subject: [PATCH] fix(common-utils): expire cached table columns after 5 minutes The module-level MetadataCache kept every DESCRIBE result for the life of the process. After an ALTER TABLE, the API kept the old column list, so it ignored new MATERIALIZED columns and failed to group by them until it restarted. getOrFetch takes an optional ttlMs. getColumns uses a 5 minute TTL, so getMaterializedColumnsLookupTable and the computed metric column lookup see a new column within 5 minutes. Entries without a TTL keep their current behavior. Closes #3475 --- .changeset/metadata-columns-cache-ttl.md | 12 +++ .../src/__tests__/metadata.test.ts | 100 ++++++++++++++++++ packages/common-utils/src/core/metadata.ts | 33 ++++-- 3 files changed, 138 insertions(+), 7 deletions(-) create mode 100644 .changeset/metadata-columns-cache-ttl.md diff --git a/.changeset/metadata-columns-cache-ttl.md b/.changeset/metadata-columns-cache-ttl.md new file mode 100644 index 0000000000..4a8b98db6a --- /dev/null +++ b/.changeset/metadata-columns-cache-ttl.md @@ -0,0 +1,12 @@ +--- +'@hyperdx/common-utils': patch +'@hyperdx/app': patch +'@hyperdx/api': patch +--- + +fix: refresh cached table columns after 5 minutes + +The shared metadata cache kept each table's `DESCRIBE` result for the life of +the process, so the API did not see columns added with `ALTER TABLE`, such as +new MATERIALIZED columns, until it restarted. Cached columns now expire after 5 +minutes. Other cached metadata is unchanged. diff --git a/packages/common-utils/src/__tests__/metadata.test.ts b/packages/common-utils/src/__tests__/metadata.test.ts index 1489354d2e..dab2349b0b 100644 --- a/packages/common-utils/src/__tests__/metadata.test.ts +++ b/packages/common-utils/src/__tests__/metadata.test.ts @@ -1,6 +1,7 @@ import { ColumnMeta } from '@/clickhouse'; import { ClickhouseClient } from '@/clickhouse/node'; import { + COLUMNS_CACHE_TTL_MS, GET_ALL_KEY_VALUES_CHUNK_SIZE, Metadata, MetadataCache, @@ -157,6 +158,48 @@ describe('MetadataCache', () => { // And no value should be stored in the cache expect(metadataCache.get(key)).toBeUndefined(); }); + + it('refetches a value stored with a TTL once the TTL has passed', async () => { + const now = jest.spyOn(Date, 'now').mockReturnValue(1_000); + const queryFn = jest + .fn() + .mockResolvedValueOnce('first') + .mockResolvedValueOnce('second'); + + try { + await expect( + metadataCache.getOrFetch('key', queryFn, { ttlMs: 100 }), + ).resolves.toBe('first'); + + now.mockReturnValue(1_099); + await expect( + metadataCache.getOrFetch('key', queryFn, { ttlMs: 100 }), + ).resolves.toBe('first'); + + now.mockReturnValue(1_100); + await expect( + metadataCache.getOrFetch('key', queryFn, { ttlMs: 100 }), + ).resolves.toBe('second'); + expect(queryFn).toHaveBeenCalledTimes(2); + } finally { + now.mockRestore(); + } + }); + + it('keeps a value stored without a TTL', async () => { + const now = jest.spyOn(Date, 'now').mockReturnValue(1_000); + const queryFn = jest.fn().mockResolvedValue('value'); + + try { + await metadataCache.getOrFetch('key', queryFn); + now.mockReturnValue(1_000 + 365 * 24 * 60 * 60 * 1000); + await metadataCache.getOrFetch('key', queryFn); + + expect(queryFn).toHaveBeenCalledTimes(1); + } finally { + now.mockRestore(); + } + }); }); }); @@ -168,6 +211,63 @@ describe('Metadata', () => { jest.clearAllMocks(); }); + describe('getMaterializedColumnsLookupTable', () => { + it('sees a MATERIALIZED column added after the first read once the cached columns expire', async () => { + const now = jest.spyOn(Date, 'now').mockReturnValue(0); + const client = { query: jest.fn() } as unknown as ClickhouseClient; + const md = new Metadata(client, new MetadataCache()); + const describeResult = (columns: Partial[]) => ({ + json: jest.fn().mockResolvedValue({ data: columns }), + }); + const resourceAttributes = { + name: 'ResourceAttributes', + type: 'Map(LowCardinality(String), String)', + default_type: '', + default_expression: '', + }; + const materialized = { + name: '__otel_materialized_k8s.namespace.name', + type: 'LowCardinality(String)', + default_type: 'MATERIALIZED', + default_expression: "ResourceAttributes['k8s.namespace.name']", + }; + (client.query as jest.Mock) + .mockResolvedValueOnce(describeResult([resourceAttributes])) + .mockResolvedValueOnce( + describeResult([resourceAttributes, materialized]), + ); + const table = { + databaseName: 'otel', + tableName: 'otel_metrics_gauge', + connectionId: 'conn-1', + }; + + try { + expect(await md.getMaterializedColumnsLookupTable(table)).toEqual( + new Map(), + ); + + now.mockReturnValue(COLUMNS_CACHE_TTL_MS - 1); + expect(await md.getMaterializedColumnsLookupTable(table)).toEqual( + new Map(), + ); + + now.mockReturnValue(COLUMNS_CACHE_TTL_MS); + expect(await md.getMaterializedColumnsLookupTable(table)).toEqual( + new Map([ + [ + "ResourceAttributes['k8s.namespace.name']", + '__otel_materialized_k8s.namespace.name', + ], + ]), + ); + expect(client.query).toHaveBeenCalledTimes(2); + } finally { + now.mockRestore(); + } + }); + }); + describe('getTableMetadata', () => { beforeEach(() => { mockCache.getOrFetch.mockImplementation((key, queryFn) => queryFn()); diff --git a/packages/common-utils/src/core/metadata.ts b/packages/common-utils/src/core/metadata.ts index c6a5f64311..7471296951 100644 --- a/packages/common-utils/src/core/metadata.ts +++ b/packages/common-utils/src/core/metadata.ts @@ -251,17 +251,27 @@ const renderJsonStringSubcolumn = ( }; export class MetadataCache { - private cache = new Map(); + private cache = new Map(); private pendingQueries = new Map>(); // this should be getOrUpdate... or just query to follow react query get(key: string): T | undefined { - return this.cache.get(key); + const entry = this.cache.get(key); + if (entry?.expiresAt != null && Date.now() >= entry.expiresAt) { + this.cache.delete(key); + return undefined; + } + return entry?.value; } - async getOrFetch(key: string, query: () => Promise): Promise { + /** Entries stored without `ttlMs` are kept for the life of the cache. */ + async getOrFetch( + key: string, + query: () => Promise, + { ttlMs }: { ttlMs?: number } = {}, + ): Promise { // Check if value exists in cache - const cachedValue: T | undefined = this.cache.get(key); + const cachedValue: T | undefined = this.get(key); if (cachedValue != null) { return cachedValue; } @@ -279,7 +289,7 @@ export class MetadataCache { try { const result = await queryPromise; - this.cache.set(key, result); + this.set(key, result, ttlMs); return result; } finally { // Clean up the pending query map @@ -287,8 +297,11 @@ export class MetadataCache { } } - set(key: string, value: T) { - return this.cache.set(key, value); + set(key: string, value: T, ttlMs?: number) { + this.cache.set(key, { + value, + expiresAt: ttlMs == null ? undefined : Date.now() + ttlMs, + }); } // TODO: This needs to be async, and use tanstack query on frontend for cache @@ -353,6 +366,11 @@ export type SkipIndexMetadata = { const FIELD_METADATA_LOOKBACK_MS = 24 * 60 * 60 * 1000; +// The API keeps one cache for the life of the process, so without a TTL a +// column added by ALTER TABLE (such as a MATERIALIZED one) is only seen after a +// restart. #3475 +export const COLUMNS_CACHE_TTL_MS = 5 * 60 * 1000; + // Hour-aligned so a range-keyed cache rotates hourly instead of every call. export function defaultFieldMetadataDateRange(): [Date, Date] { const now = new Date(); @@ -619,6 +637,7 @@ export class Metadata { .then(d => d.data); return columns; }, + { ttlMs: COLUMNS_CACHE_TTL_MS }, ); }