Re: Review Request 50841: Added GPU scheduling logic to docker containerizer process.

[Yubo Li]

> On 十月 9, 2016, 5:16 a.m., Guangya Liu wrote:
> > src/slave/containerizer/docker.cpp, lines 2105-2109
> > 
> >
> > Not yours, but I think that the comments can be removed as we have the 
> > logic of killing those containers forcely in #2118 and #2123. You can fix 
> > this in a separate patch.

Will move to another patch


> On 十月 9, 2016, 5:16 a.m., Guangya Liu wrote:
> > src/slave/containerizer/docker.cpp, lines 2162-2182
> > 
> >
> > What about moving this to a new function named as `destroy` and use 
> > continuation for `deallocateNvidiaGpus` in #2159?

Fixed, thanks!


> On 十月 9, 2016, 5:16 a.m., Guangya Liu wrote:
> > src/slave/containerizer/docker.cpp, lines 1554-1564
> > 
> >
> > I think we do not need the update here as `destroy` will always be 
> > called even if the container exit normally, relasing the GPU in 
> > `___destroy` is good enough.

removed


- Yubo


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


On 九月 22, 2016, 6:16 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50841/
> ---
> 
> (Updated 九月 22, 2016, 6:16 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added control logic to allocate/deallocate GPUs to GPU-related task
> when the task is started/terminated.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp 1d27761fcb3f310cf954d45ed41f4c89ecbd5982 
> 
> Diff: https://reviews.apache.org/r/50841/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Review Request 52732: Made mesos-containerizer launch helper inherit agent env variables.

[Jie Yu]

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

Review request for mesos, Gilbert Song and Kevin Klues.


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


Repository: mesos


Description
---

This patch addressed MESOS-6323. The idea is that we pass the
environment variables that the command needs to the
mesos-containerizer launch helper, and let the helper inherit agent
environment variables.


Diffs
-

  src/slave/containerizer/mesos/containerizer.hpp 
eef2cbb033faf9428970369f0434d6653c5d3713 
  src/slave/containerizer/mesos/containerizer.cpp 
32058c35ea9ca95f0a2665994c1ebccd5c840345 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Review Request 52731: Added 'environment' flag to mesos-containerizer launch helper.

[Jie Yu]

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

Review request for mesos, Gilbert Song and Kevin Klues.


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


Repository: mesos


Description
---

The flag is optional. If set, the command to be launched will use the
specified environment variables. If not set, the command will inherit
the environment variables of the calling process.


Diffs
-

  src/slave/containerizer/mesos/launch.hpp 
208ad2ba105af1e5cdd4b9e3f9f216c0fe1b648d 
  src/slave/containerizer/mesos/launch.cpp 
c6b669a04c006edfc78c06560d1eb088278c2f8e 

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


Testing
---

make check


Thanks,

Jie Yu

Review Request 52729: Made execvp explicit in posix/shell.hpp.

[Jie Yu]

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

Review request for mesos, Gilbert Song and Kevin Klues.


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


Repository: mesos


Description
---

Made execvp explicit in posix/shell.hpp.


Diffs
-

  3rdparty/stout/include/stout/os/posix/shell.hpp 
e7047ac5827867c1f5285dcace1d93e8c07e08ae 

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


Testing
---

make check


Thanks,

Jie Yu

Review Request 52730: Added an abstraction for Envp pointer expected by exec routines.

[Jie Yu]

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

Review request for mesos, Gilbert Song and Kevin Klues.


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


Repository: mesos


Description
---

Added an abstraction for Envp pointer expected by exec routines.


Diffs
-

  3rdparty/stout/include/stout/os/raw/environment.hpp 
80cc45b24cfe6f6429d4d0ed4acdafba51c590eb 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 52092: Avoided to concat cgroup internally in subsystems.

[haosdent huang]

> On Oct. 10, 2016, 4:57 p.m., Jie Yu wrote:
> > Is it up-to-date? Can you do a base and I'll get this committed.

Rebase.


- haosdent


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


On Oct. 11, 2016, 3:53 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52092/
> ---
> 
> (Updated Oct. 11, 2016, 3:53 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Qian Zhang.
> 
> 
> Bugs: MESOS-6218
> https://issues.apache.org/jira/browse/MESOS-6218
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To avoid using `path::join(flags.cgroups_root, containerId.value())` to
> concat cgroup internally in subsystems, pass cgroup to the subsystem
> interfaces directly.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 9359ceff3f1f6204822b511b46d62d2fceec25e4 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 6594110bd45a71c6d41a24b3f5b73c4a4e3a7335 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> 05cb8b8efadd2355789d08ce5c9da9d3de0fc72a 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.hpp 
> c1f1004d7578f59dfa9e28323e7a55df41c4cb6b 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp 
> 5912c6504a375178b892bf8099d5f75fa9e91c05 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.hpp 
> 10fbbc9cb39a3b41d64cc090b0e890a2416b2306 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.cpp 
> a35ecb073acefb903d7e8c4737d6cd048a2b494d 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp 
> b5d5ed233c0c8ddc339d2550c0f57a02dd3f14c3 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp 
> 0afc24867eb0b1949e95b3f5a8914be013dbf531 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 
> a1c87ce2ace33f1a779e843578f55d7502562c8d 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 
> 5b3ce15c209f6ce0e430386d97bc6768fca805c8 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp 
> 16208655e16b81f1e4bbf97bf5e32e75f4c3f3a9 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.cpp 
> 93539f1fc8265f66b62294c22f4eaba704b8b876 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.hpp 
> 5a0adfdb8705ae6b20c61a827380142e418c0ae4 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 
> cc39f287a64a4125260597e74784dc0847953376 
> 
> Diff: https://reviews.apache.org/r/52092/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52092: Avoided to concat cgroup internally in subsystems.

[haosdent huang]

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

(Updated Oct. 11, 2016, 3:53 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Qian Zhang.


Changes
---

Rebase.


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


Repository: mesos


Description
---

To avoid using `path::join(flags.cgroups_root, containerId.value())` to
concat cgroup internally in subsystems, pass cgroup to the subsystem
interfaces directly.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
9359ceff3f1f6204822b511b46d62d2fceec25e4 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
6594110bd45a71c6d41a24b3f5b73c4a4e3a7335 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
05cb8b8efadd2355789d08ce5c9da9d3de0fc72a 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.hpp 
c1f1004d7578f59dfa9e28323e7a55df41c4cb6b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp 
5912c6504a375178b892bf8099d5f75fa9e91c05 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.hpp 
10fbbc9cb39a3b41d64cc090b0e890a2416b2306 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.cpp 
a35ecb073acefb903d7e8c4737d6cd048a2b494d 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp 
b5d5ed233c0c8ddc339d2550c0f57a02dd3f14c3 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp 
0afc24867eb0b1949e95b3f5a8914be013dbf531 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 
a1c87ce2ace33f1a779e843578f55d7502562c8d 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 
5b3ce15c209f6ce0e430386d97bc6768fca805c8 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp 
16208655e16b81f1e4bbf97bf5e32e75f4c3f3a9 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.cpp 
93539f1fc8265f66b62294c22f4eaba704b8b876 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.hpp 
5a0adfdb8705ae6b20c61a827380142e418c0ae4 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 
cc39f287a64a4125260597e74784dc0847953376 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52667: Added mesos test helper 'createCommandInfo()'.

[Anand Mazumdar]

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




src/tests/mesos.hpp (lines 449 - 461)


Why not enhance the existing `createCommandInfo()` helper method?


- Anand Mazumdar


On Oct. 11, 2016, 1:14 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52667/
> ---
> 
> (Updated Oct. 11, 2016, 1:14 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We keep 'shell' as a parameter with a default value as 'false'. It
> is redundant for now, but still keep it as we don't want to create
> a CommandInfo implicitly.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 5e15f5164cddebd60bb6d9856ae63ad357643cb2 
> 
> Diff: https://reviews.apache.org/r/52667/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 52716: Added mesos test helper 'createV1Task()'.

[Anand Mazumdar]

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




src/tests/mesos.hpp (lines 559 - 564)


hmm, similar to comment in my earlier review, it's weird that the first 
argument has a `v1` argument but others are `v0` based. Can we consistenly use 
`v1`?


- Anand Mazumdar


On Oct. 11, 2016, 1:15 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52716/
> ---
> 
> (Updated Oct. 11, 2016, 1:15 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added mesos test helper 'createV1Task()'.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 5e15f5164cddebd60bb6d9856ae63ad357643cb2 
> 
> Diff: https://reviews.apache.org/r/52716/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 52715: Added a test helper 'createV1ExecutorInfo()'.

[Anand Mazumdar]

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




src/tests/mesos.hpp (line 446)


It's a bit weird that we need to pass on v0 arguments to create a v1 
executor info. Can you pass in `v1::ExecutorInfo::Type` instead?


- Anand Mazumdar


On Oct. 11, 2016, 1:15 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52715/
> ---
> 
> (Updated Oct. 11, 2016, 1:15 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test helper 'createV1ExecutorInfo()'.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 5e15f5164cddebd60bb6d9856ae63ad357643cb2 
> 
> Diff: https://reviews.apache.org/r/52715/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 52718: Added v1 api helpers to create calls in 'tests/mesos.hpp'.

[Anand Mazumdar]

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



Thanks for introducing these test helpers Gilbert. These would be very useful 
for everyone.

Most of my comments are around whether we can add these as gtest custom actions 
instead. Though, we can argue for having both, I would prefer having them as 
actions instead since most use-cases would be around just invoking the default 
actions (looking at how they are being used today in the present code).


src/tests/mesos.hpp (lines 934 - 943)


I would prefer this to be an action to be consistent with the 
`LaunchTasks(...)` action we already have for the v0 API.

The action should be able to take an `Option`/`Option

I would prefer this to be an action. It would be then consistent with the 
`executor::SendSubscribe` action we already have.

The usage would be like this:

```cpp
EXPECT_CALL(*scheduler, connected(_))
 .WillOnce(scheduler::SendSubscribe(frameworkInfo);
```



src/tests/mesos.hpp (lines 957 - 971)


We won't need this if we add an action `LaunchTasks` as per my earlier 
comment.



src/tests/mesos.hpp (lines 974 - 990)


Most users won't _ever_ bother acknowledging their updates explicitly. In 
fact, we should think about even getting rid of the one liner they need to 
write. How about introducing a constructor argument to `TestMesos` to take 
`implicitAcknowledgements` as an argument? This would then be consistent with 
the old driver based interface.


- Anand Mazumdar


On Oct. 11, 2016, 1:15 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52718/
> ---
> 
> (Updated Oct. 11, 2016, 1:15 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added v1 api helpers to create calls in 'tests/mesos.hpp'.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 5e15f5164cddebd60bb6d9856ae63ad357643cb2 
> 
> Diff: https://reviews.apache.org/r/52718/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 52612: Added nested container tests for docker runtime isolator.

[Gilbert Song]

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

(Updated Oct. 10, 2016, 6:15 p.m.)


Review request for mesos, Anand Mazumdar, Benjamin Hindman, Artem Harutyunyan, 
Jie Yu, Timothy Chen, and Vinod Kone.


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


Repository: mesos


Description
---

Added nested container tests for docker runtime isolator.


Diffs (updated)
-

  src/tests/containerizer/runtime_isolator_tests.cpp 
c0c11607024b6a80d5bf5a486b91f7905a9083d7 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 52717: Added mesos test helper 'createV1ContainerInfo()'.

[Gilbert Song]

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

Review request for mesos, Anand Mazumdar, Jie Yu, and Vinod Kone.


Repository: mesos


Description
---

Added mesos test helper 'createV1ContainerInfo()'.


Diffs
-

  src/tests/mesos.hpp 5e15f5164cddebd60bb6d9856ae63ad357643cb2 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 52715: Added a test helper 'createV1ExecutorInfo()'.

[Gilbert Song]

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

Review request for mesos, Anand Mazumdar, Jie Yu, and Vinod Kone.


Repository: mesos


Description
---

Added a test helper 'createV1ExecutorInfo()'.


Diffs
-

  src/tests/mesos.hpp 5e15f5164cddebd60bb6d9856ae63ad357643cb2 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 52718: Added v1 api helpers to create calls in 'tests/mesos.hpp'.

[Gilbert Song]

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

Review request for mesos, Anand Mazumdar, Jie Yu, and Vinod Kone.


Repository: mesos


Description
---

Added v1 api helpers to create calls in 'tests/mesos.hpp'.


Diffs
-

  src/tests/mesos.hpp 5e15f5164cddebd60bb6d9856ae63ad357643cb2 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 52714: Added optional 'Type' to createExecutorInfo().

[Gilbert Song]

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

Review request for mesos, Anand Mazumdar, Jie Yu, and Vinod Kone.


Repository: mesos


Description
---

This patch made 2 changes to the helper because of the changes from
supporting nested containers:
1. Added an optional 'Type' in ExecutorInfo.
2. Make the parameter 'command' as optional since the CommandInfo
   became optional in 'ExecutorInfo' protobuf.


Diffs
-

  src/tests/mesos.hpp 5e15f5164cddebd60bb6d9856ae63ad357643cb2 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 52716: Added mesos test helper 'createV1Task()'.

[Gilbert Song]

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

Review request for mesos, Anand Mazumdar, Jie Yu, and Vinod Kone.


Repository: mesos


Description
---

Added mesos test helper 'createV1Task()'.


Diffs
-

  src/tests/mesos.hpp 5e15f5164cddebd60bb6d9856ae63ad357643cb2 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 52668: Refactored nested container tests launcher orphans.

[Gilbert Song]

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

(Updated Oct. 10, 2016, 6:14 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Repository: mesos


Description
---

This patch changed the way we simulate orphans in nested container
tests, by removing the container's runtime directory, so that we
no longer rely on creating freezer cgroup which is specific to
linux launcher.

Some cleanup and nits fixed are included in this patch as well.


Diffs (updated)
-

  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
9b278e546dacc14b10dac51ee50e8fb89df86b87 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 52713: Added evolve helper for ContainerInfo.

[Gilbert Song]

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

Review request for mesos, Anand Mazumdar, Jie Yu, and Vinod Kone.


Repository: mesos


Description
---

Added evolve helper for ContainerInfo.


Diffs
-

  src/internal/evolve.hpp a38842a726f0e2634ae74b91f83b15ab1e656a47 
  src/internal/evolve.cpp b4a95771974ef11fda896d04a700c3d3d4fa024c 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 52667: Added mesos test helper 'createCommandInfo()'.

[Gilbert Song]

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

(Updated Oct. 10, 2016, 6:14 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Repository: mesos


Description
---

We keep 'shell' as a parameter with a default value as 'false'. It
is redundant for now, but still keep it as we don't want to create
a CommandInfo implicitly.


Diffs (updated)
-

  src/tests/mesos.hpp 5e15f5164cddebd60bb6d9856ae63ad357643cb2 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 52723: Changed agent to report TASK_DROPPED during reconciliation.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

If a framework attempts to launch a task but the launch message is
dropped after it reaches the master but before it reaches the slave, the
failed launch will be detected during master <-> agent reconciliation
when the agent re-registers. Previously, the agent would generate a
TASK_LOST status update for such dropped tasks; now it will generate
TASK_DROPPED if the framework is partition-aware.


Diffs
-

  src/slave/slave.cpp 119fb36c27b25739f2a86a55d48e964ca4a84ff7 
  src/tests/master_slave_reconciliation_tests.cpp 
2983c1b074c2d4179e95e619083f5dd4e9ac6730 
  src/tests/slave_recovery_tests.cpp 703948f7a6861a4401ee45ce9cae2644106083f3 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 52721: Fixed typo in log message.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Fixed typo in log message.


Diffs
-

  src/master/master.cpp ad8993ae43e2508a3777f4062acaca1177ac77f2 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 52720: Clarified a comment.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Clarified a comment.


Diffs
-

  src/master/master.cpp ad8993ae43e2508a3777f4062acaca1177ac77f2 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 52719: Renamed a function for clarity.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

`Master::reconcile(Framework*, const scheduler::Call::Reconcile&)` and
`Master::reconcile(Slave*, const vector&, const
vector& tasks)` are only loosely related. Per discussion on the
development list, using overloading to distinguish these two functions
is confusing. Hence, rename the latter to `reconcileKnownTasks`.


Diffs
-

  src/master/master.hpp 43518b9bf1bfaa54e26acc7f2e70c4161c667a84 
  src/master/master.cpp ad8993ae43e2508a3777f4062acaca1177ac77f2 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 52311: Pass the user value from executor of switch_user flag is set.

[Joseph Wu]

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




src/slave/slave.cpp (line 4197)


Period at the end of all comments.  The cpp linter should be catching this 
for you (a pre-commit git hook), so make sure you've run `./bootstrap` in your 
development directory.



src/slave/slave.cpp (lines 4209 - 4210)


This custom executor user should only be set if it isn't already set.
Also, you need to guard the `loggerUser.get()` with a `loggerUser.isSome()`.



src/slave/slave.cpp (line 4221)


Same here, you need to guard the `.get()` with a `.isSome()`.



src/slave/slave.cpp (lines 4358 - 4365)


It is now possible replace this block with:
```
executor.mutable_command()->add_arguments("--user=" + loggerUser.get());
```

Considering the variable's name and how it is used in this function, you 
should consider renaming it `executorUser` and changing the type from 
`Option` to `string`.  Then, each place it needs to be set, you check 
`if (flags.switch_user)`.
This is clearer than guarding it with `if (executorUser.isSome)`.


- Joseph Wu


On Oct. 5, 2016, 8:30 p.m., Sivaram Kannan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52311/
> ---
> 
> (Updated Oct. 5, 2016, 8:30 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5856
> https://issues.apache.org/jira/browse/MESOS-5856
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Pass the user value from executor of switch_user flag is set.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp d30001bc0d1798311ba3966f67aadc6be2c92306 
> 
> Diff: https://reviews.apache.org/r/52311/diff/
> 
> 
> Testing
> ---
> 
> 1. Run the mesos-logrotate-logger with un-priviledged user and verify whether 
> the logs are getting rotated. 
> 2. Run the mesos-logrotate-logger as root user and verify whether the logs 
> are getting rotated.
> 
> 
> Thanks,
> 
> Sivaram Kannan
> 
>

Re: Review Request 52309: Pass the user variable from library to binary.

[Joseph Wu]

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


Fix it, then Ship it!




Once these last few things are tweaked, this commit should be good-to-go.  I'll 
commit it with the rest of the chain, when the whole chain is ready.


src/slave/container_loggers/lib_logrotate.cpp (lines 143 - 144)


s/executorInfo/`ExecutorInfo`/
s/loggerUser/the `loggerUser`/

And period at the end.



src/slave/container_loggers/lib_logrotate.cpp (line 145)


Unless we've accidentally included the `std` namespace somewhere, this 
shouldn't compile unless it's `std::string`.


- Joseph Wu


On Oct. 5, 2016, 8:07 p.m., Sivaram Kannan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52309/
> ---
> 
> (Updated Oct. 5, 2016, 8:07 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5856
> https://issues.apache.org/jira/browse/MESOS-5856
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Pass the user variable from library to binary.
> 
> 
> Diffs
> -
> 
>   src/slave/container_loggers/lib_logrotate.cpp 
> 0ca2b3d7dbb57c11c0740aed3914a6b75329af99 
> 
> Diff: https://reviews.apache.org/r/52309/diff/
> 
> 
> Testing
> ---
> 
> 1. Run the mesos-logrotate-logger with un-priviledged user and verify whether 
> the logs are getting rotated. 
> 2. Run the mesos-logrotate-logger as root user and verify whether the logs 
> are getting rotated.
> 
> 
> Thanks,
> 
> Sivaram Kannan
> 
>

Re: Review Request 52310: Switch the uid of the binary if a user is passed from the lib_logrotate.

[Joseph Wu]

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


Fix it, then Ship it!




Once this last comment is tweaked, this commit should be good-to-go.  I'll 
commit it with the rest of the chain, when the whole chain is ready.


src/slave/container_loggers/logrotate.cpp (lines 246 - 247)


Suggestion:
```
If the `--user` flag is set, change the UID of this process to that user.
```

Note: This binary is built as the "mesos-logrotate-logger", rather than the 
"mesos-logger-logrotate" :)


- Joseph Wu


On Oct. 5, 2016, 8:07 p.m., Sivaram Kannan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52310/
> ---
> 
> (Updated Oct. 5, 2016, 8:07 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5856
> https://issues.apache.org/jira/browse/MESOS-5856
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Switch the uid of the binary if a user is passed from the lib_logrotate.
> 
> 
> Diffs
> -
> 
>   src/slave/container_loggers/logrotate.cpp 
> 431bc3cbb54e94359078e4dae0b32ad301393640 
> 
> Diff: https://reviews.apache.org/r/52310/diff/
> 
> 
> Testing
> ---
> 
> 1. Run the mesos-logrotate-logger with un-priviledged user and verify whether 
> the logs are getting rotated. 
> 2. Run the mesos-logrotate-logger as root user and verify whether the logs 
> are getting rotated.
> 
> 
> Thanks,
> 
> Sivaram Kannan
> 
>

Re: Review Request 52680: Used full docker image name to force pull a iamge.

[Timothy Chen]

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


Ship it!




This looks right to me, I'll Jie comment and merge though.

- Timothy Chen


On Oct. 10, 2016, 10:06 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52680/
> ---
> 
> (Updated Oct. 10, 2016, 10:06 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When force pull docker image, we should use the full docker image
> name to avoid pulling down the repository.
> 
> Seems the latest docker client will add `latest` automatically
> for now but I think that we still need to keep the logic in case
> someone using old docker client.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 50fda393a42afefc70790a26b44911e4cf17185e 
> 
> Diff: https://reviews.apache.org/r/52680/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 52680: Used full docker image name to force pull a iamge.

[Guangya Liu]

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

Review request for mesos, Jie Yu and Timothy Chen.


Repository: mesos


Description
---

When force pull docker image, we should use the full docker image
name to avoid pulling down the repository.

Seems the latest docker client will add `latest` automatically
for now but I think that we still need to keep the logic in case
someone using old docker client.


Diffs
-

  src/docker/docker.cpp 50fda393a42afefc70790a26b44911e4cf17185e 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Review Request 52707: Updated CLI bootstrap to search for local virtualenv installations.

[Joseph Wu]

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

Review request for mesos, Artem Harutyunyan, Kevin Klues, and Vinod Kone.


Repository: mesos


Description
---

A locally installed virtualenv does not always show up on the `PATH`,
which fails `which virtualenv`.  This adds an extra search location
based on the user's site package install directory.


Diffs
-

  src/cli_new/bootstrap 74a8568daef4e0cb93911352cc73c165a2fbea1d 

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


Testing
---

pip uninstall virtualenv -y  # Global
pip uninstall virtualenv -y  # Local

src/cli_new/bootstrap# Fails

pip install --user virtualenv

src/cli_new/bootstrap# Succeeds

pip uninstall virtualenv -y  # Local
pip install virtualenv   # Global

src/cli_new/bootstrap# Succeeds


Thanks,

Joseph Wu

Re: Review Request 50674: Stout: Enabled tests that pass on Windows.

[Alex Clemmer]

> On Aug. 2, 2016, 11:15 p.m., Joseph Wu wrote:
> > 3rdparty/stout/tests/flags_tests.cpp, lines 231-235
> > 
> >
> > This test will build on Windows, right?
> > 
> > If so, you should prepend `DISABLED_` instead of `#ifdef`-ing it.  Same 
> > with most of these tests, I believe.
> 
> Alex Clemmer wrote:
> I believe if we mark it `DISABLED_` it is disabled for all platforms. 
> Let's add a flag filter: `WINDOWS_DISABLED_`.
> 
> Alex Clemmer wrote:
> Oh. This won't work. `DISABLED_` is a gtest feature, not a Mesos feature, 
> so we have to mark any disabled tests as `DISABLED_`. Let's make a new 
> prefix, `DISABLED_WINDOWS_`, and adjust the test harness to strip this prefix 
> when the correct gtest filter is set.

Actually, that seems like it might not be the direction we want to go either. 
It seems that Stout and Libprocess do not have the same filtering facilities 
that the Agent and Master tests do, so I think we'd have to refactor that code 
to be in a common place, and then add code that will specifically opt 
non-Windows platforms in to tests marked `DISABLED_WINDOWS_`. (Or, 
alternatively, to make Windows platforms opt out of `WINDOWS_DISABLED_`.)

Joseph, what are your thoughts here? How important is it to you to not have 
`#ifdef`s in this code?


- Alex


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


On Aug. 1, 2016, 9:27 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50674/
> ---
> 
> (Updated Aug. 1, 2016, 9:27 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A large number of Stout test files are currently not being built on
> Windows.  Many of these files contain tests for parts of Stout that have
> already been ported, or require only trivial fixes to work (such as
> removing `#include`s on Windows). A small minority of the tests contain
> bugs that we should fix.
> 
> This commit will add these files to the build, fix some of the
> trivially-fixable tests, and disable tests that are known to fail
> because of bugs, including comments explaining why and links to JIRA
> issues where appropriate.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/mac.hpp 
> 91c4fdad350459b3e0bdf1744089e14ac883829a 
>   3rdparty/stout/include/stout/os/windows/kill.hpp 
> 0ddb3eeae3377688ad298a45c849c829c218890f 
>   3rdparty/stout/tests/CMakeLists.txt 
> f85dc998e8ba5f8d727933373c3e14500d0bdfae 
>   3rdparty/stout/tests/flags_tests.cpp 
> 77f3a6af110da1ffcdf2b7ab2b66431a6b5c91d3 
>   3rdparty/stout/tests/ip_tests.cpp 4d1f1c9a4dd35da2a21bef1349a662af44bb4ba1 
>   3rdparty/stout/tests/mac_tests.cpp 1ff60cf5506c1855547400aa20107ec5086d8431 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 
> ffe234baac305e26b5a29cffcdd310350d10167e 
>   3rdparty/stout/tests/os_tests.cpp e67444077eae55fd25945b451164b5bcc37552b0 
> 
> Diff: https://reviews.apache.org/r/50674/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 50674: Stout: Enabled tests that pass on Windows.

[Alex Clemmer]

> On Aug. 2, 2016, 11:15 p.m., Joseph Wu wrote:
> > 3rdparty/stout/tests/flags_tests.cpp, lines 231-235
> > 
> >
> > This test will build on Windows, right?
> > 
> > If so, you should prepend `DISABLED_` instead of `#ifdef`-ing it.  Same 
> > with most of these tests, I believe.
> 
> Alex Clemmer wrote:
> I believe if we mark it `DISABLED_` it is disabled for all platforms. 
> Let's add a flag filter: `WINDOWS_DISABLED_`.

Oh. This won't work. `DISABLED_` is a gtest feature, not a Mesos feature, so we 
have to mark any disabled tests as `DISABLED_`. Let's make a new prefix, 
`DISABLED_WINDOWS_`, and adjust the test harness to strip this prefix when the 
correct gtest filter is set.


- Alex


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


On Aug. 1, 2016, 9:27 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50674/
> ---
> 
> (Updated Aug. 1, 2016, 9:27 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A large number of Stout test files are currently not being built on
> Windows.  Many of these files contain tests for parts of Stout that have
> already been ported, or require only trivial fixes to work (such as
> removing `#include`s on Windows). A small minority of the tests contain
> bugs that we should fix.
> 
> This commit will add these files to the build, fix some of the
> trivially-fixable tests, and disable tests that are known to fail
> because of bugs, including comments explaining why and links to JIRA
> issues where appropriate.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/mac.hpp 
> 91c4fdad350459b3e0bdf1744089e14ac883829a 
>   3rdparty/stout/include/stout/os/windows/kill.hpp 
> 0ddb3eeae3377688ad298a45c849c829c218890f 
>   3rdparty/stout/tests/CMakeLists.txt 
> f85dc998e8ba5f8d727933373c3e14500d0bdfae 
>   3rdparty/stout/tests/flags_tests.cpp 
> 77f3a6af110da1ffcdf2b7ab2b66431a6b5c91d3 
>   3rdparty/stout/tests/ip_tests.cpp 4d1f1c9a4dd35da2a21bef1349a662af44bb4ba1 
>   3rdparty/stout/tests/mac_tests.cpp 1ff60cf5506c1855547400aa20107ec5086d8431 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 
> ffe234baac305e26b5a29cffcdd310350d10167e 
>   3rdparty/stout/tests/os_tests.cpp e67444077eae55fd25945b451164b5bcc37552b0 
> 
> Diff: https://reviews.apache.org/r/50674/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Review Request 52706: Updated mesos containerizer to use new 'stout/wait.hpp' header.

[Kevin Klues]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Updated mesos containerizer to use new 'stout/wait.hpp' header.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
e6bd9f7a8284d220be157a3db2da094e6b1b6d33 

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


Testing
---

sudo GTEST_FILTER="*Nested*" src/mesos-tests


Thanks,

Kevin Klues

Review Request 52705: Added 'stout/wait.hpp' header.

[Kevin Klues]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

This was motivated by the need for a default definition of
'W_EXITCODE' (since it is not technically POSIX compliant).


Diffs
-

  3rdparty/stout/include/stout/wait.hpp PRE-CREATION 

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


Testing
---

sudo GTEST_FILTER="*Nested*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 50674: Stout: Enabled tests that pass on Windows.

[Alex Clemmer]

> On Aug. 2, 2016, 11:15 p.m., Joseph Wu wrote:
> > 3rdparty/stout/tests/flags_tests.cpp, lines 231-235
> > 
> >
> > This test will build on Windows, right?
> > 
> > If so, you should prepend `DISABLED_` instead of `#ifdef`-ing it.  Same 
> > with most of these tests, I believe.

I believe if we mark it `DISABLED_` it is disabled for all platforms. Let's add 
a flag filter: `WINDOWS_DISABLED_`.


- Alex


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


On Aug. 1, 2016, 9:27 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50674/
> ---
> 
> (Updated Aug. 1, 2016, 9:27 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A large number of Stout test files are currently not being built on
> Windows.  Many of these files contain tests for parts of Stout that have
> already been ported, or require only trivial fixes to work (such as
> removing `#include`s on Windows). A small minority of the tests contain
> bugs that we should fix.
> 
> This commit will add these files to the build, fix some of the
> trivially-fixable tests, and disable tests that are known to fail
> because of bugs, including comments explaining why and links to JIRA
> issues where appropriate.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/mac.hpp 
> 91c4fdad350459b3e0bdf1744089e14ac883829a 
>   3rdparty/stout/include/stout/os/windows/kill.hpp 
> 0ddb3eeae3377688ad298a45c849c829c218890f 
>   3rdparty/stout/tests/CMakeLists.txt 
> f85dc998e8ba5f8d727933373c3e14500d0bdfae 
>   3rdparty/stout/tests/flags_tests.cpp 
> 77f3a6af110da1ffcdf2b7ab2b66431a6b5c91d3 
>   3rdparty/stout/tests/ip_tests.cpp 4d1f1c9a4dd35da2a21bef1349a662af44bb4ba1 
>   3rdparty/stout/tests/mac_tests.cpp 1ff60cf5506c1855547400aa20107ec5086d8431 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 
> ffe234baac305e26b5a29cffcdd310350d10167e 
>   3rdparty/stout/tests/os_tests.cpp e67444077eae55fd25945b451164b5bcc37552b0 
> 
> Diff: https://reviews.apache.org/r/50674/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 52596: Added special case when sorting hierarchically in MountInfoTable::read.

[Kevin Klues]

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

(Updated Oct. 10, 2016, 8:50 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Added blocker on upcoming commit.


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


Repository: mesos


Description
---

It is legal to have entries in a `MountInfoTable` whose `entry.id` is
the same as `entry.parent`. This can happen (for example), if a system
boots from the network and then keeps the original `/` in RAM.
However, to avoid cycles when walking the mount hierarchy, we should
not treat these entries as children of their parent so we skip them.

This commit adds functionality to handle this case.


Diffs (updated)
-

  src/linux/fs.cpp 4b10141a49dfb3c6defdb07e295eb14cfcdd36ce 

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


Testing
---

GTEST_FILTER="" make -j40 check
GTEST_FILTER="FsTest.MountInfoTableReadSorted" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 50737: Parametrized libprocess HTTPTests by SSL configuration.

[Greg Mann]

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

(Updated Oct. 10, 2016, 8:50 p.m.)


Review request for mesos, Anand Mazumdar, Joseph Wu, and Vinod Kone.


Changes
---

Addressed Joseph's comment.


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


Repository: mesos


Description
---

This patch parametrizes the HTTP tests in libprocess so
that they run with SSL both enabled and disabled when
the library was compiled with SSL support.


Diffs (updated)
-

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

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


Testing
---

Tested on OSX with `GTEST_REPEAT=100 GTEST_BREAK_ON_FAILURE=1 
GTEST_FILTER="*HTTPTest*" 3rdparty/libprocess/libprocess-tests`


Thanks,

Greg Mann

Review Request 52703: Added test to test corner case with sorted 'MountInfoTable::read()'.

[Kevin Klues]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

We allow entries in the MountInfoTable to be parent's of themselves.
This test makes sure that this functionality is exercised.


Diffs
-

  src/tests/containerizer/fs_tests.cpp 0dd212f94d2845494163a86bc2059f999e018cd0 

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


Testing
---

GTEST_FILTER="*Sorted*" src/mesos-tests


Thanks,

Kevin Klues

Review Request 52704: Refactored 'MountInfoTable::read()' into two separate functions.

[Kevin Klues]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

The original function now calls a helper which takes a string
representation of a 'MountInfoTable'. In a subsequent commit we will
use this helper to write more meaningful tests to stress the core
logic of the 'MountInfoTable::read()' functionality.


Diffs
-

  src/linux/fs.hpp 090c82616d965b1cedf9bb13ffcde2d37304a1f7 
  src/linux/fs.cpp 4b10141a49dfb3c6defdb07e295eb14cfcdd36ce 

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


Testing
---

GTEST_FILTER="*Sorted*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 52597: Added more detailed error message when failing in MountInfoTable::read.

[Kevin Klues]

> On Oct. 7, 2016, 5:03 p.m., Jie Yu wrote:
> > src/linux/fs.cpp, lines 141-143
> > 
> >
> > Does this compile?
> 
> Kevin Klues wrote:
> Interestingly yes...
> 
> Though I see why you think it shouldn't (I would think it shouldn't 
> either). My guess is that  the `CHECK()` macro ends with a type that allows 
> adding a quoted string to concatenate to it, without the need for `<<`.
> 
> Either way, do you want me to update it to:
> 
> ```
>   CHECK(!visitedParents.contains(parentId)) <<
> "Cycle found in mount table hierarchy at entry"
> " '" << stringify(parentId) << "': " << std::endl << lines.get();
> ```

I've fixed this as part of a refactor in a subsequent commit.


- Kevin


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


On Oct. 6, 2016, 7:31 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52597/
> ---
> 
> (Updated Oct. 6, 2016, 7:31 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6118
> https://issues.apache.org/jira/browse/MESOS-6118
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added more detailed error message when failing in MountInfoTable::read.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.cpp 4b10141a49dfb3c6defdb07e295eb14cfcdd36ce 
> 
> Diff: https://reviews.apache.org/r/52597/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 52696: Harden stout

[Aaron Wood]

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

(Updated Oct. 10, 2016, 7:53 p.m.)


Review request for mesos and Michael Park.


Changes
---

Fix spelling and clarify comment about `-Wall`.


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


Repository: mesos


Description
---

Use a default set of flags to provide additional security and hardening to 
stout. Additionally, check and catch more warnings/errors.


Diffs (updated)
-

  3rdparty/stout/Makefile.am fda069d 

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


Testing
---

Compared the benchmarks with and without the flags being used. Also did a 
comparsion with the flags being used with and without optimizations and without 
the flags being used with and without optimizations. Overall the performance 
hit was very small with a 3-8% overhead (optimizations brings this down 
slightly). Most benchmarks were about 5% (or less) slower.


Thanks,

Aaron Wood

Re: Review Request 52695: Harden libprocess

[Aaron Wood]

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

(Updated Oct. 10, 2016, 7:52 p.m.)


Review request for mesos and Michael Park.


Changes
---

Fix spelling and clarify comment about `-Wall`.


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


Repository: mesos


Description
---

Use a default set of flags to provide additional security and hardening to 
libprocess. Additionally, check and catch more warnings/errors.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 020b0e1 

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


Testing
---

Compared the benchmarks with and without the flags being used. Also did a 
comparsion with the flags being used with and without optimizations and without 
the flags being used with and without optimizations. Overall the performance 
hit was very small with a 3-8% overhead (optimizations brings this down 
slightly). Most benchmarks were about 5% (or less) slower.


Thanks,

Aaron Wood

Re: Review Request 52645: Harden Mesos

[Aaron Wood]

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

(Updated Oct. 10, 2016, 7:50 p.m.)


Review request for mesos and Michael Park.


Changes
---

Fix spelling and clarify comment about `-Wall`.


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


Repository: mesos


Description
---

Use a default set of flags to provide additional security and hardening to 
Mesos. Additionally, check and catch more warnings/errors.


Diffs (updated)
-

  configure.ac 034bb91 
  src/Makefile.am fd01e1d 

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


Testing
---

Compared the benchmarks with and without the flags being used. Also did a 
comparsion with the flags being used with and without optimizations and without 
the flags being used with and without optimizations. Overall the performance 
hit was very small with a 3-8% overhead (optimizations brings this down 
slightly). Most benchmarks were about 5% (or less) slower.


Thanks,

Aaron Wood

Re: Review Request 40512: Libprocess Reinit: Add a test-only method to reinitialize libprocess.

[Vinod Kone]

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


Fix it, then Ship it!





3rdparty/libprocess/src/process.cpp (line 862)


new line.


- Vinod Kone


On July 30, 2016, midnight, Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40512/
> ---
> 
> (Updated July 30, 2016, midnight)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3820
> https://issues.apache.org/jira/browse/MESOS-3820
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This builds upon earlier changes to complete `process::finalize`.  
> Tests which need to reconfigure libprocess, such as some SSL-related 
> tests, can use `process::reinitialize` to do so.  
> 
> This method may also be useful for providing additional isolation 
> between tests, such as cleaning up all processes after each test case.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> a73313b3221d6c80b35f21c00f35d9f9c795f1ec 
>   3rdparty/libprocess/src/process.cpp 
> 7f331b812de2f0437838f48e0959441c8e04c358 
> 
> Diff: https://reviews.apache.org/r/40512/diff/
> 
> 
> Testing
> ---
> 
> TODO: Define `process::reinitialize` in several tests, such as `HTTPTest`, 
> `MetricsTest`, and `ReapTest` and call `process::reinitialize` before and 
> after tests.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 52620: Updated InverseOffers* maintenance tests to use the new scheduler mock.

[Anand Mazumdar]

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


Ship it!




LGTM

- Anand Mazumdar


On Oct. 10, 2016, 7:18 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52620/
> ---
> 
> (Updated Oct. 10, 2016, 7:18 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Bugs: MESOS-4948
> https://issues.apache.org/jira/browse/MESOS-4948
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated InverseOffers* maintenance tests to use the new scheduler mock.
> 
> 
> Diffs
> -
> 
>   src/tests/master_maintenance_tests.cpp 
> 77eb405ab7314da906bed9ec1d0018c24928d8d8 
> 
> Diff: https://reviews.apache.org/r/52620/diff/
> 
> 
> Testing
> ---
> 
> Tested on OS X 10.11 and Ubuntu 14.04.
> ```
> make check GTEST_FILTER='MasterMaintenanceTest.*' GTEST_REPEAT=1000 
> GTEST_BREAK_ON_FAILURE=1
> ```
> Noticed a segfault caused by getenv / setenv race (MESOS-3475), but it's 
> unrelated to this change and is reproducible on current master.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 40268: Libprocess Reinit: Change Socket::DEFAULT_KIND to a non-static value.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On July 29, 2016, 11:59 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40268/
> ---
> 
> (Updated July 29, 2016, 11:59 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3820
> https://issues.apache.org/jira/browse/MESOS-3820
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is needed for tests that utilize the test-only 
> `process::reinitialize` function.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/socket.hpp 
> 881b44b987e5894cac838dae046ab7dbad20b000 
>   3rdparty/libprocess/src/socket.cpp 668f26d85cf3097ff9b90a10be14c5833fafd558 
> 
> Diff: https://reviews.apache.org/r/40268/diff/
> 
> 
> Testing
> ---
> 
> Some test cleanup issues were uncovered by this change; fixed here: 
> https://reviews.apache.org/r/40453/
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40413: Libprocess Reinit: Move ReaperProcess instantiation into process.cpp.

[Vinod Kone]

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


Fix it, then Ship it!





3rdparty/libprocess/include/process/reap.hpp (line 54)


s/reaper_singleton/reaper/



3rdparty/libprocess/include/process/reap.hpp (line 57)


new line.



3rdparty/libprocess/src/reap.cpp (line 150)


new line.



3rdparty/libprocess/src/reap.cpp (line 154)


you do initilialize here but not in metrics process?


- Vinod Kone


On Aug. 10, 2016, 8:17 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40413/
> ---
> 
> (Updated Aug. 10, 2016, 8:17 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3934
> https://issues.apache.org/jira/browse/MESOS-3934
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The reaper singleton must be unified with `process::initialize` so 
> that it also falls under the scope of reinitialization.  The singleton 
> must also not be guarded by `Once`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/reap.hpp 
> d7e0fa381df63a9ca7d438d5cea0631e1b0ec7ee 
>   3rdparty/libprocess/src/process.cpp 
> 629f1644bc0a263972ec9efc41890c33f9406a34 
>   3rdparty/libprocess/src/reap.cpp 5fc2a4d67a3a6fe56005fc2c2d16d4983d53b83a 
> 
> Diff: https://reviews.apache.org/r/40413/diff/
> 
> 
> Testing
> ---
> 
> Tests done in a subsequent review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40410: Libprocess Reinit: Move MetricsProcess instantiation into process.cpp.

[Vinod Kone]

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




3rdparty/libprocess/include/process/metrics/metrics.hpp (lines 92 - 119)


don't you need to do process::initialize() in these functions?


- Vinod Kone


On July 29, 2016, 11:56 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40410/
> ---
> 
> (Updated July 29, 2016, 11:56 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3934
> https://issues.apache.org/jira/browse/MESOS-3934
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The metrics singleton must be unified with `process::initialize` so 
> that it also falls under the scope of reinitialization.  The singleton 
> must also not be guarded by `Once`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 54487ab2614c7f8e8df10d1e0c39880a6cf5bde3 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> ac1544e70f5884f946ce3c31c5430c6a2c1f9dd1 
>   3rdparty/libprocess/src/process.cpp 
> 7f331b812de2f0437838f48e0959441c8e04c358 
>   3rdparty/libprocess/src/tests/system_tests.cpp 
> 0f4d0424689522337806ba2227ec4330c700e17e 
> 
> Diff: https://reviews.apache.org/r/40410/diff/
> 
> 
> Testing
> ---
> 
> Tests done in a subsequent review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40410: Libprocess Reinit: Move MetricsProcess instantiation into process.cpp.

[Joseph Wu]

> On Oct. 10, 2016, 12:22 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/metrics/metrics.cpp, line 106
> > 
> >
> > so MetricsProcess was not managed by gc before? was that a bug?

It was never cleaned up and re-created before, so it didn't need to be managed.
Same with the reaper process, which you'll see a bit later in the review chain.


- Joseph


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


On July 29, 2016, 4:56 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40410/
> ---
> 
> (Updated July 29, 2016, 4:56 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3934
> https://issues.apache.org/jira/browse/MESOS-3934
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The metrics singleton must be unified with `process::initialize` so 
> that it also falls under the scope of reinitialization.  The singleton 
> must also not be guarded by `Once`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 54487ab2614c7f8e8df10d1e0c39880a6cf5bde3 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> ac1544e70f5884f946ce3c31c5430c6a2c1f9dd1 
>   3rdparty/libprocess/src/process.cpp 
> 7f331b812de2f0437838f48e0959441c8e04c358 
>   3rdparty/libprocess/src/tests/system_tests.cpp 
> 0f4d0424689522337806ba2227ec4330c700e17e 
> 
> Diff: https://reviews.apache.org/r/40410/diff/
> 
> 
> Testing
> ---
> 
> Tests done in a subsequent review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40410: Libprocess Reinit: Move MetricsProcess instantiation into process.cpp.

[Vinod Kone]

> On Oct. 10, 2016, 7:22 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/include/process/metrics/metrics.hpp, line 95
> > 
> >
> > we don't name other global processes singletons, so i would just call 
> > this "metrics_process" instead.

or maybe just "metrics" like we do with "gc"


- Vinod


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


On July 29, 2016, 11:56 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40410/
> ---
> 
> (Updated July 29, 2016, 11:56 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3934
> https://issues.apache.org/jira/browse/MESOS-3934
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The metrics singleton must be unified with `process::initialize` so 
> that it also falls under the scope of reinitialization.  The singleton 
> must also not be guarded by `Once`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 54487ab2614c7f8e8df10d1e0c39880a6cf5bde3 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> ac1544e70f5884f946ce3c31c5430c6a2c1f9dd1 
>   3rdparty/libprocess/src/process.cpp 
> 7f331b812de2f0437838f48e0959441c8e04c358 
>   3rdparty/libprocess/src/tests/system_tests.cpp 
> 0f4d0424689522337806ba2227ec4330c700e17e 
> 
> Diff: https://reviews.apache.org/r/40410/diff/
> 
> 
> Testing
> ---
> 
> Tests done in a subsequent review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40411: Libprocess Reinit: Modify test to use PID.

[Vinod Kone]

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


Ship it!




LGTM modulo naming of "metrics_singleton"

- Vinod Kone


On Sept. 20, 2016, 12:41 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40411/
> ---
> 
> (Updated Sept. 20, 2016, 12:41 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3934
> https://issues.apache.org/jira/browse/MESOS-3934
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updates the test's reference to the global metrics singleton.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_driver_tests.cpp 
> faf2e6c8ad17e07964b4340d0b340654b03f9086 
>   src/tests/scheduler_tests.cpp b0ea0bbcce9d847285fda40f778caaf721804457 
> 
> Diff: https://reviews.apache.org/r/40411/diff/
> 
> 
> Testing
> ---
> 
> Tests done in a subsequent review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40410: Libprocess Reinit: Move MetricsProcess instantiation into process.cpp.

[Vinod Kone]

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


Fix it, then Ship it!





3rdparty/libprocess/include/process/metrics/metrics.hpp (line 85)


new line.



3rdparty/libprocess/include/process/metrics/metrics.hpp (line 87)


we don't name other global processes singletons, so i would just call this 
"metrics_process" instead.



3rdparty/libprocess/src/metrics/metrics.cpp 


so MetricsProcess was not managed by gc before? was that a bug?


- Vinod Kone


On July 29, 2016, 11:56 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40410/
> ---
> 
> (Updated July 29, 2016, 11:56 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3934
> https://issues.apache.org/jira/browse/MESOS-3934
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The metrics singleton must be unified with `process::initialize` so 
> that it also falls under the scope of reinitialization.  The singleton 
> must also not be guarded by `Once`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 54487ab2614c7f8e8df10d1e0c39880a6cf5bde3 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> ac1544e70f5884f946ce3c31c5430c6a2c1f9dd1 
>   3rdparty/libprocess/src/process.cpp 
> 7f331b812de2f0437838f48e0959441c8e04c358 
>   3rdparty/libprocess/src/tests/system_tests.cpp 
> 0f4d0424689522337806ba2227ec4330c700e17e 
> 
> Diff: https://reviews.apache.org/r/40410/diff/
> 
> 
> Testing
> ---
> 
> Tests done in a subsequent review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 52620: Updated InverseOffers* maintenance tests to use the new scheduler mock.

[Ilya Pronin]

> On Oct. 7, 2016, 11:51 p.m., Anand Mazumdar wrote:
> > src/tests/master_maintenance_tests.cpp, lines 1169-1171
> > 
> >
> > We prefer to keep expectations close to the actual business logic for 
> > readability. Can you move this down to L1192?

I'm dropping this issue as discussed on IRC. The reason for it is that the 
framework will receive an inverse offer as soon as it subscribes. That's 
because as soon as the offer is sent out, the resources are "allocated", which 
causes the allocator to immediately send an inverse offer.


> On Oct. 7, 2016, 11:51 p.m., Anand Mazumdar wrote:
> > src/tests/master_maintenance_tests.cpp, lines 1529-1531
> > 
> >
> > Similar comment as per previous test. Move this expectation.

Dropping for the same reason as in previous test.


- Ilya


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


On Oct. 10, 2016, 8:18 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52620/
> ---
> 
> (Updated Oct. 10, 2016, 8:18 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Bugs: MESOS-4948
> https://issues.apache.org/jira/browse/MESOS-4948
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated InverseOffers* maintenance tests to use the new scheduler mock.
> 
> 
> Diffs
> -
> 
>   src/tests/master_maintenance_tests.cpp 
> 77eb405ab7314da906bed9ec1d0018c24928d8d8 
> 
> Diff: https://reviews.apache.org/r/52620/diff/
> 
> 
> Testing
> ---
> 
> Tested on OS X 10.11 and Ubuntu 14.04.
> ```
> make check GTEST_FILTER='MasterMaintenanceTest.*' GTEST_REPEAT=1000 
> GTEST_BREAK_ON_FAILURE=1
> ```
> Noticed a segfault caused by getenv / setenv race (MESOS-3475), but it's 
> unrelated to this change and is reproducible on current master.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 52620: Updated InverseOffers* maintenance tests to use the new scheduler mock.

[Ilya Pronin]

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

(Updated Oct. 10, 2016, 8:18 p.m.)


Review request for mesos and Anand Mazumdar.


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


Repository: mesos


Description
---

Updated InverseOffers* maintenance tests to use the new scheduler mock.


Diffs (updated)
-

  src/tests/master_maintenance_tests.cpp 
77eb405ab7314da906bed9ec1d0018c24928d8d8 

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


Testing (updated)
---

Tested on OS X 10.11 and Ubuntu 14.04.
```
make check GTEST_FILTER='MasterMaintenanceTest.*' GTEST_REPEAT=1000 
GTEST_BREAK_ON_FAILURE=1
```
Noticed a segfault caused by getenv / setenv race (MESOS-3475), but it's 
unrelated to this change and is reproducible on current master.


Thanks,

Ilya Pronin

Re: Review Request 52695: Harden libprocess

[Neil Conway]

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




3rdparty/libprocess/Makefile.am (line 16)


"Enable all warnings" is not a great description for "-Wall" (despite how 
it is named).



3rdparty/libprocess/Makefile.am (line 17)


"comparison"


- Neil Conway


On Oct. 10, 2016, 3:46 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52695/
> ---
> 
> (Updated Oct. 10, 2016, 3:46 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6229
> https://issues.apache.org/jira/browse/MESOS-6229
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use a default set of flags to provide additional security and hardening to 
> libprocess. Additionally, check and catch more warnings/errors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 020b0e1 
> 
> Diff: https://reviews.apache.org/r/52695/diff/
> 
> 
> Testing
> ---
> 
> Compared the benchmarks with and without the flags being used. Also did a 
> comparsion with the flags being used with and without optimizations and 
> without the flags being used with and without optimizations. Overall the 
> performance hit was very small with a 3-8% overhead (optimizations brings 
> this down slightly). Most benchmarks were about 5% (or less) slower.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 50737: Parametrized libprocess HTTPTests by SSL configuration.

[Joseph Wu]

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




3rdparty/libprocess/src/tests/http_tests.cpp (line 257)


This one needs a `GetParam()`.


- Joseph Wu


On Oct. 7, 2016, 11:37 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50737/
> ---
> 
> (Updated Oct. 7, 2016, 11:37 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5966
> https://issues.apache.org/jira/browse/MESOS-5966
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch parametrizes the HTTP tests in libprocess so
> that they run with SSL both enabled and disabled when
> the library was compiled with SSL support.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 8d6c8c4507b93dd11f927bad2a8f0faeb957e388 
> 
> Diff: https://reviews.apache.org/r/50737/diff/
> 
> 
> Testing
> ---
> 
> Tested on OSX with `GTEST_REPEAT=100 GTEST_BREAK_ON_FAILURE=1 
> GTEST_FILTER="*HTTPTest*" 3rdparty/libprocess/libprocess-tests`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 52064: Support for multiple versions of docs.

[Vinod Kone]

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



what's the status of this?

- Vinod Kone


On Sept. 28, 2016, 9:22 p.m., Tim Anderegg wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52064/
> ---
> 
> (Updated Sept. 28, 2016, 9:22 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Bugs: MESOS-3011
> https://issues.apache.org/jira/browse/MESOS-3011
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support for multiple versions of docs.  The approach I took was to use the 
> "git" ruby library to iterate over each tagged version of Mesos, and generate 
> the documentation.  This uses the "releases.yml" file to determine which 
> versions should build documentation.  The same is done for images, since they 
> may change between versions.  A select box was added to the website's 
> breadcrumbs bar that allows the user to easily switch between documentation 
> versions.  If a page is reached that doesn't exist in an older version of the 
> documentation, the user is notified and redirect to the main page for that 
> version.
> 
> 
> Diffs
> -
> 
>   site/Gemfile 4dcd0a5ad452085e6e3bb152a6547575636cd807 
>   site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 
>   site/Rakefile 01356891c29f9e69fa0f7813cf87e7662eda400b 
>   site/build.sh 11f15e15621c4d3db1472e88911787b9b3100f97 
>   site/config.rb 9e6738fccc48365e16221188269c1ed40772eca7 
>   site/data/releases.yml 1e9bb8555d266658baaf37c4b608eebeb0d14da8 
>   site/source/assets/js/versions.js PRE-CREATION 
>   site/source/layouts/basic.erb 7464e40b619e883daad93c72c3fbdbfbdda8f152 
>   site/source/layouts/documentation.erb 
> a91f916a5fb7348b2702c272e7a2059bdf628c66 
>   site/source/layouts/gettingstarted.erb PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52064/diff/
> 
> 
> Testing
> ---
> 
> Testing was done manually to verify that the documentation was built for each 
> version of Mesos that is supported (some older versions do not have 
> compatible documentation).
> 
> 
> Thanks,
> 
> Tim Anderegg
> 
>

Re: Review Request 52645: Harden Mesos

[Alex Clemmer]

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



I will leave comments about requiring GCC 4.9 to people who are more qualified 
to comment on it, but if we are enforcing this, then we do need to make it a 
requirement in the CMake build as well.

- Alex Clemmer


On Oct. 10, 2016, 6:16 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52645/
> ---
> 
> (Updated Oct. 10, 2016, 6:16 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6229
> https://issues.apache.org/jira/browse/MESOS-6229
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use a default set of flags to provide additional security and hardening to 
> Mesos. Additionally, check and catch more warnings/errors.
> 
> 
> Diffs
> -
> 
>   configure.ac 034bb91 
>   src/Makefile.am fd01e1d 
> 
> Diff: https://reviews.apache.org/r/52645/diff/
> 
> 
> Testing
> ---
> 
> Compared the benchmarks with and without the flags being used. Also did a 
> comparsion with the flags being used with and without optimizations and 
> without the flags being used with and without optimizations. Overall the 
> performance hit was very small with a 3-8% overhead (optimizations brings 
> this down slightly). Most benchmarks were about 5% (or less) slower.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Review Request 52701: WIP: Enhanced error messages for HTTP parsing errors.

[Joseph Wu]

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

Review request for mesos, Benjamin Mahler, Greg Mann, and Artem Harutyunyan.


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


Repository: mesos


Description
---

This is an incomplete patch as there are a couple other changes
that should be made to expose the error messages (seeo MESOS-5990).


Diffs
-

  3rdparty/libprocess/src/decoder.hpp c79296bc50e5bf5adf9f2c62114bef83bb183f79 
  3rdparty/libprocess/src/tests/decoder_tests.cpp 
4535614312373b0515025f09f9f8495f9ce353a3 

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


Testing
---

make check

Now the error message looks like:
```
Failed to decode body: HPE_INVALID_EOF_STATE: stream ended at an unexpected time
```


Thanks,

Joseph Wu

Re: Review Request 52664: Moved the `decimalFloat` filter to app.js for consistency.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Oct. 8, 2016, 6:10 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52664/
> ---
> 
> (Updated Oct. 8, 2016, 6:10 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved the `decimalFloat` filter to app.js for consistency.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/js/app.js 400a4286d9358c699453326d87a0bd49dbb105a7 
>   src/webui/master/static/js/controllers.js 
> 29a5a1c8754cc2fb934854750d6dfb04f1eaeae4 
> 
> Diff: https://reviews.apache.org/r/52664/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52470: Set executor id in `protobuf::createTask`.

[Vinod Kone]

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



I think it's probably worth to just mutate TaskInfo like you did in the very 
first diff rather than doing all these changes. They are bit messy and 
incomplete.


src/master/http.cpp (line 3776)


I think this should be

```
const Option executorId = taskInfo.has_executor()
  ? taskInfo.executor().executor_id()
  : None()

```

This is not completely true because we won't set ExecutorId for default 
executor. But atleast it won't break custom/command executor tasks.



src/slave/http.cpp (line 1391)


ditto.



src/slave/http.cpp (lines 1408 - 1415)


this means we won't be setting executorId for legacy custom executors that 
do not set type to `CUSTOM` (which we allow).



src/slave/slave.cpp (line 1383)


ditto.



src/slave/slave.cpp (lines 1401 - 1409)


this means we won't be setting executorId for legacy custom executors that 
do not set type to `CUSTOM` (which we allow).



src/slave/slave.cpp (lines 6537 - 6543)


this means we won't be setting executorId for legacy custom executors that 
do not set type to `CUSTOM` (which we allow).



src/slave/slave.cpp (lines 6590 - 6596)


ditto.



src/slave/slave.cpp (lines 6667 - 6670)


ditto.


- Vinod Kone


On Oct. 8, 2016, 12:20 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52470/
> ---
> 
> (Updated Oct. 8, 2016, 12:20 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6283
> https://issues.apache.org/jira/browse/MESOS-6283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set executor id in `protobuf::createTask`.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 80c66c4f7d52fc184e7553d173195ba7714971d4 
>   src/common/protobuf_utils.cpp 7362b875ce1ffca6bc6376169a11494bdb9cf062 
>   src/master/http.cpp bb9c87327dfe2161a6f1fd4cded72aa9a5ffaf66 
>   src/master/master.hpp 43518b9bf1bfaa54e26acc7f2e70c4161c667a84 
>   src/master/master.cpp ad8993ae43e2508a3777f4062acaca1177ac77f2 
>   src/slave/http.cpp 79061c3cd94d856ec695e5a82bf6792bf089d1f8 
>   src/slave/slave.cpp 119fb36c27b25739f2a86a55d48e964ca4a84ff7 
>   src/tests/command_executor_tests.cpp 
> 6e47243941626bb5b6224430f9a12ced8a3f5062 
>   src/tests/common/http_tests.cpp aea338674f1a3b769958af134d1435292b827ba7 
>   src/tests/slave_recovery_tests.cpp 703948f7a6861a4401ee45ce9cae2644106083f3 
> 
> Diff: https://reviews.apache.org/r/52470/diff/
> 
> 
> Testing
> ---
> 
> Add a new test case `CommandExecutorTest.EmptyExecutorIdInTask`.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52638: Populated `recovered` field in `GetAgents` response.

[Anand Mazumdar]

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



Would you be following up with the change to `/state` in a separate review?


src/master/http.cpp (lines 2315 - 2316)


Nit: Combine these two statements?

```cpp
getAgents.add_recovered()->CopyFrom(slaveId);
```


- Anand Mazumdar


On Oct. 7, 2016, 4 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52638/
> ---
> 
> (Updated Oct. 7, 2016, 4 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Xiaojian Huang.
> 
> 
> Bugs: MESOS-6177
> https://issues.apache.org/jira/browse/MESOS-6177
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Populated `recovered` field in `GetAgents` response.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp bb9c87327dfe2161a6f1fd4cded72aa9a5ffaf66 
> 
> Diff: https://reviews.apache.org/r/52638/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 52637: Added recovered `AgentID`s in `GetAgents` response.

[Anand Mazumdar]

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



Can you also update the review description to be more informative than what it 
is now i.e., around why do we need this new field?


include/mesos/master/master.proto (line 314)


s/recovered/recovered_agents to be consistent with other similar fields in 
this file.


- Anand Mazumdar


On Oct. 7, 2016, 3:59 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52637/
> ---
> 
> (Updated Oct. 7, 2016, 3:59 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Xiaojian Huang.
> 
> 
> Bugs: MESOS-6177
> https://issues.apache.org/jira/browse/MESOS-6177
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added recovered `AgentID`s in `GetAgents` response.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto d2312dce31f56dd65ac4d0e554b680749da61335 
>   include/mesos/v1/master/master.proto 
> 0dc6cca367590ea8e9f22b22b85baa4c11e69b13 
> 
> Diff: https://reviews.apache.org/r/52637/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 50912: Added the infrastructure for a new python-based CLI.

[Joseph Wu]

> On Oct. 5, 2016, 5:36 p.m., Vinod Kone wrote:
> > src/cli_new/bin/main.py, line 22
> > 
> >
> > why the space?
> > 
> > also alphabetical ordering.

This separates standard python libraries from local libraries (`config.py` and 
`__init__.py`).

Should be fine as is.


> On Oct. 5, 2016, 5:36 p.m., Vinod Kone wrote:
> > src/cli_new/bootstrap, line 19
> > 
> >
> > who sets VIRTUAL_ENV env variable?

This is something virtualenv sets.  For example, the shell script:
https://github.com/pypa/virtualenv/blob/15.0.3/virtualenv_embedded/activate.sh#L43-L44


- Joseph


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


On Oct. 4, 2016, 9:12 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50912/
> ---
> 
> (Updated Oct. 4, 2016, 9:12 p.m.)
> 
> 
> Review request for mesos, Haris Choudhary and Joseph Wu.
> 
> 
> Bugs: MESOS-6008
> https://issues.apache.org/jira/browse/MESOS-6008
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the infrastructure for a new python-based CLI.
> 
> 
> Diffs
> -
> 
>   src/cli_new/.gitignore PRE-CREATION 
>   src/cli_new/README.md PRE-CREATION 
>   src/cli_new/activate PRE-CREATION 
>   src/cli_new/bin/config.py PRE-CREATION 
>   src/cli_new/bin/main.py PRE-CREATION 
>   src/cli_new/bin/mesos PRE-CREATION 
>   src/cli_new/bootstrap PRE-CREATION 
>   src/cli_new/deactivate PRE-CREATION 
>   src/cli_new/lib/mesos/__init__.py PRE-CREATION 
>   src/cli_new/lib/mesos/docopt.py PRE-CREATION 
>   src/cli_new/lib/mesos/exceptions.py PRE-CREATION 
>   src/cli_new/lib/mesos/plugins/__init__.py PRE-CREATION 
>   src/cli_new/lib/mesos/plugins/base.py PRE-CREATION 
>   src/cli_new/lib/mesos/util.py PRE-CREATION 
>   src/cli_new/mesos.bash_completion PRE-CREATION 
>   src/cli_new/pip-requirements.txt PRE-CREATION 
>   src/cli_new/pylint.config PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50912/diff/
> 
> 
> Testing
> ---
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos --help
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 52645: Harden Mesos

[Aaron Wood]

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

(Updated Oct. 10, 2016, 6:16 p.m.)


Review request for mesos and Michael Park.


Changes
---

Extra docs about some of the flags we moved out of MESOS_CPPFLAGS.


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


Repository: mesos


Description
---

Use a default set of flags to provide additional security and hardening to 
Mesos. Additionally, check and catch more warnings/errors.


Diffs (updated)
-

  configure.ac 034bb91 
  src/Makefile.am fd01e1d 

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


Testing
---

Compared the benchmarks with and without the flags being used. Also did a 
comparsion with the flags being used with and without optimizations and without 
the flags being used with and without optimizations. Overall the performance 
hit was very small with a 3-8% overhead (optimizations brings this down 
slightly). Most benchmarks were about 5% (or less) slower.


Thanks,

Aaron Wood

Re: Review Request 52645: Harden Mesos

[Aaron Wood]

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

(Updated Oct. 10, 2016, 6:13 p.m.)


Review request for mesos and Michael Park.


Changes
---

Move some of the warnings that were set in MESOS_CPPFLAGS to AM_CXXFLAGS.


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


Repository: mesos


Description
---

Use a default set of flags to provide additional security and hardening to 
Mesos. Additionally, check and catch more warnings/errors.


Diffs (updated)
-

  configure.ac 034bb91 
  src/Makefile.am fd01e1d 

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


Testing
---

Compared the benchmarks with and without the flags being used. Also did a 
comparsion with the flags being used with and without optimizations and without 
the flags being used with and without optimizations. Overall the performance 
hit was very small with a 3-8% overhead (optimizations brings this down 
slightly). Most benchmarks were about 5% (or less) slower.


Thanks,

Aaron Wood

Re: Review Request 52645: Harden Mesos

[Aaron Wood]

> On Oct. 10, 2016, 5:32 p.m., James Peach wrote:
> > configure.ac, line 460
> > 
> >
> > GCC 4.9 is in devtoolset-3 for RHEL 6, so I think this is fine. IMHO it 
> > is still worth giving the mailing list a heads-up though.

Sure, will send out a note about this.


> On Oct. 10, 2016, 5:32 p.m., James Peach wrote:
> > src/Makefile.am, line 117
> > 
> >
> > Since you are now putting the compiler flags in the correct variables, 
> > you can remove then from ``MESOS_CPPFLAGS`` (which should only contain 
> > preprocessor options).

These were here before my changes so I had left them as is. I'll move them over 
to the right spot now.


- Aaron


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


On Oct. 10, 2016, 3:42 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52645/
> ---
> 
> (Updated Oct. 10, 2016, 3:42 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6229
> https://issues.apache.org/jira/browse/MESOS-6229
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use a default set of flags to provide additional security and hardening to 
> Mesos. Additionally, check and catch more warnings/errors.
> 
> 
> Diffs
> -
> 
>   configure.ac 034bb91 
>   src/Makefile.am fd01e1d 
> 
> Diff: https://reviews.apache.org/r/52645/diff/
> 
> 
> Testing
> ---
> 
> Compared the benchmarks with and without the flags being used. Also did a 
> comparsion with the flags being used with and without optimizations and 
> without the flags being used with and without optimizations. Overall the 
> performance hit was very small with a 3-8% overhead (optimizations brings 
> this down slightly). Most benchmarks were about 5% (or less) slower.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Joseph Wu]

> On Oct. 4, 2016, 5:18 p.m., Joseph Wu wrote:
> > support/mesos-style.py, lines 270-272
> > 
> >
> > It may help to change the order of reviews (no rebase necessary, just 
> > the "depends on" field).  
> > 
> > Because you add a linter in this review, that depends on the next one ( 
> > https://reviews.apache.org/r/50912/ ), this review fails to lint itself :)
> 
> Kevin Klues wrote:
> I'm confused. The order of these patches (and their dependencies) should 
> be correct. This patch "depends on" 50907, not 50912. Also, when I 
> cherry-pick each of these patches onto master individually, they all get 
> linted just as expected.
> 
> Kevin Klues wrote:
> For example, from master:
> ```
> klueska@c99:~/projects/mesos$ support/apply-reviews.py -r 50907
> 2016-10-04 20:31:30 URL:https://reviews.apache.org/r/50907/diff/raw/ 
> [16055/16055] -> "50907.patch" [1]
> No C++ files to lint
> [cli-test d536361] Abstracted mesos-style.py to wrap the cpp linter in a 
> class.
>  1 file changed, 249 insertions(+), 169 deletions(-)
>  rewrite support/mesos-style.py (95%)
> klueska@c99:~/projects/mesos$ support/apply-reviews.py -r 50910
> 2016-10-04 20:31:39 URL:https://reviews.apache.org/r/50910/diff/raw/ 
> [2997/2997] -> "50910.patch" [1]
> No C++ files to lint
> No Python files to lint
> [cli-test 2b9502e] Added a python linter to mesos-style.cpp.
>  Author: Haris Choudhary 
>  1 file changed, 67 insertions(+), 2 deletions(-)
> klueska@c99:~/projects/mesos$ support/apply-reviews.py -r 50912
> 2016-10-04 20:31:51 URL:https://reviews.apache.org/r/50912/diff/raw/ 
> [40930/40930] -> "50912.patch" [1]
> No C++ files to lint
> Checking 8 Python files
> Total errors found: 0
> [cli-test 88e5726] Added the infrastructure for a new python-based CLI.
>  20 files changed, 1019 insertions(+), 3 deletions(-)
>  create mode 100644 src/cli_new/.gitignore
>  create mode 100644 src/cli_new/README.md
>  create mode 100644 src/cli_new/activate
>  create mode 100644 src/cli_new/bin/config.py
>  create mode 100644 src/cli_new/bin/main.py
>  create mode 100755 src/cli_new/bin/mesos
>  create mode 100755 src/cli_new/bootstrap
>  create mode 100644 src/cli_new/deactivate
>  create mode 100644 src/cli_new/lib/mesos/__init__.py
>  create mode 100644 src/cli_new/lib/mesos/docopt.py
>  create mode 100644 src/cli_new/lib/mesos/exceptions.py
>  create mode 100644 src/cli_new/lib/mesos/plugins/__init__.py
>  create mode 100644 src/cli_new/lib/mesos/plugins/base.py
>  create mode 100644 src/cli_new/lib/mesos/util.py
>  create mode 100644 src/cli_new/mesos.bash_completion
>  create mode 100644 src/cli_new/pip-requirements.txt
>  create mode 100644 src/cli_new/pylint.config
> ```
> 
> Joseph Wu wrote:
> Presumably, that's because your source directory contains leftover 
> directories/files (like `*.pyc` files, which prevent git from deleting the 
> `cli_new` folder):
> ```
> $ support/apply-reviews.py -n -r 50910 -c
> 2016-10-05 10:49:31 URL:https://reviews.apache.org/r/50907/diff/raw/ 
> [16055/16055] -> "50907.patch" [1]
> No C++ files to lint
> [detached HEAD faed65f] Abstracted mesos-style.py to wrap the cpp linter 
> in a class.
>  Author: Kevin Klues 
>  1 file changed, 249 insertions(+), 169 deletions(-)
>  rewrite support/mesos-style.py (95%)
> 2016-10-05 10:49:32 URL:https://reviews.apache.org/r/50910/diff/raw/ 
> [2997/2997] -> "50910.patch" [1]
> No C++ files to lint
> Could not find 'src/cli_new'
> Please run from the root of the mesos source directory
> ```
> 
> If I do:
> ```
> $ support/apply-reviews.py -n -r 50907
> $ support/apply-reviews.py -n -r 50912 # <- reordered this one.
> $ support/apply-reviews.py -n -r 50910
> ```
> 
> It applies cleanly.
> 
> Kevin Klues wrote:
> I see. I think we should then add the linter in this commit (but with an 
> emty array of folders to check). And then introduce this folder into the 
> array in the next commit.  Otherwise we miss out on linting the entire base 
> patch for the CLI.

Hm, probably not worth the trouble of splitting up the reviews.  I'll just do 
this:
```
$ support/apply-reviews.py -n -r 50907
$ mkdir src/cli_new
$ support/apply-reviews.py -n -r 50910  # Applies cleanly now.
```


- Joseph


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


On Aug. 11, 2016, 2:54 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> 

Re: Review Request 45963: Allow tasks to set persistent volume as readonly or readwrite resource.

[Greg Mann]

> On Oct. 7, 2016, 5:49 p.m., Greg Mann wrote:
> > src/examples/persistent_shared_volume_framework.cpp, line 191
> > 
> >
> > s/used/uses/

It looks like this typo is still present? Or perhaps you meant to drop the 
issue instead of resolve?


- Greg


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


On Oct. 5, 2016, 12:04 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45963/
> ---
> 
> (Updated Oct. 5, 2016, 12:04 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4324
> https://issues.apache.org/jira/browse/MESOS-4324
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow the task to specify the persistent volume access to be read-only
> or read-write. Note that the persistent volume is always created as
> read-write.
> If the task is the first consumer of the shared persistent volume, then
> set the ownership of the persistent volume to match that of the task.
> Otherwise, allow the task to be executed only if the ownership of the
> persistent volume matches that of the task.
> Added an option to run the test in mixed (default) mode or shared-only
> mode. In mixed mode, multiple shards alternate between shared and
> unshared persistent volumes for the tasks. In shared-only mode, all
> shards use shared persistent volumes for the tasks.
> 
> 
> Diffs
> -
> 
>   src/examples/persistent_shared_volume_framework.cpp PRE-CREATION 
>   src/master/validation.hpp 035f721c610ae566c89a1cf0e65ff0df11679f15 
>   src/master/validation.cpp e5da3c9bf0a1042b42522f1ab74ce798fbb1738d 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
> 0a85935550e36c9142d845465cfa70a1634a647a 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> ea418252956c8089acc5a491888ed7f6df6cafcd 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
> 794b6e5990db5f8eb21a6535872f284ca02e0553 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
> af427c6e5691f1770ab3ebef79502eb2c2176c4a 
> 
> Diff: https://reviews.apache.org/r/45963/diff/
> 
> 
> Testing
> ---
> 
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 52689: Fixed a typo in CMakeLists.txt.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Oct. 10, 2016, 5:59 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52689/
> ---
> 
> (Updated Oct. 10, 2016, 5:59 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo in CMakeLists.txt.
> 
> 
> Diffs
> -
> 
>   src/log/CMakeLists.txt b2b930c68f64b402baff0310afe10fa2bdb62ad1 
> 
> Diff: https://reviews.apache.org/r/52689/diff/
> 
> 
> Testing
> ---
> 
> `ninja test`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52617: Improved symmetry of code in related utility functions.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Oct. 6, 2016, 2:37 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52617/
> ---
> 
> (Updated Oct. 6, 2016, 2:37 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved symmetry of code in related utility functions.
> 
> 
> Diffs
> -
> 
>   src/tests/utils.cpp cc5259a720bb6451714a100b0451b473395e3650 
> 
> Diff: https://reviews.apache.org/r/52617/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 52618: Added comment describing a common gotcha.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Oct. 6, 2016, 2:37 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52618/
> ---
> 
> (Updated Oct. 6, 2016, 2:37 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-6231
> https://issues.apache.org/jira/browse/MESOS-6231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added comment describing a common gotcha.
> 
> 
> Diffs
> -
> 
>   src/tests/utils.cpp cc5259a720bb6451714a100b0451b473395e3650 
> 
> Diff: https://reviews.apache.org/r/52618/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 52616: Fixed whitespace infelicities.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Oct. 6, 2016, 2:37 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52616/
> ---
> 
> (Updated Oct. 6, 2016, 2:37 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed whitespace infelicities.
> 
> 
> Diffs
> -
> 
>   src/tests/utils.cpp cc5259a720bb6451714a100b0451b473395e3650 
> 
> Diff: https://reviews.apache.org/r/52616/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 52645: Harden Mesos

[James Peach]

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




configure.ac (line 460)


GCC 4.9 is in devtoolset-3 for RHEL 6, so I think this is fine. IMHO it is 
still worth giving the mailing list a heads-up though.



src/Makefile.am (line 117)


Since you are now putting the compiler flags in the correct variables, you 
can remove then from ``MESOS_CPPFLAGS`` (which should only contain preprocessor 
options).


- James Peach


On Oct. 10, 2016, 3:42 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52645/
> ---
> 
> (Updated Oct. 10, 2016, 3:42 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6229
> https://issues.apache.org/jira/browse/MESOS-6229
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use a default set of flags to provide additional security and hardening to 
> Mesos. Additionally, check and catch more warnings/errors.
> 
> 
> Diffs
> -
> 
>   configure.ac 034bb91 
>   src/Makefile.am fd01e1d 
> 
> Diff: https://reviews.apache.org/r/52645/diff/
> 
> 
> Testing
> ---
> 
> Compared the benchmarks with and without the flags being used. Also did a 
> comparsion with the flags being used with and without optimizations and 
> without the flags being used with and without optimizations. Overall the 
> performance hit was very small with a 3-8% overhead (optimizations brings 
> this down slightly). Most benchmarks were about 5% (or less) slower.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52208: Fixed warnings in health_checker.cpp.

[Joseph Wu]

> On Oct. 10, 2016, 8:25 a.m., Alexander Rukletsov wrote:
> > src/health-check/health_checker.cpp, line 179
> > 
> >
> > Why haven't you used `Duration::create()`?

Didn't realize that existed :)

That does look safer than plain-old casting.  I will leave it up to you, if you 
want to fix it.


- Joseph


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


On Sept. 22, 2016, 9:50 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52208/
> ---
> 
> (Updated Sept. 22, 2016, 9:50 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed warnings in health_checker.cpp.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.cpp 
> 758cbb382b08743f518022ba66eaafbea7615592 
> 
> Diff: https://reviews.apache.org/r/52208/diff/
> 
> 
> Testing
> ---
> 
> Windows: buils
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 51999: Refactor parsing of resources.

[Jiang Yan Xu]

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


Fix it, then Ship it!




Committing with the following adjustment.


src/v1/resources.cpp (line 636)


Let's perserve this TODO as I think some form of this is still necessary.


- Jiang Yan Xu


On Oct. 5, 2016, 7 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51999/
> ---
> 
> (Updated Oct. 5, 2016, 7 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6062
> https://issues.apache.org/jira/browse/MESOS-6062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored `Resources::parse()` into 2 separate static functions:
> 1. Resources::fromJSON() to parse JSON representation of resources.
> 2. Resources::fromSimpleString() to parse text representation of
>resources.
> 
> Since these 2 new functions return a `Try`, the
> existing `Resources::parse()` implicitly converts that to a
> `Resources` object. This refactor is done to retrieve all resources
> (include empty resources) required for auto detection of root
> and MOUNT disks.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 3ef8cacee529addc745b4aeb6398d7606c61b749 
>   include/mesos/v1/resources.hpp ef56b4960b103a3efd916fab64796aa334ba44c6 
>   src/common/resources.cpp 0774ff0669e831494d5b12b88e19dfa0a4a3f757 
>   src/tests/resources_tests.cpp 3e493007d6d1d8194d07035aaa1cde28dedf2b5a 
>   src/v1/resources.cpp 62a644ebbd13cfc0862bd118ba16c43e0f6aaf90 
> 
> Diff: https://reviews.apache.org/r/51999/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 52690: Added 3rdparty build byproducts to cmake setup.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52690]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Oct. 10, 2016, 12:59 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52690/
> ---
> 
> (Updated Oct. 10, 2016, 12:59 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5656
> https://issues.apache.org/jira/browse/MESOS-5656
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We depend on certain artifacts in built 3rdparty components. With the
> make generator just defining it appears just defining an external
> project is good enough since make just uses these as order-only
> targets. With the ninja generator on the other hand this appears to be
> insufficient as its dependency graph analysis detects that 3rdparty
> libraries we are about to link against do not exist and no explicit
> rule defines a recipe exists to generate them.
> 
> This commits adds BUILD_BYPRODUCTS clauses for zookeeper and leveldb.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 770a3828a6e0ffaa4f185392fdc1a2152446449d 
> 
> Diff: https://reviews.apache.org/r/52690/diff/
> 
> 
> Testing
> ---
> 
> `ninja test`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52556: Added the MESOS_RUNTIME_DIRECTORY executor env variable.

[Jiang Yan Xu]

> On Oct. 10, 2016, 8:32 a.m., Jiang Yan Xu wrote:
> > src/launcher/executor.cpp, lines 903-910
> > 
> >
> > You didn't start this but I wonder why we can't consistently use the 
> > flags, which can be provided through environment variables, i.e., use 
> > `flags.load("MESOS_", , );` above.
> > 
> > That question aside, I don't think we need to be concerned about 
> > backwards-compatibility here: this is the command executor bundled with the 
> > agent so they are upgraded together. i.e., we are sure that the agent is 
> > setting the new variable.
> 
> Gastón Kleiman wrote:
> Wouldn't then the tasks started between the moment in which the binary is 
> overwritten and the moment in which the Mesos Agent is restarted fail?

OK I guess it'll be helpful in this regard. My main point is that this is not a 
deprecation cycle concern.

Generally it's advisable to restart the agent right after upgrading it but even 
though it's done this way with most Mesos toolings I have seen, I've also seen 
tasks failing when launched mid-upgrade with libmesos and executable mismatch. 

Therefore, fine with me to do this to aid N -> N+1 upgrade.

If we do this, the cleaniest approach may be flag aliases.


- Jiang Yan


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


On Oct. 10, 2016, 5:35 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52556/
> ---
> 
> (Updated Oct. 10, 2016, 5:35 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, Jie Yu, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6288
> https://issues.apache.org/jira/browse/MESOS-6288
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This environment variable points to the directory containing the
> binaries used by the Mesos Agent. It will eventually replace the
> `--launcher-dir` executor flag.
> 
> 
> Diffs
> -
> 
>   docs/executor-http-api.md 50b4cb4efff48bcc56330b81bd7c4c217b8a22b5 
>   src/launcher/executor.cpp 8a1051b886a0c95d19ff370e5c77d9c4033c8b61 
>   src/slave/slave.cpp d30001bc0d1798311ba3966f67aadc6be2c92306 
> 
> Diff: https://reviews.apache.org/r/52556/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 52092: Avoided to concat cgroup internally in subsystems.

[Jie Yu]

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



Is it up-to-date? Can you do a base and I'll get this committed.

- Jie Yu


On Sept. 25, 2016, 5:44 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52092/
> ---
> 
> (Updated Sept. 25, 2016, 5:44 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Qian Zhang.
> 
> 
> Bugs: MESOS-6218
> https://issues.apache.org/jira/browse/MESOS-6218
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To avoid using `path::join(flags.cgroups_root, containerId.value())` to
> concat cgroup internally in subsystems, pass cgroup to the subsystem
> interfaces directly.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> c2fcc0b21c5b1e51cb38b61245d4bbd3856a9512 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 6594110bd45a71c6d41a24b3f5b73c4a4e3a7335 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> 05cb8b8efadd2355789d08ce5c9da9d3de0fc72a 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.hpp 
> c1f1004d7578f59dfa9e28323e7a55df41c4cb6b 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp 
> 5912c6504a375178b892bf8099d5f75fa9e91c05 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.hpp 
> 10fbbc9cb39a3b41d64cc090b0e890a2416b2306 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.cpp 
> a35ecb073acefb903d7e8c4737d6cd048a2b494d 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp 
> b5d5ed233c0c8ddc339d2550c0f57a02dd3f14c3 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp 
> 0afc24867eb0b1949e95b3f5a8914be013dbf531 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 
> a1c87ce2ace33f1a779e843578f55d7502562c8d 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 
> 5b3ce15c209f6ce0e430386d97bc6768fca805c8 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp 
> 16208655e16b81f1e4bbf97bf5e32e75f4c3f3a9 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.cpp 
> 93539f1fc8265f66b62294c22f4eaba704b8b876 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.hpp 
> 5a0adfdb8705ae6b20c61a827380142e418c0ae4 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 
> cc39f287a64a4125260597e74784dc0847953376 
> 
> Diff: https://reviews.apache.org/r/52092/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52556: Added the MESOS_RUNTIME_DIRECTORY executor env variable.

[Gastón Kleiman]

> On Oct. 10, 2016, 3:32 p.m., Jiang Yan Xu wrote:
> > src/launcher/executor.cpp, lines 903-910
> > 
> >
> > You didn't start this but I wonder why we can't consistently use the 
> > flags, which can be provided through environment variables, i.e., use 
> > `flags.load("MESOS_", , );` above.
> > 
> > That question aside, I don't think we need to be concerned about 
> > backwards-compatibility here: this is the command executor bundled with the 
> > agent so they are upgraded together. i.e., we are sure that the agent is 
> > setting the new variable.

Wouldn't then the tasks started between the moment in which the binary is 
overwritten and the moment in which the Mesos Agent is restarted fail?


- Gastón


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


On Oct. 10, 2016, 12:35 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52556/
> ---
> 
> (Updated Oct. 10, 2016, 12:35 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, Jie Yu, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6288
> https://issues.apache.org/jira/browse/MESOS-6288
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This environment variable points to the directory containing the
> binaries used by the Mesos Agent. It will eventually replace the
> `--launcher-dir` executor flag.
> 
> 
> Diffs
> -
> 
>   docs/executor-http-api.md 50b4cb4efff48bcc56330b81bd7c4c217b8a22b5 
>   src/launcher/executor.cpp 8a1051b886a0c95d19ff370e5c77d9c4033c8b61 
>   src/slave/slave.cpp d30001bc0d1798311ba3966f67aadc6be2c92306 
> 
> Diff: https://reviews.apache.org/r/52556/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 52647: Fix new errors/warnings produced by hardened flags

[Aaron Wood]

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

(Updated Oct. 10, 2016, 3:51 p.m.)


Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

The hardening flags produced many new warnings/errors that need to be fixed for 
Mesos to compile/run.


Diffs
-

  3rdparty/libprocess/include/process/profiler.hpp f6fccfb 
  3rdparty/libprocess/src/encoder.hpp af083d1 
  3rdparty/libprocess/src/process.cpp 02a1925 
  3rdparty/libprocess/src/profiler.cpp 0c4949e 
  3rdparty/libprocess/src/socket.cpp 1e49518 
  3rdparty/libprocess/src/tests/benchmarks.cpp 945007c 
  3rdparty/libprocess/src/tests/decoder_tests.cpp 4535614 
  3rdparty/libprocess/src/tests/encoder_tests.cpp 9e57375 
  3rdparty/libprocess/src/tests/http_tests.cpp 2538f56 
  3rdparty/libprocess/src/tests/io_tests.cpp b85c79f 
  3rdparty/libprocess/src/tests/process_tests.cpp b9feec7 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 66ccff9 
  3rdparty/stout/tests/cache_tests.cpp 0950c85 
  3rdparty/stout/tests/flags_tests.cpp 94ba915 
  3rdparty/stout/tests/hashmap_tests.cpp 2626d67 
  3rdparty/stout/tests/hashset_tests.cpp 66e59db 
  3rdparty/stout/tests/ip_tests.cpp 59e69a5 
  3rdparty/stout/tests/json_tests.cpp 2bc4c88 
  3rdparty/stout/tests/linkedhashmap_tests.cpp 7a80769 
  3rdparty/stout/tests/multimap_tests.cpp 488991b 
  3rdparty/stout/tests/os/process_tests.cpp 1e26877 
  3rdparty/stout/tests/os/sendfile_tests.cpp e221689 
  3rdparty/stout/tests/os/systems_tests.cpp 110ba5b 
  3rdparty/stout/tests/os_tests.cpp c2900b8 
  3rdparty/stout/tests/strings_tests.cpp 7dd3301 

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


Testing
---

Made sure compilation, tests, and benchmarks worked with both gcc and clang. 
Ran `make && make check && make bench`.


Thanks,

Aaron Wood

Re: Review Request 51719: Permitted specifying custom test driver in stout.

[Till Toenshoff]

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


Fix it, then Ship it!




Ship It!


3rdparty/stout/configure.ac (line 159)


Not yours but please add a period here.


- Till Toenshoff


On Oct. 1, 2016, 10:54 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51719/
> ---
> 
> (Updated Oct. 1, 2016, 10:54 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-6140
> https://issues.apache.org/jira/browse/MESOS-6140
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Permitted specifying custom test driver in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/Makefile.am fda069d0f7d75ca80b56eadac415cba57afb03f7 
>   3rdparty/stout/configure.ac 2236a2d900c740e2f254ef885332f80d37fb6e11 
> 
> Diff: https://reviews.apache.org/r/51719/diff/
> 
> 
> Testing
> ---
> 
> * `make check` with `--enable-parallel-test-execution` (OS X, python 2.7.12)
> * `make check MESOS_TEST_RUNNER=''` temporarily disables parallel test 
> execution
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 51718: Permitted specifying custom test driver in libprocess.

[Till Toenshoff]

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


Fix it, then Ship it!




Ship It!


3rdparty/libprocess/configure.ac (line 216)


Period please.


- Till Toenshoff


On Oct. 1, 2016, 10:54 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51718/
> ---
> 
> (Updated Oct. 1, 2016, 10:54 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-6140
> https://issues.apache.org/jira/browse/MESOS-6140
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Permitted specifying custom test driver in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 020b0e1e4c49805a3c7a61b308d897fc82f5e0ce 
>   3rdparty/libprocess/configure.ac bc0e6ab14725e5a90df4cacf8b1b0e807a69b8b1 
> 
> Diff: https://reviews.apache.org/r/51718/diff/
> 
> 
> Testing
> ---
> 
> `make check` with `--enable-parallel-test-execution` (OS X, python 2.7.12)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 52696: Harden stout

[Aaron Wood]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Use a default set of flags to provide additional security and hardening to 
stout. Additionally, check and catch more warnings/errors.


Diffs
-

  3rdparty/stout/Makefile.am fda069d 

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


Testing
---

Compared the benchmarks with and without the flags being used. Also did a 
comparsion with the flags being used with and without optimizations and without 
the flags being used with and without optimizations. Overall the performance 
hit was very small with a 3-8% overhead (optimizations brings this down 
slightly). Most benchmarks were about 5% (or less) slower.


Thanks,

Aaron Wood

Review Request 52695: Harden libprocess

[Aaron Wood]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Use a default set of flags to provide additional security and hardening to 
libprocess. Additionally, check and catch more warnings/errors.


Diffs
-

  3rdparty/libprocess/Makefile.am 020b0e1 

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


Testing
---

Compared the benchmarks with and without the flags being used. Also did a 
comparsion with the flags being used with and without optimizations and without 
the flags being used with and without optimizations. Overall the performance 
hit was very small with a 3-8% overhead (optimizations brings this down 
slightly). Most benchmarks were about 5% (or less) slower.


Thanks,

Aaron Wood

Re: Review Request 52645: Harden Mesos

[Aaron Wood]

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

(Updated Oct. 10, 2016, 3:42 p.m.)


Review request for mesos and Michael Park.


Changes
---

Addressed comments. Only target Mesos in this patch. Other RR's will contain 
changes for libprocess and stout.


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


Repository: mesos


Description
---

Use a default set of flags to provide additional security and hardening to 
Mesos. Additionally, check and catch more warnings/errors.


Diffs (updated)
-

  configure.ac 034bb91 
  src/Makefile.am fd01e1d 

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


Testing
---

Compared the benchmarks with and without the flags being used. Also did a 
comparsion with the flags being used with and without optimizations and without 
the flags being used with and without optimizations. Overall the performance 
hit was very small with a 3-8% overhead (optimizations brings this down 
slightly). Most benchmarks were about 5% (or less) slower.


Thanks,

Aaron Wood

Re: Review Request 52645: Harden Mesos

[Aaron Wood]

On Oct. 7, 2016, 10:15 p.m., Aaron Wood wrote:
> > (1) Do we need to make the `CXXFLAGS` conditional on being supported by the 
> > current compiler? Seems like these flags are quite specific to (certain 
> > versions of?) gcc/clang.
> > 
> > (2) You should split this review into three separate reviews: a single 
> > review should make changes to at most one of Mesos, libprocess, and stout.
> > 
> > (3) What _specific_ attack vectors are these changes intended to prevent?

1. I believe the only flag that we need to watch out for with compatability is 
the `-fstack-protector-strong`. Since Mesos currently requires GCC >= 4.8.1 I 
think we should be good with the rest. Since `-fstack-protector-strong` is 
supported in GCC >= 4.9 I propose that we require at least this version.
2. Will do that right now :)
3. Overall the changes here should help prevent buffer overflows, stack 
overflows, and general memory corruption attacks. Having position independent 
code/binaries will also better take advantage of address space layout 
randomization which makes it much harder to successfully perform exploits. This 
should ideally give us better protection from zero days as well.


- Aaron


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


On Oct. 7, 2016, 7:22 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52645/
> ---
> 
> (Updated Oct. 7, 2016, 7:22 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6229
> https://issues.apache.org/jira/browse/MESOS-6229
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use a default set of flags to provide additional security and hardening to 
> Mesos. Additionally, check and catch more warnings/errors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 020b0e1 
>   3rdparty/stout/Makefile.am fda069d 
>   src/Makefile.am bfdb66a 
> 
> Diff: https://reviews.apache.org/r/52645/diff/
> 
> 
> Testing
> ---
> 
> Compared the benchmarks with and without the flags being used. Also did a 
> comparsion with the flags being used with and without optimizations and 
> without the flags being used with and without optimizations. Overall the 
> performance hit was very small with a 3-8% overhead (optimizations brings 
> this down slightly). Most benchmarks were about 5% (or less) slower.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52626: Ensured allocations are processed before querying metrics in a test.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Oct. 7, 2016, 9:43 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52626/
> ---
> 
> (Updated Oct. 7, 2016, 9:43 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6321
> https://issues.apache.org/jira/browse/MESOS-6321
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 7957873102b3777d47fa9304b85a93d58a746b9d 
> 
> Diff: https://reviews.apache.org/r/52626/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS X.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52556: Added the MESOS_RUNTIME_DIRECTORY executor env variable.

[Jiang Yan Xu]

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




src/launcher/executor.cpp (lines 903 - 910)


You didn't start this but I wonder why we can't consistently use the flags, 
which can be provided through environment variables, i.e., use 
`flags.load("MESOS_", , );` above.

That question aside, I don't think we need to be concerned about 
backwards-compatibility here: this is the command executor bundled with the 
agent so they are upgraded together. i.e., we are sure that the agent is 
setting the new variable.



src/slave/slave.cpp (lines 4328 - 4331)


Same here. Since the logic here is specific for the bundled command 
executor, I don't think there's backwards-compatibility concerns here. We can 
just change it.



src/slave/slave.cpp (line 6847)


I asked about this on slack then started discussing it on the dev list. I'd 
say we don't comment on the deprecation until we reach conclusion on that 
thread.


- Jiang Yan Xu


On Oct. 10, 2016, 5:35 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52556/
> ---
> 
> (Updated Oct. 10, 2016, 5:35 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, Jie Yu, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6288
> https://issues.apache.org/jira/browse/MESOS-6288
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This environment variable points to the directory containing the
> binaries used by the Mesos Agent. It will eventually replace the
> `--launcher-dir` executor flag.
> 
> 
> Diffs
> -
> 
>   docs/executor-http-api.md 50b4cb4efff48bcc56330b81bd7c4c217b8a22b5 
>   src/launcher/executor.cpp 8a1051b886a0c95d19ff370e5c77d9c4033c8b61 
>   src/slave/slave.cpp d30001bc0d1798311ba3966f67aadc6be2c92306 
> 
> Diff: https://reviews.apache.org/r/52556/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 51717: Enable Mesos test runner.

[Till Toenshoff]

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


Fix it, then Ship it!




Ship It!


src/Makefile.am (line 2343)


s/;//


- Till Toenshoff


On Oct. 1, 2016, 10:54 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51717/
> ---
> 
> (Updated Oct. 1, 2016, 10:54 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-6140
> https://issues.apache.org/jira/browse/MESOS-6140
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable Mesos test runner.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am f093000e0282a8d5ac17e7ba33711690ccdfe68a 
> 
> Diff: https://reviews.apache.org/r/51717/diff/
> 
> 
> Testing
> ---
> 
> * `make check` with `--enable-parallel-test-execution` (OS X, python 2.7.12)
> * `make check MESOS_TEST_RUNNER=''` temporarily disables parallel test 
> execution
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 51716: Added configure option for Mesos test runner.

[Till Toenshoff]

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


Fix it, then Ship it!




Ship It!


configure.ac (lines 616 - 617)


Not a biggy but maybe it is a good idea to make this better readable / 
parseable by creating a local temporary variable.


- Till Toenshoff


On Oct. 1, 2016, 10:54 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51716/
> ---
> 
> (Updated Oct. 1, 2016, 10:54 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-6140
> https://issues.apache.org/jira/browse/MESOS-6140
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added configure option for Mesos test runner.
> 
> 
> Diffs
> -
> 
>   configure.ac 57482d39db1f83e92e75fca959cd6df329a1c24f 
> 
> Diff: https://reviews.apache.org/r/51716/diff/
> 
> 
> Testing
> ---
> 
> `make check` with `--enable-parallel-test-execution` (OS X, python 2.7.12)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52208: Fixed warnings in health_checker.cpp.

[Alexander Rukletsov]

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




src/health-check/health_checker.cpp (line 179)


Why haven't you used `Duration::create()`?


- Alexander Rukletsov


On Sept. 23, 2016, 4:50 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52208/
> ---
> 
> (Updated Sept. 23, 2016, 4:50 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed warnings in health_checker.cpp.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.cpp 
> 758cbb382b08743f518022ba66eaafbea7615592 
> 
> Diff: https://reviews.apache.org/r/52208/diff/
> 
> 
> Testing
> ---
> 
> Windows: buils
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 51715: Added a parallel gtest runner.

[Till Toenshoff]

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


Fix it, then Ship it!




Looks very good guys - thanks Kevin for reviewing and Benjamin for actually 
making it happen - we all will love this stuff! :)

I specifically like the idea of not finishing right here but keeping an eye on 
the results / demands and doing further iterations.


support/mesos-gtest-runner.py (line 4)


Can we please add a license header - AFAIK the Apache Foundation is rather 
clear on this; every single source file needs to contain such header.

Maybe it made sense to add a ticket which fixes that lack of license 
headers in the other support-scripts?


- Till Toenshoff


On Oct. 1, 2016, 7:06 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51715/
> ---
> 
> (Updated Oct. 1, 2016, 7:06 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Till Toenshoff.
> 
> 
> Bugs: MESOS-6140
> https://issues.apache.org/jira/browse/MESOS-6140
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds `b814987b28cfb65a7c9635c83399545e423e690a` of
> https://github.com/bbannier/gtest-shrdr.
> 
> 
> Diffs
> -
> 
>   support/mesos-gtest-runner.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51715/diff/
> 
> 
> Testing
> ---
> 
> Tested with e.g., `stout-tests`
> 
> $ ./support/mesos-gtest-runner.py build/3rdparty/stout/stout-tests
> 
> [PASS]
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 52693: Changed master to send TASK_UNKNOWN during reconciliation.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Previously, the master would send TASK_LOST in response to explicit
reconciliation requests for (a) unknown tasks at registered slaves and
(b) tasks at unknown slaves (neither registered nor unreachable). The
master will now send TASK_UNKNOWN for these situations if the framework
has the PARTITION_AWARE capability.


Diffs
-

  src/master/master.cpp ad8993ae43e2508a3777f4062acaca1177ac77f2 
  src/tests/partition_tests.cpp 12fe8593ff17c35d540f944c428cf7f33b7dcbb3 
  src/tests/reconciliation_tests.cpp 1412090299df388456f04ed58a1d384ce3ff550a 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 52676: Made stout's tests a phony target.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52676]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Oct. 10, 2016, 11:54 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52676/
> ---
> 
> (Updated Oct. 10, 2016, 11:54 a.m.)
> 
> 
> Review request for mesos, Kapil Arya and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the stout root folder contains a folder `tests/` so the automake
> build target `tests` conflicts that one. This e.g., leads to the
> `tests` target no being built unless a user executes e.g.,
> 
> % make check
> 
> Make `tests` a phony target to avoid the name conflict.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/Makefile.am fda069d0f7d75ca80b56eadac415cba57afb03f7 
> 
> Diff: https://reviews.apache.org/r/52676/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52563: Minor clean ups in `health_check_tests.cpp` for consistency.

[Alexander Rukletsov]

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




src/tests/health_check_tests.cpp (line 231)


s/reconcilliation/reconciliation



src/tests/health_check_tests.cpp (line 359)


ditto.



src/tests/health_check_tests.cpp (line 477)


ditto.


- Alexander Rukletsov


On Oct. 7, 2016, 2:40 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52563/
> ---
> 
> (Updated Oct. 7, 2016, 2:40 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> * Replace `Testing` to `Tests` in comments.
> * Remove redundant `.Times(1)`.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
> 
> Diff: https://reviews.apache.org/r/52563/diff/
> 
> 
> Testing
> ---
> 
> ```
> sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*HealthCheckTest.*" 
> --gtest_also_run_disabled_tests
> [--] Global test environment tear-down
> [==] 19 tests from 1 test case ran. (35264 ms total)
> [  PASSED  ] 19 tests.
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52561: Renamed `flags` to `agentFlags` in health check test cases.

[Alexander Rukletsov]

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




src/tests/health_check_tests.cpp (line 376)


If you rename slave -> agent in this test, why not in all others in this 
file? Please either rename all accurencies, or not at all.


- Alexander Rukletsov


On Oct. 7, 2016, 2:40 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52561/
> ---
> 
> (Updated Oct. 7, 2016, 2:40 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed `flags` to `agentFlags` in health check test cases.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
> 
> Diff: https://reviews.apache.org/r/52561/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52556: Added the MESOS_RUNTIME_DIRECTORY executor env variable.

[Alexander Rukletsov]

> On Oct. 6, 2016, 3:40 p.m., Alexander Rukletsov wrote:
> > Please add support for
> > * docker executor
> > * pod (default) executor
> 
> Gastón Kleiman wrote:
> The default executor has no use for this env variable yet. Should I add 
> it as an unused attribute in the `DefaultExecutor` class?

We will need it for MESOS-6119, hence it's fine to introduce an yet unused 
variable.


- Alexander


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


On Oct. 10, 2016, 12:35 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52556/
> ---
> 
> (Updated Oct. 10, 2016, 12:35 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, Jie Yu, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6288
> https://issues.apache.org/jira/browse/MESOS-6288
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This environment variable points to the directory containing the
> binaries used by the Mesos Agent. It will eventually replace the
> `--launcher-dir` executor flag.
> 
> 
> Diffs
> -
> 
>   docs/executor-http-api.md 50b4cb4efff48bcc56330b81bd7c4c217b8a22b5 
>   src/launcher/executor.cpp 8a1051b886a0c95d19ff370e5c77d9c4033c8b61 
>   src/slave/slave.cpp d30001bc0d1798311ba3966f67aadc6be2c92306 
> 
> Diff: https://reviews.apache.org/r/52556/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Review Request 52689: Fixed a typo in CMakeLists.txt.

[Benjamin Bannier]

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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

Fixed a typo in CMakeLists.txt.


Diffs
-

  src/log/CMakeLists.txt b2b930c68f64b402baff0310afe10fa2bdb62ad1 

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


Testing
---

`ninja test`


Thanks,

Benjamin Bannier

Review Request 52690: Added 3rdparty build byproducts to cmake setup.

[Benjamin Bannier]

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

Review request for mesos and Joseph Wu.


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


Repository: mesos


Description
---

We depend on certain artifacts in built 3rdparty components. With the
make generator just defining it appears just defining an external
project is good enough since make just uses these as order-only
targets. With the ninja generator on the other hand this appears to be
insufficient as its dependency graph analysis detects that 3rdparty
libraries we are about to link against do not exist and no explicit
rule defines a recipe exists to generate them.

This commits adds BUILD_BYPRODUCTS clauses for zookeeper and leveldb.


Diffs
-

  3rdparty/CMakeLists.txt 770a3828a6e0ffaa4f185392fdc1a2152446449d 

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


Testing
---

`ninja test`


Thanks,

Benjamin Bannier