Review Request 36916: Doxygenified a comment in the allocator.proto.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36916/ --- Review request for mesos, Joerg Schad and Till Toenshoff. Repository: mesos Description --- Doxygenified a comment in the allocator.proto. Diffs - include/mesos/master/allocator.proto c32b88ed0252bf71e738ee64d64ffa828e97b2a3 Diff: https://reviews.apache.org/r/36916/diff/ Testing --- none. Thanks, Alexander Rukletsov
Re: Review Request 36914: Marked Path::basename, Path::dirname const.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36914/#review93461 --- Ship it! Ship It! - Till Toenshoff On July 29, 2015, 2:45 p.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36914/ --- (Updated July 29, 2015, 2:45 p.m.) Review request for mesos and Till Toenshoff. Bugs: MESOS-3173 https://issues.apache.org/jira/browse/MESOS-3173 Repository: mesos Description --- Marked Path::basename, Path::dirname const. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp d58f8a4ab8f6f761adb6a9db2dac438ffe0e211b Diff: https://reviews.apache.org/r/36914/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/ --- (Updated July 29, 2015, 5:07 p.m.) Review request for mesos, Alexander Rukletsov and Bernd Mathiske. Changes --- Addressed comment. Bugs: MESOS-3073 https://issues.apache.org/jira/browse/MESOS-3073 Repository: mesos Description --- Added /quota HTTP Endpoint for Quota handling. Diffs (updated) - src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f src/master/master.hpp 2331173fb9aeca77227fb09b899cb123eb205b4b src/master/master.cpp c584cb9f5aeb6806657059a3204ce1c680d4214a src/master/quota_handler.hpp PRE-CREATION Diff: https://reviews.apache.org/r/36913/diff/ Testing --- make check Thanks, Joerg Schad
Re: Review Request 36429: Add filesystem/linux isolator for persistent volumes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36429/#review93457 --- src/slave/containerizer/isolators/filesystem/linux.cpp (line 238) https://reviews.apache.org/r/36429/#comment147836 why MS_SHARED (bidirectional) vs MS_SLAVE (one-way)? MS_SLAVE would probably give better isolation to the host mount-ns. MS_SHARED would probably be better for a use case that I have in mind (doc'd in MESOS-349), especially since cleanup() here does GC on mount points that are children of the sandbox. - James DeFelice On July 12, 2015, 4:46 a.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36429/ --- (Updated July 12, 2015, 4:46 a.m.) Review request for mesos, Jie Yu, Timothy Chen, and Vinod Kone. Repository: mesos Description --- Moved filesystem/linux from review https://reviews.apache.org/r/34135/ Diffs - src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c src/slave/containerizer/isolators/filesystem/linux.hpp PRE-CREATION src/slave/containerizer/isolators/filesystem/linux.cpp PRE-CREATION src/slave/containerizer/mesos/containerizer.cpp 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 Diff: https://reviews.apache.org/r/36429/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
On July 29, 2015, 3:34 p.m., Bernd Mathiske wrote: src/tests/fetcher_cache_tests.cpp, line 1474 https://reviews.apache.org/r/36773/diff/7/?file=1024458#file1024458line1474 const Path Jan Schlicht wrote: Path::basename() is not marked const (which it probably should), hence we have to iterate over Path by value. Jan Schlicht wrote: Created a ticket for that: https://issues.apache.org/jira/browse/MESOS-3173 Path::basename() is const since 3eb17f6. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/#review93443 --- On July 29, 2015, 6:34 p.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 29, 2015, 6:34 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp 4e1d348c412c0cec6a73d6e43f2c7124db1f3bfe Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36909: Call parent SetUp() and TearDown() in MesosZooKeeperTest.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36909/#review93472 --- Patch looks great! Reviews applied: [36909] All tests passed. - Mesos ReviewBot On July 29, 2015, 1:56 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36909/ --- (Updated July 29, 2015, 1:56 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-3168 https://issues.apache.org/jira/browse/MESOS-3168 Repository: mesos Description --- Call parent SetUp() and TearDown() in MesosZooKeeperTest. Diffs - src/tests/mesos.hpp 192067d90aec2a114091e413e92e1ced10225afc Diff: https://reviews.apache.org/r/36909/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 36908: Added QuotaInfo Protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36908/ --- (Updated July 29, 2015, 4:59 p.m.) Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Till Toenshoff. Bugs: MESOS-3164 https://issues.apache.org/jira/browse/MESOS-3164 Repository: mesos Description --- Added QuotaInfo Protobuf. Diffs (updated) - include/mesos/master/quota.hpp PRE-CREATION include/mesos/master/quota.proto PRE-CREATION src/Makefile.am 0794969b4b147e0f837006f2f2eba6c6b28eb332 Diff: https://reviews.apache.org/r/36908/diff/ Testing --- make distcheck Thanks, Joerg Schad
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 29, 2015, 6:34 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Changes --- Use `const Path`. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs (updated) - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp 4e1d348c412c0cec6a73d6e43f2c7124db1f3bfe Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36916: Doxygenified a comment in the allocator.proto.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36916/#review93473 --- Ship it! Ship It! - Joerg Schad On July 29, 2015, 4:58 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36916/ --- (Updated July 29, 2015, 4:58 p.m.) Review request for mesos, Joerg Schad and Till Toenshoff. Repository: mesos Description --- Doxygenified a comment in the allocator.proto. Diffs - include/mesos/master/allocator.proto c32b88ed0252bf71e738ee64d64ffa828e97b2a3 Diff: https://reviews.apache.org/r/36916/diff/ Testing --- none. Thanks, Alexander Rukletsov
Re: Review Request 36912: Fixed Mesos version in getting started docs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36912/#review93474 --- Ship it! Ship It! - haosdent huang On July 29, 2015, 2:11 p.m., Ryuichi Okumura wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36912/ --- (Updated July 29, 2015, 2:11 p.m.) Review request for mesos. Bugs: MESOS-3172 https://issues.apache.org/jira/browse/MESOS-3172 Repository: mesos Description --- Fixed Mesos version in getting started docs. Diffs - docs/getting-started.md 38b0fe8a52a122dc41eff7fe498ed865b2c91051 Diff: https://reviews.apache.org/r/36912/diff/ Testing --- Thanks, Ryuichi Okumura
Re: Review Request 36821: Fix disable endpoints rule fails to recognize HTTP path delegates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36821/ --- (Updated July 29, 2015, 5:08 p.m.) Review request for mesos, Adam B and Alexander Rojas. Bugs: MESOS-3143 https://issues.apache.org/jira/browse/MESOS-3143 Repository: mesos Description --- Fix disable endpoints rule fails to recognize HTTP path delegates. Diffs (updated) - 3rdparty/libprocess/Makefile.am e9c42d7ecd44eebeadcc3d1d5011e01674b3415a 3rdparty/libprocess/include/process/firewall.hpp f92a6c5bbd4af0cab43e487b0c347c30059cf162 3rdparty/libprocess/include/process/process.hpp 8620547148f8a69d5b661eaf08063ca72347b6a4 3rdparty/libprocess/src/firewall.cpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 6d3609d06d017d8e50d7935b335fda7ebecbd04c Diff: https://reviews.apache.org/r/36821/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 36663: Added ip_address field to MasterInfo
On July 29, 2015, 1:01 p.m., Alexander Rukletsov wrote: include/mesos/mesos.proto, line 399 https://reviews.apache.org/r/36663/diff/5/?file=1021578#file1021578line399 `ip` and `port` are required, while `address` is optional. Is it intentional / doesn't it introduce a pitfall? You cannot add a `required` field in an existing protobuf, or you break existing servers (in particular, we would have all =0.22.x Mesos to break a 0.23 when the latter does Leader contention/detection) - that's PB 101 :) Did you mean the fields **inside** `Address` are required, or the **siblings** of `address`? If the latter, there's nothing you can do about it - as I mentioned, new fields **Must** be `optional` and you can't mess with existing ones. If the former, no - it's perfectly logically consistent: `address` may or may not be there; if it is there, then it **must** have `ip` and `port` set. - Marco --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36663/#review93441 --- On July 25, 2015, 12:36 a.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36663/ --- (Updated July 25, 2015, 12:36 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 - CHANGELOG 1a8649d00f3bcc792a75c11dedeb42a667e2ce88 include/mesos/mesos.proto e015c81d5052214ef8207642e23b3892a6123c9a src/common/protobuf_utils.cpp d900707ae64ad92c0c0ddc2996324b61121c8594 src/master/master.cpp 7796630a93705bd62157e98e1e4855c68ea7cd0a src/tests/master_contender_detector_tests.cpp d7a3b46b2e437818631064ae34317e49c9aa3748 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 35702: Added /reserve HTTP endpoint to the master.
On July 13, 2015, 4:46 p.m., Alexander Rukletsov wrote: src/master/http.cpp, lines 515-516 https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line515 It looks like we actually have the role, but it's buried in resources. Do you envision having resources collection with various roles in one request? Maybe it makes sense to add a validation step which ensures there is just one role per request and use it here, also avoiding changes in the `validate()`function. Michael Park wrote: I didn't see a good reason to require a one role per request condition. The current interface accurately models the fact that an operator does not have a role associated to it like a framework does, and I don't think avoiding changes in the `validate()` function should have any influence in deciding how an interface behaves. If we required such a condition, the per-request atomicity guarantee comes with a limitation that it can only be for a single role. While I'm not sure of its value, I'm also not sure what we gain by requiring it from the user's perspective? Alexander Rukletsov wrote: I think I'm missing something, my understanding is that each dynamic reservation is associated with a role, regardless, who issues a reservation request. I don't think limiting users to one role per request gives them any benefit, but it looks like we can be closer to framework-issued request if we do so. What am I missing? Michael Park wrote: Your understanding is correct. Aside from the resources being associated with a role, frameworks are also associated with a role. We check that every resource is being reserved for the framework's role because a framework is associated with a role and it wouldn't make sense to allow a framework to reserve resources for a role that does not match its role. On the contrary, the same rule doesn't apply for an operator since there's no such thing as an operator's role. Alexander Rukletsov wrote: That's right. Let me try to reformulate my proposal. If we require an operator to reserve resources for one role per request, it can be interpreted as an operator role. An advantage here is that `validate()` method doesn't need to be changed, while a disadvantage is that this approach is a bit artificial and can lead to confusion. What do you think? Michael Park wrote: As I mentioned in my initial comment, I think that the `validate()` function is an implementation detail and shouldn't be a source of motivation in deciding how an API should behave. Fair enough, feel free to drop. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/#review91472 --- On July 28, 2015, 9:03 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/ --- (Updated July 28, 2015, 9:03 p.m.) Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2600 https://issues.apache.org/jira/browse/MESOS-2600 Repository: mesos Description --- This involved a lot more challenges than I anticipated, I've captured the various approaches and limitations and deal-breakers of those approaches here: [Master Endpoint Implementation Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#) Key points: * This is a stop-gap solution until we shift the offer creation/management logic from the master to the allocator. * `updateAvailable` and `updateSlave` are kept separate because (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not. (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not. (3) `updateAvailable` never leaves the allocator in an over-allocated state and must not, whereas `updateSlave` does, and can. * The algorithm: * Initially, the master pessimistically assume that what seems like available resources will be gone. This is due to the race between the allocator scheduling an `allocate` call to itself vs master's `allocator-updateAvailable` invocation. As such, we first try to satisfy the request only with the offered resources. * We greedily rescind one offer at a time until we've rescinded sufficiently many offers. IMPORTANT: We perform `recoverResources(..., Filters())` rather than `recoverResources(..., None())` so that we can pretty much always win the race against `allocate`. In the case that we lose, no disaster occurs. We simply fail to satisfy the request. * If we still don't have
Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/#review93456 --- src/master/http.cpp (line 524) https://reviews.apache.org/r/36913/#comment147835 s/Method/method ? - Alexander Rukletsov On July 29, 2015, 3:05 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/ --- (Updated July 29, 2015, 3:05 p.m.) Review request for mesos, Alexander Rukletsov and Bernd Mathiske. Bugs: MESOS-3073 https://issues.apache.org/jira/browse/MESOS-3073 Repository: mesos Description --- Added /quota HTTP Endpoint for Quota handling. Diffs - src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f src/master/master.hpp 2331173fb9aeca77227fb09b899cb123eb205b4b src/master/master.cpp c584cb9f5aeb6806657059a3204ce1c680d4214a src/master/quota_handler.hpp PRE-CREATION Diff: https://reviews.apache.org/r/36913/diff/ Testing --- make check Thanks, Joerg Schad
Re: Review Request 35702: Added /reserve HTTP endpoint to the master.
On July 13, 2015, 4:46 p.m., Alexander Rukletsov wrote: src/master/http.cpp, lines 515-516 https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line515 It looks like we actually have the role, but it's buried in resources. Do you envision having resources collection with various roles in one request? Maybe it makes sense to add a validation step which ensures there is just one role per request and use it here, also avoiding changes in the `validate()`function. Michael Park wrote: I didn't see a good reason to require a one role per request condition. The current interface accurately models the fact that an operator does not have a role associated to it like a framework does, and I don't think avoiding changes in the `validate()` function should have any influence in deciding how an interface behaves. If we required such a condition, the per-request atomicity guarantee comes with a limitation that it can only be for a single role. While I'm not sure of its value, I'm also not sure what we gain by requiring it from the user's perspective? Alexander Rukletsov wrote: I think I'm missing something, my understanding is that each dynamic reservation is associated with a role, regardless, who issues a reservation request. I don't think limiting users to one role per request gives them any benefit, but it looks like we can be closer to framework-issued request if we do so. What am I missing? Michael Park wrote: Your understanding is correct. Aside from the resources being associated with a role, frameworks are also associated with a role. We check that every resource is being reserved for the framework's role because a framework is associated with a role and it wouldn't make sense to allow a framework to reserve resources for a role that does not match its role. On the contrary, the same rule doesn't apply for an operator since there's no such thing as an operator's role. Alexander Rukletsov wrote: That's right. Let me try to reformulate my proposal. If we require an operator to reserve resources for one role per request, it can be interpreted as an operator role. An advantage here is that `validate()` method doesn't need to be changed, while a disadvantage is that this approach is a bit artificial and can lead to confusion. What do you think? As I mentioned in my initial comment, I think that the `validate()` function is an implementation detail and shouldn't be a source of motivation in deciding how an API should behave. - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/#review91472 --- On July 28, 2015, 9:03 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/ --- (Updated July 28, 2015, 9:03 p.m.) Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2600 https://issues.apache.org/jira/browse/MESOS-2600 Repository: mesos Description --- This involved a lot more challenges than I anticipated, I've captured the various approaches and limitations and deal-breakers of those approaches here: [Master Endpoint Implementation Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#) Key points: * This is a stop-gap solution until we shift the offer creation/management logic from the master to the allocator. * `updateAvailable` and `updateSlave` are kept separate because (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not. (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not. (3) `updateAvailable` never leaves the allocator in an over-allocated state and must not, whereas `updateSlave` does, and can. * The algorithm: * Initially, the master pessimistically assume that what seems like available resources will be gone. This is due to the race between the allocator scheduling an `allocate` call to itself vs master's `allocator-updateAvailable` invocation. As such, we first try to satisfy the request only with the offered resources. * We greedily rescind one offer at a time until we've rescinded sufficiently many offers. IMPORTANT: We perform `recoverResources(..., Filters())` rather than `recoverResources(..., None())` so that we can pretty much always win the race against `allocate`. In the case that we lose, no disaster occurs. We simply fail to satisfy the request. * If we still don't have enough resources after resciding all offers, be optimistic and
Re: Review Request 36909: Call parent SetUp() and TearDown() in MesosZooKeeperTest.
On July 29, 2015, 5:43 p.m., Vinod Kone wrote: Aah. Good catch. Thank you! You are welcome. :-) - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36909/#review93482 --- On July 29, 2015, 1:56 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36909/ --- (Updated July 29, 2015, 1:56 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-3168 https://issues.apache.org/jira/browse/MESOS-3168 Repository: mesos Description --- Call parent SetUp() and TearDown() in MesosZooKeeperTest. Diffs - src/tests/mesos.hpp 192067d90aec2a114091e413e92e1ced10225afc Diff: https://reviews.apache.org/r/36909/diff/ Testing --- make check Thanks, haosdent huang
Review Request 36919: Pulled out call validation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36919/ --- Review request for mesos, Anand Mazumdar and Vinod Kone. Repository: mesos Description --- Call validation was duplicated across scheduler.cpp and the master. With the http api, we will also want an easy way to validate and drop calls that are coming in to the master. Diffs - src/master/master.hpp 2331173fb9aeca77227fb09b899cb123eb205b4b src/master/master.cpp c584cb9f5aeb6806657059a3204ce1c680d4214a src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 src/scheduler/scheduler.cpp 6887ed109a4b7342a460fff2ebbe1d98ff3e9092 Diff: https://reviews.apache.org/r/36919/diff/ Testing --- make check This also makes it easier to unit test this validation going forward. Thanks, Ben Mahler
Re: Review Request 36909: Call parent SetUp() and TearDown() in MesosZooKeeperTest.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36909/#review93509 --- src/tests/mesos.hpp (line 317) https://reviews.apache.org/r/36909/#comment147904 Shouldn't it come after network shutdown? https://reviews.apache.org/r/36920/ - Alexander Rukletsov On July 29, 2015, 1:56 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36909/ --- (Updated July 29, 2015, 1:56 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-3168 https://issues.apache.org/jira/browse/MESOS-3168 Repository: mesos Description --- Call parent SetUp() and TearDown() in MesosZooKeeperTest. Diffs - src/tests/mesos.hpp 192067d90aec2a114091e413e92e1ced10225afc Diff: https://reviews.apache.org/r/36909/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 36807: Adding a test for MasterInfo
On July 29, 2015, 8 p.m., Vinod Kone wrote: src/tests/master_tests.cpp, line 2170 https://reviews.apache.org/r/36807/diff/2/?file=1024091#file1024091line2170 new line. would you like me to push another revision or will you do this yourself when committing? I'm easy either way, thanks! - Marco --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36807/#review93505 --- On July 29, 2015, 5:19 a.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36807/ --- (Updated July 29, 2015, 5:19 a.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2736 https://issues.apache.org/jira/browse/MESOS-2736 Repository: mesos Description --- We want to test that when the master gets initialized, the MasterInfo that it creates (and gets ultimately serialized to ZooKeeper) has the correct information in its Address field. Currently, the test hangs when run in after another MasterZooKeeperTest, due to some issues around the recovery. Jira: MESOS-2736 Diffs - src/tests/master_tests.cpp 05c148ee1660b86428afe4eda718b17052743a8c Diff: https://reviews.apache.org/r/36807/diff/ Testing --- make check (after @vinodkone fix all tests pass) Thanks, Marco Massenzio
Re: Review Request 36822: Used std::thread instead of pthread for Long Lived Executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36822/#review93522 --- Ship it! Ship It! - Benjamin Hindman On July 27, 2015, 8:18 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36822/ --- (Updated July 27, 2015, 8:18 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park. Bugs: MESOS-3120 https://issues.apache.org/jira/browse/MESOS-3120 Repository: mesos Description --- See summary. Diffs - src/examples/long_lived_executor.cpp d9b7fa1eefd1c53447244586797cb96a5c33d4f1 Diff: https://reviews.apache.org/r/36822/diff/ Testing --- make check Thanks, Joris Van Remoortere
Review Request 36920: Fixed TearDown order in ZK test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36920/ --- Review request for mesos, haosdent huang and Vinod Kone. Bugs: MESOS-3168 https://issues.apache.org/jira/browse/MESOS-3168 Repository: mesos Description --- Fixed TearDown order in ZK test. Diffs - src/tests/mesos.hpp 0bbfb1188350ab1fc09cb3d098234347f6dc9203 Diff: https://reviews.apache.org/r/36920/diff/ Testing --- make check (Mac OS 10.10.4) Thanks, Alexander Rukletsov
Re: Review Request 36807: Adding a test for MasterInfo
On July 29, 2015, 8 p.m., Vinod Kone wrote: src/tests/master_tests.cpp, line 2170 https://reviews.apache.org/r/36807/diff/2/?file=1024091#file1024091line2170 new line. Marco Massenzio wrote: would you like me to push another revision or will you do this yourself when committing? I'm easy either way, thanks! actually, never mind - takes me a minute... will post shortly. - Marco --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36807/#review93505 --- On July 29, 2015, 5:19 a.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36807/ --- (Updated July 29, 2015, 5:19 a.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2736 https://issues.apache.org/jira/browse/MESOS-2736 Repository: mesos Description --- We want to test that when the master gets initialized, the MasterInfo that it creates (and gets ultimately serialized to ZooKeeper) has the correct information in its Address field. Currently, the test hangs when run in after another MasterZooKeeperTest, due to some issues around the recovery. Jira: MESOS-2736 Diffs - src/tests/master_tests.cpp 05c148ee1660b86428afe4eda718b17052743a8c Diff: https://reviews.apache.org/r/36807/diff/ Testing --- make check (after @vinodkone fix all tests pass) Thanks, Marco Massenzio
Re: Review Request 36807: Adding a test for MasterInfo
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36807/ --- (Updated July 29, 2015, 8:54 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2736 https://issues.apache.org/jira/browse/MESOS-2736 Repository: mesos Description --- We want to test that when the master gets initialized, the MasterInfo that it creates (and gets ultimately serialized to ZooKeeper) has the correct information in its Address field. Currently, the test hangs when run in after another MasterZooKeeperTest, due to some issues around the recovery. Jira: MESOS-2736 Diffs (updated) - 3rdparty/libprocess/src/http.cpp 4dcbd74b894b483c4d166c23fac55ac8dba75166 3rdparty/libprocess/src/tests/ssl_tests.cpp 7a316bc10575325ffe732fcc87d72d15a4fc5eaf src/tests/master_tests.cpp 05c148ee1660b86428afe4eda718b17052743a8c src/tests/mesos.hpp 0bbfb1188350ab1fc09cb3d098234347f6dc9203 Diff: https://reviews.apache.org/r/36807/diff/ Testing --- make check (after @vinodkone fix all tests pass) Thanks, Marco Massenzio
Re: Review Request 36807: Adding a test for MasterInfo
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36807/#review93515 --- forgot to rebase? - Vinod Kone On July 29, 2015, 8:54 p.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36807/ --- (Updated July 29, 2015, 8:54 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2736 https://issues.apache.org/jira/browse/MESOS-2736 Repository: mesos Description --- We want to test that when the master gets initialized, the MasterInfo that it creates (and gets ultimately serialized to ZooKeeper) has the correct information in its Address field. Currently, the test hangs when run in after another MasterZooKeeperTest, due to some issues around the recovery. Jira: MESOS-2736 Diffs - 3rdparty/libprocess/src/http.cpp 4dcbd74b894b483c4d166c23fac55ac8dba75166 3rdparty/libprocess/src/tests/ssl_tests.cpp 7a316bc10575325ffe732fcc87d72d15a4fc5eaf src/tests/master_tests.cpp 05c148ee1660b86428afe4eda718b17052743a8c src/tests/mesos.hpp 0bbfb1188350ab1fc09cb3d098234347f6dc9203 Diff: https://reviews.apache.org/r/36807/diff/ Testing --- make check (after @vinodkone fix all tests pass) Thanks, Marco Massenzio
Re: Review Request 36920: Fixed TearDown order in ZK test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36920/#review93510 --- Ship it! - Vinod Kone On July 29, 2015, 8:39 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36920/ --- (Updated July 29, 2015, 8:39 p.m.) Review request for mesos, haosdent huang and Vinod Kone. Bugs: MESOS-3168 https://issues.apache.org/jira/browse/MESOS-3168 Repository: mesos Description --- Fixed TearDown order in ZK test. Diffs - src/tests/mesos.hpp 0bbfb1188350ab1fc09cb3d098234347f6dc9203 Diff: https://reviews.apache.org/r/36920/diff/ Testing --- make check (Mac OS 10.10.4) Thanks, Alexander Rukletsov
Re: Review Request 36625: Windows: Split up platform specific functions into separate headers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36625/ --- (Updated July 29, 2015, 3:01 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Alex Clemmer, and Joris Van Remoortere. Changes --- Updated copyright header language in format.hpp. Bugs: MESOS-3101 https://issues.apache.org/jira/browse/MESOS-3101 Repository: mesos Description --- To support the upcoming Windows Containerizer (MESOS-3094), we're splitting up (refactoring) platform specific functions into separate files. We will avoid having `#ifdef __WINDOWS__` all over the stout/libprcess code by separating Posix/Windows versions. This first patch is to establish a pattern in splitting up the headers. Patterns: * gzip.hpp, thread.hpp - Functions are moved to a Posix folder; copied to a Windows folder and gutted for later implementation. * abort.hpp, exit.hpp, unreachable.hpp - Added macro for `__attribute__((noreturn))`. * duration.hpp - An #ifdef for one of the headers (time.h vs Winsock2.h). No need to split the header. * format.hpp - Missing Windows function (vasprintf) implementation added. * ip.hpp - Added aliases for Windows functions. * net.hpp - Curl functions were moved to Posix/Windows folders. Other: * Instances of #include local file.hpp were changed to #include stout/local file.hpp to match os.hpp. * Some missing #endif comments (i.e. `// __APPLE__`) were added. Diffs (updated) - 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 5c19e3ef8ba50ab007eda26b752441f076ca7ed0 3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp 3aa9487bed2df038ca27a8bb94c24608ca7910a4 3rdparty/libprocess/3rdparty/stout/include/stout/attributes.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/duration.hpp bba8303347aac3f70566a9e69625a928cfb1bd24 3rdparty/libprocess/3rdparty/stout/include/stout/exit.hpp 8c16a224433d7a43bf6bf17e1129e6eb9bbbd573 3rdparty/libprocess/3rdparty/stout/include/stout/format.hpp 4e8c3bd1e9abf0ff24f78c8385ed9625719dcf8c 3rdparty/libprocess/3rdparty/stout/include/stout/gzip.hpp 0b95819205af6caae05c01cb4d0b25620abe791c 3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp a0ea23797376288e8dc96886fd3c0702e5edf846 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp a538fb1a343aab039aecabe508b7747e683fd46e 3rdparty/libprocess/3rdparty/stout/include/stout/posix/gzip.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/posix/net.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/posix/thread.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/thread.hpp 552d6e97c882a36d6a889af205c422e51f544b34 3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp fed0a7ba81c98be83a0d66c2317e768877f8e40d 3rdparty/libprocess/3rdparty/stout/include/stout/windows/format.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/windows/gzip.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/windows/net.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/windows/preprocessor.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/windows/thread.hpp PRE-CREATION Diff: https://reviews.apache.org/r/36625/diff/ Testing --- `make` and `make check` (Mac OSX) Thanks, Joseph Wu
Re: Review Request 36900: Publish MasterInfo to ZK in JSON format
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36900/ --- (Updated July 29, 2015, 9:07 p.m.) Review request for mesos, Anand Mazumdar, Isabel Jimenez, and Vinod Kone. Changes --- rebased Bugs: MESOS-3135 https://issues.apache.org/jira/browse/MESOS-3135 Repository: mesos Description --- As part of the support for non-libmesos linked client to interact with the HTTP API, we also provide the Master information stored in ZooKeeper in a way that is deserializable also for clients that do not necessarily know how to decode a MasterInfo protocol buffer. This patch publishes the data in JSON format and has been tested, summarily, with 2 masters running this patch alongside a 0.23 Master and one each of 0.24 and 0.23 Agent nodes. Please note this patch does not require specific unit testing, as the functionality is already widely tested in existing `ZooKeeper*` tests, as well as `MasterDetector*` ones. Diffs (updated) - src/master/contender.cpp 2af70c7d1acf274db40cae1641cd305249a471ec Diff: https://reviews.apache.org/r/36900/diff/ Testing --- make check also ran: 3x Masters (2x 0.24 + 1x 0.23) / 2x Slaves (1x 0.23 + 1x 0.24) the 0.24's were running this patch. Also verified in ZK that the data was correct JSON: ``` { address:{ hostname:10.0.77.243, ip:10.0.77.243, port:5050 }, hostname:10.0.77.243, id:20150728-165830-4081909770-5050-64014, ip:4081909770, pid:master@10.0.77.243:5050, port:5050, version:0.24.0 } ``` Not sure why Reviewbot failed, but the tests all passed: ``` [--] Global test environment tear-down [==] 684 tests from 97 test cases ran. (274274 ms total) [ PASSED ] 684 tests. YOU HAVE 12 DISABLED TESTS ``` the error seems to be with files left in `src/credentials` and `src/master/replicated_log`: ``` ERROR: files left in build directory after distclean: ./src/credentials ./src/master/replicated_log/CURRENT ./src/master/replicated_log/LOG ./src/master/replicated_log/MANIFEST-06 ./src/master/replicated_log/11.sst ./src/master/replicated_log/LOCK ./src/master/replicated_log/09.log ./src/master/replicated_log/LOG.old make[1]: *** [distcleancheck] Error 1 ``` Am I missing something here? Thanks, Marco Massenzio
Re: Review Request 36821: Fix disable endpoints rule fails to recognize HTTP path delegates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36821/#review93520 --- Patch looks great! Reviews applied: [36821] All tests passed. - Mesos ReviewBot On July 29, 2015, 5:40 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36821/ --- (Updated July 29, 2015, 5:40 p.m.) Review request for mesos, Adam B and Alexander Rojas. Bugs: MESOS-3143 https://issues.apache.org/jira/browse/MESOS-3143 Repository: mesos Description --- Fix disable endpoints rule fails to recognize HTTP path delegates. Diffs - 3rdparty/libprocess/Makefile.am e9c42d7ecd44eebeadcc3d1d5011e01674b3415a 3rdparty/libprocess/include/process/firewall.hpp f92a6c5bbd4af0cab43e487b0c347c30059cf162 3rdparty/libprocess/include/process/process.hpp 8620547148f8a69d5b661eaf08063ca72347b6a4 3rdparty/libprocess/src/firewall.cpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 6d3609d06d017d8e50d7935b335fda7ebecbd04c Diff: https://reviews.apache.org/r/36821/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 36919: Pulled out call validation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36919/ --- (Updated July 29, 2015, 7:43 p.m.) Review request for mesos, Anand Mazumdar and Vinod Kone. Changes --- Added MESOS-2497 Bugs: MESOS-2497 https://issues.apache.org/jira/browse/MESOS-2497 Repository: mesos Description --- Call validation was duplicated across scheduler.cpp and the master. With the http api, we will also want an easy way to validate and drop calls that are coming in to the master. Diffs - src/master/master.hpp 2331173fb9aeca77227fb09b899cb123eb205b4b src/master/master.cpp c584cb9f5aeb6806657059a3204ce1c680d4214a src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 src/scheduler/scheduler.cpp 6887ed109a4b7342a460fff2ebbe1d98ff3e9092 Diff: https://reviews.apache.org/r/36919/diff/ Testing --- make check This also makes it easier to unit test this validation going forward. Thanks, Ben Mahler
Re: Review Request 34136: Add ContainerImage protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34136/#review93481 --- include/mesos/mesos.proto (line 1213) https://reviews.apache.org/r/34136/#comment147861 Hmm... I don't think this should be required. It's too inflexible and tasks likely will use name and labels. - Jiang Yan Xu On July 11, 2015, 9:47 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34136/ --- (Updated July 11, 2015, 9:47 p.m.) Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone. Repository: mesos Description --- Add ContainerImage protobuf. Diffs - include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 Diff: https://reviews.apache.org/r/34136/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 36821: Fix disable endpoints rule fails to recognize HTTP path delegates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36821/ --- (Updated July 29, 2015, 5:40 p.m.) Review request for mesos, Adam B and Alexander Rojas. Bugs: MESOS-3143 https://issues.apache.org/jira/browse/MESOS-3143 Repository: mesos Description --- Fix disable endpoints rule fails to recognize HTTP path delegates. Diffs (updated) - 3rdparty/libprocess/Makefile.am e9c42d7ecd44eebeadcc3d1d5011e01674b3415a 3rdparty/libprocess/include/process/firewall.hpp f92a6c5bbd4af0cab43e487b0c347c30059cf162 3rdparty/libprocess/include/process/process.hpp 8620547148f8a69d5b661eaf08063ca72347b6a4 3rdparty/libprocess/src/firewall.cpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 6d3609d06d017d8e50d7935b335fda7ebecbd04c Diff: https://reviews.apache.org/r/36821/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 36916: Doxygenified a comment in the allocator.proto.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36916/#review93500 --- Patch looks great! Reviews applied: [36916] All tests passed. - Mesos ReviewBot On July 29, 2015, 4:58 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36916/ --- (Updated July 29, 2015, 4:58 p.m.) Review request for mesos, Joerg Schad and Till Toenshoff. Repository: mesos Description --- Doxygenified a comment in the allocator.proto. Diffs - include/mesos/master/allocator.proto c32b88ed0252bf71e738ee64d64ffa828e97b2a3 Diff: https://reviews.apache.org/r/36916/diff/ Testing --- none. Thanks, Alexander Rukletsov
Re: Review Request 36900: Publish MasterInfo to ZK in JSON format
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36900/#review93374 --- src/master/contender.cpp (line 239) https://reviews.apache.org/r/36900/#comment147893 s/jsonInfo/json/ src/master/contender.cpp (line 240) https://reviews.apache.org/r/36900/#comment147894 kill this. src/master/contender.cpp (line 241) https://reviews.apache.org/r/36900/#comment147744 kill this line. src/master/contender.cpp (line 245) https://reviews.apache.org/r/36900/#comment147895 s/data/stringify(json)/ - Vinod Kone On July 29, 2015, 5:27 a.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36900/ --- (Updated July 29, 2015, 5:27 a.m.) Review request for mesos, Anand Mazumdar, Isabel Jimenez, and Vinod Kone. Bugs: MESOS-3135 https://issues.apache.org/jira/browse/MESOS-3135 Repository: mesos Description --- As part of the support for non-libmesos linked client to interact with the HTTP API, we also provide the Master information stored in ZooKeeper in a way that is deserializable also for clients that do not necessarily know how to decode a MasterInfo protocol buffer. This patch publishes the data in JSON format and has been tested, summarily, with 2 masters running this patch alongside a 0.23 Master and one each of 0.24 and 0.23 Agent nodes. Please note this patch does not require specific unit testing, as the functionality is already widely tested in existing `ZooKeeper*` tests, as well as `MasterDetector*` ones. Diffs - src/master/contender.cpp 2af70c7d1acf274db40cae1641cd305249a471ec Diff: https://reviews.apache.org/r/36900/diff/ Testing --- make check also ran: 3x Masters (2x 0.24 + 1x 0.23) / 2x Slaves (1x 0.23 + 1x 0.24) the 0.24's were running this patch. Also verified in ZK that the data was correct JSON: ``` { address:{ hostname:10.0.77.243, ip:10.0.77.243, port:5050 }, hostname:10.0.77.243, id:20150728-165830-4081909770-5050-64014, ip:4081909770, pid:master@10.0.77.243:5050, port:5050, version:0.24.0 } ``` Not sure why Reviewbot failed, but the tests all passed: ``` [--] Global test environment tear-down [==] 684 tests from 97 test cases ran. (274274 ms total) [ PASSED ] 684 tests. YOU HAVE 12 DISABLED TESTS ``` the error seems to be with files left in `src/credentials` and `src/master/replicated_log`: ``` ERROR: files left in build directory after distclean: ./src/credentials ./src/master/replicated_log/CURRENT ./src/master/replicated_log/LOG ./src/master/replicated_log/MANIFEST-06 ./src/master/replicated_log/11.sst ./src/master/replicated_log/LOCK ./src/master/replicated_log/09.log ./src/master/replicated_log/LOG.old make[1]: *** [distcleancheck] Error 1 ``` Am I missing something here? Thanks, Marco Massenzio
Re: Review Request 36900: Publish MasterInfo to ZK in JSON format
On July 29, 2015, 7:44 p.m., Vinod Kone wrote: Thanks, I have the impression you were looking at an earlier revision, but no matter - the one I'll be uploading in a second will reflect your comments. - Marco --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36900/#review93374 --- On July 29, 2015, 5:27 a.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36900/ --- (Updated July 29, 2015, 5:27 a.m.) Review request for mesos, Anand Mazumdar, Isabel Jimenez, and Vinod Kone. Bugs: MESOS-3135 https://issues.apache.org/jira/browse/MESOS-3135 Repository: mesos Description --- As part of the support for non-libmesos linked client to interact with the HTTP API, we also provide the Master information stored in ZooKeeper in a way that is deserializable also for clients that do not necessarily know how to decode a MasterInfo protocol buffer. This patch publishes the data in JSON format and has been tested, summarily, with 2 masters running this patch alongside a 0.23 Master and one each of 0.24 and 0.23 Agent nodes. Please note this patch does not require specific unit testing, as the functionality is already widely tested in existing `ZooKeeper*` tests, as well as `MasterDetector*` ones. Diffs - src/master/contender.cpp 2af70c7d1acf274db40cae1641cd305249a471ec Diff: https://reviews.apache.org/r/36900/diff/ Testing --- make check also ran: 3x Masters (2x 0.24 + 1x 0.23) / 2x Slaves (1x 0.23 + 1x 0.24) the 0.24's were running this patch. Also verified in ZK that the data was correct JSON: ``` { address:{ hostname:10.0.77.243, ip:10.0.77.243, port:5050 }, hostname:10.0.77.243, id:20150728-165830-4081909770-5050-64014, ip:4081909770, pid:master@10.0.77.243:5050, port:5050, version:0.24.0 } ``` Not sure why Reviewbot failed, but the tests all passed: ``` [--] Global test environment tear-down [==] 684 tests from 97 test cases ran. (274274 ms total) [ PASSED ] 684 tests. YOU HAVE 12 DISABLED TESTS ``` the error seems to be with files left in `src/credentials` and `src/master/replicated_log`: ``` ERROR: files left in build directory after distclean: ./src/credentials ./src/master/replicated_log/CURRENT ./src/master/replicated_log/LOG ./src/master/replicated_log/MANIFEST-06 ./src/master/replicated_log/11.sst ./src/master/replicated_log/LOCK ./src/master/replicated_log/09.log ./src/master/replicated_log/LOG.old make[1]: *** [distcleancheck] Error 1 ``` Am I missing something here? Thanks, Marco Massenzio
Re: Review Request 36909: Call parent SetUp() and TearDown() in MesosZooKeeperTest.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36909/#review93482 --- Ship it! Aah. Good catch. Thank you! - Vinod Kone On July 29, 2015, 1:56 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36909/ --- (Updated July 29, 2015, 1:56 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-3168 https://issues.apache.org/jira/browse/MESOS-3168 Repository: mesos Description --- Call parent SetUp() and TearDown() in MesosZooKeeperTest. Diffs - src/tests/mesos.hpp 192067d90aec2a114091e413e92e1ced10225afc Diff: https://reviews.apache.org/r/36909/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
On July 29, 2015, 7:12 p.m., Ben Mahler wrote: src/master/master.cpp, lines 2128-2159 https://reviews.apache.org/r/36720/diff/3/?file=1021871#file1021871line2128 Any reason you're skipping validation (authorization) of the framework? Looks like we should pull out the pid specific code from validate for now: it looks like we already have to do synchronous checks inside the continuations anyway. For example, `_reregisterFramework` checks to see if it is authenticated, but it is missing the check that it is the right principal! Looks like we should s/validate/authorize/ and pull out an `isAuthenticated` that we can call synchronously as well. Primarily due to the reasons you had mentioned earlier. There was a lot of pid specific code in validate. Also , call validation in general were supposed to be handled in MESOS-2497, so I refrained from making the changes. Looks like you already did them in r36919, Thanks ! On July 29, 2015, 7:12 p.m., Ben Mahler wrote: src/master/master.cpp, lines 2162-2193 https://reviews.apache.org/r/36720/diff/3/?file=1021871#file1021871line2162 This was a bit confusing, since it says registering but you're planning to also have it include re-registration? This looks like `_subscribe` given it is the continuation. Yes, I intend to make re-register also call into this function. You are also right about this being the _subscribe. Since I had not yet taken care of validation as per my earlier argument, I went ahead with naming it as subscribe for now. When we have the validation routine , it would just become a continuation function and can be renamed. Also , failover logic would be implemented inside this function and the existing _reregisterFramework(...) would be calling into this function. What do you think ? On July 29, 2015, 7:12 p.m., Ben Mahler wrote: src/master/http.cpp, lines 324-379 https://reviews.apache.org/r/36720/diff/3/?file=1021869#file1021869line324 JSON should be do-able already, here's a suggestion to clean this up a bit (note that it is handling JSON): ``` FutureResponse Master::Http::call(const Request request) const { scheduler::Call call; // TODO(anand): Content type values are case-insensitive. Optionstring contentType = request.headers.get(Content-Type); if (contentType.isNone()) { return BadRequest(Expecting 'Content-Type' to be present); } if (contentType.get() == APPLICATION_PROTOBUF) { if (!call.ParseFromString(request.body)) { return BadRequest(Failed to deserialize body into Call protobuf); } } else if (contentType.get() == APPLICATION_JSON) { TryJSON::Value value = JSON::parse(request.body); if (value.isError()) { return BadRequest(Failed to parse body into JSON: + value.error()); } Tryscheduler::Call parse = ::protobuf::parsescheduler::Call(value.get()); if (parse.isError()) { return BadRequest(Failed to convert JSON into Call protobuf: + parse.error()); } call = parse.get(); } // Default to sending back JSON. ContentType responseContentType = ContentType::JSON; // TODO(anand): Use request.accepts() once available. Optionstring acceptType = request.headers.get(Accept); if (acceptType.get() == APPLICATION_PROTOBUF) { responseContentType = ContentType::PROTOBUF; } switch (call.type()) { case scheduler::Call::SUBSCRIBE: { Pipe pipe; OK ok; ok.type = Response::PIPE; ok.reader = pipe.reader(); master-subscribeHttp( call.subscribe(), responseContentType, pipe.writer()); return ok; } default: break; } return NotImplemented(); } ``` Thanks, I would include the JSON logic. I thought adding the JSON workflow might be more convoluted then this and hence wanted it to be part of a different patch, my bad. - Anand --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review93499 --- On July 25, 2015, 2:32 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated July 25, 2015, 2:32 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2294
Re: Review Request 36821: Fix disable endpoints rule fails to recognize HTTP path delegates.
On July 28, 2015, 7:46 a.m., Alexander Rojas wrote: Thank you for your review. I updated the code, could you reivew it again? - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36821/#review93252 --- On July 29, 2015, 5:40 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36821/ --- (Updated July 29, 2015, 5:40 p.m.) Review request for mesos, Adam B and Alexander Rojas. Bugs: MESOS-3143 https://issues.apache.org/jira/browse/MESOS-3143 Repository: mesos Description --- Fix disable endpoints rule fails to recognize HTTP path delegates. Diffs - 3rdparty/libprocess/Makefile.am e9c42d7ecd44eebeadcc3d1d5011e01674b3415a 3rdparty/libprocess/include/process/firewall.hpp f92a6c5bbd4af0cab43e487b0c347c30059cf162 3rdparty/libprocess/include/process/process.hpp 8620547148f8a69d5b661eaf08063ca72347b6a4 3rdparty/libprocess/src/firewall.cpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 6d3609d06d017d8e50d7935b335fda7ebecbd04c Diff: https://reviews.apache.org/r/36821/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 36908: Added QuotaInfo Protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36908/#review93496 --- Patch looks great! Reviews applied: [36908] All tests passed. - Mesos ReviewBot On July 29, 2015, 4:59 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36908/ --- (Updated July 29, 2015, 4:59 p.m.) Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Till Toenshoff. Bugs: MESOS-3164 https://issues.apache.org/jira/browse/MESOS-3164 Repository: mesos Description --- Added QuotaInfo Protobuf. Diffs - include/mesos/master/quota.hpp PRE-CREATION include/mesos/master/quota.proto PRE-CREATION src/Makefile.am 0794969b4b147e0f837006f2f2eba6c6b28eb332 Diff: https://reviews.apache.org/r/36908/diff/ Testing --- make distcheck Thanks, Joerg Schad
Re: Review Request 36807: Adding a test for MasterInfo
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36807/#review93505 --- Ship it! src/tests/master_tests.cpp (line 2170) https://reviews.apache.org/r/36807/#comment147901 new line. - Vinod Kone On July 29, 2015, 5:19 a.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36807/ --- (Updated July 29, 2015, 5:19 a.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2736 https://issues.apache.org/jira/browse/MESOS-2736 Repository: mesos Description --- We want to test that when the master gets initialized, the MasterInfo that it creates (and gets ultimately serialized to ZooKeeper) has the correct information in its Address field. Currently, the test hangs when run in after another MasterZooKeeperTest, due to some issues around the recovery. Jira: MESOS-2736 Diffs - src/tests/master_tests.cpp 05c148ee1660b86428afe4eda718b17052743a8c Diff: https://reviews.apache.org/r/36807/diff/ Testing --- make check (after @vinodkone fix all tests pass) Thanks, Marco Massenzio
Re: Review Request 36816: Support HTTP checks in Mesos health check program
On July 28, 2015, 8:05 a.m., Adam B wrote: src/health-check/main.cpp, line 243 https://reviews.apache.org/r/36816/diff/1/?file=1021956#file1021956line243 Maybe we should add `http.protocol()` in case the user wants https? Or `http.ssl` like BenH suggested. Would we ever want anything besides http/https? We can always add that in a later patch, so feel free to ignore for now. Could add https now. - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36816/#review93254 --- On July 25, 2015, 6:57 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36816/ --- (Updated July 25, 2015, 6:57 p.m.) Review request for mesos and Adam B. Bugs: MESOS-2533 https://issues.apache.org/jira/browse/MESOS-2533 Repository: mesos Description --- Support HTTP checks in Mesos health check program Diffs - include/mesos/mesos.proto a6748d1cd82238f005c6a49c70d22d095462f1ba src/health-check/main.cpp 97b25716335ec5719c1100bd73d06b7fc98036c9 src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea Diff: https://reviews.apache.org/r/36816/diff/ Testing --- * Add a new unit test: HealthCheckTest.HealthyTaskThroughHttp make check Thanks, haosdent huang
Re: Review Request 36816: Support HTTP checks in Mesos health check program
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36816/ --- (Updated July 29, 2015, 6:08 p.m.) Review request for mesos and Adam B. Bugs: MESOS-2533 https://issues.apache.org/jira/browse/MESOS-2533 Repository: mesos Description --- Support HTTP checks in Mesos health check program Diffs (updated) - include/mesos/mesos.proto a6748d1cd82238f005c6a49c70d22d095462f1ba src/health-check/main.cpp 97b25716335ec5719c1100bd73d06b7fc98036c9 src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea Diff: https://reviews.apache.org/r/36816/diff/ Testing --- * Add a new unit test: HealthCheckTest.HealthyTaskThroughHttp make check Thanks, haosdent huang
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/#review93499 --- Looks like this needs a rebase? Wasn't able to get it applied without `--3way`. I'm going to pull out the call Call validation to hopefully make this a bit easier. Per my comment below, I'd suggest we try to tackle the 'validate' refactor in an independent patch to pull apart the authorization vs authentication checks. src/master/http.cpp (lines 324 - 379) https://reviews.apache.org/r/36720/#comment147888 JSON should be do-able already, here's a suggestion to clean this up a bit (note that it is handling JSON): ``` FutureResponse Master::Http::call(const Request request) const { scheduler::Call call; // TODO(anand): Content type values are case-insensitive. Optionstring contentType = request.headers.get(Content-Type); if (contentType.isNone()) { return BadRequest(Expecting 'Content-Type' to be present); } if (contentType.get() == APPLICATION_PROTOBUF) { if (!call.ParseFromString(request.body)) { return BadRequest(Failed to deserialize body into Call protobuf); } } else if (contentType.get() == APPLICATION_JSON) { TryJSON::Value value = JSON::parse(request.body); if (value.isError()) { return BadRequest(Failed to parse body into JSON: + value.error()); } Tryscheduler::Call parse = ::protobuf::parsescheduler::Call(value.get()); if (parse.isError()) { return BadRequest(Failed to convert JSON into Call protobuf: + parse.error()); } call = parse.get(); } // Default to sending back JSON. ContentType responseContentType = ContentType::JSON; // TODO(anand): Use request.accepts() once available. Optionstring acceptType = request.headers.get(Accept); if (acceptType.get() == APPLICATION_PROTOBUF) { responseContentType = ContentType::PROTOBUF; } switch (call.type()) { case scheduler::Call::SUBSCRIBE: { Pipe pipe; OK ok; ok.type = Response::PIPE; ok.reader = pipe.reader(); master-subscribeHttp( call.subscribe(), responseContentType, pipe.writer()); return ok; } default: break; } return NotImplemented(); } ``` src/master/master.cpp (lines 2110 - 2141) https://reviews.apache.org/r/36720/#comment147890 Any reason you're skipping validation (authorization) of the framework? Looks like we should pull out the pid specific code from validate for now: it looks like we already have to do synchronous checks inside the continuations anyway. For example, `_reregisterFramework` checks to see if it is authenticated, but it is missing the check that it is the right principal! Looks like we should s/validate/authorize/ and pull out an `isAuthenticated` that we can call synchronously as well. src/master/master.cpp (lines 2144 - 2175) https://reviews.apache.org/r/36720/#comment147889 This was a bit confusing, since it says registering but you're planning to also have it include re-registration? This looks like `_subscribe` given it is the continuation. - Ben Mahler On July 25, 2015, 2:32 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated July 25, 2015, 2:32 p.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 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca 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 36919: Pulled out call validation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36919/#review93504 --- Ship it! nice cleanup! src/master/master.cpp (lines 1697 - 1698) https://reviews.apache.org/r/36919/#comment147898 Make this LOG(FATAL) then because it's not possible? src/master/validation.cpp (line 55) https://reviews.apache.org/r/36919/#comment147897 can you put it inside scheduler::call namespace? src/scheduler/scheduler.cpp https://reviews.apache.org/r/36919/#comment147900 Can you keep this TODO? - Vinod Kone On July 29, 2015, 7:43 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36919/ --- (Updated July 29, 2015, 7:43 p.m.) Review request for mesos, Anand Mazumdar and Vinod Kone. Bugs: MESOS-2497 https://issues.apache.org/jira/browse/MESOS-2497 Repository: mesos Description --- Call validation was duplicated across scheduler.cpp and the master. With the http api, we will also want an easy way to validate and drop calls that are coming in to the master. Diffs - src/master/master.hpp 2331173fb9aeca77227fb09b899cb123eb205b4b src/master/master.cpp c584cb9f5aeb6806657059a3204ce1c680d4214a src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 src/scheduler/scheduler.cpp 6887ed109a4b7342a460fff2ebbe1d98ff3e9092 Diff: https://reviews.apache.org/r/36919/diff/ Testing --- make check This also makes it easier to unit test this validation going forward. Thanks, Ben Mahler
Re: Review Request 36920: Fixed TearDown order in ZK test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36920/#review93525 --- Bad patch! Reviews applied: [36920] Failed command: ./support/apply-review.sh -n -r 36920 Error: 2015-07-29 23:09:22 URL:https://reviews.apache.org/r/36920/diff/raw/ [372/372] - 36920.patch [1] error: patch failed: src/tests/mesos.hpp:314 error: src/tests/mesos.hpp: patch does not apply Failed to apply patch - Mesos ReviewBot On July 29, 2015, 8:39 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36920/ --- (Updated July 29, 2015, 8:39 p.m.) Review request for mesos, haosdent huang and Vinod Kone. Bugs: MESOS-3168 https://issues.apache.org/jira/browse/MESOS-3168 Repository: mesos Description --- Fixed TearDown order in ZK test. Diffs - src/tests/mesos.hpp 0bbfb1188350ab1fc09cb3d098234347f6dc9203 Diff: https://reviews.apache.org/r/36920/diff/ Testing --- make check (Mac OS 10.10.4) Thanks, Alexander Rukletsov
Re: Review Request 36865: Style change: Space after the ... in variadic templates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36865/ --- (Updated July 29, 2015, 4:51 p.m.) Review request for mesos, Benjamin Hindman and Joris Van Remoortere. Summary (updated) - Style change: Space after the ... in variadic templates. Repository: mesos Description (updated) --- There are a few instances of `typename...` and `typename ...` in the code base. This patch normalizes those to just `typename...` . Diffs (updated) - 3rdparty/libprocess/include/process/help.hpp 316ed21b5300496f283d0c331fa6d9dad441d332 Diff: https://reviews.apache.org/r/36865/diff/ Testing --- `make check` Thanks, Joseph Wu
Review Request 36930: Forced the network isolator to use the mount namespace.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36930/ --- Review request for mesos, Chi Zhang and Vinod Kone. Repository: mesos Description --- Forced the network isolator to use the mount namespace. The code of the network isolator actually relies on the fact that the child is in a seprate mount namespace. For example: https://github.com/apache/mesos/blob/master/src/slave/containerizer/isolators/network/port_mapping.cpp#L1527 https://github.com/apache/mesos/blob/master/src/slave/containerizer/isolators/network/port_mapping.cpp#L3533 It originally depends on mount namespace, but was removed in this patch: https://reviews.apache.org/r/26274 That was a bug to me. It didn't cause any issue because we don't clone the mounts (since we are not using mount namespace) anymore after the above patch. So the kernel won't have an extra reference to the mount when we try to umount it in `_cleanup()`. Diffs - src/slave/containerizer/isolators/network/port_mapping.cpp 3f6e9df8711995d0dd3903c6170fdd5ad61aac5a Diff: https://reviews.apache.org/r/36930/diff/ Testing --- sudo make check Thanks, Jie Yu
Re: Review Request 36625: Windows: Split up platform specific functions into separate headers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36625/ --- (Updated July 29, 2015, 4:18 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Alex Clemmer, and Joris Van Remoortere. Changes --- Fix some build issues on Windows: * inline'd the function in windows/format.hpp. * Added MAXHOSTNAMELEN definition. Bugs: MESOS-3101 https://issues.apache.org/jira/browse/MESOS-3101 Repository: mesos Description --- To support the upcoming Windows Containerizer (MESOS-3094), we're splitting up (refactoring) platform specific functions into separate files. We will avoid having `#ifdef __WINDOWS__` all over the stout/libprcess code by separating Posix/Windows versions. This first patch is to establish a pattern in splitting up the headers. Patterns: * gzip.hpp, thread.hpp - Functions are moved to a Posix folder; copied to a Windows folder and gutted for later implementation. * abort.hpp, exit.hpp, unreachable.hpp - Added macro for `__attribute__((noreturn))`. * duration.hpp - An #ifdef for one of the headers (time.h vs Winsock2.h). No need to split the header. * format.hpp - Missing Windows function (vasprintf) implementation added. * ip.hpp - Added aliases for Windows functions. * net.hpp - Curl functions were moved to Posix/Windows folders. Other: * Instances of #include local file.hpp were changed to #include stout/local file.hpp to match os.hpp. * Some missing #endif comments (i.e. `// __APPLE__`) were added. Diffs (updated) - 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 5c19e3ef8ba50ab007eda26b752441f076ca7ed0 3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp 3aa9487bed2df038ca27a8bb94c24608ca7910a4 3rdparty/libprocess/3rdparty/stout/include/stout/attributes.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/duration.hpp bba8303347aac3f70566a9e69625a928cfb1bd24 3rdparty/libprocess/3rdparty/stout/include/stout/exit.hpp 8c16a224433d7a43bf6bf17e1129e6eb9bbbd573 3rdparty/libprocess/3rdparty/stout/include/stout/format.hpp 4e8c3bd1e9abf0ff24f78c8385ed9625719dcf8c 3rdparty/libprocess/3rdparty/stout/include/stout/gzip.hpp 0b95819205af6caae05c01cb4d0b25620abe791c 3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp a0ea23797376288e8dc96886fd3c0702e5edf846 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp a538fb1a343aab039aecabe508b7747e683fd46e 3rdparty/libprocess/3rdparty/stout/include/stout/posix/gzip.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/posix/net.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/posix/thread.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/thread.hpp 552d6e97c882a36d6a889af205c422e51f544b34 3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp fed0a7ba81c98be83a0d66c2317e768877f8e40d 3rdparty/libprocess/3rdparty/stout/include/stout/windows/format.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/windows/gzip.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/windows/net.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/windows/preprocessor.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/windows/thread.hpp PRE-CREATION Diff: https://reviews.apache.org/r/36625/diff/ Testing (updated) --- `make` and `make check` (Mac OSX). Build with MSVC Enterprise 2015 [thanks to Alex (hausdorff)]. Thanks, Joseph Wu
Re: Review Request 36627: Fixed cgroups oom killer and memory pressure tests on Ubuntu 14.04.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36627/ --- (Updated July 29, 2015, 4:11 p.m.) Review request for mesos, Benjamin Hindman and Joris Van Remoortere. Changes --- Rebasing. Bugs: MESOS-2660 https://issues.apache.org/jira/browse/MESOS-2660 Repository: mesos Description --- See summary. Diffs (updated) - src/tests/containerizer/cgroups_tests.cpp caecd5dfa3fef33dba35cfc1b5934a11e2cc961a src/tests/containerizer/memory_test_helper.cpp 48a35632786963f484f66642b5c67afd4f7a89cc 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 inactive_file:0KB active_file:0KB unevictable:65536KB Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052467] [ pid ] uid tgid total_vm rss nr_ptes swapents oom_score_adj name Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052574] [76599] 0 765998245231766 1221 0 lt-memory-test- Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052575] Memory
Review Request 36929: Fixed a few issues in test launcher header.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36929/ --- Review request for mesos and Vinod Kone. Repository: mesos Description --- Fixed a few issues in test launcher header. Diffs - src/tests/containerizer/launcher.hpp b80e84196f8b494e6e5a3c96c86f63fe432a7bb0 Diff: https://reviews.apache.org/r/36929/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 36927: Pulled apart authorization and authentication validation for frameworks in the master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36927/#review93532 --- src/master/master.cpp (lines 1574 - 1576) https://reviews.apache.org/r/36927/#comment147918 so this line will be presented twice if principal is not set (which will be for most frameworks) because this function is called twice. that is kinda unfortunate and likely confusing to users. not sure what we could do here yet. src/master/master.cpp (lines 1720 - 1742) https://reviews.apache.org/r/36927/#comment147917 checking validationError.isNone() in each if statement looks a bit weird. how about doing these in an else if instead? if (frameworkInfo.has_id() !(frameworkInfo.id() == )) { } else if (!roles.contains(frameworkInfo.role()) { } else if (frameworkInfo.user() == root !flags.root_submissions) { } else { } i think the above is easier to read? src/master/master.cpp (lines 1731 - 1732) https://reviews.apache.org/r/36927/#comment147916 why did you use 2 if loops here instead of ? anyway, see above comment to make it simpler. src/master/master.cpp (lines 1866 - 1882) https://reviews.apache.org/r/36927/#comment147919 ditto. please use else if. - Vinod Kone On July 29, 2015, 11:52 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36927/ --- (Updated July 29, 2015, 11:52 p.m.) Review request for mesos and Vinod Kone. Repository: mesos Description --- Framework authorization is now done through `authorizeFramework` which is consistent with `authorizeTask`. Also, `validateFrameworkAuthentication` captures the validation for authentication. I also made registration and re-registration consistent: * Both perform the check for root submission, rather than just registration. * Authentication checks in `_registerFramework` and `_reregisterFramework` are now comprehensive (thanks to `validateFrameworkAuthentication`). Diffs - src/master/master.hpp 1135049d34f8ed4d56c795ad28c9ca1d07237b63 src/master/master.cpp 2bfec2f69375444925252480142ee409b8474761 Diff: https://reviews.apache.org/r/36927/diff/ Testing --- make check Thanks, Ben Mahler
Review Request 36927: Pulled apart authorization and authentication validation for frameworks in the master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36927/ --- Review request for mesos and Vinod Kone. Repository: mesos Description --- Framework authorization is now done through `authorizeFramework` which is consistent with `authorizeTask`. Also, `validateFrameworkAuthentication` captures the validation for authentication. I also made registration and re-registration consistent: * Both perform the check for root submission, rather than just registration. * Authentication checks in `_registerFramework` and `_reregisterFramework` are now comprehensive (thanks to `validateFrameworkAuthentication`). Diffs - src/master/master.hpp 1135049d34f8ed4d56c795ad28c9ca1d07237b63 src/master/master.cpp 2bfec2f69375444925252480142ee409b8474761 Diff: https://reviews.apache.org/r/36927/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 36824: Used std::thread instead of pthread for ns tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36824/#review93528 --- Ship it! src/tests/containerizer/ns_tests.cpp (line 149) https://reviews.apache.org/r/36824/#comment147913 Same comment here as 36823, using Latch* instead of OwnedLatch. - Benjamin Hindman On July 27, 2015, 8:48 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36824/ --- (Updated July 27, 2015, 8:48 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park. Bugs: MESOS-3120 https://issues.apache.org/jira/browse/MESOS-3120 Repository: mesos Description --- See summary. Diffs - src/tests/containerizer/ns_tests.cpp c71c33fa1247d1658be91ce81c93bfffd5b282b7 Diff: https://reviews.apache.org/r/36824/diff/ Testing --- sudo ./bin/mesos-tests.sh --gtest_filter=NsTest.* Thanks, Joris Van Remoortere
Re: Review Request 36827: Removed and guarded pthread specifics for libevent-openssl.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36827/#review93529 --- Ship it! Ship It! - Benjamin Hindman On July 27, 2015, 9:14 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36827/ --- (Updated July 27, 2015, 9:14 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/libevent.cpp 1f175a4ae83391152d064779c6ab69d31cbaf867 3rdparty/libprocess/src/openssl.cpp 6aa2a4db8d64011d0fde6ff0cf4b144c41949d39 Diff: https://reviews.apache.org/r/36827/diff/ Testing --- make check. Waiting on validation from someone with an OSX build Thanks, Joris Van Remoortere
Re: Review Request 36823: Used std::thread instead of pthread for cgroups tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36823/#review93527 --- Ship it! src/tests/containerizer/cgroups_tests.cpp (line 814) https://reviews.apache.org/r/36823/#comment147912 We don't use '' as a default capture, didn't you write that style guide rule? ;-) So, I'll just swap for a 'Latch*' instead of an OwnedLatch for you before committing, thanks! - Benjamin Hindman On July 27, 2015, 8:35 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36823/ --- (Updated July 27, 2015, 8:35 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park. Bugs: MESOS-3120 https://issues.apache.org/jira/browse/MESOS-3120 Repository: mesos Description --- See summary. Diffs - src/tests/containerizer/cgroups_tests.cpp caecd5dfa3fef33dba35cfc1b5934a11e2cc961a Diff: https://reviews.apache.org/r/36823/diff/ Testing --- sudo ./bin/mesos-tests.sh --gtest_filter=CgroupsAnyHierarchyWithFreezerTest.* Thanks, Joris Van Remoortere
Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/#review93530 --- Patch looks great! Reviews applied: [36913] All tests passed. - Mesos ReviewBot On July 29, 2015, 8:48 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/ --- (Updated July 29, 2015, 8:48 p.m.) Review request for mesos, Alexander Rukletsov and Bernd Mathiske. Bugs: MESOS-3073 https://issues.apache.org/jira/browse/MESOS-3073 Repository: mesos Description --- Added /quota HTTP Endpoint for Quota handling. Diffs - src/Makefile.am 0794969b4b147e0f837006f2f2eba6c6b28eb332 src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f src/master/master.hpp 2331173fb9aeca77227fb09b899cb123eb205b4b src/master/master.cpp c584cb9f5aeb6806657059a3204ce1c680d4214a src/master/quota_handler.hpp PRE-CREATION Diff: https://reviews.apache.org/r/36913/diff/ Testing --- make check Thanks, Joerg Schad
Re: Review Request 36909: Call parent SetUp() and TearDown() in MesosZooKeeperTest.
On July 29, 2015, 8:40 p.m., Alexander Rukletsov wrote: src/tests/mesos.hpp, line 317 https://reviews.apache.org/r/36909/diff/3/?file=1024461#file1024461line317 Shouldn't it come after network shutdown? https://reviews.apache.org/r/36920/ sorry, my fault - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36909/#review93509 --- On July 29, 2015, 1:56 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36909/ --- (Updated July 29, 2015, 1:56 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-3168 https://issues.apache.org/jira/browse/MESOS-3168 Repository: mesos Description --- Call parent SetUp() and TearDown() in MesosZooKeeperTest. Diffs - src/tests/mesos.hpp 192067d90aec2a114091e413e92e1ced10225afc Diff: https://reviews.apache.org/r/36909/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 36865: Style change: Space after the ... in variadic templates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36865/#review93542 --- Patch looks great! Reviews applied: [36864, 36865] All tests passed. - Mesos ReviewBot On July 29, 2015, 11:51 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36865/ --- (Updated July 29, 2015, 11:51 p.m.) Review request for mesos, Benjamin Hindman and Joris Van Remoortere. Repository: mesos Description --- There are a few instances of `typename...` and `typename ...` in the code base. This patch normalizes those to just `typename...` . Diffs - 3rdparty/libprocess/include/process/help.hpp 316ed21b5300496f283d0c331fa6d9dad441d332 Diff: https://reviews.apache.org/r/36865/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 36816: Support HTTP checks in Mesos health check program
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36816/ --- (Updated July 30, 2015, 2:47 a.m.) Review request for mesos and Adam B. Bugs: MESOS-2533 https://issues.apache.org/jira/browse/MESOS-2533 Repository: mesos Description --- Support HTTP checks in Mesos health check program Diffs (updated) - include/mesos/mesos.proto a6748d1cd82238f005c6a49c70d22d095462f1ba src/health-check/main.cpp 97b25716335ec5719c1100bd73d06b7fc98036c9 src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea Diff: https://reviews.apache.org/r/36816/diff/ Testing --- * Add a new unit test: HealthCheckTest.HealthyTaskThroughHttp make check Thanks, haosdent huang
Re: Review Request 36930: Forced the network isolator to use the mount namespace.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36930/#review93548 --- Patch looks great! Reviews applied: [36929, 36930] All tests passed. - Mesos ReviewBot On July 30, 2015, 12:19 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36930/ --- (Updated July 30, 2015, 12:19 a.m.) Review request for mesos, Chi Zhang and Vinod Kone. Repository: mesos Description --- Forced the network isolator to use the mount namespace. The code of the network isolator actually relies on the fact that the child is in a seprate mount namespace. For example: https://github.com/apache/mesos/blob/master/src/slave/containerizer/isolators/network/port_mapping.cpp#L1527 https://github.com/apache/mesos/blob/master/src/slave/containerizer/isolators/network/port_mapping.cpp#L3533 It originally depends on mount namespace, but was removed in this patch: https://reviews.apache.org/r/26274 That was a bug to me. It didn't cause any issue because we don't clone the mounts (since we are not using mount namespace) anymore after the above patch. So the kernel won't have an extra reference to the mount when we try to umount it in `_cleanup()`. Diffs - src/slave/containerizer/isolators/network/port_mapping.cpp 3f6e9df8711995d0dd3903c6170fdd5ad61aac5a Diff: https://reviews.apache.org/r/36930/diff/ Testing --- sudo make check Thanks, Jie Yu
Re: Review Request 36927: Pulled apart authorization and authentication validation for frameworks in the master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36927/#review93549 --- Patch looks great! Reviews applied: [36927] All tests passed. - Mesos ReviewBot On July 29, 2015, 11:52 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36927/ --- (Updated July 29, 2015, 11:52 p.m.) Review request for mesos and Vinod Kone. Repository: mesos Description --- Framework authorization is now done through `authorizeFramework` which is consistent with `authorizeTask`. Also, `validateFrameworkAuthentication` captures the validation for authentication. I also made registration and re-registration consistent: * Both perform the check for root submission, rather than just registration. * Authentication checks in `_registerFramework` and `_reregisterFramework` are now comprehensive (thanks to `validateFrameworkAuthentication`). Diffs - src/master/master.hpp 1135049d34f8ed4d56c795ad28c9ca1d07237b63 src/master/master.cpp 2bfec2f69375444925252480142ee409b8474761 Diff: https://reviews.apache.org/r/36927/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 36920: Fixed TearDown order in ZK test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36920/#review93540 --- Ship it! Ship It! - haosdent huang On July 29, 2015, 8:39 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36920/ --- (Updated July 29, 2015, 8:39 p.m.) Review request for mesos, haosdent huang and Vinod Kone. Bugs: MESOS-3168 https://issues.apache.org/jira/browse/MESOS-3168 Repository: mesos Description --- Fixed TearDown order in ZK test. Diffs - src/tests/mesos.hpp 0bbfb1188350ab1fc09cb3d098234347f6dc9203 Diff: https://reviews.apache.org/r/36920/diff/ Testing --- make check (Mac OS 10.10.4) Thanks, Alexander Rukletsov
Re: Review Request 36819: Use setup.py in python cli package.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/ --- (Updated July 30, 2015, 2:49 a.m.) Review request for mesos, Benjamin Hindman and Marco Massenzio. Bugs: MESOS-3149 https://issues.apache.org/jira/browse/MESOS-3149 Repository: mesos Description --- Use setup.py in python cli package. Diffs (updated) - Makefile.am f8e958d376efa55ace7c5727a31b8e747403641e bin/mesos.sh.in 5cbeac4330a9f45fc6d54b8c2d383f48e4098f95 configure.ac 546c9bbf775a4ef481fafb3a58c85c6d80e19500 src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/cli/python/mesos/__init__.py 028b0d27fb193bac96f2a6a3201ee4cc8fd369ef src/cli/python/mesos/cli.py src/cli/python/mesos/futures.py src/cli/python/mesos/http.py src/python/cli/src/mesos/__init__.py PRE-CREATION src/python/interface/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/native/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/protocol/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 Diff: https://reviews.apache.org/r/36819/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 32961: Allow framework re-registeration to update master http fields.
On July 27, 2015, 11:38 p.m., Ben Mahler wrote: src/master/master.cpp, line 1850 https://reviews.apache.org/r/32961/diff/4/?file=927192#file927192line1850 Anand discovered that we shouldn't be calling this in the FrameworkErrorMessage case below, where we drop the message. Looks like a bug? Vinod Kone wrote: Definitely looks like a bug. @joris do you want to post a fix? I think there are a couple of error cases in which we don't want to update the framework info so I have created MESOS-3169 to address this issue. - Joris --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32961/#review93199 --- On April 14, 2015, 4:30 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32961/ --- (Updated April 14, 2015, 4:30 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Bugs: MESOS-1218, MESOS-2614 and MESOS-703 https://issues.apache.org/jira/browse/MESOS-1218 https://issues.apache.org/jira/browse/MESOS-2614 https://issues.apache.org/jira/browse/MESOS-703 Repository: mesos Description --- Fields: 'name', 'hostname', 'failover_timeout', 'webui_url' Diffs - src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 src/master/master.cpp 44b0a0147f5354824d86332a67b30018634c9a36 Diff: https://reviews.apache.org/r/32961/diff/ Testing --- make check. re-registered no_executor_framework with different 'name', 'hostname', 'failover_timeout', and 'webui_url' Thanks, Joris Van Remoortere
Re: Review Request 36811: Don't check protobuf jar when --disable-java flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36811/ --- (Updated July 29, 2015, 7:16 a.m.) Review request for mesos, Adam B and Niklas Nielsen. Bugs: MESOS-2480 https://issues.apache.org/jira/browse/MESOS-2480 Repository: mesos Description --- Don't check protobuf jar when --disable-java flag. Diffs - configure.ac 546c9bbf775a4ef481fafb3a58c85c6d80e19500 Diff: https://reviews.apache.org/r/36811/diff/ Testing (updated) --- ../configure --with-protobuf=/usr/local --disable-java make -j4 make check ``` [--] Global test environment tear-down [==] 644 tests from 91 test cases ran. (431596 ms total) [ PASSED ] 644 tests. ``` ../configure make -j4 make check ``` [--] Global test environment tear-down [==] 685 tests from 98 test cases ran. (554759 ms total) [ PASSED ] 685 tests. ``` ../configure --disable-java make -j4 make check ``` [--] Global test environment tear-down [==] 644 tests from 91 test cases ran. (427688 ms total) [ PASSED ] 644 tests. ``` ../configure --with-protobuf=/usr/local make -j4 make check ``` [--] Global test environment tear-down [==] 685 tests from 98 test cases ran. (551493 ms total) [ PASSED ] 685 tests. ``` Thanks, haosdent huang
Re: Review Request 36819: Use setup.py in python cli package.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/ --- (Updated July 29, 2015, 7:44 a.m.) Review request for mesos, Benjamin Hindman and Marco Massenzio. Bugs: MESOS-3149 https://issues.apache.org/jira/browse/MESOS-3149 Repository: mesos Description --- Use setup.py in python cli package. Diffs (updated) - Makefile.am f8e958d376efa55ace7c5727a31b8e747403641e bin/mesos.sh.in 5cbeac4330a9f45fc6d54b8c2d383f48e4098f95 configure.ac 546c9bbf775a4ef481fafb3a58c85c6d80e19500 src/Makefile.am 0794969b4b147e0f837006f2f2eba6c6b28eb332 src/cli/python/mesos/__init__.py 028b0d27fb193bac96f2a6a3201ee4cc8fd369ef src/cli/python/mesos/cli.py src/cli/python/mesos/futures.py src/cli/python/mesos/http.py src/python/cli/src/mesos/__init__.py PRE-CREATION src/python/interface/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/native/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/protocol/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 Diff: https://reviews.apache.org/r/36819/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 36811: Don't check protobuf jar when --disable-java flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36811/ --- (Updated July 29, 2015, 7:18 a.m.) Review request for mesos, Adam B and Niklas Nielsen. Changes --- Retrigger jenkins Bugs: MESOS-2480 https://issues.apache.org/jira/browse/MESOS-2480 Repository: mesos Description --- Don't check protobuf jar when --disable-java flag. Diffs (updated) - configure.ac 546c9bbf775a4ef481fafb3a58c85c6d80e19500 Diff: https://reviews.apache.org/r/36811/diff/ Testing --- ../configure --with-protobuf=/usr/local --disable-java make -j4 make check ``` [--] Global test environment tear-down [==] 644 tests from 91 test cases ran. (431596 ms total) [ PASSED ] 644 tests. ``` ../configure make -j4 make check ``` [--] Global test environment tear-down [==] 685 tests from 98 test cases ran. (554759 ms total) [ PASSED ] 685 tests. ``` ../configure --disable-java make -j4 make check ``` [--] Global test environment tear-down [==] 644 tests from 91 test cases ran. (427688 ms total) [ PASSED ] 644 tests. ``` ../configure --with-protobuf=/usr/local make -j4 make check ``` [--] Global test environment tear-down [==] 685 tests from 98 test cases ran. (551493 ms total) [ PASSED ] 685 tests. ``` Thanks, haosdent huang
Re: Review Request 36811: Don't check protobuf jar when --disable-java flag.
On July 28, 2015, 5:58 a.m., Adam B wrote: Did you test this with java enabled as well? What about without `--with-protobuf=`? All four combinations still need to compile pass the unit tests. haosdent huang wrote: I only run it with --disable-java and --with_protobuf, let me test other 3 combinations. @adam-mesos, @marco I update the code and test steps, could you help review it again? Thank you in advance. - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36811/#review93240 --- On July 29, 2015, 7:19 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36811/ --- (Updated July 29, 2015, 7:19 a.m.) Review request for mesos, Adam B and Marco Massenzio. Bugs: MESOS-2480 https://issues.apache.org/jira/browse/MESOS-2480 Repository: mesos Description --- Don't check protobuf jar when --disable-java flag. Diffs - configure.ac 546c9bbf775a4ef481fafb3a58c85c6d80e19500 Diff: https://reviews.apache.org/r/36811/diff/ Testing --- ../configure --with-protobuf=/usr/local --disable-java make -j4 make check ``` [--] Global test environment tear-down [==] 644 tests from 91 test cases ran. (431596 ms total) [ PASSED ] 644 tests. ``` ../configure make -j4 make check ``` [--] Global test environment tear-down [==] 685 tests from 98 test cases ran. (554759 ms total) [ PASSED ] 685 tests. ``` ../configure --disable-java make -j4 make check ``` [--] Global test environment tear-down [==] 644 tests from 91 test cases ran. (427688 ms total) [ PASSED ] 644 tests. ``` ../configure --with-protobuf=/usr/local make -j4 make check ``` [--] Global test environment tear-down [==] 685 tests from 98 test cases ran. (551493 ms total) [ PASSED ] 685 tests. ``` Thanks, haosdent huang
Re: Review Request 36819: Use setup.py in python cli package.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/ --- (Updated July 29, 2015, 7:43 a.m.) Review request for mesos and Benjamin Hindman. Bugs: MESOS-3149 https://issues.apache.org/jira/browse/MESOS-3149 Repository: mesos Description --- Use setup.py in python cli package. Diffs (updated) - 3rdparty/libprocess/include/process/http.hpp b8d930058aa3ae9d21ae0f7b420d49f2448f84b1 3rdparty/libprocess/src/openssl.hpp efa40c4c73210eae87c3fc904334794ca80e097a 3rdparty/libprocess/src/openssl.cpp 1f68460a760403f88a00a32137969456ce1c2266 3rdparty/libprocess/src/tests/http_tests.cpp ecbcbd552ac834659860627c82628ed38e6139b3 3rdparty/libprocess/src/tests/process_tests.cpp 95e3257b030128e9d03dde9aa048602c68c6a446 3rdparty/libprocess/src/tests/ssl_tests.cpp a7173dfbbf42065634762bfa7c89c0be0ea15079 Makefile.am f8e958d376efa55ace7c5727a31b8e747403641e bin/mesos.sh.in 5cbeac4330a9f45fc6d54b8c2d383f48e4098f95 configure.ac 546c9bbf775a4ef481fafb3a58c85c6d80e19500 docs/authorization.md 2a5e74782754e446184a297f91112e9f94077896 docs/home.md 3dff97ba35fafe8fca7e18866f0c7e8d526022e1 docs/mesos-ssl.md 9df34dbf3873fc2a074e6d864f3bdbc389517bf7 docs/operational-guide.md ef81db672bd5c8fab172336956ab03461ae35064 docs/oversubscription.md b1a6c9979025ce33190792a07b8e9618c518430e docs/reconciliation.md b50d692d415cf677d23051fdd8a2fddbd4c4d134 include/mesos/slave/isolator.hpp 22f1e3686f50c3b9290561aa7e5073e24a702824 include/mesos/slave/isolator.proto 3d9222be5e9bd9e9f665fb2e57db6b7e925c8fbd src/Makefile.am 0794969b4b147e0f837006f2f2eba6c6b28eb332 src/cli/python/mesos/__init__.py 028b0d27fb193bac96f2a6a3201ee4cc8fd369ef src/cli/python/mesos/cli.py src/cli/python/mesos/futures.py src/cli/python/mesos/http.py src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b src/common/protobuf_utils.hpp a4708ed286ef237f17d9fd7813be2f6e7218b42a src/common/protobuf_utils.cpp 3cb684598d0492a2f57b46fabcf13565ff42f27a src/master/allocator/mesos/hierarchical.hpp e278139f856888d6c6f538f7c0f664087e97f629 src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f src/master/master.hpp 2331173fb9aeca77227fb09b899cb123eb205b4b src/master/master.cpp c584cb9f5aeb6806657059a3204ce1c680d4214a src/python/cli/src/mesos/__init__.py PRE-CREATION src/python/interface/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/native/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/protocol/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/slave/containerizer/isolator.hpp 710c584f95d60c1931b40ca041409aa819a06cba src/slave/containerizer/isolator.cpp ed610f9f8fe328fb3b73f620858dc632725e51f8 src/slave/containerizer/isolators/cgroups/cpushare.hpp 6b980f26fe8bb51dd989a0578337bc13dbd087ad src/slave/containerizer/isolators/cgroups/cpushare.cpp 907d7e78bfb591197e150ac053bb857d15a1e6dc src/slave/containerizer/isolators/cgroups/mem.hpp e831878ab47b8455a4831ebe305373130b194a40 src/slave/containerizer/isolators/cgroups/mem.cpp e343d0b9751b46bc5a4a8ccd32c0b2745e110e6b src/slave/containerizer/isolators/cgroups/perf_event.hpp 73f245bc9166e1f7550466ddd97113c63ce44e73 src/slave/containerizer/isolators/cgroups/perf_event.cpp 0e421cb6ad3e04b71746033ab15d0f1695fcd5e7 src/slave/containerizer/isolators/filesystem/posix.hpp 4c7a6f2b7530c88c34d533dba9593006ad5284b2 src/slave/containerizer/isolators/filesystem/posix.cpp 4861ee13fc34eef03d28f26d57a7d11aebed81a6 src/slave/containerizer/isolators/filesystem/shared.hpp 45e4ba09993e7b77f2df45a5c86bc00fa2d83977 src/slave/containerizer/isolators/filesystem/shared.cpp f3c2916b5c5bddbf3f7220ae75ef75fe8e7d7cae src/slave/containerizer/isolators/namespaces/pid.hpp 858e43683c88ac62abfc74ff28e8073895cf6f64 src/slave/containerizer/isolators/namespaces/pid.cpp 8e643f4afae8c24cd4d68aa349148b6f402b286b src/slave/containerizer/isolators/network/port_mapping.hpp 2599c9800e3edf12ec883b31c280324b24b195c5 src/slave/containerizer/isolators/network/port_mapping.cpp 3f6e9df8711995d0dd3903c6170fdd5ad61aac5a src/slave/containerizer/isolators/posix.hpp ef19749c0d5b795fee54d67cfc0d983b0f7084ec src/slave/containerizer/isolators/posix/disk.hpp 9fa584ff4a2f3c90c4d81aecefbcba57fa2294ad src/slave/containerizer/isolators/posix/disk.cpp 6dda77bad7ab135b6d339a80b98a291ea7120e95 src/slave/containerizer/launcher.hpp 8cc832e99838683c14d8158bdf52a267299508c7 src/slave/containerizer/launcher.cpp 5267eecbdf31c88cd3c7dc7172b5aff7bac534b0 src/slave/containerizer/linux_launcher.hpp bf6bf3f27dac007429c224d4f6798ac68316accb src/slave/containerizer/linux_launcher.cpp 12acf4d08cb7fb1161820cf4bb480fd1bc2c6858 src/slave/containerizer/mesos/containerizer.hpp
Re: Review Request 36819: Use setup.py in python cli package.
On July 27, 2015, 5:58 p.m., Marco Massenzio wrote: thanks for doing this! (this may also fix a long-standing issue of mine: https://issues.apache.org/jira/browse/MESOS-2337 - could you please take a look?) Only a few minor nits about formatting and stuff, then I think this is good. We'll need to find a committer to push this. Did you test it? on which platforms? Hi, @marco. Thank you for your review. I test it on CentOS 6.6. Could you review this again? Thank you very much. - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/#review93141 --- On July 29, 2015, 7:49 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/ --- (Updated July 29, 2015, 7:49 a.m.) Review request for mesos, Benjamin Hindman and Marco Massenzio. Bugs: MESOS-3149 https://issues.apache.org/jira/browse/MESOS-3149 Repository: mesos Description --- Use setup.py in python cli package. Diffs - Makefile.am f8e958d376efa55ace7c5727a31b8e747403641e bin/mesos.sh.in 5cbeac4330a9f45fc6d54b8c2d383f48e4098f95 configure.ac 546c9bbf775a4ef481fafb3a58c85c6d80e19500 src/Makefile.am 0794969b4b147e0f837006f2f2eba6c6b28eb332 src/cli/python/mesos/__init__.py 028b0d27fb193bac96f2a6a3201ee4cc8fd369ef src/cli/python/mesos/cli.py src/cli/python/mesos/futures.py src/cli/python/mesos/http.py src/python/cli/src/mesos/__init__.py PRE-CREATION src/python/interface/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/native/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/protocol/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 Diff: https://reviews.apache.org/r/36819/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.
On July 16, 2015, 3:04 p.m., Alexander Rukletsov wrote: src/master/http.cpp, lines 1325-1332 https://reviews.apache.org/r/35983/diff/1/?file=994085#file994085line1325 Why do we need to recover resources for unreserve? Michael Park wrote: If reserved resources are offered, we need to recover the reserved resources before we proceed to updating them to be unreserved. Alexander Rukletsov wrote: But you recover all active offers on the slave, not just those with dynamic reservations, or am I missing something? Michael Park wrote: I see what you're saying I think. I've updated the code to only rescind the offer if rescinding the offer will contribute to satisfying the request, which I think is more accurate than checking that the offer contains dynamically reserved resources. I think your change is fine and will do the job. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35983/#review91890 --- On July 28, 2015, 9:06 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35983/ --- (Updated July 28, 2015, 9:06 p.m.) Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2600 https://issues.apache.org/jira/browse/MESOS-2600 Repository: mesos Description --- See summary. Diffs - src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 src/master/master.cpp 5b5e3c37d4433c8524db267866aebc0a35a181f1 Diff: https://reviews.apache.org/r/35983/diff/ Testing --- `make check` Thanks, Michael Park
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 29, 2015, 1:52 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs (updated) - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Review Request 36909: Add shutdownNetwork in MesosZooKeeperTest.TearDownTestCase.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36909/ --- Review request for mesos. Repository: mesos Description --- Add shutdownNetwork in MesosZooKeeperTest.TearDownTestCase. Diffs - src/tests/mesos.hpp 192067d90aec2a114091e413e92e1ced10225afc Diff: https://reviews.apache.org/r/36909/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 35702: Added /reserve HTTP endpoint to the master.
On June 22, 2015, 1:32 p.m., Alexander Rukletsov wrote: src/master/master.cpp, line 749 https://reviews.apache.org/r/35702/diff/6/?file=989449#file989449line749 I think reserve is too abstract and may collide with future actions (think quota). How about `/dynamic/reserve`? Alexander Rukletsov wrote: Though we currently do not support slashes in endpoints, I think we should fix that first before introducing a `/reserve` endpoint, given these endpoint are not targeted for 0.23. Joris Van Remoortere wrote: Cody had some patches for enabling sub namespaces in endpoints (as in enabling slashes). Might be worth pulling those in. Alexander Rukletsov wrote: Yep, it's https://issues.apache.org/jira/browse/MESOS-2130, I plan to bring up the discussion today at the community sync. Michael Park wrote: The concensus for now seems that (1) we introduce the allocator changes, but address the allocator refactor sooner rather than later, (2) go with `/reserve` for now and update them once the HTTP API folks get to supporting the nested endpoint stuff. Alexander Rukletsov wrote: And (3) we update endpoints names before the following release, i.e. there is no Mesos release, where `/reserve` will exist. Correct? Michael Park wrote: That is the ideal outcome. But if we commit this now/soon, whether we can update the names before 0.24.0 gets out entirely depends on whether the nested endpoint names capabilities get committed on time. I think we agreed that we should, in order to avoid deprecation cycle for old endpoints. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/#review88781 --- On July 28, 2015, 9:03 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/ --- (Updated July 28, 2015, 9:03 p.m.) Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2600 https://issues.apache.org/jira/browse/MESOS-2600 Repository: mesos Description --- This involved a lot more challenges than I anticipated, I've captured the various approaches and limitations and deal-breakers of those approaches here: [Master Endpoint Implementation Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#) Key points: * This is a stop-gap solution until we shift the offer creation/management logic from the master to the allocator. * `updateAvailable` and `updateSlave` are kept separate because (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not. (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not. (3) `updateAvailable` never leaves the allocator in an over-allocated state and must not, whereas `updateSlave` does, and can. * The algorithm: * Initially, the master pessimistically assume that what seems like available resources will be gone. This is due to the race between the allocator scheduling an `allocate` call to itself vs master's `allocator-updateAvailable` invocation. As such, we first try to satisfy the request only with the offered resources. * We greedily rescind one offer at a time until we've rescinded sufficiently many offers. IMPORTANT: We perform `recoverResources(..., Filters())` rather than `recoverResources(..., None())` so that we can pretty much always win the race against `allocate`. In the case that we lose, no disaster occurs. We simply fail to satisfy the request. * If we still don't have enough resources after resciding all offers, be optimistic and forward the request to the allocator since there may be available resources to satisfy the request. * If the allocator returns a failure, report the error to the user with `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` maybe as well. We'll pick one eventually. This approach is clearly not ideal, since we would prefer to rescind as little offers as possible. The challenges of implementing the ideal solution in the current state is described in the document above. TODO(mpark): Add more comments and test cases. Diffs - src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 src/master/master.cpp 5b5e3c37d4433c8524db267866aebc0a35a181f1 src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 Diff:
Re: Review Request 35702: Added /reserve HTTP endpoint to the master.
On July 13, 2015, 4:46 p.m., Alexander Rukletsov wrote: src/master/http.cpp, lines 515-516 https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line515 It looks like we actually have the role, but it's buried in resources. Do you envision having resources collection with various roles in one request? Maybe it makes sense to add a validation step which ensures there is just one role per request and use it here, also avoiding changes in the `validate()`function. Michael Park wrote: I didn't see a good reason to require a one role per request condition. The current interface accurately models the fact that an operator does not have a role associated to it like a framework does, and I don't think avoiding changes in the `validate()` function should have any influence in deciding how an interface behaves. If we required such a condition, the per-request atomicity guarantee comes with a limitation that it can only be for a single role. While I'm not sure of its value, I'm also not sure what we gain by requiring it from the user's perspective? Alexander Rukletsov wrote: I think I'm missing something, my understanding is that each dynamic reservation is associated with a role, regardless, who issues a reservation request. I don't think limiting users to one role per request gives them any benefit, but it looks like we can be closer to framework-issued request if we do so. What am I missing? Michael Park wrote: Your understanding is correct. Aside from the resources being associated with a role, frameworks are also associated with a role. We check that every resource is being reserved for the framework's role because a framework is associated with a role and it wouldn't make sense to allow a framework to reserve resources for a role that does not match its role. On the contrary, the same rule doesn't apply for an operator since there's no such thing as an operator's role. That's right. Let me try to reformulate my proposal. If we require an operator to reserve resources for one role per request, it can be interpreted as an operator role. An advantage here is that `validate()` method doesn't need to be changed, while a disadvantage is that this approach is a bit artificial and can lead to confusion. What do you think? - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/#review91472 --- On July 28, 2015, 9:03 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/ --- (Updated July 28, 2015, 9:03 p.m.) Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2600 https://issues.apache.org/jira/browse/MESOS-2600 Repository: mesos Description --- This involved a lot more challenges than I anticipated, I've captured the various approaches and limitations and deal-breakers of those approaches here: [Master Endpoint Implementation Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#) Key points: * This is a stop-gap solution until we shift the offer creation/management logic from the master to the allocator. * `updateAvailable` and `updateSlave` are kept separate because (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not. (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not. (3) `updateAvailable` never leaves the allocator in an over-allocated state and must not, whereas `updateSlave` does, and can. * The algorithm: * Initially, the master pessimistically assume that what seems like available resources will be gone. This is due to the race between the allocator scheduling an `allocate` call to itself vs master's `allocator-updateAvailable` invocation. As such, we first try to satisfy the request only with the offered resources. * We greedily rescind one offer at a time until we've rescinded sufficiently many offers. IMPORTANT: We perform `recoverResources(..., Filters())` rather than `recoverResources(..., None())` so that we can pretty much always win the race against `allocate`. In the case that we lose, no disaster occurs. We simply fail to satisfy the request. * If we still don't have enough resources after resciding all offers, be optimistic and forward the request to the allocator since there may be available resources to satisfy the request. * If the allocator returns a failure, report the error to the user with `PreconditionFailed`. This could be updated
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/#review93430 --- src/tests/fetcher_cache_tests.cpp (line 1460) https://reviews.apache.org/r/36773/#comment147794 ++taskIndex - Joerg Schad On July 28, 2015, 2:04 p.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 28, 2015, 2:04 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/#review93438 --- Ship it! Ship It! - Joerg Schad On July 29, 2015, 11:52 a.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 29, 2015, 11:52 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36908: Added QuotaInfo Protobuf.
On July 29, 2015, 12:21 p.m., Till Toenshoff wrote: src/Makefile.am, line 261 https://reviews.apache.org/r/36908/diff/1/?file=1024447#file1024447line261 This line looks too long. Till Toenshoff wrote: Seems we actually tolerate this - feel free to drop this issue. I was wondering as well but it is consistent with the sourrounding code, so i would drop this. - Joerg --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36908/#review93432 --- On July 29, 2015, 11:54 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36908/ --- (Updated July 29, 2015, 11:54 a.m.) Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Till Toenshoff. Bugs: MESOS-3164 https://issues.apache.org/jira/browse/MESOS-3164 Repository: mesos Description --- Added QuotaInfo Protobuf. Diffs - include/mesos/master/quota.hpp PRE-CREATION include/mesos/master/quota.proto PRE-CREATION src/Makefile.am 0794969b4b147e0f837006f2f2eba6c6b28eb332 Diff: https://reviews.apache.org/r/36908/diff/ Testing --- make distcheck Thanks, Joerg Schad
Re: Review Request 36889: FetcherTests: Use ASSERT instead of EXPECT if the subsequent logic relies on the outcome.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36889/ --- (Updated July 29, 2015, 1:11 p.m.) Review request for mesos, Bernd Mathiske and Jan Schlicht. Bugs: MESOS-3171 https://issues.apache.org/jira/browse/MESOS-3171 Repository: mesos Description --- FetcherTests: Use ASSERT instead of EXPECT if the subsequent logic relies on the outcome. Diffs - src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36889/diff/ Testing --- make check Thanks, Joerg Schad
Review Request 36910: Patch configure.ac to include $LIBS in the CRAM-MD5 check
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36910/ --- Review request for mesos. Repository: mesos Description --- [MESOS-3170] Add $LIBS to build path of the CRAM-MD5 test Diffs - b/configure.ac 546c9bbf775a4ef481fafb3a58c85c6d80e19500 Diff: https://reviews.apache.org/r/36910/diff/ Testing --- See https://github.com/apache/mesos/pull/51 Verified build against statically linked OpenSSL 1.0.1e and Cyrus-SASL 2.1.26 Thanks, Chris Heller
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 29, 2015, 2:59 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs (updated) - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36908: Added QuotaInfo Protobuf.
On July 29, 2015, 12:21 p.m., Till Toenshoff wrote: src/Makefile.am, line 261 https://reviews.apache.org/r/36908/diff/1/?file=1024447#file1024447line261 This line looks too long. Seems we actually tolerate this - feel free to drop this issue. - Till --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36908/#review93432 --- On July 29, 2015, 11:54 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36908/ --- (Updated July 29, 2015, 11:54 a.m.) Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Till Toenshoff. Bugs: MESOS-3164 https://issues.apache.org/jira/browse/MESOS-3164 Repository: mesos Description --- Added QuotaInfo Protobuf. Diffs - include/mesos/master/quota.hpp PRE-CREATION include/mesos/master/quota.proto PRE-CREATION src/Makefile.am 0794969b4b147e0f837006f2f2eba6c6b28eb332 Diff: https://reviews.apache.org/r/36908/diff/ Testing --- make distcheck Thanks, Joerg Schad
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/#review93441 --- include/mesos/mesos.proto (line 399) https://reviews.apache.org/r/36663/#comment147803 `ip` and `port` are required, while `address` is optional. Is it intentional / doesn't it introduce a pitfall? - Alexander Rukletsov On July 25, 2015, 12:36 a.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36663/ --- (Updated July 25, 2015, 12:36 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 - CHANGELOG 1a8649d00f3bcc792a75c11dedeb42a667e2ce88 include/mesos/mesos.proto e015c81d5052214ef8207642e23b3892a6123c9a src/common/protobuf_utils.cpp d900707ae64ad92c0c0ddc2996324b61121c8594 src/master/master.cpp 7796630a93705bd62157e98e1e4855c68ea7cd0a src/tests/master_contender_detector_tests.cpp d7a3b46b2e437818631064ae34317e49c9aa3748 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 36910: Patch configure.ac to include $LIBS in the CRAM-MD5 check
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36910/ --- (Updated July 29, 2015, 1:41 p.m.) Review request for mesos and Till Toenshoff. Changes --- Added JIRA issue. Bugs: MESOS-3170 https://issues.apache.org/jira/browse/MESOS-3170 Repository: mesos Description --- [MESOS-3170] Add $LIBS to build path of the CRAM-MD5 test Diffs - b/configure.ac 546c9bbf775a4ef481fafb3a58c85c6d80e19500 Diff: https://reviews.apache.org/r/36910/diff/ Testing --- See https://github.com/apache/mesos/pull/51 Verified build against statically linked OpenSSL 1.0.1e and Cyrus-SASL 2.1.26 Thanks, Chris Heller
Re: Review Request 36816: Support HTTP checks in Mesos health check program
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36816/#review93550 --- Patch looks great! Reviews applied: [36816] All tests passed. - Mesos ReviewBot On July 30, 2015, 2:47 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36816/ --- (Updated July 30, 2015, 2:47 a.m.) Review request for mesos and Adam B. Bugs: MESOS-2533 https://issues.apache.org/jira/browse/MESOS-2533 Repository: mesos Description --- Support HTTP checks in Mesos health check program Diffs - include/mesos/mesos.proto a6748d1cd82238f005c6a49c70d22d095462f1ba src/health-check/main.cpp 97b25716335ec5719c1100bd73d06b7fc98036c9 src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea Diff: https://reviews.apache.org/r/36816/diff/ Testing --- * Add a new unit test: HealthCheckTest.HealthyTaskThroughHttp make check Thanks, haosdent huang
Re: Review Request 36627: Fixed cgroups oom killer and memory pressure tests on Ubuntu 14.04.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36627/ --- (Updated July 29, 2015, 4:14 p.m.) Review request for mesos, Benjamin Hindman and Joris Van Remoortere. Changes --- Remove some extra whitespace. Bugs: MESOS-2660 https://issues.apache.org/jira/browse/MESOS-2660 Repository: mesos Description --- See summary. Diffs (updated) - src/tests/containerizer/cgroups_tests.cpp caecd5dfa3fef33dba35cfc1b5934a11e2cc961a src/tests/containerizer/memory_test_helper.cpp 48a35632786963f484f66642b5c67afd4f7a89cc 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 inactive_file:0KB active_file:0KB unevictable:65536KB Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052467] [ pid ] uid tgid total_vm rss nr_ptes swapents oom_score_adj name Jul 22 14:56:00 harutyunyan-virtual-machine kernel: [17440.052574] [76599] 0 765998245231766 1221 0 lt-memory-test- Jul 22 14:56:00 harutyunyan-virtual-machine kernel:
Re: Review Request 36900: Publish MasterInfo to ZK in JSON format
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36900/#review93533 --- Patch looks great! Reviews applied: [36900] All tests passed. - Mesos ReviewBot On July 29, 2015, 9:07 p.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36900/ --- (Updated July 29, 2015, 9:07 p.m.) Review request for mesos, Anand Mazumdar, Isabel Jimenez, and Vinod Kone. Bugs: MESOS-3135 https://issues.apache.org/jira/browse/MESOS-3135 Repository: mesos Description --- As part of the support for non-libmesos linked client to interact with the HTTP API, we also provide the Master information stored in ZooKeeper in a way that is deserializable also for clients that do not necessarily know how to decode a MasterInfo protocol buffer. This patch publishes the data in JSON format and has been tested, summarily, with 2 masters running this patch alongside a 0.23 Master and one each of 0.24 and 0.23 Agent nodes. Please note this patch does not require specific unit testing, as the functionality is already widely tested in existing `ZooKeeper*` tests, as well as `MasterDetector*` ones. Diffs - src/master/contender.cpp 2af70c7d1acf274db40cae1641cd305249a471ec Diff: https://reviews.apache.org/r/36900/diff/ Testing --- make check also ran: 3x Masters (2x 0.24 + 1x 0.23) / 2x Slaves (1x 0.23 + 1x 0.24) the 0.24's were running this patch. Also verified in ZK that the data was correct JSON: ``` { address:{ hostname:10.0.77.243, ip:10.0.77.243, port:5050 }, hostname:10.0.77.243, id:20150728-165830-4081909770-5050-64014, ip:4081909770, pid:master@10.0.77.243:5050, port:5050, version:0.24.0 } ``` Not sure why Reviewbot failed, but the tests all passed: ``` [--] Global test environment tear-down [==] 684 tests from 97 test cases ran. (274274 ms total) [ PASSED ] 684 tests. YOU HAVE 12 DISABLED TESTS ``` the error seems to be with files left in `src/credentials` and `src/master/replicated_log`: ``` ERROR: files left in build directory after distclean: ./src/credentials ./src/master/replicated_log/CURRENT ./src/master/replicated_log/LOG ./src/master/replicated_log/MANIFEST-06 ./src/master/replicated_log/11.sst ./src/master/replicated_log/LOCK ./src/master/replicated_log/09.log ./src/master/replicated_log/LOG.old make[1]: *** [distcleancheck] Error 1 ``` Am I missing something here? Thanks, Marco Massenzio
Re: Review Request 36927: Pulled apart authorization and authentication validation for frameworks in the master.
On July 30, 2015, 12:49 a.m., Vinod Kone wrote: src/master/master.cpp, lines 1757-1779 https://reviews.apache.org/r/36927/diff/1/?file=1024921#file1024921line1757 checking validationError.isNone() in each if statement looks a bit weird. how about doing these in an else if instead? if (frameworkInfo.has_id() !(frameworkInfo.id() == )) { } else if (!roles.contains(frameworkInfo.role()) { } else if (frameworkInfo.user() == root !flags.root_submissions) { } else { } i think the above is easier to read? That's originally what I did, but it's harder to read that way. It looks saner in your example because you used empty blocks. Here is what it would actually look like: ``` // TODO(vinod): Add != operator for FrameworkID. if (frameworkInfo.has_id() !(frameworkInfo.id() == )) { validationError = Error(Registering with 'id' already set); } else if (!roles.contains(frameworkInfo.role())) { // TODO(vinod): Deprecate this in favor of ACLs. validationError = Error(Role ' + frameworkInfo.role() + ' is not + present in the master's --roles); } else if (frameworkInfo.user() == root !flags.root_submissions) { // TODO(vinod): Deprecate this in favor of authorization. validationError = Error(User 'root' is not allowed to run frameworks without --root_submissions set); } ``` Also the it doesn't work so nicely for reregister because one of the checks requires looping through framework ids. - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36927/#review93532 --- On July 29, 2015, 11:52 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36927/ --- (Updated July 29, 2015, 11:52 p.m.) Review request for mesos and Vinod Kone. Repository: mesos Description --- Framework authorization is now done through `authorizeFramework` which is consistent with `authorizeTask`. Also, `validateFrameworkAuthentication` captures the validation for authentication. I also made registration and re-registration consistent: * Both perform the check for root submission, rather than just registration. * Authentication checks in `_registerFramework` and `_reregisterFramework` are now comprehensive (thanks to `validateFrameworkAuthentication`). Diffs - src/master/master.hpp 1135049d34f8ed4d56c795ad28c9ca1d07237b63 src/master/master.cpp 2bfec2f69375444925252480142ee409b8474761 Diff: https://reviews.apache.org/r/36927/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/ --- (Updated July 29, 2015, 3:04 p.m.) Review request for mesos and Alexander Rukletsov. Changes --- Addressed comments. Bugs: MESOS-3073 https://issues.apache.org/jira/browse/MESOS-3073 Repository: mesos Description --- Added /quota HTTP Endpoint for Quota handling. Diffs (updated) - src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f src/master/master.hpp 2331173fb9aeca77227fb09b899cb123eb205b4b src/master/master.cpp c584cb9f5aeb6806657059a3204ce1c680d4214a src/master/quota_handler.hpp PRE-CREATION Diff: https://reviews.apache.org/r/36913/diff/ Testing --- make check Thanks, Joerg Schad
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
On July 29, 2015, 3:34 p.m., Bernd Mathiske wrote: src/tests/fetcher_cache_tests.cpp, line 1474 https://reviews.apache.org/r/36773/diff/7/?file=1024458#file1024458line1474 const Path Jan Schlicht wrote: Path::basename() is not marked const (which it probably should), hence we have to iterate over Path by value. Created a ticket for that: https://issues.apache.org/jira/browse/MESOS-3173 - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/#review93443 --- On July 29, 2015, 3:59 p.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 29, 2015, 3:59 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36909: Call parent SetUp() and TearDown() in MesosZooKeeperTest.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36909/ --- (Updated July 29, 2015, 1:56 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-3168 https://issues.apache.org/jira/browse/MESOS-3168 Repository: mesos Description --- Call parent SetUp() and TearDown() in MesosZooKeeperTest. Diffs - src/tests/mesos.hpp 192067d90aec2a114091e413e92e1ced10225afc Diff: https://reviews.apache.org/r/36909/diff/ Testing (updated) --- make check Thanks, haosdent huang
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 29, 2015, 3:59 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs (updated) - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36909: Call parent SetUp() and TearDown() in MesosZooKeeperTest.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36909/ --- (Updated July 29, 2015, 1:53 p.m.) Review request for mesos. Summary (updated) - Call parent SetUp() and TearDown() in MesosZooKeeperTest. Repository: mesos Description (updated) --- Call parent SetUp() and TearDown() in MesosZooKeeperTest. Diffs (updated) - src/tests/mesos.hpp 192067d90aec2a114091e413e92e1ced10225afc Diff: https://reviews.apache.org/r/36909/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 36909: Call parent SetUp() and TearDown() in MesosZooKeeperTest.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36909/ --- (Updated July 29, 2015, 1:53 p.m.) Review request for mesos. Bugs: MESOS-3168 https://issues.apache.org/jira/browse/MESOS-3168 Repository: mesos Description --- Call parent SetUp() and TearDown() in MesosZooKeeperTest. Diffs - src/tests/mesos.hpp 192067d90aec2a114091e413e92e1ced10225afc Diff: https://reviews.apache.org/r/36909/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/#review93447 --- src/tests/fetcher_cache_tests.cpp (line 1474) https://reviews.apache.org/r/36773/#comment147817 Path::basename() is not marked const (which it probably should), hence we have to iterate over Path by value. - Jan Schlicht On July 29, 2015, 3:59 p.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 29, 2015, 3:59 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
On July 29, 2015, 3:34 p.m., Bernd Mathiske wrote: src/tests/fetcher_cache_tests.cpp, line 1474 https://reviews.apache.org/r/36773/diff/7/?file=1024458#file1024458line1474 const Path Path::basename() is not marked const (which it probably should), hence we have to iterate over Path by value. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/#review93443 --- On July 29, 2015, 3:59 p.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 29, 2015, 3:59 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Review Request 36912: Fixed Mesos version in getting started docs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36912/ --- Review request for mesos. Bugs: MESOS-3172 https://issues.apache.org/jira/browse/MESOS-3172 Repository: mesos Description --- Fixed Mesos version in getting started docs. Diffs - docs/getting-started.md 38b0fe8a52a122dc41eff7fe498ed865b2c91051 Diff: https://reviews.apache.org/r/36912/diff/ Testing --- Thanks, Ryuichi Okumura
Re: Review Request 36908: Added QuotaInfo Protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36908/#review93445 --- include/mesos/master/quota.hpp (lines 19 - 20) https://reviews.apache.org/r/36908/#comment147819 I know we are utterly inconsistent here, but my suggestion would be `__MESOS_MASTER_QUOTA_PROTO_HPP__` include/mesos/master/quota.proto (line 26) https://reviews.apache.org/r/36908/#comment147818 Let's add a comment here. Something like: ``` To stay in tune with the dynamic reservation mechanism and be able to leverage role weights, quota is reserved per role and not per framework. This behaviour may change in the future, and if so then most likely in coordination with dynamic reservations. ``` include/mesos/master/quota.proto (lines 37 - 40) https://reviews.apache.org/r/36908/#comment147816 I think we should remove it for now, since we do not plan to implement it right now, but let's leave a TODO so that folks understand that quota != limit/ include/mesos/master/quota.proto (lines 44 - 49) https://reviews.apache.org/r/36908/#comment147815 Let's remove it for now, it's not part of the MVP. src/Makefile.am (lines 154 - 155) https://reviews.apache.org/r/36908/#comment147824 Hard tabs here and everywhere else, please! src/Makefile.am (line 261) https://reviews.apache.org/r/36908/#comment147822 Can we do something like `master/%.pb.cc ../include/mesos/master/%.pb.h: $(ALLOCATOR_PROTO) $(QUOTA_PROTO)` for clarity? src/Makefile.am (lines 482 - 484) https://reviews.apache.org/r/36908/#comment147823 Hard tabs here as well, please. - Alexander Rukletsov On July 29, 2015, 11:54 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36908/ --- (Updated July 29, 2015, 11:54 a.m.) Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Till Toenshoff. Bugs: MESOS-3164 https://issues.apache.org/jira/browse/MESOS-3164 Repository: mesos Description --- Added QuotaInfo Protobuf. Diffs - include/mesos/master/quota.hpp PRE-CREATION include/mesos/master/quota.proto PRE-CREATION src/Makefile.am 0794969b4b147e0f837006f2f2eba6c6b28eb332 Diff: https://reviews.apache.org/r/36908/diff/ Testing --- make distcheck Thanks, Joerg Schad
Review Request 36911: Removed unnecessary using directive.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36911/ --- Review request for mesos, Marco Massenzio and Till Toenshoff. Repository: mesos Description --- Removed unnecessary using directive. Diffs - src/master/master.cpp c584cb9f5aeb6806657059a3204ce1c680d4214a Diff: https://reviews.apache.org/r/36911/diff/ Testing --- make check (clang on Mac OS 10.10.4) Thanks, Alexander Rukletsov
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/#review93443 --- docs/fetcher-cache-internals.md (line 107) https://reviews.apache.org/r/36773/#comment147810 s/removes the/removes src/tests/fetcher_cache_tests.cpp (line 1433) https://reviews.apache.org/r/36773/#comment147812 const int i? src/tests/fetcher_cache_tests.cpp (line 1455) https://reviews.apache.org/r/36773/#comment147813 Indentation for args is 4 spaces. src/tests/fetcher_cache_tests.cpp (line 1474) https://reviews.apache.org/r/36773/#comment147814 const Path - Bernd Mathiske On July 29, 2015, 5:59 a.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 29, 2015, 5:59 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36889: FetcherTests: Use ASSERT instead of EXPECT if the subsequent logic relies on the outcome.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36889/#review93444 --- Ship it! Ship It! - Bernd Mathiske On July 29, 2015, 6:11 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36889/ --- (Updated July 29, 2015, 6:11 a.m.) Review request for mesos, Bernd Mathiske and Jan Schlicht. Bugs: MESOS-3171 https://issues.apache.org/jira/browse/MESOS-3171 Repository: mesos Description --- FetcherTests: Use ASSERT instead of EXPECT if the subsequent logic relies on the outcome. Diffs - src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36889/diff/ Testing --- make check Thanks, Joerg Schad