tomicooler commented on code in PR #6455:
URL: https://github.com/apache/hadoop/pull/6455#discussion_r1452514103
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/policy/PriorityUtilizationQueueOrderingPolicy.java:
##########
@@ -256,7 +257,7 @@ public Iterator<CSQueue> getAssignmentIterator(String
partition) {
// Sort the snapshot of the queues in order to avoid breaking the
prerequisites of TimSort.
// See YARN-10178 for details.
- return queues.stream().map(PriorityQueueResourcesForSorting::new).sorted(
+ return
Lists.newArrayList(queues.listIterator()).stream().map(PriorityQueueResourcesForSorting::new).sorted(
Review Comment:
Prior YARN-10178 we had a comment about thread safety:
```
// Since partitionToLookAt is a thread local variable, and every time we
// copy and sort queues, so it's safe for multi-threading environment.
<----
PriorityUtilizationQueueOrderingPolicy.partitionToLookAt.set(partition);
List<CSQueue> sortedQueue = new ArrayList<>(queues);
Collections.sort(sortedQueue, new PriorityQueueComparator());
return sortedQueue.iterator();
```
I think we should extend the comment at line 258-259:
```
// Copy (for thread safety) and sort the snapshot of the queues in order to
avoid breaking
// the prerequisites of TimSort. See YARN-10178 for details.
```
NIT, we should simply just use:
```
new ArrayList<>(queues).stream().map(....
```
(The Lists.NewArrayList also have a argument for a collection instead of
iterator, which should be a little bit faster, based on the code.)
--
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]
