zentol commented on a change in pull request #7927: [FLINK-11603][metrics] Port
the MetricQueryService to the new RpcEndpoint
URL: https://github.com/apache/flink/pull/7927#discussion_r270026642
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerRunner.java
##########
@@ -159,7 +157,7 @@ public TaskManagerRunner(Configuration configuration,
ResourceID resourceId) thr
this.terminationFuture = new CompletableFuture<>();
this.shutdown = false;
- MemoryLogger.startIfConfigured(LOG, configuration,
metricQueryServiceActorSystem);
+ MemoryLogger.startIfConfigured(LOG, configuration,
((AkkaRpcService) metricQueryServiceRpcService).getActorSystem());
Review comment:
In any case I rather dislike that the MemoryLogger knows it is dealing with
an ActorSystem. This makes it necessary to break the `RpcService` abstraction
here, and generally exposes way more than necessary.
We should only pass a `CompletableFuture<Void>` that finishes upon
termination of the TM, like `terminationFuture`.
I've opened https://issues.apache.org/jira/browse/FLINK-12057 and will open
a PR shortly, which I'd prefer to resolve first.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
