----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67157/#review203871 -----------------------------------------------------------
3rdparty/libprocess/include/process/grpc.hpp Lines 27 (patched) <https://reviews.apache.org/r/67157/#comment286205> Since you are touching includes, let's make sure we include what is required. We should add #include <process/process.hpp> // Process #include <stout/errorbase.hpp> // Error #include <stout/nothing.hpp> // Nothing #include <string> // string We should remove #include <atomic> #include <stout/duration.hpp> #include <stout/synchronized.hpp> 3rdparty/libprocess/include/process/grpc.hpp Lines 152-154 (patched) <https://reviews.apache.org/r/67157/#comment286232> Let's rewrap this for better readability, typename std::enable_of< std::is_convertible< typename std::decay<Request>::type*, google::protobuf::Message*>::value, int>::type = 0> 3rdparty/libprocess/src/grpc.cpp Lines 70 (patched) <https://reviews.apache.org/r/67157/#comment286234> I am slightly worried that we only `join` this thread on `terminate`. Could we make sure we `join` the thread whenever `looper` gets `reset` or goes out of scope -- we might be able to e.g., use a custom deleter for that if the lifetimes are guaranteed to be correct. 3rdparty/libprocess/src/grpc.cpp Lines 76 (patched) <https://reviews.apache.org/r/67157/#comment286233> This seems almost like the libprocess equivalent of a throwing destructor. Could we trigger this e.g., if we fail a test `ASSERT`? Also see my comment regarding joining whenever `looper` goes out of scope. - Benjamin Bannier On May 18, 2018, 12:23 a.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67157/ > ----------------------------------------------------------- > > (Updated May 18, 2018, 12:23 a.m.) > > > Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and > Zhitao Li. > > > Bugs: MESOS-8924 > https://issues.apache.org/jira/browse/MESOS-8924 > > > Repository: mesos > > > Description > ------- > > The refactoring does the following things: > 1. Manage the gRPC completion queue and the looper thread in the runtime > process to get rid of a lock in `Runtime::Data`. > 2. Move the computation of sending a request into the runtime process. > 3. Let libprocess manage the runtime process automatically instead of > managing its lifecycle in `Runtime::Data`. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/grpc.hpp > 321a46e19c69eafb24012bcef68bb8b0cc6aa436 > 3rdparty/libprocess/src/grpc.cpp a80bcb614ec96d92d21bc88a281d3208e86141a0 > > > Diff: https://reviews.apache.org/r/67157/diff/4/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Chun-Hung Hsiao > >
