Skip to content

[Search Sessions] Optimize search session so updates#142850

Merged
Dosant merged 6 commits intoelastic:mainfrom
Dosant:d/2022-10-06-improve-search-update
Oct 11, 2022
Merged

[Search Sessions] Optimize search session so updates#142850
Dosant merged 6 commits intoelastic:mainfrom
Dosant:d/2022-10-06-improve-search-update

Conversation

@Dosant
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

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

@Dosant Dosant requested review from lukasolson and ppisljar October 6, 2022 14:52
@Dosant
Copy link
Copy Markdown
Contributor Author

Dosant commented Oct 10, 2022

@elasticmachine merge upstream

Copy link
Copy Markdown
Contributor

@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
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
Contributor Author

Dosant commented Oct 11, 2022

@elasticmachine merge upstream

@Dosant Dosant enabled auto-merge (squash) October 11, 2022 14:41
@Dosant
Copy link
Copy Markdown
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
Copy Markdown

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

Copy link
Copy Markdown
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 PR 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