You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by wf...@apache.org on 2017/07/31 13:04:37 UTC

aurora git commit: Fix preemption ordering when tasks contain different ResourceTypes

Repository: aurora
Updated Branches:
  refs/heads/master cdc5b8efd -> 65257d63b


Fix preemption ordering when tasks contain different ResourceTypes

Bugs closed: AURORA-1943

Reviewed at https://reviews.apache.org/r/61238/


Project: http://git-wip-us.apache.org/repos/asf/aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/65257d63
Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/65257d63
Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/65257d63

Branch: refs/heads/master
Commit: 65257d63ba815241651c2bad9b9531db034c6096
Parents: cdc5b8e
Author: Bill Farner <wf...@apache.org>
Authored: Mon Jul 31 09:04:43 2017 -0400
Committer: Bill Farner <wf...@apache.org>
Committed: Mon Jul 31 09:04:43 2017 -0400

----------------------------------------------------------------------
 .../preemptor/PreemptionVictimFilter.java       | 36 ++++++++++++++------
 .../preemptor/PreemptionVictimFilterTest.java   | 24 +++++++++++++
 2 files changed, 50 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/aurora/blob/65257d63/src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java b/src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
index 5ed578c..1b12397 100644
--- a/src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
+++ b/src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
@@ -24,7 +24,6 @@ import com.google.common.base.Function;
 import com.google.common.base.Optional;
 import com.google.common.base.Predicate;
 import com.google.common.collect.FluentIterable;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Ordering;
@@ -40,6 +39,7 @@ import org.apache.aurora.scheduler.filter.SchedulingFilter.UnusedResource;
 import org.apache.aurora.scheduler.filter.SchedulingFilter.Veto;
 import org.apache.aurora.scheduler.resources.ResourceBag;
 import org.apache.aurora.scheduler.resources.ResourceManager;
+import org.apache.aurora.scheduler.resources.ResourceType;
 import org.apache.aurora.scheduler.storage.Storage.StoreProvider;
 import org.apache.aurora.scheduler.storage.entities.IHostAttributes;
 import org.apache.aurora.scheduler.storage.entities.ITaskConfig;
@@ -131,8 +131,6 @@ public interface PreemptionVictimFilter {
           }
         };
 
-    private static final java.util.function.Predicate<Integer> IS_ZERO = e -> e == 0;
-
     /**
      * A Resources object is greater than another iff _all_ of its resource components are greater.
      * A Resources object compares as equal if some but not all components are greater
@@ -142,21 +140,39 @@ public interface PreemptionVictimFilter {
     static final Ordering<ResourceBag> ORDER = new Ordering<ResourceBag>() {
       @Override
       public int compare(ResourceBag left, ResourceBag right) {
-        ImmutableList.Builder<Integer> builder = ImmutableList.builder();
-        left.streamResourceVectors().forEach(
-            entry -> builder.add(entry.getValue().compareTo(right.valueOf(entry.getKey()))));
+        Set<ResourceType> types = ImmutableSet.<ResourceType>builder()
+            .addAll(left.streamResourceVectors().map(e -> e.getKey()).iterator())
+            .addAll(right.streamResourceVectors().map(e -> e.getKey()).iterator())
+            .build();
+
+        boolean allZero = true;
+        boolean allGreaterOrEqual = true;
+        boolean allLessOrEqual = true;
+
+        for (ResourceType type : types) {
+          int compare = left.valueOf(type).compareTo(right.valueOf(type));
+          if (compare != 0) {
+            allZero = false;
+          }
+
+          if (compare < 0) {
+            allGreaterOrEqual = false;
+          }
 
-        List<Integer> results = builder.build();
+          if (compare > 0) {
+            allLessOrEqual = false;
+          }
+        }
 
-        if (results.stream().allMatch(IS_ZERO))  {
+        if (allZero) {
           return 0;
         }
 
-        if (results.stream().filter(IS_ZERO.negate()).allMatch(e -> e > 0)) {
+        if (allGreaterOrEqual) {
           return 1;
         }
 
-        if (results.stream().filter(IS_ZERO.negate()).allMatch(e -> e < 0)) {
+        if (allLessOrEqual) {
           return -1;
         }
 

http://git-wip-us.apache.org/repos/asf/aurora/blob/65257d63/src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java b/src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
index c39b00d..b8d7506 100644
--- a/src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
@@ -20,6 +20,7 @@ import java.util.stream.IntStream;
 import com.google.common.base.Optional;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 
@@ -77,6 +78,7 @@ import static org.apache.aurora.scheduler.resources.ResourceTestUtil.mesosRange;
 import static org.apache.aurora.scheduler.resources.ResourceTestUtil.mesosScalar;
 import static org.apache.aurora.scheduler.resources.ResourceType.CPUS;
 import static org.apache.aurora.scheduler.resources.ResourceType.DISK_MB;
+import static org.apache.aurora.scheduler.resources.ResourceType.GPUS;
 import static org.apache.aurora.scheduler.resources.ResourceType.PORTS;
 import static org.apache.aurora.scheduler.resources.ResourceType.RAM_MB;
 import static org.apache.mesos.v1.Protos.Offer;
@@ -565,6 +567,28 @@ public class PreemptionVictimFilterTest extends EasyMockTest {
         ORDER.sortedCopy(ImmutableList.of(three, one, two, three)));
   }
 
+  @Test
+  public void testOrderDifferentResources() {
+    control.replay();
+
+    ResourceBag one = bag(ImmutableMap.of(
+        CPUS, 1.0,
+        RAM_MB, 1.0
+    ));
+
+    ResourceBag two = bag(ImmutableMap.of(
+        CPUS, 1.0,
+        GPUS, 1.0
+    ));
+
+    ResourceBag three = bag(ImmutableMap.of(
+        CPUS, 1.0
+    ));
+
+    assertEquals(0, ORDER.compare(one, two));
+    assertEquals(1, ORDER.compare(one, three));
+  }
+
   private static ImmutableSet<PreemptionVictim> preemptionVictims(ScheduledTask... tasks) {
     return FluentIterable.from(ImmutableSet.copyOf(tasks))
         .transform(