[ https://issues.apache.org/jira/browse/YARN-6492?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16988370#comment-16988370 ]
Jonathan Hung edited comment on YARN-6492 at 12/5/19 1:36 AM: -------------------------------------------------------------- A couple more high level comments: * In places like {noformat} public void allocateResources(String partition, String user, int containers, Resource res, boolean decrPending) {{noformat} should we only call _allocateResources if partition is null or empty? Otherwise metrics for default partition will be updated when this is called for non-null partition. Same comment for other places like reserveResource, incrPendingResources, decrPendingResources, etc * Related to above, in getPartitionQueueMetrics, can we just return null QueueMetrics object if partition is null or empty? With the change described above, the outer functions which call getPartitionQueueMetrics (e.g. allocateResources) should already update default partition's metrics. Then getPartitionQueueMetrics only returns a non-null PartitionQueueMetrics object if partition is non-null, so we don't have to maintain a duplicate PartitionQueueMetrics object for default partition. * I see currently PartitionQueueMetrics#getPartitionQueueMetrics keys by partition, while QueueMetrics#getPartitionQueueMetrics keys by partition + queue. Can we just remove the logic in PartitionQueueMetrics#getPartitionQueueMetrics? I don't think we need to maintain a separate QueueMetrics object for the entire partition. * In QueueMetrics#getPartitionQueueMetrics, can we add the partition + queueName key to a separate map, instead of adding it to QUEUE_METRICS? Like a PARTITION_QUEUE_METRICS cache. We can do something like a nested map, with partition -> queue -> QueueMetrics object. I feel it's weird to add both queue metrics, and partition queue metrics to the same map. Also it avoids the metricName = partition + this.queueName concatenation logic, which seems not very intuitive. * was (Author: jhung): A couple more high level comments: * In places like {noformat} public void allocateResources(String partition, String user, int containers, Resource res, boolean decrPending) { {noformat} > Generate queue metrics for each partition > ----------------------------------------- > > Key: YARN-6492 > URL: https://issues.apache.org/jira/browse/YARN-6492 > Project: Hadoop YARN > Issue Type: Improvement > Components: capacity scheduler > Reporter: Jonathan Hung > Assignee: Manikandan R > Priority: Major > Attachments: PartitionQueueMetrics_default_partition.txt, > PartitionQueueMetrics_x_partition.txt, PartitionQueueMetrics_y_partition.txt, > YARN-6492.001.patch, YARN-6492.002.patch, YARN-6492.003.patch, > YARN-6492.004.patch, YARN-6492.005.WIP.patch, YARN-6492.006.WIP.patch, > YARN-6492.007.WIP.patch, partition_metrics.txt > > > We are interested in having queue metrics for all partitions. Right now each > queue has one QueueMetrics object which captures metrics either in default > partition or across all partitions. (After YARN-6467 it will be in default > partition) > But having the partition metrics would be very useful. -- This message was sent by Atlassian Jira (v8.3.4#803005) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org