dmvk commented on a change in pull request #17735:
URL: https://github.com/apache/flink/pull/17735#discussion_r754116367
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/adaptive/AdaptiveScheduler.java
##########
@@ -265,7 +268,8 @@ public AdaptiveScheduler(
this.executionGraphFactory = executionGraphFactory;
- registerMetrics();
+ SchedulerBase.registerJobMetrics(
+ jobManagerJobMetricGroup, jobStatusStore, () -> (long)
numRestarts);
Review comment:
Does `numRestarts` need to be volatile? If I understand that correctly
if we access the metric eg. using JMX, then it gets accessed by a different
thread. Or is there some synchronization in the metrics system that I'm missing?
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/adaptive/AdaptiveSchedulerTest.java
##########
@@ -679,9 +790,10 @@ public void
testJobStatusListenerNotifiedOfJobStatusChanges() throws Exception {
new TaskExecutionState(
submittedTask.getExecutionAttemptId(),
ExecutionState.FINISHED)));
- scheduler.getJobTerminationFuture().get();
- assertThat(jobStatusNotifications, hasItems(JobStatus.RUNNING,
JobStatus.FINISHED));
+ jobRunningNotification.get();
+ jobFinishedNotificaiton.get();
+ assertThat(unexpectedJobStatusNotification.getNow(null),
is(nullValue()));
Review comment:
```suggestion
assertFalse(unexpectedJobStatusNotification.isDone());
```
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/adaptive/AdaptiveSchedulerTest.java
##########
@@ -645,13 +743,26 @@ public void
testJobStatusListenerNotifiedOfJobStatusChanges() throws Exception {
final Configuration configuration = new Configuration();
configuration.set(JobManagerOptions.RESOURCE_WAIT_TIMEOUT,
Duration.ofMillis(1L));
- final Collection<JobStatus> jobStatusNotifications = new ArrayList<>();
+ final CompletableFuture<Void> jobRunningNotification = new
CompletableFuture<>();
+ final CompletableFuture<Void> jobFinishedNotificaiton = new
CompletableFuture<>();
Review comment:
```suggestion
final CompletableFuture<Void> jobFinishedNotification = new
CompletableFuture<>();
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
