brumi1024 commented on a change in pull request #3361:
URL: https://github.com/apache/hadoop/pull/3361#discussion_r700119052
##########
File path:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/AbstractManagedParentQueue.java
##########
@@ -173,50 +168,31 @@ public AutoCreatedLeafQueueConfig getLeafQueueTemplate() {
return queueManagementPolicy;
}
- protected SortedMap<String, String> getConfigurationsWithPrefix
- (SortedMap<String, String> sortedConfigs, String prefix) {
- return sortedConfigs.subMap( prefix, prefix + Character.MAX_VALUE );
- }
-
- protected SortedMap<String, String> sortCSConfigurations() {
- SortedMap<String, String> sortedConfigs = new TreeMap(
- new Comparator<String>() {
- public int compare(String s1, String s2) {
- return s1.compareToIgnoreCase(s2);
- }
+ protected Map<String, String> getCSConfigurationsWithPrefix(String prefix) {
+ Map<String, String> configsWithPrefix = new HashMap<>();
- });
-
- for (final Iterator<Map.Entry<String, String>> iterator =
- csContext.getConfiguration().iterator(); iterator.hasNext(); ) {
- final Map.Entry<String, String> confKeyValuePair = iterator.next();
- sortedConfigs.put(confKeyValuePair.getKey(),
confKeyValuePair.getValue());
+ for (Map.Entry<String, String> confKeyValuePair :
+ csContext.getConfiguration().getPropsWithPrefix(prefix).entrySet()) {
+ configsWithPrefix.put(prefix + confKeyValuePair.getKey(),
confKeyValuePair.getValue());
}
- return sortedConfigs;
+
+ return configsWithPrefix;
}
- protected CapacitySchedulerConfiguration initializeLeafQueueConfigs(String
- configPrefix) {
+ protected CapacitySchedulerConfiguration initializeLeafQueueConfigs(String
configPrefix) {
CapacitySchedulerConfiguration leafQueueConfigs = new
- CapacitySchedulerConfiguration(new Configuration(false), false);
+ CapacitySchedulerConfiguration(csContext.getConf(), false);
Review comment:
This isn't entirely what we had in mind. Now every time a new dynamic
queue is created every single property of capacity-scheduler.xml and
yarn-site.xml is loaded into the newly created leafQueueConfigs, which could
hinder the performance. The jira was more about getting rid of the config
object duplication in the first place.
##########
File path:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/AbstractManagedParentQueue.java
##########
@@ -173,50 +168,31 @@ public AutoCreatedLeafQueueConfig getLeafQueueTemplate() {
return queueManagementPolicy;
}
- protected SortedMap<String, String> getConfigurationsWithPrefix
- (SortedMap<String, String> sortedConfigs, String prefix) {
- return sortedConfigs.subMap( prefix, prefix + Character.MAX_VALUE );
- }
-
- protected SortedMap<String, String> sortCSConfigurations() {
- SortedMap<String, String> sortedConfigs = new TreeMap(
- new Comparator<String>() {
- public int compare(String s1, String s2) {
- return s1.compareToIgnoreCase(s2);
- }
+ protected Map<String, String> getCSConfigurationsWithPrefix(String prefix) {
+ Map<String, String> configsWithPrefix = new HashMap<>();
- });
-
- for (final Iterator<Map.Entry<String, String>> iterator =
- csContext.getConfiguration().iterator(); iterator.hasNext(); ) {
- final Map.Entry<String, String> confKeyValuePair = iterator.next();
- sortedConfigs.put(confKeyValuePair.getKey(),
confKeyValuePair.getValue());
+ for (Map.Entry<String, String> confKeyValuePair :
+ csContext.getConfiguration().getPropsWithPrefix(prefix).entrySet()) {
+ configsWithPrefix.put(prefix + confKeyValuePair.getKey(),
confKeyValuePair.getValue());
Review comment:
Instead of using the slower getPropsWithPrefix the optimised version
could be used which was introduced in YARN-10838 and will be integrated in
YARN-10872.
##########
File path:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/AbstractManagedParentQueue.java
##########
@@ -173,50 +168,31 @@ public AutoCreatedLeafQueueConfig getLeafQueueTemplate() {
return queueManagementPolicy;
}
- protected SortedMap<String, String> getConfigurationsWithPrefix
- (SortedMap<String, String> sortedConfigs, String prefix) {
- return sortedConfigs.subMap( prefix, prefix + Character.MAX_VALUE );
- }
-
- protected SortedMap<String, String> sortCSConfigurations() {
- SortedMap<String, String> sortedConfigs = new TreeMap(
- new Comparator<String>() {
- public int compare(String s1, String s2) {
- return s1.compareToIgnoreCase(s2);
- }
+ protected Map<String, String> getCSConfigurationsWithPrefix(String prefix) {
+ Map<String, String> configsWithPrefix = new HashMap<>();
- });
-
- for (final Iterator<Map.Entry<String, String>> iterator =
- csContext.getConfiguration().iterator(); iterator.hasNext(); ) {
- final Map.Entry<String, String> confKeyValuePair = iterator.next();
- sortedConfigs.put(confKeyValuePair.getKey(),
confKeyValuePair.getValue());
+ for (Map.Entry<String, String> confKeyValuePair :
+ csContext.getConfiguration().getPropsWithPrefix(prefix).entrySet()) {
+ configsWithPrefix.put(prefix + confKeyValuePair.getKey(),
confKeyValuePair.getValue());
}
- return sortedConfigs;
+
+ return configsWithPrefix;
}
- protected CapacitySchedulerConfiguration initializeLeafQueueConfigs(String
- configPrefix) {
+ protected CapacitySchedulerConfiguration initializeLeafQueueConfigs(String
configPrefix) {
CapacitySchedulerConfiguration leafQueueConfigs = new
- CapacitySchedulerConfiguration(new Configuration(false), false);
+ CapacitySchedulerConfiguration(csContext.getConf(), false);
- String prefix = YarnConfiguration.RESOURCE_TYPES + ".";
- Map<String, String> rtProps = csContext
- .getConfiguration().getPropsWithPrefix(prefix);
- for (Map.Entry<String, String> entry : rtProps.entrySet()) {
- leafQueueConfigs.set(prefix + entry.getKey(), entry.getValue());
+ String resourceTypePrefix = YarnConfiguration.RESOURCE_TYPES + ".";
+ Map<String, String> rtConfigs =
getCSConfigurationsWithPrefix(resourceTypePrefix);
+ for (Map.Entry<String, String> confKeyValuePair: rtConfigs.entrySet()) {
+ leafQueueConfigs.set(confKeyValuePair.getKey(),
confKeyValuePair.getValue());
Review comment:
I don't think this is necessary anymore, if the whole config is
duplicated. This is a property that comes from yarn-site.xml and it is
necessary for the dynamic queues as well. If however the
CapacitySchedulerConfiguration object is copied into the new one there is no
need for copying these properties separately.
--
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]
