-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50496/#review144422
-----------------------------------------------------------




src/log/network.cpp (lines 49 - 57)
<https://reviews.apache.org/r/50496/#comment210438>

    Modifying the parameters is a somewhat odd thing to do.  It suggests that 
the module interface is insufficient.
    
    In this case, the Network abstraction appears to want the "base" PID (i.e. 
of the current `ReplicaProcess`).
    See: https://issues.apache.org/jira/browse/MESOS-1231
    
    We have the following options to pass in this PID:
    
    1) Override the module parameters like you've done.
    2) Add a new method to the network interface and call it after creating the 
module.
    3) Grab the pid some other way, from inside the module.  (i.e. NOT 
RECOMMENDED! `curl localhost:5050/__processes__ | grep log-replica`)
    
    (2) Seems like the cleanest solution, as it will ensure the "base" PID of 
the network is always set.  If we pass in the "base" PID through module 
parameters, the module may choose to ignore it, which may break later 
(MESOS-1231).


- Joseph Wu


On July 27, 2016, 10:27 p.m., Jay Guo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50496/
> -----------------------------------------------------------
> 
> (Updated July 27, 2016, 10:27 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5828
>     https://issues.apache.org/jira/browse/MESOS-5828
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is a factory method used to create a log::Network module instance.
> 
> 
> Diffs
> -----
> 
>   include/mesos/log/network.hpp PRE-CREATION 
>   src/log/network.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50496/diff/
> 
> 
> Testing
> -------
> 
> see end of review chain.
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Reply via email to