Re: Review Request 52103: Implemented quota update in `/quota` endpoint.

[Zhitao Li]

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

(Updated Aug. 5, 2017, 4:41 a.m.)


Review request for mesos, Alexander Rukletsov, Xiaojian Huang, and Neil Conway.


Changes
---

Implement update through same `POST` method and `setQuota` functions.


Summary (updated)
-

Implemented quota update in `/quota` endpoint.


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


Repository: mesos


Description (updated)
---

This patch implemented atomic quota update through
on the `/quota` endpoint by relaxing existing `setQuota`
functions in allocator interface.

Changes included:
- implementation in hierarchical allocator;
- implementation in quota_handler;
- some integration tests for updating a quota.

More tests around resource rescind and operator API
implementation will be sent in later patches.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
f021c34ef11aac42026ba39c5a1b775794982035 
  src/master/http.cpp 9df086c417a9392f62d600c7a6486be0a1cf7e70 
  src/master/master.hpp 84465af782d4024f22463d981ef9d0ef7827d043 
  src/master/quota_handler.cpp 26b2dc86fa44dfdf6e0a6c4993c754a6384dd851 
  src/tests/master_quota_tests.cpp 9d52f767c112abd55ab3f49d172eb6e3caea511b 


Diff: https://reviews.apache.org/r/52103/diff/6/

Changes: https://reviews.apache.org/r/52103/diff/5-6/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 53691: Implemented some quota functionality tests.

[Zhitao Li]

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

(Updated Aug. 5, 2017, 4:40 a.m.)


Review request for mesos, Alexander Rukletsov, Xiaojian Huang, and Neil Conway.


Changes
---

Rebase recent changes.


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


Repository: mesos


Description (updated)
---

This implements some tests in previous todos for
quota functionality.

For posterity, removed comments are implemented in:
* An agent with quota'ed tasks disconnects and there are not enough free
  resources (alert and under quota situation): 
`InsufficientResourceAfterSlaveDisconnect`;
* An agent with quota'ed tasks disconnects and there are enough free
  resources (new offers): `SufficientAfterSlaveWithQuotaTaskDisconnect`
* Role quota is below its allocation (InverseOffer generation): This is not 
happening yet.
* Two roles, two frameworks, one is production but rejects offers, the
  other is greedy and tries to hijack the cluster which is prevented by
  quota: `GreedyFrameworkCannotHijackQuotaRole`
* Quota'ed and non-quota'ed roles, multiple frameworks in quota'ed role,
  ensure total allocation sums up to quota: `TotalAllocationSatisfiesQuota`
* Remove quota with no running tasks: `RemoveQuotaNoTask`
* Remove quota with running tasks: `RemoveQuotaWithTask`


Diffs (updated)
-

  src/tests/master_quota_tests.cpp 9d52f767c112abd55ab3f49d172eb6e3caea511b 


Diff: https://reviews.apache.org/r/53691/diff/4/

Changes: https://reviews.apache.org/r/53691/diff/3-4/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 52284: Implemented more quota validation tests and validate duplicate name.

[Zhitao Li]

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

(Updated Aug. 5, 2017, 4:38 a.m.)


Review request for mesos, Alexander Rukletsov, Xiaojian Huang, and Neil Conway.


Changes
---

Rebase recent changes.


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


Repository: mesos


Description
---

Implemented more quota validation tests and validate duplicate name.


Diffs (updated)
-

  src/master/quota.cpp 79e4ad8ae5cdada8ac98d663b50645ae0e792f72 
  src/tests/master_quota_tests.cpp 9d52f767c112abd55ab3f49d172eb6e3caea511b 


Diff: https://reviews.apache.org/r/52284/diff/7/

Changes: https://reviews.apache.org/r/52284/diff/6-7/


Testing
---


Thanks,

Zhitao Li

Review Request 61446: Added a test to verify a fix for MESOS-7863.

[Benjamin Mahler]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


Bugs: MESOS-7783 and MESOS-7863
https://issues.apache.org/jira/browse/MESOS-7783
https://issues.apache.org/jira/browse/MESOS-7863


Repository: mesos


Description
---

This test ensures that the agent does not consider a framework
with killed pending tasks as an idle framework. Previously the
agent would remove them and incorrectly consider the framework
idle during '__run', which resulted in dropped TASK_KILLED
status updates, see MESOS-7863.


Diffs
-

  src/tests/slave_tests.cpp 1d9d142ed9e801b79535a2c28f5a94ffbf1bf160 


Diff: https://reviews.apache.org/r/61446/diff/1/


Testing
---

Ran in repetition


Thanks,

Benjamin Mahler

Review Request 61445: Fixed a bug in the agent where a kill task is dropped.

[Benjamin Mahler]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Currently there is an assumption that when a pending task is killed,
the framework will still be stored in the agent. However, this
assumption can be violated in two cases:

  (1) Another pending task was killed and we removed the framework
  in 'Slave::run' thinking it was idle, because pending tasks
  were empty (we remove from pending tasks when processing the
  kill).

  (2) The last executor terminated without tasks to send terminal
  updates for, or the last terminated executor received its
  last acknowledgement. At this point, we remove the framework
  thinking there were no pending tasks if the task was killed
  (removed from pending).

The fix is to leave tasks as pending but mark that they have been
killed. This fixes both cases.


Diffs
-

  src/slave/slave.hpp 1fe93dab1b2bef24721cc1bcffebe1b259e96d79 
  src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 


Diff: https://reviews.apache.org/r/61445/diff/1/


Testing
---

make check, added a test in the subsequent patch


Thanks,

Benjamin Mahler

Review Request 61444: Renamed Framework::pending to Framework::pendingTasks in agent.

[Benjamin Mahler]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/slave/http.cpp 2d33f0b498c8c819d1aaa6b39ae38b1009fda3e4 
  src/slave/slave.hpp 1fe93dab1b2bef24721cc1bcffebe1b259e96d79 
  src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 


Diff: https://reviews.apache.org/r/61444/diff/1/


Testing
---

make check


Thanks,

Benjamin Mahler

Review Request 61441: Updated Framework::removePendingTask to take only a TaskID.

[Benjamin Mahler]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


Repository: mesos


Description
---

This allows the function to be used in the kill task path where we
do not have the TaskInfo or ExecutorInfo available. With this, we
now have all removals going through this function.


Diffs
-

  src/slave/slave.hpp 1fe93dab1b2bef24721cc1bcffebe1b259e96d79 
  src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 


Diff: https://reviews.apache.org/r/61441/diff/1/


Testing
---

make check


Thanks,

Benjamin Mahler

Review Request 61440: Added a TODO to correctly represent framework lifecycle in agent.

[Benjamin Mahler]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


Repository: mesos


Description
---

Currently, we store frameworks as "completed" in the agent when
the agent no longer has any active tasks for executors for the
framework. However, the framework may not be completed according
to the master. Ideally, the master informs the agent when a
framework is _actually_ completed and the agent represents these
"idle" frameworks as being in a non-completed state.


Diffs
-

  src/slave/slave.hpp 1fe93dab1b2bef24721cc1bcffebe1b259e96d79 


Diff: https://reviews.apache.org/r/61440/diff/1/


Testing
---

N/A


Thanks,

Benjamin Mahler

Review Request 61443: Marked Framework::hasTask in agent as const.

[Benjamin Mahler]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/slave/slave.hpp 1fe93dab1b2bef24721cc1bcffebe1b259e96d79 
  src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 


Diff: https://reviews.apache.org/r/61443/diff/1/


Testing
---

make check


Thanks,

Benjamin Mahler

Review Request 61442: Introduced a Framework::idle function in the agent.

[Benjamin Mahler]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


Repository: mesos


Description
---

This ensures the call-sites consistently check idleness of the
framework, it also aids readability in that it clarifies that
we remove idle frameworks.


