[data.search] Set default expiration to 1m if search sessions are dis…

…abled (elastic#105329) Co-authored-by: Anton Dosov <anton.dosov@elastic.co> Co-authored-by: Liza K <liza.katz@elastic.co>
kibanamachine · Jul 21, 2021 · 29ef40a · 29ef40a
1 parent cdc5c5b
commit 29ef40a
Show file tree

Hide file tree

Showing 6 changed files with 184 additions and 20 deletions.
diff --git a/src/plugins/data/server/search/session/mocks.ts b/src/plugins/data/server/search/session/mocks.ts
@@ -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)
     ),
   };

diff --git a/src/plugins/data/server/search/strategies/eql_search/eql_search_strategy.ts b/src/plugins/data/server/search/strategies/eql_search/eql_search_strategy.ts
@@ -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

diff --git a/src/plugins/data/server/search/strategies/ese_search/ese_search_strategy.test.ts b/src/plugins/data/server/search/strategies/ese_search/ese_search_strategy.test.ts
@@ -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 () => {

diff --git a/src/plugins/data/server/search/strategies/ese_search/ese_search_strategy.ts b/src/plugins/data/server/search/strategies/ese_search/ese_search_strategy.ts
@@ -59,7 +59,7 @@ export const enhancedEsSearchStrategyProvider = (
 
     const search = async () => {
       const params = id
-        ? getDefaultAsyncGetParams(options)
+        ? getDefaultAsyncGetParams(searchSessionsClient.getConfig(), options)
         : {
             ...(await getDefaultAsyncSubmitParams(
               uiSettingsClient,

diff --git a/src/plugins/data/server/search/strategies/ese_search/request_utils.test.ts b/src/plugins/data/server/search/strategies/ese_search/request_utils.test.ts
@@ -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');
+    });
+  });
+});
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
@@ -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
         }),
   };
 }