kfaraz commented on code in PR #16393:
URL: https://github.com/apache/druid/pull/16393#discussion_r1592615193
##########
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:
`defensive` is probably the right exception type here. Defensive exceptions
are targeted at developers to identify a bug in the code. We should probably
just stick to what we had earlier, i.e. `InvalidInput.exception()`. The older
message also read better, so let's revert this part of the change.
##########
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:
`defensive` is probably not the right exception type here. Defensive
exceptions are targeted at developers to identify a bug in the code. We should
probably just stick to what we had earlier, i.e. `InvalidInput.exception()`.
The older message also read better, so let's revert this part of the change.
--
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]
