From 12b95ab747c8a2adc0dc8120da0e7b0744b30fef Mon Sep 17 00:00:00 2001 From: Liza K Date: Mon, 10 May 2021 20:41:08 +0300 Subject: [PATCH 01/10] cancel the previous session --- .../data/public/search/session/session_service.ts | 11 ++++++++++- .../data/public/search/session/sessions_client.ts | 4 ++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/plugins/data/public/search/session/session_service.ts b/src/plugins/data/public/search/session/session_service.ts index 71f51b4bc8d83..e034a966ab118 100644 --- a/src/plugins/data/public/search/session/session_service.ts +++ b/src/plugins/data/public/search/session/session_service.ts @@ -213,6 +213,10 @@ export class SessionService { */ public start() { if (!this.currentApp) throw new Error('this.currentApp is missing'); + + // cancel previous session if needed + this.clear(); + this.state.transitions.start({ appName: this.currentApp }); return this.getSessionId()!; } @@ -241,7 +245,12 @@ export class SessionService { ); return; } - + const { sessionId, isRestore, isStored } = this.state.get(); + if (sessionId && !isRestore && !isStored) { + this.sessionsClient + .cancel(this.state.get().sessionId!) + .catch(() => {}); + } this.state.transitions.clear(); this.searchSessionInfoProvider = undefined; this.searchSessionIndicatorUiConfig = undefined; diff --git a/src/plugins/data/public/search/session/sessions_client.ts b/src/plugins/data/public/search/session/sessions_client.ts index 0b6f1b79f0c63..3bcb0294ff98b 100644 --- a/src/plugins/data/public/search/session/sessions_client.ts +++ b/src/plugins/data/public/search/session/sessions_client.ts @@ -92,6 +92,10 @@ export class SessionsClient { }); } + public cancel(sessionId: string): Promise { + return this.http!.post(`/internal/session/${encodeURIComponent(sessionId)}/cancel`); + } + public delete(sessionId: string): Promise { return this.http!.delete(`/internal/session/${encodeURIComponent(sessionId)}`); } From 6e870ddec5efd2a05d71695e659ca5c895dade22 Mon Sep 17 00:00:00 2001 From: Liza K Date: Wed, 12 May 2021 20:43:26 +0300 Subject: [PATCH 02/10] fixes --- .../data/public/search/session/mocks.ts | 1 + .../search/session/session_service.test.ts | 10 ++++++++ .../public/search/session/session_service.ts | 23 +++++++++++-------- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/plugins/data/public/search/session/mocks.ts b/src/plugins/data/public/search/session/mocks.ts index 18d32463864e3..e607bbb1b6e02 100644 --- a/src/plugins/data/public/search/session/mocks.ts +++ b/src/plugins/data/public/search/session/mocks.ts @@ -20,6 +20,7 @@ export function getSessionsClientMock(): jest.Mocked { extend: jest.fn(), delete: jest.fn(), rename: jest.fn(), + cancel: jest.fn(), }; } diff --git a/src/plugins/data/public/search/session/session_service.test.ts b/src/plugins/data/public/search/session/session_service.test.ts index 13a1a1bd388ba..dedb271dd5fb6 100644 --- a/src/plugins/data/public/search/session/session_service.test.ts +++ b/src/plugins/data/public/search/session/session_service.test.ts @@ -59,6 +59,7 @@ describe('Session service', () => { id, attributes: { ...mockSavedObject.attributes, sessionId: id }, })); + sessionsClient.cancel.mockResolvedValue(undefined); sessionService = new SessionService( initializerContext, () => @@ -98,6 +99,15 @@ describe('Session service', () => { expect(nowProvider.reset).toHaveBeenCalled(); }); + it('Calling start twice clears the previous session', async () => { + sessionService.start(); + expect(sessionService.getSessionId()).not.toBeUndefined(); + expect(nowProvider.set).toHaveBeenCalled(); + sessionService.start(); + expect(sessionService.getSessionId()).not.toBeUndefined(); + expect(sessionsClient.cancel).toHaveBeenCalled(); + }); + it("Can't clear other apps' session", async () => { sessionService.start(); expect(sessionService.getSessionId()).not.toBeUndefined(); diff --git a/src/plugins/data/public/search/session/session_service.ts b/src/plugins/data/public/search/session/session_service.ts index e034a966ab118..b330f8cf58ab2 100644 --- a/src/plugins/data/public/search/session/session_service.ts +++ b/src/plugins/data/public/search/session/session_service.ts @@ -215,7 +215,7 @@ export class SessionService { if (!this.currentApp) throw new Error('this.currentApp is missing'); // cancel previous session if needed - this.clear(); + this.cancelIfNotPersisted(); this.state.transitions.start({ appName: this.currentApp }); return this.getSessionId()!; @@ -245,28 +245,31 @@ export class SessionService { ); return; } - const { sessionId, isRestore, isStored } = this.state.get(); - if (sessionId && !isRestore && !isStored) { - this.sessionsClient - .cancel(this.state.get().sessionId!) - .catch(() => {}); - } + this.cancelIfNotPersisted(); this.state.transitions.clear(); this.searchSessionInfoProvider = undefined; this.searchSessionIndicatorUiConfig = undefined; } + private async cancelIfNotPersisted() { + const { sessionId, isRestore, isStored } = this.state.get(); + if (sessionId && !isRestore && !isStored) { + return this.cancel(); + } + } + /** * Request a cancellation of on-going search requests within current session */ public async cancel(): Promise { - const isStoredSession = this.state.get().isStored; this.state.get().pendingSearches.forEach((s) => { s.abort(); }); this.state.transitions.cancel(); - if (isStoredSession) { - await this.sessionsClient.delete(this.state.get().sessionId!); + + const { sessionId, isRestore, isStored } = this.state.get(); + if (sessionId && !isRestore && !isStored) { + await this.sessionsClient.cancel(sessionId).catch(() => {}); } } From 436dd51117ce5dfa0a546c2f88b46b3be1815d30 Mon Sep 17 00:00:00 2001 From: Liza K Date: Thu, 13 May 2021 13:52:25 +0300 Subject: [PATCH 03/10] cancellation --- .../data/public/search/session/session_service.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/plugins/data/public/search/session/session_service.ts b/src/plugins/data/public/search/session/session_service.ts index b330f8cf58ab2..f871d7e25263f 100644 --- a/src/plugins/data/public/search/session/session_service.ts +++ b/src/plugins/data/public/search/session/session_service.ts @@ -262,15 +262,14 @@ export class SessionService { * Request a cancellation of on-going search requests within current session */ public async cancel(): Promise { - this.state.get().pendingSearches.forEach((s) => { + const { pendingSearches, sessionId } = this.state.get(); + if (!sessionId) return; + + pendingSearches.forEach((s) => { s.abort(); }); + await this.sessionsClient.cancel(sessionId).catch(() => {}); this.state.transitions.cancel(); - - const { sessionId, isRestore, isStored } = this.state.get(); - if (sessionId && !isRestore && !isStored) { - await this.sessionsClient.cancel(sessionId).catch(() => {}); - } } /** From e5904acb9642bda38bc00f49f0f44d96667babcd Mon Sep 17 00:00:00 2001 From: Liza K Date: Tue, 18 May 2021 16:58:30 +0300 Subject: [PATCH 04/10] cleanup previous session properly --- .../data/public/search/session/session_service.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/plugins/data/public/search/session/session_service.ts b/src/plugins/data/public/search/session/session_service.ts index f871d7e25263f..71ba6d2f42f35 100644 --- a/src/plugins/data/public/search/session/session_service.ts +++ b/src/plugins/data/public/search/session/session_service.ts @@ -215,7 +215,7 @@ export class SessionService { if (!this.currentApp) throw new Error('this.currentApp is missing'); // cancel previous session if needed - this.cancelIfNotPersisted(); + this.cleanupPreviousSession(); this.state.transitions.start({ appName: this.currentApp }); return this.getSessionId()!; @@ -245,16 +245,19 @@ export class SessionService { ); return; } - this.cancelIfNotPersisted(); + this.cleanupPreviousSession(); this.state.transitions.clear(); this.searchSessionInfoProvider = undefined; this.searchSessionIndicatorUiConfig = undefined; } - private async cancelIfNotPersisted() { - const { sessionId, isRestore, isStored } = this.state.get(); + private async cleanupPreviousSession() { + const { pendingSearches, sessionId, isRestore, isStored } = this.state.get(); if (sessionId && !isRestore && !isStored) { - return this.cancel(); + pendingSearches.forEach((s) => { + s.abort(); + }); + await this.sessionsClient.cancel(sessionId).catch(() => {}); } } From 44023c443ea6d94ccefa715d4723574149ad71bc Mon Sep 17 00:00:00 2001 From: Liza K Date: Tue, 18 May 2021 22:21:37 +0300 Subject: [PATCH 05/10] don't fail delete and cancel if item was already cleaned up --- .../server/routes/session.test.ts | 78 +++++++++++++++++++ .../data_enhanced/server/routes/session.ts | 10 +++ 2 files changed, 88 insertions(+) diff --git a/x-pack/plugins/data_enhanced/server/routes/session.test.ts b/x-pack/plugins/data_enhanced/server/routes/session.test.ts index ebc501346aed2..874273a39858d 100644 --- a/x-pack/plugins/data_enhanced/server/routes/session.test.ts +++ b/x-pack/plugins/data_enhanced/server/routes/session.test.ts @@ -118,6 +118,45 @@ describe('registerSessionRoutes', () => { expect(mockContext.search!.cancelSession).toHaveBeenCalledWith(id); }); + it('cancel doesnt fail if not found', async () => { + const id = 'd7170a35-7e2c-48d6-8dec-9a056721b489'; + const params = { id }; + + const mockRequest = httpServerMock.createKibanaRequest({ params }); + const mockResponse = httpServerMock.createResponseFactory(); + + const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value; + const [, cancelHandler] = mockRouter.post.mock.calls[PostHandlerIndex.CANCEL]; + + mockContext.search!.cancelSession = jest.fn().mockRejectedValue({ + statusCode: 404, + }); + + await cancelHandler(mockContext, mockRequest, mockResponse); + + expect(mockContext.search!.cancelSession).toHaveBeenCalledWith(id); + expect(mockResponse.ok).toHaveBeenCalledTimes(1); + }); + + it('cancel fail on other errors', async () => { + const id = 'd7170a35-7e2c-48d6-8dec-9a056721b489'; + const params = { id }; + + const mockRequest = httpServerMock.createKibanaRequest({ params }); + const mockResponse = httpServerMock.createResponseFactory(); + + const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value; + const [, cancelHandler] = mockRouter.post.mock.calls[PostHandlerIndex.CANCEL]; + + mockContext.search!.cancelSession = jest.fn().mockRejectedValue({ + statusCode: 500, + }); + + await cancelHandler(mockContext, mockRequest, mockResponse).catch(() => {}); + + expect(mockResponse.customError).toHaveBeenCalledTimes(1); + }); + it('delete calls deleteSession with id', async () => { const id = 'd7170a35-7e2c-48d6-8dec-9a056721b489'; const params = { id }; @@ -133,6 +172,45 @@ describe('registerSessionRoutes', () => { expect(mockContext.search!.deleteSession).toHaveBeenCalledWith(id); }); + it('delete doesnt fail if not found', async () => { + const id = 'd7170a35-7e2c-48d6-8dec-9a056721b489'; + const params = { id }; + + const mockRequest = httpServerMock.createKibanaRequest({ params }); + const mockResponse = httpServerMock.createResponseFactory(); + + const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value; + const [, deleteHandler] = mockRouter.delete.mock.calls[0]; + + mockContext.search!.deleteSession = jest.fn().mockRejectedValue({ + statusCode: 404, + }); + + await deleteHandler(mockContext, mockRequest, mockResponse); + + expect(mockContext.search!.deleteSession).toHaveBeenCalledWith(id); + expect(mockResponse.ok).toHaveBeenCalledTimes(1); + }); + + it('delete returns error if another error code occurs', async () => { + const id = 'd7170a35-7e2c-48d6-8dec-9a056721b489'; + const params = { id }; + + const mockRequest = httpServerMock.createKibanaRequest({ params }); + const mockResponse = httpServerMock.createResponseFactory(); + + const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value; + const [, deleteHandler] = mockRouter.delete.mock.calls[0]; + + mockContext.search!.deleteSession = jest.fn().mockRejectedValue({ + statusCode: 500, + }); + + await deleteHandler(mockContext, mockRequest, mockResponse).catch(() => {}); + + expect(mockResponse.customError).toHaveBeenCalledTimes(1); + }); + it('extend calls extendSession with id', async () => { const id = 'd7170a35-7e2c-48d6-8dec-9a056721b489'; const expires = new Date().toISOString(); diff --git a/x-pack/plugins/data_enhanced/server/routes/session.ts b/x-pack/plugins/data_enhanced/server/routes/session.ts index 0b786f44454a9..7722b7074d9ad 100644 --- a/x-pack/plugins/data_enhanced/server/routes/session.ts +++ b/x-pack/plugins/data_enhanced/server/routes/session.ts @@ -151,6 +151,11 @@ export function registerSessionRoutes(router: DataEnhancedPluginRouter, logger: return res.ok(); } catch (e) { const err = e.output?.payload || e; + // Don't fail on items that were already deleted + if (err.statusCode === 404) { + logger.debug(`Search session ${id} not found. Ignoring.`); + return res.ok(); + } logger.error(err); return reportServerError(res, err); } @@ -177,6 +182,11 @@ export function registerSessionRoutes(router: DataEnhancedPluginRouter, logger: return res.ok(); } catch (e) { const err = e.output?.payload || e; + // Don't fail on items that were already deleted + if (err.statusCode === 404) { + logger.debug(`Search session ${id} not found. Ignoring.`); + return res.ok(); + } logger.error(err); return reportServerError(res, err); } From 54bb6cd7963cb0bc72eb59893b00838844d2595a Mon Sep 17 00:00:00 2001 From: Liza K Date: Wed, 19 May 2021 14:02:18 +0300 Subject: [PATCH 06/10] test --- x-pack/test/api_integration/apis/search/session.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/test/api_integration/apis/search/session.ts b/x-pack/test/api_integration/apis/search/session.ts index d47199a0f1c1e..27096cbd0c1d7 100644 --- a/x-pack/test/api_integration/apis/search/session.ts +++ b/x-pack/test/api_integration/apis/search/session.ts @@ -49,8 +49,8 @@ export default function ({ getService }: FtrProviderContext) { await supertest.get(`/internal/session/${sessionId}`).set('kbn-xsrf', 'foo').expect(200); }); - it('should fail to delete an unknown session', async () => { - await supertest.delete(`/internal/session/123`).set('kbn-xsrf', 'foo').expect(404); + it('should NOT fail to delete an unknown session', async () => { + await supertest.delete(`/internal/session/123`).set('kbn-xsrf', 'foo').expect(200); }); it('should create and delete a session', async () => { From 9ffbb032269b66bd6383245a37e42fb8c7bab8dc Mon Sep 17 00:00:00 2001 From: Liza K Date: Wed, 19 May 2021 17:43:16 +0300 Subject: [PATCH 07/10] test --- x-pack/test/api_integration/apis/search/session.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/x-pack/test/api_integration/apis/search/session.ts b/x-pack/test/api_integration/apis/search/session.ts index 27096cbd0c1d7..9f6160f4a43ac 100644 --- a/x-pack/test/api_integration/apis/search/session.ts +++ b/x-pack/test/api_integration/apis/search/session.ts @@ -487,7 +487,9 @@ export default function ({ getService }: FtrProviderContext) { .delete(`/internal/session/${sessionId}`) .set('kbn-xsrf', 'foo') .auth('other_user', 'password') - .expect(404); + .expect(200); + + await supertest.get(`/internal/session/${sessionId}`).set('kbn-xsrf', 'foo').expect(200); }); it(`should prevent users from cancelling other users' sessions`, async () => { @@ -508,7 +510,15 @@ export default function ({ getService }: FtrProviderContext) { .post(`/internal/session/${sessionId}/cancel`) .set('kbn-xsrf', 'foo') .auth('other_user', 'password') - .expect(404); + .expect(200); + + const resp = await supertest + .get(`/internal/session/${sessionId}`) + .set('kbn-xsrf', 'foo') + .expect(200); + + const { status } = resp.body.attributes; + expect(status).not.to.equal(SearchSessionStatus.CANCELLED); }); it(`should prevent users from extending other users' sessions`, async () => { From 25445c64c2fa339ffaa75a59636033fdc0c47bb2 Mon Sep 17 00:00:00 2001 From: Liza K Date: Mon, 24 May 2021 15:11:24 +0300 Subject: [PATCH 08/10] ignore resource_not_found_exception when deleting an already cleared \ expired async search --- .../server/search/session/check_running_sessions.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/data_enhanced/server/search/session/check_running_sessions.ts b/x-pack/plugins/data_enhanced/server/search/session/check_running_sessions.ts index 6787d31ed2b74..7012f02dc5d73 100644 --- a/x-pack/plugins/data_enhanced/server/search/session/check_running_sessions.ts +++ b/x-pack/plugins/data_enhanced/server/search/session/check_running_sessions.ts @@ -184,9 +184,11 @@ function checkRunningSessionsPage( try { await client.asyncSearch.delete({ id: searchInfo.id }); } catch (e) { - logger.error( - `Error while deleting async_search ${searchInfo.id}: ${e.message}` - ); + if (e.message !== 'resource_not_found_exception') { + logger.error( + `Error while deleting async_search ${searchInfo.id}: ${e.message}` + ); + } } } }); From 74c145ef76749c251e5c2474ad13f8818a5c045c Mon Sep 17 00:00:00 2001 From: Liza Katz Date: Tue, 25 May 2021 15:36:01 +0300 Subject: [PATCH 09/10] Update x-pack/test/api_integration/apis/search/session.ts Co-authored-by: Jean-Louis Leysens --- x-pack/test/api_integration/apis/search/session.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/test/api_integration/apis/search/session.ts b/x-pack/test/api_integration/apis/search/session.ts index 9f6160f4a43ac..82d92e3d87c10 100644 --- a/x-pack/test/api_integration/apis/search/session.ts +++ b/x-pack/test/api_integration/apis/search/session.ts @@ -49,7 +49,7 @@ export default function ({ getService }: FtrProviderContext) { await supertest.get(`/internal/session/${sessionId}`).set('kbn-xsrf', 'foo').expect(200); }); - it('should NOT fail to delete an unknown session', async () => { + it('should NOT fail when deleting an unknown session', async () => { await supertest.delete(`/internal/session/123`).set('kbn-xsrf', 'foo').expect(200); }); From 6e6fcb25dbf8241962ba8a6b081115616367e082 Mon Sep 17 00:00:00 2001 From: Liza K Date: Sun, 30 May 2021 19:37:16 +0300 Subject: [PATCH 10/10] limit --- packages/kbn-optimizer/limits.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/kbn-optimizer/limits.yml b/packages/kbn-optimizer/limits.yml index 7f2d0768a1fb9..897de7b079a67 100644 --- a/packages/kbn-optimizer/limits.yml +++ b/packages/kbn-optimizer/limits.yml @@ -3,8 +3,7 @@ pageLoadAssetSize: alerting: 106936 apm: 64385 apmOss: 18996 - beatsManagement: 188135 - bfetch: 41874 + bfetch: 51874 canvas: 1066647 charts: 195358 cloud: 21076 @@ -77,7 +76,7 @@ pageLoadAssetSize: tileMap: 65337 timelion: 29920 transform: 41007 - triggersActionsUi: 186732 + triggersActionsUi: 100000 uiActions: 97717 uiActionsEnhanced: 313011 upgradeAssistant: 81241