Refactor search session status on-the-fly calculation and other fixes #136296

Dosant · 2022-07-13T14:26:45Z

Summary

Refactor initial on-the-fly status implantation:
- Instead of augmenting status field in a saved object, return it as part of a separate map in find API
- Instead of new param in get API, use separate status API
Mgmt screen changes to prevent overfetching: by default disable auto-refresh and reduce limit of items to 100
- Also fix sorting of search sessions in API request
- Fix sorting of options in mgmt filters
Better handle edge case with session sharing: Lens and visualize "continue" search sessions to share a client-side search response cache. This functionality already had issues in the past (Separate architecture for client side cache sharing #121543) but with new search/session implementation it gets worse:
- 1. When you in editor, we need to make sure we don't keep searches alive because it won't be possible to save. To fix this I added a check when keeping searches alive that this session can be saved by the current app
- 1. When you navigate back to dashboard and "continue" the session the client side "trackedSearches" are dropped, so it won't be possible to keep search alive or to add them to a session. To fix this - I started disabling "save" button if session was continued. I think it is better than letting users be wrong that something went wrong than they try to restore a session where we knew it probably didn't have all the searches saved with it.
Fix and unskip most of the unit tests, add more functional tests

…astic/kibana into d/2022-07-11-search-sessions-status-refactor

Dosant · 2022-07-18T13:15:54Z

src/plugins/data/public/search/session/sessions_mgmt/components/table/app_filter.tsx

-    return options;
-  }, []),
+  options: [...new Set(tableData.map((data) => data.appId ?? 'unknown'))]
+    .sort()


After fixing sort order for the find some UI session tests started failing. They failed because the order of this option has changed. I fixed it by starting sorting the options to make them consistent. Also slightly refactored the function

Dosant · 2022-07-18T13:24:03Z

src/plugins/data/public/search/session/sessions_mgmt/components/table/table.tsx


  // initial data load
  useEffect(() => {
    doRefresh();
    searchUsageCollector.trackSessionsListLoaded();
  }, [doRefresh, searchUsageCollector]);

-  useInterval(doRefresh, refreshInterval);


Improvements to fetching on the management screen:

By default disable auto-refresh

useInterval was refactored to a setTimeout, so that we had control of scheduling a new timeout only after ongoing refresh completes.

