[GitHub] [flink] GJL commented on a change in pull request #12278: [FLINK-17019][runtime] Fulfill slot requests in request order

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

GJL commented on a change in pull request #12278:
URL: https://github.com/apache/flink/pull/12278#discussion_r430919148



##########
File path: flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/slotpool/SlotPoolRequestCompletionTest.java
##########
@@ -103,7 +123,7 @@ private void runSlotRequestCompletionTest(
 			// check that the slot requests get completed in sequential order
 			for (int i = 0; i < slotRequestIds.size(); i++) {
 				final CompletableFuture<PhysicalSlot> slotRequestFuture = slotRequests.get(i);
-				slotRequestFuture.get();
+				assertThat(slotRequestFuture.getNow(null), not(is(nullValue())));

Review comment:
       `is(not(nullValue))` reads nicer

##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/slotpool/SlotPoolImpl.java
##########
@@ -698,13 +680,13 @@ boolean offerSlot(
 
 		componentMainThreadExecutor.assertRunningInMainThread();
 
-		final PendingRequest pendingRequest = pendingRequests.removeKeyB(allocationID);
+		final PendingRequest pendingRequest = pendingRequests.getKeyB(allocationID);

Review comment:
       It looks like that this should have been in a separate commit.




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