Skip to content

Commit 5717459

Browse files
drewdaemonelasticmachinekibanamachinelukasolson
authored
[performance] exclude ES request body when polling (#237191)
## Summary Close #186138 ### Background This reduces the network overhead between Kibana client and server. It leaves out the request body when polling an already-executed search. After the initial search request, there is only one reason the server has needed the request body: to associate the request with a background search session. Previously, this was done by computing a hash of the request body server-side. Now, the hash is computed on the client and transmitted _in lieu_ of the request body. For some legacy visualizations (TSVB and Timeline) which build the DSL request server-side, responsibility to provide the request hash has moved to their code bases since the session service no longer does this internally. #### Note on existing saved searches The hashing algorithm used on the frontend is identical to what was used on the backend, meaning that requests will still match any pre-existing saved searches they did before. (I checked during development, but might be worth a final test.) ### Impact To get a sense for the reduction, I pulled a few examples of the polling request body from the flights dashboard. | Request | Size (main) | Size (this branch) | % Reduction | |----------|--------------|--------------------|--------------| | 1 | 1219 | 378 | 70% | | 2 | 744 | 255 | 66% | | 3 | 1083 | 333 | 70% | | 4 | 884 | 333 | 62% | The change appears to reduce the polling payload size by roughly **60-70%**! (Only tested on DSL-powered panels.) --------- Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Lukas Olson <[email protected]>
1 parent 4f021d7 commit 5717459

File tree

19 files changed

+171
-86
lines changed

19 files changed

+171
-86
lines changed

src/platform/packages/shared/kbn-search-types/src/types.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,11 @@ export interface ISearchOptions {
119119
* When set es results are streamed back to the caller without any parsing of the content.
120120
*/
121121
stream?: boolean;
122+
123+
/**
124+
* A hash of the request params. This is attached automatically by the search interceptor. It is used to link this request with a search session.
125+
*/
126+
requestHash?: string;
122127
}
123128

124129
/**

src/platform/plugins/private/vis_types/timelion/server/series_functions/es/index.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import _ from 'lodash';
1212
import Datasource from '../../lib/classes/datasource';
1313
import buildRequest from './lib/build_request';
1414
import toSeriesList from './lib/agg_response_to_series_list';
15+
import { createRequestHash } from '@kbn/vis-type-timeseries-plugin/server';
1516

1617
function getRequestAbortedSignal(aborted$) {
1718
const controller = new AbortController();
@@ -127,11 +128,13 @@ export default new Datasource('es', {
127128
const abortSignal = getRequestAbortedSignal(tlConfig.request.events.aborted$);
128129

129130
const searchContext = await tlConfig.context.search;
131+
const requestHash = createRequestHash(body.params);
130132
const resp = await searchContext
131133
.search(
132134
body,
133135
{
134136
...tlConfig.request?.body.searchSession,
137+
requestHash,
135138
},
136139
{ ...tlConfig.context, abortSignal }
137140
)

src/platform/plugins/private/vis_types/timelion/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
"@kbn/react-kibana-context-render",
3737
"@kbn/css-utils",
3838
"@kbn/visualizations-common",
39+
"@kbn/vis-type-timeseries-plugin",
3940
],
4041
"exclude": [
4142
"target/**/*",

src/platform/plugins/shared/data/server/search/session/utils.test.ts renamed to src/platform/plugins/shared/data/public/search/search_interceptor/create_request_hash.test.ts

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,18 @@
77
* License v3.0 only", or the "Server Side Public License, v 1".
88
*/
99

10-
import { createRequestHash } from './utils';
10+
import { createRequestHash } from './create_request_hash';
1111

12-
describe('data/search/session utils', () => {
13-
describe('createRequestHash', () => {
14-
it('ignores `preference`', () => {
15-
const request = {
16-
foo: 'bar',
17-
};
12+
describe('createRequestHash', () => {
13+
it('ignores `preference`', () => {
14+
const request = {
15+
foo: 'bar',
16+
};
17+
const withPreference = {
18+
...request,
19+
preference: 1234,
20+
};
1821

19-
const withPreference = {
20-
...request,
21-
preference: 1234,
22-
};
23-
24-
expect(createRequestHash(request)).toEqual(createRequestHash(withPreference));
25-
});
22+
expect(createRequestHash(request)).toEqual(createRequestHash(withPreference));
2623
});
2724
});

src/platform/plugins/shared/data/public/search/search_interceptor/create_request_hash.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,15 @@
1010
import { Sha256 } from '@kbn/crypto-browser';
1111
import stringify from 'json-stable-stringify';
1212

13-
export async function createRequestHash(keys: Record<string, any>) {
14-
return new Sha256().update(stringify(keys), 'utf8').digest('hex');
13+
/**
14+
* Generate the hash for this request. Ignores the `preference` parameter since it generally won't
15+
* match from one request to another identical request.
16+
*
17+
* (Preference is used to ensure all queries go to the same set of shards and it doesn't need to be hashed
18+
* https://www.elastic.co/guide/en/elasticsearch/reference/current/search-shard-routing.html#shard-and-node-preference)
19+
*/
20+
export function createRequestHash(keys: Record<string, any>) {
21+
const { preference, ...params } = keys;
22+
const hash = new Sha256().update(stringify(params), 'utf8').digest('hex');
23+
return hash;
1524
}

src/platform/plugins/shared/data/public/search/search_interceptor/search_interceptor.test.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ jest.mock('./create_request_hash', () => {
3535
return {
3636
...originalModule,
3737
createRequestHash: jest.fn().mockImplementation((input) => {
38-
return Promise.resolve(JSON.stringify(input));
38+
const { preference, ...params } = input;
39+
return JSON.stringify(params);
3940
}),
4041
};
4142
});
@@ -319,7 +320,14 @@ describe('SearchInterceptor', () => {
319320

320321
mockCoreSetup.http.post.mockImplementation(getHttpMock(responses));
321322

322-
const response = searchInterceptor.search({}, { pollInterval: 0 });
323+
const response = searchInterceptor.search(
324+
{
325+
params: {
326+
body: { query: { match_all: {} } },
327+
},
328+
},
329+
{ pollInterval: 0 }
330+
);
323331
response.subscribe({ next, error, complete });
324332

325333
await timeTravel(10);
@@ -387,6 +395,18 @@ describe('SearchInterceptor', () => {
387395
`);
388396
expect(complete).toHaveBeenCalled();
389397
expect(error).not.toHaveBeenCalled();
398+
399+
// check that the request body wasn't included on the 2nd request
400+
expect(mockCoreSetup.http.post).toHaveBeenCalledTimes(2);
401+
const firstRequest = (
402+
mockCoreSetup.http.post.mock.calls[0] as unknown as [string, HttpFetchOptions]
403+
)[1];
404+
expect(JSON.parse(firstRequest?.body as string).params.body).toBeDefined();
405+
406+
const secondRequest = (
407+
mockCoreSetup.http.post.mock.calls[1] as unknown as [string, HttpFetchOptions]
408+
)[1];
409+
expect(JSON.parse(secondRequest?.body as string).params.body).not.toBeDefined();
390410
});
391411

392412
test('should abort on user abort', async () => {

src/platform/plugins/shared/data/public/search/search_interceptor/search_interceptor.ts

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -173,19 +173,14 @@ export class SearchInterceptor {
173173
options: IAsyncSearchOptions
174174
): Observable<string | undefined> {
175175
const { sessionId } = options;
176-
// Preference is used to ensure all queries go to the same set of shards and it doesn't need to be hashed
177-
// https://www.elastic.co/guide/en/elasticsearch/reference/current/search-shard-routing.html#shard-and-node-preference
178-
const { preference, ...params } = request.params || {};
179176
const hashOptions = {
180-
...params,
177+
...request.params,
181178
sessionId,
182179
};
183180

184181
if (!sessionId) return of(undefined); // don't use cache if doesn't belong to a session
185-
const sessionOptions = this.deps.session.getSearchOptions(options.sessionId);
186-
if (sessionOptions?.isRestore) return of(undefined); // don't use cache if restoring a session
187182

188-
return from(createRequestHash(hashOptions));
183+
return from(Promise.resolve(createRequestHash(hashOptions)));
189184
}
190185

191186
/*
@@ -447,7 +442,20 @@ export class SearchInterceptor {
447442
request: IKibanaSearchRequest,
448443
options?: ISearchOptions
449444
): Promise<IKibanaSearchResponse> {
450-
const { abortSignal } = options || {};
445+
const { abortSignal, requestHash } = options || {};
446+
447+
if (request.id) {
448+
// just polling an existing search, no need to send the body, just the hash
449+
450+
const { params, ...requestWithoutParams } = request;
451+
if (params) {
452+
const { body, ...paramsWithoutBody } = params;
453+
request = {
454+
...requestWithoutParams,
455+
params: paramsWithoutBody,
456+
};
457+
}
458+
}
451459

452460
const { executionContext, strategy, ...searchOptions } = this.getSerializableOptions(options);
453461
return this.deps.http
@@ -460,6 +468,7 @@ export class SearchInterceptor {
460468
body: JSON.stringify({
461469
...request,
462470
...searchOptions,
471+
requestHash,
463472
stream:
464473
strategy === ESQL_ASYNC_SEARCH_STRATEGY ||
465474
strategy === ENHANCED_ES_SEARCH_STRATEGY ||
@@ -537,12 +546,11 @@ export class SearchInterceptor {
537546
* Creates a new search observable and a corresponding search abort controller
538547
* If requestHash is defined, tries to return them first from cache.
539548
*/
540-
private getSearchResponse$(
541-
request: IKibanaSearchRequest,
542-
options: IAsyncSearchOptions,
543-
requestHash?: string
544-
) {
545-
const cached = requestHash ? this.responseCache.get(requestHash) : undefined;
549+
private getSearchResponse$(request: IKibanaSearchRequest, options: IAsyncSearchOptions) {
550+
const { requestHash } = options;
551+
const sessionOptions = this.deps.session.getSearchOptions(options.sessionId);
552+
const cached =
553+
requestHash && !sessionOptions?.isRestore ? this.responseCache.get(requestHash) : undefined; // don't use cache if restoring a session
546554

547555
const searchAbortController =
548556
cached?.searchAbortController || new SearchAbortController(this.searchTimeout);
@@ -586,10 +594,10 @@ export class SearchInterceptor {
586594

587595
return this.createRequestHash$(request, searchOptions).pipe(
588596
switchMap((requestHash) => {
597+
searchOptions.requestHash = requestHash;
589598
const { searchAbortController, response$ } = this.getSearchResponse$(
590599
request,
591-
searchOptions,
592-
requestHash
600+
searchOptions
593601
);
594602

595603
this.pendingCount$.next(this.pendingCount$.getValue() + 1);

src/platform/plugins/shared/data/server/search/routes/search.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ export function registerSearchRoute(
5454
isRestore: schema.maybe(schema.boolean()),
5555
retrieveResults: schema.maybe(schema.boolean()),
5656
stream: schema.maybe(schema.boolean()),
57+
requestHash: schema.maybe(schema.string()),
5758
},
5859
{ unknowns: 'allow' }
5960
),
@@ -68,6 +69,7 @@ export function registerSearchRoute(
6869
isRestore,
6970
retrieveResults,
7071
stream,
72+
requestHash,
7173
...searchRequest
7274
} = request.body;
7375
const { strategy, id } = request.params;
@@ -101,6 +103,7 @@ export function registerSearchRoute(
101103
isRestore,
102104
retrieveResults,
103105
stream,
106+
requestHash,
104107
}
105108
)
106109
.pipe(first())

src/platform/plugins/shared/data/server/search/search_service.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ describe('Search service', () => {
247247

248248
expect(mockSessionClient.trackId).toBeCalledTimes(1);
249249

250-
expect(mockSessionClient.trackId.mock.calls[0]).toEqual([searchRequest, 'my_id', options]);
250+
expect(mockSessionClient.trackId.mock.calls[0]).toEqual(['my_id', options]);
251251
});
252252

253253
it('does not call `trackId` if search is already tracked', async () => {

src/platform/plugins/shared/data/server/search/search_service.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -410,9 +410,7 @@ export class SearchService {
410410
isStored: true,
411411
});
412412
} else {
413-
return from(
414-
deps.searchSessionsClient.trackId(request, response.id, options)
415-
).pipe(
413+
return from(deps.searchSessionsClient.trackId(response.id, options)).pipe(
416414
tap(() => {
417415
isInternalSearchStored = true;
418416
}),

0 commit comments

Comments
 (0)