flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [flink] rmetzger commented on a change in pull request #15884: [FLINK-22266] Fix stop-with-savepoint operation in AdaptiveScheduler
Date Thu, 20 May 2021 07:05:57 GMT

rmetzger commented on a change in pull request #15884:
URL: https://github.com/apache/flink/pull/15884#discussion_r635821863



##########
File path: flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/adaptive/ExecutingTest.java
##########
@@ -258,6 +305,23 @@ public void testFalseReportsViaUpdateTaskExecutionStateAreIgnored() throws
Excep
         }
     }
 
+    @Test
+    public void testExecutionVertexMarkedAsFailedOnDeploymentFailure() throws Exception {
+        try (MockExecutingContext ctx = new MockExecutingContext()) {
+            MockExecutionJobVertex mejv =
+                    new MockExecutionJobVertex(FailOnDeployMockExecutionVertex::new);
+            ExecutionGraph executionGraph =
+                    new MockExecutionGraph(() -> Collections.singletonList(mejv));
+            Executing exec =
+                    new ExecutingStateBuilder().setExecutionGraph(executionGraph).build(ctx);
+
+            assertThat(
+                    ((FailOnDeployMockExecutionVertex) mejv.getMockExecutionVertex())
+                            .getMarkedFailure(),
+                    is(instanceOf(JobException.class)));

Review comment:
       The error handling of markFailed is difficult to test, because so many components are
involved. But in my opinion, we have good test coverage:
   
   markFailed will (through the DefaultExecutionGraph) notify the `InternalFailuresListener`
about the task failure. The `UpdateSchedulerNgOnInternalFailuresListener` implementation used
by adaptive scheduler will call updateTaskExecutionState on the scheduler. This chain of calls
will be used for example for the failure in the `AdaptiveSchedulerITCase.testGlobalFailoverCanRecoverState()`
test.
   
   For the Executing state, we have tests that exceptions during deployment lead to a markFailed
call (`testExecutionVertexMarkedAsFailedOnDeploymentFailure`), and failures reported via updateTaskExecutionState
to appropriate error handling (`testFailureReportedViaUpdateTaskExecutionStateCausesFailingOnNoRestart`,
`testFailureReportedViaUpdateTaskExecutionStateCausesRestart`, `testFalseReportsViaUpdateTaskExecutionStateAreIgnored`).
   
   Adding a test that a markFailed call will notify the `InternalFailuresListener` is out
of the scope of the ExecutingTest (because we are testing the ExecutionVertex and Execution
classes).
   Adding a test that a markFailed call will call updateTaskExecutionState will need to go
through a test specific `InternalFailuresListener`: Since all the relevant calls on ExecutingState
are already covered, this would only test the test specific `InternalFailuresListener`.
   
   




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



Mime
View raw message