brumi1024 commented on code in PR #7278:
URL: https://github.com/apache/hadoop/pull/7278#discussion_r1913320001
##########
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:
Nit: why not use the randomQueueCapacities method below? Would make the code
a bit cleaner.
##########
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
+
+ CSQueue mockQueue = mock(CSQueue.class);
+ when(mockQueue.getQueueCapacities())
+ .thenReturn(mockQueueCapacities);
+ when(mockQueue.getPriority())
+ .thenReturn(Priority.newInstance(7)); // Could be any number
+ when(mockQueue.getAccessibleNodeLabels())
+ .thenReturn(Collections.singleton("label3")); // Could be any label
+
+ QueueResourceQuotas mockResourceQuotas = mock(QueueResourceQuotas.class);
+ when(mockResourceQuotas.getConfiguredMinResource(partition))
+ .thenReturn(resource);
+ when(mockQueue.getQueueResourceQuotas())
+ .thenReturn(mockResourceQuotas);
Review Comment:
Same here, randomResourceQuotas seems to be similar functionality. AFAICS
the (0,0) case is the reason this was needed, that probably could be solved by
creating a "zeroResourceQuotas" or something similar which ensures a few queues
with zero minResource are present.
##########
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
+ ) {
Review Comment:
Formatting nit: No need to put the closing braces in a new line.
--
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]
