[GitHub] [flink] GJL commented on a change in pull request #9105: [FLINK-13241][Yarn/Mesos] Fix Yarn/MesosResourceManager setting managed memory size into wrong configuration instance.

[GitBox <gi...@apache.org>]

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

 ##########
 File path: flink-mesos/src/test/java/org/apache/flink/mesos/runtime/clusterframework/MesosResourceManagerTest.java
 ##########
 @@ -829,4 +839,45 @@ public void testClearStateAfterRevokeLeadership() throws Exception {
 			verify(rmServices.schedulerDriver).stop(true);
 		}};
 	}
+
+	/**
+	 * Tests that RM and TM calculate same slot resource profile.
+	 */
+	@Test
+	public void testCreateSlotsPerWorker() throws Exception {
 
 Review comment:
   @xintongsong I think it is not maintenance friendly to have a verbatim copy of `YarnResourceManagerTest#testCreateSlotsPerWorker()` for Mesos:
   
   1. These tests should be kept in sync but this is not obvious to the developer
   1. It looks like that we have to copy these tests once again for the Kubernetes ResourceManager. The development efforts are already in progress:
       * https://issues.apache.org/jira/browse/FLINK-9953
       * https://issues.apache.org/jira/browse/FLINK-9495
   
   I wonder if: 
   1. it is necessary to duplicate the entire test to be confident that `updateTaskManagerConfigAndCreateWorkerSlotProfiles()` is called in the constructor of the ResourceManager?
   1. duplicating the invocations of `updateTaskManagerConfigAndCreateWorkerSlotProfiles()` in the ResourceManager implementations, which consequently requires us to duplicate tests, is a design flaw?
   
   Maybe someone who was in involved in the design of this feature can chime in (cc: @StephanEwen)

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services