diff --git a/.changeset/rotten-squids-destroy.md b/.changeset/rotten-squids-destroy.md new file mode 100644 index 0000000000..553b83e469 --- /dev/null +++ b/.changeset/rotten-squids-destroy.md @@ -0,0 +1,7 @@ +--- +"@hyperdx/common-utils": minor +"@hyperdx/api": minor +"@hyperdx/app": minor +--- + +feat: use optimization settings if available for use in CH diff --git a/packages/common-utils/src/__tests__/clickhouse.test.ts b/packages/common-utils/src/__tests__/clickhouse.test.ts index 6136ffaec5..d8beade8f1 100644 --- a/packages/common-utils/src/__tests__/clickhouse.test.ts +++ b/packages/common-utils/src/__tests__/clickhouse.test.ts @@ -8,6 +8,8 @@ import { convertCHDataTypeToJSType, JSDataType, } from '@/clickhouse'; +import { ClickhouseClient } from '@/clickhouse/node'; +import { Metadata, MetadataCache } from '@/core/metadata'; describe('convertCHDataTypeToJSType - unit - type', () => { it('Date type', () => { @@ -354,3 +356,248 @@ describe('computeResultSetRatio', () => { ); }); }); + +describe('processClickhouseSettings - optimization settings', () => { + let client: ClickhouseClient; + let mockQueryMethod: jest.Mock; + let metadataCache: MetadataCache; + + const createClient = () => { + const newClient = new ClickhouseClient({ + host: 'http://localhost:8123', + username: 'default', + password: '', + }); + + // Mock the underlying ClickHouse client's query method + const newMockQueryMethod = jest.fn(); + (newClient as any).client = { + query: newMockQueryMethod, + }; + + // Create a fresh metadata cache for each test + const newCache = new MetadataCache(); + + // Mock getMetadata to return a metadata instance with our fresh cache + jest + // eslint-disable-next-line + .spyOn(require('@/core/metadata'), 'getMetadata') + .mockImplementation(() => { + return new Metadata(newClient, newCache); + }); + + return { + client: newClient, + mockQueryMethod: newMockQueryMethod, + cache: newCache, + }; + }; + + beforeEach(() => { + const setup = createClient(); + client = setup.client; + mockQueryMethod = setup.mockQueryMethod; + metadataCache = setup.cache; + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + const setupMockQuery = ( + settingsData?: Array<{ name: string; value: string }>, + ) => { + mockQueryMethod.mockImplementation( + async ({ query, clickhouse_settings }: any) => { + // Return mocked settings for getSettings query + if (query === 'SELECT name, value FROM system.settings') { + return { + json: async () => ({ + data: settingsData || [], + }), + }; + } + // Return the settings so we can inspect them + return { + json: async () => ({ data: [], clickhouse_settings }), + }; + }, + ); + }; + + it('should apply default settings without server settings', async () => { + setupMockQuery([]); + + await client.query({ + query: 'SELECT 1', + format: 'JSON', + }); + + // Find the actual query call (not the settings query) + const actualQueryCall = mockQueryMethod.mock.calls.find( + (call: any) => call[0].query === 'SELECT 1', + ); + + expect(actualQueryCall).toBeDefined(); + expect(actualQueryCall[0].clickhouse_settings).toEqual({ + allow_experimental_analyzer: 1, + date_time_output_format: 'iso', + wait_end_of_query: 0, + cancel_http_readonly_queries_on_client_close: 1, + }); + }); + + it('should apply all optimization settings when available on server', async () => { + setupMockQuery([ + { name: 'query_plan_optimize_lazy_materialization', value: '1' }, + { + name: 'query_plan_max_limit_for_lazy_materialization', + value: '100000', + }, + { name: 'use_skip_indexes_for_top_k', value: '1' }, + { name: 'query_plan_max_limit_for_top_k_optimization', value: '100000' }, + { name: 'use_top_k_dynamic_filtering', value: '1' }, + { name: 'use_skip_indexes_on_data_read', value: '1' }, + { name: 'use_skip_indexes_for_disjunctions', value: '1' }, + ]); + + await client.query({ + query: 'SELECT 1', + format: 'JSON', + }); + + const actualQueryCall = mockQueryMethod.mock.calls.find( + (call: any) => call[0].query === 'SELECT 1', + ); + + expect(actualQueryCall).toBeDefined(); + expect(actualQueryCall[0].clickhouse_settings).toEqual({ + allow_experimental_analyzer: 1, + date_time_output_format: 'iso', + wait_end_of_query: 0, + cancel_http_readonly_queries_on_client_close: 1, + query_plan_optimize_lazy_materialization: '1', + query_plan_max_limit_for_lazy_materialization: '100000', + use_skip_indexes_for_top_k: '1', + query_plan_max_limit_for_top_k_optimization: '100000', + use_top_k_dynamic_filtering: '1', + use_skip_indexes_on_data_read: '1', + use_skip_indexes_for_disjunctions: '1', + }); + }); + + it('should only apply available optimization settings', async () => { + setupMockQuery([ + { name: 'use_skip_indexes_for_top_k', value: '1' }, + { name: 'use_skip_indexes_on_data_read', value: '1' }, + ]); + + await client.query({ + query: 'SELECT 1', + format: 'JSON', + }); + + const actualQueryCall = mockQueryMethod.mock.calls.find( + (call: any) => call[0].query === 'SELECT 1', + ); + + expect(actualQueryCall).toBeDefined(); + const settings = actualQueryCall[0].clickhouse_settings; + expect(settings).toEqual({ + allow_experimental_analyzer: 1, + date_time_output_format: 'iso', + wait_end_of_query: 0, + cancel_http_readonly_queries_on_client_close: 1, + use_skip_indexes_for_top_k: '1', + use_skip_indexes_on_data_read: '1', + }); + expect(settings.query_plan_optimize_lazy_materialization).toBeUndefined(); + expect(settings.use_top_k_dynamic_filtering).toBeUndefined(); + }); + + it('should merge external clickhouse settings with optimization settings', async () => { + setupMockQuery([{ name: 'use_skip_indexes_for_top_k', value: '1' }]); + + await client.query({ + query: 'SELECT 1', + format: 'JSON', + clickhouse_settings: { + max_rows_to_read: '1000000', + }, + }); + + const actualQueryCall = mockQueryMethod.mock.calls.find( + (call: any) => call[0].query === 'SELECT 1', + ); + + expect(actualQueryCall).toBeDefined(); + expect(actualQueryCall[0].clickhouse_settings).toEqual({ + allow_experimental_analyzer: 1, + date_time_output_format: 'iso', + wait_end_of_query: 0, + cancel_http_readonly_queries_on_client_close: 1, + use_skip_indexes_for_top_k: '1', + max_rows_to_read: '1000000', + }); + }); + + it('should not apply settings when shouldSkipApplySettings is true', async () => { + setupMockQuery([{ name: 'use_skip_indexes_for_top_k', value: '1' }]); + + await client.query({ + query: 'SELECT name, value FROM system.settings', + format: 'JSON', + shouldSkipApplySettings: true, + }); + + const settingsQueryCall = mockQueryMethod.mock.calls.find( + (call: any) => + call[0].query === 'SELECT name, value FROM system.settings', + ); + + expect(settingsQueryCall).toBeDefined(); + expect(settingsQueryCall[0].clickhouse_settings).toBeUndefined(); + }); + + it('should handle metadata getSettings returning undefined (permissions error)', async () => { + mockQueryMethod.mockImplementation(async ({ query }: any) => { + if (query === 'SELECT name, value FROM system.settings') { + throw new Error('Not enough privileges'); + } + return { json: async () => ({ data: [] }) }; + }); + + // Should not throw, but silently continue without optimization settings + await client.query({ + query: 'SELECT 1', + format: 'JSON', + }); + + const actualQueryCall = mockQueryMethod.mock.calls.find( + (call: any) => call[0].query === 'SELECT 1', + ); + + expect(actualQueryCall).toBeDefined(); + expect(actualQueryCall[0].clickhouse_settings).toEqual({ + allow_experimental_analyzer: 1, + date_time_output_format: 'iso', + wait_end_of_query: 0, + cancel_http_readonly_queries_on_client_close: 1, + }); + }); + + it('should cache settings result across multiple queries', async () => { + setupMockQuery([{ name: 'use_skip_indexes_for_top_k', value: '1' }]); + + // Run two queries + await client.query({ query: 'SELECT 1', format: 'JSON' }); + await client.query({ query: 'SELECT 2', format: 'JSON' }); + + // Should only fetch settings once + const settingsCalls = mockQueryMethod.mock.calls.filter( + (call: any) => + call[0].query === 'SELECT name, value FROM system.settings', + ); + expect(settingsCalls.length).toEqual(1); + }); +}); diff --git a/packages/common-utils/src/clickhouse/browser.ts b/packages/common-utils/src/clickhouse/browser.ts index 36d741e143..f36348eded 100644 --- a/packages/common-utils/src/clickhouse/browser.ts +++ b/packages/common-utils/src/clickhouse/browser.ts @@ -112,6 +112,7 @@ export class ClickhouseClient extends BaseClickhouseClient { clickhouse_settings: externalClickhouseSettings, connectionId, queryId, + shouldSkipApplySettings, }: QueryInputs): Promise> { // FIXME: we couldn't initialize the client in the constructor // since the window is not avalible @@ -121,9 +122,14 @@ export class ClickhouseClient extends BaseClickhouseClient { this.logDebugQuery(query, query_params); - const clickhouseSettings = this.processClickhouseSettings( - externalClickhouseSettings, - ); + let clickhouseSettings: ClickHouseSettings | undefined; + // If this is the settings query, we must not process the clickhouse settings, or else we will infinitely recurse + if (!shouldSkipApplySettings) { + clickhouseSettings = await this.processClickhouseSettings({ + connectionId, + externalClickhouseSettings, + }); + } const httpHeaders: { [header: string]: string } = { ...(connectionId && connectionId !== 'local' diff --git a/packages/common-utils/src/clickhouse/index.ts b/packages/common-utils/src/clickhouse/index.ts index 8e00e74fa4..bfb14463ca 100644 --- a/packages/common-utils/src/clickhouse/index.ts +++ b/packages/common-utils/src/clickhouse/index.ts @@ -11,7 +11,7 @@ import type { ClickHouseClient as WebClickHouseClient } from '@clickhouse/client import * as SQLParser from 'node-sql-parser'; import objectHash from 'object-hash'; -import { Metadata } from '@/core/metadata'; +import { getMetadata, Metadata } from '@/core/metadata'; import { renderChartConfig, setChartSelectsAlias, @@ -422,6 +422,7 @@ export interface QueryInputs { clickhouse_settings?: ClickHouseSettings; connectionId?: string; queryId?: string; + shouldSkipApplySettings?: boolean; } export type ClickhouseClientOptions = { @@ -496,11 +497,15 @@ export abstract class BaseClickhouseClient { console.debug('--------------------------------------------------------'); } - protected processClickhouseSettings( - external_clickhouse_settings?: ClickHouseSettings, - ): ClickHouseSettings { + protected async processClickhouseSettings({ + connectionId, + externalClickhouseSettings, + }: { + connectionId?: string; + externalClickhouseSettings?: ClickHouseSettings; + }): Promise { const clickhouse_settings = structuredClone( - external_clickhouse_settings || {}, + externalClickhouseSettings || {}, ); if (clickhouse_settings?.max_rows_to_read && this.maxRowReadOnly) { delete clickhouse_settings['max_rows_to_read']; @@ -512,11 +517,43 @@ export abstract class BaseClickhouseClient { clickhouse_settings.max_execution_time = this.queryTimeout; } - return { + const defaultSettings: ClickHouseSettings = { allow_experimental_analyzer: 1, date_time_output_format: 'iso', wait_end_of_query: 0, cancel_http_readonly_queries_on_client_close: 1, + }; + + const metadata = getMetadata(this); + const serverSettings = await metadata.getSettings({ connectionId }); + + const applySettingIfAvailable = (name: string, value: string) => { + if (!serverSettings || !serverSettings.has(name)) return; + // eslint-disable-next-line security/detect-object-injection + defaultSettings[name] = value; + }; + + // Enables lazy materialization up to the given LIMIT + applySettingIfAvailable('query_plan_optimize_lazy_materialization', '1'); + applySettingIfAvailable( + 'query_plan_max_limit_for_lazy_materialization', + '100000', + ); + // Enables skip indexes to be used for top k style queries up to the given LIMIT + applySettingIfAvailable('use_skip_indexes_for_top_k', '1'); + applySettingIfAvailable( + 'query_plan_max_limit_for_top_k_optimization', + '100000', + ); + applySettingIfAvailable('use_top_k_dynamic_filtering', '1'); + // Enables skip indexes to be used on data read + applySettingIfAvailable('use_skip_indexes_on_data_read', '1'); + // Evaluate WHERE filters with mixed AND and OR conditions using skip indexes. + // If value is 0, then skip indicies only used on AND queries + applySettingIfAvailable('use_skip_indexes_for_disjunctions', '1'); + + return { + ...defaultSettings, ...clickhouse_settings, }; } diff --git a/packages/common-utils/src/clickhouse/node.ts b/packages/common-utils/src/clickhouse/node.ts index 116789e1e5..834496ac27 100644 --- a/packages/common-utils/src/clickhouse/node.ts +++ b/packages/common-utils/src/clickhouse/node.ts @@ -1,5 +1,9 @@ import { createClient } from '@clickhouse/client'; -import type { BaseResultSet, DataFormat } from '@clickhouse/client-common'; +import type { + BaseResultSet, + ClickHouseSettings, + DataFormat, +} from '@clickhouse/client-common'; import { BaseClickhouseClient, @@ -31,12 +35,17 @@ export class ClickhouseClient extends BaseClickhouseClient { abort_signal, clickhouse_settings: externalClickhouseSettings, queryId, + shouldSkipApplySettings, }: QueryInputs): Promise> { this.logDebugQuery(query, query_params); - const clickhouseSettings = this.processClickhouseSettings( - externalClickhouseSettings, - ); + let clickhouseSettings: ClickHouseSettings | undefined; + // If this is the settings query, we must not process the clickhouse settings, or else we will infinitely recurse + if (!shouldSkipApplySettings) { + clickhouseSettings = await this.processClickhouseSettings({ + externalClickhouseSettings, + }); + } // TODO: Custom error handling // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- client library type mismatch diff --git a/packages/common-utils/src/core/metadata.ts b/packages/common-utils/src/core/metadata.ts index fd61080593..6a23936142 100644 --- a/packages/common-utils/src/core/metadata.ts +++ b/packages/common-utils/src/core/metadata.ts @@ -13,12 +13,7 @@ import { tableExpr, } from '@/clickhouse'; import { renderChartConfig } from '@/core/renderChartConfig'; -import type { - ChartConfig, - ChartConfigWithDateRange, - QuerySettings, - TSource, -} from '@/types'; +import type { ChartConfig, ChartConfigWithDateRange, TSource } from '@/types'; import { optimizeGetKeyValuesCalls } from './materializedViews'; import { objectHash } from './utils'; @@ -660,6 +655,42 @@ export class Metadata { }); } + async getSettings({ connectionId }: { connectionId?: string }) { + return this.cache.getOrFetch( + `${connectionId}.availableSettings`, + async () => { + const query = 'SELECT name, value FROM system.settings'; + try { + const json = await this.clickhouseClient + .query<'JSON'>({ + connectionId, + query, + query_params: undefined, + clickhouse_settings: this.getClickHouseSettings(), + shouldSkipApplySettings: true, + }) + .then(res => res.json<{ name: string; value: string }>()); + + return new Map(json.data.map(row => [row.name, row.value])); + } catch (e) { + // Don't retry permissions errors, just silently return undefined + if ( + e instanceof Error && + e.message.includes('Not enough privileges') + ) { + console.warn( + 'Not enough privileges to fetch settings, may result in unoptimized queries:', + e, + ); + return new Map(); + } + + throw e; + } + }, + ); + } + /** * Queries system.data_skipping_indices to retrieve skip index metadata for a table. * Results are cached using MetadataCache.