You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/04/08 02:39:21 UTC

[GitHub] [druid] maytasm opened a new pull request #9644: [WIP] Add API to trigger a compaction by the coordinator for integration tests

maytasm opened a new pull request #9644: [WIP] Add API to trigger a compaction by the coordinator for integration tests
URL: https://github.com/apache/druid/pull/9644
 
 
   Add API to trigger a compaction by the coordinator for integration tests
   
   ### Description
   
   Implement a new API to manually trigger the compaction by the coordinator.
   
   This PR has:
   - [ ] been self-reviewed.
      - [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
   - [ ] added documentation for new or modified features or behaviors.
   - [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
   - [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/licenses.yaml)
   - [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
   - [ ] added unit tests or modified existing tests to cover new code paths.
   - [ ] added integration tests.
   - [ ] been tested in a test Druid cluster.
   
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] ccaominh commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests

Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests
URL: https://github.com/apache/druid/pull/9644#discussion_r406368963
 
 

 ##########
 File path: server/src/main/java/org/apache/druid/server/http/CompactionResource.java
 ##########
 @@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.server.http;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.inject.Inject;
+import com.sun.jersey.spi.container.ResourceFilters;
+import org.apache.druid.server.coordinator.DruidCoordinator;
+import org.apache.druid.server.http.security.ConfigResourceFilter;
+import org.apache.druid.server.http.security.StateResourceFilter;
+
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+
+@Path("/druid/coordinator/v1/compaction")
+public class CompactionResource
+{
+  private final DruidCoordinator coordinator;
+
+  @Inject
+  public CompactionResource(
+      DruidCoordinator coordinator
+  )
+  {
+    this.coordinator = coordinator;
+  }
+
+  @POST
 
 Review comment:
   Can we add tests for these?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] ccaominh commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests

Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests
URL: https://github.com/apache/druid/pull/9644#discussion_r406359497
 
 

 ##########
 File path: server/src/main/java/org/apache/druid/server/http/CompactionResource.java
 ##########
 @@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.server.http;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.inject.Inject;
+import com.sun.jersey.spi.container.ResourceFilters;
+import org.apache.druid.server.coordinator.DruidCoordinator;
+import org.apache.druid.server.http.security.ConfigResourceFilter;
+import org.apache.druid.server.http.security.StateResourceFilter;
+
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+
+@Path("/druid/coordinator/v1/compaction")
+public class CompactionResource
+{
+  private final DruidCoordinator coordinator;
+
+  @Inject
+  public CompactionResource(
+      DruidCoordinator coordinator
+  )
+  {
+    this.coordinator = coordinator;
+  }
+
+  @POST
+  @Path("/compact")
+  @ResourceFilters(ConfigResourceFilter.class)
+  public Response forceTriggerCompaction()
+  {
+    coordinator.runCompactSegmentsDuty();
+    return Response.ok().build();
+  }
+
+  @GET
+  @Path("/progress")
 
 Review comment:
   This endpoint has been renamed. What happens to old clients that are calling the removed endpoint?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] maytasm commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests

Posted by GitBox <gi...@apache.org>.
maytasm commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests
URL: https://github.com/apache/druid/pull/9644#discussion_r406607720
 
 

 ##########
 File path: server/src/main/java/org/apache/druid/server/http/CompactionResource.java
 ##########
 @@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.server.http;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.inject.Inject;
+import com.sun.jersey.spi.container.ResourceFilters;
+import org.apache.druid.server.coordinator.DruidCoordinator;
+import org.apache.druid.server.http.security.ConfigResourceFilter;
+import org.apache.druid.server.http.security.StateResourceFilter;
+
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+
+@Path("/druid/coordinator/v1/compaction")
+public class CompactionResource
+{
+  private final DruidCoordinator coordinator;
+
+  @Inject
+  public CompactionResource(
+      DruidCoordinator coordinator
+  )
+  {
+    this.coordinator = coordinator;
+  }
+
+  @POST
 
 Review comment:
   - /progress is an old API (I simply renamed it), hence didn't add any tests for it. 
   - Added integration tests for /compact

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] ccaominh commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests

Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests
URL: https://github.com/apache/druid/pull/9644#discussion_r406359966
 
 

 ##########
 File path: server/src/test/java/org/apache/druid/server/http/security/SecurityResourceFilterTest.java
 ##########
 @@ -72,8 +73,9 @@
             getRequestPathsWithAuthorizer(StatusResource.class),
             getRequestPathsWithAuthorizer(SelfDiscoveryResource.class),
             getRequestPathsWithAuthorizer(BrokerQueryResource.class),
-            getRequestPathsWithAuthorizer(RouterResource.class)
-        )
+            getRequestPathsWithAuthorizer(RouterResource.class),
+            getRequestPathsWithAuthorizer(CompactionResource.class)
+            )
 
 Review comment:
   Indentation for `)` seems off

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] maytasm commented on a change in pull request #9644: Add missing integration tests for the compaction by the coordinator

