diff --git a/.changeset/shared-merge-tree-settings.md b/.changeset/shared-merge-tree-settings.md new file mode 100644 index 0000000000..4c1e0e6932 --- /dev/null +++ b/.changeset/shared-merge-tree-settings.md @@ -0,0 +1,6 @@ +--- +'@hyperdx/app': patch +'@hyperdx/common-utils': patch +--- + +feat: on ClickHouse Cloud, read `system.shared_merge_tree_settings` when detecting `_block_number` and `_block_offset` columns, falling back to `system.merge_tree_settings` per key diff --git a/packages/common-utils/src/__tests__/metadata.test.ts b/packages/common-utils/src/__tests__/metadata.test.ts index 840aca7682..8198b1724a 100644 --- a/packages/common-utils/src/__tests__/metadata.test.ts +++ b/packages/common-utils/src/__tests__/metadata.test.ts @@ -547,15 +547,35 @@ describe('Metadata', () => { }); describe('getMergeTreeSettings', () => { - it('returns the server MergeTree settings by name', async () => { - mockCache.getOrFetch.mockImplementation((key, queryFn) => queryFn()); - (mockClickhouseClient.query as jest.Mock).mockResolvedValue({ - json: jest.fn().mockResolvedValue({ - data: [ - { name: 'enable_block_number_column', value: '1' }, - { name: 'enable_block_offset_column', value: '0' }, - ], + type SettingRow = { name: string; value: string }; + const mockSettingsTables = ({ + mergeTree, + sharedMergeTree = [], + }: { + mergeTree: SettingRow[]; + sharedMergeTree?: SettingRow[]; + }) => + (mockClickhouseClient.query as jest.Mock).mockImplementation( + async ({ query }: { query: string }) => ({ + json: async () => ({ + data: query.includes('shared_merge_tree_settings') + ? sharedMergeTree + : mergeTree, + }), }), + ); + + beforeEach(() => { + mockCache.getOrFetch.mockImplementation((key, queryFn) => queryFn()); + }); + + it('reads only merge_tree_settings outside Cloud', async () => { + jest.spyOn(metadata, 'isClickHouseCloud').mockResolvedValue(false); + mockSettingsTables({ + mergeTree: [ + { name: 'enable_block_number_column', value: '1' }, + { name: 'enable_block_offset_column', value: '0' }, + ], }); const result = await metadata.getMergeTreeSettings({ @@ -568,6 +588,7 @@ describe('Metadata', () => { ['enable_block_offset_column', '0'], ]), ); + expect(mockClickhouseClient.query).toHaveBeenCalledTimes(1); expect(mockClickhouseClient.query).toHaveBeenCalledWith( expect.objectContaining({ query: 'SELECT name, value FROM system.merge_tree_settings', @@ -575,11 +596,57 @@ describe('Metadata', () => { ); }); + it('prefers shared_merge_tree_settings per key on Cloud', async () => { + jest.spyOn(metadata, 'isClickHouseCloud').mockResolvedValue(true); + mockSettingsTables({ + mergeTree: [ + { name: 'enable_block_number_column', value: '0' }, + { name: 'enable_block_offset_column', value: '1' }, + ], + sharedMergeTree: [{ name: 'enable_block_number_column', value: '1' }], + }); + + const result = await metadata.getMergeTreeSettings({ + connectionId: 'test_connection', + }); + + expect(result).toEqual( + new Map([ + ['enable_block_number_column', '1'], + ['enable_block_offset_column', '1'], + ]), + ); + expect(mockClickhouseClient.query).toHaveBeenCalledWith( + expect.objectContaining({ + query: 'SELECT name, value FROM system.shared_merge_tree_settings', + }), + ); + }); + + it('returns undefined when the shared read fails on Cloud', async () => { + jest.spyOn(metadata, 'isClickHouseCloud').mockResolvedValue(true); + (mockClickhouseClient.query as jest.Mock).mockImplementation( + async ({ query }: { query: string }) => { + if (query.includes('shared_merge_tree_settings')) { + throw new Error('timeout'); + } + return { json: async () => ({ data: [] }) }; + }, + ); + + const result = await metadata.getMergeTreeSettings({ + connectionId: 'test_connection', + }); + + expect(result).toBeUndefined(); + }); + it('re-queries after a failure instead of caching it', async () => { const realMetadata = new Metadata( mockClickhouseClient, new MetadataCache(), ); + jest.spyOn(realMetadata, 'isClickHouseCloud').mockResolvedValue(false); (mockClickhouseClient.query as jest.Mock) .mockRejectedValueOnce(new Error('Not enough privileges')) .mockResolvedValueOnce({ diff --git a/packages/common-utils/src/core/metadata.ts b/packages/common-utils/src/core/metadata.ts index fc540588bf..9953e01a88 100644 --- a/packages/common-utils/src/core/metadata.ts +++ b/packages/common-utils/src/core/metadata.ts @@ -2242,28 +2242,44 @@ export class Metadata { } /** - * Reads and caches system.merge_tree_settings per connection. Returns - * undefined on failure so the next call retries. + * Reads and caches MergeTree settings per connection; on Cloud, + * system.shared_merge_tree_settings overrides system.merge_tree_settings per + * key. Returns undefined on failure so the next call retries. */ async getMergeTreeSettings({ connectionId, }: { connectionId: string; }): Promise | undefined> { + const readSettings = (query: string) => + this.clickhouseClient + .query<'JSON'>({ + connectionId, + query, + query_params: undefined, + clickhouse_settings: this.getClickHouseSettings(), + }) + .then(res => res.json<{ name: string; value: string }>()) + .then(json => json.data); + return this.cache.getOrFetch( `${connectionId}.mergeTreeSettings`, async () => { try { - const json = await this.clickhouseClient - .query<'JSON'>({ - connectionId, - query: 'SELECT name, value FROM system.merge_tree_settings', - query_params: undefined, - clickhouse_settings: this.getClickHouseSettings(), - }) - .then(res => res.json<{ name: string; value: string }>()); - - return new Map(json.data.map(row => [row.name, row.value])); + // OSS has no shared_merge_tree_settings; querying it fails. + const isCloud = await this.isClickHouseCloud({ connectionId }); + const [mergeTreeRows, sharedMergeTreeRows] = await Promise.all([ + readSettings('SELECT name, value FROM system.merge_tree_settings'), + isCloud + ? readSettings( + 'SELECT name, value FROM system.shared_merge_tree_settings', + ) + : [], + ]); + + // Later rows win. + const rows = [...mergeTreeRows, ...sharedMergeTreeRows]; + return new Map(rows.map(row => [row.name, row.value])); } catch (e) { console.warn('Error fetching MergeTree settings:', e); return undefined;