Diffs
-

  src/slave/slave.hpp 1fe93dab1b2bef24721cc1bcffebe1b259e96d79 
  src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 


Diff: https://reviews.apache.org/r/61442/diff/1/


Testing
---

make check


Thanks,

Benjamin Mahler

Re: Review Request 61096: Building gRPC with CMake.

[Chun-Hung Hsiao]

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

(Updated Aug. 5, 2017, 2:08 a.m.)


Review request for mesos, Andrew Schwartzmeyer, Jie Yu, and Joseph Wu.


Changes
---

Fixed building issue on OSX (due to non-standard openssl path).


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


Repository: mesos


Description (updated)
---

This patch is based on @andschwa's branch:
https://github.com/andschwa/mesos/commits/cmake-refactor-andy
Related patches: r/61365.

This patch enables building the bundled gRPC library for libprocess
under Linux.

A patch for gRPC is generated from cherry-picking the following commits
for gRPC's bundled c-ares and openssl library linkage issue:
https807693bd89832a75d6678a94226f4f6276fe2343
https://github.com/grpc/grpc/pull/11565
https://github.com/grpc/grpc/pull/12091

TODO(chhsiao): Windows support.


Diffs (updated)
-

  3rdparty/CMakeLists.txt f4feaf90b4750338cfa2127524a5e6556a86d063 
  3rdparty/cmake/Versions.cmake 9586296420d76bd4493034fd710209008ee03595 
  3rdparty/grpc-1.4.2.patch PRE-CREATION 


Diff: https://reviews.apache.org/r/61096/diff/4/

Changes: https://reviews.apache.org/r/61096/diff/3-4/


Testing
---

make


Thanks,

Chun-Hung Hsiao

Re: Review Request 61270: Added container PID namespace control protobuf field in LinuxInfo.

[Gastón Kleiman]

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




include/mesos/mesos.proto
Lines 2741-2746 (patched)


I'd put something like this:

```
If set, then the container will share the PID namespace with its parent. If 
the container is a top-level container, then it will share the PID namespace 
with the agent. If it is a nested container, it will shar the PID namespace 
with its parent container. This field will be ignored if the `namespaces/pid` 
isolator is not enabled.
```


- Gastón Kleiman


