Nagarjunanm opened a new pull request, #20824:
URL: https://github.com/apache/druid/pull/20824
PR title: fix: Restore the shutdownAllTasks Overlord API
---
Fixes #18419.
### Description
`POST /druid/indexer/v1/datasources/{datasource}/shutdownAllTasks` has
returned 404 since Druid 32.0.0, even when the datasource has running tasks.
#17545 added `OverlordDataSourcesResource` at
`/druid/indexer/v1/datasources`. Jersey sends a request to the root resource
with the most specific matching path and doesn't fall back to other classes, so
`shutdownAllTasks` requests started going to `OverlordDataSourcesResource`,
which has no such method. `OverlordResource.shutdownTasksForDataSource`, under
`/druid/indexer/v1`, could no longer be reached. The unit tests call that
method directly, which is why they kept passing.
The web console's e2e cleanup helper (`deleteDatasource` in
`web-console/e2e-tests/util/druid.ts`) also calls this API, but it treats a 404
as "no running tasks", so it has been quietly skipping task shutdown since
32.0.0 instead of failing. It doesn't need a change: with this fix, a 404 once
again means the datasource has no active tasks.
#### Moved the endpoint to `OverlordDataSourcesResource`
The endpoint now lives in `OverlordDataSourcesResource`, at the same URL.
Its behavior is unchanged: 503 when the Overlord isn't the leader, 404 when the
datasource has no active tasks, and otherwise it shuts down each active task
and returns the datasource name. Authorization is unchanged too, since
`DatasourceResourceFilter` reads the datasource from the path segment after
`datasources`.
The class Javadoc now notes that every Overlord API under
`/druid/indexer/v1/datasources` has to be defined in this class, to keep this
from happening again.
I also considered leaving the method in `OverlordResource` under a new path,
but that would break the documented URL.
#### Tests
- Moved the two unit tests from `OverlordResourceTest` to
`OverlordDataSourcesResourceTest`, and added one for the non-leader case.
- Added an embedded test to `OverlordClientTest` that sends a real HTTP
request, so the routing itself is covered. Without this fix it fails with a 404.
#### Release note
Fixed the `shutdownAllTasks` Overlord API (`POST
/druid/indexer/v1/datasources/{datasource}/shutdownAllTasks`), which returned a
404 error since Druid 32.0.0.
<hr>
##### Key changed/added classes in this PR
* `OverlordDataSourcesResource`
* `OverlordResource`
* `OverlordDataSourcesResourceTest`
* `OverlordResourceTest`
* `OverlordClientTest`
<hr>
This PR has:
- [x] been self-reviewed.
- [x] a release note entry in the PR description.
- [x] added comments explaining the "why" and the intent of the code
wherever would not be obvious for an unfamiliar reader.
- [x] added unit tests or modified existing tests to cover new code paths,
ensuring the threshold for [code
coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md)
is met.
- [x] added integration tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
