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 new x-pack endpoints to track the progress of a search asynchronously #49931

Merged
merged 81 commits into from
Mar 10, 2020

Conversation

jimczi
Copy link
Contributor

@jimczi jimczi commented Dec 6, 2019

High level view

This change introduces a new API in x-pack basic that allows to track the progress of a search.
Users can submit an asynchronous search through a new endpoint called _async_search that
works exactly the same as the _search endpoint but instead of blocking and returning the final response when available, it returns a response after a provided wait_for_completion time.

# Submit an _async_search and waits up to 100ms for a final response
GET my_index_pattern*/_async_search?wait_for_completion=100ms
{
  "aggs": {
    "date_histogram": {
      "field": "@timestamp",
      "fixed_interval": "1h"
    }
  }
}

If after 100ms the final response is not available, a partial_response is included in the body:

{
  "id": "9N3J1m4BgyzUDzqgC15b",
  "version": 1,
  "is_running": true,
  "is_partial": true,
  "response": {
   "_shards": {
       "total": 100,
       "successful": 5,
       "failed": 0
    },
    "total_hits": {
      "value": 1653433,
      "relation": "eq"
    },
    "aggs": {
      ...
    }
  }
}

The partial response contains the total number of requested shards, the number of shards that successfully returned and the number of shards that failed.
It also contains the total hits as well as partial aggregations computed from the successful shards.
To continue to monitor the progress of the search users can call the get _async_search API like the following:

GET _async_search/9N3J1m4BgyzUDzqgC15b/?wait_for_completion=100ms

That returns a new response that can contain the same partial response than the previous call if the search didn't progress, in such case the returned version
should be the same. If new partial results are available, the version is incremented and the partial_response contains the updated progress.
Finally if the response is fully available while or after waiting for completion, the partial_response is replaced by a response section that contains the usual _search response:

{
  "id": "9N3J1m4BgyzUDzqgC15b",
  "version": 10,
  "is_running": false,
  "response": {
     "is_partial": false,
     ...
  }
}

Persistency

Asynchronous search are stored in a restricted index called .async-search if they survive (still running) after the initial submit. Each request has a keep alive that defaults to 5 days but this value can be changed/updated any time:

GET my_index_pattern*/_async_search?wait_for_completion=100ms&keep_alive=10d

The default can be changed when submitting the search, the example above raises the default value for the search to 10d.

GET _async_search/9N3J1m4BgyzUDzqgC15b/?wait_for_completion=100ms&keep_alive=10d

The time to live for a specific search can be extended when getting the progress/result. In the example above we extend the keep alive to 10 more days.
A background service that runs only on the node that holds the first primary shard of the async-search index is responsible for deleting the expired results. It runs every hour but the expiration is also checked by running queries (if they take longer than the keep_alive) and when getting a result.

Like a normal _search, if the http channel that is used to submit a request is closed before getting a response, the search is automatically cancelled. Note that this behavior is only for the submit API, subsequent GET requests will not cancel if they are closed.

Resiliency

Asynchronous search are not persistent, if the coordinator node crashes or is restarted during the search, the asynchronous search will stop. To know if the search is still running or not the response contains a field called is_running that indicates if the task is up or not. It is the responsibility of the user to resume an asynchronous search that didn't reach a final response by re-submitting the query. However final responses and failures are persisted in a system index that allows
to retrieve a response even if the task finishes.

DELETE _async_search/9N3J1m4BgyzUDzqgC15b

The response is also not stored if the initial submit action returns a final response. This allows to not add any overhead to queries that completes within the initial wait_for_completion.

Security

The .async-search index is a restricted index (should be migrated to a system index in +8.0) that is accessible only through the async search APIs. These APIs also ensure that only the user that submitted the initial query can retrieve or delete the running search. Note that admins/superusers would still be able to cancel the search task through the task manager like any other tasks.

Relates #49091

@jimczi jimczi added :Search/Search Search-related issues that do not fall into other categories v8.0.0 WIP labels Dec 6, 2019
@elasticmachine
Copy link
Collaborator

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

@jimczi jimczi added the >feature label Dec 6, 2019
@javanna javanna mentioned this pull request Dec 6, 2019
6 tasks
@jimczi
Copy link
Contributor Author

jimczi commented Dec 6, 2019

@elasticmachine run elasticsearch-ci/packaging-sample-matrix

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.

Woohoo! This looks great in general, some questions:

  • If I read correctly, it's up to the user to garbage collect responses manually. Should we do this automatically when a final response has been retrieved? We already have a wait_for_completion parameter that allows to reduce the number of roundtrips for fast requests, so it doesn't feel consistent to always require a new roundtrip to delete the response? I'm also a bit biased towards reducing the number of cases when responses need to be garbage-collected via ILM, as you could accumulate a large volume of responses in 5 days?
  • If we moved from time-based ids to true uuids - which can't be guessed, I wonder whether we'd still need to require that the user that views a response is the same as the user who submitted the request. I don't think it would be surprising to users that sharing the id of an async search has pretty much the same consequences as sharing the response of the search request?
  • Since response and partial_response should have mostly the same format, I wonder whether we should use the same response key combined with a partial flag?

@jimczi
Copy link
Contributor Author

jimczi commented Dec 10, 2019

If I read correctly, it's up to the user to garbage collect responses manually. Should we do this automatically when a final response has been retrieved? We already have a wait_for_completion parameter that allows to reduce the number of roundtrips for fast requests, so it doesn't feel consistent to always require a new roundtrip to delete the response? I'm also a bit biased towards reducing the number of cases when responses need to be garbage-collected via ILM, as you could accumulate a large volume of responses in 5 days?

I like the idea, getting the same final response twice is something that our regular caches should handle transparently so this would also emphasize the fact that this response are not meant to be used as an additional cache.

If we moved from time-based ids to true uuids - which can't be guessed, I wonder whether we'd still need to require that the user that views a response is the same as the user who submitted the request. I don't think it would be surprising to users that sharing the id of an async search has pretty much the same consequences as sharing the response of the search request?

+1 for true uuids, I agree with the response sharing analogy but since we want to delete final responses when they are reported back I think it would be nice to have this extra layer. It's not a lot of work and something that we already implement in scrolls.

Since response and partial_response should have mostly the same format, I wonder whether we should use the same response key combined with a partial flag?

Are you talking of the rest format or the internal response ? I think it's important to keep the distinction internally (for the hlrc) but I agree that the response could look like this:

{
  "id": "9N3J1m4BgyzUDzqgC15b",
  "version": 1,
  "is_running": true,
  "response": {
      "is_partial": true

Is it what you meant ?

@jpountz
Copy link
Contributor

jpountz commented Dec 10, 2019

Yes that's what I meant.

@giladgal
Copy link
Contributor

This all sounds great. Two suggestions to consider:

  1. If there is a burst of queries for which the result sets are not fully retrieved (e.g. because they are lengthy and the user checks them on a long interval). Can we crash the system because there are too many result sets? Should we start deleting based on the index size of the results index (not just on time or retrieval)?

  2. Would it make sense to have a limit on the time that the async query runs, e.g. have a default for the environment and allow users to specifically indicate that their query should be killed after a certain time interval (e.g. kill this query if it doesn't conclude after 24 hours). My concern is that if we don't have a limit, we can end up having zombie queries that queue on a resource starved system because there is no time-out to kill them. Such a mechanism would be an automated way for the admin to kill queries based on a very lengthy timeout.

@jimczi
Copy link
Contributor Author

jimczi commented Dec 12, 2019

I pushed another iteration that addresses @jpountz's comments.
We now use true random uuids for the async search index and the document is automatically deleted when the final response (completed or failed) is returned to the user (through submit or get).

We also discussed the best options to secure the system index with @elastic/es-security. The simplest way today would be to add the async search index in the RestrictedIndicesNames like we do for the .security index. Only the superuser would be able to access this index directly which is enough imo. The restricted indices don't support index pattern today but @albertzaharovits said that it would be trivial to add. The other option would be to use a single index and to replace the ilm garbage collection with a periodic delete by query. I am leaning towards the first option (adding support for index patterns in RestrictedIndicesNames) since it simpler and ensures that we re-create the index periodically but I am curious to hear what others think.
I'll now focus on securing the async-search APIs to ensure that only the user that submitted the query can retrieve or delete the running search. Note that admins/superusers would still be able to cancel the search task through the task manager like any other tasks.

@colings86
Copy link
Contributor

I wonder if we should add "pending_shards" to the partial_response section so it's explicit that we are expecting those shard to return?

Also I wonder if we should group the shard counts together into its own object?

@mark-vieira
Copy link
Contributor

FYI, you'll need to merge in the latest changes from master to fix these CI failure due to the new Java 13 build requirement.

Copy link
Member

@javanna javanna left a comment

Choose a reason for hiding this comment

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

I left a couple more small comments but LGTM. Can you also update the description and remove the mention of the 304 status code which I believe is outdated? Thanks for taking this to the finish line.

ActionRequestValidationException validationException = submit.validate();
if (validationException != null) {
throw validationException;
}
Copy link
Member

Choose a reason for hiding this comment

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

I still see it, did you mean to remove it?

request.setCcsMinimizeRoundtrips(false);
request.setPreFilterShardSize(1);
request.setBatchedReduceSize(5);
request.requestCache(true);
Copy link
Member

Choose a reason for hiding this comment

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

I am still missing where we reject ccs minimize roundtrips set to false


@Override
public Task createTask(long id, String type, String action, TaskId parentTaskId, Map<String, String> headers) {
return new CancellableTask(id, type, action, "", parentTaskId, headers) {
Copy link
Member

Choose a reason for hiding this comment

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

could you address this too?

"path":"/_async_search",
"methods":[
"GET",
"POST"
Copy link
Member

Choose a reason for hiding this comment

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

one more thing I had missed before, here we may want to remove GET? I tend to think that POST is the only method that suits an API that submits something. Was it here only for consistency with search?

},
"keep_alive": {
"type": "time",
"description": "Specify the time that the request should remain reachable in the cluster."
Copy link
Member

Choose a reason for hiding this comment

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

maybe rephrase to something like "Specify the time interval in which the results (partial or final) for this search will be available"

@jimczi jimczi merged commit 146b2a8 into elastic:master Mar 10, 2020
@jimczi jimczi deleted the async_search branch March 10, 2020 15:33
jimczi added a commit that referenced this pull request Mar 10, 2020
AsyncSearchActionTests#testCleanupOnFailure fails sporadically in CI but not locally.
This commit switches the tests into a SuiteScopeTestCase that creates internal states
once on static members in order to make the tests more reproducible.

Relates #49931
jimczi added a commit to jimczi/elasticsearch that referenced this pull request Mar 12, 2020
Deleting an async search id can throw a ResourceNotFoundException even if the query was successfully cancelled.
We delete the stored response automatically if the query is cancelled so that creates a race with the delete
action that also ensures that the task is removed. This change ensures that we ignore missing async search ids
in the async search index if they were successfuly cancelled.

Relates elastic#53360
Relates elastic#49931
jimczi added a commit that referenced this pull request Mar 12, 2020
Deleting an async search id can throw a ResourceNotFoundException even if the query was successfully cancelled.
We delete the stored response automatically if the query is cancelled so that creates a race with the delete action that also ensures that the task is removed. This change ensures that we ignore missing async search ids in the async search index if they were successfully cancelled.

Relates #53360
Relates #49931
jimczi added a commit that referenced this pull request Mar 16, 2020
…usly (#49931) (#53591)

This change introduces a new API in x-pack basic that allows to track the progress of a search.
Users can submit an asynchronous search through a new endpoint called `_async_search` that
works exactly the same as the `_search` endpoint but instead of blocking and returning the final response when available, it returns a response after a provided `wait_for_completion` time.

````
GET my_index_pattern*/_async_search?wait_for_completion=100ms
{
  "aggs": {
    "date_histogram": {
      "field": "@timestamp",
      "fixed_interval": "1h"
    }
  }
}
````

If after 100ms the final response is not available, a `partial_response` is included in the body:

````
{
  "id": "9N3J1m4BgyzUDzqgC15b",
  "version": 1,
  "is_running": true,
  "is_partial": true,
  "response": {
   "_shards": {
       "total": 100,
       "successful": 5,
       "failed": 0
    },
    "total_hits": {
      "value": 1653433,
      "relation": "eq"
    },
    "aggs": {
      ...
    }
  }
}
````

The partial response contains the total number of requested shards, the number of shards that successfully returned and the number of shards that failed.
It also contains the total hits as well as partial aggregations computed from the successful shards.
To continue to monitor the progress of the search users can call the get `_async_search` API like the following:

````
GET _async_search/9N3J1m4BgyzUDzqgC15b/?wait_for_completion=100ms
````

That returns a new response that can contain the same partial response than the previous call if the search didn't progress, in such case the returned `version`
should be the same. If new partial results are available, the version is incremented and the `partial_response` contains the updated progress.
Finally if the response is fully available while or after waiting for completion, the `partial_response` is replaced by a `response` section that contains the usual _search response:

````
{
  "id": "9N3J1m4BgyzUDzqgC15b",
  "version": 10,
  "is_running": false,
  "response": {
     "is_partial": false,
     ...
  }
}
````

Asynchronous search are stored in a restricted index called `.async-search` if they survive (still running) after the initial submit. Each request has a keep alive that defaults to 5 days but this value can be changed/updated any time:
`````
GET my_index_pattern*/_async_search?wait_for_completion=100ms&keep_alive=10d
`````
The default can be changed when submitting the search, the example above raises the default value for the search to `10d`.
`````
GET _async_search/9N3J1m4BgyzUDzqgC15b/?wait_for_completion=100ms&keep_alive=10d
`````
The time to live for a specific search can be extended when getting the progress/result. In the example above we extend the keep alive to 10 more days.
A background service that runs only on the node that holds the first primary shard of the `async-search` index is responsible for deleting the expired results. It runs every hour but the expiration is also checked by running queries (if they take longer than the keep_alive) and when getting a result.

Like a normal `_search`, if the http channel that is used to submit a request is closed before getting a response, the search is automatically cancelled. Note that this behavior is only for the submit API, subsequent GET requests will not cancel if they are closed.

Asynchronous search are not persistent, if the coordinator node crashes or is restarted during the search, the asynchronous search will stop. To know if the search is still running or not the response contains a field called `is_running` that indicates if the task is up or not. It is the responsibility of the user to resume an asynchronous search that didn't reach a final response by re-submitting the query. However final responses and failures are persisted in a system index that allows
to retrieve a response even if the task finishes.

````
DELETE _async_search/9N3J1m4BgyzUDzqgC15b
````

The response is also not stored if the initial submit action returns a final response. This allows to not add any overhead to queries that completes within the initial `wait_for_completion`.

The `.async-search` index is a restricted index (should be migrated to a system index in +8.0) that is accessible only through the async search APIs. These APIs also ensure that only the user that submitted the initial query can retrieve or delete the running search. Note that admins/superusers would still be able to cancel the search task through the task manager like any other tasks.

Relates #49091

Co-authored-by: Luca Cavanna <javanna@users.noreply.github.com>
@lizozom
Copy link

lizozom commented Apr 19, 2020

@jimczi

Could you please provide an example on how to update the keep alive time of a currently running async search?

I tried this without success.

Also, the final GET returns all of the data on a single response, without pagination. Is that the way this should always be?

@jimczi
Copy link
Contributor Author

jimczi commented Apr 19, 2020

Could you please provide an example on how to update the keep alive time of a currently running async search?

The response returns the initial keep alive instead of the updated one, I opened #55435 to fix the bug.

Also, the final GET returns all of the data on a single response, without pagination. Is that the way this should always be?

What do you mean by pagination ? It should return the same final response than a normal _search.

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

Successfully merging this pull request may close these issues.

None yet