Re: Review Request 30459: Refactored task/offer/resource valiation in master.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30459/#review70488
---


Patch looks great!

Reviews applied: [30459]

All tests passed.

- Mesos ReviewBot


On Jan. 31, 2015, 2:09 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30459/
> ---
> 
> (Updated Jan. 31, 2015, 2:09 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone.
> 
> 
> Bugs: MESOS-2305
> https://issues.apache.org/jira/browse/MESOS-2305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored task/offer/resource valiation in master.
> 
> See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 07bea1fb8f0035413f2119859e16fa4f9383f68e 
>   src/master/master.hpp 337e00aa46ea127f3667e3383d631c3fb8e22f30 
>   src/master/master.cpp 10056861b95ed9453c971787982db7d09f09f323 
>   src/master/validation.hpp PRE-CREATION 
>   src/master/validation.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/30459/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 30441: Query string encode / decode

[Benjamin Hindman]

> On Jan. 31, 2015, 2:26 a.m., Benjamin Hindman wrote:
> >

Small fixes, I'll take care of them!


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30441/#review70485
---


On Jan. 30, 2015, 3:10 a.m., Cody Maloney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30441/
> ---
> 
> (Updated Jan. 30, 2015, 3:10 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Query string encode / decode
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 9cf05acbb724ab9af8010d1788621d37a0e48e86 
>   3rdparty/libprocess/src/decoder.hpp 
> 816846436cd8710c476902146c4df06f09aa9da1 
>   3rdparty/libprocess/src/http.cpp 869b205656fb73edb9f02a1856d10f79ed1349b4 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> c71a91d516214d20d3df4c220e4a39a070db3260 
> 
> Diff: https://reviews.apache.org/r/30441/diff/
> 
> 
> Testing
> ---
> 
> make distcheck on Ubuntu 14.04 for the series
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Re: Review Request 30442: Update mesos for query string encode/decode

[Benjamin Hindman]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30442/#review70486
---

Ship it!


Ship It!

- Benjamin Hindman


On Jan. 30, 2015, 3:10 a.m., Cody Maloney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30442/
> ---
> 
> (Updated Jan. 30, 2015, 3:10 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Update mesos for query string encode/decode
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 3981b18cb82d3b8bd974b80d27f14c304898a43c 
>   src/tests/repair_tests.cpp 5235058263d661dfa6445b0b443af6290e2ea016 
> 
> Diff: https://reviews.apache.org/r/30442/diff/
> 
> 
> Testing
> ---
> 
> make distcheck on Ubuntu 14.04
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Re: Review Request 30441: Query string encode / decode

[Benjamin Hindman]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30441/#review70485
---

Ship it!



3rdparty/libprocess/src/http.cpp


What about only splitting maximum of 2 tokens?



3rdparty/libprocess/src/http.cpp


Same line?



3rdparty/libprocess/src/tests/http_tests.cpp


Let's use 'typedef' until we talk about bringing in both ways and decide on 
one or the other. Also, s/args/HashmapStringString/ (I think that exists in 
another test as well).


- Benjamin Hindman


On Jan. 30, 2015, 3:10 a.m., Cody Maloney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30441/
> ---
> 
> (Updated Jan. 30, 2015, 3:10 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Query string encode / decode
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 9cf05acbb724ab9af8010d1788621d37a0e48e86 
>   3rdparty/libprocess/src/decoder.hpp 
> 816846436cd8710c476902146c4df06f09aa9da1 
>   3rdparty/libprocess/src/http.cpp 869b205656fb73edb9f02a1856d10f79ed1349b4 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> c71a91d516214d20d3df4c220e4a39a070db3260 
> 
> Diff: https://reviews.apache.org/r/30441/diff/
> 
> 
> Testing
> ---
> 
> make distcheck on Ubuntu 14.04 for the series
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Re: Review Request 28252: Add initializer_list constructor to hashmap

[Benjamin Hindman]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28252/#review70484
---

Ship it!


Ship It!

- Benjamin Hindman


On Nov. 20, 2014, 12:06 a.m., Cody Maloney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28252/
> ---
> 
> (Updated Nov. 20, 2014, 12:06 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-2132
> https://issues.apache.org/jira/browse/MESOS-2132
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Simplified signature from the C++11 standard specification.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp 
> aa4d9ba68a27c1a7dea8cfe948a90487c4708add 
>   3rdparty/libprocess/3rdparty/stout/tests/hashmap_tests.cpp 
> eb3abfc9ba62f92d2ed5bc0e1c093791003a8319 
> 
> Diff: https://reviews.apache.org/r/28252/diff/
> 
> 
> Testing
> ---
> 
> make distcheck ubuntu 14.04
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Re: Review Request 29541: Added network::resolve and network::hostname.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29541/#review70483
---


Bad patch!

Reviews applied: [29526, 29527]

Failed command: ./support/apply-review.sh -n -r 29527

Error:
 2015-01-31 02:14:33 URL:https://reviews.apache.org/r/29527/diff/raw/ 
[8219/8219] -> "29527.patch" [1]
error: patch failed: 3rdparty/libprocess/src/tests/process_tests.cpp:21
error: 3rdparty/libprocess/src/tests/process_tests.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Jan. 31, 2015, 1:54 a.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29541/
> ---
> 
> (Updated Jan. 31, 2015, 1:54 a.m.)
> 
> 
> Review request for mesos, Dominic Hamon, Evelina Dumitrescu, Joris Van 
> Remoortere, and Niklas Nielsen.
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/network.hpp PRE-CREATION 
>   3rdparty/libprocess/src/http.cpp 869b205656fb73edb9f02a1856d10f79ed1349b4 
>   3rdparty/libprocess/src/pid.cpp 085e0b9abe9a9f33a63247915835decbf942274d 
>   3rdparty/libprocess/src/process.cpp 
> 67b6b3b9c13d95fa1a24b48a12c5c831c7f249bf 
> 
> Diff: https://reviews.apache.org/r/29541/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 30459: Refactored task/offer/resource valiation in master.

[Dominic Hamon]

> On Jan. 30, 2015, 4:48 p.m., Ben Mahler wrote:
> > Thanks for following up on the cleanup!
> > 
> > Looks much better! Some higher level feedback per our chat:
> > 
> > (1) Can we make this header more minimal by moving out everything except 
> > `validation::task::validate(...)` and `validation::offer::validate(...)`. 
> > There's a few reasons to motivate this, first is that this makes it easier 
> > to read this header and understand what the caller (master) should care 
> > about (right now it's a bit tough to come in to this header and "grok" it). 
> > Also, the other helpers still have a bit of dependencies between them (e.g. 
> > `validateResourceUsage` assumes the `ExecutorInfo` is valid) so we might 
> > want to avoid anyone calling these just yet :). If we want to test these, 
> > we can move them back to the header but hide them below the caller's API 
> > with a NOTE that they are exposed for testing :)
> > 
> > (2) Right now this calls every validator even if the previous validators 
> > fail. That's different from the existing semantics and it's unnecessary 
> > work. Could we use lambda::bind per our chat to avoid this?

i thought the same for 1) but they need to be there for tests in the future. 
given how that's coming soon (right?) the churn to remove and add them seems 
unnecessary.

for 2, is it a better experience to run all the validators and append the 
errors?


- Dominic


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30459/#review70468
---


On Jan. 30, 2015, 6:09 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30459/
> ---
> 
> (Updated Jan. 30, 2015, 6:09 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone.
> 
> 
> Bugs: MESOS-2305
> https://issues.apache.org/jira/browse/MESOS-2305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored task/offer/resource valiation in master.
> 
> See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 07bea1fb8f0035413f2119859e16fa4f9383f68e 
>   src/master/master.hpp 337e00aa46ea127f3667e3383d631c3fb8e22f30 
>   src/master/master.cpp 10056861b95ed9453c971787982db7d09f09f323 
>   src/master/validation.hpp PRE-CREATION 
>   src/master/validation.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/30459/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 30459: Refactored task/offer/resource valiation in master.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30459/
---

(Updated Jan. 31, 2015, 2:09 a.m.)


Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone.


Changes
---

BenM's comments.


Bugs: MESOS-2305
https://issues.apache.org/jira/browse/MESOS-2305


Repository: mesos


Description
---

Refactored task/offer/resource valiation in master.

See ticket for motivation.


Diffs (updated)
-

  src/Makefile.am 07bea1fb8f0035413f2119859e16fa4f9383f68e 
  src/master/master.hpp 337e00aa46ea127f3667e3383d631c3fb8e22f30 
  src/master/master.cpp 10056861b95ed9453c971787982db7d09f09f323 
  src/master/validation.hpp PRE-CREATION 
  src/master/validation.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/30459/diff/


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 29569: Refactored Once abstraction to not use Promise.

[Benjamin Hindman]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29569/
---

(Updated Jan. 31, 2015, 1:54 a.m.)


Review request for mesos, Joris Van Remoortere and Niklas Nielsen.


Repository: mesos-incubating


Description
---

The original once abstraction was implemented using promises, which made it 
unusable to initialize libprocess because libprocess must be initialized in 
order to use a Promise. This removes that requirement, but it also means that 
an actor blocking on a Once will no longer donate it's thread while it's 
waiting (since it doesn't wait on a Promise, which waits on a Latch, which 
waits on a ProcessBase, which donates the thread while waiting).


Diffs
-

  3rdparty/libprocess/include/process/once.hpp 
e85b38205065b34d438f68a6441905eb2f90169c 

Diff: https://reviews.apache.org/r/29569/diff/


Testing
---

make check


Thanks,

Benjamin Hindman

Re: Review Request 29541: Added network::resolve and network::hostname.

[Benjamin Hindman]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29541/
---

(Updated Jan. 31, 2015, 1:54 a.m.)


Review request for mesos, Dominic Hamon, Evelina Dumitrescu, Joris Van 
Remoortere, and Niklas Nielsen.


Repository: mesos-incubating


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/include/process/network.hpp PRE-CREATION 
  3rdparty/libprocess/src/http.cpp 869b205656fb73edb9f02a1856d10f79ed1349b4 
  3rdparty/libprocess/src/pid.cpp 085e0b9abe9a9f33a63247915835decbf942274d 
  3rdparty/libprocess/src/process.cpp 67b6b3b9c13d95fa1a24b48a12c5c831c7f249bf 

Diff: https://reviews.apache.org/r/29541/diff/


Testing
---

make check


Thanks,

Benjamin Hindman

Re: Review Request 29890: Refactored allocator interface to support general implementations.

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/#review70462
---



src/master/allocator.hpp


Can you do a forward declaration of MesosAllocatorProcess and pull the 
implementation below MesosAllocator?



src/master/allocator.hpp


s/invokations/invocations/



src/tests/cluster.hpp


This comment should be inside the else block.



src/tests/mesos.hpp


why does T have to be a HierarchicalDRFAllocator instead of an Allocator or 
MesosAllocator?


- Vinod Kone


On Jan. 30, 2015, 3:33 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> ---
> 
> (Updated Jan. 30, 2015, 3:33 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-2213
> https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Introduce a basic Allocator interface that every allocator should implement. 
> This interface does not require allocators to be based on libprocess process. 
> For allocators they do implement allocation logic via an internal libprocess 
> process, a special wrapper is provided. Allocator uses and tests are updated 
> to use Allocator type instead of AllocatorProcess.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 318a756 
>   src/master/hierarchical_allocator_process.hpp 6b44892 
>   src/master/main.cpp e5e76ce 
>   src/tests/cluster.hpp 90fda52 
>   src/tests/fault_tolerance_tests.cpp f927d4a 
>   src/tests/hierarchical_allocator_tests.cpp f44d9e9 
>   src/tests/master_allocator_tests.cpp 018a6cc 
>   src/tests/master_authorization_tests.cpp 20adaa9 
>   src/tests/master_tests.cpp 678d27f 
>   src/tests/mesos.hpp 17c2d8f 
>   src/tests/mesos.cpp 5ed4df5 
>   src/tests/partition_tests.cpp fea7801 
>   src/tests/rate_limiting_tests.cpp 7f5ca25 
>   src/tests/resource_offers_tests.cpp 24a7eab 
>   src/tests/slave_recovery_tests.cpp 7e2e63d 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> ---
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 30338: Introduced ExecutorRunState struct for Isolators and Launchers.

[Ian Downes]

> On Jan. 30, 2015, 4:43 p.m., Ian Downes wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 346
> > 
> >
> > Are you sure run.get().id.isSome()?
> 
> Kapil Arya wrote:
> RunState is created with the id field set to ContainerId. Unless it's 
> changed somewhere I didn't notice, I think the value will be set.  If we want 
> to be extra careful, we can use "cotainerId" local variable from a few lines 
> above.

True, should be set in RunState::recover. Add a CHECK_SOME then?


> On Jan. 30, 2015, 4:43 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolator.hpp, line 64
> > 
> >
> > What about moving this in containerizer.hpp? Then the launcher won't 
> > need to include isolator.hpp.
> 
> Kapil Arya wrote:
> Do you mean include/mesos/containerizer/containerizer.hpp or the one in 
> src/slave/containerizer? We can't do that latter since we want to expose 
> Isolator in the next patch and can't have any dependency on src/"

Hmm, I was thinking containerizer/mesos/containerizer.hpp but drop issue this 
if it'll mess up the dependencies.


- Ian


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30338/#review70465
---


On Jan. 30, 2015, 1:55 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30338/
> ---
> 
> (Updated Jan. 30, 2015, 1:55 p.m.)
> 
> 
> Review request for mesos, Ian Downes, nnichddq nnichddq, Till Toenshoff, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-2096
> https://issues.apache.org/jira/browse/MESOS-2096
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> [4/9] Build Mesos Modules outside Mesos source tree.
> 
> The struct is constructed from some fields of RunState and passed on to
> Isolators.  This further allows us to do cleanup various isolators/launchers.
> The goal was to remove the dependency on RunState so that we
> can eventually expose slave/containerizer/isolator.hpp as a public header.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/external_containerizer.hpp 
> 8363cec490a2918e08a8d30d9b8832fd1e2dd4f5 
>   src/slave/containerizer/isolator.hpp 
> a27c3e955ff25b87599af0ac2c97427a88b786f6 
>   src/slave/containerizer/isolator.cpp 
> 90a47c4fc8ce2d1a2aa09fa9ad905b76eab74c80 
>   src/slave/containerizer/isolators/cgroups/cpushare.hpp 
> 4ded0c4678ad61742a69e14233006a448f3fc3ca 
>   src/slave/containerizer/isolators/cgroups/cpushare.cpp 
> 90aabb85da452a3a50f738379a73b7465dc6 
>   src/slave/containerizer/isolators/cgroups/mem.hpp 
> 2fa755571b4d21b8b13301fcfd57ae05ea66e6e6 
>   src/slave/containerizer/isolators/cgroups/mem.cpp 
> 711d66d7771cac13be831d73af3ef570d6785473 
>   src/slave/containerizer/isolators/cgroups/perf_event.hpp 
> e511c3efe4cc4ec13cc74bdcda993477a4db2e36 
>   src/slave/containerizer/isolators/cgroups/perf_event.cpp 
> 6f67164d3963769148fb1749d1d590b7b2999fdb 
>   src/slave/containerizer/isolators/filesystem/shared.hpp 
> 727f63e16fa69293c472a4b6b95fb0be04e689c8 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 5c347af803bf512698dd580d6533c0103b289427 
>   src/slave/containerizer/isolators/namespaces/pid.hpp 
> 8da6cccff04edf11fa37f5a33648821299cf49d5 
>   src/slave/containerizer/isolators/namespaces/pid.cpp 
> fdd430e946c386e37ffc5b24907063180cddac17 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> 90d19a95092bc18c1077fbaec9ebf9c87291474c 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> b484bbf34060ccc9debd48a3e840c03c8f95de09 
>   src/slave/containerizer/isolators/posix.hpp 
> 7a667e3e30712b98484947520b58965d3b768659 
>   src/slave/containerizer/isolators/posix/disk.hpp 
> cf4e143eb50cf5ee59ca0d47dd6f8d0a845c3732 
>   src/slave/containerizer/isolators/posix/disk.cpp 
> bb1779fd644a33527068868a45cf9c19d7732727 
>   src/slave/containerizer/launcher.hpp 
> 18b3546a5e51e8a3819bca11630f699a0aadf762 
>   src/slave/containerizer/launcher.cpp 
> 92c0657728ddb9f6a4f9065ff8a7093c5371bc6f 
>   src/slave/containerizer/linux_launcher.hpp 
> 3d9794dd4983ecf59833f3971cbb094d4bf7710f 
>   src/slave/containerizer/linux_launcher.cpp 
> 10c12039cf684bef9398da72c3eceb9ed8b8b9c9 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 802988c90ac872b0cefa5e28f06e6fec98e8d032 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d712278428889ebdfd598537690138329d8464f0 
>   src/slave/flags.hpp a3c5c68a553b1c88ce6d5177e625079f7cdb2e5f 
>   src/tests/isolator.hpp 2c3a694fade996df6fb679e78f96876a7d32d214 
> 
> Diff: https://reviews.apache.org/r/30338/diff/
> 
> 
> Testing
> ---
> 
> make chec

Re: Review Request 30459: Refactored task/offer/resource valiation in master.

[Ben Mahler]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30459/#review70468
---


Thanks for following up on the cleanup!

Looks much better! Some higher level feedback per our chat:

(1) Can we make this header more minimal by moving out everything except 
`validation::task::validate(...)` and `validation::offer::validate(...)`. 
There's a few reasons to motivate this, first is that this makes it easier to 
read this header and understand what the caller (master) should care about 
(right now it's a bit tough to come in to this header and "grok" it). Also, the 
other helpers still have a bit of dependencies between them (e.g. 
`validateResourceUsage` assumes the `ExecutorInfo` is valid) so we might want 
to avoid anyone calling these just yet :). If we want to test these, we can 
move them back to the header but hide them below the caller's API with a NOTE 
that they are exposed for testing :)

(2) Right now this calls every validator even if the previous validators fail. 
That's different from the existing semantics and it's unnecessary work. Could 
we use lambda::bind per our chat to avoid this?

- Ben Mahler


On Jan. 30, 2015, 10:27 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30459/
> ---
> 
> (Updated Jan. 30, 2015, 10:27 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone.
> 
> 
> Bugs: MESOS-2305
> https://issues.apache.org/jira/browse/MESOS-2305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored task/offer/resource valiation in master.
> 
> See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 07bea1fb8f0035413f2119859e16fa4f9383f68e 
>   src/master/master.hpp 337e00aa46ea127f3667e3383d631c3fb8e22f30 
>   src/master/master.cpp 10056861b95ed9453c971787982db7d09f09f323 
>   src/master/validation.hpp PRE-CREATION 
>   src/master/validation.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/30459/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 30338: Introduced ExecutorRunState struct for Isolators and Launchers.

[Kapil Arya]

> On Jan. 30, 2015, 7:43 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolator.hpp, line 64
> > 
> >
> > What about moving this in containerizer.hpp? Then the launcher won't 
> > need to include isolator.hpp.

Do you mean include/mesos/containerizer/containerizer.hpp or the one in 
src/slave/containerizer? We can't do that latter since we want to expose 
Isolator in the next patch and can't have any dependency on src/"


> On Jan. 30, 2015, 7:43 p.m., Ian Downes wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 346
> > 
> >
> > Are you sure run.get().id.isSome()?

RunState is created with the id field set to ContainerId. Unless it's changed 
somewhere I didn't notice, I think the value will be set.  If we want to be 
extra careful, we can use "cotainerId" local variable from a few lines above.


- Kapil


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30338/#review70465
---


On Jan. 30, 2015, 4:55 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30338/
> ---
> 
> (Updated Jan. 30, 2015, 4:55 p.m.)
> 
> 
> Review request for mesos, Ian Downes, nnichddq nnichddq, Till Toenshoff, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-2096
> https://issues.apache.org/jira/browse/MESOS-2096
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> [4/9] Build Mesos Modules outside Mesos source tree.
> 
> The struct is constructed from some fields of RunState and passed on to
> Isolators.  This further allows us to do cleanup various isolators/launchers.
> The goal was to remove the dependency on RunState so that we
> can eventually expose slave/containerizer/isolator.hpp as a public header.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/external_containerizer.hpp 
> 8363cec490a2918e08a8d30d9b8832fd1e2dd4f5 
>   src/slave/containerizer/isolator.hpp 
> a27c3e955ff25b87599af0ac2c97427a88b786f6 
>   src/slave/containerizer/isolator.cpp 
> 90a47c4fc8ce2d1a2aa09fa9ad905b76eab74c80 
>   src/slave/containerizer/isolators/cgroups/cpushare.hpp 
> 4ded0c4678ad61742a69e14233006a448f3fc3ca 
>   src/slave/containerizer/isolators/cgroups/cpushare.cpp 
> 90aabb85da452a3a50f738379a73b7465dc6 
>   src/slave/containerizer/isolators/cgroups/mem.hpp 
> 2fa755571b4d21b8b13301fcfd57ae05ea66e6e6 
>   src/slave/containerizer/isolators/cgroups/mem.cpp 
> 711d66d7771cac13be831d73af3ef570d6785473 
>   src/slave/containerizer/isolators/cgroups/perf_event.hpp 
> e511c3efe4cc4ec13cc74bdcda993477a4db2e36 
>   src/slave/containerizer/isolators/cgroups/perf_event.cpp 
> 6f67164d3963769148fb1749d1d590b7b2999fdb 
>   src/slave/containerizer/isolators/filesystem/shared.hpp 
> 727f63e16fa69293c472a4b6b95fb0be04e689c8 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 5c347af803bf512698dd580d6533c0103b289427 
>   src/slave/containerizer/isolators/namespaces/pid.hpp 
> 8da6cccff04edf11fa37f5a33648821299cf49d5 
>   src/slave/containerizer/isolators/namespaces/pid.cpp 
> fdd430e946c386e37ffc5b24907063180cddac17 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> 90d19a95092bc18c1077fbaec9ebf9c87291474c 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> b484bbf34060ccc9debd48a3e840c03c8f95de09 
>   src/slave/containerizer/isolators/posix.hpp 
> 7a667e3e30712b98484947520b58965d3b768659 
>   src/slave/containerizer/isolators/posix/disk.hpp 
> cf4e143eb50cf5ee59ca0d47dd6f8d0a845c3732 
>   src/slave/containerizer/isolators/posix/disk.cpp 
> bb1779fd644a33527068868a45cf9c19d7732727 
>   src/slave/containerizer/launcher.hpp 
> 18b3546a5e51e8a3819bca11630f699a0aadf762 
>   src/slave/containerizer/launcher.cpp 
> 92c0657728ddb9f6a4f9065ff8a7093c5371bc6f 
>   src/slave/containerizer/linux_launcher.hpp 
> 3d9794dd4983ecf59833f3971cbb094d4bf7710f 
>   src/slave/containerizer/linux_launcher.cpp 
> 10c12039cf684bef9398da72c3eceb9ed8b8b9c9 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 802988c90ac872b0cefa5e28f06e6fec98e8d032 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d712278428889ebdfd598537690138329d8464f0 
>   src/slave/flags.hpp a3c5c68a553b1c88ce6d5177e625079f7cdb2e5f 
>   src/tests/isolator.hpp 2c3a694fade996df6fb679e78f96876a7d32d214 
> 
> Diff: https://reviews.apache.org/r/30338/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 30338: Introduced ExecutorRunState struct for Isolators and Launchers.

[Ian Downes]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30338/#review70465
---

Ship it!



src/slave/containerizer/isolator.hpp


What about moving this in containerizer.hpp? Then the launcher won't need 
to include isolator.hpp.



src/slave/containerizer/mesos/containerizer.cpp


Are you sure run.get().id.isSome()?


- Ian Downes


On Jan. 30, 2015, 1:55 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30338/
> ---
> 
> (Updated Jan. 30, 2015, 1:55 p.m.)
> 
> 
> Review request for mesos, Ian Downes, nnichddq nnichddq, Till Toenshoff, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-2096
> https://issues.apache.org/jira/browse/MESOS-2096
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> [4/9] Build Mesos Modules outside Mesos source tree.
> 
> The struct is constructed from some fields of RunState and passed on to
> Isolators.  This further allows us to do cleanup various isolators/launchers.
> The goal was to remove the dependency on RunState so that we
> can eventually expose slave/containerizer/isolator.hpp as a public header.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/external_containerizer.hpp 
> 8363cec490a2918e08a8d30d9b8832fd1e2dd4f5 
>   src/slave/containerizer/isolator.hpp 
> a27c3e955ff25b87599af0ac2c97427a88b786f6 
>   src/slave/containerizer/isolator.cpp 
> 90a47c4fc8ce2d1a2aa09fa9ad905b76eab74c80 
>   src/slave/containerizer/isolators/cgroups/cpushare.hpp 
> 4ded0c4678ad61742a69e14233006a448f3fc3ca 
>   src/slave/containerizer/isolators/cgroups/cpushare.cpp 
> 90aabb85da452a3a50f738379a73b7465dc6 
>   src/slave/containerizer/isolators/cgroups/mem.hpp 
> 2fa755571b4d21b8b13301fcfd57ae05ea66e6e6 
>   src/slave/containerizer/isolators/cgroups/mem.cpp 
> 711d66d7771cac13be831d73af3ef570d6785473 
>   src/slave/containerizer/isolators/cgroups/perf_event.hpp 
> e511c3efe4cc4ec13cc74bdcda993477a4db2e36 
>   src/slave/containerizer/isolators/cgroups/perf_event.cpp 
> 6f67164d3963769148fb1749d1d590b7b2999fdb 
>   src/slave/containerizer/isolators/filesystem/shared.hpp 
> 727f63e16fa69293c472a4b6b95fb0be04e689c8 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 5c347af803bf512698dd580d6533c0103b289427 
>   src/slave/containerizer/isolators/namespaces/pid.hpp 
> 8da6cccff04edf11fa37f5a33648821299cf49d5 
>   src/slave/containerizer/isolators/namespaces/pid.cpp 
> fdd430e946c386e37ffc5b24907063180cddac17 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> 90d19a95092bc18c1077fbaec9ebf9c87291474c 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> b484bbf34060ccc9debd48a3e840c03c8f95de09 
>   src/slave/containerizer/isolators/posix.hpp 
> 7a667e3e30712b98484947520b58965d3b768659 
>   src/slave/containerizer/isolators/posix/disk.hpp 
> cf4e143eb50cf5ee59ca0d47dd6f8d0a845c3732 
>   src/slave/containerizer/isolators/posix/disk.cpp 
> bb1779fd644a33527068868a45cf9c19d7732727 
>   src/slave/containerizer/launcher.hpp 
> 18b3546a5e51e8a3819bca11630f699a0aadf762 
>   src/slave/containerizer/launcher.cpp 
> 92c0657728ddb9f6a4f9065ff8a7093c5371bc6f 
>   src/slave/containerizer/linux_launcher.hpp 
> 3d9794dd4983ecf59833f3971cbb094d4bf7710f 
>   src/slave/containerizer/linux_launcher.cpp 
> 10c12039cf684bef9398da72c3eceb9ed8b8b9c9 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 802988c90ac872b0cefa5e28f06e6fec98e8d032 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d712278428889ebdfd598537690138329d8464f0 
>   src/slave/flags.hpp a3c5c68a553b1c88ce6d5177e625079f7cdb2e5f 
>   src/tests/isolator.hpp 2c3a694fade996df6fb679e78f96876a7d32d214 
> 
> Diff: https://reviews.apache.org/r/30338/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 29932: Renamed test allocator actions for consistency.

[Alexander Rukletsov]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29932/
---

(Updated Jan. 30, 2015, 3:44 p.m.)


Review request for mesos, Bernd Mathiske and Niklas Nielsen.


Changes
---

Rebased.


Bugs: MESOS-2213
https://issues.apache.org/jira/browse/MESOS-2213


Repository: mesos-git


Description
---

See summary.


Diffs (updated)
-

  src/tests/master_allocator_tests.cpp 018a6cc 
  src/tests/mesos.hpp 17c2d8f 

Diff: https://reviews.apache.org/r/29932/diff/


Testing
---

make check (Ubuntu, OS X)


Thanks,

Alexander Rukletsov

Re: Review Request 30082: Cleaned up namespace hierarchy in allocation sources.

[Alexander Rukletsov]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30082/
---

(Updated Jan. 30, 2015, 3:44 p.m.)


Review request for mesos, Bernd Mathiske and Niklas Nielsen.


Changes
---

Rebased.


Bugs: MESOS-2213
https://issues.apache.org/jira/browse/MESOS-2213


Repository: mesos-git


Description
---

See summary.


Diffs (updated)
-

  src/local/local.hpp 0aa50ef 
  src/local/local.cpp 76e73a4 
  src/master/allocation/allocator.hpp PRE-CREATION 
  src/master/allocation/mesos/allocator.hpp PRE-CREATION 
  src/master/allocation/mesos/hierarchical.hpp PRE-CREATION 
  src/master/allocation/sorter.hpp PRE-CREATION 
  src/master/allocation/sorters/drf.hpp PRE-CREATION 
  src/master/allocation/sorters/drf.cpp PRE-CREATION 
  src/master/main.cpp e5e76ce 
  src/master/master.hpp 337e00a 
  src/master/master.cpp 1005686 
  src/tests/cluster.hpp 90fda52 
  src/tests/fault_tolerance_tests.cpp f927d4a 
  src/tests/hierarchical_allocator_tests.cpp f44d9e9 
  src/tests/master_allocator_tests.cpp 018a6cc 
  src/tests/master_authorization_tests.cpp 20adaa9 
  src/tests/master_tests.cpp 678d27f 
  src/tests/mesos.hpp 17c2d8f 
  src/tests/mesos.cpp 5ed4df5 
  src/tests/partition_tests.cpp fea7801 
  src/tests/rate_limiting_tests.cpp 7f5ca25 
  src/tests/resource_offers_tests.cpp 24a7eab 
  src/tests/slave_recovery_tests.cpp 7e2e63d 
  src/tests/sorter_tests.cpp 520a42e 

Diff: https://reviews.apache.org/r/30082/diff/


Testing (updated)
---

make check (OS X, Ubuntu)


Thanks,

Alexander Rukletsov

Re: Review Request 29931: Extracted MesosAllocator into a separate file.

[Alexander Rukletsov]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29931/
---

(Updated Jan. 30, 2015, 3:41 p.m.)


Review request for mesos, Bernd Mathiske and Niklas Nielsen.


Changes
---

Rebased.


Bugs: MESOS-2213
https://issues.apache.org/jira/browse/MESOS-2213


Repository: mesos-git


Description
---

See summary.


Diffs (updated)
-

  src/Makefile.am 07bea1f 
  src/master/allocation/allocator.hpp PRE-CREATION 
  src/master/allocation/mesos/allocator.hpp PRE-CREATION 
  src/master/allocation/mesos/hierarchical.hpp PRE-CREATION 

Diff: https://reviews.apache.org/r/29931/diff/


Testing
---

make check (Ubuntu, OS X)


Thanks,

Alexander Rukletsov

Re: Review Request 29929: Cleaned up includes in allocation sources.

[Alexander Rukletsov]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29929/
---

(Updated Jan. 30, 2015, 3:40 p.m.)


Review request for mesos, Bernd Mathiske and Niklas Nielsen.


Changes
---

Rebased.


Bugs: MESOS-2213
https://issues.apache.org/jira/browse/MESOS-2213


Repository: mesos-git


Description
---

See summary.


Diffs (updated)
-

  src/local/local.cpp 76e73a4 
  src/master/allocation/allocator.hpp PRE-CREATION 
  src/master/allocation/mesos/hierarchical.hpp PRE-CREATION 
  src/master/main.cpp e5e76ce 
  src/tests/master_tests.cpp 678d27f 

Diff: https://reviews.apache.org/r/29929/diff/


Testing
---

make check (Ubuntu, OS X)


Thanks,

Alexander Rukletsov

Re: Review Request 29927: Removed unnecessary lifecycle method from MasterAllocatorTest.

[Alexander Rukletsov]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29927/
---

(Updated Jan. 30, 2015, 3:36 p.m.)


Review request for mesos, Bernd Mathiske and Niklas Nielsen.


Changes
---

Address BenM's comments; rebased.


Summary (updated)
-

Removed unnecessary lifecycle method from MasterAllocatorTest.


Bugs: MESOS-2213
https://issues.apache.org/jira/browse/MESOS-2213


Repository: mesos-git


Description (updated)
---

Updated TestAllocator does not cause GMOCK warnings for unused method calls. 
Hence there is no need for stopping allocation explicitly.


Diffs (updated)
-

  src/tests/master_allocator_tests.cpp 018a6cc 

Diff: https://reviews.apache.org/r/29927/diff/


Testing
---

make check (Ubuntu, OS X)


Thanks,

Alexander Rukletsov

Re: Review Request 29925: Moved allocation related sources into a separate directory.

[Alexander Rukletsov]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29925/
---

(Updated Jan. 30, 2015, 3:33 p.m.)


Review request for mesos, Bernd Mathiske and Niklas Nielsen.


Changes
---

Rebased.


Bugs: MESOS-2213
https://issues.apache.org/jira/browse/MESOS-2213


Repository: mesos-git


Description
---

See summary.


Diffs (updated)
-

  src/Makefile.am 07bea1f 
  src/local/local.cpp 76e73a4 
  src/master/allocation/allocator.hpp PRE-CREATION 
  src/master/allocation/mesos/hierarchical.hpp PRE-CREATION 
  src/master/allocation/sorter.hpp PRE-CREATION 
  src/master/allocation/sorters/drf.hpp PRE-CREATION 
  src/master/allocation/sorters/drf.cpp PRE-CREATION 
  src/master/allocator.hpp 318a756 
  src/master/drf_sorter.hpp 5a66313 
  src/master/drf_sorter.cpp 584e26c 
  src/master/hierarchical_allocator_process.hpp 6b44892 
  src/master/main.cpp e5e76ce 
  src/master/master.cpp 1005686 
  src/master/sorter.hpp 8915c61 
  src/tests/cluster.hpp 90fda52 
  src/tests/fault_tolerance_tests.cpp f927d4a 
  src/tests/hierarchical_allocator_tests.cpp f44d9e9 
  src/tests/master_allocator_tests.cpp 018a6cc 
  src/tests/master_authorization_tests.cpp 20adaa9 
  src/tests/master_slave_reconciliation_tests.cpp 04806ed 
  src/tests/master_tests.cpp 678d27f 
  src/tests/mesos.hpp 17c2d8f 
  src/tests/rate_limiting_tests.cpp 7f5ca25 
  src/tests/resource_offers_tests.cpp 24a7eab 
  src/tests/sorter_tests.cpp 520a42e 

Diff: https://reviews.apache.org/r/29925/diff/


Testing
---

make check (Ubuntu, OS X)


Thanks,

Alexander Rukletsov

Re: Review Request 29890: Refactored allocator interface to support general implementations.

[Alexander Rukletsov]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/
---

(Updated Jan. 30, 2015, 3:33 p.m.)


Review request for mesos, Bernd Mathiske, Niklas Nielsen, and Vinod Kone.


Changes
---

Rebased.


Bugs: MESOS-2213
https://issues.apache.org/jira/browse/MESOS-2213


Repository: mesos-git


Description
---

Introduce a basic Allocator interface that every allocator should implement. 
This interface does not require allocators to be based on libprocess process. 
For allocators they do implement allocation logic via an internal libprocess 
process, a special wrapper is provided. Allocator uses and tests are updated to 
use Allocator type instead of AllocatorProcess.


Diffs (updated)
-

  src/local/local.cpp 76e73a4 
  src/master/allocator.hpp 318a756 
  src/master/hierarchical_allocator_process.hpp 6b44892 
  src/master/main.cpp e5e76ce 
  src/tests/cluster.hpp 90fda52 
  src/tests/fault_tolerance_tests.cpp f927d4a 
  src/tests/hierarchical_allocator_tests.cpp f44d9e9 
  src/tests/master_allocator_tests.cpp 018a6cc 
  src/tests/master_authorization_tests.cpp 20adaa9 
  src/tests/master_tests.cpp 678d27f 
  src/tests/mesos.hpp 17c2d8f 
  src/tests/mesos.cpp 5ed4df5 
  src/tests/partition_tests.cpp fea7801 
  src/tests/rate_limiting_tests.cpp 7f5ca25 
  src/tests/resource_offers_tests.cpp 24a7eab 
  src/tests/slave_recovery_tests.cpp 7e2e63d 

Diff: https://reviews.apache.org/r/29890/diff/


Testing
---

make distcheck (Ubuntu, OS X)


Thanks,

Alexander Rukletsov

Re: Review Request 30463: Increased the default AWAIT timeout.

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30463/#review70420
---

Ship it!


Ship It!

- Vinod Kone


On Jan. 30, 2015, 7:54 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30463/
> ---
> 
> (Updated Jan. 30, 2015, 7:54 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Attempt #2 of: https://reviews.apache.org/r/30439/
> 
> I've observed a number of CI VM stalls that lead to test failures, this is an 
> attempt to mitigate some of the flaky tests in the face of VM stalls.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/gtest.hpp 
> 10f991d4e6a6ec8479f342ee812faa3f5c3870bb 
> 
> Diff: https://reviews.apache.org/r/30463/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 29288: stout: Created IP address abstraction for different protocol families

[Evelina Dumitrescu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29288/
---

(Updated Jan. 30, 2015, 6:03 p.m.)


Review request for mesos, Benjamin Hindman, Dominic Hamon, Jie Yu, Joris Van 
Remoortere, and Niklas Nielsen.


Changes
---

added more tests


Bugs: MESOS-1919
https://issues.apache.org/jira/browse/MESOS-1919


Repository: mesos-git


Description
---

Created the inner class InAddrStorage encapsulated inside the IP class.
The class uses a union with the in_addr and in6_addr fields.
I considered that the The MasterInfo protobuffers should have both an ipv4 and 
an ipv6 field.
I intend to use the same Classifiers, addition, removal and update of container 
filters, but write different encode/decode functions for IPv4/ICMP and 
IPv6/ICMPv6 because the processing of the protocol headers differ.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
a0210ea6440086246aafe632f86498abbb70719a 
  3rdparty/libprocess/3rdparty/stout/tests/net_tests.cpp 
425132e5d7c3770be4a5a39feea5a2f22179b871 

Diff: https://reviews.apache.org/r/29288/diff/


Testing
---

make check


Thanks,

Evelina Dumitrescu

Re: Review Request 29289: libprocess: Created IP address abstraction for different protocol families

[Evelina Dumitrescu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29289/
---

(Updated Jan. 30, 2015, 6:03 p.m.)


Review request for mesos, Benjamin Hindman, Dominic Hamon, Jie Yu, Joris Van 
Remoortere, and Niklas Nielsen.


Changes
---

split the long line


Bugs: MESOS-1919
https://issues.apache.org/jira/browse/MESOS-1919


Repository: mesos-git


Description
---

Created the inner class InAddrStorage encapsulated inside the IP class.
The class uses a union with the in_addr and in6_addr fields.
I considered that the The MasterInfo protobuffers should have both an ipv4 and 
an ipv6 field.
I intend to use the same Classifiers, addition, removal and update of container 
filters, but write different encode/decode functions for IPv4/ICMP and 
IPv6/ICMPv6 because the processing of the protocol headers differ.


Diffs (updated)
-

  3rdparty/libprocess/include/process/node.hpp 
173eb8abe0d0930c92f261be7a95e07b1d4c16af 
  3rdparty/libprocess/include/process/pid.hpp 
7dccf297ea473a9b19dd450f02c7ae4fd0c60a22 
  3rdparty/libprocess/include/process/socket.hpp 
ddb9e365fc1e65a568bdac4973964df1ab8cc05e 
  3rdparty/libprocess/src/http.cpp 869b205656fb73edb9f02a1856d10f79ed1349b4 
  3rdparty/libprocess/src/pid.cpp 085e0b9abe9a9f33a63247915835decbf942274d 
  3rdparty/libprocess/src/process.cpp 67b6b3b9c13d95fa1a24b48a12c5c831c7f249bf 
  3rdparty/libprocess/src/socket.cpp 4b0f6bec8051f938812dbc90a7312e4082ea203f 

Diff: https://reviews.apache.org/r/29289/diff/


Testing
---

make check


Thanks,

Evelina Dumitrescu

Re: Review Request 29534: Added http::get/put/post that take URL.

[Benjamin Hindman]

> On Jan. 22, 2015, 11:43 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/http.cpp, line 86
> > 
> >
> > Do we intend to add a seperate header / impl for https? What's the plan 
> > looking forward?

The plan for now was just to create an SSL socket if scheme == "https". If we 
want to create an https namespace in the future we could do that too.


> On Jan. 22, 2015, 11:43 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/http.cpp, line 102
> > 
> >
> > Once URL uses unrestricted union to store ip or domain you won't have 
> > to protect against this case :-)

I'll still have to check something though!


> On Jan. 22, 2015, 11:43 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/http.cpp, lines 146-158
> > 
> >
> > This is the same (painful) code as in the URL streamer. Maybe this can 
> > be factored out?

Yup, once I pull in Cody's changes.


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29534/#review69301
---


On Jan. 21, 2015, 5:45 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29534/
> ---
> 
> (Updated Jan. 21, 2015, 5:45 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 9cf05acbb724ab9af8010d1788621d37a0e48e86 
>   3rdparty/libprocess/src/http.cpp 869b205656fb73edb9f02a1856d10f79ed1349b4 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> c71a91d516214d20d3df4c220e4a39a070db3260 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> 0c80c69bc3457fd70ab745ff8a02be6ec9cfd9e7 
> 
> Diff: https://reviews.apache.org/r/29534/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 29932: Renamed test allocator actions for consistency.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29932/#review70371
---


Patch looks great!

Reviews applied: [29890, 29925, 29927, 29929, 29931, 30082, 29932]

All tests passed.

- Mesos ReviewBot


On Jan. 30, 2015, 3:44 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29932/
> ---
> 
> (Updated Jan. 30, 2015, 3:44 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2213
> https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/master_allocator_tests.cpp 018a6cc 
>   src/tests/mesos.hpp 17c2d8f 
> 
> Diff: https://reviews.apache.org/r/29932/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 29533: Added a URL abstraction to stout.

[Benjamin Hindman]

> On Jan. 22, 2015, 11:28 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/url.hpp, line 88
> > 
> >
> > Is there any less painful way to do this? It seems rather round-about / 
> > in-efficient. (Constructing a vector of strings, then a giant string, and 
> > then appending that to a stringstream.
> > 
> > I know we're trying to cleanly truncate one '&' character.
> > 
> > For example, if we had an overload for std::pair in stringify, 
> > then we could pass the hashmap to the string::join('&', url.query) function.
> > 
> > We might also want to support a strings::join for iterable that appends 
> > to the stream (like the templated version does) as opposed to constring a 
> > string and then streaming that out again.

I'm going to pull in Cody's changes and then fix this!


> On Jan. 22, 2015, 11:28 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/url.hpp, line 82
> > 
> >
> > Do we have a policy around using single quotes for single character 
> > literals? ':' instead of ":"

No policy, but for consistency I've always just used quotes.


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29533/#review69295
---


On Jan. 21, 2015, 5:44 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29533/
> ---
> 
> (Updated Jan. 21, 2015, 5:44 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 8e51957d141af0be64cac42f65e03bca5929c8a9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/url.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/url_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29533/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 29533: Added a URL abstraction to stout.

[Benjamin Hindman]

> On Jan. 23, 2015, 7:39 a.m., Cody Maloney wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/url.hpp, line 91
> > 
> >
> > This is actually incorrect URL encoding if key or value contain '&' 
> > (which is legal). See the long thread in https://reviews.apache.org/r/28253/

Yes, I'll pull in your patch and replace here, thanks!


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29533/#review69367
---


On Jan. 21, 2015, 5:44 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29533/
> ---
> 
> (Updated Jan. 21, 2015, 5:44 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 8e51957d141af0be64cac42f65e03bca5929c8a9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/url.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/url_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29533/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 29533: Added a URL abstraction to stout.

[Benjamin Hindman]

> On Jan. 27, 2015, 11:42 p.m., Cody Maloney wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/url.hpp, line 1
> > 
> >
> > This file needs to be added to 
> > `3rdparty/libprocess/3rdparty/stout/include/Makefile.am`
> > 
> > Otherwise `make distcheck` fails.

Thanks!


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29533/#review69915
---


On Jan. 21, 2015, 5:44 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29533/
> ---
> 
> (Updated Jan. 21, 2015, 5:44 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 8e51957d141af0be64cac42f65e03bca5929c8a9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/url.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/url_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29533/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 30439: Increased the default AWAIT timeout.

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30439/#review70391
---


Unable to load the diff?

- Vinod Kone


On Jan. 30, 2015, 2:12 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30439/
> ---
> 
> (Updated Jan. 30, 2015, 2:12 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> I've observed a number of CI VM stalls that lead to test failures, this is an 
> attempt to mitigate some of the flaky tests in the face of VM stalls.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/gtest.hpp 
> 10f991d4e6a6ec8479f342ee812faa3f5c3870bb 
> 
> Diff: https://reviews.apache.org/r/30439/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 30459: Refactored task/offer/resource valiation in master.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30459/#review70409
---


The diff is pretty unreadable because the diff algorithm. It would be better to 
just apply the patch. I didn't change any core logic, just code morevement.

- Jie Yu


On Jan. 30, 2015, 7:13 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30459/
> ---
> 
> (Updated Jan. 30, 2015, 7:13 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-2305
> https://issues.apache.org/jira/browse/MESOS-2305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored task/offer/resource valiation in master.
> 
> See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 337e00aa46ea127f3667e3383d631c3fb8e22f30 
>   src/master/master.cpp 10056861b95ed9453c971787982db7d09f09f323 
> 
> Diff: https://reviews.apache.org/r/30459/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 29890: Refactored allocator interface to support general implementations.

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/#review70405
---


Looks like there is a problem viewing diffs on review board :(

- Vinod Kone


On Jan. 30, 2015, 3:33 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> ---
> 
> (Updated Jan. 30, 2015, 3:33 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-2213
> https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Introduce a basic Allocator interface that every allocator should implement. 
> This interface does not require allocators to be based on libprocess process. 
> For allocators they do implement allocation logic via an internal libprocess 
> process, a special wrapper is provided. Allocator uses and tests are updated 
> to use Allocator type instead of AllocatorProcess.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 318a756 
>   src/master/hierarchical_allocator_process.hpp 6b44892 
>   src/master/main.cpp e5e76ce 
>   src/tests/cluster.hpp 90fda52 
>   src/tests/fault_tolerance_tests.cpp f927d4a 
>   src/tests/hierarchical_allocator_tests.cpp f44d9e9 
>   src/tests/master_allocator_tests.cpp 018a6cc 
>   src/tests/master_authorization_tests.cpp 20adaa9 
>   src/tests/master_tests.cpp 678d27f 
>   src/tests/mesos.hpp 17c2d8f 
>   src/tests/mesos.cpp 5ed4df5 
>   src/tests/partition_tests.cpp fea7801 
>   src/tests/rate_limiting_tests.cpp 7f5ca25 
>   src/tests/resource_offers_tests.cpp 24a7eab 
>   src/tests/slave_recovery_tests.cpp 7e2e63d 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> ---
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 29603: Exposed slave/containerizer/isolator.hpp as mesos/slave/isolator.hpp

[Kapil Arya]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29603/
---

(Updated Jan. 30, 2015, 4:55 p.m.)


Review request for mesos, Niklas Nielsen, Till Toenshoff, and Timothy Chen.


Changes
---

rebased.


Bugs: MESOS-2096
https://issues.apache.org/jira/browse/MESOS-2096


Repository: mesos-incubating


Description
---

[5/9] Build Mesos Modules outside Mesos source tree.

Expose "slave/state.hpp" and "slave/containerizer/isolator.hpp" to allow 
modules to include them directly from the Mesos install location.


Diffs (updated)
-

  src/Makefile.am 07bea1fb8f0035413f2119859e16fa4f9383f68e 
  src/examples/test_isolator_module.cpp 
dc107a1315271b0275c60819c4b5aaa1f527e73c 
  src/module/isolator.hpp 0f71e700ed2de20bdc9a1fdad087005a14e208a8 
  src/slave/containerizer/containerizer.cpp 
421bb868b353e644578fa27f04bdd636bfc89134 
  src/slave/containerizer/external_containerizer.hpp 
8363cec490a2918e08a8d30d9b8832fd1e2dd4f5 
  src/slave/containerizer/isolator.hpp a27c3e955ff25b87599af0ac2c97427a88b786f6 
  src/slave/containerizer/isolator.cpp 90a47c4fc8ce2d1a2aa09fa9ad905b76eab74c80 
  src/slave/containerizer/isolators/cgroups/cpushare.hpp 
4ded0c4678ad61742a69e14233006a448f3fc3ca 
  src/slave/containerizer/isolators/cgroups/mem.hpp 
2fa755571b4d21b8b13301fcfd57ae05ea66e6e6 
  src/slave/containerizer/isolators/cgroups/perf_event.hpp 
e511c3efe4cc4ec13cc74bdcda993477a4db2e36 
  src/slave/containerizer/isolators/filesystem/shared.hpp 
727f63e16fa69293c472a4b6b95fb0be04e689c8 
  src/slave/containerizer/isolators/namespaces/pid.hpp 
8da6cccff04edf11fa37f5a33648821299cf49d5 
  src/slave/containerizer/isolators/network/port_mapping.hpp 
90d19a95092bc18c1077fbaec9ebf9c87291474c 
  src/slave/containerizer/isolators/posix.hpp 
7a667e3e30712b98484947520b58965d3b768659 
  src/slave/containerizer/isolators/posix/disk.hpp 
cf4e143eb50cf5ee59ca0d47dd6f8d0a845c3732 
  src/slave/containerizer/launcher.hpp 18b3546a5e51e8a3819bca11630f699a0aadf762 
  src/slave/containerizer/mesos/containerizer.hpp 
802988c90ac872b0cefa5e28f06e6fec98e8d032 
  src/slave/containerizer/mesos/containerizer.cpp 
d712278428889ebdfd598537690138329d8464f0 
  src/tests/containerizer_tests.cpp cfe31a6b528ec44c5f4ff9eaf453c68d9e518db4 
  src/tests/isolator.hpp 2c3a694fade996df6fb679e78f96876a7d32d214 
  src/tests/isolator_tests.cpp 1f1c26d4a8faf6fcea822fccc00bb58a478285f8 
  src/tests/module_tests.cpp df30e169a1e9ee2e6c419ba30612d55328e3adbc 
  src/tests/port_mapping_tests.cpp 18d58ff310d189f7461eead445e186172cbcd101 

Diff: https://reviews.apache.org/r/29603/diff/


Testing
---

make check.


Thanks,

Kapil Arya

Re: Review Request 29781: Exposed type_utils.hpp as include/mesos/type_utils.hpp.

[Kapil Arya]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29781/
---

(Updated Jan. 30, 2015, 4:55 p.m.)


Review request for mesos, Niklas Nielsen, Till Toenshoff, and Timothy Chen.


Changes
---

Rebased.


Bugs: MESOS-2096
https://issues.apache.org/jira/browse/MESOS-2096


Repository: mesos-incubating


Description
---

[3/9] Build Mesos Modules outside Mesos source tree.

This file includes definitions for operators on public protobufs
(defined in mesos.proto) that don't have these operators generated
by the protobuf compiler.  The corresponding definitions are in
src/common/type_utils.cpp.

Mesos modules need some of the protobufs defined in mesos.proto and
require some of these operators declared in type_utils.hpp. Exposing
type_utils.hpp allows us to build modules without having a dependency
on mesos source tree (src/*).


Diffs (updated)
-

  src/Makefile.am 07bea1fb8f0035413f2119859e16fa4f9383f68e 
  src/cli/execute.cpp 77deec91177c90a8a20ccd4fe063b82ca08cefbd 
  src/common/protobuf_utils.cpp c4b53a81c0426d361363c12920d67c261e381553 
  src/common/type_utils.hpp 6809a12611ccd5de9e75c642d7dd09227c2eb5d2 
  src/common/type_utils.cpp fcc9eb0e27a9cbf53f7923f0c051a6578527ec37 
  src/examples/low_level_scheduler_libprocess.cpp 
a0ec131fff1773280b17e97cb78a3da88afe6f7d 
  src/examples/low_level_scheduler_pthread.cpp 
f05489ae10032dcf177ce149938cd94186355351 
  src/examples/no_executor_framework.cpp 
5e47cfcfa1677052be114f8a9b0e24f6ef0238d9 
  src/examples/test_framework.cpp e5ec3b9f75446980ce9235209b8b74128e3e5e39 
  src/exec/exec.cpp aada24664dba9060a92230e25689c89852585443 
  src/launcher/executor.cpp d527fec3b9e658099571f808aa824bd6e17a6e0d 
  src/log/coordinator.cpp fe0c5f9c55fd7a19b9c85a0860f88dd14205a631 
  src/log/recover.cpp 679741b843a1df112090cd2ce2b1bc20abbbcf44 
  src/log/replica.cpp dedfde4eebc12c6c4d1e75fe1f6751a56324688a 
  src/master/http.cpp 46890bed05d7c4b63e1f7be5bb35217173e0ade8 
  src/master/master.hpp 1d342e56116ad63aade43484b6899ce26f25abfd 
  src/master/registrar.cpp 0831963e01e3c87bf0ac27af24b832ab2ae09d97 
  src/sched/sched.cpp a822c002781b35872d3c477366775a3705343cd2 
  src/scheduler/scheduler.cpp 44713cae36a83081c9a665d2eb73f9dbec2d4268 
  src/slave/containerizer/external_containerizer.cpp 
8d5a9047afb24a29413bfc7226f47b1edbfa4ff9 
  src/slave/containerizer/isolators/cgroups/cpushare.cpp 
90aabb85da452a3a50f738379a73b7465dc6 
  src/slave/containerizer/isolators/cgroups/mem.cpp 
711d66d7771cac13be831d73af3ef570d6785473 
  src/slave/http.cpp d1cf8a68fab9a2df44f6c753683ad37fd4b1a1f9 
  src/slave/monitor.hpp f4ebafc5bbba43669dc1436899072b009936a22e 
  src/slave/paths.cpp fe951abac1254748dbd5bdd81b7e50da75afad6d 
  src/slave/slave.hpp 70bd8c1fde4ea09fa54c76aa93424a1adb0309f6 
  src/slave/slave.cpp fca83b3977b95ddda30f9830da10e124b5c605e6 
  src/slave/state.hpp de631fb2c8a8d2bcbb861c438b18141ba6211024 
  src/slave/status_update_manager.hpp daab8e332a286ed98fd54d170853b4edcf9f915e 
  src/tests/flags.hpp 0d3db6cd13b898674e7e62ce90543c5834225294 
  src/tests/protobuf_io_tests.cpp d5d788c32ef9d4e9434401464ed1c549b4559b38 
  src/tests/registrar_tests.cpp 6bfb679d6ab07143f9c77b8cd9719a36d363645f 
  src/tests/state_tests.cpp ef2e96fb8b3a5e3b9c3c045b22f43f818d327b9e 

Diff: https://reviews.apache.org/r/29781/diff/


Testing
---

make check.


Thanks,

Kapil Arya

Re: Review Request 29526: Replaced network::socket with network::Socket.

[Joris Van Remoortere]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29526/#review70425
---

Ship it!


Ship It!

- Joris Van Remoortere


On Jan. 2, 2015, 4:45 a.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29526/
> ---
> 
> (Updated Jan. 2, 2015, 4:45 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Niklas Nielsen.
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> c71a91d516214d20d3df4c220e4a39a070db3260 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 3bbfe0a7a65acb52d139fda81816acf305d891f5 
> 
> Diff: https://reviews.apache.org/r/29526/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 29598: Moved Modules protobuf to mesos/modules.proto.

[Kapil Arya]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29598/
---

(Updated Jan. 30, 2015, 4:54 p.m.)


Review request for mesos, Niklas Nielsen, Till Toenshoff, and Timothy Chen.


Changes
---

Update Makefile to install modules.pb.h as well.


Bugs: MESOS-2096
https://issues.apache.org/jira/browse/MESOS-2096


Repository: mesos-incubating


Description
---

[1/9] Build Mesos Modules outside Mesos source tree.

To expose Modules message outside the mesos source tree.  This allows modules 
to #include mesos/mesos.hpp and get the required protobuf definition.


Diffs (updated)
-

  include/mesos/module.hpp 594212605127522a1b5f0d1335d6c208b499289b 
  include/mesos/module.proto PRE-CREATION 
  src/Makefile.am 07bea1fb8f0035413f2119859e16fa4f9383f68e 
  src/common/parse.hpp ae581e5168606bf7ea7e092c5095ddf9d321c792 
  src/common/type_utils.hpp 6809a12611ccd5de9e75c642d7dd09227c2eb5d2 
  src/master/flags.hpp 6c18a1af625311ef149b5877b08f63c2b12c040d 
  src/messages/messages.proto 5bd075e9bfe17be334294b5e4c573eb0233030be 
  src/slave/flags.hpp a3c5c68a553b1c88ce6d5177e625079f7cdb2e5f 

Diff: https://reviews.apache.org/r/29598/diff/


Testing
---

make check


Thanks,

Kapil Arya

Re: Review Request 29605: Moved authentication messages to mesos/authentication.proto.

[Kapil Arya]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29605/
---

(Updated Jan. 30, 2015, 4:56 p.m.)


Review request for mesos, Niklas Nielsen, Till Toenshoff, and Timothy Chen.


Changes
---

Install authentication.pb.h as well.


Bugs: MESOS-2096
https://issues.apache.org/jira/browse/MESOS-2096


Repository: mesos-incubating


Description
---

[7/9] Build Mesos Modules outside Mesos source tree.

To allow authentication modules being built outside the Mesos source tree to 
use protobufs related to authentication.


Diffs (updated)
-

  include/mesos/authentication.hpp PRE-CREATION 
  include/mesos/authentication.proto PRE-CREATION 
  src/Makefile.am 07bea1fb8f0035413f2119859e16fa4f9383f68e 
  src/authentication/authenticatee.hpp 361083b5e1ca5ab1a594b3108fb768626313a725 
  src/authentication/authenticator.hpp 460494a49ad7c71803507afe62005280d5929d41 
  src/messages/messages.proto 5bd075e9bfe17be334294b5e4c573eb0233030be 
  src/tests/authentication_tests.cpp 5cf2da4602edbf7c168351d5f581aca10591d5d0 
  src/tests/fault_tolerance_tests.cpp e0065b3a89b9af27c5584bcfdd8c790814bf1af6 

Diff: https://reviews.apache.org/r/29605/diff/


Testing
---

make check.


Thanks,

Kapil Arya

Re: Review Request 30439: Increased the default AWAIT timeout.

[Benjamin Mahler]

Weird, this was ok last night, but I see now that the repository is set as
mesos-incubating. Deleted my incubating remote from git and re-posted here:

https://reviews.apache.org/r/30463/diff/#

I see cody also has this problem:

https://reviews.apache.org/r/30441/diff/#

On Fri, Jan 30, 2015 at 10:42 AM, Vinod Kone  wrote:

>This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30439/
>
> Unable to load the diff?
>
>
> - Vinod Kone
>
> On January 30th, 2015, 2:12 a.m. UTC, Ben Mahler wrote:
>   Review request for mesos and Vinod Kone.
> By Ben Mahler.
>
> *Updated Jan. 30, 2015, 2:12 a.m.*
>  *Repository: * mesos-incubating
> Description
>
> I've observed a number of CI VM stalls that lead to test failures, this is an 
> attempt to mitigate some of the flaky tests in the face of VM stalls.
>
>   Testing
>
> make check
>
>   Diffs
>
>- 3rdparty/libprocess/include/process/gtest.hpp
>(10f991d4e6a6ec8479f342ee812faa3f5c3870bb)
>
> View Diff 
>

Re: Review Request 30459: Refactored task/offer/resource valiation in master.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30459/#review70415
---


Patch looks great!

Reviews applied: [30459]

All tests passed.

- Mesos ReviewBot


On Jan. 30, 2015, 7:49 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30459/
> ---
> 
> (Updated Jan. 30, 2015, 7:49 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-2305
> https://issues.apache.org/jira/browse/MESOS-2305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored task/offer/resource valiation in master.
> 
> See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 07bea1fb8f0035413f2119859e16fa4f9383f68e 
>   src/master/master.hpp 337e00aa46ea127f3667e3383d631c3fb8e22f30 
>   src/master/master.cpp 10056861b95ed9453c971787982db7d09f09f323 
>   src/master/validation.hpp PRE-CREATION 
>   src/master/validation.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/30459/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 30338: Introduced ExecutorRunState struct for Isolators and Launchers.

[Kapil Arya]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30338/
---

(Updated Jan. 30, 2015, 4:55 p.m.)


Review request for mesos, Ian Downes, nnichddq nnichddq, Till Toenshoff, and 
Vinod Kone.


Changes
---

Fixed some #includes.


Bugs: MESOS-2096
https://issues.apache.org/jira/browse/MESOS-2096


Repository: mesos-incubating


Description
---

[4/9] Build Mesos Modules outside Mesos source tree.

The struct is constructed from some fields of RunState and passed on to
Isolators.  This further allows us to do cleanup various isolators/launchers.
The goal was to remove the dependency on RunState so that we
can eventually expose slave/containerizer/isolator.hpp as a public header.


Diffs (updated)
-

  src/slave/containerizer/external_containerizer.hpp 
8363cec490a2918e08a8d30d9b8832fd1e2dd4f5 
  src/slave/containerizer/isolator.hpp a27c3e955ff25b87599af0ac2c97427a88b786f6 
  src/slave/containerizer/isolator.cpp 90a47c4fc8ce2d1a2aa09fa9ad905b76eab74c80 
  src/slave/containerizer/isolators/cgroups/cpushare.hpp 
4ded0c4678ad61742a69e14233006a448f3fc3ca 
  src/slave/containerizer/isolators/cgroups/cpushare.cpp 
90aabb85da452a3a50f738379a73b7465dc6 
  src/slave/containerizer/isolators/cgroups/mem.hpp 
2fa755571b4d21b8b13301fcfd57ae05ea66e6e6 
  src/slave/containerizer/isolators/cgroups/mem.cpp 
711d66d7771cac13be831d73af3ef570d6785473 
  src/slave/containerizer/isolators/cgroups/perf_event.hpp 
e511c3efe4cc4ec13cc74bdcda993477a4db2e36 
  src/slave/containerizer/isolators/cgroups/perf_event.cpp 
6f67164d3963769148fb1749d1d590b7b2999fdb 
  src/slave/containerizer/isolators/filesystem/shared.hpp 
727f63e16fa69293c472a4b6b95fb0be04e689c8 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
5c347af803bf512698dd580d6533c0103b289427 
  src/slave/containerizer/isolators/namespaces/pid.hpp 
8da6cccff04edf11fa37f5a33648821299cf49d5 
  src/slave/containerizer/isolators/namespaces/pid.cpp 
fdd430e946c386e37ffc5b24907063180cddac17 
  src/slave/containerizer/isolators/network/port_mapping.hpp 
90d19a95092bc18c1077fbaec9ebf9c87291474c 
  src/slave/containerizer/isolators/network/port_mapping.cpp 
b484bbf34060ccc9debd48a3e840c03c8f95de09 
  src/slave/containerizer/isolators/posix.hpp 
7a667e3e30712b98484947520b58965d3b768659 
  src/slave/containerizer/isolators/posix/disk.hpp 
cf4e143eb50cf5ee59ca0d47dd6f8d0a845c3732 
  src/slave/containerizer/isolators/posix/disk.cpp 
bb1779fd644a33527068868a45cf9c19d7732727 
  src/slave/containerizer/launcher.hpp 18b3546a5e51e8a3819bca11630f699a0aadf762 
  src/slave/containerizer/launcher.cpp 92c0657728ddb9f6a4f9065ff8a7093c5371bc6f 
  src/slave/containerizer/linux_launcher.hpp 
3d9794dd4983ecf59833f3971cbb094d4bf7710f 
  src/slave/containerizer/linux_launcher.cpp 
10c12039cf684bef9398da72c3eceb9ed8b8b9c9 
  src/slave/containerizer/mesos/containerizer.hpp 
802988c90ac872b0cefa5e28f06e6fec98e8d032 
  src/slave/containerizer/mesos/containerizer.cpp 
d712278428889ebdfd598537690138329d8464f0 
  src/slave/flags.hpp a3c5c68a553b1c88ce6d5177e625079f7cdb2e5f 
  src/tests/isolator.hpp 2c3a694fade996df6fb679e78f96876a7d32d214 

Diff: https://reviews.apache.org/r/30338/diff/


Testing
---

make check.


Thanks,

Kapil Arya

Review Request 30463: Increased the default AWAIT timeout.

[Ben Mahler]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30463/
---

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Attempt #2 of: https://reviews.apache.org/r/30439/

I've observed a number of CI VM stalls that lead to test failures, this is an 
attempt to mitigate some of the flaky tests in the face of VM stalls.


Diffs
-

  3rdparty/libprocess/include/process/gtest.hpp 
10f991d4e6a6ec8479f342ee812faa3f5c3870bb 

Diff: https://reviews.apache.org/r/30463/diff/


Testing
---

make check


Thanks,

Ben Mahler

Re: Review Request 30459: Refactored task/offer/resource valiation in master.

[Dominic Hamon]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30459/#review70429
---



src/master/master.hpp


const Offer* ?



src/master/master.hpp


const Master*? or const Master& even.

or make const versions of getOffer and getSlave that are public in the 
master?



src/master/validation.hpp


these two helpers don't need to be in the header.


- Dominic Hamon


On Jan. 30, 2015, 2:27 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30459/
> ---
> 
> (Updated Jan. 30, 2015, 2:27 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone.
> 
> 
> Bugs: MESOS-2305
> https://issues.apache.org/jira/browse/MESOS-2305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored task/offer/resource valiation in master.
> 
> See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 07bea1fb8f0035413f2119859e16fa4f9383f68e 
>   src/master/master.hpp 337e00aa46ea127f3667e3383d631c3fb8e22f30 
>   src/master/master.cpp 10056861b95ed9453c971787982db7d09f09f323 
>   src/master/validation.hpp PRE-CREATION 
>   src/master/validation.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/30459/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 30459: Refactored task/offer/resource valiation in master.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30459/
---

Review request for mesos, Ben Mahler and Vinod Kone.


Bugs: MESOS-2305
https://issues.apache.org/jira/browse/MESOS-2305


Repository: mesos


Description
---

Refactored task/offer/resource valiation in master.

See ticket for motivation.


Diffs
-

  src/master/master.hpp 337e00aa46ea127f3667e3383d631c3fb8e22f30 
  src/master/master.cpp 10056861b95ed9453c971787982db7d09f09f323 

Diff: https://reviews.apache.org/r/30459/diff/


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 30459: Refactored task/offer/resource valiation in master.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30459/
---

(Updated Jan. 30, 2015, 7:49 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

OK, pull validation logics into a separate source file so that the diff is more 
readable.


Bugs: MESOS-2305
https://issues.apache.org/jira/browse/MESOS-2305


Repository: mesos


Description
---

Refactored task/offer/resource valiation in master.

See ticket for motivation.


Diffs (updated)
-

  src/Makefile.am 07bea1fb8f0035413f2119859e16fa4f9383f68e 
  src/master/master.hpp 337e00aa46ea127f3667e3383d631c3fb8e22f30 
  src/master/master.cpp 10056861b95ed9453c971787982db7d09f09f323 
  src/master/validation.hpp PRE-CREATION 
  src/master/validation.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/30459/diff/


Testing
---

make check


Thanks,

Jie Yu

Re: GPU computing resource add into Mesos

[Chester Kuo]

Hi dev,

I put some design/info on JIRA , please help to put comment if you interested.

https://issues.apache.org/jira/browse/MESOS-2262

This could be a two stage on this feature , first will focus on
resource discovery.

Chester

On Tue, Jan 27, 2015 at 10:25 PM, Tim St Clair  wrote:
> Just remember, GPU's are non-fungible and there can be many per blade.
>
> Also, many folks in the scientific community had found OpenCL limiting fwiw, 
> and found themselves eventually supporting CUDA, so the JIRA should probably 
> plan for either.
>
> I recommend crib-ing ideas from here (namely discovery):
> http://research.cs.wisc.edu/htcondor/HTCondorWeek2014/presentations/KnoellerJ-GPU.pptx
>
> Cheers,
> Tim
>
> - Original Message -
>> From: "Chester Kuo" 
>> To: dev@mesos.apache.org
>> Sent: Tuesday, January 27, 2015 3:18:21 AM
>> Subject: Re: GPU computing resource add into Mesos
>>
>> Ya, i'm working on it, will publish to JIRA once done.
>>
>> On Tue, Jan 27, 2015 at 12:16 AM, Christos Kozyrakis
>>  wrote:
>> > Chester, this sounds great. Do you want to start a design doc about
>> > extensions needed in slave/isolators/containerizer/... for GPUs. It would
>> > be useful to separate what is a minimum vs complete set of features to
>> > consider. The doc will be a good starting point for discussion.
>> >
>> > On Mon, Jan 26, 2015 at 1:18 AM, Chester Kuo  wrote:
>> >
>> >> Hi Tom
>> >>
>> >> Ya, the GPGPU resources needs to provided from slave , but we need to
>> >> extend to have it  to query GPGPU resources such as GPU devices
>> >> (single or multiple) ,CU(compute unit) , global/local memory embedded
>> >> in the slave node, with this info , framework can utilize it as we did
>> >> of generic CPU/Memory.
>> >>
>> >> Besides i'd like to have OpenCL (https://www.khronos.org/opencl/)  to
>> >> help to query slave's capability and its more generic and portable,
>> >> and i also plan to have other framework (such as Spark) have knowledge
>> >> about GPGPU resources for computing performance up (Planning).
>> >>
>> >>
>> >> Chester
>> >>
>> >>
>> >> On Mon, Jan 26, 2015 at 4:48 PM, Tom Arnfeld  wrote:
>> >> > Chester, you can specify arbitrary resources using the --resources flag
>> >> to the slave and Mesos will share out the resources to frameworks, and
>> >> then
>> >> your framework can do as it pleases.
>> >> >
>> >> >
>> >> > I'm not sure any changes are required in Mesos itself to support this,
>> >> unless I'm missing something.
>> >> >
>> >> >
>> >> > --
>> >> >
>> >> >
>> >> > Tom Arnfeld
>> >> >
>> >> > Developer // DueDil
>> >> >
>> >> >
>> >> >
>> >> >
>> >> >
>> >> > (+44) 7525940046
>> >> >
>> >> > 25 Christopher Street, London, EC2A 2BS
>> >> >
>> >> > On Mon, Jan 26, 2015 at 6:15 AM, Chester Kuo 
>> >> > wrote:
>> >> >
>> >> >> Hi All
>> >> >> I'd like to extend and add new feature into Mesos to support GPU
>> >> >> resource allocation, so we can put OpenCL application/framework on top
>> >> >> of Mesos and make it write once run across cluster.
>> >> >> Why choose OpenCL, due to it was widely supported by Intel , Nvidia,
>> >> >> AMD, Qualcomm GPGPU, so we may extended to have other framework (ex:
>> >> >> Spark) can try to utilize GPGPU computing resource.
>> >> >> Any Comments?
>> >> >> Chester
>> >>
>>
>
> --
> Cheers,
> Timothy St. Clair
> Red Hat Inc.

Re: Review Request 30428: Added hidden field to labels

[Kapil Arya]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30428/#review70336
---

Ship it!


Ship It!


src/tests/master_tests.cpp


Why not add one more label with `hidden` set to `false` to test the third 
case as well.



src/tests/master_tests.cpp


What if we parse the `labelsObject_` JSON::Array object directly in to 
`Labels` protobuf? Do we have a preference one way or the other? That way, we 
won't have to construct the expect json ourselves.



src/tests/slave_tests.cpp


Same as above -- add a third test with `hidden` set to false.


- Kapil Arya


On Jan. 29, 2015, 5:30 p.m., Niklas Nielsen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30428/
> ---
> 
> (Updated Jan. 29, 2015, 5:30 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Kapil Arya.
> 
> 
> Bugs: MESOS-2259
> https://issues.apache.org/jira/browse/MESOS-2259
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Task labels are used by label-decorator hooks to insert hook-specific labels. 
> Some hook implementation might not want to expose their label for security 
> reasons.
> We should allow certain labels to be marked as "hidden", such that they are 
> invisible from the "web ui" to start with.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 26003fada2e4c4be0e9ebc4663f7ebab858cc48d 
>   src/common/http.cpp 915cba10725685da55a53b249b8b676b5e229d86 
>   src/tests/master_tests.cpp 678d27f41a2f246c714c77adb132263c0c2c61ed 
>   src/tests/slave_tests.cpp e7e2af63da785644f3f7e6e23607c02be962a2c6 
> 
> Diff: https://reviews.apache.org/r/30428/diff/
> 
> 
> Testing
> ---
> 
> make check with two new tests
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>