You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by ch...@apache.org on 2023/05/11 07:54:52 UTC
[druid] branch master updated: Remove incorrect optimization (#14246)
This is an automated email from the ASF dual-hosted git repository.
cheddar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 47e48ee657 Remove incorrect optimization (#14246)
47e48ee657 is described below
commit 47e48ee65710719c6af1605fedf7a5063c83cc38
Author: AmatyaAvadhanula <am...@imply.io>
AuthorDate: Thu May 11 13:24:41 2023 +0530
Remove incorrect optimization (#14246)
---
.../druid/indexing/overlord/TaskLockbox.java | 15 +------
.../druid/indexing/overlord/TaskLockboxTest.java | 46 ++++++++++++++++++++++
2 files changed, 47 insertions(+), 14 deletions(-)
diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/TaskLockbox.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/TaskLockbox.java
index 9a2f785422..927b09557f 100644
--- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/TaskLockbox.java
+++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/TaskLockbox.java
@@ -24,7 +24,6 @@ import com.google.common.base.Objects;
import com.google.common.base.Preconditions;
import com.google.common.collect.ComparisonChain;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Ordering;
import com.google.inject.Inject;
@@ -55,7 +54,6 @@ import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.NavigableMap;
-import java.util.NavigableSet;
import java.util.Set;
import java.util.SortedMap;
import java.util.TreeMap;
@@ -64,7 +62,6 @@ import java.util.concurrent.locks.Condition;
import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Consumer;
import java.util.stream.Collectors;
-import java.util.stream.StreamSupport;
/**
* Remembers which activeTasks have locked which intervals or which segments. Tasks are permitted to lock an interval
@@ -1155,17 +1152,7 @@ public class TaskLockbox
// No locks at all
return Collections.emptyList();
} else {
- // Tasks are indexed by locked interval, which are sorted by interval start. Intervals are non-overlapping, so:
- final NavigableSet<DateTime> dsLockbox = dsRunning.navigableKeySet();
- final Iterable<DateTime> searchStartTimes = Iterables.concat(
- // Single interval that starts at or before ours
- Collections.singletonList(dsLockbox.floor(interval.getStart())),
-
- // All intervals that start somewhere between our start instant (exclusive) and end instant (exclusive)
- dsLockbox.subSet(interval.getStart(), false, interval.getEnd(), false)
- );
-
- return StreamSupport.stream(searchStartTimes.spliterator(), false)
+ return dsRunning.navigableKeySet().stream()
.filter(java.util.Objects::nonNull)
.map(dsRunning::get)
.filter(java.util.Objects::nonNull)
diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/TaskLockboxTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/TaskLockboxTest.java
index fd93aff2ba..de847d59c0 100644
--- a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/TaskLockboxTest.java
+++ b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/TaskLockboxTest.java
@@ -1357,6 +1357,52 @@ public class TaskLockboxTest
result.getTasksToFail());
}
+ @Test
+ public void testConflictsWithOverlappingSharedLocks() throws Exception
+ {
+ final List<Task> tasks = new ArrayList<>();
+
+ final Task conflictingTask = NoopTask.create(10);
+ tasks.add(conflictingTask);
+ lockbox.add(conflictingTask);
+ taskStorage.insert(conflictingTask, TaskStatus.running(conflictingTask.getId()));
+ TaskLock conflictingLock = tryTimeChunkLock(
+ TaskLockType.SHARED,
+ conflictingTask,
+ Intervals.of("2023-05-01/2023-06-01")
+ ).getTaskLock();
+ Assert.assertNotNull(conflictingLock);
+ Assert.assertFalse(conflictingLock.isRevoked());
+
+ final Task floorTask = NoopTask.create(10);
+ tasks.add(floorTask);
+ lockbox.add(floorTask);
+ taskStorage.insert(floorTask, TaskStatus.running(floorTask.getId()));
+ TaskLock floorLock = tryTimeChunkLock(
+ TaskLockType.SHARED,
+ floorTask,
+ Intervals.of("2023-05-26/2023-05-27")
+ ).getTaskLock();
+ Assert.assertNotNull(floorLock);
+ Assert.assertFalse(floorLock.isRevoked());
+
+ final Task rightOverlapTask = NoopTask.create(10);
+ tasks.add(rightOverlapTask);
+ lockbox.add(rightOverlapTask);
+ taskStorage.insert(rightOverlapTask, TaskStatus.running(rightOverlapTask.getId()));
+ TaskLock rightOverlapLock = tryTimeChunkLock(
+ TaskLockType.EXCLUSIVE,
+ rightOverlapTask,
+ Intervals.of("2023-05-28/2023-06-03")
+ ).getTaskLock();
+ Assert.assertNull(rightOverlapLock);
+
+ Assert.assertEquals(
+ ImmutableSet.of(conflictingLock, floorLock),
+ getAllActiveLocks(tasks)
+ );
+ }
+
private Set<TaskLock> getAllActiveLocks(List<Task> tasks)
{
return tasks.stream()
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org