Skip to content

Commit

Permalink
[Search] Reuse uiSettings within bsearch request (#114088)
Browse files Browse the repository at this point in the history
  • Loading branch information
Dosant authored Oct 7, 2021
1 parent 7802133 commit 1b8ec4e
Show file tree
Hide file tree
Showing 8 changed files with 121 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/plugins/data/server/search/routes/bsearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, () =>
Expand Down
5 changes: 4 additions & 1 deletion src/plugins/data/server/search/search_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, ISearchStrategy<any, any>>;

Expand Down Expand Up @@ -453,7 +454,9 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
searchSessionsClient,
savedObjectsClient,
esClient: elasticsearch.client.asScoped(request),
uiSettingsClient: uiSettings.asScopedToClient(savedObjectsClient),
uiSettingsClient: new CachedUiSettingsClient(
uiSettings.asScopedToClient(savedObjectsClient)
),
request,
};
return {
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
});
Original file line number Diff line number Diff line change
@@ -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<IUiSettingsClient, 'get'> {
private cache: Promise<Record<string, unknown>> | undefined;

constructor(private readonly client: IUiSettingsClient) {}

async get<T = any>(key: string): Promise<T> {
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;
});
}
}
9 changes: 9 additions & 0 deletions src/plugins/data/server/search/services/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export function getShardTimeout(config: SharedGlobalConfig): Pick<Search, 'timeo
}

export async function getDefaultSearchParams(
uiSettingsClient: IUiSettingsClient
uiSettingsClient: Pick<IUiSettingsClient, 'get'>
): Promise<
Pick<Search, 'max_concurrent_shard_requests' | 'ignore_unavailable' | 'track_total_hits'>
> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { SearchSessionsConfigSchema } from '../../../../config';
* @internal
*/
export async function getIgnoreThrottled(
uiSettingsClient: IUiSettingsClient
uiSettingsClient: Pick<IUiSettingsClient, 'get'>
): Promise<Pick<Search, 'ignore_throttled'>> {
const includeFrozen = await uiSettingsClient.get(UI_SETTINGS.SEARCH_INCLUDE_FROZEN);
return includeFrozen ? { ignore_throttled: false } : {};
Expand All @@ -30,7 +30,7 @@ export async function getIgnoreThrottled(
@internal
*/
export async function getDefaultAsyncSubmitParams(
uiSettingsClient: IUiSettingsClient,
uiSettingsClient: Pick<IUiSettingsClient, 'get'>,
searchSessionsConfig: SearchSessionsConfigSchema | null,
options: ISearchOptions
): Promise<
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/data/server/search/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export interface SearchEnhancements {
export interface SearchStrategyDependencies {
savedObjectsClient: SavedObjectsClientContract;
esClient: IScopedClusterClient;
uiSettingsClient: IUiSettingsClient;
uiSettingsClient: Pick<IUiSettingsClient, 'get'>;
searchSessionsClient: IScopedSearchSessionsClient;
request: KibanaRequest;
}
Expand Down

0 comments on commit 1b8ec4e

Please sign in to comment.