You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2021/01/05 10:35:18 UTC
[GitHub] [flink] KarmaGYZ opened a new pull request #14560: [FLINK-20837] Refactor dynamic SlotID
KarmaGYZ opened a new pull request #14560:
URL: https://github.com/apache/flink/pull/14560
## What is the purpose of the change
Refactor dynamic SlotID.
In FLINK-14189, we specified that the SlotID whose slot number is -1 represents the dynamic slot. However, the SlotID with a negative slot number will be treated as invalid in other components, e.g. SlotPool and SlotManager. We have to remove a lot of sanity checks with this change, which is error-prone. The evidence is that there are still some sanity checks that have not been removed, e.g. sanity checks in AllocatedSlotInfo.
In this PR, we refactor the dynamic SlotID:
- In SlotManager, it still uses -1 as slot number when allocating dynamic slot.
- In TM, when allocating dynamic slots, it will asssign an increasing number larger than the numberSlots as the slot number.
## Verifying this change
This change is already covered by existing tests:
- TaskExecutorTest
- TaskSlotTableImplTest
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] xintongsong commented on a change in pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
xintongsong commented on a change in pull request #14560:
URL: https://github.com/apache/flink/pull/14560#discussion_r552509039
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/slot/TaskSlotTableImpl.java
##########
@@ -463,7 +473,7 @@ public boolean isAllocated(int index, JobID jobId, AllocationID allocationId) {
TaskSlot<T> taskSlot = taskSlots.get(index);
if (taskSlot != null) {
return taskSlot.isAllocated(jobId, allocationId);
- } else if (index < 0) {
+ } else if (index >= numberSlots) {
Review comment:
If we also insert dynamic slot to the `taskSlot`, we won't need this `else-if` branch anymore.
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/slot/TaskSlotTableImpl.java
##########
@@ -95,6 +96,9 @@
/** The table state. */
private volatile State state;
+ /** Current index for dynamic slot, should always not less than numberSlots */
+ private AtomicInteger dynamicSlotIndex;
Review comment:
I think `TaskSlotTableImpl` is not designed to be thread-safe, and should always be accessed from the rpc main thread. So we should not need `AtomicInteger` here.
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/slot/TaskSlotTableImpl.java
##########
@@ -321,6 +325,12 @@ public boolean allocateSlot(
return false;
}
+ // The negative index indicate that the SlotManger allocate a dynamic slot, we transfer the
+ // index to an increasing number not less than the numberSlots.
+ if (index < 0) {
+ index = nextDynamicSlotIndex();
+ }
Review comment:
It's quite implicit that the method argument is overwritten in the middle of the method body.
I would suggest the following to convert `index` into a `effectiveIndex` at the beginning of this method. (Or maybe rename the argument to `requestedIndex` and convert it to `index`). Then use the effective index for the rest of the method.
That also means all the `index < 0` checks should be replaced with `index >= numberSlots`. Maybe introduce a util method `isDynamicIndex`.
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/slot/TaskSlotTableImpl.java
##########
@@ -288,6 +288,11 @@ public boolean allocateSlot(
TaskSlot<T> taskSlot = allocatedSlots.get(allocationId);
if (taskSlot != null) {
+ if (index < 0 && taskSlot.isAllocated(jobId, allocationId)) {
+ // If the slot is a dynamic slot with expected jobId and allocationId, it should be
+ // treated as duplicate allocate request.
+ return true;
+ }
Review comment:
These boolean expressions in the `if` and `return` statements have become quite hard to understand.
Maybe we can wrap them into separate methods with meaningful names.
Something like:
```
if (isAllocationIdExist()) {
return isDuplicateSlot();
} else if (isSlotIndexTaken()) {
return false;
}
```
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/slot/TaskSlotTableImpl.java
##########
@@ -288,6 +288,11 @@ public boolean allocateSlot(
TaskSlot<T> taskSlot = allocatedSlots.get(allocationId);
if (taskSlot != null) {
+ if (index < 0 && taskSlot.isAllocated(jobId, allocationId)) {
+ // If the slot is a dynamic slot with expected jobId and allocationId, it should be
+ // treated as duplicate allocate request.
+ return true;
+ }
Review comment:
I think this is a reported issue, FLINK-15660.
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/slot/TaskSlotTableImpl.java
##########
@@ -329,7 +339,7 @@ public boolean allocateSlot(
jobId,
allocationId,
memoryVerificationExecutor);
- if (index >= 0) {
+ if (index < numberSlots) {
Review comment:
Now since the dynamic slots also have unique indexes, we can also insert them into `taskSlots`.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] KarmaGYZ commented on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
KarmaGYZ commented on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-755933152
Thanks for the review @xintongsong . PR updated.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-754563038
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"triggerType" : "PUSH"
}, {
"hash" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651",
"triggerID" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"triggerType" : "PUSH"
}, {
"hash" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11720",
"triggerID" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"triggerType" : "PUSH"
}, {
"hash" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a4504bda6bf4e1b38aefb248bc8355bc7864f496 UNKNOWN
* b19b9ae12b807b3e012b0952a70b824a91df0ce5 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651)
* b8a38b363e2552a8ad2c174c7e0bc945e42398ff Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11720)
* 28384be50bb624a3080c2bca2b6d509a5a586fac UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-754563038
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"triggerType" : "PUSH"
}, {
"hash" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651",
"triggerID" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"triggerType" : "PUSH"
}, {
"hash" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11720",
"triggerID" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a4504bda6bf4e1b38aefb248bc8355bc7864f496 UNKNOWN
* b19b9ae12b807b3e012b0952a70b824a91df0ce5 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651)
* b8a38b363e2552a8ad2c174c7e0bc945e42398ff Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11720)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] xintongsong closed pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
xintongsong closed pull request #14560:
URL: https://github.com/apache/flink/pull/14560
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-754554960
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit 3868ec572b20eb0a4a2b53223ebd7834d0732136 (Fri May 28 07:02:54 UTC 2021)
**Warnings:**
* No documentation files were touched! Remember to keep the Flink docs up to date!
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>
--
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-754563038
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"triggerType" : "PUSH"
}, {
"hash" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651",
"triggerID" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"triggerType" : "PUSH"
}, {
"hash" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11720",
"triggerID" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"triggerType" : "PUSH"
}, {
"hash" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11730",
"triggerID" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"triggerType" : "PUSH"
}, {
"hash" : "428ee6ca325943b56fe944c455f0d67f2ce3f94b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11738",
"triggerID" : "428ee6ca325943b56fe944c455f0d67f2ce3f94b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a4504bda6bf4e1b38aefb248bc8355bc7864f496 UNKNOWN
* 428ee6ca325943b56fe944c455f0d67f2ce3f94b Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11738)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] xintongsong commented on a change in pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-754563038
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"triggerType" : "PUSH"
}, {
"hash" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651",
"triggerID" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a4504bda6bf4e1b38aefb248bc8355bc7864f496 UNKNOWN
* b19b9ae12b807b3e012b0952a70b824a91df0ce5 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-754563038
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"triggerType" : "PUSH"
}, {
"hash" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651",
"triggerID" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"triggerType" : "PUSH"
}, {
"hash" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11720",
"triggerID" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"triggerType" : "PUSH"
}, {
"hash" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11730",
"triggerID" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"triggerType" : "PUSH"
}, {
"hash" : "428ee6ca325943b56fe944c455f0d67f2ce3f94b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11738",
"triggerID" : "428ee6ca325943b56fe944c455f0d67f2ce3f94b",
"triggerType" : "PUSH"
}, {
"hash" : "3868ec572b20eb0a4a2b53223ebd7834d0732136",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11779",
"triggerID" : "3868ec572b20eb0a4a2b53223ebd7834d0732136",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a4504bda6bf4e1b38aefb248bc8355bc7864f496 UNKNOWN
* 428ee6ca325943b56fe944c455f0d67f2ce3f94b Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11738)
* 3868ec572b20eb0a4a2b53223ebd7834d0732136 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11779)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-754563038
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"triggerType" : "PUSH"
}, {
"hash" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651",
"triggerID" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"triggerType" : "PUSH"
}, {
"hash" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11720",
"triggerID" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"triggerType" : "PUSH"
}, {
"hash" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11730",
"triggerID" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"triggerType" : "PUSH"
}, {
"hash" : "428ee6ca325943b56fe944c455f0d67f2ce3f94b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11738",
"triggerID" : "428ee6ca325943b56fe944c455f0d67f2ce3f94b",
"triggerType" : "PUSH"
}, {
"hash" : "3868ec572b20eb0a4a2b53223ebd7834d0732136",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11779",
"triggerID" : "3868ec572b20eb0a4a2b53223ebd7834d0732136",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a4504bda6bf4e1b38aefb248bc8355bc7864f496 UNKNOWN
* 3868ec572b20eb0a4a2b53223ebd7834d0732136 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11779)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-754563038
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"triggerType" : "PUSH"
}, {
"hash" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651",
"triggerID" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"triggerType" : "PUSH"
}, {
"hash" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11720",
"triggerID" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"triggerType" : "PUSH"
}, {
"hash" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11730",
"triggerID" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"triggerType" : "PUSH"
}, {
"hash" : "428ee6ca325943b56fe944c455f0d67f2ce3f94b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11738",
"triggerID" : "428ee6ca325943b56fe944c455f0d67f2ce3f94b",
"triggerType" : "PUSH"
}, {
"hash" : "3868ec572b20eb0a4a2b53223ebd7834d0732136",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11779",
"triggerID" : "3868ec572b20eb0a4a2b53223ebd7834d0732136",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a4504bda6bf4e1b38aefb248bc8355bc7864f496 UNKNOWN
* 3868ec572b20eb0a4a2b53223ebd7834d0732136 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11779)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-754563038
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"triggerType" : "PUSH"
}, {
"hash" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651",
"triggerID" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"triggerType" : "PUSH"
}, {
"hash" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11720",
"triggerID" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"triggerType" : "PUSH"
}, {
"hash" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11730",
"triggerID" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"triggerType" : "PUSH"
}, {
"hash" : "428ee6ca325943b56fe944c455f0d67f2ce3f94b",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11738",
"triggerID" : "428ee6ca325943b56fe944c455f0d67f2ce3f94b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a4504bda6bf4e1b38aefb248bc8355bc7864f496 UNKNOWN
* 28384be50bb624a3080c2bca2b6d509a5a586fac Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11730)
* 428ee6ca325943b56fe944c455f0d67f2ce3f94b Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11738)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] xintongsong commented on a change in pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
xintongsong commented on a change in pull request #14560:
URL: https://github.com/apache/flink/pull/14560#discussion_r553190497
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/slot/TaskSlotTableImpl.java
##########
@@ -349,6 +345,28 @@ public boolean allocateSlot(
return true;
}
+ private boolean isDuplicatedSlot(
+ TaskSlot taskSlot, JobID jobId, ResourceProfile resourceProfile, int index) {
+ LOG.info(
+ "Slot with allocationId {} already exist, with resource profile {}, job id {} and index {}. The required index is {}.",
+ taskSlot.getAllocationId(),
+ taskSlot.getResourceProfile(),
+ taskSlot.getJobId(),
+ taskSlot.getIndex(),
+ index);
+ return taskSlot.getJobId().equals(jobId)
+ && taskSlot.getResourceProfile().equals(resourceProfile)
+ && (isDynamicIndex(index) || taskSlot.getIndex() == index);
+ }
+
+ private boolean isIndexAlreadyTaken(int index) {
+ return !isDynamicIndex(index) && taskSlots.get(index) != null;
Review comment:
Is `!isDynamicIndex(index)` necessary.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-754563038
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"triggerType" : "PUSH"
}, {
"hash" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a4504bda6bf4e1b38aefb248bc8355bc7864f496 UNKNOWN
* b19b9ae12b807b3e012b0952a70b824a91df0ce5 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-754563038
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"triggerType" : "PUSH"
}, {
"hash" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651",
"triggerID" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"triggerType" : "PUSH"
}, {
"hash" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11720",
"triggerID" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"triggerType" : "PUSH"
}, {
"hash" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11730",
"triggerID" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"triggerType" : "PUSH"
}, {
"hash" : "428ee6ca325943b56fe944c455f0d67f2ce3f94b",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11738",
"triggerID" : "428ee6ca325943b56fe944c455f0d67f2ce3f94b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a4504bda6bf4e1b38aefb248bc8355bc7864f496 UNKNOWN
* b8a38b363e2552a8ad2c174c7e0bc945e42398ff Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11720)
* 28384be50bb624a3080c2bca2b6d509a5a586fac Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11730)
* 428ee6ca325943b56fe944c455f0d67f2ce3f94b Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11738)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] xintongsong closed pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
xintongsong closed pull request #14560:
URL: https://github.com/apache/flink/pull/14560
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-754563038
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"triggerType" : "PUSH"
}, {
"hash" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651",
"triggerID" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"triggerType" : "PUSH"
}, {
"hash" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a4504bda6bf4e1b38aefb248bc8355bc7864f496 UNKNOWN
* b19b9ae12b807b3e012b0952a70b824a91df0ce5 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651)
* b8a38b363e2552a8ad2c174c7e0bc945e42398ff UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-754563038
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"triggerType" : "PUSH"
}, {
"hash" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651",
"triggerID" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"triggerType" : "PUSH"
}, {
"hash" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11720",
"triggerID" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"triggerType" : "PUSH"
}, {
"hash" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11730",
"triggerID" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a4504bda6bf4e1b38aefb248bc8355bc7864f496 UNKNOWN
* b19b9ae12b807b3e012b0952a70b824a91df0ce5 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651)
* b8a38b363e2552a8ad2c174c7e0bc945e42398ff Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11720)
* 28384be50bb624a3080c2bca2b6d509a5a586fac Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11730)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-754563038
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"triggerType" : "PUSH"
}, {
"hash" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651",
"triggerID" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"triggerType" : "PUSH"
}, {
"hash" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11720",
"triggerID" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"triggerType" : "PUSH"
}, {
"hash" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11730",
"triggerID" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"triggerType" : "PUSH"
}, {
"hash" : "428ee6ca325943b56fe944c455f0d67f2ce3f94b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11738",
"triggerID" : "428ee6ca325943b56fe944c455f0d67f2ce3f94b",
"triggerType" : "PUSH"
}, {
"hash" : "3868ec572b20eb0a4a2b53223ebd7834d0732136",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "3868ec572b20eb0a4a2b53223ebd7834d0732136",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a4504bda6bf4e1b38aefb248bc8355bc7864f496 UNKNOWN
* 428ee6ca325943b56fe944c455f0d67f2ce3f94b Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11738)
* 3868ec572b20eb0a4a2b53223ebd7834d0732136 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-754563038
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a4504bda6bf4e1b38aefb248bc8355bc7864f496 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] xintongsong commented on a change in pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
xintongsong commented on a change in pull request #14560:
URL: https://github.com/apache/flink/pull/14560#discussion_r553155630
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/slot/TaskSlotTableImpl.java
##########
@@ -288,22 +288,13 @@ public boolean allocateSlot(
TaskSlot<T> taskSlot = allocatedSlots.get(allocationId);
if (taskSlot != null) {
- LOG.info("Allocation ID {} is already allocated in {}.", allocationId, taskSlot);
- return false;
- }
-
- if (taskSlots.containsKey(index)) {
- TaskSlot<T> duplicatedTaskSlot = taskSlots.get(index);
+ return isDuplicatedSlot(taskSlot, jobId, resourceProfile, index);
+ } else if (!isIndexAlreadyTaken(index)) {
Review comment:
`!` should be removed.
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/slot/TaskSlotTableImpl.java
##########
@@ -277,27 +280,31 @@ public boolean allocateSlot(
@Override
public boolean allocateSlot(
- int index,
+ int requestedIndex,
JobID jobId,
AllocationID allocationId,
ResourceProfile resourceProfile,
Time slotTimeout) {
checkRunning();
- Preconditions.checkArgument(index < numberSlots);
+ Preconditions.checkArgument(requestedIndex < numberSlots);
+
+ // The negative requestIndex indicate that the SlotManger allocate a dynamic slot, we
+ // transfer the index to an increasing number not less than the numberSlots.
+ int index = requestedIndex < 0 ? nextDynamicSlotIndex() : requestedIndex;
TaskSlot<T> taskSlot = allocatedSlots.get(allocationId);
if (taskSlot != null) {
return isDuplicatedSlot(taskSlot, jobId, resourceProfile, index);
- } else if (!isIndexAlreadyTaken(index)) {
+ } else if (isIndexAlreadyTaken(index)) {
LOG.info(
"The static slot with index {} is already assigned to another allocation with id {}.",
index,
taskSlots.get(index).getAllocationId());
return false;
}
- resourceProfile = index >= 0 ? defaultSlotResourceProfile : resourceProfile;
+ resourceProfile = index < numberSlots ? defaultSlotResourceProfile : resourceProfile;
Review comment:
There are 3 occurrences of `index < numberSlots` and 1 occurrence of `index >= numberSlots` in this file.
Let's deduplicate it with a util method.
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/slot/TaskSlotTableImpl.java
##########
@@ -288,22 +288,13 @@ public boolean allocateSlot(
TaskSlot<T> taskSlot = allocatedSlots.get(allocationId);
if (taskSlot != null) {
- LOG.info("Allocation ID {} is already allocated in {}.", allocationId, taskSlot);
- return false;
- }
-
- if (taskSlots.containsKey(index)) {
- TaskSlot<T> duplicatedTaskSlot = taskSlots.get(index);
+ return isDuplicatedSlot(taskSlot, jobId, resourceProfile, index);
+ } else if (!isIndexAlreadyTaken(index)) {
LOG.info(
- "Slot with index {} already exist, with resource profile {}, job id {} and allocation id {}.",
+ "The static slot with index {} is already assigned to another allocation with id {}.",
Review comment:
Not sure about exposing the concept *static* slot here.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-754563038
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a4504bda6bf4e1b38aefb248bc8355bc7864f496",
"triggerType" : "PUSH"
}, {
"hash" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651",
"triggerID" : "b19b9ae12b807b3e012b0952a70b824a91df0ce5",
"triggerType" : "PUSH"
}, {
"hash" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11720",
"triggerID" : "b8a38b363e2552a8ad2c174c7e0bc945e42398ff",
"triggerType" : "PUSH"
}, {
"hash" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11730",
"triggerID" : "28384be50bb624a3080c2bca2b6d509a5a586fac",
"triggerType" : "PUSH"
}, {
"hash" : "428ee6ca325943b56fe944c455f0d67f2ce3f94b",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "428ee6ca325943b56fe944c455f0d67f2ce3f94b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a4504bda6bf4e1b38aefb248bc8355bc7864f496 UNKNOWN
* b19b9ae12b807b3e012b0952a70b824a91df0ce5 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11651)
* b8a38b363e2552a8ad2c174c7e0bc945e42398ff Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11720)
* 28384be50bb624a3080c2bca2b6d509a5a586fac Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11730)
* 428ee6ca325943b56fe944c455f0d67f2ce3f94b UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-754554960
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit a4504bda6bf4e1b38aefb248bc8355bc7864f496 (Tue Jan 05 10:38:24 UTC 2021)
**Warnings:**
* No documentation files were touched! Remember to keep the Flink docs up to date!
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] xintongsong commented on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
xintongsong commented on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-755237812
And please take a look at the compiling error reported by AZP.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] KarmaGYZ commented on pull request #14560: [FLINK-20837] Refactor dynamic SlotID
Posted by GitBox <gi...@apache.org>.
KarmaGYZ commented on pull request #14560:
URL: https://github.com/apache/flink/pull/14560#issuecomment-756555948
PR updated.
----------------------------------------------------------------
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:
users@infra.apache.org