Re: Review Request 67955: Added some new generic flag parsers.
> On July 19, 2018, 6:51 p.m., Kevin Klues wrote: > > 3rdparty/stout/include/stout/flags/parse.hpp > > Lines 249-251 (patched) > > <https://reviews.apache.org/r/67955/diff/1/?file=2061303#file2061303line249> > > > > This comment is wrong. It's not for unsigned ints, it's for strings. Okay I'll fix it, any reason I can't ship it once I change the comment? - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67955/#review206242 ------- On July 18, 2018, 1:19 a.m., Benjamin Hindman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67955/ > --- > > (Updated July 18, 2018, 1:19 a.m.) > > > Review request for mesos, Benjamin Mahler and Kevin Klues. > > > Repository: mesos > > > Description > --- > > Added some new generic flag parsers. > > > Diffs > - > > 3rdparty/stout/include/stout/flags/parse.hpp > 4566b798b8b66d47779a28cabdea06f588012686 > > > Diff: https://reviews.apache.org/r/67955/diff/1/ > > > Testing > --- > > make check > > > Thanks, > > Benjamin Hindman > >
Review Request 67959: Added support for instrumenting processes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67959/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Added support for instrumenting processes. Diffs - 3rdparty/libprocess/include/process/process.hpp 7c255acc21c695eba37062a3dcf72ce33f650cd0 3rdparty/libprocess/src/process.cpp 7c0a0bc0c1e50354b6da219032ac830cbeec0a0d 3rdparty/libprocess/src/tests/process_tests.cpp 8baf60d8bbb675e26fea5e76c825ef73fedbc629 Diff: https://reviews.apache.org/r/67959/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 67958: Added LibprocessTest for easily configuring the library for a test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67958/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- There was not a generic way for configuring and then reinitializing the libprocess library in a test and the LibprocessTest fixture provides this functionality. Diffs - 3rdparty/libprocess/include/process/gtest.hpp fd4de8ab7c79940519b2e890a9b0b615ca9ca292 Diff: https://reviews.apache.org/r/67958/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 67960: Added support for instrumenting HTTP endpoints.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67960/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Added support for instrumenting HTTP endpoints. Diffs - 3rdparty/libprocess/include/process/process.hpp 7c255acc21c695eba37062a3dcf72ce33f650cd0 3rdparty/libprocess/include/process/ssl/gtest.hpp 23d7aee963b6fb489403a94500d39e3413c7fcdd 3rdparty/libprocess/src/process.cpp 7c0a0bc0c1e50354b6da219032ac830cbeec0a0d 3rdparty/libprocess/src/tests/http_tests.cpp acbd6973829411652fc5d57ef473c0d8ba9cd5b4 Diff: https://reviews.apache.org/r/67960/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 67957: Refactored TemporaryDirectoryTest to be a mixin.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67957/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Currently you can't easily "mixin" the functionality of the TemporaryDirectoryTest which makes for tests that might have a weird inheritance structure. This refactor makes it easy for you to get the functionality of the TemporaryDirectoryTest while easily composing it with other test classes. Diffs - 3rdparty/stout/include/stout/tests/utils.hpp eb13cceec29c5db8a75067aac5a00b8f8e9b5046 Diff: https://reviews.apache.org/r/67957/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 67956: Removed some generic flag parsers that are now in stout.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67956/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Removed some generic flag parsers that are now in stout. Diffs - src/common/parse.hpp 03814e3112a043a1001764b316b9d49501d33665 Diff: https://reviews.apache.org/r/67956/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 67955: Added some new generic flag parsers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67955/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Added some new generic flag parsers. Diffs - 3rdparty/stout/include/stout/flags/parse.hpp 4566b798b8b66d47779a28cabdea06f588012686 Diff: https://reviews.apache.org/r/67955/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 67927: Removed potentially unneeded lambda capture.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67927/#review206117 --- Ship it! Looks good and thanks for a great comment! - Benjamin Hindman On July 16, 2018, 7:22 p.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67927/ > --- > > (Updated July 16, 2018, 7:22 p.m.) > > > Review request for mesos, Benjamin Hindman and Dario Rexin. > > > Repository: mesos > > > Description > --- > > In some instances variables used in the body of a lambda do not need > to be captured explicitly, see e.g., > https://stackoverflow.com/a/43468519/176922. Support for this across > compiler vendors seems to be incomplete at this point; while e.g., > clang emits a warning (which we potentially turn into a fatal error) > for this, msvc still seems to require such variables to be captured. > > To provide a warning free build this patch moves from the usually > preferred explicit capture list to an implicit capture by reference. > This allows us to capture all needed variables correctly without > having to capture too much. > > > Diffs > - > > 3rdparty/libprocess/src/tests/benchmarks.cpp > e8ef21fd9c203ac5e10c17694393322151231435 > > > Diff: https://reviews.apache.org/r/67927/diff/3/ > > > Testing > --- > > Tested on a number of platforms and compilers in internal CI. > > > Thanks, > > Benjamin Bannier > >
Review Request 67921: Bug fix for semaphore decomission "deadlock".
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67921/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-8239 https://issues.apache.org/jira/browse/MESOS-8239 Repository: mesos Description --- Fixes MESOS-8239. When using the DecomissionableLastInFirstOutFixedSizeSemaphore it's possible that waiting threads may never be properly signaled. This bug fix makes sure that every waiting thread gets a signal after a decomission. Diffs - 3rdparty/libprocess/src/semaphore.hpp 50501b9797894ad274eb73f74b3eed00cd719114 Diff: https://reviews.apache.org/r/67921/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 65151: Added a test for discarding a future from Queue::get.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65151/#review195436 --- Ship it! Ship It! - Benjamin Hindman On Jan. 13, 2018, 9:35 p.m., Benjamin Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65151/ > --- > > (Updated Jan. 13, 2018, 9:35 p.m.) > > > Review request for mesos and Benjamin Hindman. > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > 3rdparty/libprocess/src/tests/queue_tests.cpp > 95b738133fa50641f8f9b83014837d2808e0e4ff > > > Diff: https://reviews.apache.org/r/65151/diff/1/ > > > Testing > --- > > Ran in repetitino to ensure there was no memory growth. > > > Thanks, > > Benjamin Mahler > >
Re: Review Request 65150: Added support for discarding a Future from Queue::get.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65150/#review195431 --- Fix it, then Ship it! 3rdparty/libprocess/include/process/queue.hpp Lines 69 (patched) <https://reviews.apache.org/r/65150/#comment274613> It seems like some mental overhead to realize that `future` should only ever be pending if it comes from the first branch of the if block above and not the else. Why not just include this within the if block above? 3rdparty/libprocess/include/process/queue.hpp Lines 74 (patched) <https://reviews.apache.org/r/65150/#comment274614> I personally prefer nesting these as it's less prone to copy-pasta mistakes, but up to you. - Benjamin Hindman On Jan. 13, 2018, 9:35 p.m., Benjamin Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65150/ > --- > > (Updated Jan. 13, 2018, 9:35 p.m.) > > > Review request for mesos and Benjamin Hindman. > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > 3rdparty/libprocess/include/process/queue.hpp > 4d1c2cfa811b9377da8631311aaf39634695eae6 > > > Diff: https://reviews.apache.org/r/65150/diff/1/ > > > Testing > --- > > Added a test in the subsequent patch. > > > Thanks, > > Benjamin Mahler > >
Review Request 65171: Reduced copying of Response by adding extra override Headers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65171/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Reduced copying of Response by adding extra override Headers. Diffs - 3rdparty/libprocess/src/encoder.hpp 70b5ec479e90c0eb6ac729b465739b581729a956 3rdparty/libprocess/src/http.cpp cc41fa6f671cf029a46722299eded1a27da210d3 Diff: https://reviews.apache.org/r/65171/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 64909: Removed unnecessary `SocketManager::dispose` data structure.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64909/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- We used `dispose` to capture the sockets that `HttpProxy` did not want to persist but now that `HttpProxy` does not use `SocketManager` we no longer need to use `dispose` because `temps` is sufficient for knowing which sockets are temporary. Diffs - 3rdparty/libprocess/src/process.cpp 75cf1d3b6d3d257ba9bc81c68017a74a6511cebf 3rdparty/libprocess/src/socket_manager.hpp dd4d111c4ae579420060e547dd12f8f0711c Diff: https://reviews.apache.org/r/64909/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 64907: Added abandonment and process exit support to `loop()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64907/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- The current semantics for `loop()` mean that we'll leak resources under two different circumstances: (1) If either the "iteration" or "body" functions return future's that are abandoned. (2) If the loop is using a process that exits. This patch adds support to make sure that abandoned futures are properly propagated in the event that either of these situations occurs. Diffs - 3rdparty/libprocess/include/process/loop.hpp 69d90f3e1b16189e0b1a6f1981e8509c18b38465 3rdparty/libprocess/src/tests/loop_tests.cpp 8d1837a0baedc12591f92c8f0f8ea83d0aa44ab0 Diff: https://reviews.apache.org/r/64907/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 64908: Refactored `HttpProxy` to not rely on `SocketManager`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64908/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- This simplifies the introduction of `http::Server` so that it's easier to reason about and in the future will make removing the `HttpProxy` implementation much easier. Diffs - 3rdparty/libprocess/src/http_proxy.hpp 5b6e7e8786ed9eab50cd4c2cfdec455c92d72eca 3rdparty/libprocess/src/http_proxy.cpp f584238aadd86875d7c87736653f394e716397de 3rdparty/libprocess/src/process.cpp 75cf1d3b6d3d257ba9bc81c68017a74a6511cebf 3rdparty/libprocess/src/socket_manager.hpp dd4d111c4ae579420060e547dd12f8f0711c Diff: https://reviews.apache.org/r/64908/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 64906: Exposed `send()` helpers for `Socket`'s.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64906/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Pulled out two `send()` functions that simplify sending an `http::Response` and an `Encoder` using a `Socket`. Diffs - 3rdparty/libprocess/include/process/http.hpp 055447e13117c0a3ba79d0fc326ece657e8f064f 3rdparty/libprocess/src/encoder.hpp 70b5ec479e90c0eb6ac729b465739b581729a956 3rdparty/libprocess/src/http.cpp f51d2aaa639ee36ce960268cb32a3aa88f14aa29 Diff: https://reviews.apache.org/r/64906/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 64291: Refactored authorization logic.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64291/ --- Review request for mesos. Repository: mesos Description --- * Introduced `ObjectApprovers` to unify authorization logic. * Updated code in master and agent to use `ObjectApprovers`. * Deleted unused code. Diffs - src/common/http.hpp 750d3bfba1647624983108bdd23295a3b3091fe4 src/common/http.cpp 3ada1f064b4bd001cd4d7dccc186641f475011a0 src/common/protobuf_utils.hpp 03b7cafb264b07a5a27dbd8d5c074e96ce576ea0 src/common/protobuf_utils.cpp c5504a00a7a84b824743f00c6b97ea299ac66eb4 src/master/http.cpp bb040ced65495cfe78b67ca7258499c16ac5d8a1 src/master/master.hpp 5d2ae658070d9c5a0bc630c15ff89dc449857f46 src/master/master.cpp 883793a226849434eb833c3027d121635a86fdc4 src/slave/http.hpp a51831cdcebc1998ce6d4c3c19285e598a4ec9a3 src/slave/http.cpp fd0e809d5dc7722e573b66621f75d791f0911dfa src/slave/slave.cpp c07e25f668bef6b6fa3cae4b69ba90de3eb3bdcc Diff: https://reviews.apache.org/r/64291/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 64081: Fix a code snippet in libprocess documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64081/#review191892 --- Ship it! Ship It! - Benjamin Hindman On Nov. 27, 2017, 11:06 a.m., Alexander Rukletsov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64081/ > --- > > (Updated Nov. 27, 2017, 11:06 a.m.) > > > Review request for mesos and Benjamin Hindman. > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > 3rdparty/libprocess/README.md ba09653dbc26a997856b712c9f082c6e06c39137 > > > Diff: https://reviews.apache.org/r/64081/diff/1/ > > > Testing > --- > > None: not a functional change. > > > Thanks, > > Alexander Rukletsov > >
Review Request 64073: Fixed ignored Socket::accept discard when using LibeventSSLSocketImpl.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64073/ --- Review request for mesos, Avinash sridharan and Benjamin Mahler. Repository: mesos Description --- Fixed ignored Socket::accept discard when using LibeventSSLSocketImpl. Diffs - 3rdparty/libprocess/src/libevent_ssl_socket.cpp 1c95ebabfefd07aaeb053b965ab8e4550dfccaef Diff: https://reviews.apache.org/r/64073/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61158: Introduced http::Server in process.cpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61158/ --- Review request for mesos, Avinash sridharan and Benjamin Mahler. Repository: mesos Description --- Using http::Server can be compile time configured via USE_HTTP_SERVER. Diffs - 3rdparty/libprocess/src/http_proxy.hpp 5b6e7e8786ed9eab50cd4c2cfdec455c92d72eca 3rdparty/libprocess/src/http_proxy.cpp f584238aadd86875d7c87736653f394e716397de 3rdparty/libprocess/src/process.cpp 64bcce215d19558dd493e30e96ca16577fe0722a 3rdparty/libprocess/src/socket_manager.hpp dd4d111c4ae579420060e547dd12f8f0711c Diff: https://reviews.apache.org/r/61158/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 64074: Added ENABLE_HTTP_SERVER option for cmake.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64074/ --- Review request for mesos, Avinash sridharan and Benjamin Mahler. Repository: mesos Description --- Added ENABLE_HTTP_SERVER option for cmake. Diffs - cmake/CompilationConfigure.cmake deb574200bceb605a60b536a396c98d9247c6018 Diff: https://reviews.apache.org/r/64074/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 63942: Added a state machine abstraction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63942/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- An actor is often implemented like a state machine but the actual management of the state and transitions is very manual. This abstraction helps reduce a lot of the boilerplate assocaited with state machines. Diffs - 3rdparty/libprocess/Makefile.am 03a0ca87f31744c716c99e05aa07242fed480675 3rdparty/libprocess/include/Makefile.am d57a6103881aefef420dc0cf2e85cdf37d560887 3rdparty/libprocess/include/process/state_machine.hpp PRE-CREATION 3rdparty/libprocess/src/tests/CMakeLists.txt b01bf9784563a8f06168f8471222d8da2ffb9b85 3rdparty/libprocess/src/tests/state_machine_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/63942/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 63941: Added support for enum's to hashset and hashmap.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63941/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Added support for enum's to hashset and hashmap. Diffs - 3rdparty/stout/include/stout/hashmap.hpp 539bbfd9250f2f372c2b3211e2379ad2229fa35e 3rdparty/stout/include/stout/hashset.hpp d2f7d5ddba62cc87e2b61cb06c86b8ef4cd1a212 Diff: https://reviews.apache.org/r/63941/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 63940: Added a Future constructor for Try>.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63940/#review191462 --- Ship it! Ship It! - Benjamin Hindman On Nov. 20, 2017, 12:23 a.m., Benjamin Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63940/ > --- > > (Updated Nov. 20, 2017, 12:23 a.m.) > > > Review request for mesos and Benjamin Hindman. > > > Repository: mesos > > > Description > --- > > Longer term we may want to figure out an alternative "flattening" > strategy, but for now the approach has been to add constructors > that allow flattening. > > > Diffs > - > > 3rdparty/libprocess/include/process/future.hpp > 4cf44ba1a7052d5f84beb8fd6914b8e9e0437c0a > 3rdparty/libprocess/src/tests/future_tests.cpp > 76a32bd69499e52ea1623ab982d65e1c7a0cbd32 > > > Diff: https://reviews.apache.org/r/63940/diff/1/ > > > Testing > --- > > Added a test. > > > Thanks, > > Benjamin Mahler > >
Re: Review Request 63606: Avoid an extra copy during ProtobufProcess::send.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63606/#review190319 --- Ship it! Ship It! - Benjamin Hindman On Nov. 7, 2017, 4:23 a.m., Benjamin Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63606/ > --- > > (Updated Nov. 7, 2017, 4:23 a.m.) > > > Review request for mesos, Benjamin Hindman and Jiang Yan Xu. > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > 3rdparty/libprocess/include/process/protobuf.hpp > fe152f273332470ac50f9715291897bb04cf95b9 > > > Diff: https://reviews.apache.org/r/63606/diff/1/ > > > Testing > --- > > make check > > > Thanks, > > Benjamin Mahler > >
Re: Review Request 63605: Support moving in data during ProcessBase::send.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63605/#review190318 --- Ship it! Ship It! - Benjamin Hindman On Nov. 7, 2017, 3:40 a.m., Benjamin Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63605/ > --- > > (Updated Nov. 7, 2017, 3:40 a.m.) > > > Review request for mesos, Benjamin Hindman and Jiang Yan Xu. > > > Repository: mesos > > > Description > --- > > This allows callers that can move in data to avoid an extra copy. > > > Diffs > - > > 3rdparty/libprocess/include/process/event.hpp > 2e246205020c3c5b8c2eb5187a8eb3643d1e6d4d > 3rdparty/libprocess/include/process/process.hpp > dc3375ce62556322eb2bc60ade61f313ade123b8 > 3rdparty/libprocess/src/process.cpp > 71ae7129ffbd0e22eda2863b17bbcf588298c37b > 3rdparty/libprocess/src/tests/benchmarks.cpp > e6c77d565d5acf72b475a085e9504679253b4b97 > 3rdparty/libprocess/src/tests/process_tests.cpp > 952c92c033e2363cff0c2c68610d3820b97d177e > > > Diff: https://reviews.apache.org/r/63605/diff/1/ > > > Testing > --- > > Ran benchmarks before and after per benh's request, no difference: > https://docs.google.com/spreadsheets/d/1I6iUziBwouQSn2veoj-xVpq7B9n6zdO9zPimi1IlJZ4/edit?usp=sharing > > > Thanks, > > Benjamin Mahler > >
Re: Review Request 63604: Removed an unnecessary serialization in protobuf process.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63604/#review190315 --- Ship it! Ship It! - Benjamin Hindman On Nov. 7, 2017, 3:40 a.m., Benjamin Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63604/ > --- > > (Updated Nov. 7, 2017, 3:40 a.m.) > > > Review request for mesos, Benjamin Hindman and Jiang Yan Xu. > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > 3rdparty/libprocess/include/process/protobuf.hpp > fe152f273332470ac50f9715291897bb04cf95b9 > > > Diff: https://reviews.apache.org/r/63604/diff/1/ > > > Testing > --- > > make check > > > Thanks, > > Benjamin Mahler > >
Review Request 63468: Added TERMINATED state to processes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63468/ --- Review request for mesos, Alexander Rukletsov and Benjamin Mahler. Repository: mesos Description --- Added TERMINATED state to processes. Diffs - 3rdparty/libprocess/include/process/process.hpp dc3375ce62556322eb2bc60ade61f313ade123b8 3rdparty/libprocess/src/process.cpp 71ae7129ffbd0e22eda2863b17bbcf588298c37b Diff: https://reviews.apache.org/r/63468/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 63292: Added lamdba::zip.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63292/ --- Review request for mesos, Alexander Rojas and Vinod Kone. Repository: mesos Description --- Added lamdba::zip. Diffs - 3rdparty/stout/include/stout/lambda.hpp a61d97b69e7eebd057c94148d39c6e1fc3066017 3rdparty/stout/tests/lambda_tests.cpp ad8c2efddb6b64184670d0cfb33188ef843351ab Diff: https://reviews.apache.org/r/63292/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 63208: Fixed a crash in ProcessManager::resume due to race.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63208/#review188906 --- Fix it, then Ship it! Good debugging! Thanks Ben! 3rdparty/libprocess/src/process.cpp Lines 3291 (patched) <https://reviews.apache.org/r/63208/#comment265878> Any way we could not have to grab this reference each time we loop? This is in the hot path. - Benjamin Hindman On Oct. 22, 2017, 3 a.m., Benjamin Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63208/ > --- > > (Updated Oct. 22, 2017, 3 a.m.) > > > Review request for mesos and Benjamin Hindman. > > > Bugs: MESOS-7921 > https://issues.apache.org/jira/browse/MESOS-7921 > > > Repository: mesos > > > Description > --- > > When ProcessManager::resume moves the process into a BLOCKED > state, it's possible that a TerminateEvent is enqueued and the > process is placed back in the run queue. Another worker thread > can pick it off the run queue, process the TerminateEvent, and > delete the process! At this point, when the original thread > in ProcessManager::resume tries to check if any events were > enqueued before transitioning to BLOCKED, it will access a > deleted process and crash. > > Some example crash paths involving process::Latch below. > > Path 1: > > T1 creates latch, spawns latch process, and puts it in run queue > T1 waits on latch > T1 ProcessManager::wait on latch see it in BOTTOM > > T2 worker thread dequeues the latch process > T2 ProcessManager::resume runs initialize, moves it to READY > T2 ProcessManager::resume sees empty queue > T2 ProcessManager::resume sets to BLOCKED > > T3 triggers the latch, terminates the latch process > T3 enqueue TerminateEvent > T3 enqueue sees state BLOCKED > T3 swaps from BLOCKED->READY & enqueues latch process into run queue > > T1 extracts latch process from run queue > T1 ProcessManager::resume sees READY > T1 ProcessManager::resume dequeues terminate event > T1 ProcessManager::resume calls ProcessManager::cleanup > T1 ProcessManager::cleanup sets to TERMINATING > T1 ProcessManager::cleanup decommissions event queue > T1 ProcessManager::cleanup waits for latch refs to go away > T1 ProcessManager::cleanup calls SocketManager::exited > T1 ProcessManager::cleanup opens gate > T1 ProcessManager::cleanup deletes the latch process > > T2 ProcessManager::resume checks if event queue is empty again (crash) > > Path 2: > > T1 creates latch, spawns latch process, and puts it in run queue > T1 waits on latch > T1 ProcessManager::wait on latch see it in BOTTOM > T1 ProcessManager::wait extracts latch process from run queue > T1 ProcessManager::resume runs initialize, moves it to READY > T1 ProcessManager::resume sees empty queue > T1 ProcessManager::resume sets to BLOCKED > > T3 triggers the latch, terminates the latch process > T3 enqueue TerminateEvent > T3 enqueue sees state BLOCKED > T3 swaps from BLOCKED->READY & enqueues latch process into run queue > > T2 worker thread dequeues the latch process > T2 ProcessManager::resume sees READY > T2 ProcessManager::resume dequeues terminate event > T2 ProcessManager::resume calls ProcessManager::cleanup > T2 ProcessManager::cleanup sets to TERMINATING > T2 ProcessManager::cleanup decommissions event queue > T2 ProcessManager::cleanup waits for latch refs to go away > T2 ProcessManager::cleanup calls SocketManager::exited > T2 ProcessManager::cleanup opens gate > T2 ProcessManager::resume deletes the latch process > > T1 ProcessManager::resume checks if event queue is empty again (crash) > > > Diffs > - > > 3rdparty/libprocess/src/process.cpp > 4d8d483cfa5c3bac8bbe6a985f1cc2b737ae691e > > > Diff: https://reviews.apache.org/r/63208/diff/1/ > > > Testing > --- > > * Tested by injecting a sleep > [here](https://github.com/apache/mesos/blob/1.4.0/3rdparty/libprocess/src/process.cpp#L3278). > * Ran throughput benchmark, results are highly variable across runs, see > results > [here](https://docs.google.com/spreadsheets/d/1yoQtvAWiojLWBV0TlqjCxdc1RMtNERGSEZzgCxkWoxI/edit?usp=sharing). > > > Thanks, > > Benjamin Mahler > >
Re: Review Request 62698: Fixed the clang complation error for the executor tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62698/#review186749 --- Ship it! Ship It! - Benjamin Hindman On Sept. 29, 2017, 9:45 p.m., Chun-Hung Hsiao wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62698/ > --- > > (Updated Sept. 29, 2017, 9:45 p.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Repository: mesos > > > Description > --- > > clang does not allow conversions from `Future` to > `Future`, so we specify the return type of the lambda > in the `THREADSAFE_Executor_Execute` test explicitly. > > > Diffs > - > > 3rdparty/libprocess/src/tests/process_tests.cpp > e49431873393e835bf0e0a9c240e5d54f06e0ab7 > > > Diff: https://reviews.apache.org/r/62698/diff/1/ > > > Testing > --- > > make check with gcc 5.4.0 and clang 3.4.2. > > > Thanks, > > Chun-Hung Hsiao > >
Review Request 62609: Protect against Future callbacks deleting instance.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62609/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-8010 https://issues.apache.org/jira/browse/MESOS-8010 Repository: mesos Description --- See MESOS-8010. Diffs - 3rdparty/libprocess/include/process/future.hpp a11a588941b02d776da2f563dd246a92dfbbc360 Diff: https://reviews.apache.org/r/62609/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 62553: Fixed a flaky test.
> On Sept. 26, 2017, 1:08 a.m., Gilbert Song wrote: > > src/tests/containerizer/cgroups_isolator_tests.cpp > > Line 445 (original), 444 (patched) > > <https://reviews.apache.org/r/62553/diff/1/?file=1834730#file1834730line445> > > > > The flaky behavior is still reproducable: > > ``` > > [ RUN ] CgroupsIsolatorTest.ROOT_CGROUPS_CFS_EnableCfs > > I0926 01:07:11.409624 20616 exec.cpp:162] Version: 1.5.0 > > I0926 01:07:11.10 20637 exec.cpp:237] Executor registered on agent > > e36d5653-eee2-4f69-92ee-3d8aec9a3cdc-S0 > > I0926 01:07:11.563771 20636 executor.cpp:171] Received SUBSCRIBED event > > I0926 01:07:11.565313 20636 executor.cpp:175] Subscribed executor on > > vagrant-ubuntu-wily-64 > > I0926 01:07:11.566074 20636 executor.cpp:171] Received LAUNCH event > > I0926 01:07:11.567462 20636 executor.cpp:633] Starting task > > 7995dc33-b190-4295-b643-b274f278ad50 > > I0926 01:07:11.583531 20636 executor.cpp:477] Running > > '/vagrant/mesos/build/src/mesos-containerizer launch > > ' > > I0926 01:07:11.594823 20636 executor.cpp:646] Forked command at 20639 > > ../../src/tests/containerizer/cgroups_isolator_tests.cpp:444: Failure > > Expected: (0.35) >= (cpuTime), actual: 0.35 vs 0.39 > > *** Aborted at 1506388031 (unix time) try "date -d @1506388031" if you > > are using GNU date *** > > PC: @ 0x263f886 testing::UnitTest::AddTestPartResult() > > *** SIGSEGV (@0x0) received by PID 17734 (TID 0x7f106c83a800) from PID > > 0; stack trace: *** > > @ 0x7f1062d24d10 (unknown) > > @ 0x263f886 testing::UnitTest::AddTestPartResult() > > @ 0x263f409 testing::internal::AssertHelper::operator=() > > @ 0x246ebd7 > > mesos::internal::tests::CgroupsIsolatorTest_ROOT_CGROUPS_CFS_EnableCfs_Test::TestBody() > > @ 0x267d903 > > testing::internal::HandleSehExceptionsInMethodIfSupported<>() > > @ 0x266a467 > > testing::internal::HandleExceptionsInMethodIfSupported<>() > > @ 0x26495d5 testing::Test::Run() > > @ 0x264a301 testing::TestInfo::Run() > > @ 0x264aa17 testing::TestCase::Run() > > @ 0x2652418 testing::internal::UnitTestImpl::RunAllTests() > > @ 0x267ae33 > > testing::internal::HandleSehExceptionsInMethodIfSupported<>() > > @ 0x266c457 > > testing::internal::HandleExceptionsInMethodIfSupported<>() > > @ 0x26520d5 testing::UnitTest::Run() > > @ 0x1619be1 RUN_ALL_TESTS() > > @ 0x161861d main > > @ 0x7f106296aa40 (unknown) > > @ 0xaec8a9 _start > > I0926 01:07:11.910209 20630 exec.cpp:517] Agent exited ... shutting down > > I0926 01:07:11.910732 20630 executor.cpp:171] Received SHUTDOWN event > > I0926 01:07:11.911000 20630 executor.cpp:743] Shutting down > > I0926 01:07:11.911231 20630 executor.cpp:850] Sending SIGTERM to > > process tree at pid 20639 > > ``` > > > > Should we give a 200ms buffer instead? Yes, that's fine with me. But I don't love this test. The real crux here is that this test is really dependent on the environment, i.e., what is running on that machine. Is there a better way to do this? Is there something else we can "test" to ensure that CFS has been turned on, rather than trying to actually exercise the CFS algorithm? - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62553/#review186190 --- On Sept. 25, 2017, 10:47 p.m., Benjamin Hindman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62553/ > --- > > (Updated Sept. 25, 2017, 10:47 p.m.) > > > Review request for mesos, Gilbert Song and Jie Yu. > > > Repository: mesos > > > Description > --- > > Fixed a flaky test. > > > Diffs > - > > src/tests/containerizer/cgroups_isolator_tests.cpp > 3fc93417f2d3febf2feca3ec1c8476c9edcfbf4d > > > Diff: https://reviews.apache.org/r/62553/diff/1/ > > > Testing > --- > > make check > > > Thanks, > > Benjamin Hindman > >
Review Request 62553: Fixed a flaky test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62553/ --- Review request for mesos, Gilbert Song and Jie Yu. Repository: mesos Description --- Fixed a flaky test. Diffs - src/tests/containerizer/cgroups_isolator_tests.cpp 3fc93417f2d3febf2feca3ec1c8476c9edcfbf4d Diff: https://reviews.apache.org/r/62553/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 62252: Added `process::Executor::execute()`.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62252/#review186154
---
Fix it, then Ship it!
Just a few minor cleanups and then I'll commit it.
3rdparty/libprocess/src/tests/process_tests.cpp
Line 1212 (original), 1215 (patched)
<https://reviews.apache.org/r/62252/#comment262599>
Let's use lambdas! I've seen you do this in previous patches and it's just
easier to read (than to have to specify the member function):
```
.WillOnce(InvokeWithoutArgs([&]() {
event1Called.decrement();
}));
```
3rdparty/libprocess/src/tests/process_tests.cpp
Lines 1214-1215 (original), 1217-1218 (patched)
<https://reviews.apache.org/r/62252/#comment262600>
As long as you're moving stuff around, let's move this and the second
`CountDownLatch` down to after the first `AWAIT_READY` too.
3rdparty/libprocess/src/tests/process_tests.cpp
Lines 1252 (patched)
<https://reviews.apache.org/r/62252/#comment262601>
`AWAIT_READY(executor.execute(f1));`
3rdparty/libprocess/src/tests/process_tests.cpp
Lines 1264 (patched)
<https://reviews.apache.org/r/62252/#comment262602>
`AWAIT_READY(executor.execute(std::bind(f2, 42)));`
3rdparty/libprocess/src/tests/process_tests.cpp
Lines 1274-1276 (patched)
<https://reviews.apache.org/r/62252/#comment262603>
We can simplify these next ones: `AWAIT_EXPECT_EQ(f3Result,
executor.execute(f3));`
3rdparty/libprocess/src/tests/process_tests.cpp
Lines 1286-1288 (patched)
<https://reviews.apache.org/r/62252/#comment262604>
`AWAIT_EXPECT_EQ(f4Result, executor.execute(std::bind(f4, 42)));`
- Benjamin Hindman
On Sept. 21, 2017, 8:53 p.m., Chun-Hung Hsiao wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62252/
> ---
>
> (Updated Sept. 21, 2017, 8:53 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Benjamin Mahler, and Michael Park.
>
>
> Bugs: MESOS-7970
> https://issues.apache.org/jira/browse/MESOS-7970
>
>
> Repository: mesos
>
>
> Description
> ---
>
> This patch adds a convenient interface to `process::Executor` to
> asynchronously execute arbitrary functions.
>
>
> Diffs
> -
>
> 3rdparty/libprocess/include/process/executor.hpp
> cd2f2f03cba8a435f142b31def9f89a6cd61af73
> 3rdparty/libprocess/src/tests/process_tests.cpp
> 82efb2f8449e4cb13620cae1a49321fc42e2db60
>
>
> Diff: https://reviews.apache.org/r/62252/diff/6/
>
>
> Testing
> ---
>
> make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 62252: Added `process::Executor::execute()`.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62252/#review185814
---
3rdparty/libprocess/include/process/executor.hpp
Lines 71 (patched)
<https://reviews.apache.org/r/62252/#comment262158>
Let's use the `std::enable_if<..., int>::type = 0` for now since that's
what we've been trying to converge on other places. Here and below, thanks!
3rdparty/libprocess/include/process/executor.hpp
Lines 87 (patched)
<https://reviews.apache.org/r/62252/#comment262159>
Can we also add a TODO that we ultimately want to capture `f` via a
forward/move? Thanks!
Can we do this?
`std::bind([](F&& f) { f(); return Nothing(); }, std::forward(f));`
3rdparty/libprocess/src/tests/process_tests.cpp
Lines 1242 (patched)
<https://reviews.apache.org/r/62252/#comment262161>
Can we update the code to use `CountDownLatch` so we can use
`AWAIT_READY(latch.triggered())` and also pull all the test chunks together
"locally" so that it's easier to see what each piece does. Thanks!
- Benjamin Hindman
On Sept. 15, 2017, 3:17 a.m., Chun-Hung Hsiao wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62252/
> ---
>
> (Updated Sept. 15, 2017, 3:17 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Benjamin Mahler.
>
>
> Bugs: MESOS-7970
> https://issues.apache.org/jira/browse/MESOS-7970
>
>
> Repository: mesos
>
>
> Description
> ---
>
> This patch adds a convenient interface to `process::Executor` to
> asynchronously execute arbitrary functions.
>
>
> Diffs
> -
>
> 3rdparty/libprocess/include/process/executor.hpp
> cd2f2f03cba8a435f142b31def9f89a6cd61af73
> 3rdparty/libprocess/src/tests/process_tests.cpp
> 82efb2f8449e4cb13620cae1a49321fc42e2db60
>
>
> Diff: https://reviews.apache.org/r/62252/diff/5/
>
>
> Testing
> ---
>
> make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 62252: Added `process::Executor::execute()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62252/#review185464 --- 3rdparty/libprocess/include/process/executor.hpp Lines 62 (patched) <https://reviews.apache.org/r/62252/#comment261762> What about a function that returns a `Future` all ready? I think we should flatten the result on behalf of the user, rather than getting back a `Future>`. Let's add a test for this case too please! 3rdparty/libprocess/include/process/executor.hpp Lines 63 (patched) <https://reviews.apache.org/r/62252/#comment261761> Let's still take a universal reference for `F`, e.g., `F&&`. Then we can `std::forward(f)` to `dispatch()`. Same below even though we make a copy, eventually we'll be able to forward it as a lambda capture. 3rdparty/libprocess/include/process/executor.hpp Lines 68-75 (patched) <https://reviews.apache.org/r/62252/#comment261763> Can we comment that this overload for `void` returns `Future` specifically so we can chain? And that it follows the `async()` pattern? It'll be great for future readers (including ourselves!) to understand (or rememeber) why we made this design decision. 3rdparty/libprocess/src/tests/process_tests.cpp Lines 1238 (patched) <https://reviews.apache.org/r/62252/#comment261764> For the non-void returning function it seems unnecessary to also check `f2Result`. I'd suggest we simplify the test so nobody is guessing whether or not that part is really testing anything. And what about using a `Promise` inside the void function instead of a `std::atomic`? Or even a `Latch`? And let's add a test for a function/lambda that already returns a `Future`. - Benjamin Hindman On Sept. 13, 2017, 5:34 p.m., Chun-Hung Hsiao wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62252/ > --- > > (Updated Sept. 13, 2017, 5:34 p.m.) > > > Review request for mesos, Benjamin Hindman and Benjamin Mahler. > > > Bugs: MESOS-7970 > https://issues.apache.org/jira/browse/MESOS-7970 > > > Repository: mesos > > > Description > --- > > This patch adds a convenient interface to `process::Executor` to > asynchronously execute arbitrary functions. > > > Diffs > - > > 3rdparty/libprocess/include/process/executor.hpp > cd2f2f03cba8a435f142b31def9f89a6cd61af73 > 3rdparty/libprocess/src/tests/process_tests.cpp > 82efb2f8449e4cb13620cae1a49321fc42e2db60 > > > Diff: https://reviews.apache.org/r/62252/diff/3/ > > > Testing > --- > > make check > > > Thanks, > > Chun-Hung Hsiao > >
Re: Review Request 62252: Added `process::Executor::execute()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62252/#review185261 --- Thanks for taking this on Chun! A few high level comments to start. (1) I don't think we need to implement a version of `execute()` that takes arguments that we'll apply to the function. With lambda captures we can easily and cleanly accomplish that and any of the corner cases will be cleanly solved for with C++14 which we should move too sooner rather than later. If folks really want that functionality I'd rather just see them use `std::bind()` and do `executor.execute(std::bind(f, arg1, arg2))`. It's not that many more characters and it greately simplifies your implementation! (2) Given the simplification of (1) I feel like you probably don't need a separate `Executor::Process` class, and instead can just use `dispatch()` on the existing `process`. You should be able to simplify your SFINAE by leveraging the return type `dispatch()` since it already takes care of the `void` issue for you. - Benjamin Hindman On Sept. 12, 2017, 8:12 p.m., Chun-Hung Hsiao wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62252/ > --- > > (Updated Sept. 12, 2017, 8:12 p.m.) > > > Review request for mesos, Benjamin Hindman and Benjamin Mahler. > > > Bugs: MESOS-7970 > https://issues.apache.org/jira/browse/MESOS-7970 > > > Repository: mesos > > > Description > --- > > This patch adds a convenient interface to `process::Executor` to > asynchronously execute arbitrary functions. > > > Diffs > - > > 3rdparty/libprocess/include/process/executor.hpp > cd2f2f03cba8a435f142b31def9f89a6cd61af73 > 3rdparty/libprocess/src/tests/process_tests.cpp > 82efb2f8449e4cb13620cae1a49321fc42e2db60 > > > Diff: https://reviews.apache.org/r/62252/diff/2/ > > > Testing > --- > > make check > > > Thanks, > > Chun-Hung Hsiao > >
Review Request 62203: Use a `process::Executor` to ensure safety of asynchronous callbacks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62203/ --- Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone. Repository: mesos Description --- When passing callbacks to `scheduler::Mesos` and `executor::Mesos` from the tests `TestMesos` it's possible that we'll invoke those callbacks erroneously after we've destructed `TestMesos` because the callbacks are executed asynchronously. By using a `process::Executor` we can guarantee that after `TestMesos` is destroyed (and thereby the `process::Executor` instance is destroyed) we won't execute the callbacks. Diffs - src/tests/mesos.hpp 444c75763aea995708db0b17eafee2d22c912554 Diff: https://reviews.apache.org/r/62203/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 62148: Made the `stop()` logic in the scheduler library simpler.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62148/#review184823 --- include/mesos/v1/scheduler.hpp Lines 25 (patched) <https://reviews.apache.org/r/62148/#comment261015> Have we pulled in libprocess dependencies in public headers in the past? - Benjamin Hindman On Sept. 7, 2017, 5:54 a.m., Anand Mazumdar wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62148/ > --- > > (Updated Sept. 7, 2017, 5:54 a.m.) > > > Review request for mesos, Benjamin Hindman, Benjamin Mahler, and Vinod Kone. > > > Repository: mesos > > > Description > --- > > Instead of relying on `Clock::pause()/settle()` to ensure that no > callbacks on the scheduler were in flight, the test > helper can wait on the future returned by `stop()` now. This is > possible as we acquire the mutex explicitly in `stop()` to ensure > that there can't be any queued callbacks that get executed after > `TestMesos` is destroyed. > > > Diffs > - > > include/mesos/v1/scheduler.hpp d56e08861d5190ef66992d383dc5710d8f6ce661 > src/scheduler/scheduler.cpp ce69258027ed50867569374d2d827fc3cc651744 > src/tests/mesos.hpp 444c75763aea995708db0b17eafee2d22c912554 > > > Diff: https://reviews.apache.org/r/62148/diff/1/ > > > Testing > --- > > make check (Ensured tests still work when run in a loop) > > > Thanks, > > Anand Mazumdar > >
Review Request 62053: Removed garbage collector.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62053/ --- Review request for mesos, Benjamin Mahler and Jiang Yan Xu. Bugs: MESOS-7921 https://issues.apache.org/jira/browse/MESOS-7921 Repository: mesos Description --- The garbage collector had at least two bugs: (1) If someone dispatched `manage()` twice in a row the process we're waiting for will get overwritten which can wreak havoc depending on when the calls to `link()` happen. (2) The garbage collector was deleting after an exited event rather than actually doing a `wait()`. The simpler implementation that this patch introduces is to just delete the process in `ProcessManager::cleanup()`. Diffs - 3rdparty/libprocess/include/Makefile.am c5dc0bb0d2d77987531ead50277940700c62b84f 3rdparty/libprocess/include/process/gc.hpp 603bb8bb084a8d2774ab1077da671f659a22a376 3rdparty/libprocess/include/process/process.hpp 8cca782ae89727bc5570afc4ed96c556f14c8c68 3rdparty/libprocess/src/process.cpp afa53537a5c7d4d8b0f4e3b8e04d7d0f2c4c6631 3rdparty/libprocess/src/tests/process_tests.cpp 8d36600701a795a7fa8d73a844657ff98eee6aa7 Diff: https://reviews.apache.org/r/62053/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 61987: Added an undiscardable() helper that blocks discards from propagating.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61987/ --- (Updated Aug. 31, 2017, 4:42 a.m.) Review request for mesos, Benjamin Mahler, Gilbert Song, and Jie Yu. Bugs: MESOS-7926 https://issues.apache.org/jira/browse/MESOS-7926 Repository: mesos Description --- This can be useful in circumstances where you don't want some asynchronous operation to be canceled. Diffs (updated) - 3rdparty/libprocess/include/process/future.hpp 2f5f0a20b5a39b04fd684b1cb44b6a33b647bbef 3rdparty/libprocess/src/tests/future_tests.cpp 0c8725b9a5e64aaac6e3979e450a11e84f9bd45e Diff: https://reviews.apache.org/r/61987/diff/2/ Changes: https://reviews.apache.org/r/61987/diff/1-2/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61987: Added an undiscardable() helper that blocks discards from propagating.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61987/ --- Review request for mesos, Benjamin Mahler, Gilbert Song, and Jie Yu. Bugs: MESOS-7926 https://issues.apache.org/jira/browse/MESOS-7926 Repository: mesos Description --- This can be useful in circumstances where you don't want some asynchronous operation to be canceled. Diffs - 3rdparty/libprocess/include/process/future.hpp 2f5f0a20b5a39b04fd684b1cb44b6a33b647bbef 3rdparty/libprocess/src/tests/future_tests.cpp 0c8725b9a5e64aaac6e3979e450a11e84f9bd45e Diff: https://reviews.apache.org/r/61987/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61611: Added hashset::contains for iterable.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61611/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Added hashset::contains for iterable. Diffs - 3rdparty/stout/include/stout/hashset.hpp d2f7d5ddba62cc87e2b61cb06c86b8ef4cd1a212 Diff: https://reviews.apache.org/r/61611/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61609: Refactored UPID to be immutable and updated libprocess as necessary.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61609/ --- Review request for mesos, Avinash sridharan and Benjamin Mahler. Repository: mesos Description --- UPID has historically been used in a mostly immutable way and with the recent optimizations that introduced the weak reference to the underlying `ProcessBase` enforcing this immutability is critical. This should have the added benefit of improving message passing performance where the same UPIDs are copied into each `Message` since we'll simply be copying a `std::shared_ptr`. Diffs - 3rdparty/libprocess/include/process/pid.hpp 9f09ab46fa3ceaeac09b0fbf9f532728c4ed2d7a 3rdparty/libprocess/include/process/system.hpp 21bd3300b104eaa56642f19c3dcb95950ab94830 3rdparty/libprocess/src/encoder.hpp 70b5ec479e90c0eb6ac729b465739b581729a956 3rdparty/libprocess/src/http.cpp a4d71fb6c345d3c7a7611004830f6c2c0fbf6046 3rdparty/libprocess/src/pid.cpp fdc61b5ab6c75b33ce33de7edd11e9302550f300 3rdparty/libprocess/src/process.cpp dcd9c6738816764aae066fe56cd5f468c98fc9bd 3rdparty/libprocess/src/tests/http_tests.cpp dde05f6a554fcb8c6c89e690bbdcd2bf509854d5 3rdparty/libprocess/src/tests/process_tests.cpp 8d36600701a795a7fa8d73a844657ff98eee6aa7 3rdparty/libprocess/src/tests/test_linkee.cpp cc482717290f72a5fd95fe745ac01893c0ce41f8 Diff: https://reviews.apache.org/r/61609/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61610: Changes in Mesos necessary for UPID immutability.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61610/ --- Review request for mesos, Avinash sridharan and Benjamin Mahler. Repository: mesos Description --- Changes in Mesos necessary for UPID immutability. Diffs - src/common/protobuf_utils.cpp 3ae68e93a985a4cfe23be9c9bd8f92e418102a39 src/executor/executor.cpp 9c14054f2bb08c1041138fb297d29ff6fd807792 src/launcher/default_executor.cpp c25cc941eefc0cca998a99d76497bfdd05babe92 src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 src/scheduler/scheduler.cpp ce69258027ed50867569374d2d827fc3cc651744 src/slave/http.cpp 2d33f0b498c8c819d1aaa6b39ae38b1009fda3e4 src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b src/tests/api_tests.cpp 75f7a585ec75f1d8b78453dd203bb78822ee7bc9 src/tests/containerizer/cni_isolator_tests.cpp 60c85adab11af06be474661544957ca20b7de8c3 src/tests/containerizer/docker_containerizer_tests.cpp c7984592aec2d4e7d1eb7ce077e742c4edc03bb9 src/tests/containerizer/provisioner_appc_tests.cpp 89fe4fcd1b123edc762835595432f24cb699fd61 src/tests/fetcher_cache_tests.cpp 6d212cd460322ad6e97f4cf6ef537323275b7da0 src/tests/fetcher_tests.cpp 01da9fef8e6766cc550d6dc5fe595de86906fb54 src/tests/logging_tests.cpp 886dcd0d6d6ccf509c98578e8b86b1c6ecd5775f src/tests/master_contender_detector_tests.cpp f499136c0b981072af5bc8accf2238b7ba4bf430 src/tests/master_maintenance_tests.cpp e7a80ff4199927df8bf0fb54458d357ae444260d src/tests/master_tests.cpp e6ed02e07a9860ca3a56bba9c502a09444c5b26e src/tests/slave_tests.cpp 1d9d142ed9e801b79535a2c28f5a94ffbf1bf160 src/tests/uri_fetcher_tests.cpp 8a40f46b215bb1f267a59a9edfca83445f86b430 Diff: https://reviews.apache.org/r/61610/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 61410: Remove support for omitting 202 responses to old libprocess clients.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61410/#review182140 --- Fix it, then Ship it! Random question, do you know if there is a test that sends responses back to libprocess to exercise the `process::internal::ignore_recv_data` function? 3rdparty/libprocess/src/tests/process_tests.cpp Lines 1318 (patched) <https://reviews.apache.org/r/61410/#comment257983> Were these actually necessary? I thought this was only necessary after we started to use `http::Server` since without `keepAlive` the `http::Server` will shutdown the socket right away (and `connection.disconnect()` would fail?). - Benjamin Hindman On Aug. 3, 2017, 6:30 p.m., Benjamin Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61410/ > --- > > (Updated Aug. 3, 2017, 6:30 p.m.) > > > Review request for mesos and Benjamin Hindman. > > > Repository: mesos > > > Description > --- > > Prior to commit d5fe51c on April 11 2014, we needed to omit the 202 > responses for libprocess messages because libprocess did not read > the data from its message passing sockets. > > This change removes support for omitting the responses to old clients. > This also means that any 3rdparty libprocess clients (e.g. someone's > go library) need to be correctly reading from their sockets to > communicate with libprocess after this change. > > > Diffs > - > > 3rdparty/libprocess/src/process.cpp > af5a75950bf24059d291acfd48399ab2d55eb8c2 > 3rdparty/libprocess/src/tests/process_tests.cpp > 30d0fb845468ff993e42a5568f57be131f0cd24b > > > Diff: https://reviews.apache.org/r/61410/diff/1/ > > > Testing > --- > > make check > > > Thanks, > > Benjamin Mahler > >
Re: Review Request 61058: Added a lock-free event queue.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61058/ --- (Updated July 29, 2017, 8:52 p.m.) Review request for mesos and Benjamin Mahler. Bugs: MESOS-7798 https://issues.apache.org/jira/browse/MESOS-7798 Repository: mesos Description --- Added a lock-free event queue. Diffs (updated) - 3rdparty/libprocess/configure.ac cb2cf4f32be5cbdf9df1e32f9aaf2bbba0a5ae03 3rdparty/libprocess/src/event_queue.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp b268cdad776a3ca2a87cbe60eb098bde2a70667c Diff: https://reviews.apache.org/r/61058/diff/3/ Changes: https://reviews.apache.org/r/61058/diff/2-3/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 61058: Added a lock-free event queue.
> On July 26, 2017, 2:08 a.m., Benjamin Mahler wrote: > > 3rdparty/libprocess/src/event_queue.hpp > > Lines 127-136 (patched) > > <https://reviews.apache.org/r/61058/diff/2/?file=1781185#file1781185line127> > > > > Hm.. if a thread is checking `empty()` after the sequence is > > incremented and before its decremented back in the decomissioned case, it > > will think incorrectly think it's not empty? > > > > Shouldn't `empty()` check `decomissioned` first, since once set the > > queue is empty or being actively emptied by `decomission()`? This falls into the semantics of the queue being single consumer. The consumer SHOULD NOT be checking if the queue is empty after it has called `decomission()`, hence there shouldn't be a race here. I added quite a few comments to cover this. > On July 26, 2017, 2:08 a.m., Benjamin Mahler wrote: > > 3rdparty/libprocess/src/event_queue.hpp > > Lines 138-145 (patched) > > <https://reviews.apache.org/r/61058/diff/2/?file=1781185#file1781185line138> > > > > Interesting that you took a different strategy here compared to the > > other queue, might be worth a comment saying we have to spin due to the > > nature of `try_dequeue`, and that's ok since the caller *must* check empty > > before calling this? Implemented the TODO I had in the locking implementation and also added more comments to the top of the `EventQueue` class capturing this semantic. > On July 26, 2017, 2:08 a.m., Benjamin Mahler wrote: > > 3rdparty/libprocess/src/event_queue.hpp > > Lines 147-150 (patched) > > <https://reviews.apache.org/r/61058/diff/2/?file=1781185#file1781185line147> > > > > There seems to be a requirement that only the single consumer is the > > one that calls decomission and that once the consumer calls decomission, no > > more empty->dequeue looping will occur. Otherwise, the consumer can loop: > > > > (1) decomission, but not drained yet > > (2) consumer checks empty, returns true since not yet drained by > > decomission > > (3) consumer then calls dequeue > > (4) decomission completes the drain > > (5) consumer loops in dequeue > > > > Or: > > > > (1) decomission, drained > > (2) producer calls enqueue, sequence is incremented > > (3) consumer checks empty, returns true since sequence was incremented > > (5 optional) producer then finishes call to enqueue by decrementing > > sequence > > (4) consumer calls dequeue, loops You got it! > On July 26, 2017, 2:08 a.m., Benjamin Mahler wrote: > > 3rdparty/libprocess/src/event_queue.hpp > > Lines 186-187 (patched) > > <https://reviews.apache.org/r/61058/diff/2/?file=1781185#file1781185line186> > > > > Why 256 instead of unbounded dequeue? I don't know how to do that, I have to pass a value into `try_dequeue_bulk`. > On July 26, 2017, 2:08 a.m., Benjamin Mahler wrote: > > 3rdparty/libprocess/src/event_queue.hpp > > Lines 209-211 (patched) > > <https://reviews.apache.org/r/61058/diff/2/?file=1781185#file1781185line209> > > > > Shouldn't an item smaller than the next entry we expect be something we > > CHECK against rather than silently drop? I added a handful of comments to explain what's going on here! - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61058/#review181297 --- On July 22, 2017, 1:16 a.m., Benjamin Hindman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61058/ > --- > > (Updated July 22, 2017, 1:16 a.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-7798 > https://issues.apache.org/jira/browse/MESOS-7798 > > > Repository: mesos > > > Description > --- > > Added a lock-free event queue. > > > Diffs > - > > 3rdparty/libprocess/configure.ac cb2cf4f32be5cbdf9df1e32f9aaf2bbba0a5ae03 > 3rdparty/libprocess/src/event_queue.hpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/61058/diff/2/ > > > Testing > --- > > make check > > > Thanks, > > Benjamin Hindman > >
Review Request 61198: Added explicit EventQueue::Producer/Consumer interfaces.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61198/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Added explicit EventQueue::Producer/Consumer interfaces. Diffs - 3rdparty/libprocess/src/event_queue.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp b268cdad776a3ca2a87cbe60eb098bde2a70667c Diff: https://reviews.apache.org/r/61198/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61155: Added http::Server.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61155/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Added http::Server. Diffs - 3rdparty/libprocess/include/process/http.hpp f637999174d92a98208b5fc49a65f9929efb11a0 3rdparty/libprocess/src/http.cpp a4d71fb6c345d3c7a7611004830f6c2c0fbf6046 3rdparty/libprocess/src/tests/http_tests.cpp dde05f6a554fcb8c6c89e690bbdcd2bf509854d5 Diff: https://reviews.apache.org/r/61155/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61154: Added safe downcasts for Socket.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61154/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Added safe downcasts for Socket. Diffs - 3rdparty/libprocess/include/process/socket.hpp ae6154d5d142f65352e00f37b4e66d0b62fdc3c2 Diff: https://reviews.apache.org/r/61154/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61147: Added Future::onAbandoned and Future::isAbandoned.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61147/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Added Future::onAbandoned and Future::isAbandoned. Diffs - 3rdparty/libprocess/include/process/future.hpp cce950509f58022e79bb51a6e72ea1a005b9cb50 3rdparty/libprocess/include/process/queue.hpp ab08e30df742412f22a06202526f8b55350ed435 3rdparty/libprocess/src/http.cpp a4d71fb6c345d3c7a7611004830f6c2c0fbf6046 3rdparty/libprocess/src/tests/collect_tests.cpp 155e0bb75cf723a0a6c29020f9f767e3ba3d7401 3rdparty/libprocess/src/tests/future_tests.cpp 0c8725b9a5e64aaac6e3979e450a11e84f9bd45e 3rdparty/libprocess/src/tests/metrics_tests.cpp 161ca0dc7aea526d450d71a80839d8cc075aaa31 3rdparty/libprocess/src/tests/process_tests.cpp ed11909a2a5e3214fa974bdea098f4057cea9666 3rdparty/libprocess/src/tests/shared_tests.cpp 2a2ffe76b7b7ce016b559de7b5d3a28a06f422ef Diff: https://reviews.apache.org/r/61147/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61149: Added Future::condition.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61149/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Added Future::condition. Diffs - 3rdparty/libprocess/include/process/future.hpp cce950509f58022e79bb51a6e72ea1a005b9cb50 Diff: https://reviews.apache.org/r/61149/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61148: Added Future::recover.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61148/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Added Future::recover. Diffs - 3rdparty/libprocess/include/process/future.hpp cce950509f58022e79bb51a6e72ea1a005b9cb50 3rdparty/libprocess/src/tests/future_tests.cpp 0c8725b9a5e64aaac6e3979e450a11e84f9bd45e Diff: https://reviews.apache.org/r/61148/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61157: Refactored ProcessManager::handle for future use with http::Server.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61157/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Refactored ProcessManager::handle for future use with http::Server. Diffs - 3rdparty/libprocess/src/process.cpp b268cdad776a3ca2a87cbe60eb098bde2a70667c 3rdparty/libprocess/src/tests/process_tests.cpp ed11909a2a5e3214fa974bdea098f4057cea9666 Diff: https://reviews.apache.org/r/61157/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61150: Added Future::onAbandoned semantics to process::collect/await.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61150/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Added Future::onAbandoned semantics to process::collect/await. Diffs - 3rdparty/libprocess/include/process/collect.hpp fccf55a26a2ef61fa3b73d100a0741832e4dfa56 3rdparty/libprocess/src/tests/collect_tests.cpp 155e0bb75cf723a0a6c29020f9f767e3ba3d7401 Diff: https://reviews.apache.org/r/61150/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61152: Added overload of process::await that takes and returns single future.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61152/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Added overload of process::await that takes and returns single future. Diffs - 3rdparty/libprocess/include/process/collect.hpp fccf55a26a2ef61fa3b73d100a0741832e4dfa56 3rdparty/libprocess/src/tests/collect_tests.cpp 155e0bb75cf723a0a6c29020f9f767e3ba3d7401 Diff: https://reviews.apache.org/r/61152/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61153: Added discard happens-before relationship in process::await/collect.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61153/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Added discard happens-before relationship in process::await/collect. Diffs - 3rdparty/libprocess/include/process/collect.hpp fccf55a26a2ef61fa3b73d100a0741832e4dfa56 Diff: https://reviews.apache.org/r/61153/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61156: Factored out HttpProxy.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61156/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Factored out HttpProxy. Diffs - 3rdparty/libprocess/Makefile.am 378a4340df56b1d84fbd46f89875ca56ca0a8997 3rdparty/libprocess/src/CMakeLists.txt f97291bd7cadcb440231619a8a2d1029a447ec27 3rdparty/libprocess/src/http_proxy.hpp PRE-CREATION 3rdparty/libprocess/src/http_proxy.cpp PRE-CREATION 3rdparty/libprocess/src/process.cpp b268cdad776a3ca2a87cbe60eb098bde2a70667c 3rdparty/libprocess/src/socket_manager.hpp PRE-CREATION Diff: https://reviews.apache.org/r/61156/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61151: Used Future::recover in Mesos.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61151/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Used Future::recover in Mesos. Diffs - src/slave/containerizer/mesos/io/switchboard.cpp 64180944f8680828781168faa67417489abf1bc8 Diff: https://reviews.apache.org/r/61151/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 61005: Added a test to check for copy assignment of `net::IP::Network`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61005/#review181307 --- Ship it! Ship It! - Benjamin Hindman On July 20, 2017, 6:08 p.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61005/ > --- > > (Updated July 20, 2017, 6:08 p.m.) > > > Review request for mesos and Benjamin Hindman. > > > Bugs: MESOS-7806 > https://issues.apache.org/jira/browse/MESOS-7806 > > > Repository: mesos > > > Description > --- > > Added a test to check for copy assignment of `net::IP::Network`. > > > Diffs > - > > 3rdparty/stout/tests/ip_tests.cpp dba0de6f0b6483f41e10ad38fe0d87946f186b9b > > > Diff: https://reviews.apache.org/r/61005/diff/1/ > > > Testing > --- > > ~/dev/mesosphere/mesos/build$ ./3rdparty/stout/tests/stout-tests > --gtest_filter="NetTest.CopyIPNetwork" > Note: Google Test filter = NetTest.CopyIPNetwork- > [==] Running 1 test from 1 test case. > [--] Global test environment set-up. > [--] 1 test from NetTest > [ RUN ] NetTest.CopyIPNetwork > [ OK ] NetTest.CopyIPNetwork (0 ms) > [--] 1 test from NetTest (0 ms total) > > [--] Global test environment tear-down > [==] 1 test from 1 test case ran. (0 ms total) > [ PASSED ] 1 test. > > > Thanks, > > Avinash sridharan > >
Re: Review Request 61004: Added a copy assignment operator to `net::IP::Network`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61004/#review181306 --- Ship it! Ship It! - Benjamin Hindman On July 20, 2017, 6:06 p.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61004/ > --- > > (Updated July 20, 2017, 6:06 p.m.) > > > Review request for mesos and Benjamin Hindman. > > > Bugs: MESOS-7806 > https://issues.apache.org/jira/browse/MESOS-7806 > > > Repository: mesos > > > Description > --- > > Added a copy assignment operator to `net::IP::Network`. > > > Diffs > - > > 3rdparty/stout/include/stout/ip.hpp > 51000b4f275466d6ed29105c15941c920af73516 > > > Diff: https://reviews.apache.org/r/61004/diff/1/ > > > Testing > --- > > make check > > > Thanks, > > Avinash sridharan > >
Review Request 61069: Introduced an optimized fixed size last-in-first-out semaphore.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61069/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-7798 https://issues.apache.org/jira/browse/MESOS-7798 Repository: mesos Description --- Introduced an optimized fixed size last-in-first-out semaphore. Diffs - 3rdparty/libprocess/src/process.cpp b268cdad776a3ca2a87cbe60eb098bde2a70667c 3rdparty/libprocess/src/run_queue.hpp 109c300b8292f109b699c096eff0c72d674f4f14 3rdparty/libprocess/src/semaphore.hpp 01438838f67e2b3093d95d49931f72888955f11c Diff: https://reviews.apache.org/r/61069/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61068: Added a copy-on-write optimization for UPID 'id' field.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61068/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-7798 https://issues.apache.org/jira/browse/MESOS-7798 Repository: mesos Description --- Added a copy-on-write optimization for UPID 'id' field. Diffs - 3rdparty/libprocess/include/process/pid.hpp 6ed936db2e45f3aab2c3765d57772844e702f914 3rdparty/libprocess/src/pid.cpp 634ac44337980fec03ceadaa81a7557d6a714af8 3rdparty/libprocess/src/process.cpp b268cdad776a3ca2a87cbe60eb098bde2a70667c Diff: https://reviews.apache.org/r/61068/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61067: More optimizations for creating a MessageEvent.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61067/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-7798 https://issues.apache.org/jira/browse/MESOS-7798 Repository: mesos Description --- More optimizations for creating a MessageEvent. Diffs - 3rdparty/libprocess/include/process/event.hpp a0ec05380066e1065af2d9da3c920c8a786b6f06 3rdparty/libprocess/include/process/process.hpp d40179f874754e00b58f271c401650138dc7d01c 3rdparty/libprocess/src/process.cpp b268cdad776a3ca2a87cbe60eb098bde2a70667c Diff: https://reviews.apache.org/r/61067/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61066: Added 'process::workers'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61066/ --- Review request for mesos. Repository: mesos Description --- Added 'process::workers'. Diffs - 3rdparty/libprocess/include/process/process.hpp d40179f874754e00b58f271c401650138dc7d01c 3rdparty/libprocess/src/process.cpp b268cdad776a3ca2a87cbe60eb098bde2a70667c 3rdparty/libprocess/src/tests/benchmarks.cpp 694a842e8e18d82ac551749a71764825ba7cb3a9 Diff: https://reviews.apache.org/r/61066/diff/1/ Testing --- Thanks, Benjamin Hindman
Review Request 61057: Removed ProcessBase::inject to support future event queues.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61057/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-7798 https://issues.apache.org/jira/browse/MESOS-7798 Repository: mesos Description --- Removed ProcessBase::inject to support future event queues. Diffs - 3rdparty/libprocess/include/process/event.hpp a0ec05380066e1065af2d9da3c920c8a786b6f06 3rdparty/libprocess/include/process/process.hpp d40179f874754e00b58f271c401650138dc7d01c 3rdparty/libprocess/src/process.cpp b268cdad776a3ca2a87cbe60eb098bde2a70667c 3rdparty/libprocess/src/tests/process_tests.cpp ed11909a2a5e3214fa974bdea098f4057cea9666 Diff: https://reviews.apache.org/r/61057/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61060: Cached a reference to a ProcessBase in every UPID.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61060/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-7798 https://issues.apache.org/jira/browse/MESOS-7798 Repository: mesos Description --- Cached a reference to a ProcessBase in every UPID. Diffs - 3rdparty/libprocess/include/process/pid.hpp 6ed936db2e45f3aab2c3765d57772844e702f914 3rdparty/libprocess/include/process/process.hpp d40179f874754e00b58f271c401650138dc7d01c 3rdparty/libprocess/src/pid.cpp 634ac44337980fec03ceadaa81a7557d6a714af8 3rdparty/libprocess/src/process.cpp b268cdad776a3ca2a87cbe60eb098bde2a70667c 3rdparty/libprocess/src/process_reference.hpp a6d6d20e3f40634c5e16fd193443e5f1e02faff5 Diff: https://reviews.apache.org/r/61060/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61058: Added a lock-free event queue.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61058/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Added a lock-free event queue. Diffs - 3rdparty/libprocess/configure.ac cb2cf4f32be5cbdf9df1e32f9aaf2bbba0a5ae03 3rdparty/libprocess/src/event_queue.hpp PRE-CREATION Diff: https://reviews.apache.org/r/61058/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61059: Added --enable-lock-free-event-queue configuration in Mesos.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61059/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-7798 https://issues.apache.org/jira/browse/MESOS-7798 Repository: mesos Description --- Added --enable-lock-free-event-queue configuration in Mesos. Diffs - configure.ac b2eeedab65d70718451cb8bbe556794b6d8e8db6 Diff: https://reviews.apache.org/r/61059/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61056: Removed unused mutex.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61056/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-7798 https://issues.apache.org/jira/browse/MESOS-7798 Repository: mesos Description --- Removed unused mutex. Diffs - 3rdparty/libprocess/include/process/process.hpp d40179f874754e00b58f271c401650138dc7d01c 3rdparty/libprocess/src/process.cpp b268cdad776a3ca2a87cbe60eb098bde2a70667c Diff: https://reviews.apache.org/r/61056/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61054: Added a functional "map" abstraction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61054/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Added a functional "map" abstraction. Diffs - 3rdparty/stout/include/stout/lambda.hpp e67acf5e5eac09ac760da0eb711e982605e389db Diff: https://reviews.apache.org/r/61054/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61053: Removed unnecessary states RUNNING and TERMINATED.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61053/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Removed unnecessary states RUNNING and TERMINATED. Diffs - 3rdparty/libprocess/include/process/process.hpp d40179f874754e00b58f271c401650138dc7d01c 3rdparty/libprocess/src/process.cpp b268cdad776a3ca2a87cbe60eb098bde2a70667c Diff: https://reviews.apache.org/r/61053/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 61052: Refactored Gate and updated Gate per Process implementation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61052/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- In the original implementation we needed a Gate to potentially outlive a Process so we stored it in a global map. This patch instead uses std::shared_ptr to simplfy the code. Diffs - 3rdparty/libprocess/include/process/process.hpp d40179f874754e00b58f271c401650138dc7d01c 3rdparty/libprocess/src/gate.hpp a51610e9b20acfe6cd22ea932efd1e6afad84cf2 3rdparty/libprocess/src/process.cpp b268cdad776a3ca2a87cbe60eb098bde2a70667c Diff: https://reviews.apache.org/r/61052/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Review Request 60983: Added a message passing benchmark.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60983/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-7798 https://issues.apache.org/jira/browse/MESOS-7798 Repository: mesos Description --- Added a message passing benchmark. Diffs - 3rdparty/libprocess/src/tests/benchmarks.cpp 8e8f426d3c8ba52f6c91b97fbff8702b9a6f2095 Diff: https://reviews.apache.org/r/60983/diff/1/ Testing --- Thanks, Benjamin Hindman
Review Request 60979: Added --enable-lock-free-run-queue configuration to Mesos.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60979/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Added --enable-lock-free-run-queue configuration to Mesos. Diffs - configure.ac 5f2ef525e5ee00d204342fa8f374ef9807800f95 Diff: https://reviews.apache.org/r/60979/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 60831: Removed extra/unnecessary allocations of Message.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60831/ --- (Updated July 18, 2017, 11:44 p.m.) Review request for mesos and Benjamin Mahler. Bugs: MESOS-7798 https://issues.apache.org/jira/browse/MESOS-7798 Repository: mesos Description --- Removed extra/unnecessary allocations of Message. Diffs (updated) - 3rdparty/libprocess/include/process/event.hpp 8afe6266eb0dc5a17af35d79efb6bfdf9e6a0ee9 3rdparty/libprocess/include/process/gmock.hpp e9af943b39436f365fe687301febb5c7fbefffc4 3rdparty/libprocess/include/process/protobuf.hpp ba6e6d6504250a2609b336f3e9854cfbe5da52ec 3rdparty/libprocess/src/encoder.hpp ea629d72a68e093343562db1ef7e5d00c723f03b 3rdparty/libprocess/src/process.cpp 7ce6d2b13baa68906e091a95c0dd58ee1ca2bc7d 3rdparty/libprocess/src/tests/process_tests.cpp 38d787a083a5eb31e922d283f4b4bed2bd62eb0a 3rdparty/libprocess/src/tests/test_linkee.cpp 77df385d3388788658fa40d033816e1fbb8d8f2c Diff: https://reviews.apache.org/r/60831/diff/2/ Changes: https://reviews.apache.org/r/60831/diff/1-2/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 60830: Replaced std::map with hashmap for ProcessBase::handlers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60830/ --- (Updated July 18, 2017, 11:44 p.m.) Review request for mesos and Benjamin Mahler. Bugs: MESOS-7798 https://issues.apache.org/jira/browse/MESOS-7798 Repository: mesos Description --- Replaced std::map with hashmap for ProcessBase::handlers. Diffs (updated) - 3rdparty/libprocess/include/process/process.hpp 944fcc6449edfd022db4048f70a13aff4a1ff345 Diff: https://reviews.apache.org/r/60830/diff/2/ Changes: https://reviews.apache.org/r/60830/diff/1-2/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 60831: Removed extra/unnecessary allocations of Message.
> On July 15, 2017, 1:25 a.m., Benjamin Mahler wrote: > > 3rdparty/libprocess/src/process.cpp > > Line 455 (original), 455 (patched) > > <https://reviews.apache.org/r/60831/diff/1/?file=1776847#file1776847line455> > > > > Why did you decide to take `Message` here instead of `Message&&`? It > > seems a little odd that both `send` and `send_connect` took a pointer but > > now they've diverged in signature? I cleaned that up just now. We have to do a single copy no matter what, but I think the way I updated the code reads the easiest now. - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60831/#review180623 ------- On July 15, 2017, 12:13 a.m., Benjamin Hindman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60831/ > --- > > (Updated July 15, 2017, 12:13 a.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-7798 > https://issues.apache.org/jira/browse/MESOS-7798 > > > Repository: mesos > > > Description > --- > > Removed extra/unnecessary allocations of Message. > > > Diffs > - > > 3rdparty/libprocess/include/process/event.hpp > 8afe6266eb0dc5a17af35d79efb6bfdf9e6a0ee9 > 3rdparty/libprocess/include/process/gmock.hpp > e9af943b39436f365fe687301febb5c7fbefffc4 > 3rdparty/libprocess/include/process/protobuf.hpp > ba6e6d6504250a2609b336f3e9854cfbe5da52ec > 3rdparty/libprocess/src/encoder.hpp > ea629d72a68e093343562db1ef7e5d00c723f03b > 3rdparty/libprocess/src/process.cpp > 7ce6d2b13baa68906e091a95c0dd58ee1ca2bc7d > 3rdparty/libprocess/src/tests/process_tests.cpp > 38d787a083a5eb31e922d283f4b4bed2bd62eb0a > 3rdparty/libprocess/src/tests/test_linkee.cpp > 77df385d3388788658fa40d033816e1fbb8d8f2c > > > Diff: https://reviews.apache.org/r/60831/diff/1/ > > > Testing > --- > > make check > > > Thanks, > > Benjamin Hindman > >
Re: Review Request 60825: Performance optimizations for message passing.
> On July 15, 2017, 2:08 a.m., Benjamin Mahler wrote: > > 3rdparty/libprocess/src/process.cpp > > Line 560 (original), 561-562 (patched) > > <https://reviews.apache.org/r/60825/diff/1/?file=1776832#file1776832line561> > > > > This seems to suggest there will be a comment about it in the > > run_queue.hpp header but there isn't? Added a comment, thanks! > On July 15, 2017, 2:08 a.m., Benjamin Mahler wrote: > > 3rdparty/libprocess/src/run_queue.hpp > > Lines 73 (patched) > > <https://reviews.apache.org/r/60825/diff/1/?file=1776833#file1776833line73> > > > > const? Agreed. - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60825/#review180626 ------- On July 18, 2017, 10:22 p.m., Benjamin Hindman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60825/ > --- > > (Updated July 18, 2017, 10:22 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-7798 > https://issues.apache.org/jira/browse/MESOS-7798 > > > Repository: mesos > > > Description > --- > > Optimizations include: > > * Factored out run queue and introduced lock free implementation. This > also required adding the concept of an `epoch` to support proper > settling and refactoring the increments/decrements of `running` to > make it easier to reason about. > > * Replaced the use of a condition variable (the `Gate`) with a > semaphore. > > > Diffs > - > > 3rdparty/libprocess/Makefile.am c2190b40b748c19a7db1605f69449cc59f95d1cb > 3rdparty/libprocess/cmake/ProcessConfigure.cmake > 2c46d43ed8418f128f12fad10151700611e81b1e > 3rdparty/libprocess/configure.ac cec01908db73418a549cd79d1d701a19dd623821 > 3rdparty/libprocess/src/CMakeLists.txt > 670dc1510c0ca656abd7f7dfc781f799e54303ec > 3rdparty/libprocess/src/concurrentqueue.hpp PRE-CREATION > 3rdparty/libprocess/src/process.cpp > 7ce6d2b13baa68906e091a95c0dd58ee1ca2bc7d > 3rdparty/libprocess/src/run_queue.hpp PRE-CREATION > 3rdparty/libprocess/src/semaphore.hpp PRE-CREATION > configure.ac 4d7c4a4679e5c624ee750226d542e0d8c228507a > > > Diff: https://reviews.apache.org/r/60825/diff/2/ > > > Testing > --- > > make check > > > Thanks, > > Benjamin Hindman > >
Re: Review Request 60825: Performance optimizations for message passing.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60825/ --- (Updated July 18, 2017, 10:22 p.m.) Review request for mesos and Benjamin Mahler. Summary (updated) - Performance optimizations for message passing. Bugs: MESOS-7798 https://issues.apache.org/jira/browse/MESOS-7798 Repository: mesos Description (updated) --- Optimizations include: * Factored out run queue and introduced lock free implementation. This also required adding the concept of an `epoch` to support proper settling and refactoring the increments/decrements of `running` to make it easier to reason about. * Replaced the use of a condition variable (the `Gate`) with a semaphore. Diffs (updated) - 3rdparty/libprocess/Makefile.am c2190b40b748c19a7db1605f69449cc59f95d1cb 3rdparty/libprocess/cmake/ProcessConfigure.cmake 2c46d43ed8418f128f12fad10151700611e81b1e 3rdparty/libprocess/configure.ac cec01908db73418a549cd79d1d701a19dd623821 3rdparty/libprocess/src/CMakeLists.txt 670dc1510c0ca656abd7f7dfc781f799e54303ec 3rdparty/libprocess/src/concurrentqueue.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 7ce6d2b13baa68906e091a95c0dd58ee1ca2bc7d 3rdparty/libprocess/src/run_queue.hpp PRE-CREATION 3rdparty/libprocess/src/semaphore.hpp PRE-CREATION configure.ac 4d7c4a4679e5c624ee750226d542e0d8c228507a Diff: https://reviews.apache.org/r/60825/diff/2/ Changes: https://reviews.apache.org/r/60825/diff/1-2/ Testing --- make check Thanks, Benjamin Hindman
Review Request 60831: Removed extra/unnecessary allocations of Message.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60831/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Removed extra/unnecessary allocations of Message. Diffs - 3rdparty/libprocess/include/process/event.hpp 8afe6266eb0dc5a17af35d79efb6bfdf9e6a0ee9 3rdparty/libprocess/include/process/gmock.hpp e9af943b39436f365fe687301febb5c7fbefffc4 3rdparty/libprocess/include/process/protobuf.hpp ba6e6d6504250a2609b336f3e9854cfbe5da52ec 3rdparty/libprocess/src/encoder.hpp ea629d72a68e093343562db1ef7e5d00c723f03b 3rdparty/libprocess/src/process.cpp 7ce6d2b13baa68906e091a95c0dd58ee1ca2bc7d 3rdparty/libprocess/src/tests/process_tests.cpp 38d787a083a5eb31e922d283f4b4bed2bd62eb0a 3rdparty/libprocess/src/tests/test_linkee.cpp 77df385d3388788658fa40d033816e1fbb8d8f2c Diff: https://reviews.apache.org/r/60831/diff/1/ Testing --- Thanks, Benjamin Hindman
Review Request 60884: Changes for libprocess Message optimization.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60884/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Changes for libprocess Message optimization. Diffs - src/master/master.cpp 287a5b71bc61949648ac0edff7668f217357a054 Diff: https://reviews.apache.org/r/60884/diff/1/ Testing --- Thanks, Benjamin Hindman
Review Request 60829: Replaced std::map with hashmap for ProcessManager::processes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60829/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Replaced std::map with hashmap for ProcessManager::processes. Diffs - 3rdparty/libprocess/src/process.cpp 7ce6d2b13baa68906e091a95c0dd58ee1ca2bc7d Diff: https://reviews.apache.org/r/60829/diff/1/ Testing --- Thanks, Benjamin Hindman
Review Request 60871: Added double-checked locking for filter.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60871/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- When running in"production" a filter will not be (likely) be set yet with the current code the threads will experience head-of-line blocking becaues they'll all queue up on the mutex. Test code will still queue up but we don't care about the performance in these situations! Note that this patch also moves the filter into ProcessManager in the effort towards eliminating globals. Diffs - 3rdparty/libprocess/include/process/filter.hpp cafa7be065244fe428dee78c5e02e4513f8c1c35 3rdparty/libprocess/src/process.cpp 7ce6d2b13baa68906e091a95c0dd58ee1ca2bc7d Diff: https://reviews.apache.org/r/60871/diff/1/ Testing --- Thanks, Benjamin Hindman
Review Request 60830: Replaced std::map with hashmap for ProcessBase::handlers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60830/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Replaced std::map with hashmap for ProcessBase::handlers. Diffs - 3rdparty/libprocess/include/process/process.hpp 944fcc6449edfd022db4048f70a13aff4a1ff345 3rdparty/libprocess/src/process.cpp 7ce6d2b13baa68906e091a95c0dd58ee1ca2bc7d Diff: https://reviews.apache.org/r/60830/diff/1/ Testing --- Thanks, Benjamin Hindman
Review Request 60832: Added concept of an `epoch` to support proper settling.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60832/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- Also refactored increments/decrements of `running` to make it easier to reason about. Diffs - 3rdparty/libprocess/src/process.cpp 7ce6d2b13baa68906e091a95c0dd58ee1ca2bc7d Diff: https://reviews.apache.org/r/60832/diff/1/ Testing --- make check Thanks, Benjamin Hindman
Re: Review Request 60724: Fixed initialization of `LIBPROCESS_IP6` on agent.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60724/#review180066
---
Ship it!
- Benjamin Hindman
On July 9, 2017, 8:23 p.m., Avinash sridharan wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60724/
> ---
>
> (Updated July 9, 2017, 8:23 p.m.)
>
>
> Review request for mesos, Benjamin Bannier and Benjamin Hindman.
>
>
> Bugs: MESOS-7772
> https://issues.apache.org/jira/browse/MESOS-7772
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Fixed initialization of `LIBPROCESS_IP6`.
>
>
> Diffs
> -
>
> src/slave/main.cpp 358a4394d27d2d123c9cdc9ed3e5295ecbaf9130
>
>
> Diff: https://reviews.apache.org/r/60724/diff/1/
>
>
> Testing
> ---
>
> make check
>
> MANUAL TESTING
>
> ~/dev/mesosphere/mesos$ sudo ./build/bin/mesos-agent.sh --ip=10.0.2.15
> --ip6=1::1 --master=10.0.2.15:5050 --work_dir=/tmp/mesos
> --containerizers=mesos,docker
>
> ~/dev/mesosphere/mesos$ sudo ./build/src/mesos-execute
> --master=10.0.2.15:5050
> --task=file:///home/vagrant/dev/mesos_apps/docker_host.json
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> W0709 20:01:42.282433 4953 scheduler.cpp:169]
> **
> Scheduler driver bound to loopback interface! Cannot communicate with remote
> master(s). You might want to set 'LIBPROCESS_IP' environment variable to use
> a routable IP address.
> **
> I0709 20:01:42.282627 4953 scheduler.cpp:184] Version: 1.4.0
> I0709 20:01:42.300417 4972 scheduler.cpp:470] New master detected at
> master@10.0.2.15:5050
> Subscribed with ID 70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-0002
> Submitted task 'test' to agent '70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-S0'
> Received status update TASK_RUNNING for task 'test_docker'
> source: SOURCE_EXECUTOR
>
> vagrant@centos7:~$ curl http://10.0.2.15:5051/state | jq
> "frameworks": [
> {
> "id": "70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-0001",
> "name": "mesos-execute instance",
> "user": "root",
> "failover_timeout": 0,
> "checkpoint": false,
> "hostname": "",
> "role": "*",
> "executors": [
> {
> "id": "test_docker",
> "name": "Command Executor (Task: test_docker) (Command: NO
> EXECUTABLE)",
> "source": "test_docker",
> "container": "8a51cdfb-2039-407a-942e-52bb66bd1a4c",
> "directory":
> "/tmp/mesos/slaves/70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-S0/frameworks/70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-0001/executors/test_docker/runs/8a51cdfb-2039-407a-942e-52bb66bd1a4c",
> "resources": {
> "disk": 0,
> "mem": 64,
> "gpus": 0,
> "cpus": 0.2
> },
> "role": "*",
> "tasks": [
> {
> "id": "test_docker",
> "name": "test",
> "framework_id": "70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-0001",
> "executor_id": "",
> "slave_id": "70b8df2f-4b14-4597-a3c2-01f16ccd5b6f-S0",
> "state": "TASK_RUNNING",
> "resources": {
> "disk": 0,
> "mem": 32,
> "gpus": 0,
> "cpus": 0.1
> },
> "role": "*",
> "statuses": [
> {
> "state": "TASK_RUNNING",
> "timestamp": 1499630245.73584,
> "container_status": {
> "container_id": {
> "value": "8a51cdfb-2039-407a-942e-52bb66bd1a4c"
> },
>
Re: Review Request 59127: Added IPv6 flags for Mesos agent.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59127/#review179840 --- Ship it! Ship It! - Benjamin Hindman On July 7, 2017, 1:02 a.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59127/ > --- > > (Updated July 7, 2017, 1:02 a.m.) > > > Review request for mesos, Benjamin Hindman and Jie Yu. > > > Bugs: MESOS-7488 > https://issues.apache.org/jira/browse/MESOS-7488 > > > Repository: mesos > > > Description > --- > > Added IPv6 flags for Mesos agent. > > > Diffs > - > > src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 > src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 > > > Diff: https://reviews.apache.org/r/59127/diff/5/ > > > Testing > --- > > make check > > > Thanks, > > Avinash sridharan > >
Re: Review Request 59131: Added an IPv6 address storage to UPID.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59131/#review179835 --- Ship it! Ship It! - Benjamin Hindman On July 6, 2017, 11:31 p.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59131/ > --- > > (Updated July 6, 2017, 11:31 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-7488 > https://issues.apache.org/jira/browse/MESOS-7488 > > > Repository: mesos > > > Description > --- > > Added an IPv6 address storage to UPID. > > > Diffs > - > > 3rdparty/libprocess/include/process/pid.hpp > b0c47ff562c5dfa019f6b392ff269f88a72d40d2 > 3rdparty/libprocess/src/pid.cpp 5191a9dbf478acfa758c602748cafdb296182953 > > > Diff: https://reviews.apache.org/r/59131/diff/6/ > > > Testing > --- > > make check. > > > Thanks, > > Avinash sridharan > >
Re: Review Request 60692: Fixed `net::IPv4` to return `INADDR_ANY` for `ANY()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60692/#review179833 --- Ship it! Ship It! - Benjamin Hindman On July 6, 2017, 11:46 p.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60692/ > --- > > (Updated July 6, 2017, 11:46 p.m.) > > > Review request for mesos and Benjamin Hindman. > > > Bugs: MESOS-7765 > https://issues.apache.org/jira/browse/MESOS-7765 > > > Repository: mesos > > > Description > --- > > Fixed `net::IPv4` to return `INADDR_ANY` for `ANY()`. > > > Diffs > - > > 3rdparty/stout/include/stout/ip.hpp > 25779fa857d138d78c52d1528c070763eba486a1 > > > Diff: https://reviews.apache.org/r/60692/diff/2/ > > > Testing > --- > > make check. > > Also sudo ./build/bin/mesos-tests.sh --gtest_filter=MasterTest.KillUnknownTask > > ``` > [==] Running 1 test from 1 test case. > [--] Global test environment set-up. > [--] 1 test from MasterTest > [ RUN ] MasterTest.KillUnknownTask > [ OK ] MasterTest.KillUnknownTask (87 ms) > [--] 1 test from MasterTest (88 ms total) > > [--] Global test environment tear-down > [==] 1 test from 1 test case ran. (99 ms total) > [ PASSED ] 1 test. > ``` > > > Thanks, > > Avinash sridharan > >
Re: Review Request 60002: Added ENUM_TEMPLATE_PARAMS preprocessor macro.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60002/#review179822 --- Fix it, then Ship it! Happy to ship this, but see comment below. 3rdparty/stout/include/stout/preprocessor.hpp Lines 37 (patched) <https://reviews.apache.org/r/60002/#comment254679> Why make this specific to "templates" and instead just do: ``` #define ENUM BOOST_PP_ENUM ``` - Benjamin Hindman On June 15, 2017, 7:32 a.m., Dmitry Zhuk wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60002/ > --- > > (Updated June 15, 2017, 7:32 a.m.) > > > Review request for mesos, Benjamin Hindman, haosdent huang, James Peach, and > Michael Park. > > > Bugs: MESOS-7713 > https://issues.apache.org/jira/browse/MESOS-7713 > > > Repository: mesos > > > Description > --- > > Added ENUM_TEMPLATE_PARAMS preprocessor macro. > > This allows using templates to generate parameters lists, for example: > #define MOVE_TEMPLATE(Z, N, DATA) std::move(DATA ## N) > > Then it can be used as > call(ENUM_TEMPLATE_PARAMS(2, MOVE_TEMPLATE, p)); > which is expanded as > call(std::move(p1), std::move(p2)); > > > Diffs > - > > 3rdparty/stout/include/stout/preprocessor.hpp > 48d63318712d454b118db64af11c694da92bca6a > > > Diff: https://reviews.apache.org/r/60002/diff/1/ > > > Testing > --- > > > Thanks, > > Dmitry Zhuk > >
Re: Review Request 60551: Added a test for docker container running on a v4/6 host network.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60551/#review179729 --- Fix it, then Ship it! src/tests/containerizer/docker_containerizer_tests.cpp Lines 4348 (patched) <https://reviews.apache.org/r/60551/#comment254572> You can kill the `;` at the end. src/tests/containerizer/docker_containerizer_tests.cpp Lines 4350 (patched) <https://reviews.apache.org/r/60551/#comment254571> You can kill the `;` at the end. src/tests/containerizer/docker_containerizer_tests.cpp Lines 4367-4370 (patched) <https://reviews.apache.org/r/60551/#comment254574> What if the host doesn't have both networks? Do we need more checks in place to keep this from being run on all machines that just have `ROOT_DOCKER`? src/tests/mesos.hpp Lines 248 (patched) <https://reviews.apache.org/r/60551/#comment254567> Comment why this class exists. Also, why doesn't this extend from the test fixture class? I think we should just factor this into the `DockerContainerizerIPv6Test::SetUp` and `DockerContainerizerIPv6Test::TearDown` below for now and also add a comment that we eventually want to pull this out into it's own class. - Benjamin Hindman On June 29, 2017, 11:22 p.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60551/ > --- > > (Updated June 29, 2017, 11:22 p.m.) > > > Review request for mesos, Benjamin Hindman and Jie Yu. > > > Bugs: MESOS-7488 > https://issues.apache.org/jira/browse/MESOS-7488 > > > Repository: mesos > > > Description > --- > > Added a test for docker container running on a v4/6 host network. > > > Diffs > - > > src/tests/containerizer/docker_containerizer_tests.cpp > cf9470245ebc81e799fb2b2a67464298564fc56f > src/tests/mesos.hpp eac2c96985cdbbf1a50cfc054862eae2d44fbfcd > src/tests/mesos.cpp d3e6bc0a92ec9b410712891c6a02d01ed2b587c1 > > > Diff: https://reviews.apache.org/r/60551/diff/2/ > > > Testing > --- > > sudo ./bin/mesos-tests.sh > > > Thanks, > > Avinash sridharan > >
Re: Review Request 59131: Added an IPv6 address storage to UPID.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59131/#review179727
---
3rdparty/libprocess/include/process/pid.hpp
Lines 97 (patched)
<https://reviews.apache.org/r/59131/#comment254564>
```
struct
{
#ifdef __WINDOWS__
Option unix;
#endif // __WINDOWS__
Option v4;
Option v6;
} address;
```
?
- Benjamin Hindman
On June 29, 2017, 11:23 p.m., Avinash sridharan wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59131/
> ---
>
> (Updated June 29, 2017, 11:23 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Added an IPv6 address storage to UPID.
>
>
> Diffs
> -
>
> 3rdparty/libprocess/include/process/pid.hpp
> c634916a37f570194945b9c7d7b786ffeae0408a
> 3rdparty/libprocess/src/pid.cpp 023f881841f466d91243d2773961b67f82da4e91
>
>
> Diff: https://reviews.apache.org/r/59131/diff/5/
>
>
> Testing
> ---
>
> make check.
>
>
> Thanks,
>
> Avinash sridharan
>
>
Re: Review Request 59127: Added IPv6 flags for Mesos agent.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59127/#review179692 --- src/slave/flags.hpp Line 173 (original), 173 (patched) <https://reviews.apache.org/r/59127/#comment254509> Let's keep this generic to IP not IPv4 and add validation like you did in libprocess that we currently only support IPv4. src/slave/flags.hpp Lines 181-182 (original), 181-182 (patched) <https://reviews.apache.org/r/59127/#comment254510> s/IPv4/IP/g src/slave/flags.hpp Lines 187-190 (patched) <https://reviews.apache.org/r/59127/#comment254511> How about printing this out as a warning as part of some validation logic so that users that set this understand that they're not getting what they might think they're getting? src/slave/flags.cpp Lines 1101-1104 (patched) <https://reviews.apache.org/r/59127/#comment254512> I'd like to make the comments be the same as for the `--ip` set of flags with an extra comment that currently the flag is ONLY being used to "advertise containers running on the IPv6 network ..." etc. Because the long term intention is that this will also be used for specifying a flag that we'll bind to? - Benjamin Hindman On June 29, 2017, 11:25 p.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59127/ > --- > > (Updated June 29, 2017, 11:25 p.m.) > > > Review request for mesos, Benjamin Hindman and Jie Yu. > > > Bugs: MESOS-7488 > https://issues.apache.org/jira/browse/MESOS-7488 > > > Repository: mesos > > > Description > --- > > Added IPv6 flags for Mesos agent. > > > Diffs > - > > src/slave/flags.hpp e75c1b4227b443aedf445921b3f2108d930c112c > src/slave/flags.cpp c84aa6724170bba46bbe8410b71d42a1626e > > > Diff: https://reviews.apache.org/r/59127/diff/4/ > > > Testing > --- > > make check > > > Thanks, > > Avinash sridharan > >
Re: Review Request 59233: Updated v6 address for containers running on host network.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59233/#review179691 --- Fix it, then Ship it! src/slave/slave.cpp Lines 4543 (patched) <https://reviews.apache.org/r/59233/#comment254506> s/Setup/Set up/ src/slave/slave.cpp Lines 4545 (patched) <https://reviews.apache.org/r/59233/#comment254507> ... the protocol is set to `IPv4` and therefore we don't explicitly set the protocol here. - Benjamin Hindman On June 29, 2017, 11:22 p.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59233/ > --- > > (Updated June 29, 2017, 11:22 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-7488 > https://issues.apache.org/jira/browse/MESOS-7488 > > > Repository: mesos > > > Description > --- > > Currently the agent is populating only the v4 address for containers > running on the host network. However, clusters running on a dual stack > network need to report v4 and v6 address for containers running on the > host network. This change allows v6 address specified by operators to be > advertised for containers running on the host network. > > > Diffs > - > > include/mesos/mesos.proto 3338349c399c9fb5b7fb2fc5886fb492ffb944c4 > include/mesos/v1/mesos.proto 6f0ad71260c36a9cb401c716e02c561580fb433b > src/slave/slave.cpp 1f7aed48eb3ab3ee2a797d4061a6e289852b8199 > > > Diff: https://reviews.apache.org/r/59233/diff/6/ > > > Testing > --- > > make check > > > Thanks, > > Avinash sridharan > >
Re: Review Request 59130: Added storage for IPv6 in a `libprocess` process.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59130/#review179690 --- Fix it, then Ship it! 3rdparty/libprocess/src/process.cpp Line 170 (original), 172 (patched) <https://reviews.apache.org/r/59130/#comment254504> I like the way you did this where you validated that it's an `IPv4` below but still take the type `IP`, so let's not label this description as "IPv4" but keep it at "IP". 3rdparty/libprocess/src/process.cpp Lines 1392-1393 (patched) <https://reviews.apache.org/r/59130/#comment254505> Can we move this up to just below where we reset `__address__` please? Thanks! - Benjamin Hindman On June 29, 2017, 11:22 p.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59130/ > --- > > (Updated June 29, 2017, 11:22 p.m.) > > > Review request for mesos, Benjamin Hindman and Jie Yu. > > > Bugs: MESOS-7488 > https://issues.apache.org/jira/browse/MESOS-7488 > > > Repository: mesos > > > Description > --- > > Added storage for IPv6 in a `libprocess` process. > > > Diffs > - > > 3rdparty/libprocess/src/process.cpp > 8ff37d1b5781c42f96be2da391ed000158eea7b8 > > > Diff: https://reviews.apache.org/r/59130/diff/6/ > > > Testing > --- > > make check. > > > Thanks, > > Avinash sridharan > >
Re: Review Request 60149: Added support for `net::IPv4` and `net::IPv6` in `FlagsBase`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60149/#review179688 --- Ship it! Ship It! - Benjamin Hindman On June 29, 2017, 11:23 p.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60149/ > --- > > (Updated June 29, 2017, 11:23 p.m.) > > > Review request for mesos, Benjamin Hindman and Jie Yu. > > > Bugs: MESOS-7488 > https://issues.apache.org/jira/browse/MESOS-7488 > > > Repository: mesos > > > Description > --- > > Added support for `net::IPv4` and `net::IPv6` in `FlagsBase`. > > > Diffs > - > > 3rdparty/stout/include/stout/flags/parse.hpp > 3448c571ece36b8d45bbe1317efc824468fe3466 > > > Diff: https://reviews.apache.org/r/60149/diff/3/ > > > Testing > --- > > make check. > > > Thanks, > > Avinash sridharan > >
Re: Review Request 60148: Mesos updates for new inet4::Address.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60148/#review179687 --- Ship it! Ship It! - Benjamin Hindman On June 29, 2017, 11:23 p.m., Avinash sridharan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60148/ > --- > > (Updated June 29, 2017, 11:23 p.m.) > > > Review request for mesos and Benjamin Hindman. > > > Bugs: MESOS-7488 > https://issues.apache.org/jira/browse/MESOS-7488 > > > Repository: mesos > > > Description > --- > > Mesos updates for new inet4::Address. > > > Diffs > - > > src/tests/utils.cpp 053976d3c4cf120ff5e7dff6e96c6862a5b617b1 > > > Diff: https://reviews.apache.org/r/60148/diff/2/ > > > Testing > --- > > make check. > > > Thanks, > > Avinash sridharan > >
