AmatyaAvadhanula commented on code in PR #16393:
URL: https://github.com/apache/druid/pull/16393#discussion_r1592758876


##########
indexing-service/src/main/java/org/apache/druid/indexing/common/actions/SegmentTransactionalAppendAction.java:
##########
@@ -148,46 +148,43 @@ public SegmentPublishResult perform(Task task, 
TaskActionToolbox toolbox)
           task.getType()
       );
     }
-    // Verify that all the locks are of expected type
-    final List<TaskLock> locks = 
toolbox.getTaskLockbox().findLocksForTask(task);
-    for (TaskLock lock : locks) {
-      if (lock.getType() != TaskLockType.APPEND) {
-        throw InvalidInput.exception(
-            "Cannot use action[%s] for task[%s] as it is holding a lock of 
type[%s] instead of [APPEND].",
-            "SegmentTransactionalAppendAction", task.getId(), lock.getType()
+    final TaskLockbox taskLockbox = toolbox.getTaskLockbox();
+    TaskLocks.checkLockCoversSegments(task, taskLockbox, segments);
+    for (TaskLock taskLock : taskLockbox.findLocksForTask(task)) {
+      if (taskLock.getType() != TaskLockType.APPEND) {
+        throw DruidException.defensive(

Review Comment:
   Done.
   However, defensive does seem like the right exception since the code 
dictates which task action is called based on the context. The same context 
determines the lock type. Any mismatch is likely the result of a bug.



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

Reply via email to