From a8bd516e39fa82eea2a42b8085f3171fb0e1a883 Mon Sep 17 00:00:00 2001
From: Szilard Nemeth <snemeth@apache.org>
Date: Wed, 10 Feb 2021 11:15:53 +0100
Subject: [PATCH] YARN-10620. fs2cs: parentQueue for certain placement rules
 are not set during conversion. Contributed by Peter Bacsko

---
 .../converter/QueuePlacementConverter.java    | 20 +++++++++
 .../TestQueuePlacementConverter.java          | 45 +++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/QueuePlacementConverter.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/QueuePlacementConverter.java
index 0dd6ba4e67..65d9505340 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/QueuePlacementConverter.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/QueuePlacementConverter.java
@@ -17,7 +17,9 @@ package org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.converter;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
 
+import org.apache.hadoop.thirdparty.com.google.common.collect.Sets;
 import org.apache.hadoop.yarn.server.resourcemanager.placement.DefaultPlacementRule;
 import org.apache.hadoop.yarn.server.resourcemanager.placement.FSPlacementRule;
 import org.apache.hadoop.yarn.server.resourcemanager.placement.PlacementManager;
@@ -38,6 +40,12 @@ class QueuePlacementConverter {
   private static final FallbackResult SKIP_RESULT = FallbackResult.SKIP;
   private static final String DEFAULT_QUEUE = "root.default";
   private static final String MATCH_ALL_USER = "*";
+  private static final Set<Policy> NEED_ROOT_PARENT = Sets.newHashSet(
+      Policy.USER,
+      Policy.PRIMARY_GROUP,
+      Policy.PRIMARY_GROUP_USER,
+      Policy.SECONDARY_GROUP,
+      Policy.SECONDARY_GROUP_USER);
 
   MappingRulesDescription convertPlacementPolicy(
       PlacementManager placementManager,
@@ -162,6 +170,16 @@ class QueuePlacementConverter {
       }
     }
 
+    // Need to set the parent queue in weight mode.
+    //
+    // We *don't* set in pct mode, because auto-creation under "root"
+    // is not possible and probably it can cause the validation step to fail
+    // if create=true.
+    if (!usePercentages &&
+        NEED_ROOT_PARENT.contains(policy)) {
+      rule.setParentQueue("root");
+    }
+
     return rule;
   }
 
@@ -175,6 +193,8 @@ class QueuePlacementConverter {
 
     Rule rule = createRule(policy, create, ruleHandler, usePercentages);
 
+    // "parent" is already set to "root" at this point,
+    // so we override it if necessary
     if (parentQueue != null) {
       rule.setParentQueue(parentQueue);
     }
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestQueuePlacementConverter.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestQueuePlacementConverter.java
index 2f041c2afb..a1a19f0702 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestQueuePlacementConverter.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestQueuePlacementConverter.java
@@ -368,6 +368,51 @@ public class TestQueuePlacementConverter {
     }
   }
 
+  @Test
+  public void testParentSetToRootInWeightModeUserPolicy() {
+    UserPlacementRule fsRule = mock(UserPlacementRule.class);
+    testParentSetToRootInWeightMode(fsRule);
+  }
+
+  @Test
+  public void testParentSetToRootInWeightModePrimaryGroupPolicy() {
+    PrimaryGroupPlacementRule fsRule = mock(PrimaryGroupPlacementRule.class);
+    testParentSetToRootInWeightMode(fsRule);
+  }
+
+  @Test
+  public void testParentSetToRootInWeightModePrimaryGroupUserPolicy() {
+    UserPlacementRule fsRule = mock(UserPlacementRule.class);
+    PrimaryGroupPlacementRule parent = mock(PrimaryGroupPlacementRule.class);
+    when(fsRule.getParentRule()).thenReturn(parent);
+    testParentSetToRootInWeightMode(fsRule);
+  }
+
+  @Test
+  public void testParentSetToRootInWeightModeSecondaryGroupPolicy() {
+    SecondaryGroupExistingPlacementRule fsRule =
+        mock(SecondaryGroupExistingPlacementRule.class);
+    testParentSetToRootInWeightMode(fsRule);
+  }
+
+  @Test
+  public void testParentSetToRootInWeightModeSecondaryGroupUserPolicy() {
+    UserPlacementRule fsRule = mock(UserPlacementRule.class);
+    SecondaryGroupExistingPlacementRule parent =
+        mock(SecondaryGroupExistingPlacementRule.class);
+    when(fsRule.getParentRule()).thenReturn(parent);
+    testParentSetToRootInWeightMode(fsRule);
+  }
+
+  private void testParentSetToRootInWeightMode(FSPlacementRule fsRule) {
+    initPlacementManagerMock(fsRule);
+
+    MappingRulesDescription desc = convertInWeightMode();
+    Rule rule = desc.getRules().get(0);
+
+    assertEquals("Parent queue", "root", rule.getParentQueue());
+  }
+
   @Test
   public void testConvertNestedPrimaryGroupRuleWithParentCreate() {
     UserPlacementRule fsRule = mock(UserPlacementRule.class);