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:
[email protected]
With regards,
Apache Git Services
