brumi1024 commented on code in PR #5320:
URL: https://github.com/apache/hadoop/pull/5320#discussion_r1083831502
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSQueueConverter.java:
##########
@@ -150,19 +152,19 @@ public void testConvertQueueHierarchy() {
// root children
assertEquals("root children", "admins,users,misc,default",
- csConfig.get(PREFIX + "root.queues"));
+ capacitySchedulerConfig.get(PREFIX + "root.queues"));
Review Comment:
There is a helper method called getQueues, which returns the list of queues
that could be used here.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSQueueConverter.java:
##########
@@ -150,19 +152,19 @@ public void testConvertQueueHierarchy() {
// root children
assertEquals("root children", "admins,users,misc,default",
- csConfig.get(PREFIX + "root.queues"));
+ capacitySchedulerConfig.get(PREFIX + "root.queues"));
// root.admins children
assertEquals("root.admins children", "bob,alice",
- csConfig.get(PREFIX + "root.admins.queues"));
+ capacitySchedulerConfig.get(PREFIX + "root.admins.queues"));
Review Comment:
Same as above.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSQueueConverter.java:
##########
@@ -182,17 +184,18 @@ public void testQueueMaxAMShare() {
// root.admins.bob
assertEquals("root.admins.bob AM share", "1.0",
- csConfig.get(PREFIX + "root.admins.bob.maximum-am-resource-percent"));
+ capacitySchedulerConfig.get(
Review Comment:
getMaximumAMResourcePercentPerPartition checks this correctly if queried for
No Label label.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSQueueConverter.java:
##########
@@ -182,17 +184,18 @@ public void testQueueMaxAMShare() {
// root.admins.bob
assertEquals("root.admins.bob AM share", "1.0",
- csConfig.get(PREFIX + "root.admins.bob.maximum-am-resource-percent"));
+ capacitySchedulerConfig.get(
+ PREFIX + "root.admins.bob.maximum-am-resource-percent"));
// root.admins.alice
assertEquals("root.admins.alice AM share", "0.15",
- csConfig.get(PREFIX +
- "root.admins.alice.maximum-am-resource-percent"));
+ capacitySchedulerConfig.get(
Review Comment:
Same as the previous comment.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSQueueConverter.java:
##########
@@ -218,21 +222,26 @@ public void testQueueMaxAllocations() {
// root.admins vcores + mb
assertEquals("root.admins max vcores", 3,
- csConfig.getInt(PREFIX + "root.admins.maximum-allocation-vcores", -1));
+ capacitySchedulerConfig.getInt(
+ PREFIX + "root.admins.maximum-allocation-vcores", -1));
assertEquals("root.admins max memory", 4096,
- csConfig.getInt(PREFIX + "root.admins.maximum-allocation-mb", -1));
+ capacitySchedulerConfig.getInt(
Review Comment:
getQueueMaximumAllocationMb is a helper method for this.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSQueueConverter.java:
##########
@@ -242,15 +251,16 @@ public void testQueuePreemptionDisabled() {
converter.convertQueueHierarchy(rootQueue);
assertTrue("root.admins.alice preemption setting",
- csConfig.getBoolean(PREFIX + "root.admins.alice.disable_preemption",
- false));
+ capacitySchedulerConfig.getBoolean(
+ PREFIX + "root.admins.alice.disable_preemption", false));
assertTrue("root.users.joe preemption setting",
- csConfig.getBoolean(PREFIX + "root.users.joe.disable_preemption",
- false));
+ capacitySchedulerConfig.getBoolean(
Review Comment:
Same as above.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSQueueConverter.java:
##########
@@ -202,12 +205,13 @@ public void testQueueMaxParallelApps() {
converter.convertQueueHierarchy(rootQueue);
assertEquals("root.admins.alice max apps", 2,
- csConfig.getInt(PREFIX + "root.admins.alice.max-parallel-apps",
- -1));
+ capacitySchedulerConfig.getInt(
Review Comment:
Please use getMaxParallelAppsForQueue.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSQueueConverter.java:
##########
@@ -218,21 +222,26 @@ public void testQueueMaxAllocations() {
// root.admins vcores + mb
assertEquals("root.admins max vcores", 3,
- csConfig.getInt(PREFIX + "root.admins.maximum-allocation-vcores", -1));
+ capacitySchedulerConfig.getInt(
Review Comment:
getQueueMaximumAllocationVcores returns this.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSQueueConverter.java:
##########
@@ -150,19 +152,19 @@ public void testConvertQueueHierarchy() {
// root children
assertEquals("root children", "admins,users,misc,default",
- csConfig.get(PREFIX + "root.queues"));
+ capacitySchedulerConfig.get(PREFIX + "root.queues"));
// root.admins children
assertEquals("root.admins children", "bob,alice",
- csConfig.get(PREFIX + "root.admins.queues"));
+ capacitySchedulerConfig.get(PREFIX + "root.admins.queues"));
// root.default children - none
- assertNull("root.default children", csConfig.get(PREFIX + "root.default" +
- ".queues"));
+ assertNull("root.default children",
+ capacitySchedulerConfig.get(PREFIX + "root.default" + ".queues"));
Review Comment:
Same as above.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSQueueConverter.java:
##########
@@ -150,19 +152,19 @@ public void testConvertQueueHierarchy() {
// root children
assertEquals("root children", "admins,users,misc,default",
- csConfig.get(PREFIX + "root.queues"));
+ capacitySchedulerConfig.get(PREFIX + "root.queues"));
// root.admins children
assertEquals("root.admins children", "bob,alice",
- csConfig.get(PREFIX + "root.admins.queues"));
+ capacitySchedulerConfig.get(PREFIX + "root.admins.queues"));
// root.default children - none
- assertNull("root.default children", csConfig.get(PREFIX + "root.default" +
- ".queues"));
+ assertNull("root.default children",
+ capacitySchedulerConfig.get(PREFIX + "root.default" + ".queues"));
// root.users children
assertEquals("root.users children", "john,joe",
- csConfig.get(PREFIX + "root.users.queues"));
+ capacitySchedulerConfig.get(PREFIX + "root.users.queues"));
Review Comment:
Same as above.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSQueueConverter.java:
##########
@@ -218,21 +222,26 @@ public void testQueueMaxAllocations() {
// root.admins vcores + mb
assertEquals("root.admins max vcores", 3,
- csConfig.getInt(PREFIX + "root.admins.maximum-allocation-vcores", -1));
+ capacitySchedulerConfig.getInt(
+ PREFIX + "root.admins.maximum-allocation-vcores", -1));
assertEquals("root.admins max memory", 4096,
- csConfig.getInt(PREFIX + "root.admins.maximum-allocation-mb", -1));
+ capacitySchedulerConfig.getInt(
+ PREFIX + "root.admins.maximum-allocation-mb", -1));
// root.users.john max vcores + mb
assertEquals("root.users.john max vcores", 2,
- csConfig.getInt(PREFIX + "root.users.john.maximum-allocation-vcores",
- -1));
+ capacitySchedulerConfig.getInt(
Review Comment:
Same as the comment before the last one.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSQueueConverter.java:
##########
@@ -242,15 +251,16 @@ public void testQueuePreemptionDisabled() {
converter.convertQueueHierarchy(rootQueue);
assertTrue("root.admins.alice preemption setting",
- csConfig.getBoolean(PREFIX + "root.admins.alice.disable_preemption",
- false));
+ capacitySchedulerConfig.getBoolean(
Review Comment:
getPreemptionDisabled helper method checks this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
