[
https://issues.apache.org/jira/browse/YARN-11745?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
chhinlinghean updated YARN-11745:
---------------------------------
Description:
The TimSort Transitivity rules got broken down when comparing both queues with
resources (0, 0), another queue with resources(some number, some number) and
with the same queues absolute capacity.
*Steps reproduce with Unit test:*
/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
{code:java}
@Test
public void testPriorityQueueComparatorClassDoesNotViolateTimSortContract() {
String partition = "testPartition";
List<PriorityUtilizationQueueOrderingPolicy.PriorityQueueResourcesForSorting>
queues = new ArrayList<>();
for (int i = 0; i < 300; i++) { // Have to be from 300 to make the test
deterministic
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(8, 8)) // Could be any number
);
}
Collections.shuffle(queues);
// java.lang.IllegalArgumentException: Comparison method violates its general
contract!
assertDoesNotThrow(() -> Collections.sort(queues, new
PriorityUtilizationQueueOrderingPolicy(true)
.new PriorityQueueComparator(partition)));
}
private PriorityUtilizationQueueOrderingPolicy.PriorityQueueResourcesForSorting
createMockPriorityQueueResourcesForSorting(String partition, Resource
resource)
{
QueueCapacities mockQueueCapacities = mock(QueueCapacities.class);
when(mockQueueCapacities.getAbsoluteUsedCapacity(partition)).thenReturn(3.2f);
// Could be any number
when(mockQueueCapacities.getUsedCapacity(partition)).thenReturn(1.0f); //
Could be any number
when(mockQueueCapacities.getAbsoluteCapacity(partition)).thenReturn(4.2f); //
Could be any number
CSQueue mockQueue = mock(CSQueue.class);
when(mockQueue.getQueueCapacities()).thenReturn(mockQueueCapacities);
when(mockQueue.getPriority()).thenReturn(Priority.newInstance(5)); // Could
be any number
when(mockQueue.getAccessibleNodeLabels()).thenReturn(Collections.singleton("label1"));
// simulated label
QueueResourceQuotas randomQuotas = mock(QueueResourceQuotas.class);
when(randomQuotas.getConfiguredMinResource(partition)).thenReturn(resource);
when(mockQueue.getQueueResourceQuotas()).thenReturn(randomQuotas);
return new
PriorityUtilizationQueueOrderingPolicy.PriorityQueueResourcesForSorting(
mockQueue, partition
);{code}
*How to fix it?*
Instead of checking with an AND condition when both queues resources are not
none to compare its resources, we should check with an OR condition instead.
Because in the case one queue's resource is none another one is not we should
still compare by its resources.
Previous code:
{code:java}
if (!minEffRes1.equals(Resources.none()) &&
!minEffRes2.equals(Resources.none())) {
return minEffRes2.compareTo(minEffRes1);
}
float abs1 = q1Sort.absoluteCapacity;
float abs2 = q2Sort.absoluteCapacity;
return Float.compare(abs2, abs1); {code}
Changed code to:
{code:java}
if (!minEffRes1.equals(Resources.none()) ||
!minEffRes2.equals(Resources.none())) {
return minEffRes2.compareTo(minEffRes1);
}
float abs1 = q1Sort.absoluteCapacity;
float abs2 = q2Sort.absoluteCapacity;
return Float.compare(abs2, abs1); {code}
was:
The TimSort Transitivity rules got broken down when comparing both queues with
resources (0, 0), another queue with resources(some number, some number) and
with the same queues absolute capacity.
*Steps reproduce with Unit test:*
/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
{code:java}
@Test
public void testComparatorClassDoesNotViolateTimSortContract() {
String partition = "testPartition";
List<PriorityUtilizationQueueOrderingPolicy.PriorityQueueResourcesForSorting>
queues = new ArrayList<>();
for (int i = 0; i < 300; i++) {
queues.add(createTestElement(partition, Resource.newInstance(0, 0))); //
Need to be (0, 0)
queues.add(createTestElement(partition, Resource.newInstance(8, 20))); //
Could be any number
queues.add(createTestElement(partition, Resource.newInstance(8, 8))); //
Could be any number
}
Collections.shuffle(queues);
Collections.sort(queues, new PriorityUtilizationQueueOrderingPolicy(true)
.new PriorityQueueComparator(partition));
}
private PriorityUtilizationQueueOrderingPolicy.PriorityQueueResourcesForSorting
createTestElement(
String partition, Resource resource
) {
QueueCapacities mockQueueCapacities = mock(QueueCapacities.class);
when(mockQueueCapacities.getAbsoluteUsedCapacity(partition)).thenReturn(3.2f);
// Could be any number
when(mockQueueCapacities.getUsedCapacity(partition)).thenReturn(1.0f); //
Could be any number
when(mockQueueCapacities.getAbsoluteCapacity(partition)).thenReturn(4.2f); //
Could be any number
CSQueue mockQueue = mock(CSQueue.class);
when(mockQueue.getQueueCapacities()).thenReturn(mockQueueCapacities);
when(mockQueue.getPriority()).thenReturn(Priority.newInstance(5)); // Could
be any number
when(mockQueue.getAccessibleNodeLabels()).thenReturn(Collections.singleton("label1"));
QueueResourceQuotas randomQuotas = mock(QueueResourceQuotas.class);
when(randomQuotas.getConfiguredMinResource(partition)).thenReturn(resource);
when(mockQueue.getQueueResourceQuotas()).thenReturn(randomQuotas);
return new
PriorityUtilizationQueueOrderingPolicy.PriorityQueueResourcesForSorting(
mockQueue, partition
);
}{code}
*How to fix it?*
Instead of checking with an AND condition when both queues resources are not
none to compare its resources, we should check with an OR condition instead.
Because in the case one queue's resource is none another one is not we should
still compare by its resources.
Previous code:
{code:java}
if (!minEffRes1.equals(Resources.none()) &&
!minEffRes2.equals(Resources.none())) {
return minEffRes2.compareTo(minEffRes1);
}
float abs1 = q1Sort.absoluteCapacity;
float abs2 = q2Sort.absoluteCapacity;
return Float.compare(abs2, abs1); {code}
Changed code to:
{code:java}
if (!minEffRes1.equals(Resources.none()) ||
!minEffRes2.equals(Resources.none())) {
return minEffRes2.compareTo(minEffRes1);
}
float abs1 = q1Sort.absoluteCapacity;
float abs2 = q2Sort.absoluteCapacity;
return Float.compare(abs2, abs1); {code}
> YARN ResourceManager throws java.lang.IllegalArgumentExceptio: Comparison
> method violates its general contract!
> ---------------------------------------------------------------------------------------------------------------
>
> Key: YARN-11745
> URL: https://issues.apache.org/jira/browse/YARN-11745
> Project: Hadoop YARN
> Issue Type: Bug
> Components: yarn
> Affects Versions: 3.4.0
> Reporter: chhinlinghean
> Assignee: chhinlinghean
> Priority: Major
> Attachments: ExampleZeroQueueResourceproblem.pdf
>
>
> The TimSort Transitivity rules got broken down when comparing both queues
> with resources (0, 0), another queue with resources(some number, some number)
> and with the same queues absolute capacity.
> *Steps reproduce with Unit test:*
> /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
> {code:java}
> @Test
> public void testPriorityQueueComparatorClassDoesNotViolateTimSortContract() {
> String partition = "testPartition";
>
> List<PriorityUtilizationQueueOrderingPolicy.PriorityQueueResourcesForSorting>
> queues = new ArrayList<>();
> for (int i = 0; i < 300; i++) { // Have to be from 300 to make the test
> deterministic
> 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(8, 8)) // Could be any number
> );
> }
> Collections.shuffle(queues);
> // java.lang.IllegalArgumentException: Comparison method violates its
> general contract!
> assertDoesNotThrow(() -> Collections.sort(queues, new
> PriorityUtilizationQueueOrderingPolicy(true)
> .new PriorityQueueComparator(partition)));
> }
> private
> PriorityUtilizationQueueOrderingPolicy.PriorityQueueResourcesForSorting
> createMockPriorityQueueResourcesForSorting(String partition, Resource
> resource)
> {
> QueueCapacities mockQueueCapacities = mock(QueueCapacities.class);
>
> when(mockQueueCapacities.getAbsoluteUsedCapacity(partition)).thenReturn(3.2f);
> // Could be any number
> when(mockQueueCapacities.getUsedCapacity(partition)).thenReturn(1.0f); //
> Could be any number
> when(mockQueueCapacities.getAbsoluteCapacity(partition)).thenReturn(4.2f);
> // Could be any number
> CSQueue mockQueue = mock(CSQueue.class);
> when(mockQueue.getQueueCapacities()).thenReturn(mockQueueCapacities);
> when(mockQueue.getPriority()).thenReturn(Priority.newInstance(5)); // Could
> be any number
>
> when(mockQueue.getAccessibleNodeLabels()).thenReturn(Collections.singleton("label1"));
> // simulated label
> QueueResourceQuotas randomQuotas = mock(QueueResourceQuotas.class);
> when(randomQuotas.getConfiguredMinResource(partition)).thenReturn(resource);
> when(mockQueue.getQueueResourceQuotas()).thenReturn(randomQuotas);
> return new
> PriorityUtilizationQueueOrderingPolicy.PriorityQueueResourcesForSorting(
> mockQueue, partition
> );{code}
> *How to fix it?*
> Instead of checking with an AND condition when both queues resources are not
> none to compare its resources, we should check with an OR condition instead.
> Because in the case one queue's resource is none another one is not we should
> still compare by its resources.
> Previous code:
> {code:java}
> if (!minEffRes1.equals(Resources.none()) &&
> !minEffRes2.equals(Resources.none())) {
> return minEffRes2.compareTo(minEffRes1);
> }
> float abs1 = q1Sort.absoluteCapacity;
> float abs2 = q2Sort.absoluteCapacity;
> return Float.compare(abs2, abs1); {code}
> Changed code to:
> {code:java}
> if (!minEffRes1.equals(Resources.none()) ||
> !minEffRes2.equals(Resources.none())) {
> return minEffRes2.compareTo(minEffRes1);
> }
> float abs1 = q1Sort.absoluteCapacity;
> float abs2 = q2Sort.absoluteCapacity;
> return Float.compare(abs2, abs1); {code}
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
