-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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] Split tasks #99967
Conversation
… session/cancellation
… session/cancellation
…\ expired async search
… sessions/split-tasks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me and didn't notice any regressions, tested with fake shart delay agg focusing on expiration. Tested multi spaces.
I'd give @lukasolson a chance to review.
Also:
- There are a couple more redundant
any
usages forfilter
I didn't point all of them in the previous review - This will need to be addressed (possibly OK after pr is merged): https://github.com/elastic/kibana/pull/99967/files#r648170897
- I this a bad merge? https://github.com/elastic/kibana/pull/99967/files#r648176354 pls review
x-pack/plugins/data_enhanced/server/search/session/get_search_session_page.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/data_enhanced/server/search/session/check_persisted_sessions.ts
Outdated
Show resolved
Hide resolved
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @lizozom |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, tested and things seem to be working properly.
@@ -5,33 +5,28 @@ | |||
* 2.0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Rename to check_non_persisted_sessions.ts
and check_non_persisted_sessions.test.ts
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
* cancel the previous session * split to 3 tasks * fixes * cancellation * updated tests * split out and improve jest tests * cleanup previous session properly * don't fail delete and cancel if item was already cleaned up * test * test * ignore resource_not_found_exception when deleting an already cleared \ expired async search * jest * update jest * api int * fix jest * testssss * Code review @Dosant * types * remove any * Fix merge * type * test * jest Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
* cancel the previous session * split to 3 tasks * fixes * cancellation * updated tests * split out and improve jest tests * cleanup previous session properly * don't fail delete and cancel if item was already cleaned up * test * test * ignore resource_not_found_exception when deleting an already cleared \ expired async search * jest * update jest * api int * fix jest * testssss * Code review @Dosant * types * remove any * Fix merge * type * test * jest Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Liza Katz <lizka.k@gmail.com> Co-authored-by: Liza K <liza.katz@elastic.co>
Summary
Part of #80406
Resolves #99967
Split out the search sesssion monitoring task into 3:
10s
. (configurable bytrackingInterval
)1m
. (configurable bycleanupInterval
)1h
(Not configurable (should it be?))Checklist
Delete any items that are not applicable to this PR.
For maintainers