brumi1024 commented on code in PR #7278:
URL: https://github.com/apache/hadoop/pull/7278#discussion_r1918771186
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/policy/TestPriorityUtilizationQueueOrderingPolicy.java:
##########
@@ -331,6 +386,7 @@ private QueueResourceQuotas randomResourceQuotas(String
partition) {
QueueResourceQuotas qr = new QueueResourceQuotas();
qr.setConfiguredMinResource(partition,
Resource.newInstance(randInt(1, 10) * 1024, randInt(1, 10)));
+ // System.out.println("Quotas: " + qr.getConfiguredMinResource(partition));
Review Comment:
Is this needed?
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/policy/TestPriorityUtilizationQueueOrderingPolicy.java:
##########
@@ -309,6 +311,70 @@ public void testComparatorDoesNotValidateGeneralContract()
{
assertDoesNotThrow(() -> policy.getAssignmentIterator(partition));
}
+ @Test
+ public void testComparatorClassDoesNotViolateTimSortContract() {
+ String partition = "testPartition";
+
+ List<PriorityUtilizationQueueOrderingPolicy.
+ PriorityQueueResourcesForSorting> queues = new ArrayList<>();
+ for (int i = 0; i < 300; i++) {
+ queues.add(
+ createMockPriorityQueueResourcesForSorting(
+ partition, Resource.newInstance(0, 0) // Need to be (0,
0)
+ )
+ );
+ queues.add(
+ createMockPriorityQueueResourcesForSorting(
+ partition, Resource.newInstance(8, 20) // Could be any
number
+ )
+ );
+ queues.add(
+ createMockPriorityQueueResourcesForSorting(
+ partition, Resource.newInstance(10, 5) // Could be any
number
+ )
+ );
+ }
+
+ Collections.shuffle(queues);
+ // java.lang.IllegalArgumentException: Comparison method violates its
general contract!
+ assertDoesNotThrow(() -> queues.sort(new
PriorityUtilizationQueueOrderingPolicy(true)
+ .new PriorityQueueComparator(partition)));
+
+ }
+
+ private PriorityUtilizationQueueOrderingPolicy.
+ PriorityQueueResourcesForSorting
createMockPriorityQueueResourcesForSorting(
+ String partition, Resource resource
+ ) {
+
+ QueueCapacities mockQueueCapacities = mock(QueueCapacities.class);
+ when(mockQueueCapacities.getAbsoluteUsedCapacity(partition))
+ .thenReturn(4.2f); // Could be any number
+ when(mockQueueCapacities.getUsedCapacity(partition))
+ .thenReturn(1.0f); // Could be any number
+ when(mockQueueCapacities.getAbsoluteCapacity(partition))
+ .thenReturn(6.2f); // Could be any number
+
Review Comment:
I see, thanks. In that case I think the "// Could be any number" comment is
a bit misleading, maybe we could specify that it could be any specific number,
so that there are equal values.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/policy/TestPriorityUtilizationQueueOrderingPolicy.java:
##########
@@ -309,6 +311,59 @@ public void testComparatorDoesNotValidateGeneralContract()
{
assertDoesNotThrow(() -> policy.getAssignmentIterator(partition));
}
+ @Test
+ public void testComparatorClassDoesNotViolateTimSortContract() {
+ String partition = "testPartition";
+
+ List<PriorityUtilizationQueueOrderingPolicy.
+ PriorityQueueResourcesForSorting> queues = new ArrayList<>();
+ for (int i = 0; i < 300; i++) {
+ queues.add(createMockPriorityQueueResourcesForSorting(partition));
+ queues.add(createMockPriorityQueueResourcesForSorting(partition));
+ queues.add(createMockPriorityQueueResourcesForSorting(partition));
Review Comment:
Since there are no values supplied in the params the 3 calls seems
unnecessary, you could simply increate the limit from 300 to 900.
--
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]
