Skip to content

Commit

Permalink
[Search Sessions] Optimize search session so updates (#142850)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Dosant
Dosant authored Oct 11, 2022
1 parent b5bacc3 commit a6b1c7a
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 307 deletions.
2 changes: 1 addition & 1 deletion docs/settings/search-sessions-settings.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ How long {kib} stores search results from unsaved sessions,
after the last search in the session completes. The default is `5m`.

`data.search.sessions.maxUpdateRetries` {ess-icon}::
How many retries {kib} can perform while attempting to save a search session. The default is `3`.
How many retries {kib} can perform while attempting to save a search session. The default is `10`.

`data.search.sessions.defaultExpiration` {ess-icon}::
How long search session results are stored before they are deleted.
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/data/config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export const searchSessionsConfigSchema = schema.object({
/**
* maxUpdateRetries controls how many retries we perform while attempting to save a search session
*/
maxUpdateRetries: schema.number({ defaultValue: 3 }),
maxUpdateRetries: schema.number({ defaultValue: 10 }),

/**
* defaultExpiration controls how long search sessions are valid for, until they are expired.
Expand Down
6 changes: 3 additions & 3 deletions src/plugins/data/server/search/collectors/search/usage.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,11 @@ export function searchUsageObserver(
return {
next(response: IEsSearchResponse) {
if (isRestore || !isCompleteResponse(response)) return;
logger.debug(`trackSearchStatus:next ${response.rawResponse.took}`);
logger.debug(`trackSearchStatus:success, took:${response.rawResponse.took}`);
usage?.trackSuccess(response.rawResponse.took);
},
error() {
logger.debug(`trackSearchStatus:error`);
error(e: Error) {
logger.debug(`trackSearchStatus:error, ${e}`);
usage?.trackError();
},
};
Expand Down
169 changes: 5 additions & 164 deletions src/plugins/data/server/search/session/session_service.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,43 +187,12 @@ describe('SearchSessionService', () => {
).rejects.toMatchInlineSnapshot(`[Error: locatorId is required]`);
});

it('saving updates an existing saved object and persists it', async () => {
const mockUpdateSavedObject = {
...mockSavedObject,
attributes: {},
};
savedObjectsClient.get.mockResolvedValue(mockSavedObject);
savedObjectsClient.update.mockResolvedValue(mockUpdateSavedObject);

await service.save({ savedObjectsClient }, mockUser1, sessionId, {
name: 'banana',
appId: 'nanana',
locatorId: 'panama',
});

expect(savedObjectsClient.update).toHaveBeenCalled();
expect(savedObjectsClient.create).not.toHaveBeenCalled();

const [type, id, callAttributes] = savedObjectsClient.update.mock.calls[0];
expect(type).toBe(SEARCH_SESSION_TYPE);
expect(id).toBe(sessionId);
expect(callAttributes).not.toHaveProperty('idMapping');
expect(callAttributes).toHaveProperty('name', 'banana');
expect(callAttributes).toHaveProperty('appId', 'nanana');
expect(callAttributes).toHaveProperty('locatorId', 'panama');
expect(callAttributes).toHaveProperty('initialState', {});
expect(callAttributes).toHaveProperty('restoreState', {});
});

it('saving creates a new persisted saved object, if it did not exist', async () => {
it('saving creates a new persisted saved object', async () => {
const mockCreatedSavedObject = {
...mockSavedObject,
attributes: {},
};

savedObjectsClient.update.mockRejectedValue(
SavedObjectsErrorHelpers.createGenericNotFoundError(sessionId)
);
savedObjectsClient.create.mockResolvedValue(mockCreatedSavedObject);

await service.save({ savedObjectsClient }, mockUser1, sessionId, {
Expand All @@ -232,7 +201,7 @@ describe('SearchSessionService', () => {
locatorId: 'panama',
});

expect(savedObjectsClient.update).toHaveBeenCalledTimes(1);
expect(savedObjectsClient.update).toHaveBeenCalledTimes(0);
expect(savedObjectsClient.create).toHaveBeenCalledTimes(1);

const [type, callAttributes, options] = savedObjectsClient.create.mock.calls[0];
Expand Down Expand Up @@ -738,147 +707,19 @@ describe('SearchSessionService', () => {
});
});

it('retries updating the saved object if there was a ES conflict 409', async () => {
it('passes retryOnConflict param to es', async () => {
const searchRequest = { params: {} };
const searchId = 'FnpFYlBpeXdCUTMyZXhCLTc1TWFKX0EbdDFDTzJzTE1Sck9PVTBIcW1iU05CZzo4MDA0';

const mockUpdateSavedObject = {
...mockSavedObject,
attributes: {},
};

let counter = 0;

savedObjectsClient.update.mockImplementation(() => {
return new Promise((resolve, reject) => {
if (counter === 0) {
counter++;
reject(SavedObjectsErrorHelpers.createConflictError(SEARCH_SESSION_TYPE, searchId));
} else {
resolve(mockUpdateSavedObject);
}
});
});

await service.trackId({ savedObjectsClient }, mockUser1, searchRequest, searchId, {
sessionId,
strategy: MOCK_STRATEGY,
});

expect(savedObjectsClient.update).toHaveBeenCalledTimes(2);
expect(savedObjectsClient.create).not.toHaveBeenCalled();
});

it('retries updating the saved object if theres a ES conflict 409, but stops after MAX_RETRIES times', async () => {
const searchRequest = { params: {} };
const searchId = 'FnpFYlBpeXdCUTMyZXhCLTc1TWFKX0EbdDFDTzJzTE1Sck9PVTBIcW1iU05CZzo4MDA0';

savedObjectsClient.update.mockImplementation(() => {
return new Promise((resolve, reject) => {
reject(SavedObjectsErrorHelpers.createConflictError(SEARCH_SESSION_TYPE, searchId));
});
});

await service.trackId({ savedObjectsClient }, mockUser1, searchRequest, searchId, {
sessionId,
strategy: MOCK_STRATEGY,
});

// Track ID doesn't throw errors even in cases of failure!
expect(savedObjectsClient.update).toHaveBeenCalledTimes(MAX_UPDATE_RETRIES);
expect(savedObjectsClient.create).not.toHaveBeenCalled();
});

it('creates the saved object in non persisted state, if search session doesnt exists', async () => {
const searchRequest = { params: {} };
const requestHash = createRequestHash(searchRequest.params);
const searchId = 'FnpFYlBpeXdCUTMyZXhCLTc1TWFKX0EbdDFDTzJzTE1Sck9PVTBIcW1iU05CZzo4MDA0';

const mockCreatedSavedObject = {
...mockSavedObject,
attributes: {},
};

savedObjectsClient.update.mockRejectedValue(
SavedObjectsErrorHelpers.createGenericNotFoundError(sessionId)
);
savedObjectsClient.create.mockResolvedValue(mockCreatedSavedObject);

await service.trackId({ savedObjectsClient }, mockUser1, searchRequest, searchId, {
sessionId,
strategy: MOCK_STRATEGY,
});

expect(savedObjectsClient.update).toHaveBeenCalled();
expect(savedObjectsClient.create).toHaveBeenCalled();

const [type, callAttributes, options] = savedObjectsClient.create.mock.calls[0];
expect(type).toBe(SEARCH_SESSION_TYPE);
expect(options).toStrictEqual({ id: sessionId });
expect(callAttributes).toHaveProperty('idMapping', {
[requestHash]: {
id: searchId,
strategy: MOCK_STRATEGY,
},
});
expect(callAttributes).toHaveProperty('expires');
expect(callAttributes).toHaveProperty('created');
expect(callAttributes).toHaveProperty('sessionId', sessionId);
});

it('retries updating if update returned 404 and then update returned conflict 409 (first create race condition)', async () => {
const searchRequest = { params: {} };
const searchId = 'FnpFYlBpeXdCUTMyZXhCLTc1TWFKX0EbdDFDTzJzTE1Sck9PVTBIcW1iU05CZzo4MDA0';

const mockUpdateSavedObject = {
...mockSavedObject,
attributes: {},
};

let counter = 0;

savedObjectsClient.update.mockImplementation(() => {
return new Promise((resolve, reject) => {
if (counter === 0) {
counter++;
reject(SavedObjectsErrorHelpers.createGenericNotFoundError(sessionId));
} else {
resolve(mockUpdateSavedObject);
}
});
});

savedObjectsClient.create.mockRejectedValue(
SavedObjectsErrorHelpers.createConflictError(SEARCH_SESSION_TYPE, searchId)
);

await service.trackId({ savedObjectsClient }, mockUser1, searchRequest, searchId, {
sessionId,
strategy: MOCK_STRATEGY,
});

expect(savedObjectsClient.update).toHaveBeenCalledTimes(2);
expect(savedObjectsClient.create).toHaveBeenCalledTimes(1);
});

it('retries everything at most MAX_RETRIES times', async () => {
const searchRequest = { params: {} };
const searchId = 'FnpFYlBpeXdCUTMyZXhCLTc1TWFKX0EbdDFDTzJzTE1Sck9PVTBIcW1iU05CZzo4MDA0';

savedObjectsClient.update.mockRejectedValue(
SavedObjectsErrorHelpers.createGenericNotFoundError(sessionId)
);
savedObjectsClient.create.mockRejectedValue(
SavedObjectsErrorHelpers.createConflictError(SEARCH_SESSION_TYPE, searchId)
);

await service.trackId({ savedObjectsClient }, mockUser1, searchRequest, searchId, {
sessionId,
strategy: MOCK_STRATEGY,
});

expect(savedObjectsClient.update).toHaveBeenCalledTimes(MAX_UPDATE_RETRIES);
expect(savedObjectsClient.create).toHaveBeenCalledTimes(MAX_UPDATE_RETRIES);
const [, , , opts] = savedObjectsClient.update.mock.calls[0];
expect(opts).toHaveProperty('retryOnConflict', MAX_UPDATE_RETRIES);
});

it('batches updates for the same session', async () => {
Expand Down
Loading

0 comments on commit a6b1c7a

Please sign in to comment.