Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Search Sessions] Optimize search session so updates #142850

Merged
merged 6 commits into from
Oct 11, 2022
Merged

[Search Sessions] Optimize search session so updates #142850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7b6c0d0
improve search session updates
Dosant Oct 6, 2022
a8639a1
Merge branch 'main' into d/2022-10-06-improve-search-update
kibanamachine Oct 10, 2022
b40b95c
Merge branch 'main' into d/2022-10-06-improve-search-update
kibanamachine Oct 11, 2022
850d08b
improve types
Dosant Oct 11, 2022
386cccd
Merge branch 'main' into d/2022-10-06-improve-search-update
kibanamachine Oct 11, 2022
557da66
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Oct 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/settings/search-sessions-settings.asciidoc
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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