XComp commented on a change in pull request #15170:
URL: https://github.com/apache/flink/pull/15170#discussion_r595105549
##########
File path:
flink-tests/src/test/java/org/apache/flink/test/recovery/ProcessFailureCancelingITCase.java
##########
@@ -92,17 +78,21 @@
@SuppressWarnings("serial")
public class ProcessFailureCancelingITCase extends TestLogger {
+ private static final String TASK_DEPLOYED_MARKER = "deployed";
+ private static final Duration TIMEOUT = Duration.ofMinutes(1);
Review comment:
duration was set to `2 minutes`
##########
File path:
flink-tests/src/test/java/org/apache/flink/test/recovery/ProcessFailureCancelingITCase.java
##########
@@ -269,57 +231,20 @@ public Long map(Long value) throws Exception {
}
}
- /**
- * Helper method to wait until the {@link Dispatcher} has set its fencing
token.
- *
- * @param rpcService to use to connect to the dispatcher
- * @param haServices high availability services to connect to the
dispatcher
- * @return {@link DispatcherGateway}
- * @throws Exception if something goes wrong
- */
- static DispatcherGateway retrieveDispatcherGateway(
- RpcService rpcService, HighAvailabilityServices haServices) throws
Exception {
- final LeaderConnectionInfo leaderConnectionInfo =
- LeaderRetrievalUtils.retrieveLeaderConnectionInfo(
- haServices.getDispatcherLeaderRetriever(),
Duration.ofSeconds(10L));
-
- return rpcService
- .connect(
- leaderConnectionInfo.getAddress(),
-
DispatcherId.fromUuid(leaderConnectionInfo.getLeaderSessionId()),
- DispatcherGateway.class)
- .get();
- }
-
- private void waitUntilAllSlotsAreUsed(DispatcherGateway dispatcherGateway,
Time timeout)
- throws ExecutionException, InterruptedException {
- FutureUtils.retrySuccessfulWithDelay(
- () ->
dispatcherGateway.requestClusterOverview(timeout),
- Time.milliseconds(50L),
-
Deadline.fromNow(Duration.ofMillis(timeout.toMilliseconds())),
- clusterOverview ->
- clusterOverview.getNumTaskManagersConnected()
>= 1
- &&
clusterOverview.getNumSlotsAvailable() == 0
- && clusterOverview.getNumSlotsTotal()
== 2,
- TestingUtils.defaultScheduledExecutor())
- .get();
- }
-
- private Collection<JobID> waitForRunningJobs(ClusterClient<?>
clusterClient, Time timeout)
- throws ExecutionException, InterruptedException {
- return FutureUtils.retrySuccessfulWithDelay(
- CheckedSupplier.unchecked(clusterClient::listJobs),
- Time.milliseconds(50L),
-
Deadline.fromNow(Duration.ofMillis(timeout.toMilliseconds())),
- jobs -> !jobs.isEmpty(),
- TestingUtils.defaultScheduledExecutor())
- .get().stream()
- .map(JobStatusMessage::getJobId)
- .collect(Collectors.toList());
+ private static void waitUntilAtLeastOneTaskHasBeenDeployed(TestProcess
taskManagerProcess)
+ throws InterruptedException, TimeoutException {
+ CommonTestUtils.waitUtil(
+ () ->
+ taskManagerProcess
+ .getProcessOutput()
+ .toString()
+ .contains(TASK_DEPLOYED_MARKER),
+ Duration.ofMinutes(2),
+ null);
}
- private void printProcessLog(String processName, String log) {
- if (log == null || log.length() == 0) {
+ private void printOutput(String processName, String logContents) {
Review comment:
The parameter renaming didn't work properly: `logContents` is used when
actually printing content
##########
File path:
flink-tests/src/test/java/org/apache/flink/test/recovery/ProcessFailureCancelingITCase.java
##########
@@ -269,57 +231,20 @@ public Long map(Long value) throws Exception {
}
}
- /**
- * Helper method to wait until the {@link Dispatcher} has set its fencing
token.
- *
- * @param rpcService to use to connect to the dispatcher
- * @param haServices high availability services to connect to the
dispatcher
- * @return {@link DispatcherGateway}
- * @throws Exception if something goes wrong
- */
- static DispatcherGateway retrieveDispatcherGateway(
- RpcService rpcService, HighAvailabilityServices haServices) throws
Exception {
- final LeaderConnectionInfo leaderConnectionInfo =
- LeaderRetrievalUtils.retrieveLeaderConnectionInfo(
- haServices.getDispatcherLeaderRetriever(),
Duration.ofSeconds(10L));
-
- return rpcService
- .connect(
- leaderConnectionInfo.getAddress(),
-
DispatcherId.fromUuid(leaderConnectionInfo.getLeaderSessionId()),
- DispatcherGateway.class)
- .get();
- }
-
- private void waitUntilAllSlotsAreUsed(DispatcherGateway dispatcherGateway,
Time timeout)
- throws ExecutionException, InterruptedException {
- FutureUtils.retrySuccessfulWithDelay(
- () ->
dispatcherGateway.requestClusterOverview(timeout),
- Time.milliseconds(50L),
-
Deadline.fromNow(Duration.ofMillis(timeout.toMilliseconds())),
- clusterOverview ->
- clusterOverview.getNumTaskManagersConnected()
>= 1
- &&
clusterOverview.getNumSlotsAvailable() == 0
- && clusterOverview.getNumSlotsTotal()
== 2,
- TestingUtils.defaultScheduledExecutor())
- .get();
- }
-
- private Collection<JobID> waitForRunningJobs(ClusterClient<?>
clusterClient, Time timeout)
- throws ExecutionException, InterruptedException {
- return FutureUtils.retrySuccessfulWithDelay(
- CheckedSupplier.unchecked(clusterClient::listJobs),
- Time.milliseconds(50L),
-
Deadline.fromNow(Duration.ofMillis(timeout.toMilliseconds())),
- jobs -> !jobs.isEmpty(),
- TestingUtils.defaultScheduledExecutor())
- .get().stream()
- .map(JobStatusMessage::getJobId)
- .collect(Collectors.toList());
+ private static void waitUntilAtLeastOneTaskHasBeenDeployed(TestProcess
taskManagerProcess)
+ throws InterruptedException, TimeoutException {
+ CommonTestUtils.waitUtil(
+ () ->
+ taskManagerProcess
+ .getProcessOutput()
+ .toString()
+ .contains(TASK_DEPLOYED_MARKER),
+ Duration.ofMinutes(2),
+ null);
}
- private void printProcessLog(String processName, String log) {
- if (log == null || log.length() == 0) {
+ private void printOutput(String processName, String logContents) {
Review comment:
Shouldn't we switch from `System.out.println` to `log.info` instead? Or
would this make the output worse as we have the log message prefix in there
twice?
----------------------------------------------------------------
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:
[email protected]
