[GitHub] [flink] zhuzhurk commented on a change in pull request #9902: [FLINK-14363][runtime] Prevent vertex from being affected by outdated deployment

[GitBox <gi...@apache.org>]

zhuzhurk commented on a change in pull request #9902: [FLINK-14363][runtime] Prevent vertex from being affected by outdated deployment
URL: https://github.com/apache/flink/pull/9902#discussion_r338191388
 
 

 ##########
 File path: flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/TestingLogicalSlot.java
 ##########
 @@ -111,10 +113,14 @@ public Payload getPayload() {
 
 	@Override
 	public CompletableFuture<?> releaseSlot(@Nullable Throwable cause) {
-		slotOwner.returnLogicalSlot(this);
+		if (!isReleased) {
 
 Review comment:
   Not problematic currently.
   
   Actually I noticed this issue before `DefaultSchedulerTest` is refactored to use `TestExecutionSlotAllocator` instead of `SimpleSlotProvider`. When testing the new test(it was different from the current implementation since `SimpleSlotProvider` does not allow queued allocation), I saw a weird case that canceling a vertex makes 2 new slots become available. The cause is that an assigned slot can be released 2 times, each time a new slot will be returned to `SimpleSlotProvider` from `TestingLogicalSlot`. (that's also why I'd like to only release unassigned slot in 8ab26a8cbf11df7301995b783a59c739e2ad26d3).
   
   So I think it's still valid to fix it to avoid to encounter such weird cases in the future.

----------------------------------------------------------------
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


With regards,
Apache Git Services