[GitHub] [druid] jihoonson commented on a change in pull request #11190: Temporarily skip compaction for locked intervals

[GitBox <gi...@apache.org>]

jihoonson commented on a change in pull request #11190:
URL: https://github.com/apache/druid/pull/11190#discussion_r626290806



##########
File path: indexing-service/src/main/java/org/apache/druid/indexing/overlord/TaskLockbox.java
##########
@@ -674,6 +675,64 @@ public TaskLock apply(TaskLockPosse taskLockPosse)
     }
   }
 
+  /**
+   * Gets a Map containing intervals locked by active tasks. Intervals locked
+   * by revoked TaskLocks are not included in the returned Map.
+   *
+   * @return Map from Task Id to locked intervals.
+   */
+  public Map<String, DatasourceIntervals> getLockedIntervals()

Review comment:
       `getLockedInterval()` seems a misnomer because this method returns the intervals of segment locks as well, but they don't lock intervals. I don't have a better suggestion though..

##########
File path: docs/operations/api-reference.md
##########
@@ -614,6 +614,11 @@ Retrieve information about the segments of a task.
 
 Retrieve a [task completion report](../ingestion/tasks.md#task-reports) for a task. Only works for completed tasks.
 
+* `/druid/indexer/v1/lockedIntervals`

Review comment:
       Is this supposed to be called by users? What is the use case?

##########
File path: server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java
##########
@@ -154,6 +160,17 @@ public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params)
           }
         }
 
+        // Skip all the locked intervals
+        LOG.debug(
+            "Skipping the following intervals for Compaction as they are currently locked: %s",
+            taskToLockedIntervals
+        );
+        taskToLockedIntervals.forEach(

Review comment:
       I think it should behave differently depending on what `lockGranularity` is used. If both the compaction task to run and the task that is already running use the segment lock, the compaction task can safely run. Otherwise, the entire locked interval should be skipped as what this code does. 




-- 
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



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