Github user EronWright commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4628#discussion_r140051191
  
    --- Diff: 
flink-mesos/src/main/java/org/apache/flink/mesos/runtime/clusterframework/MesosFlinkResourceManager.java
 ---
    @@ -663,6 +666,7 @@ private void taskTerminated(Protos.TaskID taskID, 
Protos.TaskStatus status) {
        // 
------------------------------------------------------------------------
     
        private LaunchableMesosWorker createLaunchableMesosWorker(Protos.TaskID 
taskID) {
    +           setCoTaskGetter();
    --- End diff --
    
    I believe that this approach is violating an Akka rule by passing a direct 
reference to the RM object to the launch coordinator.   I understand the 
challenge to be that Fenzo's `BalancedHostAttrConstraint` expects a 
`coTasksGetter` function 
([ref](http://netflix.github.io/Fenzo/fenzo-core/com/netflix/fenzo/plugins/BalancedHostAttrConstraint.html#BalancedHostAttrConstraint-com.netflix.fenzo.functions.Func1-java.lang.String-int-)).
   But that function cannot keep a reference to the RM object.  
    
     An imperfect solution would be to pass the `taskIds` to the 
`LaunchableMesosWorker` constructor; imperfect because the list wouldn't be 
dynamic.
    
    In truth we want all tasks to be balanced and so the getter is actually 
overkill.  If you look at the implementation of `BalancedHostAttrConstraint` 
([ref](https://github.com/Netflix/Fenzo/blob/master/fenzo-core/src/main/java/com/netflix/fenzo/plugins/BalancedHostAttrConstraint.java))
 it uses the function output to lookup tasks in the `TaskTrackerState`.   A 
custom constraint could simply use all tasks in `TaskTrackerState`  without 
need for a `coTasksGetter`.
    
    Once we get this sorted out, some of the code should be moved to a utility 
class.


---

Reply via email to