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 }, ); }