brumi1024 commented on code in PR #5201:
URL: https://github.com/apache/hadoop/pull/5201#discussion_r1055653806
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/FSConfigToCSConfigConverter.java:
##########
@@ -412,6 +418,20 @@ private void emitDefaultMaxAMShare() {
queueMaxAMShareDefault);
}
}
+
+ private void emitDefaultUserLimitFactor(Collection<FSLeafQueue> fsLeafQueue)
{
+ fsLeafQueue
+ .forEach((leafQueue) -> {
+ if (!capacitySchedulerConfig.
+ isAutoQueueCreationV2Enabled(leafQueue.getName())) {
+ capacitySchedulerConfig.setFloat(
+ CapacitySchedulerConfiguration.
+ PREFIX + leafQueue.getName() + DOT +
USER_LIMIT_FACTOR,
+ -1.0f);
+ }
+ });
+ }
+
Review Comment:
On the second look this class is more like for general config changes, while
there is a class called FSQueueConverter, which has the queue specific
converter logic implemented. Can you please move this whole logic there?
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSConfigToCSConfigConverter.java:
##########
@@ -182,7 +183,24 @@ public void testDefaultMaxAMShare() throws Exception {
conf.get(PREFIX + "root.admins.alice.maximum-am-resource-percent"));
assertNull("root.users.joe maximum-am-resource-percent should be null",
- conf.get(PREFIX + "root.users.joe maximum-am-resource-percent"));
+ conf.get(PREFIX + "root.users.joe.maximum-am-resource-percent"));
+ }
+
+ @Test
+ public void testDefaultUserLimitFactor() throws Exception {
+ converter.convert(config);
+
+ Configuration conf = converter.getCapacitySchedulerConfig();
+ String userLimitFactor =
+ conf.get(PREFIX + "root.default." + USER_LIMIT_FACTOR);
+
+ assertEquals("Default user limit factor", "-1.0", userLimitFactor);
+
+ assertEquals("root.users.joe user-limit-factor", "-1.0",
+ conf.get(PREFIX + "root.users.joe.user-limit-factor"));
+
+ assertEquals("root.admins.bob user-limit-factor", "-1.0",
+ conf.get(PREFIX + "root.admins.bob.user-limit-factor"));
Review Comment:
Can you please add an assertion where a parent class doesn't get the
user-limit-factor?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
