From 24e5e1a7dcc502b71f5cad7e6bbfc5e5c0fd2faf Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Wed, 15 Oct 2025 16:24:02 -0400 Subject: [PATCH] fix: Include connectionId in metadata cache key --- .changeset/mean-ghosts-leave.md | 5 ++ .../src/__tests__/metadata.test.ts | 4 +- packages/common-utils/src/metadata.ts | 86 +++++++++---------- 3 files changed, 50 insertions(+), 45 deletions(-) create mode 100644 .changeset/mean-ghosts-leave.md diff --git a/.changeset/mean-ghosts-leave.md b/.changeset/mean-ghosts-leave.md new file mode 100644 index 000000000..dbb0d4d98 --- /dev/null +++ b/.changeset/mean-ghosts-leave.md @@ -0,0 +1,5 @@ +--- +"@hyperdx/common-utils": patch +--- + +fix: Include connectionId in metadata cache key diff --git a/packages/common-utils/src/__tests__/metadata.test.ts b/packages/common-utils/src/__tests__/metadata.test.ts index 5f88b8cbd..7667391e6 100644 --- a/packages/common-utils/src/__tests__/metadata.test.ts +++ b/packages/common-utils/src/__tests__/metadata.test.ts @@ -198,7 +198,7 @@ describe('Metadata', () => { // Setup the cache to return the mock data mockCache.getOrFetch.mockImplementation((key, queryFn) => { - if (key === 'test_db.test_table.metadata') { + if (key === 'test_connection.test_db.test_table.metadata') { return Promise.resolve(mockTableMetadata); } return queryFn(); @@ -212,7 +212,7 @@ describe('Metadata', () => { // Verify the cache was called with the right key expect(mockCache.getOrFetch).toHaveBeenCalledWith( - 'test_db.test_table.metadata', + 'test_connection.test_db.test_table.metadata', expect.any(Function), ); diff --git a/packages/common-utils/src/metadata.ts b/packages/common-utils/src/metadata.ts index d52c08201..603853607 100644 --- a/packages/common-utils/src/metadata.ts +++ b/packages/common-utils/src/metadata.ts @@ -129,18 +129,21 @@ export class Metadata { cache: MetadataCache; connectionId: string; }) { - return cache.getOrFetch(`${database}.${table}.metadata`, async () => { - const sql = chSql`SELECT * FROM system.tables where database = ${{ String: database }} AND name = ${{ String: table }}`; - const json = await this.clickhouseClient - .query<'JSON'>({ - connectionId, - query: sql.sql, - query_params: sql.params, - clickhouse_settings: this.getClickHouseSettings(), - }) - .then(res => res.json()); - return json.data[0]; - }); + return cache.getOrFetch( + `${connectionId}.${database}.${table}.metadata`, + async () => { + const sql = chSql`SELECT * FROM system.tables where database = ${{ String: database }} AND name = ${{ String: table }}`; + const json = await this.clickhouseClient + .query<'JSON'>({ + connectionId, + query: sql.sql, + query_params: sql.params, + clickhouse_settings: this.getClickHouseSettings(), + }) + .then(res => res.json()); + return json.data[0]; + }, + ); } async getColumns({ @@ -153,7 +156,7 @@ export class Metadata { connectionId: string; }) { return this.cache.getOrFetch( - `${databaseName}.${tableName}.columns`, + `${connectionId}.${databaseName}.${tableName}.columns`, async () => { const sql = chSql`DESCRIBE ${tableExpr({ database: databaseName, table: tableName })}`; const columns = await this.clickhouseClient @@ -240,8 +243,8 @@ export class Metadata { metricName?: string; }) { const cacheKey = metricName - ? `${databaseName}.${tableName}.${column}.${metricName}.keys` - : `${databaseName}.${tableName}.${column}.keys`; + ? `${connectionId}.${databaseName}.${tableName}.${column}.${metricName}.keys` + : `${connectionId}.${databaseName}.${tableName}.${column}.keys`; const cachedKeys = this.cache.get(cacheKey); if (cachedKeys != null) { @@ -351,8 +354,8 @@ export class Metadata { // HDX-2480 delete line below to reenable json filters return []; // Need to disable JSON keys for the time being. const cacheKey = metricName - ? `${databaseName}.${tableName}.${column}.${metricName}.keys` - : `${databaseName}.${tableName}.${column}.keys`; + ? `${connectionId}.${databaseName}.${tableName}.${column}.${metricName}.keys` + : `${connectionId}.${databaseName}.${tableName}.${column}.keys`; return this.cache.getOrFetch<{ key: string; chType: string }[]>( cacheKey, @@ -420,9 +423,9 @@ export class Metadata { maxValues?: number; connectionId: string; }) { - const cachedValues = this.cache.get( - `${databaseName}.${tableName}.${column}.${key}.values`, - ); + const cacheKey = `${connectionId}.${databaseName}.${tableName}.${column}.${key}.values`; + + const cachedValues = this.cache.get(cacheKey); if (cachedValues != null) { return cachedValues; @@ -450,28 +453,25 @@ export class Metadata { }} `; - return this.cache.getOrFetch( - `${databaseName}.${tableName}.${column}.${key}.values`, - async () => { - const values = await this.clickhouseClient - .query<'JSON'>({ - query: sql.sql, - query_params: sql.params, - connectionId, - clickhouse_settings: { - max_rows_to_read: String( - this.getClickHouseSettings().max_rows_to_read ?? - DEFAULT_METADATA_MAX_ROWS_TO_READ, - ), - read_overflow_mode: 'break', - ...this.getClickHouseSettings(), - }, - }) - .then(res => res.json>()) - .then(d => d.data.map(row => row.value as string)); - return values; - }, - ); + return this.cache.getOrFetch(cacheKey, async () => { + const values = await this.clickhouseClient + .query<'JSON'>({ + query: sql.sql, + query_params: sql.params, + connectionId, + clickhouse_settings: { + max_rows_to_read: String( + this.getClickHouseSettings().max_rows_to_read ?? + DEFAULT_METADATA_MAX_ROWS_TO_READ, + ), + read_overflow_mode: 'break', + ...this.getClickHouseSettings(), + }, + }) + .then(res => res.json>()) + .then(d => d.data.map(row => row.value as string)); + return values; + }); } async getAllFields({ @@ -666,7 +666,7 @@ export class Metadata { disableRowLimit?: boolean; }) { return this.cache.getOrFetch( - `${chartConfig.from.databaseName}.${chartConfig.from.tableName}.${keys.join(',')}.${chartConfig.dateRange.toString()}.${disableRowLimit}.values`, + `${chartConfig.connection}.${chartConfig.from.databaseName}.${chartConfig.from.tableName}.${keys.join(',')}.${chartConfig.dateRange.toString()}.${disableRowLimit}.values`, async () => { const selectClause = keys .map((k, i) => `groupUniqArray(${limit})(${k}) AS param${i}`)