zentol commented on a change in pull request #13356:
URL: https://github.com/apache/flink/pull/13356#discussion_r490846909



##########
File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/TaskExecutorRegistration.java
##########
@@ -72,13 +77,15 @@ public TaskExecutorRegistration(
                        final String taskExecutorAddress,
                        final ResourceID resourceId,
                        final int dataPort,
+                       final int jmxPort,
                        final HardwareDescription hardwareDescription,
                        final TaskExecutorMemoryConfiguration 
memoryConfiguration,
                        final ResourceProfile defaultSlotResourceProfile,
                        final ResourceProfile totalResourceProfile) {
                this.taskExecutorAddress = checkNotNull(taskExecutorAddress);
                this.resourceId = checkNotNull(resourceId);
                this.dataPort = dataPort;
+               this.jmxPort = jmxPort;

Review comment:
       hmm...I think it would make sense to also include the `dataPort`; it 
doesn't appear to be used by the runtime? But that also seems to apply to the 
hardwareDescription and memoryConfiguration.
   
   hmm...maybe we should instead just bundle them in container, that is then 
just passed along. It would achieve my goal of not having explicit accesses to 
the jmx Port in various components, while still allowing us to _use_ them for 
other things down the line.




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


Reply via email to