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.

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

[Search] Review searches clean up #105441

Closed
Dosant opened this issue Jul 13, 2021 · 3 comments
Closed

[Search] Review searches clean up #105441

Dosant opened this issue Jul 13, 2021 · 3 comments
Labels
duplicate Feature:Search Sessions Feature:Search Querying infrastructure in Kibana impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:large Large Level of Effort performance

Comments

@Dosant
Copy link
Contributor

Dosant commented Jul 13, 2021

We should review how and when our searches are currently cleaned up. This includes a strategy for handling discarded searches and reviewing our timeout settings

related: #106395, #106400

Currently, all sessions (both persisted and non-persisted) are cleaned up in the monitoring task

if (isSessionStale(session, config)) {
// delete saved object to free up memory
// TODO: there's a potential rare edge case of deleting an object and then receiving a new trackId for that same session!
// Maybe we want to change state to deleted and cleanup later?
logger.debug(`Deleting stale session | ${session.id}`);
try {
deletedSessionIds.push(session.id);
await savedObjectsClient.delete(SEARCH_SESSION_TYPE, session.id, {
namespace: session.namespaces?.[0],
});
} catch (e) {
logger.error(
`${SEARCH_SESSIONS_CLEANUP_TASK_TYPE} Error while deleting session ${session.id}: ${e.message}`
);
}
// Send a delete request for each async search to ES
Object.keys(session.attributes.idMapping).map(async (searchKey: string) => {
const searchInfo = session.attributes.idMapping[searchKey];
if (searchInfo.strategy === ENHANCED_ES_SEARCH_STRATEGY) {
try {
await client.asyncSearch.delete({ id: searchInfo.id });
} catch (e) {
if (e.message !== 'resource_not_found_exception') {
logger.error(
`${SEARCH_SESSIONS_CLEANUP_TASK_TYPE} Error while deleting async_search ${searchInfo.id}: ${e.message}`
);
}
}
}
});
}
})
but we can do it earlier from the client.

Also, existing cleanup from the client is not centralized and causes an out-of-sync state between searches in elasticsearch and kibana's search sessions saved objects #104309, #98698

Possible existing issue 1

On the client we abort unfinished searches if they don't belong to a saved session:

const cancel = once(() => {
if (id && !isSavedToBackground) this.deps.http.delete(`/internal/search/${strategy}/${id}`);
});

This is causing a desync issue, as we don't delete search from non-persisted sessions here

Possible existing issue 2

We can delete a search session

if (isStoredSession) {
await this.sessionsClient.delete(this.state.get().sessionId!);
}

But since this is happening separately, searchers might get out of sync and still be executing

Possible existing issue 3

Does server side search strategy handles request cancelation? #113423


There was a pr that should have addressed this: #99658 but new use-case come up where search session is reused between Dashboard<->Lens and it needs a different approach now.

@Dosant Dosant added Feature:Search Querying infrastructure in Kibana enhancement New value added to drive a business result Team:AppServices labels Jul 13, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Jul 19, 2021
@exalate-issue-sync exalate-issue-sync bot added impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. and removed impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. labels Jul 29, 2021
@lukasolson lukasolson added performance and removed enhancement New value added to drive a business result labels Sep 28, 2021
@exalate-issue-sync exalate-issue-sync bot added loe:medium Medium Level of Effort and removed loe:small Small Level of Effort labels Sep 28, 2021
@exalate-issue-sync exalate-issue-sync bot changed the title [Search Sessions] Cleaning up abandoned sessions from the client [Search Sessions] Cleaning up abandoned sessions from the client Nov 18, 2021
@Dosant Dosant changed the title [Search Sessions] Cleaning up abandoned sessions from the client [Search] Consistent search clean up strategy Nov 30, 2021
@Dosant Dosant changed the title [Search] Consistent search clean up strategy [Search] Review searches clean up Nov 30, 2021
@exalate-issue-sync exalate-issue-sync bot added impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:large Large Level of Effort and removed impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:medium Medium Level of Effort labels Nov 30, 2021
@lizozom
Copy link
Contributor

lizozom commented Apr 18, 2022

@Dosant It feels like there a lot of duplucates of this issue (for example #106395).
I'd like to leave one issue open, close the others and add it to the performance project board. Do you think that's possible?

@Dosant
Copy link
Contributor Author

Dosant commented Apr 19, 2022

@lizozom, thanks! let's close this one in favor of #106395

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate Feature:Search Sessions Feature:Search Querying infrastructure in Kibana impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:large Large Level of Effort performance
Projects
None yet
Development

No branches or pull requests

4 participants