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] Save all sessions, with persisted flag #89570

Merged
merged 84 commits into from
Feb 3, 2021

Conversation

lizozom
Copy link
Contributor

@lizozom lizozom commented Jan 28, 2021

Summary

Depends on #87966 (@kibana-app and @elastic/logs-ui no need to review)

This PR introduces two changes to the Search Sessions feature architecture:

  • Previously, our code was holding async search ids in memory until they are saved by the user. This PR instead, immediately saves any search session as a saved object.
  • Previously ,we were saving all async searches for 1m and extending them as the browser polls for results. This PR instead stores async searches (that are a part of a session) for 7d by default.

Problems solved

  • There was a small risk of data loss if a server is shut down before a user saves a search session
  • There was a risk of race conditions while extending the async searches
  • There is no API for extending async_searches on behalf of the user, meaning we had to store the user's credientials somehow to extend on their behalf. This resolves this problem too.
  • The previous implementation had a risk of using too much memory. We don't use server memory anymore in this PR.

Limitations and things to be aware of.

  • Elasticsearch now relies on Kibana to clear any unsaved sessions. Otherwise data will remain in the .async_search index for 7 days. This is resolved by using the monitoring task to clear out abandoned search sessions.
    • Running sessions, will be killed and cleared after 1m of inactivity (set in notTouchedTimeout).
    • Completed sessions will be cleared after 5m (Set in completedTimeout), to allow users to "Send to backround" on sessions that have already finished running.
  • Since savedObjectsClient does not expose an atomic updateOrCreate function using upset, we had to implement a logic using retries. This might result in less than optimal performance.
  • There will be many more search sessions saved objects now. To handle that, this PR adds support for saved object pagination (which was previously postponed to 7.13)

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@lizozom lizozom changed the title Sessions/save all sessions [Search Sessions] Save all sessions, with persisted flag Jan 28, 2021
@lizozom lizozom self-assigned this Jan 28, 2021
@lizozom lizozom mentioned this pull request Feb 3, 2021
9 tasks
@lizozom lizozom requested a review from a team February 3, 2021 12:59
@lizozom lizozom requested a review from a team as a code owner February 3, 2021 12:59
@lizozom
Copy link
Contributor Author

lizozom commented Feb 3, 2021

@elastic/kibana-app and @elastic/logs-ui please ignore the review request.
It happened due to merging another PR into this one. I will rebase after #87966 is merged.

@lizozom lizozom requested review from lukasolson and removed request for a team February 3, 2021 16:01
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
data 614 613 -1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
dataEnhanced 150.8KB 150.6KB -265.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
data 798.4KB 797.9KB -500.0B
dataEnhanced 35.9KB 33.6KB -2.4KB
total -2.9KB

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/development-plugin-saved-objects.html#_mappings

id before after diff
search-session 8 10 +2

History

To update your PR or re-run it, just comment with:
@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!

@lizozom lizozom merged commit 7fbcf68 into elastic:master Feb 3, 2021
lizozom added a commit to lizozom/kibana that referenced this pull request Feb 3, 2021
* [data.search] Add search session methods to search service contract

* Fix types

* Fix tests and switch to cancel

* Update docs

* Fix types/tests

* Fix tests

* Update status of SO before cancelling search requests

* Add API integration test

* Fix types

* Update expiration route to use config defaultExpiration

* Fix test

* Update docs

* New logic for extend

* Remove declare module

* Search Sessions: Unskip Flaky Functional Test

* Review feedback

* fix ts

* Save all search sessions and then manage them based on their persisted state

* Get default search session expiration from config

* randomize sleep time

* fix test

* Remove test that is no longer valid

* fix test

* Make sure we poll, and dont persist, searches not in the context of a session

* Added keepalive unit tests

* fix ts

* code review @lukasolson

* ts

* More tests, rename onScreenTimeout to completedTimeout

* lint

* lint

* Delete async seaches

* Support saved object pagination
Fix get search status tests

* better PersistedSearchSessionSavedObjectAttributes ts

* test titles

* Fix undefined bug

* Remove runAt from monitoring task
Increase testing trackingInterval (caused bug)

* support workload histograms that take into account overdue tasks

* Update touched when changing session status to complete \ error

* removed test

* Updated management test data

* Rename configs

* delete tap first
add comments

* Use DataRequestHandlerContext in maps

* ts

* Fixed ts

Co-authored-by: Lukas Olson <olson.lukas@gmail.com>
Co-authored-by: Timothy Sullivan <tsullivan@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Anton Dosov <anton.dosov@elastic.co>
Co-authored-by: Gidi Meir Morris <github@gidi.io>
lizozom added a commit that referenced this pull request Feb 4, 2021
…0236)

* [data.search] Add search session methods to search service contract

* Fix types

* Fix tests and switch to cancel

* Update docs

* Fix types/tests

* Fix tests

* Update status of SO before cancelling search requests

* Add API integration test

* Fix types

* Update expiration route to use config defaultExpiration

* Fix test

* Update docs

* New logic for extend

* Remove declare module

* Search Sessions: Unskip Flaky Functional Test

* Review feedback

* fix ts

* Save all search sessions and then manage them based on their persisted state

* Get default search session expiration from config

* randomize sleep time

* fix test

* Remove test that is no longer valid

* fix test

* Make sure we poll, and dont persist, searches not in the context of a session

* Added keepalive unit tests

* fix ts

* code review @lukasolson

* ts

* More tests, rename onScreenTimeout to completedTimeout

* lint

* lint

* Delete async seaches

* Support saved object pagination
Fix get search status tests

* better PersistedSearchSessionSavedObjectAttributes ts

* test titles

* Fix undefined bug

* Remove runAt from monitoring task
Increase testing trackingInterval (caused bug)

* support workload histograms that take into account overdue tasks

* Update touched when changing session status to complete \ error

* removed test

* Updated management test data

* Rename configs

* delete tap first
add comments

* Use DataRequestHandlerContext in maps

* ts

* Fixed ts

Co-authored-by: Lukas Olson <olson.lukas@gmail.com>
Co-authored-by: Timothy Sullivan <tsullivan@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Anton Dosov <anton.dosov@elastic.co>
Co-authored-by: Gidi Meir Morris <github@gidi.io>

Co-authored-by: Lukas Olson <olson.lukas@gmail.com>
Co-authored-by: Timothy Sullivan <tsullivan@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Anton Dosov <anton.dosov@elastic.co>
Co-authored-by: Gidi Meir Morris <github@gidi.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Search Querying infrastructure in Kibana release_note:skip Skip the PR/issue when compiling release notes Team:EnterpriseSearch v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants