From 8ca90b4047ccbbc1835cb9d99641d578c620f5a5 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 24 Oct 2023 08:08:18 -0700 Subject: [PATCH 1/3] Revert "[data.search.bsearch] Forward request abortSignal to search strategy (#169041)" This reverts commit 24fd9517cf2983ac92f07a7dea59c2a8956af366. --- src/plugins/data/common/search/utils.ts | 4 +- .../data/server/search/routes/bsearch.ts | 4 +- .../ese_search/ese_search_strategy.test.ts | 70 ------------------- .../ese_search/ese_search_strategy.ts | 2 +- 4 files changed, 3 insertions(+), 77 deletions(-) diff --git a/src/plugins/data/common/search/utils.ts b/src/plugins/data/common/search/utils.ts index eeb65c04ecc582..a3695019813280 100644 --- a/src/plugins/data/common/search/utils.ts +++ b/src/plugins/data/common/search/utils.ts @@ -23,9 +23,7 @@ export const isAbortResponse = (response?: IKibanaSearchResponse) => { /** * @returns true if request is still running */ -export const isRunningResponse = (response?: IKibanaSearchResponse) => { - return response?.isRunning ?? false; -}; +export const isRunningResponse = (response?: IKibanaSearchResponse) => response?.isRunning ?? false; export const getUserTimeZone = ( getConfig: AggTypesDependencies['getConfig'], diff --git a/src/plugins/data/server/search/routes/bsearch.ts b/src/plugins/data/server/search/routes/bsearch.ts index bf1aa4aaa3cbcf..cbbe5dedb03cfa 100644 --- a/src/plugins/data/server/search/routes/bsearch.ts +++ b/src/plugins/data/server/search/routes/bsearch.ts @@ -11,7 +11,6 @@ import { catchError } from 'rxjs/operators'; import { BfetchServerSetup } from '@kbn/bfetch-plugin/server'; import type { ExecutionContextSetup } from '@kbn/core/server'; import apm from 'elastic-apm-node'; -import { getRequestAbortedSignal } from '../..'; import { IKibanaSearchRequest, IKibanaSearchResponse, @@ -29,7 +28,6 @@ export function registerBsearchRoute( IKibanaSearchResponse >('/internal/bsearch', (request) => { const search = getScoped(request); - const abortSignal = getRequestAbortedSignal(request.events.aborted$); return { /** * @param requestOptions @@ -41,7 +39,7 @@ export function registerBsearchRoute( apm.addLabels(executionContextService.getAsLabels()); return firstValueFrom( - search.search(requestData, { ...restOptions, abortSignal }).pipe( + search.search(requestData, restOptions).pipe( catchError((err) => { // Re-throw as object, to get attributes passed to the client // eslint-disable-next-line no-throw-literal 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 index 6c1746984c86bc..3b2c5e8e0e5c8b 100644 --- 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 @@ -260,38 +260,6 @@ describe('ES search strategy', () => { expect(mockApiCaller).toBeCalledTimes(0); }); - - it('should delete when aborted', async () => { - mockSubmitCaller.mockResolvedValueOnce({ - ...mockAsyncResponse, - body: { - ...mockAsyncResponse.body, - is_running: true, - }, - }); - - const params = { index: 'logstash-*', body: { query: {} } }; - const esSearch = await enhancedEsSearchStrategyProvider( - mockLegacyConfig$, - mockSearchConfig, - mockLogger - ); - const abortController = new AbortController(); - const abortSignal = abortController.signal; - - // Abort after an incomplete first response is returned - setTimeout(() => abortController.abort(), 100); - - let err: KbnServerError | undefined; - try { - await esSearch.search({ params }, { abortSignal }, mockDeps).toPromise(); - } catch (e) { - err = e; - } - expect(mockSubmitCaller).toBeCalled(); - expect(err).not.toBeUndefined(); - expect(mockDeleteCaller).toBeCalled(); - }); }); describe('with sessionId', () => { @@ -399,44 +367,6 @@ describe('ES search strategy', () => { expect(request).toHaveProperty('wait_for_completion_timeout'); expect(request).not.toHaveProperty('keep_alive'); }); - - it('should not delete a saved session when aborted', async () => { - mockSubmitCaller.mockResolvedValueOnce({ - ...mockAsyncResponse, - body: { - ...mockAsyncResponse.body, - is_running: true, - }, - }); - - const params = { index: 'logstash-*', body: { query: {} } }; - const esSearch = await enhancedEsSearchStrategyProvider( - mockLegacyConfig$, - mockSearchConfig, - mockLogger - ); - const abortController = new AbortController(); - const abortSignal = abortController.signal; - - // Abort after an incomplete first response is returned - setTimeout(() => abortController.abort(), 100); - - let err: KbnServerError | undefined; - try { - await esSearch - .search( - { params }, - { abortSignal, sessionId: '1', isSearchStored: true, isStored: true }, - mockDeps - ) - .toPromise(); - } catch (e) { - err = e; - } - expect(mockSubmitCaller).toBeCalled(); - expect(err).not.toBeUndefined(); - expect(mockDeleteCaller).not.toBeCalled(); - }); }); it('throws normalized error if ResponseError is thrown', 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 index 460d8f95ed3e4d..c4b8933f2c7c35 100644 --- 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 @@ -83,7 +83,7 @@ export const enhancedEsSearchStrategyProvider = ( }; const cancel = async () => { - if (id && !options.isStored) { + if (id) { await cancelAsyncSearch(id, esClient); } }; From ae053f486fe24f571b9839a6e78e168adf781928 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 25 Oct 2023 09:33:01 -0700 Subject: [PATCH 2/3] Unskip flaky test --- test/functional/apps/discover/group4/_adhoc_data_views.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/functional/apps/discover/group4/_adhoc_data_views.ts b/test/functional/apps/discover/group4/_adhoc_data_views.ts index 3294c92cad73e2..04880c1cfbc723 100644 --- a/test/functional/apps/discover/group4/_adhoc_data_views.ts +++ b/test/functional/apps/discover/group4/_adhoc_data_views.ts @@ -41,9 +41,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.dashboard.waitForRenderComplete(); }; - // FLAKY: https://github.com/elastic/kibana/issues/169434 - // FLAKY: https://github.com/elastic/kibana/issues/169454 - describe.skip('adhoc data views', function () { + describe('adhoc data views', function () { before(async () => { await security.testUser.setRoles(['kibana_admin', 'test_logstash_reader']); await kibanaServer.importExport.load('test/functional/fixtures/kbn_archiver/discover.json'); From 755309a50c1aa2acda9532a6b5749e86a039fc02 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 25 Oct 2023 13:43:23 -0700 Subject: [PATCH 3/3] Unskip other test --- test/functional/apps/discover/group4/_chart_hidden.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/functional/apps/discover/group4/_chart_hidden.ts b/test/functional/apps/discover/group4/_chart_hidden.ts index 7a1e1408056fb5..6bee290df896d5 100644 --- a/test/functional/apps/discover/group4/_chart_hidden.ts +++ b/test/functional/apps/discover/group4/_chart_hidden.ts @@ -20,9 +20,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { defaultIndex: 'logstash-*', }; - // FLAKY: https://github.com/elastic/kibana/issues/169458 - // FLAKY: https://github.com/elastic/kibana/issues/169459 - describe.skip('discover show/hide chart test', function () { + describe('discover show/hide chart test', function () { before(async function () { await security.testUser.setRoles(['kibana_admin', 'test_logstash_reader']); log.debug('load kibana index with default index pattern');