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




src/scheduler/scheduler.cpp (lines 117 - 119)
<https://reviews.apache.org/r/43662/#comment181864>

    This comment is old. Can you update?
    
    s/receiving messages (eventually events/HTTP messages/
    
    s/sending messages (via calls)/HTTP messages/



src/scheduler/scheduler.cpp (line 207)
<https://reviews.apache.org/r/43662/#comment181865>

    I think for both scheduler::Mesos and executor::Mesos we need to add more 
comments around send() semantics. 
    
    For example, we should mention that clients should send() only after 
connected() callback has been called.



src/scheduler/scheduler.cpp (lines 237 - 242)
<https://reviews.apache.org/r/43662/#comment181866>

    Shouldn't we do the same for executor library?
    
    More importnatly, what if a previous subscribe is in progress but 
`subscribed` is not set?



src/scheduler/scheduler.cpp (line 264)
<https://reviews.apache.org/r/43662/#comment181872>

    CHECK(state == DISCONNECTED) ?



src/scheduler/scheduler.cpp (lines 278 - 280)
<https://reviews.apache.org/r/43662/#comment181873>

    s/i.e./e.g.,/ ?
    
    s/we failed over to a new master/master failed over/
    
    Also, not sure what you wanted to say in this comment? Are you saying it is 
OK even if a new master is enqueued because....?



src/scheduler/scheduler.cpp (line 297)
<https://reviews.apache.org/r/43662/#comment181876>

    This is the current master as detected by the detector. What is the 
guarantee that the connection was made with this master and not an old master? 
IOW, should this method take `master` as an argument?
    
    Can we guarantee that the master hasn't changed because a new master 
detected causes these futures to be discarded synchronously? Looking at 
detected() that doesn't seem to be the case.
    
    Do we want to enforce the invariant that there is ever only one connection 
attempt in progress?



src/scheduler/scheduler.cpp (line 370)
<https://reviews.apache.org/r/43662/#comment181886>

    is it possible for this callback to be invoked on the scheduler after they 
get a connected callback from this new connection attempt?



src/scheduler/scheduler.cpp (line 400)
<https://reviews.apache.org/r/43662/#comment181891>

    What happens if a previous connection attempt is in progress?



src/scheduler/scheduler.cpp (line 457)
<https://reviews.apache.org/r/43662/#comment181887>

    For executor library, we did a close() here of the previous reader. Why was 
it required there and not here?


- Vinod Kone


On Feb. 22, 2016, 8:19 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43662/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2016, 8:19 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3570
>     https://issues.apache.org/jira/browse/MESOS-3570
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously, the scheduler library used to chain calls on previous call 
> responses. This was inherently slow. This change adds support for pipelining 
> all calls to the master on a single connection via the `http::Connection` 
> abstraction in libprocess.
> 
> This change also adds support for handling various error scenarios when we 
> notice a disconnection instead of just relying on the master detector for 
> invoking the `disconnected` callback.
> 
> 
> Diffs
> -----
> 
>   src/scheduler/scheduler.cpp 99a7d0dfff7b0c61decc9ff6d9e6d46ef13a7e75 
> 
> Diff: https://reviews.apache.org/r/43662/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Reply via email to