Re: Review Request 36674: Removed pthread and used Latch in executor and scheduler drivers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36674/#review92804 --- Ship it! Ship It! - Joseph Wu On July 21, 2015, 10:56 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36674/ --- (Updated July 21, 2015, 10:56 p.m.) Review request for mesos, Benjamin Hindman and Michael Park. Bugs: MESOS-3120 https://issues.apache.org/jira/browse/MESOS-3120 Repository: mesos Description --- See summary. Diffs - include/mesos/executor.hpp f3cd3ccd505ed4b308a1a42b238fc21fe45cc3b3 include/mesos/scheduler.hpp 9dae0a8f5af168f25eb462e30473f8fd7da50913 src/exec/exec.cpp a1ae074b962d8e93ab7776bd624389857da486f3 src/sched/sched.cpp 25e2d660f4ee4c0b21c887f78ad04819012966f9 Diff: https://reviews.apache.org/r/36674/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 36629: stout: Added support for 'synchronized_wait'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36629/#review92816 --- Ship it! Ship It! - Artem Harutyunyan On July 22, 2015, 12:07 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36629/ --- (Updated July 22, 2015, 12:07 p.m.) Review request for mesos, Benjamin Hindman and Joris Van Remoortere. Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/synchronized.hpp e40ec55f7818fad8703787ecb67869c9e1922e85 Diff: https://reviews.apache.org/r/36629/diff/ Testing --- `make check` Thanks, Michael Park
Re: Review Request 36663: Added ip_address field to MasterInfo
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36663/ --- (Updated July 23, 2015, 9:46 p.m.) Review request for mesos, Anand Mazumdar and Vinod Kone. Changes --- Update description to reflect commit log Bugs: MESOS-2736 https://issues.apache.org/jira/browse/MESOS-2736 Repository: mesos Description (updated) --- Added address field to MasterInfo As part of the new HTTP API and the need to provide a better interface for clients that do not integrate libmesos, we provide the IP address of the Leader Master in the information that gets serialized (in JSON) to ZooKeeper. This will eventually supersede the `ip`, `port` and `hostname` fields that are currently in MasterInfo an which cannot fully support IPv6 addresses. Jira: MESOS-2736 Review: https://reviews.apache.org/r/36663 Diffs - include/mesos/mesos.proto bcb38d934c7f223b9a23746b273e581e0e8da886 src/common/protobuf_utils.cpp e0f82b53f5e106bbf4e21d6ac946df0fae821882 src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0 src/tests/master_tests.cpp 826f276317da2507cdf99b2f2e21a14769d8 Diff: https://reviews.apache.org/r/36663/diff/ Testing --- make check (also tested via [zk-mesos](https://github.com/massenz/zk-mesos) that the information serialized to ZK is readable and as expected). Also ran 2x 0.23 master builds against a 0.24 one with this patch applied; getting both versions in turn to be Leader, and also ran a 0.23 Slave with a 0.24 Leader, and they all recognized each other. Thanks, Marco Massenzio
Re: Review Request 36742: Fixed the sanity check on OSX for persistent volumes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36742/#review92798 --- Ship it! LGTM Could you maybe add a comment or something to indicate why this failed on OSX (and what the fix was), in case someone introduces something similar in future, we'll have a reference to guide us. Thanks for super-quick fix! - Marco Massenzio On July 23, 2015, 7:37 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36742/ --- (Updated July 23, 2015, 7:37 p.m.) Review request for mesos, Ben Mahler, Marco Massenzio, and Vinod Kone. Bugs: MESOS-3138 https://issues.apache.org/jira/browse/MESOS-3138 Repository: mesos Description --- Fixed the sanity check on OSX for persistent volumes. Diffs - src/slave/containerizer/isolators/filesystem/posix.cpp 0c5662741a7562872f0dbfaf9c9b6d923d80af5d Diff: https://reviews.apache.org/r/36742/diff/ Testing --- make check on OSX Thanks, Jie Yu
Re: Review Request 36629: stout: Added support for 'synchronized_wait'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36629/#review92800 --- Ship it! - Joseph Wu On July 22, 2015, 12:07 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36629/ --- (Updated July 22, 2015, 12:07 p.m.) Review request for mesos, Benjamin Hindman and Joris Van Remoortere. Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/synchronized.hpp e40ec55f7818fad8703787ecb67869c9e1922e85 Diff: https://reviews.apache.org/r/36629/diff/ Testing --- `make check` Thanks, Michael Park
Re: Review Request 36663: Added ip_address field to MasterInfo
On July 23, 2015, 11:51 p.m., Vinod Kone wrote: src/tests/master_tests.cpp, line 1022 https://reviews.apache.org/r/36663/diff/2/?file=1020111#file1020111line1022 This test is testing the createMasterInfo() method which is not used in production. Can you also write a test with ZK in play? That is going to test the changes you made in master.cpp. Marco Massenzio wrote: Yep - I had looked in the zookeeper_* tests, but found nothing related to MasterInfo; turns out the tests are in the master_contender.cpp tests. Added one there. Actually, the test is against the 'basic contender' (which, I understand, is not against ZK) - I am assuming this is good enough? we are not really testing the ZK functionality here, only that MasterInfo gets created with the correct data. - Marco --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36663/#review92834 --- On July 24, 2015, 12:17 a.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36663/ --- (Updated July 24, 2015, 12:17 a.m.) Review request for mesos, Anand Mazumdar and Vinod Kone. Bugs: MESOS-2736 https://issues.apache.org/jira/browse/MESOS-2736 Repository: mesos Description --- Added address field to MasterInfo As part of the new HTTP API and the need to provide a better interface for clients that do not integrate libmesos, we provide the IP address of the Leader Master in the information that gets serialized (in JSON) to ZooKeeper. This will eventually supersede the `ip`, `port` and `hostname` fields that are currently in MasterInfo an which cannot fully support IPv6 addresses. Jira: MESOS-2736 Review: https://reviews.apache.org/r/36663 Diffs - include/mesos/mesos.proto e015c81d5052214ef8207642e23b3892a6123c9a src/common/protobuf_utils.cpp e0f82b53f5e106bbf4e21d6ac946df0fae821882 src/master/master.cpp bab04feaf238e51c4d50feb1d3a9c0735ec68fec src/tests/master_contender_detector_tests.cpp d7a3b46b2e437818631064ae34317e49c9aa3748 src/tests/master_tests.cpp 826f276317da2507cdf99b2f2e21a14769d8 Diff: https://reviews.apache.org/r/36663/diff/ Testing --- make check (also tested via [zk-mesos](https://github.com/massenz/zk-mesos) that the information serialized to ZK is readable and as expected). Also ran 2x 0.23 master builds against a 0.24 one with this patch applied; getting both versions in turn to be Leader, and also ran a 0.23 Slave with a 0.24 Leader, and they all recognized each other. Thanks, Marco Massenzio
Re: Review Request 36663: Added ip_address field to MasterInfo
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36663/ --- (Updated July 24, 2015, 12:17 a.m.) Review request for mesos, Anand Mazumdar and Vinod Kone. Changes --- Added test against ZK. Bugs: MESOS-2736 https://issues.apache.org/jira/browse/MESOS-2736 Repository: mesos Description --- Added address field to MasterInfo As part of the new HTTP API and the need to provide a better interface for clients that do not integrate libmesos, we provide the IP address of the Leader Master in the information that gets serialized (in JSON) to ZooKeeper. This will eventually supersede the `ip`, `port` and `hostname` fields that are currently in MasterInfo an which cannot fully support IPv6 addresses. Jira: MESOS-2736 Review: https://reviews.apache.org/r/36663 Diffs (updated) - include/mesos/mesos.proto e015c81d5052214ef8207642e23b3892a6123c9a src/common/protobuf_utils.cpp e0f82b53f5e106bbf4e21d6ac946df0fae821882 src/master/master.cpp bab04feaf238e51c4d50feb1d3a9c0735ec68fec src/tests/master_contender_detector_tests.cpp d7a3b46b2e437818631064ae34317e49c9aa3748 src/tests/master_tests.cpp 826f276317da2507cdf99b2f2e21a14769d8 Diff: https://reviews.apache.org/r/36663/diff/ Testing --- make check (also tested via [zk-mesos](https://github.com/massenz/zk-mesos) that the information serialized to ZK is readable and as expected). Also ran 2x 0.23 master builds against a 0.24 one with this patch applied; getting both versions in turn to be Leader, and also ran a 0.23 Slave with a 0.24 Leader, and they all recognized each other. Thanks, Marco Massenzio
Re: Review Request 36663: Added ip_address field to MasterInfo
On July 23, 2015, 11:51 p.m., Vinod Kone wrote: src/tests/master_tests.cpp, line 1022 https://reviews.apache.org/r/36663/diff/2/?file=1020111#file1020111line1022 This test is testing the createMasterInfo() method which is not used in production. Can you also write a test with ZK in play? That is going to test the changes you made in master.cpp. Yep - I had looked in the zookeeper_* tests, but found nothing related to MasterInfo; turns out the tests are in the master_contender.cpp tests. Added one there. - Marco --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36663/#review92834 --- On July 23, 2015, 11:54 p.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36663/ --- (Updated July 23, 2015, 11:54 p.m.) Review request for mesos, Anand Mazumdar and Vinod Kone. Bugs: MESOS-2736 https://issues.apache.org/jira/browse/MESOS-2736 Repository: mesos Description --- Added address field to MasterInfo As part of the new HTTP API and the need to provide a better interface for clients that do not integrate libmesos, we provide the IP address of the Leader Master in the information that gets serialized (in JSON) to ZooKeeper. This will eventually supersede the `ip`, `port` and `hostname` fields that are currently in MasterInfo an which cannot fully support IPv6 addresses. Jira: MESOS-2736 Review: https://reviews.apache.org/r/36663 Diffs - include/mesos/mesos.proto e015c81d5052214ef8207642e23b3892a6123c9a include/mesos/scheduler/scheduler.proto 89daf8a6b74057ee156b3ad691397e76fcb835b8 src/common/protobuf_utils.cpp e0f82b53f5e106bbf4e21d6ac946df0fae821882 src/master/master.hpp bf61bb2deb61a73303f4122383dcb7e8f5d726de src/master/master.cpp bab04feaf238e51c4d50feb1d3a9c0735ec68fec src/scheduler/scheduler.cpp badc107dbf4e2bd9146f9244724e0db5c2ae05d3 src/tests/master_tests.cpp 826f276317da2507cdf99b2f2e21a14769d8 src/tests/scheduler_tests.cpp 98fc70bf43ba99b54064a236795c7e1269004b71 Diff: https://reviews.apache.org/r/36663/diff/ Testing --- make check (also tested via [zk-mesos](https://github.com/massenz/zk-mesos) that the information serialized to ZK is readable and as expected). Also ran 2x 0.23 master builds against a 0.24 one with this patch applied; getting both versions in turn to be Leader, and also ran a 0.23 Slave with a 0.24 Leader, and they all recognized each other. Thanks, Marco Massenzio
Re: Review Request 36663: Added ip_address field to MasterInfo
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36663/ --- (Updated July 23, 2015, 11:54 p.m.) Review request for mesos, Anand Mazumdar and Vinod Kone. Changes --- Fixed test failure Bugs: MESOS-2736 https://issues.apache.org/jira/browse/MESOS-2736 Repository: mesos Description --- Added address field to MasterInfo As part of the new HTTP API and the need to provide a better interface for clients that do not integrate libmesos, we provide the IP address of the Leader Master in the information that gets serialized (in JSON) to ZooKeeper. This will eventually supersede the `ip`, `port` and `hostname` fields that are currently in MasterInfo an which cannot fully support IPv6 addresses. Jira: MESOS-2736 Review: https://reviews.apache.org/r/36663 Diffs (updated) - include/mesos/mesos.proto e015c81d5052214ef8207642e23b3892a6123c9a include/mesos/scheduler/scheduler.proto 89daf8a6b74057ee156b3ad691397e76fcb835b8 src/common/protobuf_utils.cpp e0f82b53f5e106bbf4e21d6ac946df0fae821882 src/master/master.hpp bf61bb2deb61a73303f4122383dcb7e8f5d726de src/master/master.cpp bab04feaf238e51c4d50feb1d3a9c0735ec68fec src/scheduler/scheduler.cpp badc107dbf4e2bd9146f9244724e0db5c2ae05d3 src/tests/master_tests.cpp 826f276317da2507cdf99b2f2e21a14769d8 src/tests/scheduler_tests.cpp 98fc70bf43ba99b54064a236795c7e1269004b71 Diff: https://reviews.apache.org/r/36663/diff/ Testing --- make check (also tested via [zk-mesos](https://github.com/massenz/zk-mesos) that the information serialized to ZK is readable and as expected). Also ran 2x 0.23 master builds against a 0.24 one with this patch applied; getting both versions in turn to be Leader, and also ran a 0.23 Slave with a 0.24 Leader, and they all recognized each other. Thanks, Marco Massenzio
Re: Review Request 36742: Fixed the sanity check on OSX for persistent volumes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36742/#review92799 --- Patch looks great! Reviews applied: [36742] All tests passed. - Mesos ReviewBot On July 23, 2015, 7:37 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36742/ --- (Updated July 23, 2015, 7:37 p.m.) Review request for mesos, Ben Mahler, Marco Massenzio, and Vinod Kone. Bugs: MESOS-3138 https://issues.apache.org/jira/browse/MESOS-3138 Repository: mesos Description --- Fixed the sanity check on OSX for persistent volumes. Diffs - src/slave/containerizer/isolators/filesystem/posix.cpp 0c5662741a7562872f0dbfaf9c9b6d923d80af5d Diff: https://reviews.apache.org/r/36742/diff/ Testing --- make check on OSX Thanks, Jie Yu
Re: Review Request 36673: Removed pthread from Once and Gate.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36673/#review92802 --- Ship it! Ship It! - Joseph Wu On July 21, 2015, 10:55 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36673/ --- (Updated July 21, 2015, 10:55 p.m.) Review request for mesos, Benjamin Hindman and Michael Park. Bugs: MESOS-3119 https://issues.apache.org/jira/browse/MESOS-3119 Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/include/process/once.hpp 7d0d9309a41a8cc7c074b7566b7b9c411fa8ad45 3rdparty/libprocess/src/gate.hpp 7f6b3d8ddc823f637afcf6dc81b8371e4a794a0b Diff: https://reviews.apache.org/r/36673/diff/ Testing --- make check in libprocess. Thanks, Joris Van Remoortere
Review Request 36755: Passed env vars returned by Isolator::prepare() to executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36755/ --- Review request for mesos and Jie Yu. Bugs: MESOS-3133 https://issues.apache.org/jira/browse/MESOS-3133 Repository: mesos Description --- Append the environment variables returned by Isolator::prepare() to the list of environment variables being passed to the executor. Added a test to verify that the executor is launched with the said environment variables. Diffs - src/slave/containerizer/mesos/containerizer.cpp 609620c4322e41562597ee682b311cd320bca6d2 src/tests/containerizer_tests.cpp 88c46e7214f4d92b7d6a9b433a3a0497681dc6db Diff: https://reviews.apache.org/r/36755/diff/ Testing --- make check with an added test. Thanks, Kapil Arya
Re: Review Request 36718: Converted Limitation and ExecutorRunState structs into protobufs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36718/#review92849 --- Patch looks great! Reviews applied: [36718] All tests passed. - Mesos ReviewBot On July 23, 2015, 11:24 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36718/ --- (Updated July 23, 2015, 11:24 p.m.) Review request for mesos, Benjamin Hindman and Jie Yu. Bugs: MESOS-3115 https://issues.apache.org/jira/browse/MESOS-3115 Repository: mesos Description --- Protobufs are preferred over C structs for public API. Diffs - include/mesos/slave/isolator.hpp 85e38f5e4aa66527f1756fa259b93389f45028b3 include/mesos/slave/isolator.proto PRE-CREATION src/Makefile.am 489ddb424b342635c3dbc4d14ff5d69ce76a237b src/common/protobuf_utils.hpp 2e827a0923de83d5cf853a12435b451cc7c55891 src/common/protobuf_utils.cpp e0f82b53f5e106bbf4e21d6ac946df0fae821882 src/exec/exec.cpp a1ae074b962d8e93ab7776bd624389857da486f3 src/slave/containerizer/isolator.cpp 278824c0cb662bb09150e8cb5a72dafef034e6fb src/slave/containerizer/isolators/cgroups/cpushare.hpp 2118c97d4f3d2e58fab62c6e99d5bec6f4df8bbd src/slave/containerizer/isolators/cgroups/cpushare.cpp 750bef947c11eb55236ac46109b9dd97e62b453d src/slave/containerizer/isolators/cgroups/mem.hpp dc752014236b69d34c6b33a06fc00580780b0b4c src/slave/containerizer/isolators/cgroups/mem.cpp b0e343fdc7088b2895d5dc8bb416dbcbf241cae5 src/slave/containerizer/isolators/cgroups/perf_event.hpp 560cd0359a2795a937335efa30dc380a60fcb540 src/slave/containerizer/isolators/cgroups/perf_event.cpp 512df3be7fdf6bac22ad4122f54a21d9986a1a6a src/slave/containerizer/isolators/filesystem/posix.hpp 16ba26f4f5b515acbeb3c4d514d4eecf2f277df8 src/slave/containerizer/isolators/filesystem/posix.cpp 0c5662741a7562872f0dbfaf9c9b6d923d80af5d src/slave/containerizer/isolators/filesystem/shared.hpp 4d7d9a9e310cb636dc4983de86e9e4c9321104ea src/slave/containerizer/isolators/filesystem/shared.cpp 24f30746be7b5be95a4d8f442a3a5464d1f0f2a0 src/slave/containerizer/isolators/namespaces/pid.hpp 9cda3fd418d0f0df4d12ed1367da277b81866103 src/slave/containerizer/isolators/namespaces/pid.cpp 5de0791a835d725b7c7aae1ba585a94cff9372f1 src/slave/containerizer/isolators/network/port_mapping.hpp 6b5cf62838637e9759935f9edf0c152a035005fe src/slave/containerizer/isolators/network/port_mapping.cpp a7757f2a51f04da27645074f048722c22a2be752 src/slave/containerizer/isolators/posix.hpp 271061ef97aea96bb816982e530c84554d4b08d8 src/slave/containerizer/isolators/posix/disk.hpp 5dfa815edaee9b17c19ae2a7491e63c545c8ab1f src/slave/containerizer/isolators/posix/disk.cpp b2f995cba36b1399db48af1de49d76c607f80abd src/slave/containerizer/launcher.cpp 24df1ca5f7407062f9e7b4bfa18f2cae5c72e140 src/slave/containerizer/linux_launcher.cpp 790e392645dd62e74b03ff0771f6bf0e9efeb622 src/slave/containerizer/mesos/containerizer.hpp f6c580d1b629ee799977cc8824f337764d893c5f src/slave/containerizer/mesos/containerizer.cpp 609620c4322e41562597ee682b311cd320bca6d2 src/tests/containerizer_tests.cpp 88c46e7214f4d92b7d6a9b433a3a0497681dc6db src/tests/isolator.hpp fd6aec74edf49153d31b559d56e8e06c15836f68 Diff: https://reviews.apache.org/r/36718/diff/ Testing --- make check Thanks, Kapil Arya
Review Request 36754: Updated Isolator::prepare to return IsolatorPrepareInfo.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36754/ --- Review request for mesos and Jie Yu. Bugs: MESOS-3133 https://issues.apache.org/jira/browse/MESOS-3133 Repository: mesos Description --- IsolatorPrepareInfo contains CommandInfo as well as Environment. While the CommandInfo lists the commands to be run inside the containerized context, the Environment allows the isolator to specify environment variables for the Executor itself. Diffs - include/mesos/slave/isolator.hpp 85e38f5e4aa66527f1756fa259b93389f45028b3 include/mesos/slave/isolator.proto PRE-CREATION src/slave/containerizer/isolator.cpp 278824c0cb662bb09150e8cb5a72dafef034e6fb src/slave/containerizer/isolators/cgroups/cpushare.hpp 2118c97d4f3d2e58fab62c6e99d5bec6f4df8bbd src/slave/containerizer/isolators/cgroups/cpushare.cpp 750bef947c11eb55236ac46109b9dd97e62b453d src/slave/containerizer/isolators/cgroups/mem.hpp dc752014236b69d34c6b33a06fc00580780b0b4c src/slave/containerizer/isolators/cgroups/mem.cpp b0e343fdc7088b2895d5dc8bb416dbcbf241cae5 src/slave/containerizer/isolators/cgroups/perf_event.hpp 560cd0359a2795a937335efa30dc380a60fcb540 src/slave/containerizer/isolators/cgroups/perf_event.cpp 512df3be7fdf6bac22ad4122f54a21d9986a1a6a src/slave/containerizer/isolators/filesystem/posix.hpp 16ba26f4f5b515acbeb3c4d514d4eecf2f277df8 src/slave/containerizer/isolators/filesystem/posix.cpp 0c5662741a7562872f0dbfaf9c9b6d923d80af5d src/slave/containerizer/isolators/filesystem/shared.hpp 4d7d9a9e310cb636dc4983de86e9e4c9321104ea src/slave/containerizer/isolators/filesystem/shared.cpp 24f30746be7b5be95a4d8f442a3a5464d1f0f2a0 src/slave/containerizer/isolators/namespaces/pid.hpp 9cda3fd418d0f0df4d12ed1367da277b81866103 src/slave/containerizer/isolators/namespaces/pid.cpp 5de0791a835d725b7c7aae1ba585a94cff9372f1 src/slave/containerizer/isolators/network/port_mapping.hpp 6b5cf62838637e9759935f9edf0c152a035005fe src/slave/containerizer/isolators/network/port_mapping.cpp a7757f2a51f04da27645074f048722c22a2be752 src/slave/containerizer/isolators/posix.hpp 271061ef97aea96bb816982e530c84554d4b08d8 src/slave/containerizer/isolators/posix/disk.hpp 5dfa815edaee9b17c19ae2a7491e63c545c8ab1f src/slave/containerizer/isolators/posix/disk.cpp b2f995cba36b1399db48af1de49d76c607f80abd src/slave/containerizer/mesos/containerizer.hpp f6c580d1b629ee799977cc8824f337764d893c5f src/slave/containerizer/mesos/containerizer.cpp 609620c4322e41562597ee682b311cd320bca6d2 src/tests/containerizer_tests.cpp 88c46e7214f4d92b7d6a9b433a3a0497681dc6db src/tests/isolator.hpp fd6aec74edf49153d31b559d56e8e06c15836f68 src/tests/isolator_tests.cpp 7ad0cb643f2add636dfbb4a01495c68b2f4dfdd4 Diff: https://reviews.apache.org/r/36754/diff/ Testing --- make check with an added test (in the followup review). Thanks, Kapil Arya
Re: Review Request 36663: Added ip_address field to MasterInfo
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36663/ --- (Updated July 23, 2015, 8:29 p.m.) Review request for mesos, Anand Mazumdar and Vinod Kone. Changes --- Refactored to use Address; added a unit test. Bugs: MESOS-2736 https://issues.apache.org/jira/browse/MESOS-2736 Repository: mesos Description --- As part of the new HTTP API and the need to provide a better interface for clients that do not integrate libmesos, we provide the IP address of the Leader Master in string format; this will eventually supersed the int32 existing `ip`, which also cannot support IPv6 addresses. Jira: MESOS-2736 Diffs (updated) - include/mesos/mesos.proto bcb38d934c7f223b9a23746b273e581e0e8da886 src/common/protobuf_utils.cpp e0f82b53f5e106bbf4e21d6ac946df0fae821882 src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0 src/tests/master_tests.cpp 826f276317da2507cdf99b2f2e21a14769d8 Diff: https://reviews.apache.org/r/36663/diff/ Testing --- make check (also tested via [zk-mesos](https://github.com/massenz/zk-mesos) that the information serialized to ZK is readable and as expected). Thanks, Marco Massenzio
Re: Review Request 36672: Removed pthread specialization from Synchronized.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36672/#review92817 --- Ship it! Ship It! - Artem Harutyunyan On July 21, 2015, 9:07 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36672/ --- (Updated July 21, 2015, 9:07 p.m.) Review request for mesos, Benjamin Hindman and Michael Park. Bugs: MESOS-3118 https://issues.apache.org/jira/browse/MESOS-3118 Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/synchronized.hpp e40ec55f7818fad8703787ecb67869c9e1922e85 Diff: https://reviews.apache.org/r/36672/diff/ Testing --- Breaking patch, requires dependent patches. Thanks, Joris Van Remoortere
Re: Review Request 36718: Converted Limitation and ExecutorRunState structs into protobufs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36718/ --- (Updated July 23, 2015, 7:24 p.m.) Review request for mesos, Benjamin Hindman and Jie Yu. Changes --- rebased Bugs: MESOS-3115 https://issues.apache.org/jira/browse/MESOS-3115 Repository: mesos Description --- Protobufs are preferred over C structs for public API. Diffs (updated) - include/mesos/slave/isolator.hpp 85e38f5e4aa66527f1756fa259b93389f45028b3 include/mesos/slave/isolator.proto PRE-CREATION src/Makefile.am 489ddb424b342635c3dbc4d14ff5d69ce76a237b src/common/protobuf_utils.hpp 2e827a0923de83d5cf853a12435b451cc7c55891 src/common/protobuf_utils.cpp e0f82b53f5e106bbf4e21d6ac946df0fae821882 src/exec/exec.cpp a1ae074b962d8e93ab7776bd624389857da486f3 src/slave/containerizer/isolator.cpp 278824c0cb662bb09150e8cb5a72dafef034e6fb src/slave/containerizer/isolators/cgroups/cpushare.hpp 2118c97d4f3d2e58fab62c6e99d5bec6f4df8bbd src/slave/containerizer/isolators/cgroups/cpushare.cpp 750bef947c11eb55236ac46109b9dd97e62b453d src/slave/containerizer/isolators/cgroups/mem.hpp dc752014236b69d34c6b33a06fc00580780b0b4c src/slave/containerizer/isolators/cgroups/mem.cpp b0e343fdc7088b2895d5dc8bb416dbcbf241cae5 src/slave/containerizer/isolators/cgroups/perf_event.hpp 560cd0359a2795a937335efa30dc380a60fcb540 src/slave/containerizer/isolators/cgroups/perf_event.cpp 512df3be7fdf6bac22ad4122f54a21d9986a1a6a src/slave/containerizer/isolators/filesystem/posix.hpp 16ba26f4f5b515acbeb3c4d514d4eecf2f277df8 src/slave/containerizer/isolators/filesystem/posix.cpp 0c5662741a7562872f0dbfaf9c9b6d923d80af5d src/slave/containerizer/isolators/filesystem/shared.hpp 4d7d9a9e310cb636dc4983de86e9e4c9321104ea src/slave/containerizer/isolators/filesystem/shared.cpp 24f30746be7b5be95a4d8f442a3a5464d1f0f2a0 src/slave/containerizer/isolators/namespaces/pid.hpp 9cda3fd418d0f0df4d12ed1367da277b81866103 src/slave/containerizer/isolators/namespaces/pid.cpp 5de0791a835d725b7c7aae1ba585a94cff9372f1 src/slave/containerizer/isolators/network/port_mapping.hpp 6b5cf62838637e9759935f9edf0c152a035005fe src/slave/containerizer/isolators/network/port_mapping.cpp a7757f2a51f04da27645074f048722c22a2be752 src/slave/containerizer/isolators/posix.hpp 271061ef97aea96bb816982e530c84554d4b08d8 src/slave/containerizer/isolators/posix/disk.hpp 5dfa815edaee9b17c19ae2a7491e63c545c8ab1f src/slave/containerizer/isolators/posix/disk.cpp b2f995cba36b1399db48af1de49d76c607f80abd src/slave/containerizer/launcher.cpp 24df1ca5f7407062f9e7b4bfa18f2cae5c72e140 src/slave/containerizer/linux_launcher.cpp 790e392645dd62e74b03ff0771f6bf0e9efeb622 src/slave/containerizer/mesos/containerizer.hpp f6c580d1b629ee799977cc8824f337764d893c5f src/slave/containerizer/mesos/containerizer.cpp 609620c4322e41562597ee682b311cd320bca6d2 src/tests/containerizer_tests.cpp 88c46e7214f4d92b7d6a9b433a3a0497681dc6db src/tests/isolator.hpp fd6aec74edf49153d31b559d56e8e06c15836f68 Diff: https://reviews.apache.org/r/36718/diff/ Testing --- make check Thanks, Kapil Arya
Re: Review Request 36741: Add small fixes allowing CMake to build Process lib tests on OS X 10.10
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36741/ --- (Updated July 23, 2015, 11:40 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joris Van Remoortere. Repository: mesos Description --- There are two things that have to change in the current CMake build solution in order to work on OS X 10.10: * librt (which holds the POSIX.1b Realtime Extensions interfaces in Linux, including things like async I/O and memory-mapped files, etc.) does not exist on OS X, and does not need to be linked * The version of GTest attempts to find std::tr1::tuple, which is not available on OS X 10.10. Like we have done in autotools (see[1] and also configure.ac) we simply define `GTEST_USE_OWN_TR1_TUPLE` when we're compiling on OS X. [1] https://mail-archives.apache.org/mod_mbox/mesos-dev/201403.mbox/%3c20140306223933.2625.25...@reviews.apache.org%3E Diffs - 3rdparty/libprocess/3rdparty/CMakeLists.txt 851ab94e4c0f901520c9a4d209f5b9c928116d3c 3rdparty/libprocess/cmake/ProcessConfigure.cmake d6cdd05e281f23f0a57809d3798b5766fb2e9aa6 3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 082489ca4a31843886f764105e70e6af9779ca11 Diff: https://reviews.apache.org/r/36741/diff/ Testing --- Tested fixes on OS X 10.10 and Ubuntu 14 boxes. Thanks, Alex Clemmer
Re: Review Request 36743: Cause CMake to fail and return error code if C++11 not supported
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36743/ --- (Updated July 23, 2015, 11:40 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joris Van Remoortere. Repository: mesos Description --- The current CMake build system will print a warning that your compiler doesn't support C++11, but still return exit code 0. (It also returns a garbled error message.) This commit will cause it to hard-fail the config step, and return an error code. It will also generate a non-garbled error message. Diffs - cmake/MesosConfigure.cmake 16b72f1aa135666174cf10d73d52443d51529c6c Diff: https://reviews.apache.org/r/36743/diff/ Testing --- Tested with gcc/g++ 4.4 on Ubuntu 14. Clearly fails. Thanks, Alex Clemmer
Re: Review Request 36663: Added ip_address field to MasterInfo
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36663/#review92834 --- src/master/master.cpp (lines 344 - 346) https://reviews.apache.org/r/36663/#comment147078 Move this up to #319. src/tests/master_tests.cpp (line 1019) https://reviews.apache.org/r/36663/#comment147080 why not compare ip with master.get().address.ip() ? mesos doesn't necessarily bind to 127.0.0.1 as you might've seen in reviewbot failure. src/tests/master_tests.cpp (line 1021) https://reviews.apache.org/r/36663/#comment147082 ditto. doesn't necessarily map to localhost. src/tests/master_tests.cpp (line 1022) https://reviews.apache.org/r/36663/#comment147083 This test is testing the createMasterInfo() method which is not used in production. Can you also write a test with ZK in play? That is going to test the changes you made in master.cpp. - Vinod Kone On July 23, 2015, 9:46 p.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36663/ --- (Updated July 23, 2015, 9:46 p.m.) Review request for mesos, Anand Mazumdar and Vinod Kone. Bugs: MESOS-2736 https://issues.apache.org/jira/browse/MESOS-2736 Repository: mesos Description --- Added address field to MasterInfo As part of the new HTTP API and the need to provide a better interface for clients that do not integrate libmesos, we provide the IP address of the Leader Master in the information that gets serialized (in JSON) to ZooKeeper. This will eventually supersede the `ip`, `port` and `hostname` fields that are currently in MasterInfo an which cannot fully support IPv6 addresses. Jira: MESOS-2736 Review: https://reviews.apache.org/r/36663 Diffs - include/mesos/mesos.proto bcb38d934c7f223b9a23746b273e581e0e8da886 src/common/protobuf_utils.cpp e0f82b53f5e106bbf4e21d6ac946df0fae821882 src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0 src/tests/master_tests.cpp 826f276317da2507cdf99b2f2e21a14769d8 Diff: https://reviews.apache.org/r/36663/diff/ Testing --- make check (also tested via [zk-mesos](https://github.com/massenz/zk-mesos) that the information serialized to ZK is readable and as expected). Also ran 2x 0.23 master builds against a 0.24 one with this patch applied; getting both versions in turn to be Leader, and also ran a 0.23 Slave with a 0.24 Leader, and they all recognized each other. Thanks, Marco Massenzio
Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36620/ --- (Updated July 23, 2015, 12:21 p.m.) Review request for mesos and Timothy Chen. Changes --- Improved comments. Bugs: MESOS-3086 https://issues.apache.org/jira/browse/MESOS-3086 Repository: mesos Description --- WIP Added Non-Freezeer Task Killer. Diffs (updated) - src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 Diff: https://reviews.apache.org/r/36620/diff/ Testing --- sudo make check Thanks, Joerg Schad
Re: Review Request 36617: Improved task reconciliation documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36617/#review92742 --- Patch looks great! Reviews applied: [36617] All tests passed. - Mesos ReviewBot On July 23, 2015, 12:01 p.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36617/ --- (Updated July 23, 2015, 12:01 p.m.) Review request for mesos and Joerg Schad. Bugs: MESOS-3127 https://issues.apache.org/jira/browse/MESOS-3127 Repository: mesos Description --- Improved task reconciliation documentation. Diffs - docs/reconciliation.md 17537ba8420c95d833e64ccf82ff9bb4530497f0 Diff: https://reviews.apache.org/r/36617/diff/ Testing --- https://gist.github.com/nfnt/73532d62fe39d27ff33d Thanks, Jan Schlicht
Review Request 36763: Made the Isolator interface not dependent on IsolatorProcess.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36763/ --- Review request for mesos, Ben Mahler, Kapil Arya, and Vinod Kone. Bugs: MESOS-3130 https://issues.apache.org/jira/browse/MESOS-3130 Repository: mesos Description --- Made the Isolator interface not dependent on IsolatorProcess. Diffs - include/mesos/slave/isolator.hpp 8387efd39a0e868038350ab620b38e4ab89327db src/Makefile.am 9f2d7e38b28c1208cd819d2470c7f6ec6d5a71f9 src/slave/containerizer/isolator.hpp PRE-CREATION src/slave/containerizer/isolator.cpp ec14b207be36af7ca601e0f327e7025a155f41e4 src/slave/containerizer/isolators/cgroups/cpushare.hpp 4fa9015e445872ec4a060e62dc2b5f2b30fc0f47 src/slave/containerizer/isolators/cgroups/cpushare.cpp b1ebdaddc211bab023be85084bc82b6b82093f0b src/slave/containerizer/isolators/cgroups/mem.hpp c198c832f8fb3765e932f1f20d3e16524340a7f2 src/slave/containerizer/isolators/cgroups/mem.cpp 919e0f7eccff1d4c10852260965bda2bdb1c4267 src/slave/containerizer/isolators/cgroups/perf_event.hpp 243cf5a6ff699aa37f885cc164977126413d155b src/slave/containerizer/isolators/cgroups/perf_event.cpp 367cb4364212ce057a474b574b8dbf8ccc1ae235 src/slave/containerizer/isolators/filesystem/posix.hpp d44023eb95c48d6ffbd69334f91a2b4c4b760db0 src/slave/containerizer/isolators/filesystem/posix.cpp 72d273897630b5861db0c2d55a26e9dab64f646f src/slave/containerizer/isolators/filesystem/shared.hpp 4a5dcc35b6aea665893a1406fe4e2a97cf4e5d1a src/slave/containerizer/isolators/filesystem/shared.cpp f90045e7696b54b7b5082b9aa16c78d0347afde6 src/slave/containerizer/isolators/namespaces/pid.hpp 702f331d39acc75e9c98183c30227ec47f10e904 src/slave/containerizer/isolators/namespaces/pid.cpp 4241fa7a917ab31f52ece4caf16e94b3ef9ac353 src/slave/containerizer/isolators/network/port_mapping.hpp 6ffd7290eef2f0c3d3dc1c6236248e498c5be6ca src/slave/containerizer/isolators/network/port_mapping.cpp 39d1813ee534df184cd5cf2b11af22e0195b0802 src/slave/containerizer/isolators/posix.hpp 6ddab7df489feb2a7762178183191d5d1dc99e15 src/slave/containerizer/isolators/posix/disk.hpp fdf24a18bfca97863b7c15c39378b879d7b43517 src/slave/containerizer/isolators/posix/disk.cpp 238f17941712e0709eba1cf78c6bc825b3b4459e src/slave/containerizer/mesos/containerizer.cpp c21e925e9584bc914f492d7f459498f732779fc8 src/tests/containerizer_tests.cpp 0b1338107ebe7b78821fddbefec90524f35e3858 src/tests/isolator.hpp 671b02141c0afde8718b81dddc7629bb1b9d00fb Diff: https://reviews.apache.org/r/36763/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 36742: Fixed the sanity check on OSX for persistent volumes.
On July 23, 2015, 8:15 p.m., Marco Massenzio wrote: LGTM Could you maybe add a comment or something to indicate why this failed on OSX (and what the fix was), in case someone introduces something similar in future, we'll have a reference to guide us. Thanks for super-quick fix! Added more comments. - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36742/#review92798 --- On July 23, 2015, 7:37 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36742/ --- (Updated July 23, 2015, 7:37 p.m.) Review request for mesos, Ben Mahler, Marco Massenzio, and Vinod Kone. Bugs: MESOS-3138 https://issues.apache.org/jira/browse/MESOS-3138 Repository: mesos Description --- Fixed the sanity check on OSX for persistent volumes. Diffs - src/slave/containerizer/isolators/filesystem/posix.cpp 0c5662741a7562872f0dbfaf9c9b6d923d80af5d Diff: https://reviews.apache.org/r/36742/diff/ Testing --- make check on OSX Thanks, Jie Yu
Re: Review Request 36715: Used std::thread instead of pthread for Libprocess process.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36715/#review92864 --- Ship it! Ship It! - Artem Harutyunyan On July 23, 2015, 12:49 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36715/ --- (Updated July 23, 2015, 12:49 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park. Bugs: MESOS-3119 https://issues.apache.org/jira/browse/MESOS-3119 Repository: mesos Description --- See summary Diffs - 3rdparty/libprocess/src/event_loop.hpp 45aa2a6019dc08da287b7c8a9d799619deb69747 3rdparty/libprocess/src/libev.cpp 8960c75010e3ceca3486a1fbb0fdeeffd62f4ec8 3rdparty/libprocess/src/libevent.cpp 02f9e61c1357e7b450e53b252b49d1979e782ae8 3rdparty/libprocess/src/process.cpp d6b0d5517964bf7c7276689e801a38b34a3f9db2 Diff: https://reviews.apache.org/r/36715/diff/ Testing --- make check with libev, libevent Thanks, Joris Van Remoortere
Review Request 36757: Added https support.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36757/ --- Review request for mesos, Joris Van Remoortere and Timothy Chen. Bugs: MESOS-3093 https://issues.apache.org/jira/browse/MESOS-3093 Repository: mesos Description --- Current http implementation lacks a https interface. This change exposes SSL socket for https URL scheme. JIRA: MESOS-3093 Diffs - 3rdparty/libprocess/src/http.cpp d1685799f4c53e067d0812e037e171324ee7195f 3rdparty/libprocess/src/tests/ssl_tests.cpp 2fe50601615b0bee57bd3e05dc9c932f93ca7477 Diff: https://reviews.apache.org/r/36757/diff/ Testing --- make check Thanks, Jojy Varghese
Review Request 36761: Added tests for unset framework pid in the slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36761/ --- Review request for mesos, Anand Mazumdar and Vinod Kone. Bugs: MESOS-3132 https://issues.apache.org/jira/browse/MESOS-3132 Repository: mesos Description --- Since we do not yet have HTTP schedulers, this adds tests that spoof empty pids coming from the master. Diffs - src/tests/mesos.hpp 69134e1c2664ca24a1ecd80a662c841311104a6a src/tests/slave_tests.cpp e1390ad84b0003052681600deb9ca518defc0970 Diff: https://reviews.apache.org/r/36761/diff/ Testing --- Ran in repetition. Thanks, Ben Mahler
Review Request 36760: Handle scheduler pid as optional in the slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36760/ --- Review request for mesos, Anand Mazumdar and Vinod Kone. Bugs: MESOS-3132 https://issues.apache.org/jira/browse/MESOS-3132 Repository: mesos Description --- This is anticipation of HTTP scheduler support in 0.24.0. Note that the 'pid' is set for driver-based schedulers. The corresponding master changes to not set 'pid' for HTTP schedulers have not occurred yet. Diffs - src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0 src/messages/messages.proto 165a16d91616b75c839c7fbf188dc49f029a8c45 src/slave/slave.hpp dec4ca8323e151a6d0f9139214ff0ef6e3e3375a src/slave/slave.cpp dc12c45516ab39d74a5c29b657f22f74d0acf24e src/slave/state.hpp 4e00468a777145e3c61b8dee7dfe496f8d65b0e4 src/tests/mesos.hpp 69134e1c2664ca24a1ecd80a662c841311104a6a src/tests/mesos.cpp f09ef0f99573716de8905f49dcc0c9df20e31ea9 src/tests/slave_tests.cpp e1390ad84b0003052681600deb9ca518defc0970 Diff: https://reviews.apache.org/r/36760/diff/ Testing --- Added tests in [r/36761](https://reviews.apache.org/r/36761/). Thanks, Ben Mahler
Review Request 36759: Added an ExecutorToFramework message handler on the master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36759/ --- Review request for mesos, Anand Mazumdar and Vinod Kone. Bugs: MESOS-3132 https://issues.apache.org/jira/browse/MESOS-3132 Repository: mesos Description --- This enables the slave to forward messages through the master. Diffs - src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0 src/master/metrics.hpp d37d74a472a3e5605a4689420513f56e713aa8f3 src/master/metrics.cpp 10e2937c2364b4025ec44ddfe47c3cb958a62773 src/slave/slave.cpp dc12c45516ab39d74a5c29b657f22f74d0acf24e Diff: https://reviews.apache.org/r/36759/diff/ Testing --- Added tests in [r/36761](https://reviews.apache.org/r/36761/). Thanks, Ben Mahler
Re: Review Request 36755: Passed env vars returned by Isolator::prepare() to executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36755/#review92856 --- Patch looks great! Reviews applied: [36718, 36754, 36755] All tests passed. - Mesos ReviewBot On July 24, 2015, 12:20 a.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36755/ --- (Updated July 24, 2015, 12:20 a.m.) Review request for mesos and Jie Yu. Bugs: MESOS-3133 https://issues.apache.org/jira/browse/MESOS-3133 Repository: mesos Description --- Append the environment variables returned by Isolator::prepare() to the list of environment variables being passed to the executor. Added a test to verify that the executor is launched with the said environment variables. Diffs - src/slave/containerizer/mesos/containerizer.cpp 609620c4322e41562597ee682b311cd320bca6d2 src/tests/containerizer_tests.cpp 88c46e7214f4d92b7d6a9b433a3a0497681dc6db Diff: https://reviews.apache.org/r/36755/diff/ Testing --- make check with an added test. Thanks, Kapil Arya
Re: Review Request 36627: WIP: Fixed cgroups oom killer test on Ubuntu 14.04
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36627/#review92862 --- src/tests/memory_test_helper.cpp (line 87) https://reviews.apache.org/r/36627/#comment147116 micro-nit: missing an `a` in `posix_memlign` - Marco Massenzio On July 23, 2015, 5:38 p.m., Artem Harutyunyan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36627/ --- (Updated July 23, 2015, 5:38 p.m.) Review request for mesos, Benjamin Hindman and Joris Van Remoortere. Bugs: MESOS-2660 https://issues.apache.org/jira/browse/MESOS-2660 Repository: mesos Description --- See summary. Diffs - src/tests/memory_test_helper.cpp 8093e66c45cacc4bf0295a3448c128a1aa7c751a Diff: https://reviews.apache.org/r/36627/diff/ Testing --- It seems there is still one more cgroups memory test failing on more test failing on my box. I'd like to fix that too and commit it together with this one. sudo make check Verified that the process actually gets killed by oom-killer: ``` # tail -f /var/log/syslog Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052405] lt-memory-test- invoked oom-killer: gfp_mask=0xd0, order=0, oom_score_adj=0 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052408] lt-memory-test- cpuset=/ mems_allowed=0 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052411] CPU: 7 PID: 76599 Comm: lt-memory-test- Tainted: G OE 3.16.0-41-generic #57~14.04.1-Ubuntu Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052413] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 05/20/2014 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052413] 88022efc1000 8801fd2efc30 81765721 880231f10a30 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052415] 8801fd2efcb8 8175f2d5 8802366f30c0 8801e9405b00 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052416] 8801fd2efc80 81165067 880231f10ee8 880231f10a30 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052418] Call Trace: Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052424] [81765721] dump_stack+0x45/0x56 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052426] [8175f2d5] dump_header+0x7f/0x1f1 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052430] [81165067] ? find_lock_task_mm+0x47/0xa0 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052432] [811654e5] oom_kill_process+0x205/0x360 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052434] [812eb975] ? security_capable_noaudit+0x15/0x20 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052437] [811ca2e1] mem_cgroup_oom_synchronize+0x581/0x5e0 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052439] [811c97c0] ? mem_cgroup_try_charge_mm+0xa0/0xa0 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052440] [81165ce4] pagefault_out_of_memory+0x14/0x80 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052442] [8175d97f] mm_fault_error+0x67/0x140 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052445] [8105b28c] __do_page_fault+0x4ec/0x560 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052449] [810a6208] ? __enqueue_entity+0x78/0x80 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052450] [810a7f35] ? set_next_entity+0x95/0xb0 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052452] [81011627] ? __switch_to+0x167/0x580 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052454] [8105b331] do_page_fault+0x31/0x70 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052456] [8176fe68] page_fault+0x28/0x30 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052457] Task in /mesos_test killed as a result of limit of /mesos_test Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052459] memory: usage 65536kB, limit 65536kB, failcnt 24 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052460] memory+swap: usage 0kB, limit 18014398509481983kB, failcnt 0 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052461] kmem: usage 0kB, limit 18014398509481983kB, failcnt 0 Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052461] Memory cgroup stats for /mesos_test: cache:0KB rss:65536KB rss_huge:63488KB mapped_file:0KB writeback:0KB inactive_anon:0KB active_anon:0KB
Re: Review Request 36663: Added ip_address field to MasterInfo
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36663/#review92853 --- Patch looks great! Reviews applied: [36663] All tests passed. - Mesos ReviewBot On July 24, 2015, 12:17 a.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36663/ --- (Updated July 24, 2015, 12:17 a.m.) Review request for mesos, Anand Mazumdar and Vinod Kone. Bugs: MESOS-2736 https://issues.apache.org/jira/browse/MESOS-2736 Repository: mesos Description --- Added address field to MasterInfo As part of the new HTTP API and the need to provide a better interface for clients that do not integrate libmesos, we provide the IP address of the Leader Master in the information that gets serialized (in JSON) to ZooKeeper. This will eventually supersede the `ip`, `port` and `hostname` fields that are currently in MasterInfo an which cannot fully support IPv6 addresses. Jira: MESOS-2736 Review: https://reviews.apache.org/r/36663 Diffs - include/mesos/mesos.proto e015c81d5052214ef8207642e23b3892a6123c9a src/common/protobuf_utils.cpp e0f82b53f5e106bbf4e21d6ac946df0fae821882 src/master/master.cpp bab04feaf238e51c4d50feb1d3a9c0735ec68fec src/tests/master_contender_detector_tests.cpp d7a3b46b2e437818631064ae34317e49c9aa3748 src/tests/master_tests.cpp 826f276317da2507cdf99b2f2e21a14769d8 Diff: https://reviews.apache.org/r/36663/diff/ Testing --- make check (also tested via [zk-mesos](https://github.com/massenz/zk-mesos) that the information serialized to ZK is readable and as expected). Also ran 2x 0.23 master builds against a 0.24 one with this patch applied; getting both versions in turn to be Leader, and also ran a 0.23 Slave with a 0.24 Leader, and they all recognized each other. Thanks, Marco Massenzio
Re: Review Request 36757: Added https support.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36757/ --- (Updated July 24, 2015, 2:28 a.m.) Review request for mesos, Joris Van Remoortere and Timothy Chen. Changes --- review comments; added test for post. Bugs: MESOS-3093 https://issues.apache.org/jira/browse/MESOS-3093 Repository: mesos Description --- Current http implementation lacks a https interface. This change exposes SSL socket for https URL scheme. JIRA: MESOS-3093 Diffs (updated) - 3rdparty/libprocess/src/http.cpp d1685799f4c53e067d0812e037e171324ee7195f 3rdparty/libprocess/src/tests/ssl_tests.cpp 2fe50601615b0bee57bd3e05dc9c932f93ca7477 Diff: https://reviews.apache.org/r/36757/diff/ Testing --- make check Thanks, Jojy Varghese
Re: Review Request 36759: Added an ExecutorToFramework message handler on the master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36759/#review92859 --- Ship it! src/master/master.cpp (line 716) https://reviews.apache.org/r/36759/#comment147114 Can you also add why we added it ? - Anand Mazumdar On July 24, 2015, 1:45 a.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36759/ --- (Updated July 24, 2015, 1:45 a.m.) Review request for mesos, Anand Mazumdar and Vinod Kone. Bugs: MESOS-3132 https://issues.apache.org/jira/browse/MESOS-3132 Repository: mesos Description --- This enables the slave to forward messages through the master. Diffs - src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0 src/master/metrics.hpp d37d74a472a3e5605a4689420513f56e713aa8f3 src/master/metrics.cpp 10e2937c2364b4025ec44ddfe47c3cb958a62773 src/slave/slave.cpp dc12c45516ab39d74a5c29b657f22f74d0acf24e Diff: https://reviews.apache.org/r/36759/diff/ Testing --- Added tests in [r/36761](https://reviews.apache.org/r/36761/). Thanks, Ben Mahler
Re: Review Request 36757: Added https support.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36757/#review92861 --- Patch looks great! Reviews applied: [36712, 36757] All tests passed. - Mesos ReviewBot On July 24, 2015, 2:28 a.m., Jojy Varghese wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36757/ --- (Updated July 24, 2015, 2:28 a.m.) Review request for mesos, Joris Van Remoortere and Timothy Chen. Bugs: MESOS-3093 https://issues.apache.org/jira/browse/MESOS-3093 Repository: mesos Description --- Current http implementation lacks a https interface. This change exposes SSL socket for https URL scheme. JIRA: MESOS-3093 Diffs - 3rdparty/libprocess/src/http.cpp d1685799f4c53e067d0812e037e171324ee7195f 3rdparty/libprocess/src/tests/ssl_tests.cpp 2fe50601615b0bee57bd3e05dc9c932f93ca7477 Diff: https://reviews.apache.org/r/36757/diff/ Testing --- make check Thanks, Jojy Varghese
Review Request 36742: Fixed the sanity check on OSX for persistent volumes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36742/ --- Review request for mesos, Ben Mahler, Marco Massenzio, and Vinod Kone. Bugs: MESOS-3138 https://issues.apache.org/jira/browse/MESOS-3138 Repository: mesos Description --- Fixed the sanity check on OSX for persistent volumes. Diffs - src/slave/containerizer/isolators/filesystem/posix.cpp 0c5662741a7562872f0dbfaf9c9b6d923d80af5d Diff: https://reviews.apache.org/r/36742/diff/ Testing --- make check on OSX Thanks, Jie Yu
Re: Review Request 36715: Used std::thread instead of pthread for Libprocess process.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36715/ --- (Updated July 23, 2015, 7:49 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park. Changes --- renamed `schedule` to `process_schedule` as gcc-4.8 has a name resolution problem with `schedule` Bugs: MESOS-3119 https://issues.apache.org/jira/browse/MESOS-3119 Repository: mesos Description --- See summary Diffs (updated) - 3rdparty/libprocess/src/event_loop.hpp 45aa2a6019dc08da287b7c8a9d799619deb69747 3rdparty/libprocess/src/libev.cpp 8960c75010e3ceca3486a1fbb0fdeeffd62f4ec8 3rdparty/libprocess/src/libevent.cpp 02f9e61c1357e7b450e53b252b49d1979e782ae8 3rdparty/libprocess/src/process.cpp d6b0d5517964bf7c7276689e801a38b34a3f9db2 Diff: https://reviews.apache.org/r/36715/diff/ Testing --- make check with libev, libevent Thanks, Joris Van Remoortere
Re: Review Request 36733: Moved framework struct in master to end of file
On July 23, 2015, 7:25 p.m., Joseph Wu wrote:
Is there anything preventing you from using forward declarations? Or just
implementing the templated send function after the Master has been defined?
i.e.
```
struct Framework {
...
template typename Message
void send(const Message message);
...
}
class Master { ... }
template typename Message
void Framework::send(const Message message) {
...
}
```
For some reason , the forward declaration did not work for me and the compiler
barfed out. ( might have to do with this class being a friend of master to
invoke the protected send(...) method ). I did not dig into it much.
- Anand
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36733/#review92792
---
On July 23, 2015, 5:26 p.m., Anand Mazumdar wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36733/
---
(Updated July 23, 2015, 5:26 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Repository: mesos
Description
---
Moved framework struct to end of file since the framework struct needs to
know about the implementation of master for invoking templated send(...).
Needed for r36318.
Diffs
-
src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec
Diff: https://reviews.apache.org/r/36733/diff/
Testing
---
make check
Thanks,
Anand Mazumdar
Re: Review Request 36674: Removed pthread and used Latch in executor and scheduler drivers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36674/#review92815 --- Ship it! Ship It! - Artem Harutyunyan On July 21, 2015, 10:56 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36674/ --- (Updated July 21, 2015, 10:56 p.m.) Review request for mesos, Benjamin Hindman and Michael Park. Bugs: MESOS-3120 https://issues.apache.org/jira/browse/MESOS-3120 Repository: mesos Description --- See summary. Diffs - include/mesos/executor.hpp f3cd3ccd505ed4b308a1a42b238fc21fe45cc3b3 include/mesos/scheduler.hpp 9dae0a8f5af168f25eb462e30473f8fd7da50913 src/exec/exec.cpp a1ae074b962d8e93ab7776bd624389857da486f3 src/sched/sched.cpp 25e2d660f4ee4c0b21c887f78ad04819012966f9 Diff: https://reviews.apache.org/r/36674/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 36672: Removed pthread specialization from Synchronized.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36672/#review92801 --- Ship it! Ship It! - Joseph Wu On July 21, 2015, 9:07 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36672/ --- (Updated July 21, 2015, 9:07 p.m.) Review request for mesos, Benjamin Hindman and Michael Park. Bugs: MESOS-3118 https://issues.apache.org/jira/browse/MESOS-3118 Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/synchronized.hpp e40ec55f7818fad8703787ecb67869c9e1922e85 Diff: https://reviews.apache.org/r/36672/diff/ Testing --- Breaking patch, requires dependent patches. Thanks, Joris Van Remoortere
Re: Review Request 36715: Used std::thread instead of pthread for Libprocess process.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36715/#review92811 --- Patch looks great! Reviews applied: [36629, 36672, 36673, 36674, 36676, 36705, 36715] All tests passed. - Mesos ReviewBot On July 23, 2015, 7:49 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36715/ --- (Updated July 23, 2015, 7:49 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park. Bugs: MESOS-3119 https://issues.apache.org/jira/browse/MESOS-3119 Repository: mesos Description --- See summary Diffs - 3rdparty/libprocess/src/event_loop.hpp 45aa2a6019dc08da287b7c8a9d799619deb69747 3rdparty/libprocess/src/libev.cpp 8960c75010e3ceca3486a1fbb0fdeeffd62f4ec8 3rdparty/libprocess/src/libevent.cpp 02f9e61c1357e7b450e53b252b49d1979e782ae8 3rdparty/libprocess/src/process.cpp d6b0d5517964bf7c7276689e801a38b34a3f9db2 Diff: https://reviews.apache.org/r/36715/diff/ Testing --- make check with libev, libevent Thanks, Joris Van Remoortere
Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36620/#review92747 --- Patch looks great! Reviews applied: [36612, 36620] All tests passed. - Mesos ReviewBot On July 23, 2015, 12:21 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36620/ --- (Updated July 23, 2015, 12:21 p.m.) Review request for mesos and Timothy Chen. Bugs: MESOS-3086 https://issues.apache.org/jira/browse/MESOS-3086 Repository: mesos Description --- WIP Added Non-Freezeer Task Killer. Diffs - src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 Diff: https://reviews.apache.org/r/36620/diff/ Testing --- sudo make check Thanks, Joerg Schad
Re: Review Request 36501: MESOS-3023
On July 20, 2015, 9:42 a.m., haosdent huang wrote: src/tests/fetcher_tests.cpp, line 297 https://reviews.apache.org/r/36501/diff/6/?file=1015170#file1015170line297 According https://github.com/apache/mesos/blob/master/docs/mesos-c%2B%2B-style-guide.md#function-definitioninvocation You indent is not correct here. Maybe need change to like this ``` process::http::URL url( http, process.self().address.ip, process.self().address.port, /help); ``` But I perfer chang it like this ``` const network::Address address = process.self().address; process::http::URL url(http, address.ip, address.port, /help); ``` Or add `using URL` like this ``` using process::Future; using process::http::URL; (Left a blank below and after process::Future) ``` and then ``` const network::Address address = process.self().address; URL url(http, address.ip, address.port, /help); ``` I agree. Thanks, haosdent! - Bernd --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36501/#review92270 --- On July 18, 2015, 2:47 a.m., Klaus Ma wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36501/ --- (Updated July 18, 2015, 2:47 a.m.) Review request for mesos. Bugs: MESOS-3023 https://issues.apache.org/jira/browse/MESOS-3023 Repository: mesos Description --- Fix for MESOS-3023 (Factoring out the pattern for URL generation) Diffs - src/tests/fetcher_tests.cpp ae10c42 Diff: https://reviews.apache.org/r/36501/diff/ Testing --- 1. Build successfully in Linux 2. Test passed by src/mesos-tests --gtest_filter=FetcherTest.OSNetUriTest Thanks, Klaus Ma
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review92728 --- Bad patch! Reviews applied: [36717, 36318] Failed command: ./support/apply-review.sh -n -r 36318 Error: 2015-07-23 06:05:14 URL:https://reviews.apache.org/r/36318/diff/raw/ [29019/29019] - 36318.patch [1] error: patch failed: src/master/master.hpp:1512 error: src/master/master.hpp: patch does not apply Failed to apply patch - Mesos ReviewBot On July 23, 2015, 4:50 a.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated July 23, 2015, 4:50 a.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Diffs - src/master/http.cpp 6f5ca02c52462495b84e77525a6c88299746ece2 src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + test. Thanks, Anand Mazumdar
Re: Review Request 36712: Added ONCE guard for Eventloop::initialize
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36712/#review92769 --- Ship it! Ship It! - Joris Van Remoortere On July 23, 2015, 12:17 a.m., Jojy Varghese wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36712/ --- (Updated July 23, 2015, 12:17 a.m.) Review request for mesos and Joris Van Remoortere. Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/src/libevent.cpp 02f9e61c1357e7b450e53b252b49d1979e782ae8 Diff: https://reviews.apache.org/r/36712/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 36732: Remove deprecated /shutdown master endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36732/ --- (Updated July 23, 2015, 5:27 p.m.) Review request for mesos, Jan Schlicht and Vinod Kone. Bugs: MESOS-2697 https://issues.apache.org/jira/browse/MESOS-2697 Repository: mesos Description --- Remove deprecated /shutdown master endpoint. Diffs - src/master/http.cpp 6f5ca02c52462495b84e77525a6c88299746ece2 src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0 Diff: https://reviews.apache.org/r/36732/diff/ Testing --- Thanks, Joerg Schad
Re: Review Request 36318: Refactored framework struct in master to support http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36318/ --- (Updated July 23, 2015, 5:29 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Isabel Jimenez, Marco Massenzio, and Vinod Kone. Changes --- Removed extra line-break + added the dependent review for moving struct to end of file (r36733) Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- This change refactors the framework struct in master to introduce support for http frameworks. - pid becomes a optional field now in the framework struct. - added optional fields for supporting http frameworks to the framework struct - added a subcribe(...) method that registerFramework(...)/subscribeHttpFramework(...) call into. Similar functionality needs to be added for reregister to call into subscribe too. Diffs (updated) - src/common/protobuf_utils.hpp 2e827a0923de83d5cf853a12435b451cc7c55891 src/common/protobuf_utils.cpp e0f82b53f5e106bbf4e21d6ac946df0fae821882 src/master/http.cpp 6f5ca02c52462495b84e77525a6c88299746ece2 src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0 Diff: https://reviews.apache.org/r/36318/diff/ Testing --- make check + tests now go in a separate patch now. Thanks, Anand Mazumdar
Re: Review Request 36732: Remove deprecated /shutdown master endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36732/ --- (Updated July 23, 2015, 5:33 p.m.) Review request for mesos, Jan Schlicht and Vinod Kone. Bugs: MESOS-2697 https://issues.apache.org/jira/browse/MESOS-2697 Repository: mesos Description --- Remove deprecated /shutdown master endpoint. Diffs - src/master/http.cpp 6f5ca02c52462495b84e77525a6c88299746ece2 src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0 Diff: https://reviews.apache.org/r/36732/diff/ Testing (updated) --- make check Thanks, Joerg Schad
Re: Review Request 36617: Improved task reconciliation documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36617/#review92741 --- Ship it! docs/reconciliation.md (line 94) https://reviews.apache.org/r/36617/#comment146989 When the framework receives task status updates from the master, the framework should: - Joerg Schad On July 23, 2015, 11:48 a.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36617/ --- (Updated July 23, 2015, 11:48 a.m.) Review request for mesos and Joerg Schad. Bugs: MESOS-3127 https://issues.apache.org/jira/browse/MESOS-3127 Repository: mesos Description --- Improved task reconciliation documentation. Diffs - docs/reconciliation.md 17537ba8420c95d833e64ccf82ff9bb4530497f0 Diff: https://reviews.apache.org/r/36617/diff/ Testing --- https://gist.github.com/nfnt/73532d62fe39d27ff33d Thanks, Jan Schlicht
Re: Review Request 36718: Converted Limitation and ExecutorRunState structs into protobufs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36718/#review92785 --- Overall looks good to me! I wish we could merge all the protobuf related rules in src/Makefile.am as we are going to introduce more and more protobuf definitions. include/mesos/slave/isolator.proto (line 30) https://reviews.apache.org/r/36718/#comment147037 `slave::Limitation` sounds a little too broad. How about calling it `slave::ExecutorLimitation`? include/mesos/slave/isolator.proto (line 38) https://reviews.apache.org/r/36718/#comment147027 Why a default here? It's confusing between not setting the message and a default empty string. I would suggest you remove the default here. include/mesos/slave/isolator.proto (line 51) https://reviews.apache.org/r/36718/#comment147025 s/id/container_id/ src/Makefile.am (lines 157 - 165) https://reviews.apache.org/r/36718/#comment147031 Not your fault, but please sort those PROTO definitionss alphebetically. src/Makefile.am (lines 183 - 184) https://reviews.apache.org/r/36718/#comment147032 Ditto here. src/Makefile.am (line 278) https://reviews.apache.org/r/36718/#comment147036 As we keep adding more protobuf files, this will soon become unmanagable. I am wondering if we can combine these rules. I think we can have a single variable PROTOS which keeps all .proto files. I think we should be able to write a rule that can combline all the rules here. src/common/protobuf_utils.cpp (lines 209 - 238) https://reviews.apache.org/r/36718/#comment147038 Hum, these are slave specific. How about putting them in protobuf::slave namespace? src/slave/containerizer/isolators/posix/disk.cpp (line 62) https://reviews.apache.org/r/36718/#comment147039 No need to do this. ``` protobuf::createExecutorLimitation ``` reads better. src/slave/containerizer/mesos/containerizer.cpp (line 85) https://reviews.apache.org/r/36718/#comment147040 Ditto. - Jie Yu On July 23, 2015, 3:37 a.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36718/ --- (Updated July 23, 2015, 3:37 a.m.) Review request for mesos, Benjamin Hindman and Jie Yu. Bugs: MESOS-3115 https://issues.apache.org/jira/browse/MESOS-3115 Repository: mesos Description --- Protobufs are preferred over C structs for public API. Diffs - include/mesos/slave/isolator.hpp 85e38f5e4aa66527f1756fa259b93389f45028b3 include/mesos/slave/isolator.proto PRE-CREATION src/Makefile.am 489ddb424b342635c3dbc4d14ff5d69ce76a237b src/common/protobuf_utils.hpp 2e827a0923de83d5cf853a12435b451cc7c55891 src/common/protobuf_utils.cpp e0f82b53f5e106bbf4e21d6ac946df0fae821882 src/exec/exec.cpp a1ae074b962d8e93ab7776bd624389857da486f3 src/slave/containerizer/isolators/cgroups/cpushare.cpp 750bef947c11eb55236ac46109b9dd97e62b453d src/slave/containerizer/isolators/cgroups/mem.cpp b0e343fdc7088b2895d5dc8bb416dbcbf241cae5 src/slave/containerizer/isolators/cgroups/perf_event.cpp 512df3be7fdf6bac22ad4122f54a21d9986a1a6a src/slave/containerizer/isolators/filesystem/posix.cpp 1904279c92ef00ef931c909b4bb15bef89a4fc59 src/slave/containerizer/isolators/namespaces/pid.cpp 5de0791a835d725b7c7aae1ba585a94cff9372f1 src/slave/containerizer/isolators/network/port_mapping.cpp a7757f2a51f04da27645074f048722c22a2be752 src/slave/containerizer/isolators/posix.hpp 271061ef97aea96bb816982e530c84554d4b08d8 src/slave/containerizer/isolators/posix/disk.cpp b2f995cba36b1399db48af1de49d76c607f80abd src/slave/containerizer/launcher.cpp 24df1ca5f7407062f9e7b4bfa18f2cae5c72e140 src/slave/containerizer/linux_launcher.cpp 790e392645dd62e74b03ff0771f6bf0e9efeb622 src/slave/containerizer/mesos/containerizer.cpp 609620c4322e41562597ee682b311cd320bca6d2 Diff: https://reviews.apache.org/r/36718/diff/ Testing --- make check Thanks, Kapil Arya
Re: Review Request 36733: Moved framework struct in master to end of file
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36733/#review92792
---
Is there anything preventing you from using forward declarations? Or just
implementing the templated send function after the Master has been defined?
i.e.
```
struct Framework {
...
template typename Message
void send(const Message message);
...
}
class Master { ... }
template typename Message
void Framework::send(const Message message) {
...
}
```
- Joseph Wu
On July 23, 2015, 10:26 a.m., Anand Mazumdar wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36733/
---
(Updated July 23, 2015, 10:26 a.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Repository: mesos
Description
---
Moved framework struct to end of file since the framework struct needs to
know about the implementation of master for invoking templated send(...).
Needed for r36318.
Diffs
-
src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec
Diff: https://reviews.apache.org/r/36733/diff/
Testing
---
make check
Thanks,
Anand Mazumdar
Re: Review Request 36617: Improved task reconciliation documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36617/ --- (Updated July 23, 2015, 2:01 p.m.) Review request for mesos and Joerg Schad. Bugs: MESOS-3127 https://issues.apache.org/jira/browse/MESOS-3127 Repository: mesos Description --- Improved task reconciliation documentation. Diffs (updated) - docs/reconciliation.md 17537ba8420c95d833e64ccf82ff9bb4530497f0 Diff: https://reviews.apache.org/r/36617/diff/ Testing --- https://gist.github.com/nfnt/73532d62fe39d27ff33d Thanks, Jan Schlicht
Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36620/ --- (Updated July 23, 2015, 12:19 p.m.) Review request for mesos and Timothy Chen. Changes --- Improved comments. Bugs: MESOS-3086 https://issues.apache.org/jira/browse/MESOS-3086 Repository: mesos Description --- WIP Added Non-Freezeer Task Killer. Diffs (updated) - src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 Diff: https://reviews.apache.org/r/36620/diff/ Testing --- sudo make check Thanks, Joerg Schad
Re: Review Request 36708: Added HEARTBEAT event to the scheduler API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36708/#review92784 --- Ship it! include/mesos/scheduler/scheduler.proto (lines 46 - 52) https://reviews.apache.org/r/36708/#comment147024 does not receive heartbeats, or any other events, for an extended? Just thinking that we don't care about heartbeats when there are lots of events flowing (and this may skew the scheduler's perception of time between heartbeats). - Ben Mahler On July 23, 2015, 12:23 a.m., Vinod Kone wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36708/ --- (Updated July 23, 2015, 12:23 a.m.) Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Ben Mahler. Bugs: MESOS-3131 https://issues.apache.org/jira/browse/MESOS-3131 Repository: mesos Description --- This is to deal with network partition and also making sure that network intermediaries do not close the persistent connection between scheduler and master. Will add the code in the master once it can speak HTTP. Diffs - include/mesos/scheduler/scheduler.proto e2ca8e5cfb3d6405bf7be93d621743ce48c6c7e6 Diff: https://reviews.apache.org/r/36708/diff/ Testing --- None. Thanks, Vinod Kone
Re: Review Request 36708: Added HEARTBEAT event to the scheduler API.
On July 23, 2015, 6:24 p.m., Ben Mahler wrote: include/mesos/scheduler/scheduler.proto, lines 46-52 https://reviews.apache.org/r/36708/diff/2/?file=1019318#file1019318line46 does not receive heartbeats, or any other events, for an extended? Just thinking that we don't care about heartbeats when there are lots of events flowing (and this may skew the scheduler's perception of time between heartbeats). Wondering if we should also mention about the usage of a backoff strategy here ? Something like: s/resubscribe./resubscribe using a suitable backoff strategy. ? - Anand --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36708/#review92784 --- On July 23, 2015, 12:23 a.m., Vinod Kone wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36708/ --- (Updated July 23, 2015, 12:23 a.m.) Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Ben Mahler. Bugs: MESOS-3131 https://issues.apache.org/jira/browse/MESOS-3131 Repository: mesos Description --- This is to deal with network partition and also making sure that network intermediaries do not close the persistent connection between scheduler and master. Will add the code in the master once it can speak HTTP. Diffs - include/mesos/scheduler/scheduler.proto e2ca8e5cfb3d6405bf7be93d621743ce48c6c7e6 Diff: https://reviews.apache.org/r/36708/diff/ Testing --- None. Thanks, Vinod Kone
Re: Review Request 36732: Remove deprecated /shutdown master endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36732/#review92782 --- Patch looks great! Reviews applied: [36732] All tests passed. - Mesos ReviewBot On July 23, 2015, 5:33 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36732/ --- (Updated July 23, 2015, 5:33 p.m.) Review request for mesos, Jan Schlicht and Vinod Kone. Bugs: MESOS-2697 https://issues.apache.org/jira/browse/MESOS-2697 Repository: mesos Description --- Remove deprecated /shutdown master endpoint. Diffs - src/master/http.cpp 6f5ca02c52462495b84e77525a6c88299746ece2 src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0 Diff: https://reviews.apache.org/r/36732/diff/ Testing --- make check Thanks, Joerg Schad
Re: Review Request 36732: Remove deprecated /shutdown master endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36732/#review92776 --- Ship it! Can't remember if anyone sent a deprecation notice to dev list regarding this. If not, mind sending it? - Vinod Kone On July 23, 2015, 5:33 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36732/ --- (Updated July 23, 2015, 5:33 p.m.) Review request for mesos, Jan Schlicht and Vinod Kone. Bugs: MESOS-2697 https://issues.apache.org/jira/browse/MESOS-2697 Repository: mesos Description --- Remove deprecated /shutdown master endpoint. Diffs - src/master/http.cpp 6f5ca02c52462495b84e77525a6c88299746ece2 src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0 Diff: https://reviews.apache.org/r/36732/diff/ Testing --- make check Thanks, Joerg Schad
Re: Review Request 36617: Improved task reconciliation documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36617/#review92739 --- docs/reconciliation.md (line 43) https://reviews.apache.org/r/36617/#comment146987 Could you add one sentence before what the goal of Task reconciliation is? i.e. which problem are we solving... (you mention that partially in the algorithm ... docs/reconciliation.md (line 83) https://reviews.apache.org/r/36617/#comment146982 after a failure, there a reconcilation period in which the slaves are reregistering with the master. Because the master does only know about the state of the already reregistered slaves --and the other slaves could have potentially failed--, it can only reconcile . docs/reconciliation.md (line 88) https://reviews.apache.org/r/36617/#comment146983 s/in transitioning state/not yet reregistered docs/reconciliation.md (line 89) https://reviews.apache.org/r/36617/#comment146984 Hence during reconciliation we need several iterations of task status updates until the master and slave status are in sync again. docs/reconciliation.md (line 92) https://reviews.apache.org/r/36617/#comment146985 /s/task update/task status update also below docs/reconciliation.md (line 97) https://reviews.apache.org/r/36617/#comment146986 (e.g. TASK_FINISHED, ... docs/reconciliation.md (line 118) https://reviews.apache.org/r/36617/#comment146988 /s/As/as - Joerg Schad On July 22, 2015, 9:41 a.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36617/ --- (Updated July 22, 2015, 9:41 a.m.) Review request for mesos and Joerg Schad. Bugs: MESOS-3127 https://issues.apache.org/jira/browse/MESOS-3127 Repository: mesos Description --- Improved task reconciliation documentation. Diffs - docs/reconciliation.md 17537ba8420c95d833e64ccf82ff9bb4530497f0 Diff: https://reviews.apache.org/r/36617/diff/ Testing --- https://gist.github.com/nfnt/73532d62fe39d27ff33d Thanks, Jan Schlicht