Also added a shortcut if (document.visibilityState !== 'hidden') { to skip refresh in case page is not visible.

Dosant · 2022-07-18T13:25:57Z

src/plugins/data/public/search/session/sessions_mgmt/lib/api.ts

@@ -141,7 +126,7 @@ export class SearchSessionsMgmtAPI {
        page: 1,
        perPage: mgmtConfig.maxSessions,
        sortField: 'created',
-        sortOrder: 'asc',


I think this was a mistake. We fetched all sessions and did client-side sorting. Since we changed the default max count from 10000 to 100 we need to properly sort in es

Dosant · 2022-07-18T13:26:16Z

src/plugins/data/server/search/routes/session.ts

+
+  router.get(
+    {
+      path: '/internal/session/{id}/status',


this is used only in api tests. this is instead checkStatus param in get because I think this is clearer and doesn't argument saved object response

elasticmachine · 2022-07-18T13:42:34Z

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

…astic/kibana into d/2022-07-11-search-sessions-status-refactor

Dosant · 2022-07-19T14:15:48Z

src/plugins/data/public/search/session/session_service.ts

+  public readonly disableSaveAfterSessionCompleteTimedOut$ =
+    this._disableSaveAfterSessionCompleteTimedOut$.asObservable();
+
+  private readonly _disableSaveAfterSessionContinuedFromDifferentApp$ =


See description why I think we have to add this new disable condition

Dosant · 2022-07-25T10:53:41Z

@elasticmachine merge upstream

…1-search-sessions-status-refactor

…astic/kibana into d/2022-07-11-search-sessions-status-refactor

….com:Dosant/kibana into d/2022-07-11-search-sessions-status-refactor

Dosant · 2022-07-25T13:35:20Z

src/plugins/data/public/search/search_interceptor/search_interceptor.ts

@@ -266,10 +277,27 @@ export class SearchInterceptor {
        })
      : undefined;

+    // track if this search's session will be send to background
+    // if yes, then we don't need to cancel this search when it is aborted
+    let isSavedToBackground = false;


While unskipping the remaining tests, I realized that this older implementation from main had some benefits. So I decided to get it back and unskip related tests

Dosant · 2022-07-25T14:21:20Z

buildlkite, test this

Dosant · 2022-07-25T15:39:15Z

buildkite, test this

kibana-ci · 2022-07-25T16:32:52Z

💔 Build Failed

Failed CI Steps

Test Failures

[job] [logs] FTR Configs #8 / apps machine learning basic license data visualizer index based with farequote lucene saved search displays index details
[job] [logs] FTR Configs #8 / apps machine learning basic license data visualizer index based with view in lens action farequote index pattern loads lens charts

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`data`	516	515	-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`	2423	2422	-1

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.4KB	52.2KB	-234.0B

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id	before	after	diff
`module count`	4935	4934	-1

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`	21	22	+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`	429.5KB	430.3KB	+764.0B

Unknown metric groups

API count

id	before	after	diff
`data`	3099	3101	+2

ESLint disabled line counts

id	before	after	diff
`data`	65	66	+1

Total ESLint disabled count

id	before	after	diff
`data`	67	68	+1

History

💔 Build #60344 failed 01f4b5d
💔 Build #60286 failed cdbcc7d
💔 Build #60255 failed bd6664d
💚 Build #59941 succeeded c0f7b39
💔 Build #59900 failed 266f27c

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

lukasolson

Thanks for walking me through these changes, LGTM

lukasolson · 2022-07-25T21:39:44Z

src/plugins/data/common/search/session/types.ts

+  /**
+   * Map containing calculated statuses of search sessions from the find response
+   */
+  statuses: Record<string, SearchSessionStatusResponse>;


Nit: Instead of something like savedObjects: [...], statuses: [...] I think it would be easier to use if it were more like searchSessions: [{savedObject: {...}, status: 'status'}].

Good suggestion 👍

I'll take a look if it is worth update,
I think I didn't go this route initially because we use SavedObjectFindResponse with a bunch of other properties, and instead of just extending it, I'd had to deep update it

Dosant added 4 commits July 13, 2022 16:23

refactor session status

e716aca

Merge branch 'search-sessions-stabilization-stage-1' of github.com:el…

04f101b

…astic/kibana into d/2022-07-11-search-sessions-status-refactor

fix lint

9ca1b0c

fix filter sorting

ad4cb7c

Dosant mentioned this pull request Jul 14, 2022

Search Sessions Stabilization Stage I #134983

Merged

84 tasks

Dosant added 2 commits July 14, 2022 16:51

wip: fixes and tests clean up

4ec451d

remove useInterval as no longer needed because of timeout implementation

bf274d5

Dosant commented Jul 18, 2022

View reviewed changes

Dosant added Feature:Search Querying infrastructure in Kibana Team:AppServicesSv labels Jul 18, 2022

Dosant requested a review from lukasolson July 18, 2022 13:42

Dosant marked this pull request as ready for review July 18, 2022 13:42

Dosant requested a review from a team as a code owner July 18, 2022 13:42

Dosant added 2 commits July 19, 2022 13:46

Merge branch 'search-sessions-stabilization-stage-1' of github.com:el…

54a8071

…astic/kibana into d/2022-07-11-search-sessions-status-refactor

fix edge cases for session sharing

c30ef0d

Dosant changed the title ~~Refactor search session status on-the-fly calculation~~ Refactor search session status on-the-fly calculation and other fixes Jul 19, 2022

fixes

420b57e

Dosant commented Jul 19, 2022

View reviewed changes

Dosant added 8 commits July 19, 2022 17:22

fix mgmt timeout

70bafb1

fix adding new search into session when restoring

0f707b7

more functional tests

0a42866

don’t extend keep_alive if restoring searches

9839e33

fix functional test

4925092

disable mgmt autorefresh with a config

1ef1b69

clean up

266f27c

fix ts

c0f7b39

kibanamachine and others added 2 commits July 25, 2022 06:53

Merge branch 'search-sessions-stabilization-stage-1' into d/2022-07-1…

bd6664d

…1-search-sessions-status-refactor

Merge branch 'search-sessions-stabilization-stage-1' of github.com:el…

31dcf59

…astic/kibana into d/2022-07-11-search-sessions-status-refactor

Dosant added 2 commits July 25, 2022 14:03

Merge branch 'd/2022-07-11-search-sessions-status-refactor' of github…

cdbcc7d

….com:Dosant/kibana into d/2022-07-11-search-sessions-status-refactor

unskip cache and abort related tests

01f4b5d

Dosant commented Jul 25, 2022

View reviewed changes

lukasolson approved these changes Jul 26, 2022

View reviewed changes

Dosant merged commit 810428c into elastic:search-sessions-stabilization-stage-1 Jul 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor search session status on-the-fly calculation and other fixes #136296

Refactor search session status on-the-fly calculation and other fixes #136296

Dosant commented Jul 13, 2022 •

edited

Loading

Dosant Jul 18, 2022

Dosant Jul 18, 2022 •

edited

Loading

Dosant Jul 18, 2022 •

edited

Loading

Dosant Jul 18, 2022 •

edited

Loading

elasticmachine commented Jul 18, 2022

Dosant Jul 19, 2022

Dosant commented Jul 25, 2022

Dosant Jul 25, 2022

Dosant commented Jul 25, 2022

Dosant commented Jul 25, 2022

kibana-ci commented Jul 25, 2022 •

edited

Loading

API count

ESLint disabled line counts

Total ESLint disabled count

lukasolson left a comment

lukasolson Jul 25, 2022

Dosant Jul 26, 2022

Refactor search session status on-the-fly calculation and other fixes #136296

Refactor search session status on-the-fly calculation and other fixes #136296

Conversation

Dosant commented Jul 13, 2022 • edited Loading

Summary

Dosant Jul 18, 2022

Choose a reason for hiding this comment

Dosant Jul 18, 2022 • edited Loading

Choose a reason for hiding this comment

Dosant Jul 18, 2022 • edited Loading

Choose a reason for hiding this comment

Dosant Jul 18, 2022 • edited Loading

Choose a reason for hiding this comment

elasticmachine commented Jul 18, 2022

Dosant Jul 19, 2022

Choose a reason for hiding this comment

Dosant commented Jul 25, 2022

Dosant Jul 25, 2022

Choose a reason for hiding this comment

Dosant commented Jul 25, 2022

Dosant commented Jul 25, 2022

kibana-ci commented Jul 25, 2022 • edited Loading

💔 Build Failed

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Canvas Sharable Runtime

Public APIs missing exports

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

lukasolson left a comment

Choose a reason for hiding this comment

lukasolson Jul 25, 2022

Choose a reason for hiding this comment

Dosant Jul 26, 2022

Choose a reason for hiding this comment

Dosant commented Jul 13, 2022 •

edited

Loading

Dosant Jul 18, 2022 •

edited

Loading

Dosant Jul 18, 2022 •

edited

Loading

Dosant Jul 18, 2022 •

edited

Loading

kibana-ci commented Jul 25, 2022 •

edited

Loading