aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wfar...@apache.org
Subject git commit: Properly handle filtering offers when multiple attributes have the same name.
Date Mon, 17 Mar 2014 19:20:00 GMT
Repository: incubator-aurora
Updated Branches:
  refs/heads/master a1949e5e3 -> fe6ab446a


Properly handle filtering offers when multiple attributes have the same name.

Bugs closed: AURORA-264

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


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

Branch: refs/heads/master
Commit: fe6ab446aa64cdb8052935dea1545f7843082a6a
Parents: a1949e5
Author: Bill Farner <wfarner@apache.org>
Authored: Mon Mar 17 12:19:35 2014 -0700
Committer: Bill Farner <wfarner@apache.org>
Committed: Mon Mar 17 12:19:35 2014 -0700

----------------------------------------------------------------------
 .../scheduler/filter/AttributeFilter.java       | 19 ++-----------
 .../scheduler/filter/ConstraintFilter.java      | 30 ++++++++++++++++----
 .../thrift/org/apache/aurora/gen/api.thrift     |  3 +-
 .../filter/SchedulingFilterImplTest.java        | 16 +++++++++++
 .../org/apache/aurora/gen/api.thrift.md5        |  2 +-
 5 files changed, 46 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/fe6ab446/src/main/java/org/apache/aurora/scheduler/filter/AttributeFilter.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/filter/AttributeFilter.java b/src/main/java/org/apache/aurora/scheduler/filter/AttributeFilter.java
index 5f1e431..bcd9062 100644
--- a/src/main/java/org/apache/aurora/scheduler/filter/AttributeFilter.java
+++ b/src/main/java/org/apache/aurora/scheduler/filter/AttributeFilter.java
@@ -17,10 +17,7 @@ package org.apache.aurora.scheduler.filter;
 
 import java.util.Set;
 
-import com.google.common.base.Function;
-import com.google.common.base.Optional;
 import com.google.common.base.Predicates;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 
 import org.apache.aurora.gen.Attribute;