Posted by GitBox <gi...@apache.org>.
maytasm commented on a change in pull request #9644: Add missing integration tests for the compaction by the coordinator
URL: https://github.com/apache/druid/pull/9644#discussion_r407012932
 
 

 ##########
 File path: docs/operations/api-reference.md
 ##########
 @@ -362,6 +362,15 @@ Returns total size and count for each interval within given isointerval.
 
 Returns total size and count for each datasource for each interval within given isointerval.
 
+#### Compaction Duty
 
 Review comment:
   Done

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] maytasm commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests

Posted by GitBox <gi...@apache.org>.
maytasm commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests
URL: https://github.com/apache/druid/pull/9644#discussion_r406606413
 
 

 ##########
 File path: server/src/main/java/org/apache/druid/server/http/CompactionResource.java
 ##########
 @@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.server.http;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.inject.Inject;
+import com.sun.jersey.spi.container.ResourceFilters;
+import org.apache.druid.server.coordinator.DruidCoordinator;
+import org.apache.druid.server.http.security.ConfigResourceFilter;
+import org.apache.druid.server.http.security.StateResourceFilter;
+
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+
+@Path("/druid/coordinator/v1/compaction")
+public class CompactionResource
+{
+  private final DruidCoordinator coordinator;
+
+  @Inject
+  public CompactionResource(
+      DruidCoordinator coordinator
+  )
+  {
+    this.coordinator = coordinator;
+  }
+
+  @POST
+  @Path("/compact")
+  @ResourceFilters(ConfigResourceFilter.class)
+  public Response forceTriggerCompaction()
+  {
+    coordinator.runCompactSegmentsDuty();
+    return Response.ok().build();
+  }
+
+  @GET
+  @Path("/progress")
 
 Review comment:
   I talked to @jihoonson (who added this API). He mentioned that this API is to be used by the web console but web console is not using it yet. 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jihoonson commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests

Posted by GitBox <gi...@apache.org>.
jihoonson commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests
URL: https://github.com/apache/druid/pull/9644#discussion_r406859778
 
 

 ##########
 File path: server/src/main/java/org/apache/druid/server/http/CompactionResource.java
 ##########
 @@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.server.http;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.inject.Inject;
+import com.sun.jersey.spi.container.ResourceFilters;
+import org.apache.druid.server.coordinator.DruidCoordinator;
+import org.apache.druid.server.http.security.ConfigResourceFilter;
+import org.apache.druid.server.http.security.StateResourceFilter;
+
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+
+@Path("/druid/coordinator/v1/compaction")
+public class CompactionResource
+{
+  private final DruidCoordinator coordinator;
+
+  @Inject
+  public CompactionResource(
+      DruidCoordinator coordinator
+  )
+  {
+    this.coordinator = coordinator;
+  }
+
+  @POST
+  @Path("/compact")
+  @ResourceFilters(ConfigResourceFilter.class)
+  public Response forceTriggerCompaction()
+  {
+    coordinator.runCompactSegmentsDuty();
+    return Response.ok().build();
+  }
+
+  @GET
+  @Path("/progress")
 
 Review comment:
   Oh the old API is not documented. I think it's safe to assume that no one is using it.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] ccaominh commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests

Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests
URL: https://github.com/apache/druid/pull/9644#discussion_r406365832
 
 

 ##########
 File path: server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java
 ##########
 @@ -621,13 +628,20 @@ private void stopBeingLeader()
     return ImmutableList.copyOf(duties);
   }
 
-  public class DutiesRunnable implements Runnable
+  private List<CoordinatorDuty> makeCompactSegmentsDuty()
+  {
+    List<CoordinatorDuty> duties = new ArrayList<>();
+    duties.add(compactSegments);
+    return ImmutableList.copyOf(duties);
 
 Review comment:
   Why not `return ImmutableList.of(compactSegments);` instead?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] ccaominh commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests

Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests
URL: https://github.com/apache/druid/pull/9644#discussion_r406885397
 
 

 ##########
 File path: docs/operations/api-reference.md
 ##########
 @@ -362,6 +362,15 @@ Returns total size and count for each interval within given isointerval.
 
 Returns total size and count for each datasource for each interval within given isointerval.
 
+#### Compaction Duty
 
 Review comment:
   Maybe "Compaction Status" is a better description

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] ccaominh merged pull request #9644: Add missing integration tests for the compaction by the coordinator

