xintongsong commented on a change in pull request #12620:
URL: https://github.com/apache/flink/pull/12620#discussion_r439670500
##########
File path:
flink-kubernetes/src/test/java/org/apache/flink/kubernetes/KubernetesResourceManagerTest.java
##########
@@ -390,31 +429,7 @@ public void testPreviousAttemptPodAdded() throws Exception
{
// adding current attempt pod should decrease
the pending worker count
resourceManager.onAdded(Collections.singletonList(new
KubernetesPod(currentAttemptPod)));
-
assertThat(resourceManager.getNumPendingWorkersForTesting(), is(0));
- });
- }};
- }
-
- @Test
- public void testDuplicatedPodAdded() throws Exception {
Review comment:
This test case was verifying that when duplicated `added` event is
received for the same pod, only the first one decreases the pending counter.
I removed before realizing we need two different counters. When there's only
one counter that decreases at registration of new worker, there won't be any
behavior differences between handling the first and the duplicated `added`
event.
This is no longer true since now we have two counters, one of which is still
decreased on new worker `added`. I can add this test case back and make it
verify agains the `requestedNotAllocatedWorkerCounter` counter. Although
currently the `KubernetesResourceManager` is not relying on
`requestedNotAllocatedWorkerCounter` (only `YarnResourceManager` is using it),
it does not hurt to make sure the counters are properly maintained, incase
`KubernetesResourceManager` uses it 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:
us...@infra.apache.org
