Merge pull request apache#2540 from pjain1/remove_kill

Remove extra parameter from deleteDataSourceSpecificInterval endpoint and correct exception message for invalid interval

docs/content/design/coordinator.md

@@ -266,7 +266,7 @@ Optional Header Parameters for auditing the config change can also be specified.
 
 Disables a datasource.
 
-* `/druid/coordinator/v1/datasources/{dataSourceName}/intervals/{interval}?kill=true`
+* `/druid/coordinator/v1/datasources/{dataSourceName}/intervals/{interval}`
 * `@Deprecated. /druid/coordinator/v1/datasources/{dataSourceName}?kill=true&interval={myISO8601Interval}`
 
 Runs a [Kill task](../ingestion/tasks.html) for a given interval and datasource.

integration-tests/src/main/java/io/druid/testing/clients/CoordinatorResourceTestClient.java

@@ -36,9 +36,8 @@
 import org.joda.time.Interval;
 
 import java.net.URL;
-import java.net.URLEncoder;
-import java.util.Map;
 import java.util.ArrayList;
+import java.util.Map;
 
 public class CoordinatorResourceTestClient
 {
@@ -143,7 +142,7 @@ public void deleteSegmentsDataSource(String dataSource, Interval interval)
       makeRequest(
           HttpMethod.DELETE,
           String.format(
-              "%sdatasources/%s/intervals/%s?kill=true",
+              "%sdatasources/%s/intervals/%s",
               getCoordinatorURL(),
               dataSource, interval.toString().replace("/", "_")
           )

server/src/main/java/io/druid/server/http/DatasourcesResource.java

@@ -49,7 +49,6 @@
 import javax.annotation.Nullable;
 import javax.ws.rs.Consumes;
 import javax.ws.rs.DELETE;
-import javax.ws.rs.DefaultValue;
 import javax.ws.rs.GET;
 import javax.ws.rs.POST;
 import javax.ws.rs.Path;
@@ -167,6 +166,12 @@ public Response enableDataSource(
     return Response.ok().build();
   }
 
+  /* When this method is removed, a new method needs to be introduced corresponding to
+    the end point "DELETE /druid/coordinator/v1/datasources/{dataSourceName}" (with no query parameters).
+    Ultimately we want to have no method with kill parameter -
+    DELETE `{dataSourceName}` will be used to disable datasource and
+    DELETE `{dataSourceName}/intervals/{interval}` will be used to nuke segments
+  */
   @DELETE
   @Deprecated
   @Path("/{dataSourceName}")
@@ -180,15 +185,30 @@ public Response deleteDataSource(
     if (indexingServiceClient == null) {
       return Response.ok(ImmutableMap.of("error", "no indexing service found")).build();
     }
+
     if (kill != null && Boolean.valueOf(kill)) {
       try {
         indexingServiceClient.killSegments(dataSourceName, new Interval(interval));
       }
+      catch (IllegalArgumentException e) {
+        return Response.status(Response.Status.BAD_REQUEST)
+                       .entity(
+                           ImmutableMap.of(
+                               "error",
+                               "Exception occurred. Probably the interval is invalid",
+                               "message",
+                               e.toString()
+                           )
+                       )
+                       .build();
+      }
       catch (Exception e) {
         return Response.serverError().entity(
             ImmutableMap.of(
                 "error",
-                "Exception occurred. Are you sure you have an indexing service?"
+                "Exception occurred. Are you sure you have an indexing service?",
+                "message",
+                e.toString()
             )
         )
                        .build();
@@ -207,28 +227,25 @@ public Response deleteDataSource(
   @Produces(MediaType.APPLICATION_JSON)
   public Response deleteDataSourceSpecificInterval(
       @PathParam("dataSourceName") final String dataSourceName,
-      @PathParam("interval") final String interval,
-      @QueryParam("kill") @DefaultValue("true") final String kill
+      @PathParam("interval") final String interval
   )
   {
     if (indexingServiceClient == null) {
       return Response.ok(ImmutableMap.of("error", "no indexing service found")).build();
     }
     final Interval theInterval = new Interval(interval.replace("_", "/"));
-    if (kill != null && Boolean.valueOf(kill)) {
-      try {
-        indexingServiceClient.killSegments(dataSourceName, new Interval(theInterval));
-      }
-      catch (Exception e) {
-        return Response.serverError()
-                       .entity(ImmutableMap.of(
-                           "error",
-                           "Exception occurred. Are you sure you have an indexing service?"
-                       ))
-                       .build();
-      }
-    } else {
-      return Response.ok(ImmutableMap.of("error", "kill is set to false")).build();
+    try {
+      indexingServiceClient.killSegments(dataSourceName, new Interval(theInterval));
+    }
+    catch (Exception e) {
+      return Response.serverError()
+                     .entity(ImmutableMap.of(
+                         "error",
+                         "Exception occurred. Are you sure you have an indexing service?",
+                         "message",
+                         e.toString()
+                     ))
+                     .build();
     }
     return Response.ok().build();
   }

server/src/test/java/io/druid/server/http/DatasourcesResourceTest.java

@@ -20,14 +20,11 @@
 package io.druid.server.http;
 
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import io.druid.client.CoordinatorServerView;
 import io.druid.client.DruidDataSource;
 import io.druid.client.DruidServer;
-import io.druid.client.InventoryView;
 import io.druid.client.indexing.IndexingServiceClient;
-import io.druid.metadata.MetadataSegmentManager;
 import io.druid.timeline.DataSegment;
 import org.easymock.EasyMock;
 import org.joda.time.Interval;
@@ -399,28 +396,23 @@ public void testDeleteDataSourceSpecificInterval() throws Exception
     EasyMock.replay(indexingServiceClient, server);
 
     DatasourcesResource datasourcesResource = new DatasourcesResource(inventoryView, null, indexingServiceClient);
-    Response response = datasourcesResource.deleteDataSourceSpecificInterval("datasource1", interval, "true");
+    Response response = datasourcesResource.deleteDataSourceSpecificInterval("datasource1", interval);
 
     Assert.assertEquals(200, response.getStatus());
     Assert.assertEquals(null, response.getEntity());
     EasyMock.verify(indexingServiceClient, server);
   }
 
   @Test
-  public void testDeleteDataSourceSpecificIntervalKillFalse() throws Exception
-  {
-    String interval = "2010-01-01_P1D";
-    Interval theInterval = new Interval(interval.replace("_", "/"));
-
+  public void testDeleteDataSource() {
     IndexingServiceClient indexingServiceClient = EasyMock.createStrictMock(IndexingServiceClient.class);
     EasyMock.replay(indexingServiceClient, server);
-
     DatasourcesResource datasourcesResource = new DatasourcesResource(inventoryView, null, indexingServiceClient);
-    Response response = datasourcesResource.deleteDataSourceSpecificInterval("datasource1", interval, "false");
+    Response response = datasourcesResource.deleteDataSource("datasource", "true", "???");
+    Assert.assertEquals(400, response.getStatus());
+    Assert.assertNotNull(response.getEntity());
+    Assert.assertTrue(response.getEntity().toString().contains("java.lang.IllegalArgumentException"));
 
-    Assert.assertEquals(200, response.getStatus());
-    ImmutableMap<String, String> results = (ImmutableMap<String, String>) response.getEntity();
-    Assert.assertEquals(results, ImmutableMap.<String, String>of("error", "kill is set to false"));
     EasyMock.verify(indexingServiceClient, server);
   }