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 <[email protected]> Authored: Mon Jul 31 09:04:43 2017 -0400 Committer: Bill Farner <[email protected]> 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(