@@ -31,14 +28,6 @@ import org.apache.aurora.scheduler.storage.entities.IValueConstraint;
  */
 final class AttributeFilter {
 
-  private static final Function<Attribute, Set<String>> GET_VALUES =
-      new Function<Attribute, Set<String>>() {
-        @Override
-        public Set<String> apply(Attribute attribute) {
-          return attribute.getValues();
-        }
-      };
-
   private AttributeFilter() {
     // Utility class.
   }
@@ -46,14 +35,12 @@ final class AttributeFilter {
   /**
    * Tests whether a constraint is satisfied by attributes.
    *
-   * @param attribute Host attribute.
+   * @param values Host attribute values.
    * @param constraint Constraint to match.
    * @return {@code true} if the attribute satisfies the constraint, {@code false} otherwise.
    */
-  static boolean matches(Optional<Attribute> attribute, IValueConstraint constraint)
{
-    Set<String> allAttributes =
-        ImmutableSet.copyOf(attribute.transform(GET_VALUES).or(ImmutableSet.<String>of()));
-    boolean match = Iterables.any(constraint.getValues(), Predicates.in(allAttributes));
+  static boolean matches(Set<String> values, IValueConstraint constraint) {
+    boolean match = Iterables.any(constraint.getValues(), Predicates.in(values));
     return constraint.isNegated() ^ match;
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/fe6ab446/src/main/java/org/apache/aurora/scheduler/filter/ConstraintFilter.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/filter/ConstraintFilter.java b/src/main/java/org/apache/aurora/scheduler/filter/ConstraintFilter.java
index 2360b1a..e1ebe3f 100644
--- a/src/main/java/org/apache/aurora/scheduler/filter/ConstraintFilter.java
+++ b/src/main/java/org/apache/aurora/scheduler/filter/ConstraintFilter.java
@@ -15,9 +15,13 @@
  */
 package org.apache.aurora.scheduler.filter;
 
+import java.util.Set;
+
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Function;
 import com.google.common.base.Optional;
 import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 
 import org.apache.aurora.gen.Attribute;
@@ -61,6 +65,14 @@ class ConstraintFilter {
     return new Veto("Host " + reason + " for maintenance", Veto.MAX_SCORE);
   }
 
+  private static final Function<Attribute, Set<String>> GET_VALUES =
+      new Function<Attribute, Set<String>>() {
+        @Override
+        public Set<String> apply(Attribute attribute) {
+          return attribute.getValues();
+        }
+      };
+
   /**
    * Gets the veto (if any) for a scheduling constraint based on the {@link AttributeAggregate}
this
    * filter was created with.
@@ -69,15 +81,23 @@ class ConstraintFilter {
    * @return A veto if the constraint is not satisfied based on the existing state of the
job.
    */
   Optional<Veto> getVeto(IConstraint constraint) {
-    // Per TODO in api.thrift, we expect host attributes to have unique names.
-    Optional<Attribute> attribute = Optional.fromNullable(Iterables.getOnlyElement(
-        Iterables.filter(hostAttributes, new NameFilter(constraint.getName())), null));
+    Iterable<Attribute> sameNameAttributes =
+        Iterables.filter(hostAttributes, new NameFilter(constraint.getName()));
+    Optional<Attribute> attribute;
+    if (Iterables.isEmpty(sameNameAttributes)) {
+      attribute = Optional.absent();
+    } else {
+      Set<String> attributeValues = ImmutableSet.copyOf(
+          Iterables.concat(Iterables.transform(sameNameAttributes, GET_VALUES)));
+      attribute = Optional.of(new Attribute(constraint.getName(), attributeValues));
+    }
 
     ITaskConstraint taskConstraint = constraint.getConstraint();
     switch (taskConstraint.getSetField()) {
       case VALUE:
-        boolean matches =
-            AttributeFilter.matches(attribute, taskConstraint.getValue());
+        boolean matches = AttributeFilter.matches(
+            attribute.transform(GET_VALUES).or(ImmutableSet.<String>of()),
+            taskConstraint.getValue());
         return matches
             ? Optional.<Veto>absent()
             : Optional.of(mismatchVeto(constraint.getName()));

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/fe6ab446/src/main/thrift/org/apache/aurora/gen/api.thrift
----------------------------------------------------------------------
diff --git a/src/main/thrift/org/apache/aurora/gen/api.thrift b/src/main/thrift/org/apache/aurora/gen/api.thrift
index f6c444d..e64f844 100644
--- a/src/main/thrift/org/apache/aurora/gen/api.thrift
+++ b/src/main/thrift/org/apache/aurora/gen/api.thrift
@@ -73,8 +73,7 @@ enum MaintenanceMode {
 // The attributes assigned to a host.
 struct HostAttributes {
   1: string          host
-  2: set<Attribute>  attributes  // TODO(William Farner): Change this to map<String,
Attribute>
-                                 // and remove name field from Attribute.
+  2: set<Attribute>  attributes
   3: optional MaintenanceMode mode
   4: optional string slaveId
 }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/fe6ab446/src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
b/src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
index 79d14c9..d2e1166 100644
--- a/src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
@@ -460,6 +460,22 @@ public class SchedulingFilterImplTest extends EasyMockTest {
     assertEquals((int) (Veto.MAX_SCORE * 200.0 / DISK.getRange()), DISK.veto(200).getScore());
   }
 
+  @Test
+  public void testDuplicatedAttribute() throws Exception {
+    expectGetHostAttributes(HOST_A,
+        valueAttribute("jvm", "1.4"),
+        valueAttribute("jvm", "1.6", "1.7")).atLeastOnce();
+    expectGetHostMaintenanceStatus(HOST_A).atLeastOnce();
+
+    control.replay();
+
+    // Matches attribute, matching value.
+    checkConstraint(HOST_A, "jvm", true, "1.4");
+    checkConstraint(HOST_A, "jvm", true, "1.6");
+    checkConstraint(HOST_A, "jvm", true, "1.7");
+    checkConstraint(HOST_A, "jvm", true, "1.6", "1.7");
+  }
+
   private ITaskConfig checkConstraint(
       String host,
       String constraintName,

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/fe6ab446/src/test/resources/org/apache/aurora/gen/api.thrift.md5
----------------------------------------------------------------------
diff --git a/src/test/resources/org/apache/aurora/gen/api.thrift.md5 b/src/test/resources/org/apache/aurora/gen/api.thrift.md5
index 8b6f0a1..456634c 100644
--- a/src/test/resources/org/apache/aurora/gen/api.thrift.md5
+++ b/src/test/resources/org/apache/aurora/gen/api.thrift.md5
@@ -1 +1 @@
-81e3efd49d0b177759414564a4903a3d
+40e828a92f5193776f50e982b01a4663


Mime
View raw message