[
https://issues.apache.org/jira/browse/YARN-11584?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17776735#comment-17776735
]
ASF GitHub Bot commented on YARN-11584:
---------------------------------------
brumi1024 commented on code in PR #6148:
URL: https://github.com/apache/hadoop/pull/6148#discussion_r1364065198
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/AbstractParentQueue.java:
##########
@@ -490,6 +490,12 @@ public CSQueue createNewQueue(String childQueuePath,
boolean isLeaf)
String queueShortName = childQueuePath.substring(
childQueuePath.lastIndexOf(".") + 1);
+ if (StringUtils.isEmpty(queueShortName)) {
+ throw new SchedulerDynamicEditException(
+ "Trying to create new queue=" + childQueuePath
+ + ", which has empty leaf shortname.");
+ }
Review Comment:
Not sure about this. Going up the callchain:
1. `AbstractParentQueue.createNewQueue` is only called from
`ParentQueue.addDynamicChildQueue`,
2. which (through another method) is called from
`CapacitySchedulerQueueManager.createAutoQueue`,
3. which is called from `CapacitySchedulerQueueManager.createQueue`, where
my second comment should apply
There is one difference: upon determining whether we're using legacy auto
creation or the flexible one we're using different methods, and the flexible
auto queue creation (method: `CapacitySchedulerQueueManager.createQueue
.createAutoQueue(queue)`) manipulates the QueuePath using `List<String>
parentsToCreate = determineMissingParents(queue)`. If an empty string part can
be added to the list, or can slip through I think it's probably there, which I
think should be a better place for these checks.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerQueueManager.java:
##########
@@ -509,6 +509,12 @@ public AbstractLeafQueue createQueue(QueuePath queue)
String leafQueueName = queue.getLeafName();
String parentQueueName = queue.getParent();
+ if (StringUtils.isEmpty(leafQueueName)) {
+ throw new SchedulerDynamicEditException(
+ "Trying to create new queue=" + queue.getFullPath()
+ + ", which has empty leaf shortname.");
+ }
+
Review Comment:
I don't think this part is necessary. QueueManager.createQueue(QueuePath) is
only called from CapacityScheduler.getOrCreateQueueFromPlacementContext (at
least from production code), and it has the following check right before this
call:
```
//we need to make sure there are no empty path parts present
if (queuePath.hasEmptyPart()) {
LOG.error("Application submitted to invalid path due to empty parts: " +
"'{}'", queuePath);
return null;
}
```
So essentially we should not be able to reach this from production code,
only from tests.
> [CS] Attempting to create Leaf Queue with empty shortname should fail without
> crashing RM
> -----------------------------------------------------------------------------------------
>
> Key: YARN-11584
> URL: https://issues.apache.org/jira/browse/YARN-11584
> Project: Hadoop YARN
> Issue Type: Bug
> Components: capacity scheduler
> Reporter: Brian Goerlitz
> Assignee: Brian Goerlitz
> Priority: Major
> Labels: pull-request-available
>
> If an app submission results in attempting to auto-create a leaf queue with
> an empty short name, the app submission should be rejected without the RM
> crashing. Currently, the queue will be created, but the RM encounters a FATAL
> exception due to metrics collision.
> For example, if an app is placed to 'root.' the RM will fail with the below.
> {noformat}
> 2023-09-12 20:23:43,294 FATAL org.apache.hadoop.yarn.event.EventDispatcher:
> Error in handling event type APP_ADDED to the Event Dispatcher
> org.apache.hadoop.metrics2.MetricsException: Metrics source
> QueueMetrics,q0=root already exists!
> at
> org.apache.hadoop.metrics2.lib.DefaultMetricsSystem.newSourceName(DefaultMetricsSystem.java:152)
> at
> org.apache.hadoop.metrics2.lib.DefaultMetricsSystem.sourceName(DefaultMetricsSystem.java:125)
> at
> org.apache.hadoop.metrics2.impl.MetricsSystemImpl.register(MetricsSystemImpl.java:229)
> at
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CSQueueMetrics.forQueue(CSQueueMetrics.java:309)
> at
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.AbstractCSQueue.<init>(AbstractCSQueue.java:147)
> at
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.AbstractLeafQueue.<init>(AbstractLeafQueue.java:148)
> at
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.LeafQueue.<init>(LeafQueue.java:42)
> at
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.ParentQueue.createNewQueue(ParentQueue.java:495)
> at
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.ParentQueue.addDynamicChildQueue(ParentQueue.java:563)
> at
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.ParentQueue.addDynamicLeafQueue(ParentQueue.java:517)
> at
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerQueueManager.createAutoQueue(CapacitySchedulerQueueManager.java:678)
> at
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerQueueManager.createQueue(CapacitySchedulerQueueManager.java:511)
> at
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacityScheduler.getOrCreateQueueFromPlacementContext(CapacityScheduler.java:898)
> at
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacityScheduler.addApplication(CapacityScheduler.java:962)
> at
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacityScheduler.handle(CapacityScheduler.java:1920)
> at
> org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacityScheduler.handle(CapacityScheduler.java:170)
> at
> org.apache.hadoop.yarn.event.EventDispatcher$EventProcessor.run(EventDispatcher.java:66)
> at java.base/java.lang.Thread.run(Thread.java:834)
> {noformat}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
