kfaraz commented on code in PR #15673:
URL: https://github.com/apache/druid/pull/15673#discussion_r1449867952
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java:
##########
@@ -939,10 +939,19 @@ public Response killPendingSegments(
}
if (taskMaster.isLeader()) {
- final int numDeleted =
indexerMetadataStorageAdapter.deletePendingSegments(dataSource, deleteInterval);
- return Response.ok().entity(ImmutableMap.of("numDeleted",
numDeleted)).build();
+ try {
+ final int numDeleted =
indexerMetadataStorageAdapter.deletePendingSegments(dataSource, deleteInterval);
+ return Response.ok().entity(ImmutableMap.of("numDeleted",
numDeleted)).build();
+ }
+ catch (DruidException e) {
+ return Response.status(Response.Status.BAD_REQUEST)
Review Comment:
The code should be `400` (bad request) only if the
`IndexerMetadataStorageAdapter` threw an invalid input exception. There might
be other exceptions in which case we should not give a 400 response.
I don't recall exactly but I think there was a way to convert a
`DruidException` to a `Response` and it would automatically pick up the right
HTTP status code.
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/IndexerMetadataStorageAdapter.java:
##########
@@ -58,11 +59,15 @@ public int deletePendingSegments(String dataSource,
Interval deleteInterval)
DateTimes.MAX
);
- Preconditions.checkArgument(
- !deleteInterval.overlaps(activeTaskInterval),
- "Cannot delete pendingSegments because there is at least one active
task created at %s",
- activeTaskInterval.getStart()
- );
+ if (deleteInterval.overlaps(activeTaskInterval)) {
Review Comment:
Since we are touching this code anyway, I wonder if it would be helpful to
include the Task IDs too.
--
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]
