xintongsong commented on a change in pull request #14560:
URL: https://github.com/apache/flink/pull/14560#discussion_r553743795
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/taskexecutor/slot/TaskSlotTableImplTest.java
##########
@@ -115,11 +115,11 @@ public void testRetrievingAllActiveSlots() throws
Exception {
}
/**
- * Tests that redundant slot allocation with the same AllocationID to a
different slot is
- * rejected.
+ * Tests that inconsistent static slot allocation with the same
AllocationID to a different slot
+ * is rejected.
*/
@Test
- public void testRedundantSlotAllocation() throws Exception {
+ public void testInconsistentStaticSlotAllocation() throws Exception {
Review comment:
This covers the case trying to allocate the same allocation id to
different slot index.
I think we need to also cover the case that different allocation ids trying
to take the same slot index
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/taskexecutor/slot/TaskSlotTableImplTest.java
##########
@@ -137,6 +137,68 @@ public void testRedundantSlotAllocation() throws Exception
{
}
}
+ /**
+ * Tests that inconsistent dynamic slot allocation with the same
AllocationID to a different
+ * slot is rejected.
+ */
+ @Test
+ public void testInconsistentDynamicSlotAllocation() throws Exception {
+ try (final TaskSlotTable<TaskSlotPayload> taskSlotTable =
createTaskSlotTableAndStart(1)) {
+ final JobID jobId1 = new JobID();
+ final JobID jobId2 = new JobID();
+ final AllocationID allocationId = new AllocationID();
+
+ assertThat(
+ taskSlotTable.allocateSlot(-1, jobId1, allocationId,
SLOT_TIMEOUT), is(true));
+ assertThat(
+ taskSlotTable.allocateSlot(-1, jobId2, allocationId,
SLOT_TIMEOUT), is(false));
+
+ assertThat(taskSlotTable.isAllocated(-1, jobId1, allocationId),
is(true));
+
+ Iterator<TaskSlot<TaskSlotPayload>> allocatedSlots =
+ taskSlotTable.getAllocatedSlots(jobId1);
+ assertThat(allocatedSlots.next().getAllocationId(),
is(allocationId));
+ assertThat(allocatedSlots.hasNext(), is(false));
+ }
+ }
+
+ @Test
+ public void testDuplicateStaticSlotAllocation() throws Exception {
+ try (final TaskSlotTable<TaskSlotPayload> taskSlotTable =
createTaskSlotTableAndStart(2)) {
+ final JobID jobId = new JobID();
+ final AllocationID allocationId = new AllocationID();
+
+ assertThat(taskSlotTable.allocateSlot(0, jobId, allocationId,
SLOT_TIMEOUT), is(true));
+ assertThat(taskSlotTable.allocateSlot(0, jobId, allocationId,
SLOT_TIMEOUT), is(true));
+
+ assertThat(taskSlotTable.isAllocated(0, jobId, allocationId),
is(true));
+ assertThat(taskSlotTable.isSlotFree(1), is(true));
+
+ Iterator<TaskSlot<TaskSlotPayload>> allocatedSlots =
+ taskSlotTable.getAllocatedSlots(jobId);
+ assertThat(allocatedSlots.next().getIndex(), is(0));
+ assertThat(allocatedSlots.hasNext(), is(false));
+ }
+ }
+
+ @Test
+ public void testDuplicateDynamicSlotAllocation() throws Exception {
Review comment:
I think we should also verify that the second call of `allocateSlot`
does not replace the slot allocated by the first call.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
