[ 
https://issues.apache.org/jira/browse/YARN-11511?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Tamas Domok updated YARN-11511:
-------------------------------
    Description: 
We found multiple configuration issues with 
*TestRMWebServicesCapacitySched.java* and 
*TestRMWebServicesCapacitySchedDynamicConfig.java*.

h3. 1. createMockRM created the RM with a non-intuitive queue config 
(createMockRM was used from the TestRMWebServicesCapacitySchedDynamicConfig 
where this was not expected)

Fix:
{code}
diff --git 
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesCapacitySched.java
 
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesCapacitySched.java
index ec65237fa6e..378f16e981a 100644
--- 
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesCapacitySched.java
+++ 
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesCapacitySched.java
@@ -108,13 +108,13 @@ public TestRMWebServicesCapacitySched() {
   @Override
   public void setUp() throws Exception {
     super.setUp();
-    rm = createMockRM(new CapacitySchedulerConfiguration(
-        new Configuration(false)));
+    rm = createMockRM(setupQueueConfiguration(new 
CapacitySchedulerConfiguration(
+        new Configuration(false))));
     GuiceServletConfig.setInjector(
         Guice.createInjector(new WebServletModule(rm)));
   }

-  public static void setupQueueConfiguration(
+  public CapacitySchedulerConfiguration setupQueueConfiguration(
       CapacitySchedulerConfiguration config) {

     // Define top-level queues
@@ -167,6 +167,8 @@ public static void setupQueueConfiguration(
     config.setAutoCreateChildQueueEnabled(a1C, true);
     config.setInt(PREFIX + a1C + DOT + AUTO_CREATED_LEAF_QUEUE_TEMPLATE_PREFIX
         + DOT + CAPACITY, 50);
+
+    return config;
   }

   @Test
@@ -407,7 +409,6 @@ public static WebAppDescriptor createWebAppDescriptor() {
   }

   public static MockRM createMockRM(CapacitySchedulerConfiguration csConf) {
-    setupQueueConfiguration(csConf);
     YarnConfiguration conf = new YarnConfiguration(csConf);
     conf.setClass(YarnConfiguration.RM_SCHEDULER, CapacityScheduler.class,
         ResourceScheduler.class);
{code}

h3. 2. setupQueueConfiguration creates a mixed queue hierarchy (percentage and 
absolute)

{code}
    final String c = CapacitySchedulerConfiguration.ROOT + ".c";
    config.setCapacity(c, "[memory=1024]");
{code}

root.c is configured in absolute mode while root.a and root.b are configured in 
percentage

setupQueueConfiguration should be simplified, do the configuration like the 
other tests (create a map with the string key value pairs)

h3. 3. createAbsoluteConfigLegacyAutoCreation does not set capacity for the 
default queue

That makes it mixed (percentage + absolute)

h3. 4. initAutoQueueHandler is called with wrong units

The * GB is unnecessary, and the vcores should be configured too with a value 
that makes sense.

h3. 5. CSConfigGenerator static class makes the tests hard to read.

The test cases should just have their configuration assembled in them.

h3. 6. testSchedulerResponseAbsoluteMode does not add any node

No cluster resource -> no effectiveMin/Max resource.


h1. Proposal

These tests need a rework, the configurations should be easy to follow and the 
calculated effectiveMin/Max (and any other calculated value) should result in 
reasonable numbers. The goal is to have a end to end like test suite that 
verifies the queue hierarchy initialisation.


The queue hierarchies should be simple but at least 2 level, e.g.:

root.default
root.test_1.test_1_1
root.test_1.test_1_2
root.test_2


The helper methods could be moved to a separate utility class from 
TestRMWebServicesCapacitySched.

TestRMWebServicesCapacitySched can be kept for the basic tests (json/xml slash 
at the end, apps, node-labels and resource-info).


AFAIK, the difference between the TestRMWebServicesCapacitySched and the 
TestRMWebServicesCapacitySchedDynamicConfig is that the latter is a mutable CS: 

{code}
      config.set(YarnConfiguration.SCHEDULER_CONFIGURATION_STORE_CLASS,
          YarnConfiguration.MEMORY_CONFIGURATION_STORE);
{code}


The tests in the TestRMWebServicesCapacitySchedDynamicConfig could start with 
zero resource then add some nodes and remove some nodes to verify the 
calculations.

We should have tests for Absolute, Percentage, Weight mode.

Legacy AQC and Flexible AQC (multiple level queue creation, static child queue) 
should be also tested.

  was:
We found multiple configuration issues with 
*TestRMWebServicesCapacitySched.java* and 
*TestRMWebServicesCapacitySchedDynamicConfig.java*.

# createMockRM created the RM with a non-intuitive queue config (createMockRM 
was used from the TestRMWebServicesCapacitySchedDynamicConfig where this was 
not expected)

Fix:
{code}
diff --git 
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesCapacitySched.java
 
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesCapacitySched.java
index ec65237fa6e..378f16e981a 100644
--- 
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesCapacitySched.java
+++ 
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesCapacitySched.java
@@ -108,13 +108,13 @@ public TestRMWebServicesCapacitySched() {
   @Override
   public void setUp() throws Exception {
     super.setUp();
-    rm = createMockRM(new CapacitySchedulerConfiguration(
-        new Configuration(false)));
+    rm = createMockRM(setupQueueConfiguration(new 
CapacitySchedulerConfiguration(
+        new Configuration(false))));
     GuiceServletConfig.setInjector(
         Guice.createInjector(new WebServletModule(rm)));
   }

-  public static void setupQueueConfiguration(
+  public CapacitySchedulerConfiguration setupQueueConfiguration(
       CapacitySchedulerConfiguration config) {

     // Define top-level queues
@@ -167,6 +167,8 @@ public static void setupQueueConfiguration(
     config.setAutoCreateChildQueueEnabled(a1C, true);
     config.setInt(PREFIX + a1C + DOT + AUTO_CREATED_LEAF_QUEUE_TEMPLATE_PREFIX
         + DOT + CAPACITY, 50);
+
+    return config;
   }

   @Test
@@ -407,7 +409,6 @@ public static WebAppDescriptor createWebAppDescriptor() {
   }

   public static MockRM createMockRM(CapacitySchedulerConfiguration csConf) {
-    setupQueueConfiguration(csConf);
     YarnConfiguration conf = new YarnConfiguration(csConf);
     conf.setClass(YarnConfiguration.RM_SCHEDULER, CapacityScheduler.class,
         ResourceScheduler.class);
{code}

# setupQueueConfiguration creates a mixed queue hierarchy (percentage and 
absolute)

{code}
    final String c = CapacitySchedulerConfiguration.ROOT + ".c";
    config.setCapacity(c, "[memory=1024]");
{code}

root.c is configured in absolute mode while root.a and root.b are configured in 
percentage

setupQueueConfiguration should be simplified, do the configuration like the 
other tests (create a map with the string key value pairs)

# createAbsoluteConfigLegacyAutoCreation does not set capacity for the default 
queue

That makes it mixed (percentage + absolute)

# initAutoQueueHandler is called with wrong units

The * GB is unnecessary, and the vcores should be configured too with a value 
that makes sense.

# CSConfigGenerator static class makes the tests hard to read.

The test cases should just have their configuration assembled in them.

# testSchedulerResponseAbsoluteMode does not add any node

No cluster resource -> no effectiveMin/Max resource.


h1. Proposal

These tests need a rework, the configurations should be easy to follow and the 
calculated effectiveMin/Max (and any other calculated value) should result in 
reasonable numbers.

The queue hierarchies should be simple but at least 2 level, e.g.:

root.default
root.test_1.test_1_1
root.test_1.test_1_2
root.test_2

The helper methods could be moved to a separate utility class from 
TestRMWebServicesCapacitySched.

TestRMWebServicesCapacitySched can be kept for the basic tests (json/xml slash 
at the end, apps, node-labels and resource-info).


AFAIK, the difference between the TestRMWebServicesCapacitySched and the 
TestRMWebServicesCapacitySchedDynamicConfig is that the latter is a mutable CS: 

{code}
      config.set(YarnConfiguration.SCHEDULER_CONFIGURATION_STORE_CLASS,
          YarnConfiguration.MEMORY_CONFIGURATION_STORE);
{code}

The tests in the TestRMWebServicesCapacitySchedDynamicConfig could start with 
zero resource then add some nodes and remove some nodes to verify the 
calculations.

We should have tests for Absolute, Percentage, Weight mode.

Legacy AQC and Flexible AQC should be also tested.


> Improve TestRMWebServices test config and data
> ----------------------------------------------
>
>                 Key: YARN-11511
>                 URL: https://issues.apache.org/jira/browse/YARN-11511
>             Project: Hadoop YARN
>          Issue Type: Improvement
>          Components: capacityscheduler
>    Affects Versions: 3.4.0
>            Reporter: Tamas Domok
>            Assignee: Bence Kosztolnik
>            Priority: Major
>
> We found multiple configuration issues with 
> *TestRMWebServicesCapacitySched.java* and 
> *TestRMWebServicesCapacitySchedDynamicConfig.java*.
> h3. 1. createMockRM created the RM with a non-intuitive queue config 
> (createMockRM was used from the TestRMWebServicesCapacitySchedDynamicConfig 
> where this was not expected)
> Fix:
> {code}
> diff --git 
> a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesCapacitySched.java
>  
> b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesCapacitySched.java
> index ec65237fa6e..378f16e981a 100644
> --- 
> a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesCapacitySched.java
> +++ 
> b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesCapacitySched.java
> @@ -108,13 +108,13 @@ public TestRMWebServicesCapacitySched() {
>    @Override
>    public void setUp() throws Exception {
>      super.setUp();
> -    rm = createMockRM(new CapacitySchedulerConfiguration(
> -        new Configuration(false)));
> +    rm = createMockRM(setupQueueConfiguration(new 
> CapacitySchedulerConfiguration(
> +        new Configuration(false))));
>      GuiceServletConfig.setInjector(
>          Guice.createInjector(new WebServletModule(rm)));
>    }
> -  public static void setupQueueConfiguration(
> +  public CapacitySchedulerConfiguration setupQueueConfiguration(
>        CapacitySchedulerConfiguration config) {
>      // Define top-level queues
> @@ -167,6 +167,8 @@ public static void setupQueueConfiguration(
>      config.setAutoCreateChildQueueEnabled(a1C, true);
>      config.setInt(PREFIX + a1C + DOT + 
> AUTO_CREATED_LEAF_QUEUE_TEMPLATE_PREFIX
>          + DOT + CAPACITY, 50);
> +
> +    return config;
>    }
>    @Test
> @@ -407,7 +409,6 @@ public static WebAppDescriptor createWebAppDescriptor() {
>    }
>    public static MockRM createMockRM(CapacitySchedulerConfiguration csConf) {
> -    setupQueueConfiguration(csConf);
>      YarnConfiguration conf = new YarnConfiguration(csConf);
>      conf.setClass(YarnConfiguration.RM_SCHEDULER, CapacityScheduler.class,
>          ResourceScheduler.class);
> {code}
> h3. 2. setupQueueConfiguration creates a mixed queue hierarchy (percentage 
> and absolute)
> {code}
>     final String c = CapacitySchedulerConfiguration.ROOT + ".c";
>     config.setCapacity(c, "[memory=1024]");
> {code}
> root.c is configured in absolute mode while root.a and root.b are configured 
> in percentage
> setupQueueConfiguration should be simplified, do the configuration like the 
> other tests (create a map with the string key value pairs)
> h3. 3. createAbsoluteConfigLegacyAutoCreation does not set capacity for the 
> default queue
> That makes it mixed (percentage + absolute)
> h3. 4. initAutoQueueHandler is called with wrong units
> The * GB is unnecessary, and the vcores should be configured too with a value 
> that makes sense.
> h3. 5. CSConfigGenerator static class makes the tests hard to read.
> The test cases should just have their configuration assembled in them.
> h3. 6. testSchedulerResponseAbsoluteMode does not add any node
> No cluster resource -> no effectiveMin/Max resource.
> h1. Proposal
> These tests need a rework, the configurations should be easy to follow and 
> the calculated effectiveMin/Max (and any other calculated value) should 
> result in reasonable numbers. The goal is to have a end to end like test 
> suite that verifies the queue hierarchy initialisation.
> The queue hierarchies should be simple but at least 2 level, e.g.:
> root.default
> root.test_1.test_1_1
> root.test_1.test_1_2
> root.test_2
> The helper methods could be moved to a separate utility class from 
> TestRMWebServicesCapacitySched.
> TestRMWebServicesCapacitySched can be kept for the basic tests (json/xml 
> slash at the end, apps, node-labels and resource-info).
> AFAIK, the difference between the TestRMWebServicesCapacitySched and the 
> TestRMWebServicesCapacitySchedDynamicConfig is that the latter is a mutable 
> CS: 
> {code}
>       config.set(YarnConfiguration.SCHEDULER_CONFIGURATION_STORE_CLASS,
>           YarnConfiguration.MEMORY_CONFIGURATION_STORE);
> {code}
> The tests in the TestRMWebServicesCapacitySchedDynamicConfig could start with 
> zero resource then add some nodes and remove some nodes to verify the 
> calculations.
> We should have tests for Absolute, Percentage, Weight mode.
> Legacy AQC and Flexible AQC (multiple level queue creation, static child 
> queue) should be also tested.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to