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 service] Add async search strategy #53057

Merged
merged 89 commits into from
Feb 25, 2020

Conversation

lukasolson
Copy link
Member

@lukasolson lukasolson commented Dec 14, 2019

Summary

Resolves #48811.

This PR adds a new abstract asynchronous search strategy (as opposed to the existing synchronous search strategy). It sends the first request as given (with all request parameters), and expects to receive an id in the response, which it then uses to poll the same backend endpoint until the response indicates that it is complete by sending a loaded property that is equal to the total property.

The location of this new search strategy is inside a new enhanced data plugin (data_enhanced) which is meant to contain the basic & commercial features related to the OSS data plugin (data).

At the moment, no existing search strategies are using this, but when the asynchronous search APIs land in Elasticsearch, we will override the existing ES_SEARCH_STRATEGY to use this async strategy to make the initial request and then poll.

This PR also removes the percentComplete optional property in IKibanaSearchResponse, as it can easily be derived from total / loaded in the response (if provided).

Remaining to do

  • Send cancellation request if the signal is aborted
  • Error out if it is aborted

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@lukasolson lukasolson added Feature:Search Querying infrastructure in Kibana release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. v8.0.0 Team:AppArch v7.6.0 labels Dec 14, 2019
@lukasolson lukasolson requested a review from a team as a code owner December 14, 2019 00:12
@lukasolson lukasolson self-assigned this Dec 14, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

Copy link
Contributor

@stacey-gammon stacey-gammon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@elasticmachine
Copy link
Contributor

💔 Build Failed

History

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

@LeeDr
Copy link
Contributor

LeeDr commented Jan 22, 2020

we're past 7.6.0 FF, can you please bump label to v7.7.0

@LeeDr LeeDr removed the v7.6.0 label Jan 23, 2020
@lizozom
Copy link
Contributor

lizozom commented Feb 25, 2020

@elasticmachine merge upstream

Copy link
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

One thing I would improve is to have the ID of the polled async request in the URL (rather than as a POST argument) to make it more easily debuggable.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@lukasolson lukasolson merged commit ef80ebf into elastic:master Feb 25, 2020
lukasolson added a commit to lukasolson/kibana that referenced this pull request Feb 25, 2020
* Add async search strategy

* Add async search

* Fix async strategy and add tests

* Move types to separate file

* Revert changes to demo search

* Update demo search strategy to use async

* Add cancellation to search strategies

* Add tests

* Simplify async search strategy

* Move loadingCount to search strategy

* Update abort controller library

* Bootstrap

* Abort when the request is aborted

* Add utility and update value suggestions route

* Fix bad merge conflict

* Update tests

* Move to data_enhanced plugin

* Remove bad merge

* Revert switching abort controller libraries

* Revert package.json in lib

* Move to previous abort controller

* Fix test to use fake timers to run debounced handlers

* Revert changes to example plugin

* Fix loading bar not going away when cancelling

* Call getSearchStrategy instead of passing  directly

* Add async demo search strategy

* Fix error with setting state

* Update how aborting works

* Fix type checks

* Add test for loading count

* Attempt to fix broken example test

* Revert changes to test

* Fix test

* Update name to camelCase

* Fix failing test

* Don't require data_enhanced in example plugin

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@lukasolson
Copy link
Member Author

7.x (7.7.0): b0336a7

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 26, 2020
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 53057 or prevent reminders by adding the backport:skip label.

1 similar comment
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 53057 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported Feature:Search Querying infrastructure in Kibana release_note:skip Skip the PR/issue when compiling release notes review v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Search service] Add async search strategy
7 participants