Search Sessions Stabilization Stage I

[Dosant]

Summary

Close #139349

This pr changes search service/search session infrastructure to improve performance, stability, and resiliency by ensuring that search sessions don’t add additional load on a cluster when the feature is not used

Details on motivation and implementation details in the RFC

Comparison of old vs. new implementation. More details in the RFC

	Old implementation	New implementation
Number of task manager tasks	3	0
Initial search keep_alive	1 week	1 minute, extended while polling
Search cancelation	Task manager task	Automatically deleted due to short keep_alive. In some case expedited faster from browser
Search-session saved object creation	For every search search even if user doesn’t save it	After user saves a session
Search-session status calculation	Task manager task with 10 second interval. Status is calculated and saved in saved object	Status is calculated on-the-fly when user retrieves search-sessions objects
Search-session clean-up	Sessions that are not saved by user cleaned-up by task manager	None. Because we create saved object only for sessions that saved by user
Saving session after search requests complete	5 minutes limit because task manager might have already deleted searches and session object	5 minutes artificial client-side limit to prevent keeping alive searches from "forgotten" tabs. The client continues polling on non-frequent intervals to keep the search alive while the user stays on the screen

Advantages of the new implementation:

• No task manager usage
  • Less Kibana server load
• Short default keep_alive
  • Less risk to end up with unintentional long-running search
• Saved objects created only when user saves a session
  • Less cluster load. Less storage needs.
• No need for “touched” property is search-session saved object
  • No more object updates on every search poll.
• On-the-fly calculated search-session status
  • No need for task manager to keep state in sync
  • Less synchronization delays / potential bugs

Performace benchmark
The trend will be more visible in nightly performance tests after we merge, but I run kibana-load-test dashboard scenario a couple of times and I believe the difference is more then just a margin of error:

	Total	OK	KO	% KO	Cnt/s	Min	50th pct	75th pct	95th pct	99th pct	Max	Mean	Std Dev
Main	52088	52088	0	0%	108.066	9	1775	2384	6201	8895	11273	1968	1738
Branch	56899	56899	0	0%	123.693	9	1192	1972	4089	7057	8886	1414	1337

• main https://kibana-ci.elastic.co/view/Kibana/job/elastic+kibana+load-testing/2005/gcsObjects/
• branch https://kibana-ci.elastic.co/view/Kibana/job/elastic+kibana+load-testing/2004/gcsObjects/

In this scenario, the branch performs better because there are no search-session monitoring tasks and no redundant search-session saved object creations and updates.

Minor user-facing changes:

• Management screen:
• Sessions list auto-refresh is disabled by default because the list is more expensive to fetch as we calculate session status on the fly
• For the same reason, we fetch only the last 100 sessions of the current user
• Saving the session is disabled when we continue the session from lens (or another editor). This should be revised with Separate architecture for client side cache sharing #121543
• Errored session will be in "expired" status if current time > then expired time. We do this to shortcut search status checks if the session is expired.

Nice to have follows-ups (Out of scope):

• ES should add completion_time to search status so we can add back completion time when restoring a session Add completion_time time field to async_search get or status response elasticsearch#88640
• IsRestore / IsStored / isStore search options are confusing. Make them clearer consider splitting what is session related and what is search related.
• Can simplify updateOrCreate logic server-side because now we create and update separately. This will improve performance and number of update retries. TODO create an improvement issue
• New searches added to a session when restoring (other bucket) would use default expiration, so they wouldn't match the session's expiration.

Progress tracking

• Features:
  • Add back support for TSVB / timelion (commented for now) (data-vis-team) [TSVB][Timelion] Restart request on saved session #139094
• Improvements
  • Improve useInterval on mgmt screen to be more robust . Restart the interval only when previous request completes. Refactor search session status on-the-fly calculation and other fixes #136296
  • Disable sessions auto-refresh by default Refactor search session status on-the-fly calculation and other fixes #136296
  • Make sure we not over-fetch sessions on mgmt screen pages. Limit 10000 -> 100? Remove auto-refresh? Refactor search session status on-the-fly calculation and other fixes #136296
  • Check with es team if they can add completion time to a async search response , so we could add back session completion time Add completion_time time field to async_search get or status response elasticsearch#88640
  • Don't checkId server-side. Pass state from client instead use isSearchStored param instead of server-side checkId #135036
