azagrebin commented on a change in pull request #9105: 
[FLINK-13241][Yarn/Mesos] Fix Yarn/MesosResourceManager setting managed memory 
size into wrong configuration instance.
URL: https://github.com/apache/flink/pull/9105#discussion_r304773143
 
 

 ##########
 File path: 
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YarnConfigurationITCase.java
 ##########
 @@ -187,6 +191,12 @@ public void testFlinkContainerMemory() throws Exception {
                                        assertThat(
                                                (double) 
taskManagerInfo.getHardwareDescription().getSizeOfJvmHeap() / (double) 
expectedHeadSize,
                                                is(closeTo(1.0, 0.15)));
+
+                                       final int defaultTaskManagerMemoryMB = 
ConfigurationUtils.getTaskManagerHeapMemory(configuration).getMebiBytes();
 
 Review comment:
   True, both tests have to be changed, more effort. On the other side, this 
brings attention to them and forces to check the change against any previous 
assumptions expressed by tests. In this case, the real computation might change 
in a way that `updateTaskManagerConfigAndCreateWorkerSlotProfiles` is not used 
at all etc.

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

Reply via email to