From 1b8ec4efa8011c8bdadef873e150ff30831834b2 Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Thu, 7 Oct 2021 09:51:45 +0200 Subject: [PATCH] [Search] Reuse `uiSettings` within `bsearch` request (#114088) --- .../data/server/search/routes/bsearch.ts | 2 +- .../data/server/search/search_service.ts | 5 +- .../cached_ui_settings_client.test.ts | 59 +++++++++++++++++++ .../services/cached_ui_settings_client.ts | 44 ++++++++++++++ .../data/server/search/services/index.ts | 9 +++ .../strategies/es_search/request_utils.ts | 2 +- .../strategies/ese_search/request_utils.ts | 4 +- src/plugins/data/server/search/types.ts | 2 +- 8 files changed, 121 insertions(+), 6 deletions(-) create mode 100644 src/plugins/data/server/search/services/cached_ui_settings_client.test.ts create mode 100644 src/plugins/data/server/search/services/cached_ui_settings_client.ts create mode 100644 src/plugins/data/server/search/services/index.ts diff --git a/src/plugins/data/server/search/routes/bsearch.ts b/src/plugins/data/server/search/routes/bsearch.ts index 43853aa0ea939..9a15f84687f43 100644 --- a/src/plugins/data/server/search/routes/bsearch.ts +++ b/src/plugins/data/server/search/routes/bsearch.ts @@ -25,13 +25,13 @@ export function registerBsearchRoute( { request: IKibanaSearchRequest; options?: ISearchOptionsSerializable }, IKibanaSearchResponse >('/internal/bsearch', (request) => { + const search = getScoped(request); return { /** * @param requestOptions * @throws `KibanaServerError` */ onBatchItem: async ({ request: requestData, options }) => { - const search = getScoped(request); const { executionContext, ...restOptions } = options || {}; return executionContextService.withContext(executionContext, () => diff --git a/src/plugins/data/server/search/search_service.ts b/src/plugins/data/server/search/search_service.ts index 26ecd7ebed500..fa7296c822467 100644 --- a/src/plugins/data/server/search/search_service.ts +++ b/src/plugins/data/server/search/search_service.ts @@ -89,6 +89,7 @@ import { getKibanaContext } from './expressions/kibana_context'; import { enhancedEsSearchStrategyProvider } from './strategies/ese_search'; import { eqlSearchStrategyProvider } from './strategies/eql_search'; import { NoSearchIdInSessionError } from './errors/no_search_id_in_session'; +import { CachedUiSettingsClient } from './services'; type StrategyMap = Record>; @@ -453,7 +454,9 @@ export class SearchService implements Plugin { searchSessionsClient, savedObjectsClient, esClient: elasticsearch.client.asScoped(request), - uiSettingsClient: uiSettings.asScopedToClient(savedObjectsClient), + uiSettingsClient: new CachedUiSettingsClient( + uiSettings.asScopedToClient(savedObjectsClient) + ), request, }; return { diff --git a/src/plugins/data/server/search/services/cached_ui_settings_client.test.ts b/src/plugins/data/server/search/services/cached_ui_settings_client.test.ts new file mode 100644 index 0000000000000..045e9d6d540ff --- /dev/null +++ b/src/plugins/data/server/search/services/cached_ui_settings_client.test.ts @@ -0,0 +1,59 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { coreMock, httpServerMock } from '../../../../../core/server/mocks'; + +import { CachedUiSettingsClient } from './cached_ui_settings_client'; + +test('fetching uiSettings once for the same key', async () => { + const request = httpServerMock.createKibanaRequest(); + const soClient = coreMock.createStart().savedObjects.getScopedClient(request); + const uiSettings = coreMock.createStart().uiSettings.asScopedToClient(soClient); + + const key = 'key'; + const value = 'value'; + const spy = jest + .spyOn(uiSettings, 'getAll') + .mockImplementation(() => Promise.resolve({ [key]: value })); + + const cachedUiSettings = new CachedUiSettingsClient(uiSettings); + + const res1 = cachedUiSettings.get(key); + const res2 = cachedUiSettings.get(key); + + expect(spy).toHaveBeenCalledTimes(1); // check that internally uiSettings.getAll() called only once + + expect(await res1).toBe(value); + expect(await res2).toBe(value); +}); + +test('fetching uiSettings once for different keys', async () => { + const request = httpServerMock.createKibanaRequest(); + const soClient = coreMock.createStart().savedObjects.getScopedClient(request); + const uiSettings = coreMock.createStart().uiSettings.asScopedToClient(soClient); + + const key1 = 'key1'; + const value1 = 'value1'; + + const key2 = 'key2'; + const value2 = 'value2'; + + const spy = jest + .spyOn(uiSettings, 'getAll') + .mockImplementation(() => Promise.resolve({ [key1]: value1, [key2]: value2 })); + + const cachedUiSettings = new CachedUiSettingsClient(uiSettings); + + const res1 = cachedUiSettings.get(key1); + const res2 = cachedUiSettings.get(key2); + + expect(spy).toHaveBeenCalledTimes(1); // check that internally uiSettings.getAll() called only once + + expect(await res1).toBe(value1); + expect(await res2).toBe(value2); +}); diff --git a/src/plugins/data/server/search/services/cached_ui_settings_client.ts b/src/plugins/data/server/search/services/cached_ui_settings_client.ts new file mode 100644 index 0000000000000..053f2e2c35f4b --- /dev/null +++ b/src/plugins/data/server/search/services/cached_ui_settings_client.ts @@ -0,0 +1,44 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { IUiSettingsClient } from 'kibana/server'; + +/** + * {@link IUiSettingsClient} wrapper to ensure uiSettings requested only once within a single KibanaRequest, + * {@link IUiSettingsClient} has its own cache, but it doesn't cache pending promises, so this produces two requests: + * + * const promise1 = uiSettings.get(1); // fetches config + * const promise2 = uiSettings.get(2); // fetches config + * + * And {@link CachedUiSettingsClient} solves it, so this produced a single request: + * + * const promise1 = cachedUiSettingsClient.get(1); // fetches config + * const promise2 = cachedUiSettingsClient.get(2); // reuses existing promise + * + * @internal + */ +export class CachedUiSettingsClient implements Pick { + private cache: Promise> | undefined; + + constructor(private readonly client: IUiSettingsClient) {} + + async get(key: string): Promise { + if (!this.cache) { + // caching getAll() instead of just get(key) because internally uiSettings calls `getAll()` anyways + // this way we reuse cache in case settings for different keys were requested + this.cache = this.client.getAll(); + } + + return this.cache + .then((cache) => cache[key] as T) + .catch((e) => { + this.cache = undefined; + throw e; + }); + } +} diff --git a/src/plugins/data/server/search/services/index.ts b/src/plugins/data/server/search/services/index.ts new file mode 100644 index 0000000000000..90f9bd083744d --- /dev/null +++ b/src/plugins/data/server/search/services/index.ts @@ -0,0 +1,9 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +export { CachedUiSettingsClient } from './cached_ui_settings_client'; diff --git a/src/plugins/data/server/search/strategies/es_search/request_utils.ts b/src/plugins/data/server/search/strategies/es_search/request_utils.ts index 829497929c20f..15cad34065ddc 100644 --- a/src/plugins/data/server/search/strategies/es_search/request_utils.ts +++ b/src/plugins/data/server/search/strategies/es_search/request_utils.ts @@ -17,7 +17,7 @@ export function getShardTimeout(config: SharedGlobalConfig): Pick ): Promise< Pick > { diff --git a/src/plugins/data/server/search/strategies/ese_search/request_utils.ts b/src/plugins/data/server/search/strategies/ese_search/request_utils.ts index e224215571ca9..f8fb54cfd870b 100644 --- a/src/plugins/data/server/search/strategies/ese_search/request_utils.ts +++ b/src/plugins/data/server/search/strategies/ese_search/request_utils.ts @@ -20,7 +20,7 @@ import { SearchSessionsConfigSchema } from '../../../../config'; * @internal */ export async function getIgnoreThrottled( - uiSettingsClient: IUiSettingsClient + uiSettingsClient: Pick ): Promise> { const includeFrozen = await uiSettingsClient.get(UI_SETTINGS.SEARCH_INCLUDE_FROZEN); return includeFrozen ? { ignore_throttled: false } : {}; @@ -30,7 +30,7 @@ export async function getIgnoreThrottled( @internal */ export async function getDefaultAsyncSubmitParams( - uiSettingsClient: IUiSettingsClient, + uiSettingsClient: Pick, searchSessionsConfig: SearchSessionsConfigSchema | null, options: ISearchOptions ): Promise< diff --git a/src/plugins/data/server/search/types.ts b/src/plugins/data/server/search/types.ts index 26e0416b9a4b0..026ff9139d932 100644 --- a/src/plugins/data/server/search/types.ts +++ b/src/plugins/data/server/search/types.ts @@ -35,7 +35,7 @@ export interface SearchEnhancements { export interface SearchStrategyDependencies { savedObjectsClient: SavedObjectsClientContract; esClient: IScopedClusterClient; - uiSettingsClient: IUiSettingsClient; + uiSettingsClient: Pick; searchSessionsClient: IScopedSearchSessionsClient; request: KibanaRequest; }