-
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] Client side search cache #92439
Conversation
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
…a into sessions/client-side-cache
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.
Recent changes LGTM.
I am still wondering about this thread (About tying this feature to sessions, which seem not related) and not sure if we should leave this as-is:
https://github.com/elastic/kibana/pull/92439/files?file-filters%5B%5D=.ts&file-filters%5B%5D=.tsx#r601345261
We probably should jump on a call and chat. I understand that we might want to merge as is to deliver this to Lens and follow-up if needed later.
Makes sense to me - happy to revisit this on the Lens side later one if we decide to change the way the cache is cleared. Right now it's necessary like this because otherwise the "Refresh" button won't work if there is no session id (it will try to send a new request, but it will just hit the cache) |
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.
Discussed tying caching to sessionId offline and agreed that we can revisit when there is a use-case of caching without sessions.
Also agreed to make an e2e caching test inside example plugin with custom search strategy
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 on chrome linux
* dev docs * sessions tutorial * title * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Code review * client cache * mock utils * improve code * Use cacheOnClient in Lens * mock * docs and types * unit tests! * Search response cache + tests * remove cacheOnClient evict cache on error * test ts * shouldCacheOnClient + improve tests * remove unused * clear subs * dont unsubscribe on setItem * caching mess * t * fix jest * add size to bfetch response @ppisljar use it to reduce the # of stringify in response cache * ts * ts * docs * simplify abort controller logic and extract it into a class * docs * delete unused tests * use addAbortSignal * code review * Use shareReplay, fix tests * code review * bfetch test * code review * Leave the bfetch changes out * docs + isRestore * make sure to clean up properly * Make sure that aborting in cache works correctly Clearer restructuring of code * fix test * import * code review round 1 * ts * Added functional test for search request caching * test * skip before codefreeze Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
* dev docs * sessions tutorial * title * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Update dev_docs/tutorials/data/search.mdx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> * Code review * client cache * mock utils * improve code * Use cacheOnClient in Lens * mock * docs and types * unit tests! * Search response cache + tests * remove cacheOnClient evict cache on error * test ts * shouldCacheOnClient + improve tests * remove unused * clear subs * dont unsubscribe on setItem * caching mess * t * fix jest * add size to bfetch response @ppisljar use it to reduce the # of stringify in response cache * ts * ts * docs * simplify abort controller logic and extract it into a class * docs * delete unused tests * use addAbortSignal * code review * Use shareReplay, fix tests * code review * bfetch test * code review * Leave the bfetch changes out * docs + isRestore * make sure to clean up properly * Make sure that aborting in cache works correctly Clearer restructuring of code * fix test * import * code review round 1 * ts * Added functional test for search request caching * test * skip before codefreeze Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Liza Katz <lizka.k@gmail.com> Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
💚 Build SucceededMetrics [docs]Module Count
Async chunks
Page load bundle
Unknown metric groupsAPI count
API count missing comments
History
To update your PR or re-run it, just comment with: cc @lizozom |
…te-legacy-es-client * 'master' of github.com:elastic/kibana: (102 commits) [Exploratory view] integerate page views to exploratory view (elastic#97258) Fix typo in license_api_guard README name and import http server mocks from public interface (elastic#97334) Avoid mutating KQL query when validating it (elastic#97081) Add description as title on tag badge (elastic#97109) Remove legacy ES client usages in `home` and `xpack_legacy` (elastic#97359) [Fleet] Finer-grained error information from install/upgrade API (elastic#95649) Rule registry bundle size (elastic#97251) [Partial Results] Move other bucket into Search Source (elastic#96384) [Dashboard] Makes lens default editor for creating new panels (elastic#96181) skip flaky suite (elastic#97387) [Asset Management] Agent picker follow up (elastic#97357) skip flaky suite (elastic#97382) [Security Solutions] Fixes flake with cypress tests (elastic#97329) skip flaky suite (elastic#97355) Skip test to try and stabilize master minimize number of so fild asserted in tests. it creates flakines when implementation details change (elastic#97374) [Search Sessions] Client side search cache (elastic#92439) [SavedObjects] Add aggregations support (elastic#96292) [Reporting] Remove legacy elasticsearch client usage from the reporting plugin (elastic#97184) [kbnClient] fix basePath handling and export reponse type (elastic#97277) ... # Conflicts: # x-pack/plugins/watcher/server/lib/license_pre_routing_factory/license_pre_routing_factory.ts # x-pack/plugins/watcher/server/plugin.ts # x-pack/plugins/watcher/server/routes/api/indices/register_get_route.ts # x-pack/plugins/watcher/server/routes/api/license/register_refresh_route.ts # x-pack/plugins/watcher/server/routes/api/register_list_fields_route.ts # x-pack/plugins/watcher/server/routes/api/register_load_history_route.ts # x-pack/plugins/watcher/server/routes/api/settings/register_load_route.ts # x-pack/plugins/watcher/server/routes/api/watch/action/register_acknowledge_route.ts # x-pack/plugins/watcher/server/routes/api/watch/register_activate_route.ts # x-pack/plugins/watcher/server/routes/api/watch/register_deactivate_route.ts # x-pack/plugins/watcher/server/routes/api/watch/register_delete_route.ts # x-pack/plugins/watcher/server/routes/api/watch/register_execute_route.ts # x-pack/plugins/watcher/server/routes/api/watch/register_history_route.ts # x-pack/plugins/watcher/server/routes/api/watch/register_load_route.ts # x-pack/plugins/watcher/server/routes/api/watch/register_save_route.ts # x-pack/plugins/watcher/server/routes/api/watch/register_visualize_route.ts # x-pack/plugins/watcher/server/routes/api/watches/register_delete_route.ts # x-pack/plugins/watcher/server/routes/api/watches/register_list_route.ts # x-pack/plugins/watcher/server/shared_imports.ts # x-pack/plugins/watcher/server/types.ts
Pinging @elastic/kibana-app-services (Team:AppServices) |
Summary
Depends on #95688 and #95774
This PR introduces client side caching for search requests, saving unnecessary server round trips and ES workload.
This type of caching is
sessionId
based, so that identical searches will be executed at most once within a single search session.Here are some examples when this may comes in handy:
Cache management and eviction
The new search response cache uses a hash of the query as a key. For each key, it stores a search observable (which are now concluded with a
shareReplay
operation to avoid repeated execution) and a search abort controller (which is used to coordinate the cancellation of a search, when all consumers abort it).The cache size is limited by the amount and by the total response size of cached items. Once one of those are hit, the cache is shrinked based on LRU. Individual items that are bigger than the maximum response are not cached from the get go.
If a search observable resolves with an error, the error is passed on to any existing subscribers, and then its evicted from the cache.
Testing
x-pack/plugins/data_enhanced/public/search/search_abort_controller.test.ts
SearchAbortController
constructorx-pack/plugins/data_enhanced/public/search/search_response_cache.test.ts
x-pack/plugins/data_enhanced/public/search/search_interceptor.test.ts
Release Note
Introduces client side, search session based, caching.
Checklist
Delete any items that are not applicable to this PR.
For maintainers