Posted by GitBox <gi...@apache.org>.
ccaominh merged pull request #9644: Add missing integration tests for the compaction by the coordinator
URL: https://github.com/apache/druid/pull/9644
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] maytasm commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests

Posted by GitBox <gi...@apache.org>.
maytasm commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests
URL: https://github.com/apache/druid/pull/9644#discussion_r406606506
 
 

 ##########
 File path: server/src/test/java/org/apache/druid/server/http/security/SecurityResourceFilterTest.java
 ##########
 @@ -72,8 +73,9 @@
             getRequestPathsWithAuthorizer(StatusResource.class),
             getRequestPathsWithAuthorizer(SelfDiscoveryResource.class),
             getRequestPathsWithAuthorizer(BrokerQueryResource.class),
-            getRequestPathsWithAuthorizer(RouterResource.class)
-        )
+            getRequestPathsWithAuthorizer(RouterResource.class),
+            getRequestPathsWithAuthorizer(CompactionResource.class)
+            )
 
 Review comment:
   Fixed

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] maytasm commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests

Posted by GitBox <gi...@apache.org>.
maytasm commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests
URL: https://github.com/apache/druid/pull/9644#discussion_r406606594
 
 

 ##########
 File path: server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java
 ##########
 @@ -621,13 +628,20 @@ private void stopBeingLeader()
     return ImmutableList.copyOf(duties);
   }
 
-  public class DutiesRunnable implements Runnable
+  private List<CoordinatorDuty> makeCompactSegmentsDuty()
+  {
+    List<CoordinatorDuty> duties = new ArrayList<>();
+    duties.add(compactSegments);
+    return ImmutableList.copyOf(duties);
 
 Review comment:
   Done

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] maytasm commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests

Posted by GitBox <gi...@apache.org>.
maytasm commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests
URL: https://github.com/apache/druid/pull/9644#discussion_r406603262
 
 

 ##########
 File path: server/src/main/java/org/apache/druid/server/http/CompactionResource.java
 ##########
 @@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.server.http;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.inject.Inject;
+import com.sun.jersey.spi.container.ResourceFilters;
+import org.apache.druid.server.coordinator.DruidCoordinator;
+import org.apache.druid.server.http.security.ConfigResourceFilter;
+import org.apache.druid.server.http.security.StateResourceFilter;
+
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+
+@Path("/druid/coordinator/v1/compaction")
 
 Review comment:
   The new /druid/coordinator/v1/compaction/compact will not be a public API and will not be documented. It is exclusively only use for integration test. The /druid/coordinator/v1/compaction/progress is an old endpoint. Seems like it does not originally have any documentation. I can add it.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] ccaominh commented on a change in pull request #9644: Add missing integration tests for the compaction by the coordinator

Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9644: Add missing integration tests for the compaction by the coordinator
URL: https://github.com/apache/druid/pull/9644#discussion_r407792121
 
 

 ##########
 File path: integration-tests/src/main/java/org/apache/druid/testing/clients/CompactionResourceTestClient.java
 ##########
 @@ -0,0 +1,179 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.testing.clients;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.inject.Inject;
