----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43662/#review120947 -----------------------------------------------------------
src/scheduler/scheduler.cpp (lines 203 - 206) <https://reviews.apache.org/r/43662/#comment182487> kill this and do all the drops after #213. src/scheduler/scheduler.cpp (line 214) <https://reviews.apache.org/r/43662/#comment182489> //..... if (call.type() == SUBSCRIBE && state != CONNECTED) { drop(); return; } //..... if (call.type() != SUBSCRIBE && state != SUBSCRIBED) { drop(); return; } src/scheduler/scheduler.cpp (lines 232 - 239) <https://reviews.apache.org/r/43662/#comment182490> Kill this. src/scheduler/scheduler.cpp (lines 233 - 239) <https://reviews.apache.org/r/43662/#comment182488> It's better to consolidate all drops before #215. src/scheduler/scheduler.cpp (line 246) <https://reviews.apache.org/r/43662/#comment182486> shouldn't we drop calls if we are not in SUBSCRIBED state? see above. src/scheduler/scheduler.cpp (line 249) <https://reviews.apache.org/r/43662/#comment182491> CHECK_SOME(connectionId); ? also, why is this method taking an Option<ConnectionID> instead of ConnectionID? src/scheduler/scheduler.cpp (line 276) <https://reviews.apache.org/r/43662/#comment182492> why is this taking an option? src/scheduler/scheduler.cpp (line 290) <https://reviews.apache.org/r/43662/#comment182493> Add a VLOG(1)? src/scheduler/scheduler.cpp (line 378) <https://reviews.apache.org/r/43662/#comment182494> what if we are in CONNECTING state? src/scheduler/scheduler.cpp (line 387) <https://reviews.apache.org/r/43662/#comment182495> s/old// src/scheduler/scheduler.cpp (line 457) <https://reviews.apache.org/r/43662/#comment182496> what about other states? can we be in CONNECTING here for example? src/scheduler/scheduler.cpp (line 498) <https://reviews.apache.org/r/43662/#comment182497> // We reset the state to CONNECTED if... src/scheduler/scheduler.cpp (line 533) <https://reviews.apache.org/r/43662/#comment182500> Do we need 'subscribed' as a member variable now? Can we not detect this via state and/or connectionID? src/scheduler/scheduler.cpp (line 546) <https://reviews.apache.org/r/43662/#comment182498> Can you add a comment here on when this can happen? src/scheduler/scheduler.cpp (line 567) <https://reviews.apache.org/r/43662/#comment182499> what about other states? src/scheduler/scheduler.cpp (line 609) <https://reviews.apache.org/r/43662/#comment182482> s/connection/connections/ src/scheduler/scheduler.cpp (line 610) <https://reviews.apache.org/r/43662/#comment182483> s/connection/connections/ - Vinod Kone On Feb. 25, 2016, 4:27 p.m., Anand Mazumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43662/ > ----------------------------------------------------------- > > (Updated Feb. 25, 2016, 4:27 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 > >
