Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[data.search] Set default expiration to 1m if search sessions are disabled #105329

Merged
merged 6 commits into from
Jul 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/plugins/data/server/search/session/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ export function createSearchSessionsClientMock<T = unknown>(): jest.Mocked<
getConfig: jest.fn(
() =>
(({
defaultExpiration: moment.duration('1', 'm'),
defaultExpiration: moment.duration('1', 'w'),
enabled: true,
} as unknown) as SearchSessionsConfigSchema)
),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ export const eqlSearchStrategyProvider = (
uiSettingsClient
);
const params = id
? getDefaultAsyncGetParams(options)
? getDefaultAsyncGetParams(null, options)
: {
...(await getIgnoreThrottled(uiSettingsClient)),
...defaultParams,
...getDefaultAsyncGetParams(options),
...getDefaultAsyncGetParams(null, options),
...request.params,
};
const promise = id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ describe('ES search strategy', () => {
expect(request.index).toEqual(params.index);
expect(request.body).toEqual(params.body);

expect(request).toHaveProperty('keep_alive', '60000ms');
expect(request).toHaveProperty('keep_alive', '604800000ms');
});

it('makes a GET request to async search without keepalive', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export const enhancedEsSearchStrategyProvider = (

const search = async () => {
const params = id
? getDefaultAsyncGetParams(options)
? getDefaultAsyncGetParams(searchSessionsClient.getConfig(), options)
: {
...(await getDefaultAsyncSubmitParams(
uiSettingsClient,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
/*
* 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 {
getDefaultAsyncSubmitParams,
getDefaultAsyncGetParams,
getIgnoreThrottled,
} from './request_utils';
import { IUiSettingsClient } from 'kibana/server';
import { UI_SETTINGS } from '../../../../common';
import moment from 'moment';
import { SearchSessionsConfigSchema } from '../../../../config';

const getMockUiSettingsClient = (config: Record<string, unknown>) => {
return { get: async (key: string) => config[key] } as IUiSettingsClient;
};

const getMockSearchSessionsConfig = ({
enabled = true,
defaultExpiration = moment.duration(7, 'd'),
} = {}) =>
({
enabled,
defaultExpiration,
} as SearchSessionsConfigSchema);

describe('request utils', () => {
describe('getIgnoreThrottled', () => {
test('returns `ignore_throttled` as `true` when `includeFrozen` is `false`', async () => {
const mockUiSettingsClient = getMockUiSettingsClient({
[UI_SETTINGS.SEARCH_INCLUDE_FROZEN]: false,
});
const result = await getIgnoreThrottled(mockUiSettingsClient);
expect(result.ignore_throttled).toBe(true);
});

test('returns `ignore_throttled` as `false` when `includeFrozen` is `true`', async () => {
const mockUiSettingsClient = getMockUiSettingsClient({
[UI_SETTINGS.SEARCH_INCLUDE_FROZEN]: true,
});
const result = await getIgnoreThrottled(mockUiSettingsClient);
expect(result.ignore_throttled).toBe(false);
});
});

describe('getDefaultAsyncSubmitParams', () => {
test('Uses `keep_alive` from default params if no `sessionId` is provided', async () => {
const mockUiSettingsClient = getMockUiSettingsClient({
[UI_SETTINGS.SEARCH_INCLUDE_FROZEN]: false,
});
const mockConfig = getMockSearchSessionsConfig({
defaultExpiration: moment.duration(3, 'd'),
});
const params = await getDefaultAsyncSubmitParams(mockUiSettingsClient, mockConfig, {});
expect(params).toHaveProperty('keep_alive', '1m');
});

test('Uses `keep_alive` from config if enabled', async () => {
const mockUiSettingsClient = getMockUiSettingsClient({
[UI_SETTINGS.SEARCH_INCLUDE_FROZEN]: false,
});
const mockConfig = getMockSearchSessionsConfig({
defaultExpiration: moment.duration(3, 'd'),
});
const params = await getDefaultAsyncSubmitParams(mockUiSettingsClient, mockConfig, {
sessionId: 'foo',
});
expect(params).toHaveProperty('keep_alive', '259200000ms');
});

test('Uses `keepAlive` of `1m` if disabled', async () => {
const mockUiSettingsClient = getMockUiSettingsClient({
[UI_SETTINGS.SEARCH_INCLUDE_FROZEN]: false,
});
const mockConfig = getMockSearchSessionsConfig({
defaultExpiration: moment.duration(3, 'd'),
enabled: false,
});
const params = await getDefaultAsyncSubmitParams(mockUiSettingsClient, mockConfig, {
sessionId: 'foo',
});
expect(params).toHaveProperty('keep_alive', '1m');
});

test('Uses `keep_on_completion` if enabled', async () => {
const mockUiSettingsClient = getMockUiSettingsClient({
[UI_SETTINGS.SEARCH_INCLUDE_FROZEN]: false,
});
const mockConfig = getMockSearchSessionsConfig({});
const params = await getDefaultAsyncSubmitParams(mockUiSettingsClient, mockConfig, {
sessionId: 'foo',
});
expect(params).toHaveProperty('keep_on_completion', true);
});

test('Does not use `keep_on_completion` if disabled', async () => {
const mockUiSettingsClient = getMockUiSettingsClient({
[UI_SETTINGS.SEARCH_INCLUDE_FROZEN]: false,
});
const mockConfig = getMockSearchSessionsConfig({
defaultExpiration: moment.duration(3, 'd'),
enabled: false,
});
const params = await getDefaultAsyncSubmitParams(mockUiSettingsClient, mockConfig, {
sessionId: 'foo',
});
expect(params).toHaveProperty('keep_on_completion', false);
});
});

describe('getDefaultAsyncGetParams', () => {
test('Uses `wait_for_completion_timeout`', async () => {
const mockConfig = getMockSearchSessionsConfig({
defaultExpiration: moment.duration(3, 'd'),
enabled: true,
});
const params = getDefaultAsyncGetParams(mockConfig, {});
expect(params).toHaveProperty('wait_for_completion_timeout');
});

test('Uses `keep_alive` if `sessionId` is not provided', async () => {
const mockConfig = getMockSearchSessionsConfig({
defaultExpiration: moment.duration(3, 'd'),
enabled: true,
});
const params = getDefaultAsyncGetParams(mockConfig, {});
expect(params).toHaveProperty('keep_alive', '1m');
});

test('Has no `keep_alive` if `sessionId` is provided', async () => {
const mockConfig = getMockSearchSessionsConfig({
defaultExpiration: moment.duration(3, 'd'),
enabled: true,
});
const params = getDefaultAsyncGetParams(mockConfig, { sessionId: 'foo' });
expect(params).not.toHaveProperty('keep_alive');
});

test('Uses `keep_alive` if `sessionId` is provided but sessions disabled', async () => {
const mockConfig = getMockSearchSessionsConfig({
defaultExpiration: moment.duration(3, 'd'),
enabled: false,
});
const params = getDefaultAsyncGetParams(mockConfig, { sessionId: 'foo' });
expect(params).toHaveProperty('keep_alive', '1m');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -46,37 +46,47 @@ export async function getDefaultAsyncSubmitParams(
| 'keep_on_completion'
>
> {
const useSearchSessions = searchSessionsConfig?.enabled && !!options.sessionId;

// TODO: searchSessionsConfig could be "null" if we are running without x-pack which happens only in tests.
// This can be cleaned up when we completely stop separating basic and oss
const keepAlive = useSearchSessions
? `${searchSessionsConfig!.defaultExpiration.asMilliseconds()}ms`
: '1m';

return {
// TODO: adjust for partial results
batched_reduce_size: 64,
keep_on_completion: !!options.sessionId, // Always return an ID, even if the request completes quickly
...getDefaultAsyncGetParams(options),
// Wait up to 100ms for the response to return
wait_for_completion_timeout: '100ms',
// If search sessions are used, store and get an async ID even for short running requests.
keep_on_completion: useSearchSessions,
// The initial keepalive is as defined in defaultExpiration if search sessions are used or 1m otherwise.
keep_alive: keepAlive,
...(await getIgnoreThrottled(uiSettingsClient)),
...(await getDefaultSearchParams(uiSettingsClient)),
...(options.sessionId
? {
// TODO: searchSessionsConfig could be "null" if we are running without x-pack which happens only in tests.
// This can be cleaned up when we completely stop separating basic and oss
keep_alive: searchSessionsConfig
? `${searchSessionsConfig.defaultExpiration.asMilliseconds()}ms`
: '1m',
}
: {}),
// If search sessions are used, set the initial expiration time.
};
}

/**
@internal
*/
export function getDefaultAsyncGetParams(
searchSessionsConfig: SearchSessionsConfigSchema | null,
options: ISearchOptions
): Pick<AsyncSearchGet, 'keep_alive' | 'wait_for_completion_timeout'> {
const useSearchSessions = searchSessionsConfig?.enabled && !!options.sessionId;

return {
wait_for_completion_timeout: '100ms', // Wait up to 100ms for the response to return
...(options.sessionId
? undefined
// Wait up to 100ms for the response to return
wait_for_completion_timeout: '100ms',
...(useSearchSessions
? // Don't change the expiration of search requests that are tracked in a search session
undefined
: {
// We still need to do polling for searches not within the context of a search session or when search session disabled
keep_alive: '1m',
// We still need to do polling for searches not within the context of a search session
}),
};
}