+import org.apache.druid.java.util.common.ISE;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.http.client.HttpClient;
+import org.apache.druid.java.util.http.client.Request;
+import org.apache.druid.java.util.http.client.response.StatusResponseHandler;
+import org.apache.druid.java.util.http.client.response.StatusResponseHolder;
+import org.apache.druid.server.coordinator.CoordinatorCompactionConfig;
+import org.apache.druid.server.coordinator.DataSourceCompactionConfig;
+import org.apache.druid.testing.IntegrationTestingConfig;
+import org.apache.druid.testing.guice.TestClient;
+import org.jboss.netty.handler.codec.http.HttpMethod;
+import org.jboss.netty.handler.codec.http.HttpResponseStatus;
+
+import javax.ws.rs.QueryParam;
+import java.net.URL;
+import java.util.Map;
+
+public class CompactionResourceTestClient
+{
+  private final ObjectMapper jsonMapper;
+  private final HttpClient httpClient;
+  private final String coordinator;
+  private final StatusResponseHandler responseHandler;
+
+  @Inject
+  CompactionResourceTestClient(
+      ObjectMapper jsonMapper,
+      @TestClient HttpClient httpClient,
+      IntegrationTestingConfig config
+  )
+  {
+    this.jsonMapper = jsonMapper;
+    this.httpClient = httpClient;
+    this.coordinator = config.getCoordinatorUrl();
+    this.responseHandler = StatusResponseHandler.getInstance();
+  }
+
+  private String getCoordinatorURL()
+  {
+    return StringUtils.format(
+        "%s/druid/coordinator/v1/",
+        coordinator
+    );
+  }
+
+  public void submitCompactionConfig(final DataSourceCompactionConfig dataSourceCompactionConfig) throws Exception
+  {
+    String url = StringUtils.format("%sconfig/compaction", getCoordinatorURL());
+    StatusResponseHolder response = httpClient.go(
+        new Request(HttpMethod.POST, new URL(url)).setContent(
+            "application/json",
+            jsonMapper.writeValueAsBytes(dataSourceCompactionConfig)
+        ), responseHandler
+    ).get();
+
+    if (!response.getStatus().equals(HttpResponseStatus.OK)) {
+      throw new ISE(
+          "Error while submiting compaction config status[%s] content[%s]",
+          response.getStatus(),
+          response.getContent()
+      );
+    }
+  }
+
+  public void deleteCompactionConfig(final String dataSource) throws Exception
+  {
+    String url = StringUtils.format("%sconfig/compaction/%s", getCoordinatorURL(), StringUtils.urlEncode(dataSource));
+    StatusResponseHolder response = httpClient.go(new Request(HttpMethod.DELETE, new URL(url)), responseHandler).get();
+
+    if (!response.getStatus().equals(HttpResponseStatus.OK)) {
+      throw new ISE(
+          "Error while deleting compaction config status[%s] content[%s]",
+          response.getStatus(),
+          response.getContent()
+      );
+    }
+  }
+
+  public CoordinatorCompactionConfig getCoordinatorCompactionConfigs() throws Exception
+  {
+    String url = StringUtils.format("%sconfig/compaction", getCoordinatorURL());
+    StatusResponseHolder response = httpClient.go(
+        new Request(HttpMethod.GET, new URL(url)), responseHandler
+    ).get();
+    if (!response.getStatus().equals(HttpResponseStatus.OK)) {
+      throw new ISE(
+          "Error while getting compaction config status[%s] content[%s]",
+          response.getStatus(),
+          response.getContent()
+      );
+    }
+    return jsonMapper.readValue(response.getContent(), new TypeReference<CoordinatorCompactionConfig>() {});
+  }
+
+  public DataSourceCompactionConfig getDataSourceCompactionConfig(String dataSource) throws Exception
+  {
+    String url = StringUtils.format("%sconfig/compaction/%s", getCoordinatorURL(), StringUtils.urlEncode(dataSource));
+    StatusResponseHolder response = httpClient.go(
+        new Request(HttpMethod.GET, new URL(url)), responseHandler
+    ).get();
+    if (!response.getStatus().equals(HttpResponseStatus.OK)) {
+      throw new ISE(
+          "Error while getting compaction config status[%s] content[%s]",
+          response.getStatus(),
+          response.getContent()
+      );
+    }
+    return jsonMapper.readValue(response.getContent(), new TypeReference<DataSourceCompactionConfig>() {});
+  }
+
+  public void forceTriggerAutoCompaction() throws Exception
+  {
+    String url = StringUtils.format("%scompaction/compact", getCoordinatorURL());
+    StatusResponseHolder response = httpClient.go(new Request(HttpMethod.POST, new URL(url)), responseHandler).get();
+    if (!response.getStatus().equals(HttpResponseStatus.OK)) {
+      throw new ISE(
+          "Error while force trigger auto compaction status[%s] content[%s]",
+          response.getStatus(),
+          response.getContent()
+      );
+    }
+  }
+
+  public void updateCompactionTaskSlot(Double compactionTaskSlotRatio, Integer maxCompactionTaskSlots) throws Exception
+  {
+    String url = StringUtils.format("%sconfig/compaction/taskslots?ratio=%s&max=%s",
+                                    getCoordinatorURL(),
+                                    StringUtils.urlEncode(compactionTaskSlotRatio.toString()),
+                                    StringUtils.urlEncode(maxCompactionTaskSlots.toString()));
+    StatusResponseHolder response = httpClient.go(new Request(HttpMethod.POST, new URL(url)), responseHandler).get();
+    if (!response.getStatus().equals(HttpResponseStatus.OK)) {
+      throw new ISE(
+          "Error while force trigger auto compaction status[%s] content[%s]",
 
 Review comment:
   Should the message say something about updating compaction task slots instead?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] maytasm commented on a change in pull request #9644: Add missing integration tests for the compaction by the coordinator

Posted by GitBox <gi...@apache.org>.
maytasm commented on a change in pull request #9644: Add missing integration tests for the compaction by the coordinator
URL: https://github.com/apache/druid/pull/9644#discussion_r408506563
 
 

 ##########
 File path: integration-tests/src/main/java/org/apache/druid/testing/clients/CompactionResourceTestClient.java
 ##########
 @@ -0,0 +1,179 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.testing.clients;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.inject.Inject;
+import org.apache.druid.java.util.common.ISE;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.http.client.HttpClient;
+import org.apache.druid.java.util.http.client.Request;
+import org.apache.druid.java.util.http.client.response.StatusResponseHandler;
+import org.apache.druid.java.util.http.client.response.StatusResponseHolder;
+import org.apache.druid.server.coordinator.CoordinatorCompactionConfig;
+import org.apache.druid.server.coordinator.DataSourceCompactionConfig;
+import org.apache.druid.testing.IntegrationTestingConfig;
+import org.apache.druid.testing.guice.TestClient;
+import org.jboss.netty.handler.codec.http.HttpMethod;
+import org.jboss.netty.handler.codec.http.HttpResponseStatus;
+
+import javax.ws.rs.QueryParam;
+import java.net.URL;
+import java.util.Map;
+
+public class CompactionResourceTestClient
+{
+  private final ObjectMapper jsonMapper;
+  private final HttpClient httpClient;
+  private final String coordinator;
+  private final StatusResponseHandler responseHandler;
+
+  @Inject
+  CompactionResourceTestClient(
+      ObjectMapper jsonMapper,
+      @TestClient HttpClient httpClient,
+      IntegrationTestingConfig config
+  )
+  {
+    this.jsonMapper = jsonMapper;
+    this.httpClient = httpClient;
+    this.coordinator = config.getCoordinatorUrl();
+    this.responseHandler = StatusResponseHandler.getInstance();
+  }
+
+  private String getCoordinatorURL()
+  {
+    return StringUtils.format(
+        "%s/druid/coordinator/v1/",
+        coordinator
+    );
+  }
+
+  public void submitCompactionConfig(final DataSourceCompactionConfig dataSourceCompactionConfig) throws Exception
+  {
+    String url = StringUtils.format("%sconfig/compaction", getCoordinatorURL());
+    StatusResponseHolder response = httpClient.go(
+        new Request(HttpMethod.POST, new URL(url)).setContent(
+            "application/json",
+            jsonMapper.writeValueAsBytes(dataSourceCompactionConfig)
+        ), responseHandler
+    ).get();
+
+    if (!response.getStatus().equals(HttpResponseStatus.OK)) {
+      throw new ISE(
+          "Error while submiting compaction config status[%s] content[%s]",
+          response.getStatus(),
+          response.getContent()
+      );
+    }
+  }
+
+  public void deleteCompactionConfig(final String dataSource) throws Exception
+  {
+    String url = StringUtils.format("%sconfig/compaction/%s", getCoordinatorURL(), StringUtils.urlEncode(dataSource));
+    StatusResponseHolder response = httpClient.go(new Request(HttpMethod.DELETE, new URL(url)), responseHandler).get();
+
+    if (!response.getStatus().equals(HttpResponseStatus.OK)) {
+      throw new ISE(
+          "Error while deleting compaction config status[%s] content[%s]",
+          response.getStatus(),
+          response.getContent()
+      );
+    }
+  }
+
+  public CoordinatorCompactionConfig getCoordinatorCompactionConfigs() throws Exception
+  {
+    String url = StringUtils.format("%sconfig/compaction", getCoordinatorURL());
+    StatusResponseHolder response = httpClient.go(
+        new Request(HttpMethod.GET, new URL(url)), responseHandler
+    ).get();
+    if (!response.getStatus().equals(HttpResponseStatus.OK)) {
+      throw new ISE(
+          "Error while getting compaction config status[%s] content[%s]",
+          response.getStatus(),
+          response.getContent()
+      );
+    }
+    return jsonMapper.readValue(response.getContent(), new TypeReference<CoordinatorCompactionConfig>() {});
+  }
+
+  public DataSourceCompactionConfig getDataSourceCompactionConfig(String dataSource) throws Exception
+  {
+    String url = StringUtils.format("%sconfig/compaction/%s", getCoordinatorURL(), StringUtils.urlEncode(dataSource));
+    StatusResponseHolder response = httpClient.go(
+        new Request(HttpMethod.GET, new URL(url)), responseHandler
+    ).get();
+    if (!response.getStatus().equals(HttpResponseStatus.OK)) {
+      throw new ISE(
+          "Error while getting compaction config status[%s] content[%s]",
+          response.getStatus(),
+          response.getContent()
+      );
+    }
+    return jsonMapper.readValue(response.getContent(), new TypeReference<DataSourceCompactionConfig>() {});
+  }
+
+  public void forceTriggerAutoCompaction() throws Exception
+  {
+    String url = StringUtils.format("%scompaction/compact", getCoordinatorURL());
+    StatusResponseHolder response = httpClient.go(new Request(HttpMethod.POST, new URL(url)), responseHandler).get();
+    if (!response.getStatus().equals(HttpResponseStatus.OK)) {
+      throw new ISE(
+          "Error while force trigger auto compaction status[%s] content[%s]",
+          response.getStatus(),
+          response.getContent()
+      );
+    }
+  }
+
+  public void updateCompactionTaskSlot(Double compactionTaskSlotRatio, Integer maxCompactionTaskSlots) throws Exception
+  {
+    String url = StringUtils.format("%sconfig/compaction/taskslots?ratio=%s&max=%s",
+                                    getCoordinatorURL(),
+                                    StringUtils.urlEncode(compactionTaskSlotRatio.toString()),
+                                    StringUtils.urlEncode(maxCompactionTaskSlots.toString()));
+    StatusResponseHolder response = httpClient.go(new Request(HttpMethod.POST, new URL(url)), responseHandler).get();
+    if (!response.getStatus().equals(HttpResponseStatus.OK)) {
+      throw new ISE(
+          "Error while force trigger auto compaction status[%s] content[%s]",
 
 Review comment:
   Done

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] maytasm commented on a change in pull request #9644: Add missing integration tests for the compaction by the coordinator

Posted by GitBox <gi...@apache.org>.
maytasm commented on a change in pull request #9644: Add missing integration tests for the compaction by the coordinator
URL: https://github.com/apache/druid/pull/9644#discussion_r407037142
 
 

 ##########
 File path: integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java
 ##########
 @@ -0,0 +1,351 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.tests.coordinator.duty;
+
+import com.google.inject.Inject;
+import org.apache.commons.io.IOUtils;
+import org.apache.druid.indexer.partitions.SecondaryPartitionType;
+import org.apache.druid.java.util.common.ISE;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.common.logger.Logger;
+import org.apache.druid.server.coordinator.CoordinatorCompactionConfig;
+import org.apache.druid.server.coordinator.DataSourceCompactionConfig;
+import org.apache.druid.testing.IntegrationTestingConfig;
+import org.apache.druid.testing.clients.CompactionResourceTestClient;
+import org.apache.druid.testing.guice.DruidTestModuleFactory;
+import org.apache.druid.testing.utils.ITRetryUtil;
+import org.apache.druid.tests.TestNGGroup;
+import org.apache.druid.tests.indexer.AbstractITBatchIndexTest;
+import org.apache.druid.tests.indexer.AbstractIndexerTest;
+import org.apache.druid.timeline.DataSegment;
+import org.joda.time.Period;
+import org.testng.Assert;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Guice;
+import org.testng.annotations.Test;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+
+@Test(groups = {TestNGGroup.OTHER_INDEX})
+@Guice(moduleFactory = DruidTestModuleFactory.class)
+public class ITAutoCompactionTest extends AbstractIndexerTest
+{
+  private static final Logger LOG = new Logger(ITAutoCompactionTest.class);
+  private static final String INDEX_TASK = "/indexer/wikipedia_index_task.json";
+  private static final String INDEX_QUERIES_RESOURCE = "/indexer/wikipedia_index_queries.json";
+  private static final int MAX_ROWS_PER_SEGMENT_COMPACTED = 10000;
+  private static final Period SKIP_OFFSET_FROM_LATEST = Period.seconds(0);
+
+  @Inject
+  protected CompactionResourceTestClient compactionResource;
+
+  @Inject
+  private IntegrationTestingConfig config;
+
+  private String fullDatasourceName;
+
+  @BeforeMethod
+  public void setup() throws Exception
+  {
+    // Set comapction slot to 10
+    updateCompactionTaskSlot(0.5, 10);
+    fullDatasourceName = "wikipedia_index_test_" + UUID.randomUUID() + config.getExtraDatasourceNameSuffix();
+  }
+
+  @Test
+  public void testAutoCompactionDutySubmitAndVerifyCompaction() throws Exception
+  {
+    loadData(INDEX_TASK);
+    try (final Closeable ignored = unloader(fullDatasourceName)) {
+      final List<String> intervalsBeforeCompaction = coordinator.getSegmentIntervals(fullDatasourceName);
+      intervalsBeforeCompaction.sort(null);
+      // 4 segments across 2 days (4 total)...
+      verifySegmentsCount(4);
+      verifyQuery(INDEX_QUERIES_RESOURCE);
+
+      submitCompactionConfig(MAX_ROWS_PER_SEGMENT_COMPACTED, Period.days(1));
+      forceTriggerAutoCompaction();
+      //...compacted into 1 new segment for 1 day. 1 day compacted and 1 day skipped/remains uncompacted. (5 total)
+      verifySegmentsCount(5);
+      verifyQuery(INDEX_QUERIES_RESOURCE);
+      verifySegmentsCompacted(1, MAX_ROWS_PER_SEGMENT_COMPACTED);
+      checkCompactionIntervals(intervalsBeforeCompaction);
+
+      submitCompactionConfig(MAX_ROWS_PER_SEGMENT_COMPACTED, SKIP_OFFSET_FROM_LATEST);
+      forceTriggerAutoCompaction();
+      //...compacted into 1 new segment for the remaining one day. 2 day compacted and 0 day uncompacted. (6 total)
+      verifySegmentsCount(6);
+      verifyQuery(INDEX_QUERIES_RESOURCE);
+      verifySegmentsCompacted(2, MAX_ROWS_PER_SEGMENT_COMPACTED);
+      checkCompactionIntervals(intervalsBeforeCompaction);
+    }
+  }
+
+  @Test
+  public void testAutoCompactionDutyCanUpdateCompactionConfig() throws Exception
+  {
+    loadData(INDEX_TASK);
+    try (final Closeable ignored = unloader(fullDatasourceName)) {
+      final List<String> intervalsBeforeCompaction = coordinator.getSegmentIntervals(fullDatasourceName);
+      intervalsBeforeCompaction.sort(null);
+      // 4 segments across 2 days (4 total)...
+      verifySegmentsCount(4);
+      verifyQuery(INDEX_QUERIES_RESOURCE);
+
+      // Dummy compaction config which will be overwritten
+      submitCompactionConfig(10000, SKIP_OFFSET_FROM_LATEST);
+      // New compaction config should overwrites the existing compaction config
+      submitCompactionConfig(1, SKIP_OFFSET_FROM_LATEST);
+      forceTriggerAutoCompaction();
+
+      // Instead of merging segments, the updated config will split segments!
+      //...compacted into 10 new segments across 2 days. 5 new segments each day (14 total)
+      verifySegmentsCount(14);
+      verifyQuery(INDEX_QUERIES_RESOURCE);
+      verifySegmentsCompacted(10, 2);
+
+      checkCompactionIntervals(intervalsBeforeCompaction);
+    }
+  }
+
+  @Test
+  public void testAutoCompactionDutyCanDeleteCompactionConfig() throws Exception
+  {
+    loadData(INDEX_TASK);
+    try (final Closeable ignored = unloader(fullDatasourceName)) {
+      final List<String> intervalsBeforeCompaction = coordinator.getSegmentIntervals(fullDatasourceName);
+      intervalsBeforeCompaction.sort(null);
+      // 4 segments across 2 days (4 total)...
+      verifySegmentsCount(4);
+      verifyQuery(INDEX_QUERIES_RESOURCE);
+
+      submitCompactionConfig(MAX_ROWS_PER_SEGMENT_COMPACTED, SKIP_OFFSET_FROM_LATEST);
+      deleteCompactionConfig();
+      forceTriggerAutoCompaction();
+
+      // ...should remains unchanged (4 total)
+      verifySegmentsCount(4);
+      verifyQuery(INDEX_QUERIES_RESOURCE);
+      verifySegmentsCompacted(0, null);
+
+      checkCompactionIntervals(intervalsBeforeCompaction);
+    }
+  }
+
+  @Test
+  public void testAutoCompactionDutyCanUpdateTaskSlots() throws Exception
+  {
+    loadData(INDEX_TASK);
+    try (final Closeable ignored = unloader(fullDatasourceName)) {
+      final List<String> intervalsBeforeCompaction = coordinator.getSegmentIntervals(fullDatasourceName);
+      intervalsBeforeCompaction.sort(null);
+      // 4 segments across 2 days (4 total)...
+      verifySegmentsCount(4);
+      verifyQuery(INDEX_QUERIES_RESOURCE);
+
+      // Skips first day. Should only compact one out of two days.
+      submitCompactionConfig(MAX_ROWS_PER_SEGMENT_COMPACTED, SKIP_OFFSET_FROM_LATEST);
+
+      // Set compactionTaskSlotRatio to 0 to prevent any compaction
+      updateCompactionTaskSlot(0, 100);
+      forceTriggerAutoCompaction();
+      // ...should remains unchanged (4 total)
+      verifySegmentsCount(4);
+      verifyQuery(INDEX_QUERIES_RESOURCE);
+      verifySegmentsCompacted(0, null);
+      checkCompactionIntervals(intervalsBeforeCompaction);
+
+      // Set maxCompactionTaskSlots to 0 to prevent any compaction
+      updateCompactionTaskSlot(0.1, 0);
+      forceTriggerAutoCompaction();
+      // ...should remains unchanged (4 total)
+      verifySegmentsCount(4);
+      verifyQuery(INDEX_QUERIES_RESOURCE);
+      verifySegmentsCompacted(0, null);
+      checkCompactionIntervals(intervalsBeforeCompaction);
+
+      // Update compaction slots to be 1
+      updateCompactionTaskSlot(1, 1);
+      forceTriggerAutoCompaction();
+      // One day compacted (1 new segment) and one day remains uncompacted. (5 total)
+      verifySegmentsCount(5);
+      verifyQuery(INDEX_QUERIES_RESOURCE);
+      verifySegmentsCompacted(1, MAX_ROWS_PER_SEGMENT_COMPACTED);
+      checkCompactionIntervals(intervalsBeforeCompaction);
+      Assert.assertEquals(compactionResource.getCompactionProgress(fullDatasourceName).get("remainingSegmentSize"), "14312");
+      // Run compaction again to compact the remaining day
+      forceTriggerAutoCompaction();
+      // Remaining day compacted (1 new segment). Now both days compacted (6 total)
+      verifySegmentsCount(6);
+      verifyQuery(INDEX_QUERIES_RESOURCE);
+      verifySegmentsCompacted(2, MAX_ROWS_PER_SEGMENT_COMPACTED);
+      checkCompactionIntervals(intervalsBeforeCompaction);
+//      Assert.assertEquals(compactionResource.getCompactionProgress(fullDatasourceName).get("remainingSegmentSize"), "0");
 
 Review comment:
   @jihoonson Not sure if this is expected but when I call the API for get compaction progress here I got status[400 Bad Request] content[{"error":"unknown dataSource"}] instead of 0. I expected 0 since all segments of the datasource have been compacted.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] ccaominh commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests

Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9644: Add API to trigger a compaction by the coordinator for integration tests
URL: https://github.com/apache/druid/pull/9644#discussion_r406358984
 
 

 ##########
 File path: server/src/main/java/org/apache/druid/server/http/CompactionResource.java
 ##########
 @@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.server.http;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.inject.Inject;
+import com.sun.jersey.spi.container.ResourceFilters;
+import org.apache.druid.server.coordinator.DruidCoordinator;
+import org.apache.druid.server.http.security.ConfigResourceFilter;
+import org.apache.druid.server.http.security.StateResourceFilter;
+
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+
+@Path("/druid/coordinator/v1/compaction")
 
 Review comment:
   Should these APIs be documented?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org