tillrohrmann commented on a change in pull request #7568: [FLINK-11417] Make
access to ExecutionGraph single threaded from JobMaster main thread
URL: https://github.com/apache/flink/pull/7568#discussion_r252293096
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/executiongraph/IndividualRestartsConcurrencyTest.java
##########
@@ -83,6 +84,13 @@
*/
public class IndividualRestartsConcurrencyTest extends TestLogger {
Review comment:
In our current implementation of the `RestartIndividualStrategy` we have no
longer concurrency. It is a bit odd that we only introduce it for this test
case via the `testExecutor`. Moreover, this test is in my opinion more a
`ConcurrentFailoverStrategyExecutionGraphTest` which tests that the
`ExecutionGraph` properly restricts state changes wrt the `globalVersionMod`
which is not necessarily bound restricted to the `RestartIndividualStrategy`.
What I would suggest is the following: Remove the `testExecutor` from the
`RestartIndividualStrategy` and introducing a
`ManuallyTriggeredFailoverStrategy`. The `ManuallyTriggeredFailoverStrategy` is
only for testing purposes and might simply extend `RestartIndividualStrategy`.
The `ManuallyTriggeredFailoverStrategy` will simply collect all calls to the
`restartProcedure` of the `RestartIndividualStrategy` which for this purpose
should be factored out into its own overridable method. Then we can leave the
tests basically as they are using the
`TestComponentMainThreadExecutor#forMainThread`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
