kfaraz commented on code in PR #13390:
URL: https://github.com/apache/druid/pull/13390#discussion_r1027337562
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/TaskLockbox.java:
##########
@@ -474,8 +474,12 @@ private TaskLockPosse createOrFindLockPosse(LockRequest
request)
if (reusablePosses.size() == 0) {
// case 1) this task doesn't have any lock, but others do
- if (request.getType().equals(TaskLockType.SHARED) &&
isAllSharedLocks(conflictPosses)) {
- // Any number of shared locks can be acquired for the same
dataSource and interval.
+ if (request.getType().equals(TaskLockType.SHARED)
+ &&
areAllEqualOrHigherPriorityLocksSharedOrRevoked(conflictPosses,
request.getPriority())) {
+ // Any number of shared locks can be acquired for the same
dataSource and interval
+ // Non-shared locks of equal or greater priority, if present, must
already be revoked
+ // Other exclusive (or any non-shared) locks of lower priority can
be revoked
Review Comment:
Shared and exclusive locks are the only two types of locks.
So the phrase `exclusive (or any non-shared) locks` can cause confusion to
readers.
If we add a new type of lock in the future, this can be updated.
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/TaskLockbox.java:
##########
@@ -1070,10 +1074,33 @@ Map<String, NavigableMap<DateTime, SortedMap<Interval,
List<TaskLockPosse>>>> ge
return running;
}
- private static boolean isAllSharedLocks(List<TaskLockPosse> lockPosses)
+ /**
+ * Check if all lockPosses are either shared
+ * OR of lower priority
+ * OR are revoked non-shared locks if their priorities are greater than or
equal to the provided priority
+ * @param lockPosses conflicting task lock posses to be checked
+ * @param priority priority of the lock to be acquired
+ * @return true if the condititons are met
+ */
+ private static boolean
areAllEqualOrHigherPriorityLocksSharedOrRevoked(List<TaskLockPosse> lockPosses,
int priority)
{
return lockPosses.stream()
- .allMatch(taskLockPosse ->
taskLockPosse.getTaskLock().getType().equals(TaskLockType.SHARED));
+ .filter(taskLockPosse ->
taskLockPosse.getTaskLock().getPriority() >= priority)
Review Comment:
Better use the non-null priority as done in `isRevocable`.
--
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]
