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

[data.search.session] Change search session client to implement plugin #86815

Closed
wants to merge 2 commits into from

Conversation

lukasolson
Copy link
Member

@lukasolson lukasolson commented Dec 22, 2020

Summary

This PR changes the way that the data plugin search service implements the session service. Prior to this PR, the session service didn't implement the plugin lifecycle (in either the OSS or enhanced versions).

In doing so, it also adds the cancel logic to the session service, which in the enhanced version, will actually remove it from the search mapping.

This paves the way to add additional methods directly on the session service, like extend (see #87370).

/begin braindump

The OSS session service/x-pack background session service can’t follow the normal plugin lifecycle/types. Here’s why:

  1. Data plugin search service gets created, instantiates session service
  2. In search service setup, it calls setup of session service
  3. Data enhanced plugin gets created, instantiates background session service
  4. In enhanced search service setup it calls setup of background session service
  5. Enhanced data plugin calls enhance with new background session service

I don’t really like the pattern of “enhancing” using a whole service, since it makes handling the lifecycle really funky… You have to call setup in the enhanced plugin, but then do you call stop on OSS when enhancing? And do you call stop in OSS or enhanced for the enhanced plugin?

I guess all it really means is the enhanced session service can’t extend OSS session service.

/end braindump

Checklist

@lukasolson lukasolson added Feature:Search Querying infrastructure in Kibana WIP Work in progress v8.0.0 Team:AppServices release_note:skip Skip the PR/issue when compiling release notes Project:AsyncSearch Background search, partial results, async search services. v7.12.0 labels Dec 22, 2020
@lukasolson lukasolson self-assigned this Dec 22, 2020
@lukasolson
Copy link
Member Author

@lizozom @Dosant I'd like to hear your thoughts on this approach before cleaning up this PR (fixing tests, types, etc.) I know it seems a bit funky but I can't think of better alternatives at this point.

@kibanamachine
Copy link
Contributor

kibanamachine commented Jan 5, 2021

⏳ Build in-progress, with failures

Failed CI Steps

History

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

@lukasolson
Copy link
Member Author

Superseded by #87966.

@lukasolson lukasolson closed this Jan 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Search Querying infrastructure in Kibana Project:AsyncSearch Background search, partial results, async search services. release_note:skip Skip the PR/issue when compiling release notes v7.12.0 v8.0.0 WIP Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants