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

[DOCS] Remove beta label for tasks API docs #47378

Closed
wants to merge 1 commit into from
Closed

[DOCS] Remove beta label for tasks API docs #47378

wants to merge 1 commit into from

Conversation

jrodewig
Copy link
Contributor

@jrodewig jrodewig commented Oct 1, 2019

Removes the beta tag from the /_tasks and /_cat/_tasks task management APIs.

It looks like these tags have been in place since 2.3:
https://www.elastic.co/guide/en/elasticsearch/reference/2.3/tasks.html

Before merging, I want to confirm that these tags are no longer needed.

@jrodewig jrodewig added >docs General docs changes discuss :Data Management/CAT APIs Text APIs behind /_cat :Distributed Coordination/Task Management Issues for anything around the Tasks API - both persistent and node level. v7.5.0 v7.4.1 v7.3.3 labels Oct 1, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs

@henningandersen
Copy link
Contributor

henningandersen commented Oct 25, 2019

I have a few concerns on doing this:

The docs page describes that the status can change between versions. However, really many things can change here, even in minor versions. For instance the structure of requests could change:

I would think action names can change too (though there are likely other dependencies to those).

Several examples in the documentation concern reindex. For instance how to cancel a reindex task. However, with resilient reindex, reindex cancellation is going to change and these bits would need new examples. This argument is less clear, since we of course have to make breaking changes from time to time. However, the timing in relation to the reindex project seems to suggest to at least delay making this non-beta.

Cancel on its own is also concerning. AFAICS, it allows cancelling both parent tasks and sub tasks. While this can be great during troubleshooting, it seems like the wrong API to build into external programs. Examples that I think should normally not be done:

  • Cancel the fetch phase on a single shard.
  • Cancel a persistent allocated task (i.e., the ephemeral task doing the execution).
  • Cancel a single slice task of a reindex operation (subject to change with Reindex resiliency #42612)

Before removing the beta label, I think we have to call out that the tasks API is a low-level API useful mainly for troubleshooting and that its content (not the request/response URL/JSON structure, but the task structure and content) can change in minors (potentially even in micros). To me, it has similar status as the cluster state API, an invaluable tool during troubleshooting, but not with the stability guarantees given by us for the rest of the APIs.

Sorry for being late to comment on this.

@ywelsch
Copy link
Contributor

ywelsch commented Oct 25, 2019

Strong +1 to what Henning said.

The tasks API is exposing internals of the system (same as the cluster state API) that are typically used for debugging purposes (e.g. in the context of the support diagnostics tool), and should not be considered stable APIs as they expose way too much internal state that's subject to change. While the tasks API is currently the only way for finding and cancelling reindex jobs or searches, I consider that as an API design bug. This is also why we are currently designing a proper API for reindex to check (and cancel) currently running (and past) reindex jobs, same as we already have for any other functionality with long-running tasks (think e.g. snapshot restore, recovery, ML stuff, ...).

Taken from a discussion on another related issue:

Typically when a user wants to cancel a search query, the user don't want to terminate one node task but the entire set of node level tasks that the coordinator node generated + the parent task GET _tasks?group_by=parents

This is why we shouldn't be building this type of functionality on top of the tasks API. Users want to cancel a search, they do not care about parent or child tasks. #48422 is a prime example why we should build first-level APIs for various actions. The tasks API is too low-level and should not be used by users in their day-to-day tasks (pun intended).

@jrodewig
Copy link
Contributor Author

jrodewig commented Oct 25, 2019

Thanks @henningandersen and @ywelsch. I'm going to close this PR. I'll try to open a separate PR to better capture that the response is low-level and may change from version to version.

Until that's in place, it seems best to leave the beta tag here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Coordination/Task Management Issues for anything around the Tasks API - both persistent and node level. >docs General docs changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants