Re: Review Request 36911: Removed unnecessary using directive.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36911/#review94036 --- Ship it! Ship It! - Alexander Rojas On Aug. 3, 2015, 4:58 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36911/ --- (Updated Aug. 3, 2015, 4:58 p.m.) 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 36913: Added /quota HTTP Endpoint for Quota handling.
On Aug. 4, 2015, 3:08 a.m., Alexander Rukletsov wrote: src/master/master.hpp, line 766 https://reviews.apache.org/r/36913/diff/7/?file=1028501#file1028501line766 s/and / See my previous review. This is correct syntax IMHO. - Bernd --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/#review94030 --- On Aug. 4, 2015, 2:52 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/ --- (Updated Aug. 4, 2015, 2:52 a.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 ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 src/master/quota_handler.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36913/diff/ Testing --- make check Thanks, Joerg Schad
Review Request 37072: Added test for allocator update on scheduler failover
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37072/ --- Review request for mesos and Vinod Kone. Bugs: MESOS-2880 https://issues.apache.org/jira/browse/MESOS-2880 Repository: mesos Description --- Added test for allocator update on scheduler failover Diffs - src/tests/oversubscription_tests.cpp c7a2dacb600d7703de6090e7e47f453a3d08b53a Diff: https://reviews.apache.org/r/37072/diff/ Testing --- make check Thanks, Aditi Dixit
Re: Review Request 36847: Added HTTP Delete Method.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36847/#review94040 --- 3rdparty/libprocess/include/process/http.hpp (line 747) https://reviews.apache.org/r/36847/#comment148544 This comment does not match the function signature. 3rdparty/libprocess/include/process/http.hpp (line 763) https://reviews.apache.org/r/36847/#comment148545 s/pid/upid 3rdparty/libprocess/include/process/http.hpp (line 764) https://reviews.apache.org/r/36847/#comment148546 Please explain what happens if it is none. 3rdparty/libprocess/include/process/http.hpp (line 765) https://reviews.apache.org/r/36847/#comment148547 s/header/headers 3rdparty/libprocess/src/http.cpp (line 938) https://reviews.apache.org/r/36847/#comment148548 Because of this concatenation we should explain at the function delcaration in http.hpp that it is expected that `path` starts with /. Or is it not? - Bernd Mathiske On Aug. 3, 2015, 11:45 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36847/ --- (Updated Aug. 3, 2015, 11:45 a.m.) Review request for mesos, Alexander Rukletsov and Bernd Mathiske. Bugs: MESOS-3152 https://issues.apache.org/jira/browse/MESOS-3152 Repository: mesos Description --- see summary. Diffs - 3rdparty/libprocess/include/process/http.hpp b8d930058aa3ae9d21ae0f7b420d49f2448f84b1 3rdparty/libprocess/src/http.cpp 4dcbd74b894b483c4d166c23fac55ac8dba75166 3rdparty/libprocess/src/tests/http_tests.cpp ecbcbd552ac834659860627c82628ed38e6139b3 Diff: https://reviews.apache.org/r/36847/diff/ Testing --- make check Thanks, Joerg Schad
Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.
On Aug. 4, 2015, 10:08 a.m., Alexander Rukletsov wrote: src/master/master.hpp, line 766 https://reviews.apache.org/r/36913/diff/7/?file=1028501#file1028501line766 s/and / Bernd Mathiske wrote: See my previous review. This is correct syntax IMHO. See Bernds comment below. - Joerg --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/#review94030 --- On Aug. 4, 2015, 9:52 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/ --- (Updated Aug. 4, 2015, 9:52 a.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 ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 src/master/quota_handler.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36913/diff/ Testing --- make check Thanks, Joerg Schad
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 Aug. 4, 2015, 10:40 a.m.) Review request for mesos, Alexander Rukletsov and Bernd Mathiske. Changes --- Doxygenized class 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 ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 src/master/quota_handler.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36913/diff/ Testing --- make check Thanks, Joerg Schad
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/#review94041 --- Ship it! Pending consistency with subsequent RRs in this chain, this LGTM. - Bernd Mathiske On Aug. 4, 2015, 3:40 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/ --- (Updated Aug. 4, 2015, 3:40 a.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 ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 src/master/quota_handler.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36913/diff/ Testing --- make check Thanks, Joerg Schad
Re: Review Request 37072: Added test for allocator update on scheduler failover
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37072/#review94050 --- Patch looks great! Reviews applied: [37072] All tests passed. - Mesos ReviewBot On Aug. 4, 2015, 10:42 a.m., Aditi Dixit wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37072/ --- (Updated Aug. 4, 2015, 10:42 a.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2880 https://issues.apache.org/jira/browse/MESOS-2880 Repository: mesos Description --- Added test for allocator update on scheduler failover Diffs - src/tests/oversubscription_tests.cpp c7a2dacb600d7703de6090e7e47f453a3d08b53a Diff: https://reviews.apache.org/r/37072/diff/ Testing --- make check Thanks, Aditi Dixit
Re: Review Request 36402: Adding 'Accept' header in request
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36402/#review94019 --- Patch looks great! Reviews applied: [36402] All tests passed. - Mesos ReviewBot On Aug. 3, 2015, 8:19 p.m., Isabel Jimenez wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36402/ --- (Updated Aug. 3, 2015, 8:19 p.m.) Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and Vinod Kone. Repository: mesos-incubating Description --- Adding a method for Accept header in request + refactor of Accept-Encoding Diffs - 3rdparty/libprocess/include/process/http.hpp b8d9300 3rdparty/libprocess/src/http.cpp 4dcbd74 3rdparty/libprocess/src/tests/http_tests.cpp ecbcbd5 Diff: https://reviews.apache.org/r/36402/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 36050: Added test authorizer module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36050/#review94027 --- Ship it! Ship It! - Jan Schlicht On Aug. 3, 2015, 11:47 a.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36050/ --- (Updated Aug. 3, 2015, 11:47 a.m.) Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2947 https://issues.apache.org/jira/browse/MESOS-2947 Repository: mesos Description --- Adds a test authorizer module. Updates the authorization tests in order to perform typed tests on the default authorizer implementation and on the test authorizer module. Diffs - src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 src/examples/test_authorizer_module.cpp PRE-CREATION src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 src/tests/module.hpp 03756a2536ca3e662ba422e96d121a6c39bb8c84 src/tests/module.cpp 61d4753f0f098005f56dd4a24984e30405c32558 Diff: https://reviews.apache.org/r/36050/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 37046: Merged registerFramework() and reregisterFramework().
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37046/#review94028 --- Patch looks great! Reviews applied: [37046] All tests passed. - Mesos ReviewBot On Aug. 3, 2015, 9:47 p.m., Vinod Kone wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37046/ --- (Updated Aug. 3, 2015, 9:47 p.m.) Review request for mesos, Anand Mazumdar and Ben Mahler. Bugs: MESOS-3182 https://issues.apache.org/jira/browse/MESOS-3182 Repository: mesos Description --- Merged registerFramework() and reregisterFramework() (and their corresponding continuations) into subscribe() (and _subscribe()). Diffs - src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 Diff: https://reviews.apache.org/r/37046/diff/ Testing --- make check Thanks, Vinod Kone
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/#review94030 --- src/master/master.hpp (line 766) https://reviews.apache.org/r/36913/#comment148533 s/and / - Alexander Rukletsov On Aug. 4, 2015, 9:52 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/ --- (Updated Aug. 4, 2015, 9:52 a.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 ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 src/master/quota_handler.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36913/diff/ Testing --- make check Thanks, Joerg Schad
Re: Review Request 36048: Updated authorizer to allow for modularized implementations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36048/ --- (Updated Aug. 4, 2015, 4:13 p.m.) Review request for mesos, Adam B, Alexander Rukletsov, Bernd Mathiske, Kapil Arya, Jan Schlicht, and Till Toenshoff. Changes --- Rebase, changes requested by reviewers. Bugs: MESOS-2946 https://issues.apache.org/jira/browse/MESOS-2946 Repository: mesos Description --- Splits and updates the original declaration of the `Authorizer` into its interface and a default implementation, the `LocalAuthorizer`. Following the pattern of the modularized `Authenticator`, it generates a default constructor which is required when writing a `TYPED_TEST` in a follow up patch. Additionally, an initialize method has been added, needed for passing in the current ACL definitions as provided via flags. Other changes are just updates to allow for compilation. Diffs (updated) - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/authorizer/authorizer.proto PRE-CREATION include/mesos/mesos.proto a6748d1cd82238f005c6a49c70d22d095462f1ba include/mesos/type_utils.hpp 86b37ca1f63e4687af4e86731dceeb1c9c219c5e src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/authorizer/authorizer.hpp c039d9412780aa199db169b31991bf9f45b07d0f src/authorizer/authorizer.cpp 21e97e315478a4ca9442af83732665f85eb2f8fc src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 src/examples/persistent_volume_framework.cpp c6d6ed337bfca91dc146cb31298cabebdbb13509 src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 src/tests/cluster.hpp ba17c0c74a9dc36c595c4ad77fe68be94c5c7c0b src/tests/mesos.hpp 93d87c78e5665b8104dbbc3d1e8c92e515cc67ab src/tests/mesos.cpp a2a469e2a581dc6c566dafd4acd2a95c0238399f Diff: https://reviews.apache.org/r/36048/diff/ Testing --- make check Thanks, Alexander Rojas
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/#review94058 --- Patch looks great! Reviews applied: [36913] All tests passed. - Mesos ReviewBot On Aug. 4, 2015, 10:40 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/ --- (Updated Aug. 4, 2015, 10:40 a.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 ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 src/master/quota_handler.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36913/diff/ Testing --- make check Thanks, Joerg Schad
Re: Review Request 36048: Updated authorizer to allow for modularized implementations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36048/#review94059 --- include/mesos/authorizer/authorizer.hpp (line 83) https://reviews.apache.org/r/36048/#comment148560 If the request satifies the ACLs, it returns true. There's another one below. include/mesos/authorizer/authorizer.hpp (line 91) https://reviews.apache.org/r/36048/#comment148561 s/the ACL/the ACLs to be consistent with the paragraph above. - Jan Schlicht On Aug. 4, 2015, 4:13 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36048/ --- (Updated Aug. 4, 2015, 4:13 p.m.) Review request for mesos, Adam B, Alexander Rukletsov, Bernd Mathiske, Kapil Arya, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2946 https://issues.apache.org/jira/browse/MESOS-2946 Repository: mesos Description --- Splits and updates the original declaration of the `Authorizer` into its interface and a default implementation, the `LocalAuthorizer`. Following the pattern of the modularized `Authenticator`, it generates a default constructor which is required when writing a `TYPED_TEST` in a follow up patch. Additionally, an initialize method has been added, needed for passing in the current ACL definitions as provided via flags. Other changes are just updates to allow for compilation. Diffs - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/authorizer/authorizer.proto PRE-CREATION include/mesos/mesos.proto a6748d1cd82238f005c6a49c70d22d095462f1ba include/mesos/type_utils.hpp 86b37ca1f63e4687af4e86731dceeb1c9c219c5e src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/authorizer/authorizer.hpp c039d9412780aa199db169b31991bf9f45b07d0f src/authorizer/authorizer.cpp 21e97e315478a4ca9442af83732665f85eb2f8fc src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 src/examples/persistent_volume_framework.cpp c6d6ed337bfca91dc146cb31298cabebdbb13509 src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 src/tests/cluster.hpp ba17c0c74a9dc36c595c4ad77fe68be94c5c7c0b src/tests/mesos.hpp 93d87c78e5665b8104dbbc3d1e8c92e515cc67ab src/tests/mesos.cpp a2a469e2a581dc6c566dafd4acd2a95c0238399f Diff: https://reviews.apache.org/r/36048/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36908: Added QuotaInfo Protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36908/#review94014 --- Patch looks great! Reviews applied: [36908] All tests passed. - Mesos ReviewBot On Aug. 3, 2015, 7:01 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36908/ --- (Updated Aug. 3, 2015, 7:01 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 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec Diff: https://reviews.apache.org/r/36908/diff/ Testing --- make distcheck Thanks, Joerg Schad
Re: Review Request 37065: Fixed MemIsolatorTest failure on OSX.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37065/#review94001
---
Thanks for this Artem! I applied the patch and it works. Just a minor nit and a
question.
src/tests/containerizer/memory_test_helper.cpp (line 75)
https://reviews.apache.org/r/37065/#comment148500
`s/a/the/`
src/tests/containerizer/memory_test_helper.cpp (line 81)
https://reviews.apache.org/r/37065/#comment148501
What's the significance in the order in which these are called? I would've
expected something like:
```
// Make sure that all pages that are going to be mapped into the
// address space of this process become unevictable. This is needed
// for testing cgroup oom killer.
#ifdef __APPLE__
if (mlock(rss, size.bytes()) != 0)
#else
if (mlockall(MCL_FUTURE) != 0)
#endif
{
return ErrnoError(Failed to make pages to be mapped unevictable);
}
```
- Michael Park
On Aug. 4, 2015, 6:32 a.m., Artem Harutyunyan wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37065/
---
(Updated Aug. 4, 2015, 6:32 a.m.)
Review request for mesos, Benjamin Hindman and Michael Park.
Bugs: MESOS-3197
https://issues.apache.org/jira/browse/MESOS-3197
Repository: mesos
Description
---
See summary.
Diffs
-
src/tests/containerizer/memory_test_helper.cpp
5e40b747f4266e7532baf8fd02ea5db0955124d2
Diff: https://reviews.apache.org/r/37065/diff/
Testing
---
make check
Thanks,
Artem Harutyunyan
Re: Review Request 36048: Updated authorizer to allow for modularized implementations.
On Aug. 3, 2015, 12:36 p.m., Kapil Arya wrote:
I have a stupid question. Why shouldn't the authorizer.{hpp,proto} file be
placed inside `include/mesos/master/` instead since the authorizer is only
for the master. Does it make sense to do that? If it does, then should we
also move src/authorize inside src/master?
Bernd Mathiske wrote:
It's not intended to stay just for master. We are working on generalizing
how all this works for all kinds of endpoints in all kinds of Mesos
components:
https://docs.google.com/document/d/1kM3_f7DSqXcE2MuERrLTGp_XMC6ss2wmpkNYDCY5rOM/edit
Well, this doc is about authn. But authz is supposed to follow suit eventually.
- Bernd
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36048/#review93951
---
On Aug. 3, 2015, 2:47 a.m., Alexander Rojas wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36048/
---
(Updated Aug. 3, 2015, 2:47 a.m.)
Review request for mesos, Adam B, Alexander Rukletsov, Bernd Mathiske, Kapil
Arya, Jan Schlicht, and Till Toenshoff.
Bugs: MESOS-2946
https://issues.apache.org/jira/browse/MESOS-2946
Repository: mesos
Description
---
Splits and updates the original declaration of the `Authorizer` into its
interface and a default implementation, the `LocalAuthorizer`.
Following the pattern of the modularized `Authenticator`, it generates a
default constructor which is required when writing a `TYPED_TEST` in
a follow up patch. Additionally, an initialize method has been added, needed
for passing in the current ACL definitions as provided via
flags.
Other changes are just updates to allow for compilation.
Diffs
-
include/mesos/authorizer/authorizer.hpp PRE-CREATION
include/mesos/authorizer/authorizer.proto PRE-CREATION
include/mesos/mesos.proto cb24125a3f05e0d38fb22e481a15ceb21f882d27
include/mesos/type_utils.hpp 86b37ca1f63e4687af4e86731dceeb1c9c219c5e
src/Makefile.am e2cbd1524f25b3e3a9419af6bdf8cc6e5022a784
src/authorizer/authorizer.hpp c039d9412780aa199db169b31991bf9f45b07d0f
src/authorizer/authorizer.cpp 21e97e315478a4ca9442af83732665f85eb2f8fc
src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098
src/examples/persistent_volume_framework.cpp
c6d6ed337bfca91dc146cb31298cabebdbb13509
src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff
src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f
src/master/http.cpp 6f5ca02c52462495b84e77525a6c88299746ece2
src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1
src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec
src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0
src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87
src/tests/cluster.hpp ba17c0c74a9dc36c595c4ad77fe68be94c5c7c0b
src/tests/mesos.hpp 23d9841c7cfb16abef89a53d29a256a2c5e94b52
src/tests/mesos.cpp f09ef0f99573716de8905f49dcc0c9df20e31ea9
Diff: https://reviews.apache.org/r/36048/diff/
Testing
---
make check
Thanks,
Alexander Rojas
Re: Review Request 36048: Updated authorizer to allow for modularized implementations.
On Aug. 3, 2015, 12:36 p.m., Kapil Arya wrote:
I have a stupid question. Why shouldn't the authorizer.{hpp,proto} file be
placed inside `include/mesos/master/` instead since the authorizer is only
for the master. Does it make sense to do that? If it does, then should we
also move src/authorize inside src/master?
It's not intended to stay just for master. We are working on generalizing how
all this works for all kinds of endpoints in all kinds of Mesos components:
https://docs.google.com/document/d/1kM3_f7DSqXcE2MuERrLTGp_XMC6ss2wmpkNYDCY5rOM/edit
- Bernd
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36048/#review93951
---
On Aug. 3, 2015, 2:47 a.m., Alexander Rojas wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36048/
---
(Updated Aug. 3, 2015, 2:47 a.m.)
Review request for mesos, Adam B, Alexander Rukletsov, Bernd Mathiske, Kapil
Arya, Jan Schlicht, and Till Toenshoff.
Bugs: MESOS-2946
https://issues.apache.org/jira/browse/MESOS-2946
Repository: mesos
Description
---
Splits and updates the original declaration of the `Authorizer` into its
interface and a default implementation, the `LocalAuthorizer`.
Following the pattern of the modularized `Authenticator`, it generates a
default constructor which is required when writing a `TYPED_TEST` in
a follow up patch. Additionally, an initialize method has been added, needed
for passing in the current ACL definitions as provided via
flags.
Other changes are just updates to allow for compilation.
Diffs
-
include/mesos/authorizer/authorizer.hpp PRE-CREATION
include/mesos/authorizer/authorizer.proto PRE-CREATION
include/mesos/mesos.proto cb24125a3f05e0d38fb22e481a15ceb21f882d27
include/mesos/type_utils.hpp 86b37ca1f63e4687af4e86731dceeb1c9c219c5e
src/Makefile.am e2cbd1524f25b3e3a9419af6bdf8cc6e5022a784
src/authorizer/authorizer.hpp c039d9412780aa199db169b31991bf9f45b07d0f
src/authorizer/authorizer.cpp 21e97e315478a4ca9442af83732665f85eb2f8fc
src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098
src/examples/persistent_volume_framework.cpp
c6d6ed337bfca91dc146cb31298cabebdbb13509
src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff
src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f
src/master/http.cpp 6f5ca02c52462495b84e77525a6c88299746ece2
src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1
src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec
src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0
src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87
src/tests/cluster.hpp ba17c0c74a9dc36c595c4ad77fe68be94c5c7c0b
src/tests/mesos.hpp 23d9841c7cfb16abef89a53d29a256a2c5e94b52
src/tests/mesos.cpp f09ef0f99573716de8905f49dcc0c9df20e31ea9
Diff: https://reviews.apache.org/r/36048/diff/
Testing
---
make check
Thanks,
Alexander Rojas
Re: Review Request 37045: Convert Linux perf sampler to use process:await().
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37045/#review94018 --- Patch looks great! Reviews applied: [37045] All tests passed. - Mesos ReviewBot On Aug. 3, 2015, 9:10 p.m., Paul Brett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37045/ --- (Updated Aug. 3, 2015, 9:10 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Convert Linux perf sampler to use process:await(). Diffs - src/linux/perf.cpp 697b75e846a43d4f106ad8f39a18882836d7dc02 Diff: https://reviews.apache.org/r/37045/diff/ Testing --- sudo make check Thanks, Paul Brett
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/#review93562 --- src/health-check/main.cpp (line 242) https://reviews.apache.org/r/36816/#comment147942 Why is this comment relevant here? You aren't even mentioning https here. I'd remove it. include/mesos/mesos.proto (line 207) https://reviews.apache.org/r/36816/#comment147943 s/compile/is compiled/ - Adam B On Aug. 4, 2015, 2:28 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36816/ --- (Updated Aug. 4, 2015, 2:28 a.m.) Review request for mesos, Adam B and Michael Park. 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 36913: Added /quota HTTP Endpoint for Quota handling.
On Aug. 3, 2015, 2:21 p.m., Alexander Rukletsov wrote: src/master/quota_handler.hpp, lines 26-27 https://reviews.apache.org/r/36913/diff/5/?file=1027610#file1027610line26 I think we should wrap it into `master` namespace as well. `quota_handler.cpp` still lacks the `master` namespace. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/#review93905 --- On Aug. 3, 2015, 6:09 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/ --- (Updated Aug. 3, 2015, 6:09 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 ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 src/master/quota_handler.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36913/diff/ Testing --- make check Thanks, Joerg Schad
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 Aug. 4, 2015, 9:52 a.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 (updated) - src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 src/master/quota_handler.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36913/diff/ Testing --- make check Thanks, Joerg Schad
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/#review94024 --- src/master/master.hpp (line 916) https://reviews.apache.org/r/36913/#comment148528 I think we don't need this instance any more. - Alexander Rukletsov On Aug. 3, 2015, 6:09 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/ --- (Updated Aug. 3, 2015, 6:09 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 ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 src/master/quota_handler.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36913/diff/ Testing --- make check Thanks, Joerg Schad
Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/#review94023 --- Ship it! Ship It! - Jan Schlicht On Aug. 3, 2015, 11:47 a.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated Aug. 3, 2015, 11:47 a.m.) Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2947 https://issues.apache.org/jira/browse/MESOS-2947 Repository: mesos Description --- Adds and integrates helper classes needed to support an `Authorizer` module. Also adds a flag to the master, allowing the selection of an `Authorizer` module. Diffs - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am e2cbd1524f25b3e3a9419af6bdf8cc6e5022a784 src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Review Request 37075: Protobuf definitions instructing the fetcher cache about checksums and their validation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37075/ --- Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy Chen. Bugs: MESOS-2073 https://issues.apache.org/jira/browse/MESOS-2073 Repository: mesos Description --- Protobuf definitions instructing the fetcher cache about checksums and their validation. First in a series of patches implementing phase 1 of MESOS-2073 (see comments in the ticket). There are references to docs in this patch, but updating docs will be in the last patch. Diffs - include/mesos/mesos.proto a6748d1cd82238f005c6a49c70d22d095462f1ba Diff: https://reviews.apache.org/r/37075/diff/ Testing --- Thanks, Bernd Mathiske
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 Aug. 4, 2015, 1:49 p.m.) Review request for mesos and Till Toenshoff. Changes --- Rebased 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 (updated) - configure.ac 9438d88 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 37065: Fixed MemIsolatorTest failure on OSX.
On Aug. 3, 2015, 11:36 p.m., Michael Park wrote:
src/tests/containerizer/memory_test_helper.cpp, line 81
https://reviews.apache.org/r/37065/diff/1/?file=1028301#file1028301line81
What's the significance in the order in which these are called? I
would've expected something like:
```
// Make sure that all pages that are going to be mapped into the
// address space of this process become unevictable. This is needed
// for testing cgroup oom killer.
#ifdef __APPLE__
if (mlock(rss, size.bytes()) != 0)
#else
if (mlockall(MCL_FUTURE) != 0)
#endif
{
return ErrnoError(Failed to make pages to be mapped unevictable);
}
```
The mlockall(MCL_FUTURE) has to be called before the memory allocation is made
(because it affects future allocations), whereas mlock() is called for already
allocated pages. I could probably change mlockall(MCL_FUTURE) to
mlockall(MCL_CURRENT) in your snippet and make it work that way. I'll test to
verify and will update the patch accordingly. Thanks!
- Artem
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37065/#review94001
---
On Aug. 3, 2015, 11:32 p.m., Artem Harutyunyan wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37065/
---
(Updated Aug. 3, 2015, 11:32 p.m.)
Review request for mesos, Benjamin Hindman and Michael Park.
Bugs: MESOS-3197
https://issues.apache.org/jira/browse/MESOS-3197
Repository: mesos
Description
---
See summary.
Diffs
-
src/tests/containerizer/memory_test_helper.cpp
5e40b747f4266e7532baf8fd02ea5db0955124d2
Diff: https://reviews.apache.org/r/37065/diff/
Testing
---
make check
Thanks,
Artem Harutyunyan
Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.
On Aug. 4, 2015, 2:39 a.m., Alexander Rukletsov wrote: src/master/master.hpp, line 916 https://reviews.apache.org/r/36913/diff/6/?file=1027735#file1027735line916 I think we don't need this instance any more. I think so, too. - Bernd --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/#review94024 --- On Aug. 3, 2015, 11:09 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/ --- (Updated Aug. 3, 2015, 11:09 a.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 ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 src/master/quota_handler.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36913/diff/ Testing --- make check Thanks, Joerg Schad
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/#review94033 --- src/master/master.hpp (line 764) https://reviews.apache.org/r/36913/#comment148537 Let's add something like: Operates inside the Master actor. - Alexander Rukletsov On Aug. 4, 2015, 9:52 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/ --- (Updated Aug. 4, 2015, 9:52 a.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 ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 src/master/quota_handler.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36913/diff/ Testing --- make check Thanks, Joerg Schad
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/#review94037 --- src/master/master.hpp (line 766) https://reviews.apache.org/r/36913/#comment148540 See my previous review. This is correct syntax IMHO. - Bernd Mathiske On Aug. 4, 2015, 2:52 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/ --- (Updated Aug. 4, 2015, 2:52 a.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 ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 src/master/quota_handler.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36913/diff/ Testing --- make check Thanks, Joerg Schad
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/#review94054 --- Re-based the PR - Chris Heller On July 29, 2015, 1:41 p.m., Chris Heller wrote: --- 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. 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 36847: Added HTTP Delete Method.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36847/#review94067 --- Patch looks great! Reviews applied: [36847] All tests passed. - Mesos ReviewBot On Aug. 4, 2015, 10:57 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36847/ --- (Updated Aug. 4, 2015, 10:57 a.m.) Review request for mesos, Alexander Rukletsov and Bernd Mathiske. Bugs: MESOS-3152 https://issues.apache.org/jira/browse/MESOS-3152 Repository: mesos Description --- see summary. Diffs - 3rdparty/libprocess/include/process/http.hpp b8d930058aa3ae9d21ae0f7b420d49f2448f84b1 3rdparty/libprocess/src/http.cpp 4dcbd74b894b483c4d166c23fac55ac8dba75166 3rdparty/libprocess/src/tests/http_tests.cpp ecbcbd552ac834659860627c82628ed38e6139b3 Diff: https://reviews.apache.org/r/36847/diff/ Testing --- make check Thanks, Joerg Schad
Re: Review Request 36867: Add labels to FrameworkInfo.
Hi folks, Thanks for the great feedback! I addressed almost all of these issues, but had a few things left to wrap up before uploading a new patch -- I'm traveling at the moment, but I should have a new patch by tomorrow. Neil On Mon, Aug 3, 2015 at 8:45 PM Adam B a...@mesosphere.io wrote: This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36867/ On July 27th, 2015, 11:36 p.m. PDT, *Adam B* wrote: Great first patch. Thanks for updating FrameworkInfo on reregistration with the master too! A handful of nits in my first pass. I'll take another look once you've simplified the tests with Kapil's suggestions. On August 3rd, 2015, 1:33 p.m. PDT, *Niklas Nielsen* wrote: Any updates here? :) Looks like @neilc or somebody resolved some of these issues, but I don't see a new patch revision with the changes. Neil? - Adam On July 27th, 2015, 6:25 p.m. PDT, Neil Conway wrote: Review request for mesos, Adam B, Connor Doyle, and Niklas Nielsen. By Neil Conway. *Updated July 27, 2015, 6:25 p.m.* *Bugs: * MESOS-2841 https://issues.apache.org/jira/browse/MESOS-2841 *Repository: * mesos Description This is intended to support frameworks that want to offer capabilities to the rest of the cluster (e.g., storage or some arbitrary third-party service). We want processes running in the cluster to be able to discover these capabilities; however, we don't want to commit to a fixed set of capabilities or how those capabilities should be represented. Hence, this commit represents this information using freeform key-value pairs, similar to the labels mechanism already in use elsewhere. Jira: MESOS-2841 Testing make check Diffs - include/mesos/mesos.proto (a6748d1cd82238f005c6a49c70d22d095462f1ba) - src/master/http.cpp (3a1598fad4db03e5f62fd4a6bd26b2bedeee4070) - src/master/master.hpp (2c924addfb4c52d3048ee6ded13ce638145cc93f) - src/tests/fault_tolerance_tests.cpp (7b977f5e8195d9f42b21f36eb36fb156471caa20) - src/tests/master_tests.cpp (05c148ee1660b86428afe4eda718b17052743a8c) View Diff https://reviews.apache.org/r/36867/diff/
Re: Review Request 37065: Fixed MemIsolatorTest failure on OSX.
On Aug. 4, 2015, 6:36 a.m., Michael Park wrote:
src/tests/containerizer/memory_test_helper.cpp, line 81
https://reviews.apache.org/r/37065/diff/1/?file=1028301#file1028301line81
What's the significance in the order in which these are called? I
would've expected something like:
```
// Make sure that all pages that are going to be mapped into the
// address space of this process become unevictable. This is needed
// for testing cgroup oom killer.
#ifdef __APPLE__
if (mlock(rss, size.bytes()) != 0)
#else
if (mlockall(MCL_FUTURE) != 0)
#endif
{
return ErrnoError(Failed to make pages to be mapped unevictable);
}
```
Artem Harutyunyan wrote:
The mlockall(MCL_FUTURE) has to be called before the memory allocation is
made (because it affects future allocations), whereas mlock() is called for
already allocated pages. I could probably change mlockall(MCL_FUTURE) to
mlockall(MCL_CURRENT) in your snippet and make it work that way. I'll test to
verify and will update the patch accordingly. Thanks!
IMHO it would be better to add an autoconf check for ```mlockall```
availability that to check ```__APPLE__```.
- James
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37065/#review94001
---
On Aug. 4, 2015, 6:32 a.m., Artem Harutyunyan wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37065/
---
(Updated Aug. 4, 2015, 6:32 a.m.)
Review request for mesos, Benjamin Hindman and Michael Park.
Bugs: MESOS-3197
https://issues.apache.org/jira/browse/MESOS-3197
Repository: mesos
Description
---
See summary.
Diffs
-
src/tests/containerizer/memory_test_helper.cpp
5e40b747f4266e7532baf8fd02ea5db0955124d2
Diff: https://reviews.apache.org/r/37065/diff/
Testing
---
make check
Thanks,
Artem Harutyunyan
Re: Review Request 36979: Updating all references to os::shell
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36979/#review94177 --- src/slave/containerizer/isolators/network/port_mapping.cpp (line 1579) https://reviews.apache.org/r/36979/#comment148718 nit: this seems to fit on a single line. src/tests/containerizer/isolator_tests.cpp (line 1269) https://reviews.apache.org/r/36979/#comment148719 You are right that the awk did not actually seem to accomplish anything meaningful here. src/tests/containerizer/port_mapping_tests.cpp (lines 971 - 974) https://reviews.apache.org/r/36979/#comment148720 Another illustration of why a tuple return type might be a better option for os::shell() :-) But regardless, I'd change this code to something more suggestive (it's a test case after all), or at least would add a comment that clarifies the intention. src/tests/containerizer/port_mapping_tests.cpp (line 986) https://reviews.apache.org/r/36979/#comment148721 ditto. + extra newline. - Artem Harutyunyan On Aug. 4, 2015, 5:55 p.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36979/ --- (Updated Aug. 4, 2015, 5:55 p.m.) Review request for mesos, Benjamin Hindman and Artem Harutyunyan. Bugs: MESOS-3142 https://issues.apache.org/jira/browse/MESOS-3142 Repository: mesos Description --- Updating all references to os::shell For more details see MESOS-3142. Diffs - src/hdfs/hdfs.hpp a070c3200f0a0ac48ec86451749c7faf10c7f6a7 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/slave/containerizer/isolators/network/port_mapping.cpp 3f6e9df8711995d0dd3903c6170fdd5ad61aac5a src/tests/containerizer/isolator_tests.cpp ff6e2b7e190a58a4809d6e71addb15dabe418e17 src/tests/containerizer/port_mapping_tests.cpp 4bee74acba2b1472c80cabbc9d0384bd04c543aa Diff: https://reviews.apache.org/r/36979/diff/ Testing --- make check *Note*: this patch fixes breakages introduce by the refactoring in: https://reviews.apache.org/r/36978 Thanks, Marco Massenzio
Re: Review Request 37081: Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/ --- (Updated Aug. 5, 2015, 4:19 a.m.) Review request for mesos, Kapil Arya and Marco Massenzio. Changes --- A bit dirty to work arround protobuf install to $PREFIX/lib/pythonX.Y/site-packages/ Bugs: MESOS-2337 https://issues.apache.org/jira/browse/MESOS-2337 Repository: mesos Description --- Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos. Diffs (updated) - src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf src/python/interface/setup.py.in afcaf7ad6851eb36304a3be92c062cb6b21dbb03 src/python/setup.py.in 304c4bf87870fbaff2de0615cfd2263bb8a7ad3a src/slave/containerizer/isolators/filesystem/shared.cpp b30ab3fd0013045a2843fe1e8843cc120ce180c6 src/tests/http_api_tests.cpp 586d11288828fe9997e54f5dbd7d28c200e973f5 src/tests/mesos.hpp 20418d4fbd2f4ae35ee0c707472cbf37125883b0 src/tests/resources_tests.cpp 2ae93a9c8235e5e4643539d409df51c39c6d7e56 Diff: https://reviews.apache.org/r/37081/diff/ Testing --- ## Test steps ### In CentOS 6.6 ``` ./bootstrap mkdir build cd build make -j4 sudo make install ``` And then execute ``` export PYTHONPATH=/usr/local/lib/python2.6/site-packages python -c import mesos; from mesos import native; from mesos import interface ``` Check it could success or not. Also check the `__init__.py` file ``` cat /usr/local/lib/python2.6/site-packages/mesos/__init__.py ``` Thanks, haosdent huang
Re: Review Request 37081: Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/ --- (Updated Aug. 5, 2015, 4:21 a.m.) Review request for mesos, Kapil Arya and Marco Massenzio. Bugs: MESOS-2337 https://issues.apache.org/jira/browse/MESOS-2337 Repository: mesos Description --- Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos. Diffs (updated) - src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/python/interface/setup.py.in afcaf7ad6851eb36304a3be92c062cb6b21dbb03 src/python/setup.py.in 304c4bf87870fbaff2de0615cfd2263bb8a7ad3a Diff: https://reviews.apache.org/r/37081/diff/ Testing --- ## Test steps ### In CentOS 6.6 ``` ./bootstrap mkdir build cd build make -j4 sudo make install ``` And then execute ``` export PYTHONPATH=/usr/local/lib/python2.6/site-packages python -c import mesos; from mesos import native; from mesos import interface ``` Check it could success or not. Also check the `__init__.py` file ``` cat /usr/local/lib/python2.6/site-packages/mesos/__init__.py ``` Thanks, haosdent huang
Re: Review Request 37081: Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.
On Aug. 4, 2015, 5:17 p.m., Marco Massenzio wrote: Can you please define how did you test that this works in the 'tests' section? I can try this out on Ubuntu 14.04 (and OSX) given the steps to reproduce the issue, and then applying the patch. Once we verify that it works on enough platforms, we can ship it. Thanks! haosdent huang wrote: Hi, @marco. Thank you for your help. Marco Massenzio wrote: haha thank *you* for fixing this! I'm testing it out currently on my Mac - will try it out on a Ubuntu VM or my home box later this evening. Will keep you posted. Thanks for repro steps! Marco Massenzio wrote: ok - this *almost* worked :) Are we missing an `__init__.py` in `google`? ``` $ ../configure --prefix=/Users/marco/mesos-inst/lib ``` then the usual `make make install` - then: ``` $ find . |grep __init__ ./lib/python/site-packages/google/protobuf/__init__.py ./lib/python/site-packages/google/protobuf/__init__.pyc ./lib/python/site-packages/google/protobuf/compiler/__init__.py ./lib/python/site-packages/google/protobuf/compiler/__init__.pyc ./lib/python/site-packages/google/protobuf/internal/__init__.py ./lib/python/site-packages/google/protobuf/internal/__init__.pyc ./lib/python/site-packages/google/protobuf/pyext/__init__.py ./lib/python/site-packages/google/protobuf/pyext/__init__.pyc ./lib/python/site-packages/mesos/__init__.py ./lib/python/site-packages/mesos/__init__.pyc ./lib/python/site-packages/mesos/interface/__init__.py ./lib/python/site-packages/mesos/interface/__init__.pyc ./lib/python/site-packages/mesos/native/__init__.py ./lib/python/site-packages/mesos/native/__init__.pyc ./libexec/mesos/python/mesos/__init__.py ``` as you can see, there are now the `__init__.py` files in the right places under `mesos` (if you do the same from `master` there's nothing in there - so, yay! :) but there is none under the `google/` package: ``` [~/mesos-inst/lib] $ export PYTHONPATH=./lib/python/site-packages/ [~/mesos-inst/lib] $ python Python 2.7.6 (default, Sep 9 2014, 15:04:36) [GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.39)] on darwin Type help, copyright, credits or license for more information. import mesos from mesos import native Traceback (most recent call last): File stdin, line 1, in module File /Users/marco/mesos-inst/lib/lib/python/site-packages/mesos/native/__init__.py, line 17, in module from ._mesos import MesosExecutorDriverImpl File /Users/marco/mesos-inst/lib/lib/python/site-packages/mesos/interface/mesos_pb2.py, line 4, in module from google.protobuf.internal import enum_type_wrapper ImportError: No module named google.protobuf.internal import google.protobuf.internal Traceback (most recent call last): File stdin, line 1, in module ImportError: No module named google.protobuf.internal ``` This was on a Mac OSX 10.10 (Yosemite) - about to test the same on an Ubuntu 14.04 VM, brand new. haosdent huang wrote: Oh, because I already have pip install protobuf in my machine. So don't have this problem, let me check again. Hi, @marco, Could you try again? The problem of protobuf is if you never pip install protobuf before, the protobuf would install to $PREFIX/lib/pythonX.Y/site-packages. It cause this problem because google is namespace package. I add a new dependency to workarround this. - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/#review94084 --- On Aug. 5, 2015, 4:21 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/ --- (Updated Aug. 5, 2015, 4:21 a.m.) Review request for mesos, Kapil Arya and Marco Massenzio. Bugs: MESOS-2337 https://issues.apache.org/jira/browse/MESOS-2337 Repository: mesos Description --- Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos. Diffs - src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/python/interface/setup.py.in afcaf7ad6851eb36304a3be92c062cb6b21dbb03 src/python/setup.py.in 304c4bf87870fbaff2de0615cfd2263bb8a7ad3a Diff: https://reviews.apache.org/r/37081/diff/ Testing --- ## Test steps ### In CentOS 6.6 ``` ./bootstrap mkdir build cd build make -j4 sudo make install ``` And then execute ``` export PYTHONPATH=/usr/local/lib/python2.6/site-packages python -c import mesos; from mesos import native; from
Re: Review Request 37105: Removed the code of checkpointing container root filesystem path.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37105/#review94185 --- Patch looks great! Reviews applied: [36929, 36930, 36954, 36956, 37054, 37055, 37091, 37105] All tests passed. - Mesos ReviewBot On Aug. 4, 2015, 11:51 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37105/ --- (Updated Aug. 4, 2015, 11:51 p.m.) Review request for mesos, Ian Downes, Timothy Chen, Vinod Kone, and Jiang Yan Xu. Bugs: MESOS-3205 https://issues.apache.org/jira/browse/MESOS-3205 Repository: mesos Description --- Removed the code of checkpointing container root filesystem path. See ticket for the motivation. Diffs - src/common/protobuf_utils.hpp a4708ed286ef237f17d9fd7813be2f6e7218b42a src/common/protobuf_utils.cpp 3cb684598d0492a2f57b46fabcf13565ff42f27a src/slave/containerizer/mesos/containerizer.hpp 8851d30af56b4f9fb95450ac1f42ab550e3df9ff src/slave/containerizer/mesos/containerizer.cpp 6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 src/slave/containerizer/provisioner.hpp cb4d511e8189b65df9b9803f23812dd98edc44ac src/slave/paths.cpp 404c2143e70771747d356b15eac9137495fd9a75 src/slave/state.hpp cecf200e6e79172af57ae195a73a5161be7e604a src/slave/state.cpp b9f2d8a0d6395b92bd50f1e0927f3ae9fd04b81c Diff: https://reviews.apache.org/r/37105/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 37045: Convert Linux perf sampler to use process:await().
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37045/ --- (Updated Aug. 5, 2015, 5:25 a.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Convert Linux perf sampler to use process:await(). Diffs (updated) - src/linux/perf.cpp 697b75e846a43d4f106ad8f39a18882836d7dc02 Diff: https://reviews.apache.org/r/37045/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 35702: Added /reserve HTTP endpoint to the master.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35702/#review94154
---
Ship it!
LGTM overall. Please address the remaining issues and commit yourself!
src/master/http.cpp (line 475)
https://reviews.apache.org/r/35702/#comment148677
We typically use leading undescore for temp variables. The tailing
underscore is for class members (following google style).
In fact, I think the temp variable here is not necessary. There are only
two places where this temp variable is used. I would rather use
'values.get().at(...)', but this is up to you.
src/master/http.cpp (line 498)
https://reviews.apache.org/r/35702/#comment148678
Do you need this temp variable. Looks like you can just do
```
foreach (.. value, parse.get().values) {...
```
src/master/http.cpp (line 511)
https://reviews.apache.org/r/35702/#comment148679
Kill this line.
src/master/http.cpp (line 533)
https://reviews.apache.org/r/35702/#comment148685
What does 'recovered' mean and what does remaining mean? It'll be great if
you comment on that to improve readability.
For example,
```
// The resources recovered by recinding outstanding offers.
...
// The unreserved resources needed to satify the RESERVE operation.
```
IIUC, the variable 'remaining' is only used for optimization, right? Could
you please mention that in the comments that keep this variable is for
optimization (i.e., avoid rescinding unnecessary offers).
src/master/http.cpp (line 534)
https://reviews.apache.org/r/35702/#comment148683
I don't like the name 'flatten' :(
Could you at least be more explicit about it (i.e., emphasize that
'remaining' only has unreserved resources).
```
Resources remaining = resources.flatten('*');
```
src/master/http.cpp (line 553)
https://reviews.apache.org/r/35702/#comment148686
you win the race because the default filter refuse_sec is 5 seconds? Worth
mentioning that in the comment.
src/master/http.cpp (line 573)
https://reviews.apache.org/r/35702/#comment148727
What is 'Nothing' here?
src/master/master.cpp (line 5472)
https://reviews.apache.org/r/35702/#comment148729
The name sounds weired. You are passing in an offer operation but the
function name is called 'applyResourceOperation'.
I would suggest we create two 'Master::apply' overloads and don't worry
about the code duplication.
```
void apply(framework, slave, opeartion);
FutureNothing apply(slave, operation);
```
- Jie Yu
On July 31, 2015, 9:56 p.m., Michael Park wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35702/
---
(Updated July 31, 2015, 9:56 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
Re: Review Request 37081: Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/#review94191 --- did you forget to rebase? - Marco Massenzio On Aug. 5, 2015, 4:21 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/ --- (Updated Aug. 5, 2015, 4:21 a.m.) Review request for mesos, Kapil Arya and Marco Massenzio. Bugs: MESOS-2337 https://issues.apache.org/jira/browse/MESOS-2337 Repository: mesos Description --- Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos. Diffs - src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/python/interface/setup.py.in afcaf7ad6851eb36304a3be92c062cb6b21dbb03 src/python/setup.py.in 304c4bf87870fbaff2de0615cfd2263bb8a7ad3a Diff: https://reviews.apache.org/r/37081/diff/ Testing --- ## Test steps ### In CentOS 6.6 ``` ./bootstrap mkdir build cd build make -j4 sudo make install ``` And then execute ``` export PYTHONPATH=/usr/local/lib/python2.6/site-packages python -c import mesos; from mesos import native; from mesos import interface ``` Check it could success or not. Also check the `__init__.py` file ``` cat /usr/local/lib/python2.6/site-packages/mesos/__init__.py ``` Thanks, haosdent huang
Re: Review Request 37081: Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.
On Aug. 5, 2015, 5:47 a.m., Marco Massenzio wrote: did you forget to rebase? Never mind - I was just looking at diff 4 -- 5. All good. - Marco --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/#review94191 --- On Aug. 5, 2015, 4:21 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/ --- (Updated Aug. 5, 2015, 4:21 a.m.) Review request for mesos, Kapil Arya and Marco Massenzio. Bugs: MESOS-2337 https://issues.apache.org/jira/browse/MESOS-2337 Repository: mesos Description --- Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos. Diffs - src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/python/interface/setup.py.in afcaf7ad6851eb36304a3be92c062cb6b21dbb03 src/python/setup.py.in 304c4bf87870fbaff2de0615cfd2263bb8a7ad3a Diff: https://reviews.apache.org/r/37081/diff/ Testing --- ## Test steps ### In CentOS 6.6 ``` ./bootstrap mkdir build cd build make -j4 sudo make install ``` And then execute ``` export PYTHONPATH=/usr/local/lib/python2.6/site-packages python -c import mesos; from mesos import native; from mesos import interface ``` Check it could success or not. Also check the `__init__.py` file ``` cat /usr/local/lib/python2.6/site-packages/mesos/__init__.py ``` Thanks, haosdent huang
Re: Review Request 36783: Windows: Header splitting continued (stout/os.hpp)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36783/#review94159 --- 3rdparty/libprocess/3rdparty/stout/include/stout/os/os.hpp (lines 22 - 50) https://reviews.apache.org/r/36783/#comment148688 Why do we have both os.hpp and os/os.hpp? Seems strange to put only the structs here, why arent't the structs placed in the same header as the functions that return them..? - Ben Mahler On July 29, 2015, 11:24 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36783/ --- (Updated July 29, 2015, 11:24 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Alex Clemmer, and Joris Van Remoortere. Bugs: MESOS-3102 https://issues.apache.org/jira/browse/MESOS-3102 Repository: mesos Description --- MESOS-3102: Stout library header splitting, to support the Windows Containerizer. Splits apart os.hpp only. See the prior review in the chain for the pattern. Diffs - 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 5c19e3ef8ba50ab007eda26b752441f076ca7ed0 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 818560f8ce20126e0aa4af6ce368c973c9616c74 3rdparty/libprocess/3rdparty/stout/include/stout/os/os.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/include/stout/windows/preprocessor.hpp PRE-CREATION Diff: https://reviews.apache.org/r/36783/diff/ Testing --- `make` and `make check` (Mac OSX). Build with MSVC Enterprise 2015 [thanks to Alex (hausdorff)]. Thanks, Joseph Wu
Re: Review Request 37081: Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/#review94165 --- Patch looks great! Reviews applied: [37081] All tests passed. - Mesos ReviewBot On Aug. 4, 2015, 5:33 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/ --- (Updated Aug. 4, 2015, 5:33 p.m.) Review request for mesos, Kapil Arya and Marco Massenzio. Bugs: MESOS-2337 https://issues.apache.org/jira/browse/MESOS-2337 Repository: mesos Description --- Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos. Diffs - src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/python/setup.py.in 304c4bf87870fbaff2de0615cfd2263bb8a7ad3a Diff: https://reviews.apache.org/r/37081/diff/ Testing --- ## Test steps ### In CentOS 6.6 ``` ./bootstrap mkdir build cd build make -j4 sudo make install ``` And then execute ``` export PYTHONPATH=/usr/local/lib/python2.6/site-packages python -c import mesos; from mesos import native; from mesos import interface ``` Check it could success or not. Also check the `__init__.py` file ``` cat /usr/local/lib/python2.6/site-packages/mesos/__init__.py ``` Thanks, haosdent huang
Re: Review Request 37053: Set TaskStatus.executor_id on receiving a StatusUpdate from Executor.
On Aug. 4, 2015, 11:03 p.m., Niklas Nielsen wrote: src/slave/slave.cpp, lines 2720-2721 https://reviews.apache.org/r/37053/diff/2/?file=1028868#file1028868line2720 What do you think about emitting a warning if the executor id is already set? It could be, that users have been abusing this field before (I am not aware of any, but it is a possibility). If not, we should comment on it in mesos.proto. And if it's not expected behaviour, suggest to log a warning or info message. it's better to set executor id in a central place. - Klaus --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37053/#review94143 --- On Aug. 4, 2015, 6:32 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37053/ --- (Updated Aug. 4, 2015, 6:32 p.m.) Review request for mesos, Niklas Nielsen and Vinod Kone. Bugs: MESOS-3196 https://issues.apache.org/jira/browse/MESOS-3196 Repository: mesos Description --- Always set executor_id when sending StatusUpdate. Diffs - src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 src/tests/master_tests.cpp 2aea430951d40d8fe78f656b1269c5e55a0802db src/tests/scheduler_tests.cpp 98fc70bf43ba99b54064a236795c7e1269004b71 Diff: https://reviews.apache.org/r/37053/diff/ Testing --- make check with a couple of added checks that verify that the executor_id is set in the incoming TaskStatus in Master as well as Scheduler. Thanks, Kapil Arya
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/#review94176 --- configure.ac (line 544) https://reviews.apache.org/r/36811/#comment148717 The check should come before we do the AC_SUBST. I don't want to just move the AC_SUBST way away from the rest of the protobuf checking code though. I think it would be better moving the whole protobuf / protobuf.jar check section to after the java check, then just update the if java check around the AC_CHECK_FILE where it was previously. - Cody Maloney On Aug. 4, 2015, 9:27 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36811/ --- (Updated Aug. 4, 2015, 9:27 a.m.) Review request for mesos, Adam B, Cody Maloney, Michael Park, and Timothy St. Clair. 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 37045: Convert Linux perf sampler to use process:await().
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37045/#review94156 --- Ship it! src/linux/perf.cpp (lines 292 - 294) https://reviews.apache.org/r/37045/#comment148689 The style checker doesn't catch this, but this isn't one of the styles for wrapping function calls in the style guide, can you wrap at the open paren? src/linux/perf.cpp (line 298) https://reviews.apache.org/r/37045/#comment148681 No need for this comment IMO src/linux/perf.cpp (line 301) https://reviews.apache.org/r/37045/#comment148690 Can you print the failure if the future is failed? Note that you can CHECK that it isn't discarded. src/linux/perf.cpp (lines 306 - 307) https://reviews.apache.org/r/37045/#comment148692 Note that we don't use periods when composing log messages, so: Failed to collect perf statistics: perf exited with status status Note that : is used for composition, so when we say which status we wouldn't use a : src/linux/perf.cpp (line 307) https://reviews.apache.org/r/37045/#comment148693 Can you use WSTRINGIFY instead here? Note that we should not be using WEXITSTATUS when WIFEXITED is not true. WSTRINGIFY will take care of this for you. src/linux/perf.cpp (line 312) https://reviews.apache.org/r/37045/#comment148694 Let's CHECK that this is not discarded as well, before we call .get src/linux/perf.cpp (line 314) https://reviews.apache.org/r/37045/#comment148680 Notice that we don't put a space before the : when composing failure messages, seems minor but inconsistent log message formatting is a huge pain. - Ben Mahler On Aug. 4, 2015, 11:59 p.m., Paul Brett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37045/ --- (Updated Aug. 4, 2015, 11:59 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Convert Linux perf sampler to use process:await(). Diffs - src/linux/perf.cpp 697b75e846a43d4f106ad8f39a18882836d7dc02 Diff: https://reviews.apache.org/r/37045/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37101: Remove unused sched API's
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37101/#review94174 --- Patch looks great! Reviews applied: [36410, 36411, 36412, 36413, 37101] All tests passed. - Mesos ReviewBot On Aug. 4, 2015, 10:27 p.m., Cong Wang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37101/ --- (Updated Aug. 4, 2015, 10:27 p.m.) Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone. Repository: mesos Description --- We don't use SCHED_IDLE any more. Diffs - src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/linux/sched.hpp 8cb06531b417a77766ab2b13587393b99a96b0c1 src/tests/containerizer/sched_tests.cpp 00723d01cd2cc37410d6f9fdd2de080063b7ccd8 Diff: https://reviews.apache.org/r/37101/diff/ Testing --- make Thanks, Cong Wang
Re: Review Request 37097: Fix 'Accept-Encoding' parsing
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37097/#review94161 --- 3rdparty/libprocess/src/http.cpp https://reviews.apache.org/r/37097/#comment148696 What is the rationale behind moving these comments to the header file ? I wasn't able to make it out from r36402, Pardon my ignorance. Having these rules in the cpp file is much more intuitive since it allows you to easily understand the code as each rule is followed by its implementation. Including the rules in the header file is essentially duplicating the RFC. Why do we want to do it ? 3rdparty/libprocess/src/tests/encoder_tests.cpp (line 68) https://reviews.apache.org/r/37097/#comment148695 Can we kill the extra [0] index element we just introduced ? - Anand Mazumdar On Aug. 4, 2015, 10:43 p.m., Isabel Jimenez wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37097/ --- (Updated Aug. 4, 2015, 10:43 p.m.) Review request for mesos and Ben Mahler. Repository: mesos Description --- Currently parsing only compares the begining of the header making 'gzipbug' match with candidate 'gzip' Diffs - 3rdparty/libprocess/include/process/http.hpp b8d9300 3rdparty/libprocess/src/http.cpp 4dcbd74 3rdparty/libprocess/src/tests/encoder_tests.cpp 0032137 Diff: https://reviews.apache.org/r/37097/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 36783: Windows: Header splitting continued (stout/os.hpp)
On Aug. 5, 2015, 12:38 a.m., Ben Mahler wrote:
3rdparty/libprocess/3rdparty/stout/include/stout/os/os.hpp, lines 22-50
https://reviews.apache.org/r/36783/diff/4/?file=1024905#file1024905line22
Why do we have both os.hpp and os/os.hpp? Seems strange to put only the
structs here, why arent't the structs placed in the same header as the
functions that return them..?
Joseph Wu wrote:
Ideally, `os.hpp` would just be split up into `os.hpp`, `posix/os.hpp`,
and `windows/os.hpp`. `os.hpp` would include the `posix`/`windows` versions.
We need the extra `os/os.hpp` because these structs are used in `os.hpp`
and `posix/os.hpp`, so the inclusion order matters.
We effectively need `os.hpp` to do:
```
#include existing stuff
// i.e. #include stout/os/os.hpp
struct foo { ... };
#include stout/posix/os.hpp
```
How about calling this common.hpp or something a little more intuitive? And
including a comment that these are shared?
- Ben
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36783/#review94159
---
On July 29, 2015, 11:24 p.m., Joseph Wu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36783/
---
(Updated July 29, 2015, 11:24 p.m.)
Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Alex Clemmer,
and Joris Van Remoortere.
Bugs: MESOS-3102
https://issues.apache.org/jira/browse/MESOS-3102
Repository: mesos
Description
---
MESOS-3102: Stout library header splitting, to support the Windows
Containerizer.
Splits apart os.hpp only.
See the prior review in the chain for the pattern.
Diffs
-
3rdparty/libprocess/3rdparty/stout/include/Makefile.am
5c19e3ef8ba50ab007eda26b752441f076ca7ed0
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
818560f8ce20126e0aa4af6ce368c973c9616c74
3rdparty/libprocess/3rdparty/stout/include/stout/os/os.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp
PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/windows/preprocessor.hpp
PRE-CREATION
Diff: https://reviews.apache.org/r/36783/diff/
Testing
---
`make` and `make check` (Mac OSX).
Build with MSVC Enterprise 2015 [thanks to Alex (hausdorff)].
Thanks,
Joseph Wu
Re: Review Request 37082: Tests for subscribe/failover functionality for http based framework
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37082/ --- (Updated Aug. 4, 2015, 10:02 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Rebased from master after Ben's latest changes. Repository: mesos Description --- This implements the tests for http framework subscribe/failover/upgrade from a pid based framework. The test are parameterized on content type and hence test both json/protobuf responses. Diffs (updated) - src/tests/http_api_tests.cpp 586d11288828fe9997e54f5dbd7d28c200e973f5 Diff: https://reviews.apache.org/r/37082/diff/ Testing --- make check Thanks, Anand Mazumdar
Review Request 37105: Removed the code of checkpointing container root filesystem path.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37105/ --- Review request for mesos, Ian Downes, Timothy Chen, Vinod Kone, and Jiang Yan Xu. Bugs: MESOS-3205 https://issues.apache.org/jira/browse/MESOS-3205 Repository: mesos Description --- Removed the code of checkpointing container root filesystem path. See ticket for the motivation. Diffs - src/common/protobuf_utils.hpp a4708ed286ef237f17d9fd7813be2f6e7218b42a src/common/protobuf_utils.cpp 3cb684598d0492a2f57b46fabcf13565ff42f27a src/slave/containerizer/mesos/containerizer.hpp 8851d30af56b4f9fb95450ac1f42ab550e3df9ff src/slave/containerizer/mesos/containerizer.cpp 6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 src/slave/containerizer/provisioner.hpp cb4d511e8189b65df9b9803f23812dd98edc44ac src/slave/paths.cpp 404c2143e70771747d356b15eac9137495fd9a75 src/slave/state.hpp cecf200e6e79172af57ae195a73a5161be7e604a src/slave/state.cpp b9f2d8a0d6395b92bd50f1e0927f3ae9fd04b81c Diff: https://reviews.apache.org/r/37105/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 37082: Tests for subscribe/failover functionality for http based framework
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37082/ --- (Updated Aug. 5, 2015, midnight) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Rebased from master. Also fixed tests that broke after we introduced the validation logic. Repository: mesos Description --- This implements the tests for http framework subscribe/failover/upgrade from a pid based framework. The test are parameterized on content type and hence test both json/protobuf responses. Diffs (updated) - src/tests/http_api_tests.cpp 586d11288828fe9997e54f5dbd7d28c200e973f5 Diff: https://reviews.apache.org/r/37082/diff/ Testing --- make check Thanks, Anand Mazumdar
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/ --- (Updated Aug. 4, 2015, 10 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Rebased from master from Ben's latest commit. 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. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs (updated) - src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp cd0a5c863d4f74c3bcfd61d8e3046b3ab249be67 src/master/master.cpp 87e11d512f68e2b7285ee1901d422991208baf15 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36402: Adding 'Accept' header in request
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36402/ --- (Updated Aug. 4, 2015, 11:05 p.m.) Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and Vinod Kone. Repository: mesos-incubating Description --- Adding a method for Accept header in request + refactor of Accept-Encoding Diffs - 3rdparty/libprocess/include/process/http.hpp b8d9300 3rdparty/libprocess/src/http.cpp 4dcbd74 3rdparty/libprocess/src/tests/http_tests.cpp ecbcbd5 Diff: https://reviews.apache.org/r/36402/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 36402: Adding 'Accept' header in request
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36402/ --- (Updated Aug. 4, 2015, 11:05 p.m.) Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and Vinod Kone. Repository: mesos-incubating Description --- Adding a method for Accept header in request + refactor of Accept-Encoding Diffs (updated) - 3rdparty/libprocess/include/process/http.hpp b8d9300 3rdparty/libprocess/src/http.cpp 4dcbd74 3rdparty/libprocess/src/tests/http_tests.cpp ecbcbd5 Diff: https://reviews.apache.org/r/36402/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 36402: Adding 'Accept' header in request
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36402/ --- (Updated Aug. 5, 2015, 12:24 a.m.) Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and Vinod Kone. Repository: mesos-incubating Description --- Adding a method for Accept header in request + refactor of Accept-Encoding Diffs (updated) - 3rdparty/libprocess/include/process/http.hpp b8d9300 3rdparty/libprocess/src/http.cpp 4dcbd74 3rdparty/libprocess/src/tests/http_tests.cpp ecbcbd5 Diff: https://reviews.apache.org/r/36402/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 37097: Fix 'Accept-Encoding' parsing
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37097/ --- (Updated Aug. 4, 2015, 10:43 p.m.) Review request for mesos and Ben Mahler. Repository: mesos Description --- Currently parsing only compares the begining of the header making 'gzipbug' match with candidate 'gzip' Diffs (updated) - 3rdparty/libprocess/include/process/http.hpp b8d9300 3rdparty/libprocess/src/http.cpp 4dcbd74 3rdparty/libprocess/src/tests/encoder_tests.cpp 0032137 Diff: https://reviews.apache.org/r/37097/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 37053: Set TaskStatus.executor_id on receiving a StatusUpdate from Executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37053/#review94143 --- Ship it! src/slave/slave.cpp (lines 2720 - 2721) https://reviews.apache.org/r/37053/#comment148658 What do you think about emitting a warning if the executor id is already set? It could be, that users have been abusing this field before (I am not aware of any, but it is a possibility). If not, we should comment on it in mesos.proto. - Niklas Nielsen On Aug. 4, 2015, 11:32 a.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37053/ --- (Updated Aug. 4, 2015, 11:32 a.m.) Review request for mesos, Niklas Nielsen and Vinod Kone. Bugs: MESOS-3196 https://issues.apache.org/jira/browse/MESOS-3196 Repository: mesos Description --- Always set executor_id when sending StatusUpdate. Diffs - src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 src/tests/master_tests.cpp 2aea430951d40d8fe78f656b1269c5e55a0802db src/tests/scheduler_tests.cpp 98fc70bf43ba99b54064a236795c7e1269004b71 Diff: https://reviews.apache.org/r/37053/diff/ Testing --- make check with a couple of added checks that verify that the executor_id is set in the incoming TaskStatus in Master as well as Scheduler. Thanks, Kapil Arya
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/ --- (Updated Aug. 4, 2015, 11:57 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Rebased from master after Ben's commit for exited 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. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs (updated) - src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36402: Adding 'Accept' header in request
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36402/ --- (Updated Aug. 5, 2015, 12:32 a.m.) Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and Vinod Kone. Repository: mesos-incubating Description --- Adding a method for Accept header in request + refactor of Accept-Encoding Diffs (updated) - 3rdparty/libprocess/include/process/http.hpp b8d9300 3rdparty/libprocess/src/http.cpp 4dcbd74 3rdparty/libprocess/src/tests/http_tests.cpp ecbcbd5 Diff: https://reviews.apache.org/r/36402/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
On Aug. 4, 2015, 9:07 p.m., Ben Mahler wrote: I'll get the /call validation committed. Let's follow up to get tests added for each of the error cases in /call. - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review94111 --- On Aug. 4, 2015, 10 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 4, 2015, 10 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. 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. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs - src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp cd0a5c863d4f74c3bcfd61d8e3046b3ab249be67 src/master/master.cpp 87e11d512f68e2b7285ee1901d422991208baf15 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
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/#review94130 --- I'll get the exited changes committed. src/master/master.cpp (line 969) https://reviews.apache.org/r/36720/#comment148645 Hm.. let's check that the framework ids are equal when the writers match. src/master/master.cpp (line 971) https://reviews.apache.org/r/36720/#comment148644 s/exited/disconnection/ src/master/master.cpp (line 1021) https://reviews.apache.org/r/36720/#comment148648 Let's put `_exited` after both `exited` implementations, since it is a shared continuation. - Ben Mahler On Aug. 4, 2015, 10 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 4, 2015, 10 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. 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. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs - src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp cd0a5c863d4f74c3bcfd61d8e3046b3ab249be67 src/master/master.cpp 87e11d512f68e2b7285ee1901d422991208baf15 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
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/#review94133 --- Patch looks great! Reviews applied: [36913] All tests passed. - Mesos ReviewBot On Aug. 4, 2015, 2:32 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36913/ --- (Updated Aug. 4, 2015, 2:32 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 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 src/master/quota_handler.cpp PRE-CREATION Diff: https://reviews.apache.org/r/36913/diff/ Testing --- make check Thanks, Joerg Schad
Review Request 37101: Remove unused sched API's
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37101/ --- Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone. Repository: mesos Description --- We don't use SCHED_IDLE any more. Diffs - src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/linux/sched.hpp 8cb06531b417a77766ab2b13587393b99a96b0c1 src/tests/containerizer/sched_tests.cpp 00723d01cd2cc37410d6f9fdd2de080063b7ccd8 Diff: https://reviews.apache.org/r/37101/diff/ Testing --- make Thanks, Cong Wang
Re: Review Request 37091: Stopped mutating executor info with default container info.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37091/#review94145 --- Ship it! Ship It! - Timothy Chen On Aug. 4, 2015, 7:01 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37091/ --- (Updated Aug. 4, 2015, 7:01 p.m.) Review request for mesos, Ben Mahler, Ian Downes, Timothy Chen, and Vinod Kone. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- Stopped mutating executor info with default container info. This is a bug which could cause the same issue described in MESOS-2463. Now, filling the default container info is done by the containerizer. As the TODO suggested, we probably should checkpoint the filled default container info in case the slave changes the flags. Diffs - src/slave/containerizer/mesos/containerizer.cpp 6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 Diff: https://reviews.apache.org/r/37091/diff/ Testing --- sudo make check Thanks, Jie Yu
Re: Review Request 37045: Convert Linux perf sampler to use process:await().
On Aug. 4, 2015, 11:45 p.m., Ben Mahler wrote: src/linux/perf.cpp, lines 314-316 https://reviews.apache.org/r/37045/diff/2/?file=1028754#file1028754line314 Any reason you're changing the style of the failure messages? Let's leave them untouched in this patch, since they look goot to me. I updated the error message because I realized that perf executed but returned an error status, which is of course different from failing to execute perf at all (caught be the test on line 301). - Paul --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37045/#review94148 --- On Aug. 4, 2015, 4:57 p.m., Paul Brett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37045/ --- (Updated Aug. 4, 2015, 4:57 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Convert Linux perf sampler to use process:await(). Diffs - src/linux/perf.cpp 697b75e846a43d4f106ad8f39a18882836d7dc02 Diff: https://reviews.apache.org/r/37045/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37045: Convert Linux perf sampler to use process:await().
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37045/#review94148 --- Looks great paul! Just some minor things around style before we can get this committed. src/linux/perf.cpp (lines 295 - 296) https://reviews.apache.org/r/37045/#comment148668 This isn't a 'future' so this name seems a little counter-intuitive, how about we call this something like 'results'? src/linux/perf.cpp (line 301) https://reviews.apache.org/r/37045/#comment148672 This is indented strangely, did you look over the diff? Also, notice that we don't use periods at the end of any of our failure messages or log statements, this is because it is difficult to end with one period consistently when composing error messages. src/linux/perf.cpp (lines 306 - 307) https://reviews.apache.org/r/37045/#comment148669 Any reason you're changing the style of the failure messages? Let's leave them untouched in this patch, since they look goot to me. src/linux/perf.cpp (line 313) https://reviews.apache.org/r/37045/#comment148671 I guess the sytle checker is not catching this, but if you look around the rest of the code, we add a space between if and the open paren. Can you do a sweep? src/linux/perf.cpp (line 314) https://reviews.apache.org/r/37045/#comment148670 Ditto here, can can we print the same style message as before? ``` Failed to read output of perf process: + ... ``` Note the format of these messages Failed to ...: reason - Ben Mahler On Aug. 4, 2015, 4:57 p.m., Paul Brett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37045/ --- (Updated Aug. 4, 2015, 4:57 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Convert Linux perf sampler to use process:await(). Diffs - src/linux/perf.cpp 697b75e846a43d4f106ad8f39a18882836d7dc02 Diff: https://reviews.apache.org/r/37045/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37045: Convert Linux perf sampler to use process:await().
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37045/#review94152 --- Patch looks great! Reviews applied: [37045] All tests passed. - Mesos ReviewBot On Aug. 4, 2015, 4:57 p.m., Paul Brett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37045/ --- (Updated Aug. 4, 2015, 4:57 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Convert Linux perf sampler to use process:await(). Diffs - src/linux/perf.cpp 697b75e846a43d4f106ad8f39a18882836d7dc02 Diff: https://reviews.apache.org/r/37045/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37045: Convert Linux perf sampler to use process:await().
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37045/ --- (Updated Aug. 4, 2015, 11:59 p.m.) Review request for mesos and Ben Mahler. Changes --- Incorporate review comments. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Convert Linux perf sampler to use process:await(). Diffs (updated) - src/linux/perf.cpp 697b75e846a43d4f106ad8f39a18882836d7dc02 Diff: https://reviews.apache.org/r/37045/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37081: Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/ --- (Updated Aug. 4, 2015, 5:12 p.m.) Review request for mesos, Kapil Arya and Marco Massenzio. Bugs: MESOS-2337 https://issues.apache.org/jira/browse/MESOS-2337 Repository: mesos Description --- Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos. Diffs (updated) - src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/python/setup.py.in 304c4bf87870fbaff2de0615cfd2263bb8a7ad3a Diff: https://reviews.apache.org/r/37081/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 37011: libprocess: Removed unused 'fatal' and 'fatalerror' macros.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37011/ --- (Updated Aug. 4, 2015, 5:43 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff. Bugs: MESOS-3200 https://issues.apache.org/jira/browse/MESOS-3200 Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/src/fatal.hpp 87a55dca4fe53c1f3fc7fb03914f3ec4270aa5b4 3rdparty/libprocess/src/fatal.cpp 76d5ee42be50651863f88189341d59cfd406bae4 Diff: https://reviews.apache.org/r/37011/diff/ Testing --- `make check` Thanks, Michael Park
Re: Review Request 36402: Adding 'Accept' header in request
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36402/ --- (Updated Aug. 4, 2015, 5:42 p.m.) Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and Vinod Kone. Repository: mesos-incubating Description --- Adding a method for Accept header in request + refactor of Accept-Encoding Diffs (updated) - 3rdparty/libprocess/include/process/http.hpp b8d9300 3rdparty/libprocess/src/http.cpp 4dcbd74 3rdparty/libprocess/src/tests/http_tests.cpp ecbcbd5 Diff: https://reviews.apache.org/r/36402/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 36402: Adding 'Accept' header in request
On Aug. 3, 2015, 9:38 p.m., Ben Mahler wrote: Is this ready to review? Mind updating the 'issues' accordingly? Just did, thanks for comments :) - Isabel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36402/#review93970 --- On Aug. 4, 2015, 5:42 p.m., Isabel Jimenez wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36402/ --- (Updated Aug. 4, 2015, 5:42 p.m.) Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and Vinod Kone. Repository: mesos-incubating Description --- Adding a method for Accept header in request + refactor of Accept-Encoding Diffs - 3rdparty/libprocess/include/process/http.hpp b8d9300 3rdparty/libprocess/src/http.cpp 4dcbd74 3rdparty/libprocess/src/tests/http_tests.cpp ecbcbd5 Diff: https://reviews.apache.org/r/36402/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 37045: Convert Linux perf sampler to use process:await().
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37045/ --- (Updated Aug. 4, 2015, 4:57 p.m.) Review request for mesos and Ben Mahler. Changes --- Address review comments. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Convert Linux perf sampler to use process:await(). Diffs (updated) - src/linux/perf.cpp 697b75e846a43d4f106ad8f39a18882836d7dc02 Diff: https://reviews.apache.org/r/37045/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37082: Tests for subscribe/failover functionality for http based framework
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37082/ --- (Updated Aug. 4, 2015, 5:15 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Updated depends on Repository: mesos Description --- This implements the tests for http framework subscribe/failover/upgrade from a pid based framework. The test are parameterized on content type and hence test both json/protobuf responses. Diffs - src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac Diff: https://reviews.apache.org/r/37082/diff/ Testing --- make check Thanks, Anand Mazumdar
Review Request 37053: Set TaskStatus.executor_id on receiving a StatusUpdate from Executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37053/ --- Review request for mesos, Niklas Nielsen and Vinod Kone. Bugs: MESOS-3196 https://issues.apache.org/jira/browse/MESOS-3196 Repository: mesos Description --- Always set executor_id when sending StatusUpdate. Diffs - src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 src/tests/master_tests.cpp 2aea430951d40d8fe78f656b1269c5e55a0802db src/tests/scheduler_tests.cpp 98fc70bf43ba99b54064a236795c7e1269004b71 Diff: https://reviews.apache.org/r/37053/diff/ Testing --- make check with a couple of added checks that verify that the executor_id is set in the incoming TaskStatus in Master as well as Scheduler. Thanks, Kapil Arya
Re: Review Request 36956: Created a test abstraction for preparing test rootfs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36956/#review93921 --- Ship it! src/tests/containerizer/rootfs.hpp (line 66) https://reviews.apache.org/r/36956/#comment148358 Just wanted to comment on this because this is related to a previous discussion. For `Path`, stout/path.hpp is not directly included and I don't think os.hpp is explicitly exporting `Path` through its APIs. I am less certain about what to do with the calls to the functions under `os::`. We have intentionally separated out these headers such as os/stat.hpp but in order to use `os::mkdir` we need to include os.hpp which includes everything under `stout/os/`. Maybe we should continue to refactor all these basic functions out of os.hpp so we only include os.hpp when we intentionally want to include everything. - Jiang Yan Xu On Aug. 3, 2015, 10:18 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36956/ --- (Updated Aug. 3, 2015, 10:18 a.m.) Review request for mesos, Ian Downes, Timothy Chen, Vinod Kone, and Jiang Yan Xu. Bugs: MESOS-3179 https://issues.apache.org/jira/browse/MESOS-3179 Repository: mesos Description --- Created a test abstraction for preparing test rootfs. Diffs - src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/tests/containerizer/launch_tests.cpp 73c8c5fa17936b1bab4817e9f1e691144e87c35f src/tests/containerizer/rootfs.hpp PRE-CREATION Diff: https://reviews.apache.org/r/36956/diff/ Testing --- sudo make check Thanks, Jie Yu
Re: Review Request 37012: mesos: Removed unused 'fatal' and 'fatalerror' macros.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37012/ --- (Updated Aug. 4, 2015, 5:43 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff. Bugs: MESOS-3200 https://issues.apache.org/jira/browse/MESOS-3200 Repository: mesos Description --- See summary. Diffs - src/exec/exec.cpp d59a7e1df0d4786ee72b410f4fbb3b71e585b39d src/zookeeper/zookeeper.cpp fe862aa0b2f0124a786a805ec6be9615599da2fb Diff: https://reviews.apache.org/r/37012/diff/ Testing --- `make check` Thanks, Michael Park
Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated Aug. 4, 2015, 6:26 p.m.) Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till Toenshoff. Changes --- Rebase, changes from reviewers. Bugs: MESOS-2947 https://issues.apache.org/jira/browse/MESOS-2947 Repository: mesos Description --- Adds and integrates helper classes needed to support an `Authorizer` module. Also adds a flag to the master, allowing the selection of an `Authorizer` module. Diffs (updated) - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 src/tests/cluster.hpp ba17c0c74a9dc36c595c4ad77fe68be94c5c7c0b Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36050: Added test authorizer module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36050/ --- (Updated Aug. 4, 2015, 6:27 p.m.) Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2947 https://issues.apache.org/jira/browse/MESOS-2947 Repository: mesos Description --- Adds a test authorizer module. Updates the authorization tests in order to perform typed tests on the default authorizer implementation and on the test authorizer module. Diffs (updated) - src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/examples/test_authorizer_module.cpp PRE-CREATION src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 src/tests/module.hpp 03756a2536ca3e662ba422e96d121a6c39bb8c84 src/tests/module.cpp 61d4753f0f098005f56dd4a24984e30405c32558 Diff: https://reviews.apache.org/r/36050/diff/ Testing --- make check Thanks, Alexander Rojas
Review Request 37082: Tests for subscribe/failover functionality for http based framework
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37082/ --- Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos Description --- This implements the tests for http framework subscribe/failover/upgrade from a pid based framework. The test are parameterized on content type and hence test both json/protobuf responses. Diffs - src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac Diff: https://reviews.apache.org/r/37082/diff/ Testing --- make check Thanks, Anand Mazumdar
Re: Review Request 37081: Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/ --- (Updated Aug. 4, 2015, 5:33 p.m.) Review request for mesos, Kapil Arya and Marco Massenzio. Bugs: MESOS-2337 https://issues.apache.org/jira/browse/MESOS-2337 Repository: mesos Description --- Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos. Diffs - src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/python/setup.py.in 304c4bf87870fbaff2de0615cfd2263bb8a7ad3a Diff: https://reviews.apache.org/r/37081/diff/ Testing (updated) --- ## Test steps ### In CentOS 6.6 ``` ./bootstrap mkdir build cd build make -j4 sudo make install ``` And then execute ``` export PYTHONPATH=/usr/local/lib/python2.6/site-packages python -c import mesos; from mesos import native; from mesos import interface ``` Check it could success or not. Also check the `__init__.py` file ``` cat /usr/local/lib/python2.6/site-packages/mesos/__init__.py ``` Thanks, haosdent huang
Re: Review Request 37010: stout: Removed unused 'fatal' and 'fatalerror' macros.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37010/ --- (Updated Aug. 4, 2015, 5:42 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff. Bugs: MESOS-3200 https://issues.apache.org/jira/browse/MESOS-3200 Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/3rdparty/stout/include/Makefile.am ce850337dd4591027993c242dae321f21516f065 3rdparty/libprocess/3rdparty/stout/include/stout/fatal.hpp 053ef82d9d152984b5e7a1b915353356be2cdd2f Diff: https://reviews.apache.org/r/37010/diff/ Testing --- `make check` Thanks, Michael Park
Re: Review Request 37081: Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/ --- (Updated Aug. 4, 2015, 4:33 p.m.) Review request for mesos, Kapil Arya and Marco Massenzio. Bugs: MESOS-2337 https://issues.apache.org/jira/browse/MESOS-2337 Repository: mesos Description --- Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos. Diffs (updated) - src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec Diff: https://reviews.apache.org/r/37081/diff/ Testing --- 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. Anand Mazumdar wrote: 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 ! Would take up validation in a different patch as we had discussed earlier. I am dropping the issue for now. Let me know if you think otherwise. - Anand --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review93499 --- On Aug. 4, 2015, 4:33 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 4, 2015, 4:33 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. 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. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs - src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
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/ --- (Updated Aug. 4, 2015, 4:33 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Added functionality to failover http frameworks. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description (updated) --- 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. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs (updated) - src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 Diff: https://reviews.apache.org/r/36720/diff/ Testing (updated) --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 37081: Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/#review94084 --- Can you please define how did you test that this works in the 'tests' section? I can try this out on Ubuntu 14.04 (and OSX) given the steps to reproduce the issue, and then applying the patch. Once we verify that it works on enough platforms, we can ship it. Thanks! - Marco Massenzio On Aug. 4, 2015, 5:12 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/ --- (Updated Aug. 4, 2015, 5:12 p.m.) Review request for mesos, Kapil Arya and Marco Massenzio. Bugs: MESOS-2337 https://issues.apache.org/jira/browse/MESOS-2337 Repository: mesos Description --- Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos. Diffs - src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/python/setup.py.in 304c4bf87870fbaff2de0615cfd2263bb8a7ad3a Diff: https://reviews.apache.org/r/37081/diff/ Testing --- Thanks, haosdent huang
Review Request 37091: Stopped mutating executor info with default container info.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37091/ --- Review request for mesos, Ben Mahler, Ian Downes, Timothy Chen, and Vinod Kone. Bugs: MESOS-2463 https://issues.apache.org/jira/browse/MESOS-2463 Repository: mesos Description --- Stopped mutating executor info with default container info. This is a bug which could cause the same issue described in MESOS-2463. Now, filling the default container info is done by the containerizer. As the TODO suggested, we probably should checkpoint the filled default container info in case the slave changes the flags. Diffs - src/slave/containerizer/mesos/containerizer.cpp 6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1 Diff: https://reviews.apache.org/r/37091/diff/ Testing --- sudo make check Thanks, Jie Yu
Re: Review Request 37075: Protobuf definitions instructing the fetcher cache about checksums and their validation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37075/#review94103 --- Patch looks great! Reviews applied: [37075] All tests passed. - Mesos ReviewBot On Aug. 4, 2015, 1:41 p.m., Bernd Mathiske wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37075/ --- (Updated Aug. 4, 2015, 1:41 p.m.) Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy Chen. Bugs: MESOS-2073 https://issues.apache.org/jira/browse/MESOS-2073 Repository: mesos Description --- Protobuf definitions instructing the fetcher cache about checksums and their validation. First in a series of patches implementing phase 1 of MESOS-2073 (see comments in the ticket). There are references to docs in this patch, but updating docs will be in the last patch. Diffs - include/mesos/mesos.proto a6748d1cd82238f005c6a49c70d22d095462f1ba Diff: https://reviews.apache.org/r/37075/diff/ Testing --- Thanks, Bernd Mathiske
Re: Review Request 36402: Adding 'Accept' header in request
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36402/#review94112 --- 3rdparty/libprocess/src/http.cpp (line 155) https://reviews.apache.org/r/36402/#comment148623 This will not crash, tokenize will always return a vector of at least one if one of the parameter strings is not empty and in this case the second one is always ;. (https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp#L208) - Isabel Jimenez On Aug. 4, 2015, 5:42 p.m., Isabel Jimenez wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36402/ --- (Updated Aug. 4, 2015, 5:42 p.m.) Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and Vinod Kone. Repository: mesos-incubating Description --- Adding a method for Accept header in request + refactor of Accept-Encoding Diffs - 3rdparty/libprocess/include/process/http.hpp b8d9300 3rdparty/libprocess/src/http.cpp 4dcbd74 3rdparty/libprocess/src/tests/http_tests.cpp ecbcbd5 Diff: https://reviews.apache.org/r/36402/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 37081: Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.
On Aug. 4, 2015, 5:17 p.m., Marco Massenzio wrote: Can you please define how did you test that this works in the 'tests' section? I can try this out on Ubuntu 14.04 (and OSX) given the steps to reproduce the issue, and then applying the patch. Once we verify that it works on enough platforms, we can ship it. Thanks! haosdent huang wrote: Hi, @marco. Thank you for your help. Marco Massenzio wrote: haha thank *you* for fixing this! I'm testing it out currently on my Mac - will try it out on a Ubuntu VM or my home box later this evening. Will keep you posted. Thanks for repro steps! ok - this *almost* worked :) Are we missing an `__init__.py` in `google`? ``` $ ../configure --prefix=/Users/marco/mesos-inst/lib ``` then the usual `make make install` - then: ``` $ find . |grep __init__ ./lib/python/site-packages/google/protobuf/__init__.py ./lib/python/site-packages/google/protobuf/__init__.pyc ./lib/python/site-packages/google/protobuf/compiler/__init__.py ./lib/python/site-packages/google/protobuf/compiler/__init__.pyc ./lib/python/site-packages/google/protobuf/internal/__init__.py ./lib/python/site-packages/google/protobuf/internal/__init__.pyc ./lib/python/site-packages/google/protobuf/pyext/__init__.py ./lib/python/site-packages/google/protobuf/pyext/__init__.pyc ./lib/python/site-packages/mesos/__init__.py ./lib/python/site-packages/mesos/__init__.pyc ./lib/python/site-packages/mesos/interface/__init__.py ./lib/python/site-packages/mesos/interface/__init__.pyc ./lib/python/site-packages/mesos/native/__init__.py ./lib/python/site-packages/mesos/native/__init__.pyc ./libexec/mesos/python/mesos/__init__.py ``` as you can see, there are now the `__init__.py` files in the right places under `mesos` (if you do the same from `master` there's nothing in there - so, yay! :) but there is none under the `google/` package: ``` [~/mesos-inst/lib] $ export PYTHONPATH=./lib/python/site-packages/ [~/mesos-inst/lib] $ python Python 2.7.6 (default, Sep 9 2014, 15:04:36) [GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.39)] on darwin Type help, copyright, credits or license for more information. import mesos from mesos import native Traceback (most recent call last): File stdin, line 1, in module File /Users/marco/mesos-inst/lib/lib/python/site-packages/mesos/native/__init__.py, line 17, in module from ._mesos import MesosExecutorDriverImpl File /Users/marco/mesos-inst/lib/lib/python/site-packages/mesos/interface/mesos_pb2.py, line 4, in module from google.protobuf.internal import enum_type_wrapper ImportError: No module named google.protobuf.internal import google.protobuf.internal Traceback (most recent call last): File stdin, line 1, in module ImportError: No module named google.protobuf.internal ``` This was on a Mac OSX 10.10 (Yosemite) - about to test the same on an Ubuntu 14.04 VM, brand new. - Marco --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/#review94084 --- On Aug. 4, 2015, 5:33 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/ --- (Updated Aug. 4, 2015, 5:33 p.m.) Review request for mesos, Kapil Arya and Marco Massenzio. Bugs: MESOS-2337 https://issues.apache.org/jira/browse/MESOS-2337 Repository: mesos Description --- Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos. Diffs - src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/python/setup.py.in 304c4bf87870fbaff2de0615cfd2263bb8a7ad3a Diff: https://reviews.apache.org/r/37081/diff/ Testing --- ## Test steps ### In CentOS 6.6 ``` ./bootstrap mkdir build cd build make -j4 sudo make install ``` And then execute ``` export PYTHONPATH=/usr/local/lib/python2.6/site-packages python -c import mesos; from mesos import native; from mesos import interface ``` Check it could success or not. Also check the `__init__.py` file ``` cat /usr/local/lib/python2.6/site-packages/mesos/__init__.py ``` Thanks, haosdent huang
Re: Review Request 36402: Adding 'Accept' header in request
On Aug. 4, 2015, 7:25 p.m., Ben Mahler wrote: 3rdparty/libprocess/src/http.cpp, line 219 https://reviews.apache.org/r/36402/diff/6/?file=1028798#file1028798line219 This will crash the program if tokens is empty! Isabel Jimenez wrote: This will not crash, tokenize will always return a vector of at least one if one of the parameter strings is not empty and in this case the second one is always ;. (https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp#L208) That is a test of strings::split, here is a tokenize test which returns an empty vector: https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp#L163 - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36402/#review94105 --- On Aug. 4, 2015, 5:42 p.m., Isabel Jimenez wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36402/ --- (Updated Aug. 4, 2015, 5:42 p.m.) Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and Vinod Kone. Repository: mesos-incubating Description --- Adding a method for Accept header in request + refactor of Accept-Encoding Diffs - 3rdparty/libprocess/include/process/http.hpp b8d9300 3rdparty/libprocess/src/http.cpp 4dcbd74 3rdparty/libprocess/src/tests/http_tests.cpp ecbcbd5 Diff: https://reviews.apache.org/r/36402/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 37081: Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos.
On Aug. 4, 2015, 5:17 p.m., Marco Massenzio wrote: Can you please define how did you test that this works in the 'tests' section? I can try this out on Ubuntu 14.04 (and OSX) given the steps to reproduce the issue, and then applying the patch. Once we verify that it works on enough platforms, we can ship it. Thanks! haosdent huang wrote: Hi, @marco. Thank you for your help. haha thank *you* for fixing this! I'm testing it out currently on my Mac - will try it out on a Ubuntu VM or my home box later this evening. Will keep you posted. Thanks for repro steps! - Marco --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/#review94084 --- On Aug. 4, 2015, 5:33 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37081/ --- (Updated Aug. 4, 2015, 5:33 p.m.) Review request for mesos, Kapil Arya and Marco Massenzio. Bugs: MESOS-2337 https://issues.apache.org/jira/browse/MESOS-2337 Repository: mesos Description --- Let __init__.py getting installed to $PREFIX/lib/pythonX.Y/site-packages/mesos. Diffs - src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec src/python/setup.py.in 304c4bf87870fbaff2de0615cfd2263bb8a7ad3a Diff: https://reviews.apache.org/r/37081/diff/ Testing --- ## Test steps ### In CentOS 6.6 ``` ./bootstrap mkdir build cd build make -j4 sudo make install ``` And then execute ``` export PYTHONPATH=/usr/local/lib/python2.6/site-packages python -c import mesos; from mesos import native; from mesos import interface ``` Check it could success or not. Also check the `__init__.py` file ``` cat /usr/local/lib/python2.6/site-packages/mesos/__init__.py ``` 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/#review94111 --- I'll get the /call validation committed. src/master/http.cpp (line 355) https://reviews.apache.org/r/36720/#comment148622 Need an else here for UnsupportedMediaType, I'll add this in. - Ben Mahler On Aug. 4, 2015, 4:33 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 4, 2015, 4:33 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. 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. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs - src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36402: Adding 'Accept' header in request
On Aug. 4, 2015, 7:25 p.m., Ben Mahler wrote: 3rdparty/libprocess/src/http.cpp, line 219 https://reviews.apache.org/r/36402/diff/6/?file=1028798#file1028798line219 This will crash the program if tokens is empty! This will not crash, tokenize will always return a vector of at least one if one of the parameter strings is not empty and in this case the second one is always ;. (https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp#L208) - Isabel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36402/#review94105 --- On Aug. 4, 2015, 5:42 p.m., Isabel Jimenez wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36402/ --- (Updated Aug. 4, 2015, 5:42 p.m.) Review request for mesos, Anand Mazumdar, Ben Mahler, Marco Massenzio, and Vinod Kone. Repository: mesos-incubating Description --- Adding a method for Accept header in request + refactor of Accept-Encoding Diffs - 3rdparty/libprocess/include/process/http.hpp b8d9300 3rdparty/libprocess/src/http.cpp 4dcbd74 3rdparty/libprocess/src/tests/http_tests.cpp ecbcbd5 Diff: https://reviews.apache.org/r/36402/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 37072: Added test for allocator update on scheduler failover
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37072/#review94104 --- Nice test! src/tests/oversubscription_tests.cpp (line 864) https://reviews.apache.org/r/37072/#comment148599 2 blank lines between outer elements. src/tests/oversubscription_tests.cpp (line 914) https://reviews.apache.org/r/37072/#comment148605 Add a comment here // Framework doesn't receive revocable resources because // it doesn't have the capability set. src/tests/oversubscription_tests.cpp (line 949) https://reviews.apache.org/r/37072/#comment148601 kill extra new line. src/tests/oversubscription_tests.cpp (line 962) https://reviews.apache.org/r/37072/#comment148602 Space after //. src/tests/oversubscription_tests.cpp (lines 975 - 977) https://reviews.apache.org/r/37072/#comment148604 Indent like so: ``` EXPECT_EQ(taskResources + executorResources, Resources(offers2.get([0].resources())); - Vinod Kone On Aug. 4, 2015, 10:42 a.m., Aditi Dixit wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37072/ --- (Updated Aug. 4, 2015, 10:42 a.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2880 https://issues.apache.org/jira/browse/MESOS-2880 Repository: mesos Description --- Added test for allocator update on scheduler failover Diffs - src/tests/oversubscription_tests.cpp c7a2dacb600d7703de6090e7e47f453a3d08b53a Diff: https://reviews.apache.org/r/37072/diff/ Testing --- make check Thanks, Aditi Dixit
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/#review94121 --- Patch looks great! Reviews applied: [36910] All tests passed. - Mesos ReviewBot On Aug. 4, 2015, 1:49 p.m., Chris Heller wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36910/ --- (Updated Aug. 4, 2015, 1:49 p.m.) Review request for mesos and Till Toenshoff. 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 - configure.ac 9438d88 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
