----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64910/#review195394 -----------------------------------------------------------
Looks good, but I'm a little puzzled at what invariants stop there being two concurrently active send loops, so I held off on a ship it since I couldn't quickly convince myself that this is prevented. 3rdparty/libprocess/src/process.cpp Lines 1562-1567 (original), 1563-1568 (patched) <https://reviews.apache.org/r/64910/#comment274561> I'm puzzled as to how this doesn't trigger two concurrent send loops, can we document what's preventing that? 3rdparty/libprocess/src/process.cpp Lines 1734-1757 (original), 1718-1727 (patched) <https://reviews.apache.org/r/64910/#comment274560> `Option<Encoder*>` seems a little odd since you're not CHECK_NOTNULLing the pointer within the option, maybe just use nullptr and avoid the option altogether? That would clean up the code a bit: ``` return loop( None(), [=]() { return socket_manager->next(socket); }, [=](Encoder* encoder) -> Future<ControlFlow<Nothing>> { ... }); ``` Or directly return an Owned from the next function and have nullptr represent the none case? - Benjamin Mahler On Jan. 3, 2018, 7:23 a.m., Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64910/ > ----------------------------------------------------------- > > (Updated Jan. 3, 2018, 7:23 a.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > In order to simplify the code to use the `send()` helper for encoders > as well as introduce `loop()` we also updated and simplified the code > to always enqueue the encoders into `outgoing`. > > > Diffs > ----- > > 3rdparty/libprocess/src/process.cpp > 75cf1d3b6d3d257ba9bc81c68017a74a6511cebf > 3rdparty/libprocess/src/socket_manager.hpp > dd4d111c4ae579420060e547d1111d12f8f0711c > > > Diff: https://reviews.apache.org/r/64910/diff/2/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Hindman > >