On Aug. 3, 2017, 9:40 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61270/
> ---
> 
> (Updated Aug. 3, 2017, 9:40 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gastón Kleiman, Jie Yu, Kevin 
> Klues, Qian Zhang, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> User facing interface for container configurable PID namespace.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
>   include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
> 
> 
> Diff: https://reviews.apache.org/r/61270/diff/3/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 61406: Introduced `--disallow_top_level_pid_ns_sharing` agent flag.

[Gastón Kleiman]

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


Fix it, then Ship it!





docs/configuration.md
Lines 2052-2054 (patched)


s/pid namespace/PID namespace/g



docs/configuration.md
Lines 2054 (patched)


s/ignored if `namespaces/pid`/ignored if the `namespaces/pid`/



src/slave/flags.cpp
Lines 453-458 (patched)


Ditto.


- Gastón Kleiman


On Aug. 4, 2017, 4 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61406/
> ---
> 
> (Updated Aug. 4, 2017, 4 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced `--disallow_top_level_pid_ns_sharing` agent flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 041c3dfb9c0c1718770f74dfb33a9f5d6fbe9b61 
>   src/slave/flags.hpp 032880dfa68cd29420e559d34e592e57827cfc07 
>   src/slave/flags.cpp 4171604090ffebe79fa35579458cabff7270c0de 
> 
> 
> Diff: https://reviews.apache.org/r/61406/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61428: Added pid ns sharing based on agent flag and protobuf message field.

[Gilbert Song]

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




src/slave/containerizer/mesos/isolators/namespaces/pid.cpp
Lines 85-86 (patched)


I know in proto2 and proto3 `boolean` defaults to be `false`. But how about 
`optional bool`? do you know if `has_share_pid_namespace` can be false?


- Gilbert Song


On Aug. 4, 2017, 9:39 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61428/
> ---
> 
> (Updated Aug. 4, 2017, 9:39 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added pid ns sharing based on agent flag and protobuf message field.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
> 2b316dbdf4a3735771af5bed80c6251d0d1cbd50 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
> f1dfc9f7398ffc029d7180d7f014a515338cb3f4 
> 
> 
> Diff: https://reviews.apache.org/r/61428/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61428: Added pid ns sharing based on agent flag and protobuf message field.

[Gilbert Song]

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




src/slave/containerizer/mesos/isolators/namespaces/pid.cpp
Lines 70 (patched)


one more space before {



src/slave/containerizer/mesos/isolators/namespaces/pid.cpp
Lines 130 (patched)


Could we reverse two logics above? so that we can avoid the size check 
here. E.g.,
```
if (sharePidNamespace) {
  return launchInfo;
}
```

similar to the short circuit logic for DEBUG container.



src/slave/containerizer/mesos/isolators/namespaces/pid.cpp
Lines 115-117 (original), 135-140 (patched)


We can just return launchInfo, right?


- Gilbert Song


On Aug. 4, 2017, 9:39 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61428/
> ---
> 
> (Updated Aug. 4, 2017, 9:39 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added pid ns sharing based on agent flag and protobuf message field.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
> 2b316dbdf4a3735771af5bed80c6251d0d1cbd50 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
> f1dfc9f7398ffc029d7180d7f014a515338cb3f4 
> 
> 
> Diff: https://reviews.apache.org/r/61428/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 61438: Improved `NvidiaGpuTest.ROOT_CGROUPS_NVIDIA_GPU_VerifyDeviceAccess`.

[Gastón Kleiman]

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

Review request for mesos, Kevin Klues and Vinod Kone.


Repository: mesos


Description
---

Change the test so that the agent offesr as many GPUs as available on
the box instead of restricting it to 1. This way the test will fail if
there's a bug that makes the isolator give a task access to more GPUs
than what it was allocated.


Diffs
-

  src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
9a78ae65c1cd414b5093b54ff51724e31e31c9d3 


Diff: https://reviews.apache.org/r/61438/diff/1/


Testing
---

`GLOG_v=1 sudo bin/mesos-tests.sh --gtest_filter="*NvidiaGpuTest.*Verify*" 
--verbose` passed on a machine with 4 Nvidia GPUs.


Thanks,

Gastón Kleiman

Re: Review Request 61282: Added a test verifying that DefaultExecutor tasks can use nvidia GPUs.

[Gastón Kleiman]

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

(Updated Aug. 5, 2017, 12:05 a.m.)


Review request for mesos, Kevin Klues and Vinod Kone.


Changes
---

Made the agent offer all GPUs.


Repository: mesos


Description
---

Added a test verifying that DefaultExecutor tasks can use nvidia GPUs.


Diffs (updated)
-

  src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
9a78ae65c1cd414b5093b54ff51724e31e31c9d3 


Diff: https://reviews.apache.org/r/61282/diff/3/

Changes: https://reviews.apache.org/r/61282/diff/2-3/


Testing
---

`GLOG_v=1 sudo bin/mesos-tests.sh --gtest_filter="*NvidiaGpuTest.*Default*" 
--verbose` passed on a machine with an Nvidia GPU.


Thanks,

Gastón Kleiman

Re: Review Request 58898: Send task kill for non-Partition Aware frameworks.

[Megha Sharma]

> On May 10, 2017, 11:24 p.m., Neil Conway wrote:
> > My apologies for the delay in reviewing this.
> > 
> > High-level comments:
> > 
> > (a) Can we improve the description of the problem in the commit summary? It 
> > took me quite a while to figure out what is actually going on here. My 
> > understanding is:
> > 
> > (1) Agent re-registers
> > (2) Master sends `ShutdownFrameworkMessage` for non-PA frameworks on the 
> > agent
> > (3) Master offers agent resources to framework
> > (4) Framework launches new task on agent _before the agent has finished 
> > shutting down the framework_
> > (5) Agent ignores launch task because it believes the framework is still 
> > terminating.
> > 
> > This is a race condition, because if the agent finished shutting down the 
> > framework (in #4) before the launch task was received, there would not be a 
> > problem. Is my understanding correct?
> > 
> > (2) I'd prefer a new unit test that constructs exactly this scenario, 
> > rather than changing existing unit tests.
> > 
> > (3) The new behavior is that the framework will receive `TASK_KILLED` for 
> > any non-PA tasks running on a partitioned agent that re-registers. This 
> > does not seem ideal, because `TASK_KILLED` _normally_ corresponds to a 
> > framework-initiated `killTask` operation.
> > 
> > (4) Thinking out loud, what if a custom executor ignores the `killTask` 
> > request? When shutting down a framework, it seems we forcibly destroy the 
> > container (via `containerizer->destroy()`), if the executor doesn't exit 
> > promptly upon receiving the framework shutdown request. AFAIK we don't have 
> > similar logic for `killTask` requests.
> > 
> > 3 + 4 suggests that maybe we want a different way to terminate the task on 
> > the agent -- let's discuss.
> 
> Megha Sharma wrote:
> Summarizing the 2 approaches we talked about.
> 
> Approach 1: ShutdownFrameworkMessage
> 
> 1. Upon agent re-registration, master will add tasks even for non-PA 
> frameworks on this agent. This is needed by the master to do correct resource 
> accounting and not offer resources already in use on this agent. We need to 
> mutate the TaskState on the Task before adding them to the master's data 
> structures since the TaskState might be non-terminal when the agent sends 
> these tasks with ReregisterSlaveMessage. And the master has already sent 
> TASK_LOST for these tasks to the frameworks so we need to set the TaskState 
> to TASK_LOST so that any future reconciliations with the framework doesn't 
> have this task transitioning from TASK_LOST to TASK_RUNNNG/TASK_STARTING. 
> This is to avoid unnecessary confusion about task state as observed by the 
> framework but indeed this could have happened with non-strict registry as 
> well where the framework can actually receive a non terminal task state 
> update after receiving a TASK_LOST for the same task in the past.
> 
> 2. When the agent re-registers, the master will continue to send a 
> ShutdownFrameworkMessage to the agent to kill the tasks pertaining to non-PA 
> frameworks on the agent as it does today. An additional optional field will 
> be added to the ShutdownFrameworkMessage to indicate whether or not the 
> shutdown was initiated internally.
> 
> 3. During framework shutdown the state of the framework is set to 
> Framework::TERMINATING which prevents it from launching new tasks. Here, 
> since the framework is not really terminating so in order to allow it to 
> launch new tasks, the agent will not set the state to terminating if the 
> ShutdownFrameworkMessage is generated internally.
> 
> 4. The framework shutdown today doesn't generate any status updates which 
> needs to change. The status updates will be sent if the framework shutdown is 
> triggered internally, this is needed to remove the tasks of non-PA frameworks 
> that got added when the agent re-registered (1).
> 
> Approach 2: Do not shutdown non-PA framework when agent re-registers and 
> let the frameworks make the decision on what needs to be done when they 
> receive non-terminal status updates for tasks for which they have already 
> received a TASK_LOST. This hopefully won't break any frameworks since it 
> could have happened in the past with non-strict registry as well and 
> frameworks should be resilient enough to handle this scenario.
> 
> Let me know if I have missed anything. Personally, I am in favor of 
> approach 1 as it seems less catastrophic for the frameworks. What do you 
> think?

The proposed solution to fix the race between new task launches and shutdown 
framework on the agent, was to not kill the non-partition aware tasks when an 
unreachable agent re-registers with the master. So now as far as Mesos 
internals are concerned, the tasks from non-partition aware frameworks are to 
be treated the same way as partition aware tasks and one way to do it cleanly 
in Mesos was to transition such non-partition aware tasks 

Re: Review Request 61118: Building gRPC support in libprocess with CMake.

[Chun-Hung Hsiao]

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

(Updated Aug. 4, 2017, 11:21 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Jie Yu, and Joseph Wu.


Changes
---

Renamed `grpc_libs` to just `grpc`.


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


Repository: mesos


Description
---

This patch enables CMake to build code for gRPC support in libprocess
along with tests under Linux.

TODO(chhsiao): gRPC support for Windows.


Diffs (updated)
-

  3rdparty/libprocess/src/CMakeLists.txt 
f97291bd7cadcb440231619a8a2d1029a447ec27 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
27451c275b1f5a2eb7ada78f8cbe4b7c2c63ca92 


Diff: https://reviews.apache.org/r/61118/diff/3/

Changes: https://reviews.apache.org/r/61118/diff/2-3/


Testing
---

make
sudo make check
cd 3rdparty/libprocess/src/tests && make libprocess-tests && ./libprocess-tests


Thanks,

Chun-Hung Hsiao

Re: Review Request 61096: Building gRPC with CMake.

[Chun-Hung Hsiao]

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

(Updated Aug. 4, 2017, 11:20 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Jie Yu, and Joseph Wu.


Changes
---

Exported `libprotoc` and renamed `grpc_libs` to simply `grpc`.


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


Repository: mesos


Description
---

This patch is based on @andschwa's branch:
https://github.com/andschwa/mesos/commits/cmake-refactor-andy
Related patches: r/61365.

This patch enables building the bundled gRPC library for libprocess
under Linux.

A patch for gRPC's CMake config is introduced for gRPC's bundled c-ares
library linkage issue: https://github.com/grpc/grpc/pull/11565.

TODO(chhsiao): Windows support.


Diffs (updated)
-

  3rdparty/CMakeLists.txt f4feaf90b4750338cfa2127524a5e6556a86d063 
  3rdparty/cmake/Versions.cmake 9586296420d76bd4493034fd710209008ee03595 
  3rdparty/grpc-1.4.2.patch PRE-CREATION 


Diff: https://reviews.apache.org/r/61096/diff/3/

Changes: https://reviews.apache.org/r/61096/diff/2-3/


Testing
---

make


Thanks,

Chun-Hung Hsiao

Re: Review Request 61394: Made the capabilities isolator work with nested containers.

[Gastón Kleiman]

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

(Updated Aug. 4, 2017, 10:33 p.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, and James Peach.


Changes
---

Added missing line break.


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


Repository: mesos


Description
---

Make the capabilities isolator support nesting. Without this patch
capabilities sets for tasks launched by the DefaultExecutor are silently
ignored.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/linux/capabilities.hpp 
c3afe2054bb206e9b2823c008435f78e70ce7d63 
  src/slave/containerizer/mesos/isolators/linux/capabilities.cpp 
f85a84d3871307bd1e9ebd45388ba680534acd89 
  src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
80508760f8d635b414651e521848315399918fbc 


Diff: https://reviews.apache.org/r/61394/diff/4/

Changes: https://reviews.apache.org/r/61394/diff/3-4/


Testing
---

`sudo GLOG_v=1 bin/mesos-tests.sh 
--gtest_filter="*LinuxCapabilitiesIsolatorTest*"`


Thanks,

Gastón Kleiman

Re: Review Request 61282: Added a test verifying that DefaultExecutor tasks can use nvidia GPUs.

[Gastón Kleiman]

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

(Updated Aug. 4, 2017, 10:30 p.m.)


Review request for mesos, Kevin Klues and Vinod Kone.


Changes
---

Moved the test to `nvidia_gpu_isolator_tests.cpp` and ported it to the v0 API.


Repository: mesos


Description
---

Added a test verifying that DefaultExecutor tasks can use nvidia GPUs.


Diffs (updated)
-

  src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
9a78ae65c1cd414b5093b54ff51724e31e31c9d3 


Diff: https://reviews.apache.org/r/61282/diff/2/

Changes: https://reviews.apache.org/r/61282/diff/1-2/


Testing
---

`sudo bin/mesos-tests.sh 
--gtest_filter="DefaultExecutorTest.ROOT_CGROUPS_NVIDIA_GPU_VerifyGPUDeviceAccess"
 --gtest_repeat=3000 --gtest_break_on_failure --verbose` passed on a machine 
with an Nvidia GPU.


Thanks,

Gastón Kleiman

Re: Review Request 61394: Made the capabilities isolator work with nested containers.

[Jie Yu]

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


Fix it, then Ship it!





src/tests/containerizer/linux_capabilities_isolator_tests.cpp
Lines 476 (patched)


2 lines apart.


- Jie Yu


On Aug. 4, 2017, 9:57 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61394/
> ---
> 
> (Updated Aug. 4, 2017, 9:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and James Peach.
> 
> 
> Bugs: MESOS-7849
> https://issues.apache.org/jira/browse/MESOS-7849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Make the capabilities isolator support nesting. Without this patch
> capabilities sets for tasks launched by the DefaultExecutor are silently
> ignored.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp 
> c3afe2054bb206e9b2823c008435f78e70ce7d63 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp 
> f85a84d3871307bd1e9ebd45388ba680534acd89 
>   src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
> 80508760f8d635b414651e521848315399918fbc 
> 
> 
> Diff: https://reviews.apache.org/r/61394/diff/3/
> 
> 
> Testing
> ---
> 
> `sudo GLOG_v=1 bin/mesos-tests.sh 
> --gtest_filter="*LinuxCapabilitiesIsolatorTest*"`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61387: Made the rlimits isolator work with nested containers.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Aug. 4, 2017, 9:15 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61387/
> ---
> 
> (Updated Aug. 4, 2017, 9:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7849
> https://issues.apache.org/jira/browse/MESOS-7849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Make the rlimits isolator support nesting. Without this patch rlimits
> sets for tasks launched by the DefaultExecutor are silently ignored.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/posix/rlimits.hpp 
> ee36a24ae36179d3ff3141f8c3cdc768b1e399af 
>   src/slave/containerizer/mesos/isolators/posix/rlimits.cpp 
> 3fc2b3dbd5b382e422c948adae0dc50f4fbcfc49 
>   src/tests/containerizer/posix_rlimits_isolator_tests.cpp 
> b7ccc7eeeb7e4d0d8f4fd463d506cfe7157076a4 
> 
> 
> Diff: https://reviews.apache.org/r/61387/diff/4/
> 
> 
> Testing
> ---
> 
> `bin/mesos-tests.sh 
> --gtest_filter="PosixRLimitsIsolatorTest.NestedContainers" 
> --gtest_repeat=1000 --gtest_break_on_failure` passed on a machine running 
> GNU/Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61394: Made the capabilities isolator work with nested containers.

[Gastón Kleiman]

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

(Updated Aug. 4, 2017, 9:57 p.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, and James Peach.


Changes
---

Fixed style.


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


Repository: mesos


Description
---

Make the capabilities isolator support nesting. Without this patch
capabilities sets for tasks launched by the DefaultExecutor are silently
ignored.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/linux/capabilities.hpp 
c3afe2054bb206e9b2823c008435f78e70ce7d63 
  src/slave/containerizer/mesos/isolators/linux/capabilities.cpp 
f85a84d3871307bd1e9ebd45388ba680534acd89 
  src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
80508760f8d635b414651e521848315399918fbc 


Diff: https://reviews.apache.org/r/61394/diff/3/

Changes: https://reviews.apache.org/r/61394/diff/2-3/


Testing
---

`sudo GLOG_v=1 bin/mesos-tests.sh 
--gtest_filter="*LinuxCapabilitiesIsolatorTest*"`


Thanks,

Gastón Kleiman

Re: Review Request 61387: Made the rlimits isolator work with nested containers.

[Gastón Kleiman]

> On Aug. 4, 2017, 8:58 a.m., Benjamin Bannier wrote:
> > src/tests/containerizer/posix_rlimits_isolator_tests.cpp
> > Lines 380-381 (patched)
> > 
> >
> > Indent by two spaces less.

The pattern in this file is to use 4 spaces, see the following tests (some of 
them added by you):

https://github.com/apache/mesos/blob/cdb90b91ce8ce02d6163e5e2ee5b46fb797b1dee/src/tests/containerizer/posix_rlimits_isolator_tests.cpp#L70-L73
https://github.com/apache/mesos/blob/cdb90b91ce8ce02d6163e5e2ee5b46fb797b1dee/src/tests/containerizer/posix_rlimits_isolator_tests.cpp#L147-L149
https://github.com/apache/mesos/blob/cdb90b91ce8ce02d6163e5e2ee5b46fb797b1dee/src/tests/containerizer/posix_rlimits_isolator_tests.cpp#L227-L229
https://github.com/apache/mesos/blob/cdb90b91ce8ce02d6163e5e2ee5b46fb797b1dee/src/tests/containerizer/posix_rlimits_isolator_tests.cpp#L294-L296

clang-format suggests two spaces, and I agree with you both. So I'm thinking we 
should probably keep 4 spaces here and then have a different patch cleaning up 
the whitespaces in all these tests?


> On Aug. 4, 2017, 8:58 a.m., Benjamin Bannier wrote:
> > src/tests/containerizer/posix_rlimits_isolator_tests.cpp
> > Lines 388 (patched)
> > 
> >
> > `ASSERT_FALSE(offers->empty())`.

I agree that your proposal is more readable. Yet the other tests in this file 
and a LOT of other tests use `ASSERT_NE(0u, offers->size());`.

I'm thinking again that we should keep it like this in this patch and then do a 
sweeping change?


> On Aug. 4, 2017, 8:58 a.m., Benjamin Bannier wrote:
> > src/tests/containerizer/posix_rlimits_isolator_tests.cpp
> > Lines 392-396 (patched)
> > 
> >
> > Currently the declaration and first use are not on the same page for 
> > me. Let's move this right above its use,
> >  
> > driver.acceptOffers({offer.id()}, {LAUNCH_GROUP(executorInfo, 
> > taskGroup)})

Moved this down. Now the `taskStatuses` declaration and expectations are kind 
of away from their use, but I think that's ok.


> On Aug. 4, 2017, 8:58 a.m., Benjamin Bannier wrote:
> > src/tests/containerizer/posix_rlimits_isolator_tests.cpp
> > Lines 449 (patched)
> > 
> >
> > It would be nice to avoid hardcoding `4` here, e.g.,
> > 
> > foreach (Future update, updates) {
> >   EXPECT_CALL(sched, statusUpdate(, _))
> > .WillOnce(FutureArg<1>());
> > }
> 
> Benjamin Bannier wrote:
> Dropped a ref here,
> 
> foreach (Future& update, updates) {
>   EXPECT_CALL(sched, statusUpdate(, _))
> .WillOnce(FutureArg<1>());
> }

Done (but with 2 spaces to stay consistent with the rest of the file).


> On Aug. 4, 2017, 8:58 a.m., Benjamin Bannier wrote:
> > src/tests/containerizer/posix_rlimits_isolator_tests.cpp
> > Lines 490 (patched)
> > 
> >
> > `s/updates[1]/taskStatus/`.

Good catch =).


- Gastón


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


On Aug. 4, 2017, 9:15 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61387/
> ---
> 
> (Updated Aug. 4, 2017, 9:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7849
> https://issues.apache.org/jira/browse/MESOS-7849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Make the rlimits isolator support nesting. Without this patch rlimits
> sets for tasks launched by the DefaultExecutor are silently ignored.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/posix/rlimits.hpp 
> ee36a24ae36179d3ff3141f8c3cdc768b1e399af 
>   src/slave/containerizer/mesos/isolators/posix/rlimits.cpp 
> 3fc2b3dbd5b382e422c948adae0dc50f4fbcfc49 
>   src/tests/containerizer/posix_rlimits_isolator_tests.cpp 
> b7ccc7eeeb7e4d0d8f4fd463d506cfe7157076a4 
> 
> 
> Diff: https://reviews.apache.org/r/61387/diff/4/
> 
> 
> Testing
> ---
> 
> `bin/mesos-tests.sh 
> --gtest_filter="PosixRLimitsIsolatorTest.NestedContainers" 
> --gtest_repeat=1000 --gtest_break_on_failure` passed on a machine running 
> GNU/Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61387: Made the rlimits isolator work with nested containers.

[Gastón Kleiman]

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

(Updated Aug. 4, 2017, 9:15 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Addressed feedback.


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


Repository: mesos


Description
---

Make the rlimits isolator support nesting. Without this patch rlimits
sets for tasks launched by the DefaultExecutor are silently ignored.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/posix/rlimits.hpp 
ee36a24ae36179d3ff3141f8c3cdc768b1e399af 
  src/slave/containerizer/mesos/isolators/posix/rlimits.cpp 
3fc2b3dbd5b382e422c948adae0dc50f4fbcfc49 
  src/tests/containerizer/posix_rlimits_isolator_tests.cpp 
b7ccc7eeeb7e4d0d8f4fd463d506cfe7157076a4 


Diff: https://reviews.apache.org/r/61387/diff/3/

Changes: https://reviews.apache.org/r/61387/diff/2-3/


Testing
---

`bin/mesos-tests.sh --gtest_filter="PosixRLimitsIsolatorTest.NestedContainers" 
--gtest_repeat=1000 --gtest_break_on_failure` passed on a machine running 
GNU/Linux.


Thanks,

Gastón Kleiman

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Vinod Kone]

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


Fix it, then Ship it!




This is great!


docs/health-checks.md
Lines 110 (patched)


s/1./2./ ? or is this markdown style?



docs/health-checks.md
Lines 118 (patched)


/transitions./transitions and make global decisions/



docs/health-checks.md
Lines 136 (patched)


I know you mentioned about check de-duplication above, but worthwhile 
mentioning it here as well to contrast.



docs/health-checks.md
Lines 138 (patched)


Docker executor currently supports health checks but not checks.



docs/health-checks.md
Lines 141 (patched)


s/reasons,/reasons;/



docs/health-checks.md
Lines 162 (patched)


s/adheres/adheres to/



docs/health-checks.md
Lines 209 (patched)


Is it 127.0.0.1 even in the CNI network case? cc @avinash


- Vinod Kone


On Aug. 4, 2017, 6:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 4, 2017, 6:14 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/1/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 60440: Updated local development workflow of mesos website.

[Vinod Kone]

> On Aug. 2, 2017, 9:11 a.m., Benjamin Bannier wrote:
> > While I see the value of being consistent with the CI bot's setup, I still 
> > strongly believe that we should remove this dev setup in favor of a simple 
> > `rake`-driven workflow (like we already discussed offline). I guess the 
> > most work there would be in updating the documentation.
> > 
> > Could you please either follow up with that cleanup now, or create a ticket?

Filed MESOS-7860.


- Vinod


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


On July 27, 2017, 12:30 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60440/
> ---
> 
> (Updated July 27, 2017, 12:30 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-7625
> https://issues.apache.org/jira/browse/MESOS-7625
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the layout and scripts consistent with CI based automatic
> publishing of the website.
> 
> 
> Diffs
> -
> 
>   site/Dockerfile 230cfc779fe4f183d63cd99ef26dc540c68bff85 
>   site/README.md ebd3e6a0fea7ae0fe3b28719bcab28ee8f7c356c 
>   site/build.sh 06b1b32a5cdfaf2f9a69ce59339e0fd671e335de 
>   site/entrypoint.sh PRE-CREATION 
>   site/mesos-website-dev.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60440/diff/2/
> 
> 
> Testing
> ---
> 
> Tested by running the script locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 60440: Updated local development workflow of mesos website.

[Vinod Kone]

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

(Updated Aug. 4, 2017, 7:01 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, and haosdent huang.


Changes
---

addressed comments.


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


Repository: mesos


Description
---

Made the layout and scripts consistent with CI based automatic
publishing of the website.


Diffs (updated)
-

  site/Dockerfile 230cfc779fe4f183d63cd99ef26dc540c68bff85 
  site/README.md ebd3e6a0fea7ae0fe3b28719bcab28ee8f7c356c 
  site/build.sh 06b1b32a5cdfaf2f9a69ce59339e0fd671e335de 
  site/entrypoint.sh PRE-CREATION 
  site/mesos-website-dev.sh PRE-CREATION 


Diff: https://reviews.apache.org/r/60440/diff/3/

Changes: https://reviews.apache.org/r/60440/diff/2-3/


Testing
---

Tested by running the script locally.


Thanks,

Vinod Kone

Review Request 61433: Checked openssl and zlib as required libraries.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

This patch is for grpc support in Mesos. Since grpc depends on protobuf,
ssl and zlib, we make ssl and zlib required packages for Mesos to build.
Despite of the requirement changes, The the `--enable-ssl` and
`--enable-zlib` flags are still kept, since these flags control whether
we want to compress and encrypt the communications.

We also pass the ssl lib path as an `-rpath` option to the linker so the
`AC_RUN_IFELSE` tests won't fail.


Diffs
-

  configure.ac b2eeedab65d70718451cb8bbe556794b6d8e8db6 


Diff: https://reviews.apache.org/r/61433/diff/1/


Testing
---

make


Thanks,

Chun-Hung Hsiao

Re: Review Request 61406: Introduced `--disallow_top_level_pid_ns_sharing` agent flag.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 4, 2017, 9 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61406/
> ---
> 
> (Updated Aug. 4, 2017, 9 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced `--disallow_top_level_pid_ns_sharing` agent flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 041c3dfb9c0c1718770f74dfb33a9f5d6fbe9b61 
>   src/slave/flags.hpp 032880dfa68cd29420e559d34e592e57827cfc07 
>   src/slave/flags.cpp 4171604090ffebe79fa35579458cabff7270c0de 
> 
> 
> Diff: https://reviews.apache.org/r/61406/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 61435: Added logging in docker executor on `docker stop` failure.

[Andrei Budnik]

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

Review request for mesos and Alexander Rukletsov.


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


Repository: mesos


Description
---

Added logging in docker executor on `docker stop` failure.


Diffs
-

  src/docker/executor.cpp 26f12ec002f754fab0d34c01472cf95b499d8007 


Diff: https://reviews.apache.org/r/61435/diff/1/


Testing
---

Manual testing steps:
1. force docker daemon to always return error: 
https://github.com/docker/docker-ce/blob/master/components/engine/daemon/stop.go#L21
2. build docker from sources and run it
3. run master, agent
4. launch a task using docker executor via mesos-execute in terminal
5. send SIGINT to the launched mesos-execute
6. check stderr logs of an executor in its sandbox


Thanks,

Andrei Budnik

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

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

(Updated Aug. 4, 2017, 6:09 p.m.)


Review request for mesos, Gastón Kleiman and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 


Diff: https://reviews.apache.org/r/61434/diff/1/


Testing
---

https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded

Additionally rendered in MacDown.


Thanks,

Alexander Rukletsov

Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

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

Review request for mesos, Gastón Kleiman and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 


Diff: https://reviews.apache.org/r/61434/diff/1/


Testing
---

https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded

Additionally rendered in MacDown.


Thanks,

Alexander Rukletsov

Re: Review Request 60439: Added scripts to automate website publishing.

[Benjamin Bannier]

> On Aug. 2, 2017, 10:59 a.m., Benjamin Bannier wrote:
> > support/mesos-website/entrypoint.sh
> > Lines 23 (patched)
> > 
> >
> > Do you still recall why this was needed? It would be great to add e.g., 
> > a JIRA or a small reproducer to eval in the future whether this is still 
> > needed.
> 
> Vinod Kone wrote:
> Filed MESOS-7859

Thanks. Let's maybe add some more information how to reproduce that issue to 
the ticket; this e.g., works outside of the Docker container on macos.


- Benjamin


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


On Aug. 4, 2017, 8:36 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60439/
> ---
> 
> (Updated Aug. 4, 2017, 8:36 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-7625
> https://issues.apache.org/jira/browse/MESOS-7625
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These scripts are expected to be run by ASF CI on any commit to
> the mesos repo. The directory layout is similar to tidybot CI job.
> 
> 
> Diffs
> -
> 
>   support/jenkins/websitebot.sh PRE-CREATION 
>   support/mesos-website.sh PRE-CREATION 
>   support/mesos-website/Dockerfile PRE-CREATION 
>   support/mesos-website/build.sh PRE-CREATION 
>   support/mesos-website/entrypoint.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60439/diff/3/
> 
> 
> Testing
> ---
> 
> Tested by running a CI job pointing to a branch containing this patch.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 60439: Added scripts to automate website publishing.

[Benjamin Bannier]

> On Aug. 2, 2017, 10:52 a.m., Benjamin Bannier wrote:
> > support/mesos-website/build.sh
> > Lines 22 (patched)
> > 
> >
> > This script will misbehave if run from a live dev checkout (it might 
> > e.g., reconfigure an already configued setup, remove an already generated 
> > site, rebuild the whole project if the developer builds in `build/`, or 
> > pick up changes from a dirty tree so the SHA does not correspond to the 
> > contents).
> > 
> > I'd suggest to add a safeguard against that. One option would be to 
> > refuse to run if the tree contains any untracked files or uncommitted 
> > changes, e.g., with
> > 
> > if [ "$(git status --porcelain | wc -l)" -ne 0 ]; then
> >   echo "This script is not intended to run on a live checkout, but 
> > the source tree contains untracked files or uncommitted changes."
> >   
> >   exit 1
> > fi
> > 
> > We should add this before setting up the the exit trap.
> 
> Vinod Kone wrote:
> I put this check in "support/jenkins/websitebot.sh" to fail early.

Great!


- Benjamin


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


On Aug. 4, 2017, 8:36 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60439/
> ---
> 
> (Updated Aug. 4, 2017, 8:36 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-7625
> https://issues.apache.org/jira/browse/MESOS-7625
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These scripts are expected to be run by ASF CI on any commit to
> the mesos repo. The directory layout is similar to tidybot CI job.
> 
> 
> Diffs
> -
> 
>   support/jenkins/websitebot.sh PRE-CREATION 
>   support/mesos-website.sh PRE-CREATION 
>   support/mesos-website/Dockerfile PRE-CREATION 
>   support/mesos-website/build.sh PRE-CREATION 
>   support/mesos-website/entrypoint.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60439/diff/3/
> 
> 
> Testing
> ---
> 
> Tested by running a CI job pointing to a branch containing this patch.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 60439: Added scripts to automate website publishing.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On Aug. 4, 2017, 8:36 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60439/
> ---
> 
> (Updated Aug. 4, 2017, 8:36 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-7625
> https://issues.apache.org/jira/browse/MESOS-7625
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These scripts are expected to be run by ASF CI on any commit to
> the mesos repo. The directory layout is similar to tidybot CI job.
> 
> 
> Diffs
> -
> 
>   support/jenkins/websitebot.sh PRE-CREATION 
>   support/mesos-website.sh PRE-CREATION 
>   support/mesos-website/Dockerfile PRE-CREATION 
>   support/mesos-website/build.sh PRE-CREATION 
>   support/mesos-website/entrypoint.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60439/diff/3/
> 
> 
> Testing
> ---
> 
> Tested by running a CI job pointing to a branch containing this patch.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 61428: Added pid ns sharing based on agent flag and protobuf message field.

[Qian Zhang]

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

(Updated Aug. 5, 2017, 12:39 a.m.)


Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
and Vinod Kone.


Changes
---

Minor changes.


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


Repository: mesos


Description
---

Added pid ns sharing based on agent flag and protobuf message field.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
2b316dbdf4a3735771af5bed80c6251d0d1cbd50 
  src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
f1dfc9f7398ffc029d7180d7f014a515338cb3f4 


Diff: https://reviews.apache.org/r/61428/diff/2/

Changes: https://reviews.apache.org/r/61428/diff/1-2/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 61387: Made the rlimits isolator work with nested containers.

[Benjamin Bannier]

> On Aug. 4, 2017, 10:58 a.m., Benjamin Bannier wrote:
> > src/tests/containerizer/posix_rlimits_isolator_tests.cpp
> > Lines 449 (patched)
> > 
> >
> > It would be nice to avoid hardcoding `4` here, e.g.,
> > 
> > foreach (Future update, updates) {
> >   EXPECT_CALL(sched, statusUpdate(, _))
> > .WillOnce(FutureArg<1>());
> > }

Dropped a ref here,

foreach (Future& update, updates) {
  EXPECT_CALL(sched, statusUpdate(, _))
.WillOnce(FutureArg<1>());
}


- Benjamin


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


On Aug. 4, 2017, 12:10 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61387/
> ---
> 
> (Updated Aug. 4, 2017, 12:10 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7849
> https://issues.apache.org/jira/browse/MESOS-7849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Make the rlimits isolator support nesting. Without this patch rlimits
> sets for tasks launched by the DefaultExecutor are silently ignored.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/posix/rlimits.hpp 
> ee36a24ae36179d3ff3141f8c3cdc768b1e399af 
>   src/slave/containerizer/mesos/isolators/posix/rlimits.cpp 
> 3fc2b3dbd5b382e422c948adae0dc50f4fbcfc49 
>   src/tests/containerizer/posix_rlimits_isolator_tests.cpp 
> b7ccc7eeeb7e4d0d8f4fd463d506cfe7157076a4 
> 
> 
> Diff: https://reviews.apache.org/r/61387/diff/2/
> 
> 
> Testing
> ---
> 
> `bin/mesos-tests.sh 
> --gtest_filter="PosixRLimitsIsolatorTest.NestedContainers" 
> --gtest_repeat=1000 --gtest_break_on_failure` passed on a machine running 
> GNU/Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Review Request 61428: Added pid ns sharing based on agent flag and protobuf message field.

[Qian Zhang]

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

Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
and Vinod Kone.


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


Repository: mesos


Description
---

Added pid ns sharing based on agent flag and protobuf message field.


Diffs
-

  src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
2b316dbdf4a3735771af5bed80c6251d0d1cbd50 
  src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
f1dfc9f7398ffc029d7180d7f014a515338cb3f4 


Diff: https://reviews.apache.org/r/61428/diff/1/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 61406: Introduced `--disallow_top_level_pid_ns_sharing` agent flag.

[Qian Zhang]

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

(Updated Aug. 5, 2017, midnight)


Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
and Vinod Kone.


Changes
---

Addresssed comments.


Summary (updated)
-

Introduced `--disallow_top_level_pid_ns_sharing` agent flag.


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


Repository: mesos


Description (updated)
---

Introduced `--disallow_top_level_pid_ns_sharing` agent flag.


Diffs (updated)
-

  docs/configuration.md 041c3dfb9c0c1718770f74dfb33a9f5d6fbe9b61 
  src/slave/flags.hpp 032880dfa68cd29420e559d34e592e57827cfc07 
  src/slave/flags.cpp 4171604090ffebe79fa35579458cabff7270c0de 


Diff: https://reviews.apache.org/r/61406/diff/2/

Changes: https://reviews.apache.org/r/61406/diff/1-2/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 61098: Added unit tests for gRPC support in libprocess.

[Chun-Hung Hsiao]

> On Aug. 4, 2017, 3:53 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/src/tests/grpc_tests.cpp
> > Lines 95 (patched)
> > 
> >
> > Can we just make this test conditional compile in the makefile?

This fileis already conditional compiled. I put the condition here again to 
emphasize that the tests use unix sockets for server addresses. Once we support 
windows in the future, we might still want to test grpc, so how about using a 
fixed address, say `localhost:50051` for addresses on Windows?


- Chun-Hung


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


On Aug. 3, 2017, 6:04 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61098/
> ---
> 
> (Updated Aug. 3, 2017, 6:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jie Yu, Joseph Wu, and Zhitao Li.
> 
> 
> Bugs: MESOS-7810
> https://issues.apache.org/jira/browse/MESOS-7810
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We tested the following 6 scenarios when making gRPC calls:
> 
> 1. The server responds with an OK.
> 2. The server responds with a failure.
> 3. The client discards the call before the server starts.
> 4. The client discards the call when the server is processing it.
> 5. The client makes mulitple concurrent calls.
> 6. The client shuts down during a call.
> 7. The server is unreachable.
> 8. The server dose not respond in time.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/grpc_tests.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/grpc_tests.proto PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61098/diff/5/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61098: Added unit tests for gRPC support in libprocess.

[Jie Yu]

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




3rdparty/libprocess/src/tests/grpc_tests.cpp
Lines 95 (patched)


Can we just make this test conditional compile in the makefile?


- Jie Yu


On Aug. 3, 2017, 6:04 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61098/
> ---
> 
> (Updated Aug. 3, 2017, 6:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jie Yu, Joseph Wu, and Zhitao Li.
> 
> 
> Bugs: MESOS-7810
> https://issues.apache.org/jira/browse/MESOS-7810
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We tested the following 6 scenarios when making gRPC calls:
> 
> 1. The server responds with an OK.
> 2. The server responds with a failure.
> 3. The client discards the call before the server starts.
> 4. The client discards the call when the server is processing it.
> 5. The client makes mulitple concurrent calls.
> 6. The client shuts down during a call.
> 7. The server is unreachable.
> 8. The server dose not respond in time.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/grpc_tests.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/grpc_tests.proto PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61098/diff/5/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61095: Updated LICENSE information for grpc 1.4.2.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Aug. 3, 2017, 6:06 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61095/
> ---
> 
> (Updated Aug. 3, 2017, 6:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jie Yu, Joseph Wu, and Zhitao Li.
> 
> 
> Bugs: MESOS-7808
> https://issues.apache.org/jira/browse/MESOS-7808
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Content copied from:
> https://github.com/grpc/grpc/blob/master/LICENSE.
> 
> This patch depends on https://github.com/apache/mesos/pull/224.
> 
> 
> Diffs
> -
> 
>   LICENSE 86a58c7021c2f467c0f146a81d44177b5db3fb20 
> 
> 
> Diff: https://reviews.apache.org/r/61095/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61275: Added a URL parameter to the resource provider driver.

[Jie Yu]

> On Aug. 3, 2017, 10 a.m., Benjamin Bannier wrote:
> > src/resource_provider/daemon.hpp
> > Lines 44 (patched)
> > 
> >
> > I wonder if it would make more sense to keep passing a pid here like 
> > you did previously. With that we'd form the `URL` inside the daemon when 
> > creating the drivers.
> > 
> > Since the use of http is a detail of how drivers currently work, 
> > passing a pid would prevent us from leaking that fact beyond the daemon. 
> > Also, a driver will take a `ContentType` which is specific to HTTP which 
> > makes sense together with an URL; an URL in isolation seems leaky for that 
> > reason as well to me. I believe a pid and some `slave::Flags` passed here 
> > would form a similarly intuitive package on a different level of 
> > abstraction.

`pid` is libprocess specific concept. The RP that uses this driver might not be 
using libprocess. As the other comment I have in the other review, this really 
should be an Endpoint detector if you want to make http driver general (that 
can be used for scheduler/executor drivers).

The v1 resource provider API is HTTP based API, so 'http' is not a detail, it's 
a protocol.


- Jie


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


On Aug. 2, 2017, 11:37 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61275/
> ---
> 
> (Updated Aug. 2, 2017, 11:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7469
> https://issues.apache.org/jira/browse/MESOS-7469
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The URL will be used by the resource provider driver to determine
> the endpoint of the resource provider manager API.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/resource_provider.hpp 
> 88b606212ea57fee1c1ea522d2dc7f8124a9adef 
>   src/resource_provider/daemon.hpp 467a5d30510f0c8f7f042d989c615c97bbae52af 
>   src/resource_provider/daemon.cpp adcb60af8244840c56fbd2d846bdaeecb4f62282 
>   src/resource_provider/driver.cpp 6778ec9c863022446f141ee88f70eb563178ea05 
>   src/resource_provider/local.hpp 604c5d06b393349c352d0b698609ad6bfb16454a 
>   src/resource_provider/local.cpp a57c7c99c674f18036c36ec4b6df71947f700db8 
>   src/resource_provider/storage/provider.hpp 
> c6ea440feeb4cd79c59aa1f25851513e5d8dcc86 
>   src/resource_provider/storage/provider.cpp 
> 4c39312be5e4a6d783df3d385a66be6b3dcf8603 
>   src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 
> 
> 
> Diff: https://reviews.apache.org/r/61275/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 60003: Reduced copying in defer, dispatch and Future.

[Dmitry Zhuk]

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

(Updated Aug. 4, 2017, 10:40 a.m.)


Review request for mesos, Benjamin Hindman, haosdent huang, James Peach, and 
Michael Park.


Changes
---

Replaced `Dispatcher` with `std::bind`.


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


Repository: mesos


Description (updated)
---

This reduces number of copies made for each parameter in a code like this:
future.then(defer(pid, ::someMethod, param1, param2));

For the objects that do not support move semantics (e.g. protobuf
messages), number of copies is reduced from 8-10 to 6. If move semantics
is supported, then number of copies is reduced from 6-7 to 1 if
parameter is passed with std::move, or 2 otherwise.


Diffs (updated)
-

  3rdparty/libprocess/include/process/defer.hpp 
7f3369e723cb244e97930621cbba89cf7873567d 
  3rdparty/libprocess/include/process/deferred.hpp 
e446621be11ac51f5f91c417cc8975e363c2f715 
  3rdparty/libprocess/include/process/dispatch.hpp 
3a0793888dc0df5e3ec31b06f47cd920c71e0db9 
  3rdparty/libprocess/include/process/future.hpp 
cce950509f58022e79bb51a6e72ea1a005b9cb50 
  3rdparty/libprocess/include/process/http.hpp 
f637999174d92a98208b5fc49a65f9929efb11a0 
  3rdparty/libprocess/src/tests/benchmarks.cpp 
245ff7742b1e2c01c42a66d35d8e4f686603244f 


Diff: https://reviews.apache.org/r/60003/diff/4/

Changes: https://reviews.apache.org/r/60003/diff/3-4/


Testing
---

make check

Number of copies was checked by using defer to subscribe process for Future 
callbacks, and passing parameters that count number of copies made.


Thanks,

Dmitry Zhuk

Re: Review Request 61387: Made the rlimits isolator work with nested containers.

[Benjamin Bannier]

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



LGTM. Left mostly style comments.


src/tests/containerizer/posix_rlimits_isolator_tests.cpp
Lines 354-358 (patched)


No extra indent because of preprocessor conditional; indent this like e.g., 
`flags.bounding_capabilities = ...`



src/tests/containerizer/posix_rlimits_isolator_tests.cpp
Lines 368-371 (patched)


Indent by two spaces less.



src/tests/containerizer/posix_rlimits_isolator_tests.cpp
Lines 380-381 (patched)


Indent by two spaces less.



src/tests/containerizer/posix_rlimits_isolator_tests.cpp
Lines 388 (patched)


`ASSERT_FALSE(offers->empty())`.



src/tests/containerizer/posix_rlimits_isolator_tests.cpp
Lines 392-396 (patched)


Currently the declaration and first use are not on the same page for me. 
Let's move this right above its use,
 
driver.acceptOffers({offer.id()}, {LAUNCH_GROUP(executorInfo, 
taskGroup)})



src/tests/containerizer/posix_rlimits_isolator_tests.cpp
Lines 402-407 (patched)


Indent by two spaces less.



src/tests/containerizer/posix_rlimits_isolator_tests.cpp
Lines 407 (patched)


Let's set a `TaskID` here to ease debugging in below loop over the status 
updates.



src/tests/containerizer/posix_rlimits_isolator_tests.cpp
Lines 416-417 (patched)


It seems very unlikely that the simple shell script we add every consume 
10s of CPU time, but would I wonder if it makes sense to make this number 
truely unrealistically large, e.g., 100 and 200 instead of 10 and 20. 
That way we'd never fail the test because of flakiness but only due to global 
timeouts.



src/tests/containerizer/posix_rlimits_isolator_tests.cpp
Lines 423-428 (patched)


Indent by two spaces less.



src/tests/containerizer/posix_rlimits_isolator_tests.cpp
Lines 448 (patched)


Maybe `s/dummy/inSequence/`.

It is probably a good idea to add a comment explaining that it is fine that 
we never use this var.



src/tests/containerizer/posix_rlimits_isolator_tests.cpp
Lines 449 (patched)


It would be nice to avoid hardcoding `4` here, e.g.,

foreach (Future update, updates) {
  EXPECT_CALL(sched, statusUpdate(, _))
.WillOnce(FutureArg<1>());
}



src/tests/containerizer/posix_rlimits_isolator_tests.cpp
Lines 459 (patched)


A comment briefly summarizing the testing strategy (expected transitions, 
independent tracking of task status) would be great here.



src/tests/containerizer/posix_rlimits_isolator_tests.cpp
Lines 460 (patched)


We do not seem to put these on a single line and instead would use

enum class Stage
{
  INITIAL,
  RUNNING,
  FINISHED
};

The only other case where we us a one-liner is in `check_tests.cpp` (also 
added by you).



src/tests/containerizer/posix_rlimits_isolator_tests.cpp
Lines 463-464 (patched)


This seems to be equivalent to the shorter

taskStages[task1.task_id()] = Stage::INITIAL;
taskStages[task2.task_id()] = Stage::INITIAL;

Here and in all subsequent uses of `hashmap::put`.



src/tests/containerizer/posix_rlimits_isolator_tests.cpp
Lines 466 (patched)


Stray space before closing paren.



src/tests/containerizer/posix_rlimits_isolator_tests.cpp
Lines 466-469 (patched)


Let's avoid hardcoding hardcoding `4` here, e.g., instead

foreach (const Future& update, updates) {
  AWAIT_READY(update);
  
  // Either bind `taskStatus` like you currently do, or use `update->` 
directly (would look nicer if `updates` was named `taskStatuses` so we'd have a 
future `taskStatus` instead of `update`).
}



src/tests/containerizer/posix_rlimits_isolator_tests.cpp
Lines 476 (patched)


Let's output the full `taskStatus` when this assert fires.



src/tests/containerizer/posix_rlimits_isolator_tests.cpp
Lines 483 

Re: Review Request 60439: Added scripts to automate website publishing.

[Vinod Kone]

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

(Updated Aug. 4, 2017, 6:36 a.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, and haosdent huang.


Changes
---

addressed comments.


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


Repository: mesos


Description
---

These scripts are expected to be run by ASF CI on any commit to
the mesos repo. The directory layout is similar to tidybot CI job.


Diffs (updated)
-

  support/jenkins/websitebot.sh PRE-CREATION 
  support/mesos-website.sh PRE-CREATION 
  support/mesos-website/Dockerfile PRE-CREATION 
  support/mesos-website/build.sh PRE-CREATION 
  support/mesos-website/entrypoint.sh PRE-CREATION 


Diff: https://reviews.apache.org/r/60439/diff/3/

Changes: https://reviews.apache.org/r/60439/diff/2-3/


Testing
---

Tested by running a CI job pointing to a branch containing this patch.


Thanks,

Vinod Kone

Re: Review Request 60439: Added scripts to automate website publishing.

[Vinod Kone]

> On Aug. 2, 2017, 8:59 a.m., Benjamin Bannier wrote:
> > support/mesos-website/entrypoint.sh
> > Lines 23 (patched)
> > 
> >
> > Do you still recall why this was needed? It would be great to add e.g., 
> > a JIRA or a small reproducer to eval in the future whether this is still 
> > needed.

Filed MESOS-7859


- Vinod


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


On July 27, 2017, 12:28 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60439/
> ---
> 
> (Updated July 27, 2017, 12:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-7625
> https://issues.apache.org/jira/browse/MESOS-7625
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These scripts are expected to be run by ASF CI on any commit to
> the mesos repo. The directory layout is similar to tidybot CI job.
> 
> 
> Diffs
> -
> 
>   support/jenkins/websitebot.sh PRE-CREATION 
>   support/mesos-website.sh PRE-CREATION 
>   support/mesos-website/Dockerfile PRE-CREATION 
>   support/mesos-website/build.sh PRE-CREATION 
>   support/mesos-website/entrypoint.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60439/diff/2/
> 
> 
> Testing
> ---
> 
> Tested by running a CI job pointing to a branch containing this patch.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 60439: Added scripts to automate website publishing.

[Vinod Kone]

> On Aug. 2, 2017, 8:52 a.m., Benjamin Bannier wrote:
> > support/mesos-website/build.sh
> > Lines 22 (patched)
> > 
> >
> > This script will misbehave if run from a live dev checkout (it might 
> > e.g., reconfigure an already configued setup, remove an already generated 
> > site, rebuild the whole project if the developer builds in `build/`, or 
> > pick up changes from a dirty tree so the SHA does not correspond to the 
> > contents).
> > 
> > I'd suggest to add a safeguard against that. One option would be to 
> > refuse to run if the tree contains any untracked files or uncommitted 
> > changes, e.g., with
> > 
> > if [ "$(git status --porcelain | wc -l)" -ne 0 ]; then
> >   echo "This script is not intended to run on a live checkout, but 
> > the source tree contains untracked files or uncommitted changes."
> >   
> >   exit 1
> > fi
> > 
> > We should add this before setting up the the exit trap.

I put this check in "support/jenkins/websitebot.sh" to fail early.


- Vinod


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


On July 27, 2017, 12:28 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60439/
> ---
> 
> (Updated July 27, 2017, 12:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-7625
> https://issues.apache.org/jira/browse/MESOS-7625
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These scripts are expected to be run by ASF CI on any commit to
> the mesos repo. The directory layout is similar to tidybot CI job.
> 
> 
> Diffs
> -
> 
>   support/jenkins/websitebot.sh PRE-CREATION 
>   support/mesos-website.sh PRE-CREATION 
>   support/mesos-website/Dockerfile PRE-CREATION 
>   support/mesos-website/build.sh PRE-CREATION 
>   support/mesos-website/entrypoint.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60439/diff/2/
> 
> 
> Testing
> ---
> 
> Tested by running a CI job pointing to a branch containing this patch.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>