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 Sessions] Optimize search session so updates #142850

Merged
merged 6 commits into from
Oct 11, 2022

Conversation

Dosant
Copy link
Contributor

@Dosant Dosant commented Oct 6, 2022

Summary

close #141765

This is a follow-up to #139349 that improves search-session saved object update code. This is what was done and why:

  1. Since [Search/Search Sessions] Stabilization Stage I #139349 we explicitly create a search-session object, we can simplify old create/update logic, by splitting create and update (remove updateOrCreate method)
  2. As we split create and update and as SavedObjects update can cause version conflict errors even when not using optimistic concurrency control #126240 was fixed, we remove custom retry on conflict logic. Now we can rely on elasticsearch's retryOnConflict.
  3. Change default sessionConfig.maxUpdateRetries from 3 to 10. This is done because the old "retry" default was actually 3 on Kibana side (maxUpdateRetries) * 3 on es side (default in so client) = 9. Since we removed the custom retry code on the Kibana side, to stick to older behavior it makes sense to make it 9 on es side. Rounding to 10 for a nicer number.
  4. Improve batching/debounce session saved object updates code:
    4.1. Group batching/debounce per sessionId
    4.2. Make batching explicit for trackId call only because this is where we actually need this optimization. (After the user saves the session all search requests are immediately polled to be saved into the just created search-session saved object)
  5. Some debug logs improvements

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2022

Documentation preview:

@Dosant Dosant changed the title improve search session updates [Search Sessions] Optimize search session so updates Oct 6, 2022
@Dosant Dosant added Feature:Search Querying infrastructure in Kibana Team:AppServicesSv release_note:skip Skip the PR/issue when compiling release notes Feature:Search Sessions labels Oct 6, 2022
@Dosant Dosant marked this pull request as ready for review October 6, 2022 14:52
@Dosant Dosant requested a review from a team as a code owner October 6, 2022 14:52
@elasticmachine
Copy link
Contributor

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

@Dosant
Copy link
Contributor Author

Dosant commented Oct 10, 2022

@elasticmachine merge upstream

Copy link
Member

@lukasolson lukasolson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

const batchedIdMapping = queue.reduce((res, next) => {
res[next.requestHash] = next.searchInfo;
return res;
}, {} as SearchSessionSavedObjectAttributes['idMapping']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Instead of casting, use queue.reduce<SearchSessionSavedObjectAttributes['idMapping']>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always forget about this! thanks

@Dosant
Copy link
Contributor Author

Dosant commented Oct 11, 2022

@elasticmachine merge upstream

@Dosant Dosant enabled auto-merge (squash) October 11, 2022 14:41
@Dosant
Copy link
Contributor Author

Dosant commented Oct 11, 2022

@elasticmachine merge upstream

@kibanamachine kibanamachine requested a review from a team as a code owner October 11, 2022 16:15
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Member

@legrego legrego left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import reformatting in x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/feature_table/feature_table.tsx LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Search Sessions Feature:Search Querying infrastructure in Kibana release_note:skip Skip the PR/issue when compiling release notes v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use partial updates to avoid saved object conflicts
6 participants