----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49177/#review139641 -----------------------------------------------------------
Fix it, then Ship it! 3rdparty/libprocess/include/process/process.hpp (lines 153 - 174) <https://reviews.apache.org/r/49177/#comment204907> How about: ``` enum class RemoteConnection { REUSE, RECONNECT, } link(master, RemoteConnection::REUSE); link(master, RemoteConnection::RECONNECT); ``` Be sure to update the documentation accordingly. One suggestion is to avoid discussing remote vs local within the enum value documentation and instead mention at the top that the enum describes the remote link semantics but has no effect on the local links (because as described below, they are guaranteed to provide perfect monitoring). 3rdparty/libprocess/src/process.cpp (line 1594) <https://reviews.apache.org/r/49177/#comment204913> How about: ``` Socket existing = ...; // or const Socket existing = ...; ``` Consider CHECKing the invariant that the socket map contains the fd, or using `at`: ``` CHECK(sockets.count(persists.at(to.address) == 1); Socket existing = sockets[persists[to.address]]; // or Socket existing = sockets.at(persists.at(to.address)); ``` 3rdparty/libprocess/src/process.cpp (lines 1600 - 1604) <https://reviews.apache.org/r/49177/#comment204936> Whoops, this needs to still run in the local case. The tests should fail? 3rdparty/libprocess/src/process.cpp (line 1610) <https://reviews.apache.org/r/49177/#comment204920> If you want to clean this up (and the same code in link_connect) in a separate patch (no need for to send a review), can do the following: ``` socket->connect(to.address) ``` - Benjamin Mahler On June 24, 2016, 2:43 a.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49177/ > ----------------------------------------------------------- > > (Updated June 24, 2016, 2:43 a.m.) > > > Review request for mesos, Benjamin Hindman, Benjamin Mahler, Gilbert Song, > Artem Harutyunyan, and Jie Yu. > > > Bugs: MESOS-5576 > https://issues.apache.org/jira/browse/MESOS-5576 > > > Repository: mesos > > > Description > ------- > > The `REMOTE_RELINK` option for `ProcessBase::link` will force the > `SocketManager` to create a new socket if a persistent link already > exists. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/process.hpp > 2946e50081c4a418a868083806a26f995c295007 > 3rdparty/libprocess/src/process.cpp > 9bae71246e751e491be5a989eea8aca29c9aa751 > > Diff: https://reviews.apache.org/r/49177/diff/ > > > Testing > ------- > > make check (OSX) > > > Thanks, > > Joseph Wu > >
