YARN-4537. Pull out priority comparison from fifocomparator and use compound comparator for FifoOrdering policy. Contributed by Rohith Sharma K S

hadoop-yarn-project/CHANGES.txt

@@ -683,6 +683,9 @@ Release 2.8.0 - UNRELEASED
     YARN-4544. All the log messages about rolling monitoring interval are
     shown with WARN level. (Takashi Ohnishi via aajisaka)
 
+    YARN-4537. Pull out priority comparison from fifocomparator and use compound
+    comparator for FifoOrdering policy. (Rohith Sharma K S via jianhe)
+
   OPTIMIZATIONS
 
     YARN-3339. TestDockerContainerExecutor should pull a single image and not

hadoop-yarn-project/hadoop-yarn/dev-support/findbugs-exclude.xml

@@ -158,6 +158,10 @@
     <Class name="org.apache.hadoop.yarn.server.resourcemanager.scheduler.policy.CompoundComparator" />
     <Bug pattern="SE_COMPARATOR_SHOULD_BE_SERIALIZABLE" />
   </Match>
+  <Match>
+    <Class name="org.apache.hadoop.yarn.server.resourcemanager.scheduler.policy.PriorityComparator" />
+    <Bug pattern="SE_COMPARATOR_SHOULD_BE_SERIALIZABLE" />
+  </Match>
   <Match>
     <Class name="org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.PartitionedQueueComparator" />
     <Bug pattern="SE_COMPARATOR_SHOULD_BE_SERIALIZABLE" />

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/policy/FifoComparator.java

@@ -20,9 +20,6 @@
 
 import java.util.*;
 
-import org.apache.hadoop.yarn.server.resourcemanager.rmcontainer.RMContainer;
-import org.apache.hadoop.yarn.server.resourcemanager.scheduler.*;
-
 /**
  * A Comparator which orders SchedulableEntities by input order
  */
@@ -31,10 +28,6 @@ public class FifoComparator
       
     @Override
   public int compare(SchedulableEntity r1, SchedulableEntity r2) {
-    if (r1.getPriority() != null
-        && !r1.getPriority().equals(r2.getPriority())) {
-      return r1.getPriority().compareTo(r2.getPriority());
-    }
     int res = r1.compareInputOrderTo(r2);
     return res;
   }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/policy/FifoOrderingPolicy.java

@@ -20,7 +20,6 @@
 
 import java.util.*;
 import org.apache.hadoop.yarn.server.resourcemanager.rmcontainer.RMContainer;
-import org.apache.hadoop.yarn.server.resourcemanager.scheduler.*;
 
 /**
  * An OrderingPolicy which orders SchedulableEntities by input order
@@ -28,8 +27,13 @@
 public class FifoOrderingPolicy<S extends SchedulableEntity> extends AbstractComparatorOrderingPolicy<S> {
   
   public FifoOrderingPolicy() {
-    this.comparator = new FifoComparator();
+    List<Comparator<SchedulableEntity>> comparators =
+        new ArrayList<Comparator<SchedulableEntity>>();
+    comparators.add(new PriorityComparator());
+    comparators.add(new FifoComparator());
+    this.comparator = new CompoundComparator(comparators);
     this.schedulableEntities = new TreeSet<S>(comparator);
+
   }
   
   @Override

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/policy/TestFifoOrderingPolicy.java

@@ -24,11 +24,6 @@
 import org.junit.Test;
 
 import org.apache.hadoop.yarn.api.records.Priority;
-import org.apache.hadoop.yarn.api.records.Resource;
-import org.apache.hadoop.yarn.util.resource.Resources;
-import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
-
-import org.apache.hadoop.yarn.server.resourcemanager.scheduler.common.fica.FiCaSchedulerApp;
 
 public class TestFifoOrderingPolicy {
   
@@ -80,4 +75,32 @@ public void checkSerials(Iterator<MockSchedulableEntity> si,
     }
   }
   
+  @Test
+  public void testFifoOrderingPolicyAlongWithPriorty() {
+    FifoOrderingPolicy<MockSchedulableEntity> policy =
+        new FifoOrderingPolicy<MockSchedulableEntity>();
+    MockSchedulableEntity r1 = new MockSchedulableEntity();
+    MockSchedulableEntity r2 = new MockSchedulableEntity();
+
+    Priority p1 = Priority.newInstance(1);
+    Priority p2 = Priority.newInstance(0);
+
+    // Both r1 and r1 priority is null
+    Assert.assertEquals(0, policy.getComparator().compare(r1, r2));
+
+    // r1 is null and r2 is not null
+    r2.setApplicationPriority(p2);
+    Assert.assertEquals(-1, policy.getComparator().compare(r1, r2));
+
+    // r1 is not null and r2 is null
+    r2.setApplicationPriority(null);
+    r1.setApplicationPriority(p1);
+    Assert.assertEquals(1, policy.getComparator().compare(r1, r2));
+
+    // r1 is not null and r2 is not null
+    r1.setApplicationPriority(p1);
+    r2.setApplicationPriority(p2);
+    Assert.assertEquals(-1, policy.getComparator().compare(r1, r2));
+  }
+
 }