----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/73136/#review222475 -----------------------------------------------------------
LGTM overall! 3rdparty/libprocess/src/posix/libev/libev.hpp Line 30 (original), 30 (patched) <https://reviews.apache.org/r/73136/#comment311602> Why don't we use a `std::vector` here? 3rdparty/libprocess/src/posix/libev/libev.hpp Lines 46-49 (patched) <https://reviews.apache.org/r/73136/#comment311603> Should `get_loop` be a part of `LoopIndex` construction to avoid being overlooked? 3rdparty/libprocess/src/posix/libev/libev.hpp Lines 94-95 (patched) <https://reviews.apache.org/r/73136/#comment311573> I think we should use 4-space indentation here: http://mesos.apache.org/documentation/latest/c++-style-guide/#function-definition-invocation 3rdparty/libprocess/src/posix/libev/libev.cpp Lines 185-186 (original), 257-258 (patched) <https://reviews.apache.org/r/73136/#comment311601> Should we reduce the number of libprocess actor threads by the number of additional IO threads we spawn here? - Ilya Pronin On Jan. 12, 2021, 11:22 a.m., Benjamin Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/73136/ > ----------------------------------------------------------- > > (Updated Jan. 12, 2021, 11:22 a.m.) > > > Review request for mesos and Ilya Pronin. > > > Bugs: MESOS-10208 > https://issues.apache.org/jira/browse/MESOS-10208 > > > Repository: mesos > > > Description > ------- > > The current approach to I/O in libprocess, with a single thread > performing all of the the I/O polling and I/O syscalls, cannot keep > up with the I/O load on massive scale mesos clusters (which use > libev rather than libevent). > > This adds support via a LIBPROCESS_LIBEV_NUM_IO_THREADS env variable > for configuring the number of threads running libev event loops, > which allows users to spread the IO load across multiple threads. > > > Diffs > ----- > > 3rdparty/libprocess/src/posix/libev/libev.hpp > d451931871db650894e4a6e5b0d19ba876f65391 > 3rdparty/libprocess/src/posix/libev/libev.cpp > b38e7a0f882a8c24950bdc6fd74a4d25fc68549e > 3rdparty/libprocess/src/posix/libev/libev_poll.cpp > 96913a65507ca3540066e28448684d1e3fa540ca > > > Diff: https://reviews.apache.org/r/73136/diff/1/ > > > Testing > ------- > > So far only manual testing: > > ``` > # Error > $ make check -j16 TEST_DRIVER="" > GTEST_FILTER="-ProcessRemoteLinkTest.RemoteLinkLeak" > LIBPROCESS_LIBEV_NUM_IO_THREADS=0 > $ make check -j16 TEST_DRIVER="" > GTEST_FILTER="-ProcessRemoteLinkTest.RemoteLinkLeak" > LIBPROCESS_LIBEV_NUM_IO_THREADS=1025 > > # Success > $ make check -j16 TEST_DRIVER="" > GTEST_FILTER="-ProcessRemoteLinkTest.RemoteLinkLeak" > LIBPROCESS_LIBEV_NUM_IO_THREADS=1 > $ make check -j16 TEST_DRIVER="" > GTEST_FILTER="-ProcessRemoteLinkTest.RemoteLinkLeak" > LIBPROCESS_LIBEV_NUM_IO_THREADS=32 > ``` > > Will follow up with some test(s) that leverage the reinitialize support in > libprocess, so that the testing doesn't need to be done manually. > > > Thanks, > > Benjamin Mahler > >