• Clean up
  • Refactor session status: Calculating status on the fly is a bit messy now because added it only to get() and findAll(), so it is inconsisten between different APIs. Consider a separate API for status? Refactor search session status on-the-fly calculation and other fixes #136296
  • IsRestore / IsStored / isStore are confusing. Make them clearer consider splitting what is session related and what is search related. out of scope of this pr. Create a separate issue
  • Can simplify updateOrCreate logic server-side because now we create and update separately. out of scope of this pr. Create a separate issue
• Bugs / edge cases / things to double check:
  • trackSearch and cache: Check that moving trackSearch() in search interceptor inside runSearch$ didn't break anything. The difference is that now cached searches aren't tracked (Tested, seems to be OK)
  • Sessions sharing edge case (Lens / visualize continue dashboard session to use search client side cache. This interferes with session saving). We had to disallow saving sessions that were continued from editors: Refactor search session status on-the-fly calculation and other fixes #136296, better fix should be done in Separate architecture for client side cache sharing #121543
  • keep_alive loop when the tab is suspended: Test / fix when tab goes to sleep and then getting back. keep_alive loop would likely start failing with 404 and need to make sure it gracefully handles it https://developer.chrome.com/blog/timer-throttling-in-chrome-88/
  • Stopping / cancelling a session (tested in Refactor search session status on-the-fly calculation and other fixes #136296, works, but there is also a bug in main that the "cancel" button is available when you restore a session and it doesn't work properly. Needs a separate issue)
  • Bug with disabledSave$ - works only for the first session when it emits, then it emits right away so polling no longer works. Should we get rid of disableSave$ completely?
  • Extend from mgmt works and extends searches
  • Cancel from mgmt works and cleans up searches (both for in-progress and completed sessions)
  • Restoring a session doesn't automatically extend searches
    • New searches added to a session when restoring (other bucket) would use default expiration, so they wouldn't match the session's expiration. TODO: create a bug issue
  • Other bucket / new requests when restoring a session (tested in Refactor search session status on-the-fly calculation and other fixes #136296)
  • can disabled sessions
  • works in different space
  • works if user doesn't have access to sessions
• Unit Tests:
  • client search_interceptor tests:
    • comment out and skip everything related to untrack to make it compile
    • skipped failing tests related to abort + delete as logic was changed. Need to double check everything there
    • Add tests around search tracking
  • client search_session_state tests:
    • Add tests around search tracking
  • client session_service
    • Add tests around search tracking and polling
    • Add tests for disabledSave$ + tracker
  • server search service - unskip and change tests around track id Refactor search session status on-the-fly calculation and other fixes #136296
  • server Review unit tests of server/search_service Refactor search session status on-the-fly calculation and other fixes #136296
  • server strategies tests (some skipped tests) Refactor search session status on-the-fly calculation and other fixes #136296
  • server async_utils tests (skipped) Refactor search session status on-the-fly calculation and other fixes #136296
  • check session status / check search status
• Functional / API tests
  • unskip apis search search session - (unskip status tests)
  • unskip New search sessions Saves a session and verifies it in the Management app
    • Expired status takes precedence over error status now. So had to adjust
    • Fixed bug with cache hit when restoring a session (TODO: attach issue link)
  • Add a test that checks that searches are created with 1m keep_alive and then extended when session is saved
  • Add a test that checks that searches are delete when session is deleted or cancelled
  • Add a test that checks that searches are extended when session is extended
  • Add a test that checks that search with other bucket is added to a session Add restoring other bucket test, fix bug with deleting restoring search #137260
• Migrations:
  • Add migrations
    • Remove touched? Search session object migrations  #137272
    • Remove search status? Search session object migrations  #137272
    • Remove session status? Search session object migrations  #137272
    • Remove completed, because no longer possible to determine Add completion_time time field to async_search get or status response elasticsearch#88640 Search session object migrations  #137272
  • check that old not persisted sessions are not migrated to a newer version as persisted
• Configs:
  • Review and remove unused config options [ignore - goes into feature branch] Search session configs clean up #137469
• Async search status API requires the cluster privileges [monitor,manage,all]
  • Exception message security_exception: [security_exception] Reason: action [cluster:monitor/async_search/status] is unauthorized for user [analyst] with roles [data_analyst], this action is granted by the cluster privileges [monitor,manage,all]
  • Solved by using internal client

Testing Scenarios

Some advanced scenarios I've went through and to give you an idea what to look for:

• Extend from mgmt works and extends sessions and searches
• Deleting a saved session also deletes/cancels related async searches
• When restoring a session, searches aren't extended further
• When restoring a session and new searches are added - they are added to a session and warning is shown
• Searches start with 1 minutes keep_alive, but after the session is saved are extended to 1 week (by default)
• Everything works in a different space
• Everything works when user doesn't have access to save a session
• Searches work when the session feature is disabled (data.search.session.enabled:false). The keep_alive is 1 minute, and no client-side polling to extend complete searches
• Migrations: old non-persisted sessions are dropped; persisted are migrated

Testing tips

Get search sessions in dev tools

GET .kibana*/_search
{
  "query": {
    "match": {
      "type": "search-session"
    }
  }
}

Check async search status

Look at idMapping object in search session attributes. Values would have id which is async_search_id from elasticsearch.

Use status API to check search's expiration_time and completion status:

GET /_async_search/status/FmRldE8zREVEUzA2ZVpUeGs2ejJFUFEaMkZ5QTVrSTZSaVN3WlNFVmtlWHJsdzoxMDc=

Checklist

Delete any items that are not applicable to this PR.

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
• Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
• If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
• This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
• This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Worth testing in multiple spaces
Search session migrations	Medium	High	Old, not persisted sessions should be dropped. New should be migrated

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[Dosant]

@elasticmachine merge upstream

[Dosant]

@elasticmachine merge upstream

[Dosant]

@elasticmachine merge upstream

[Dosant]

@elasticmachine merge upstream

[Dosant]

@elasticmachine merge upstream

[Dosant]

@elasticmachine merge upstream

[Dosant]

@elasticmachine merge upstream

[Dosant]

@elasticmachine merge upstream

[Dosant]

@elasticmachine merge upstream

[elasticmachine]

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

[Dosant]

@elasticmachine merge upstream

[watson]

Kibana Platform Security changes LGTM 👍

[Dosant]

@elasticmachine merge upstream

[ymao1]

Response Ops changes LGTM. Saw the tasks get marked as "unrecognized".

[ThomThomson]

Functional test changes LGTM! Code review only.

[Dosant]

@elasticmachine merge upstream

[flash1293]

I checked the timelion integration and just making sure I'm getting this correctly - if the session is saved, then a new request is kicked off so the timelion server can store it, but as long as the user stays on the dashboard, it's still "waiting" for the original request which isn't cancelled, right? If that's the case, then LGTM

[ppisljar]

lgtm

[Dosant]

@elasticmachine merge upstream

[Dosant]

I checked the timelion integration and just making sure I'm getting this correctly - if the session is saved, then a new request is kicked off so the timelion server can store it, but as long as the user stays on the dashboard, it's still "waiting" for the original request which isn't cancelled, right?

Right. This is expected.
I think this is OK because of a smaller bug surface as the implementation is simpler. Also potentially users may get the on-screen results earlier (without waiting for the second request to finish)

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: e77c66f

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
data	521	520	-1

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
data	2509	2513	+4

Async chunks

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

id	before	after	diff
data	52.7KB	52.4KB	-291.0B
inspector	15.9KB	16.0KB	+55.0B
visTypeTimelion	109.1KB	109.2KB	+92.0B
total			-144.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
data	23	24	+1

Page load bundle

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

id	before	after	diff
data	433.2KB	437.0KB	+3.9KB
visTypeTimeseries	19.4KB	19.5KB	+97.0B
total			+4.0KB

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/saved-objects-service.html#_mappings

id	before	after	diff
search-session	15	12	-3

Unknown metric groups

API count

id	before	after	diff
data	3213	3221	+8

ESLint disabled line counts

id	before	after	diff
data	46	52	+6

Total ESLint disabled count

id	before	after	diff
data	48	54	+6

History

• 💔 Build #77822 failed 46366c5
• 💚 Build #76384 succeeded 6941f6f
• 💚 Build #75944 succeeded 280428f
• 💚 Build #75603 succeeded afe8dfb
• 💛 Build #74793 was flaky ba240d3

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