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

Add a listener to track the progress of a search request locally #49471

Merged
merged 16 commits into from
Nov 28, 2019

Conversation

jimczi
Copy link
Contributor

@jimczi jimczi commented Nov 21, 2019

This commit adds a function in NodeClient that allows to track the progress
of a search request locally. Progress is tracked through a SearchProgressListener
that exposes query and fetch responses as well as partial and final reduces.
This new method can be used by modules/plugins inside a node in order to track the
progress of a local search request.

Notes for reviewer:

  • The first commit is a revert of Split search in two when made against read-only and write indices #42510.
    We cannot track the progress efficiently if the request is splitted into read-only and write indices.
    This optimization is also not effective since it doesn't protect against the lost of a context if read-only indices are slow to answer, hence the revert.
  • The second commit is a split of the SearchTask to separate the main task from shard tasks (query, fetch, ...).
    This is needed to add the search progress listener in the main SearchTask.
  • The last commit adds the progress listener in the search action and exposes the new action listener in a method of the NodeClient.

Relates #49091

This commit adds a function in NodeClient that allows to track the progress
of a search request locally. Progress is tracked through a SearchProgressListener
that exposes query and fetch responses as well as partial and final reduces.
This new method can be used by modules/plugins inside a node in order to track the
progress of a local search request.

Relates elastic#49091
@jimczi jimczi added >feature :Search/Search Search-related issues that do not fall into other categories v8.0.0 v7.6.0 labels Nov 21, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/Search)

@javanna javanna mentioned this pull request Nov 21, 2019
6 tasks
Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

This makes sense to me.

@jimczi jimczi merged commit cab99c0 into elastic:master Nov 28, 2019
@jimczi jimczi deleted the search_progress_listener branch November 28, 2019 13:35
jimczi added a commit to jimczi/elasticsearch that referenced this pull request Nov 28, 2019
…stic#49471)

This commit adds a function in NodeClient that allows to track the progress
of a search request locally. Progress is tracked through a SearchProgressListener
that exposes query and fetch responses as well as partial and final reduces.
This new method can be used by modules/plugins inside a node in order to track the
progress of a local search request.

Relates elastic#49091
jimczi added a commit that referenced this pull request Nov 28, 2019
) (#49691)

This commit adds a function in NodeClient that allows to track the progress
of a search request locally. Progress is tracked through a SearchProgressListener
that exposes query and fetch responses as well as partial and final reduces.
This new method can be used by modules/plugins inside a node in order to track the
progress of a local search request.

Relates #49091
}
}

final List<SearchShard> searchShards(List<? extends SearchPhaseResult> results) {
Copy link
Member

Choose a reason for hiding this comment

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

I would make these methods static and rename them to buildSearchShards

Copy link
Member

Choose a reason for hiding this comment

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

I have opened #53373 to address these comments I left last week (besides the one on concurrency that I need to have a closer look at), plus a few other things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>feature :Search/Search Search-related issues that do not fall into other categories v7.6.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants