Re: Review Request 50763: Updated Linux 'MountInfoTable' entries to be sorted as expected.

2016-08-08 Thread Avinash sridharan

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


Ship it!




Ship It!

- Avinash sridharan


On Aug. 9, 2016, 5:18 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50763/
> ---
> 
> (Updated Aug. 9, 2016, 5:18 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-5969
> https://issues.apache.org/jira/browse/MESOS-5969
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Many places in the codebase assume that the mountinfo table is sorted
> according to the order: 'parent mount point < child mount point'.
> 
> However, in some cases this may not be true if (for example), a parent
> mount point (say '/') is remounted to add some extra flags to it.
> When this happens, the remounted file system will appear in the
> mountinfo table at the point where it was remounted.
> 
> We actually encountered this problem in the wild for the case of '/'
> being remounted after '/run' was mounted -- causing problems in the
> 'NvidiaVolume' which assumes the 'parent < child' ordering.
> 
> This commit fixes this problem by building the list of MountInfoTable
> entries in sorted order when 'read()' is called. An optional flag can
> be used to disable sorting produce the the original ordering.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp ec3b5b8cd6926b1f69ad499de1c13b989766a84e 
>   src/linux/fs.cpp f57db80ad0d7235d47910e05d663c77e233f8228 
>   src/tests/containerizer/fs_tests.cpp 
> 4cfafad28daac6bed849992a254660117d7ff30b 
> 
> Diff: https://reviews.apache.org/r/50763/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check
> src/mesos-tests
> sudo src/mesos-tests
> 
> Appeared to have one unrelated flaky test fail: 
> `ResourceOffersTest.ResourcesGetReofferedAfterTaskInfoError`
> Rerunning the tests a second time passed.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 50763: Updated Linux 'MountInfoTable' entries to be sorted as expected.

2016-08-08 Thread Kevin Klues

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

(Updated Aug. 9, 2016, 5:18 a.m.)


Review request for mesos, Benjamin Mahler and Jie Yu.


Changes
---

Addressed Avinash's comments (round 2).


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


Repository: mesos


Description
---

Many places in the codebase assume that the mountinfo table is sorted
according to the order: 'parent mount point < child mount point'.

However, in some cases this may not be true if (for example), a parent
mount point (say '/') is remounted to add some extra flags to it.
When this happens, the remounted file system will appear in the
mountinfo table at the point where it was remounted.

We actually encountered this problem in the wild for the case of '/'
being remounted after '/run' was mounted -- causing problems in the
'NvidiaVolume' which assumes the 'parent < child' ordering.

This commit fixes this problem by building the list of MountInfoTable
entries in sorted order when 'read()' is called. An optional flag can
be used to disable sorting produce the the original ordering.


Diffs (updated)
-

  src/linux/fs.hpp ec3b5b8cd6926b1f69ad499de1c13b989766a84e 
  src/linux/fs.cpp f57db80ad0d7235d47910e05d663c77e233f8228 
  src/tests/containerizer/fs_tests.cpp 4cfafad28daac6bed849992a254660117d7ff30b 

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


Testing
---

GTEST_FILTER="" make -j check
src/mesos-tests
sudo src/mesos-tests

Appeared to have one unrelated flaky test fail: 
`ResourceOffersTest.ResourcesGetReofferedAfterTaskInfoError`
Rerunning the tests a second time passed.


Thanks,

Kevin Klues



Re: Review Request 50763: Updated Linux 'MountInfoTable' entries to be sorted as expected.

2016-08-08 Thread Kevin Klues


> On Aug. 9, 2016, 5 a.m., Avinash sridharan wrote:
> > src/linux/fs.cpp, line 139
> > 
> >
> > This still seems problematic to me. Calling std::move an object makes 
> > it "empty", so accessing that objects memory after calling std::move could 
> > cause problems. Either we should copy out entry.id before calling the 
> > `std::move` on entry or we should just use copy semantics here.

I admittedly don't know much about the semantics of `std::move()` other than 
that it is typically used as an optimization to avoid copying. I should 
probably look into it more before just blindly using it. Intuitively though, I 
can see how it would be problematic to access a variable that you just "moved" 
by reading from its previous reference. I've updated the code accordingly.


- Kevin


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


On Aug. 9, 2016, 12:54 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50763/
> ---
> 
> (Updated Aug. 9, 2016, 12:54 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-5969
> https://issues.apache.org/jira/browse/MESOS-5969
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Many places in the codebase assume that the mountinfo table is sorted
> according to the order: 'parent mount point < child mount point'.
> 
> However, in some cases this may not be true if (for example), a parent
> mount point (say '/') is remounted to add some extra flags to it.
> When this happens, the remounted file system will appear in the
> mountinfo table at the point where it was remounted.
> 
> We actually encountered this problem in the wild for the case of '/'
> being remounted after '/run' was mounted -- causing problems in the
> 'NvidiaVolume' which assumes the 'parent < child' ordering.
> 
> This commit fixes this problem by building the list of MountInfoTable
> entries in sorted order when 'read()' is called. An optional flag can
> be used to disable sorting produce the the original ordering.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp ec3b5b8cd6926b1f69ad499de1c13b989766a84e 
>   src/linux/fs.cpp f57db80ad0d7235d47910e05d663c77e233f8228 
>   src/tests/containerizer/fs_tests.cpp 
> 4cfafad28daac6bed849992a254660117d7ff30b 
> 
> Diff: https://reviews.apache.org/r/50763/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check
> src/mesos-tests
> sudo src/mesos-tests
> 
> Appeared to have one unrelated flaky test fail: 
> `ResourceOffersTest.ResourcesGetReofferedAfterTaskInfoError`
> Rerunning the tests a second time passed.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 50763: Updated Linux 'MountInfoTable' entries to be sorted as expected.

2016-08-08 Thread Avinash sridharan

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




src/linux/fs.cpp (line 139)


This still seems problematic to me. Calling std::move an object makes it 
"empty", so accessing that objects memory after calling std::move could cause 
problems. Either we should copy out entry.id before calling the `std::move` on 
entry or we should just use copy semantics here.


- Avinash sridharan


On Aug. 9, 2016, 12:54 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50763/
> ---
> 
> (Updated Aug. 9, 2016, 12:54 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-5969
> https://issues.apache.org/jira/browse/MESOS-5969
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Many places in the codebase assume that the mountinfo table is sorted
> according to the order: 'parent mount point < child mount point'.
> 
> However, in some cases this may not be true if (for example), a parent
> mount point (say '/') is remounted to add some extra flags to it.
> When this happens, the remounted file system will appear in the
> mountinfo table at the point where it was remounted.
> 
> We actually encountered this problem in the wild for the case of '/'
> being remounted after '/run' was mounted -- causing problems in the
> 'NvidiaVolume' which assumes the 'parent < child' ordering.
> 
> This commit fixes this problem by building the list of MountInfoTable
> entries in sorted order when 'read()' is called. An optional flag can
> be used to disable sorting produce the the original ordering.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp ec3b5b8cd6926b1f69ad499de1c13b989766a84e 
>   src/linux/fs.cpp f57db80ad0d7235d47910e05d663c77e233f8228 
>   src/tests/containerizer/fs_tests.cpp 
> 4cfafad28daac6bed849992a254660117d7ff30b 
> 
> Diff: https://reviews.apache.org/r/50763/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check
> src/mesos-tests
> sudo src/mesos-tests
> 
> Appeared to have one unrelated flaky test fail: 
> `ResourceOffersTest.ResourcesGetReofferedAfterTaskInfoError`
> Rerunning the tests a second time passed.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 50915: Fixed the flaky SlaveRecoveryTest.* failures upon TearDown.

2016-08-08 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 8, 2016, 9:39 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50915/
> ---
> 
> (Updated Aug. 8, 2016, 9:39 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5889
> https://issues.apache.org/jira/browse/MESOS-5889
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> During TearDown of ContainerizerTest,
> we attempt to destroy all cgroups that we can find within the
> cgroup root. We observed in CI that on some distributions it
> appears that the freeze is not instantaneous and the freezer
> delay is needed to complete the destroy process. Howevever,
> the clock may be paused at this point which can lead to the
> TearDown failing to destroy!
> 
> There does not appear to be a listener hook to run code before
> test tear down, so a one-off Clock::resume here seems to be the
> simplest route.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.cpp 30492d7e3b4c5e9ae9d2b2446cadba62d43a3c65 
> 
> Diff: https://reviews.apache.org/r/50915/diff/
> 
> 
> Testing
> ---
> 
> Ran in internal CI on distributions that were seeing this flakiness.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Review Request 50915: Fixed the flaky SlaveRecoveryTest.* failures upon TearDown.

2016-08-08 Thread Benjamin Mahler

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

During TearDown of ContainerizerTest,
we attempt to destroy all cgroups that we can find within the
cgroup root. We observed in CI that on some distributions it
appears that the freeze is not instantaneous and the freezer
delay is needed to complete the destroy process. Howevever,
the clock may be paused at this point which can lead to the
TearDown failing to destroy!

There does not appear to be a listener hook to run code before
test tear down, so a one-off Clock::resume here seems to be the
simplest route.


Diffs
-

  src/tests/mesos.cpp 30492d7e3b4c5e9ae9d2b2446cadba62d43a3c65 

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


Testing
---

Ran in internal CI on distributions that were seeing this flakiness.


Thanks,

Benjamin Mahler



Re: Review Request 40266: Libprocess Reinit: Cleanup SocketManager alongside ProcessManager.

2016-08-08 Thread Joseph Wu

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

(Updated Aug. 8, 2016, 6:18 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
and Vinod Kone.


Changes
---

Updated some comments.  Added `authenticator_manager` to the list of things to 
clean up.


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


Repository: mesos


Description
---

The `SocketManager` and `ProcessManager` are highly inter-dependent, 
which requires some untangling in `process::finalize`.

* Logic originally found in `~ProcessManager` has been split into 
  `ProcessManager::finalize` due to what happens during cleanup.
* The future from `__s__->accept()` must be explicitly discarded as 
  libevent does not detect a locally closed socket.
* Terminating `HttpProxy`s must close the associated socket.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp 629f1644bc0a263972ec9efc41890c33f9406a34 

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


Testing
---

`make check` (libev)
`make check` (--enable-libevent --enable-ssl)


Thanks,

Joseph Wu



Re: Review Request 40266: Libprocess Reinit: Cleanup SocketManager alongside ProcessManager.

2016-08-08 Thread Joseph Wu


> On Aug. 4, 2016, 4:48 p.m., Greg Mann wrote:
> > 3rdparty/libprocess/src/process.cpp, line 1113
> > 
> >
> > Remove `gc` from this list, since it now gets deleted in the destructor

This entire comment block is actually superceded by one that is added in this 
review.  I'll delete the old comment.


> On Aug. 4, 2016, 4:48 p.m., Greg Mann wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 1116-1118
> > 
> >
> > It's not clear to me precisely which logic this comment refers to - 
> > HTTP authentication logic occurs in `ProcessBase` rather than 
> > `ProcessManager`, so perhaps it's a typo. AFAIK all HTTP authentication 
> > happens within libprocess processes, so terminating all processes should be 
> > sufficient to complete any pending authentication requests and exit safely, 
> > but perhaps it's worth checking with Alexander to see exactly what he meant 
> > here?

The authentication manager was not considered in this review because it didn't 
exist before :D

The codepath for auth roughly follows:
1) Something connects to the OS process via HTTP.
2) `ProcessManager::handle` is called.  This delegates the request to the 
appropriate process and enqueues an `HttpEvent`.  It also creates an 
`HttpProxy` process.
3) `ProcessBase::visit(const HttpEvent&)` will call 
`AuthenticatorManager::authenticate` for some routes.
4) `AuthenticatorManager` will dispatch to a `AuthenticatorManagerProcess`, 
which dispatches to some other processes (i.e. `BasicAuthenticatorProcess`).
5) The Authenticator returns a future, which is chained to some 
`AuthorizationCallbacks` (literally a bunch of lambdas).

(3) dereferences `authenticator_manager` from inside a `ProcessBase`.  During 
finalization, we will have deleted the `AuthenticatorManagerProcess`, which 
means that dispatches will go into the ether (the associated futures are never 
satisfied nor discarded).  

This means, we can safely delete `authenticator_manager` after closing the 
server socket (no further `HttpEvent`s).  We must delete it before 
`process_manager` because the `authenticator_manager` dereferences 
`process_manager`.


- Joseph


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


On July 29, 2016, 4:53 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40266/
> ---
> 
> (Updated July 29, 2016, 4:53 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3910
> https://issues.apache.org/jira/browse/MESOS-3910
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `SocketManager` and `ProcessManager` are highly inter-dependent, 
> which requires some untangling in `process::finalize`.
> 
> * Logic originally found in `~ProcessManager` has been split into 
>   `ProcessManager::finalize` due to what happens during cleanup.
> * The future from `__s__->accept()` must be explicitly discarded as 
>   libevent does not detect a locally closed socket.
> * Terminating `HttpProxy`s must close the associated socket.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 7f331b812de2f0437838f48e0959441c8e04c358 
> 
> Diff: https://reviews.apache.org/r/40266/diff/
> 
> 
> Testing
> ---
> 
> `make check` (libev)
> `make check` (--enable-libevent --enable-ssl)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



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

2016-08-08 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [50907, 50910, 50912]

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 Aug. 8, 2016, 11:44 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50912/
> ---
> 
> (Updated Aug. 8, 2016, 11:44 p.m.)
> 
> 
> Review request for mesos, Haris Choudhary and Joseph Wu.
> 
> 
> Bugs: MESOS-5676
> https://issues.apache.org/jira/browse/MESOS-5676
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the infrastructure for a new python-based CLI.
> 
> 
> Diffs
> -
> 
>   configure.ac f82c6314d65a8ea3eb118bab8e110063cae46121 
>   docs/getting-started.md ebe52705b8c8757d4a507ce3ae75f56d535a39d1 
>   src/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
>   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 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50912/diff/
> 
> 
> Testing
> ---
> 
> ../configure --enable-new-cli
> make -C src mesos
> src/mesos
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 40266: Libprocess Reinit: Cleanup SocketManager alongside ProcessManager.

2016-08-08 Thread Joseph Wu


> On April 25, 2016, 3:55 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 781-788
> > 
> >
> > Write a test for this.  i.e. Try to connect to this socket (with plain 
> > sockets) while `process::finalize` is running.

Note: This comment is superceded by 
https://reviews.apache.org/r/40266/#comment211141


- Joseph


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


On July 29, 2016, 4:53 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40266/
> ---
> 
> (Updated July 29, 2016, 4:53 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3910
> https://issues.apache.org/jira/browse/MESOS-3910
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `SocketManager` and `ProcessManager` are highly inter-dependent, 
> which requires some untangling in `process::finalize`.
> 
> * Logic originally found in `~ProcessManager` has been split into 
>   `ProcessManager::finalize` due to what happens during cleanup.
> * The future from `__s__->accept()` must be explicitly discarded as 
>   libevent does not detect a locally closed socket.
> * Terminating `HttpProxy`s must close the associated socket.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 7f331b812de2f0437838f48e0959441c8e04c358 
> 
> Diff: https://reviews.apache.org/r/40266/diff/
> 
> 
> Testing
> ---
> 
> `make check` (libev)
> `make check` (--enable-libevent --enable-ssl)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 50621: Libprocess reinit: Moved HttpProxy finalization and destruction.

2016-08-08 Thread Joseph Wu

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

(Updated Aug. 8, 2016, 6:07 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
and Vinod Kone.


Changes
---

Rebase and remove initialization code (unused).


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


Repository: mesos


Description (updated)
---

Moves the destructor code in `HttpProxy` into the `Process::finalize`
function.  And changes the `HttpProxy`s termination logic to 
terminate via `UPID` which guards against double-termination.

Removes some unused initialization code, too.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp 629f1644bc0a263972ec9efc41890c33f9406a34 

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


Testing (updated)
---

make

(More testing done later in the chain)


Thanks,

Joseph Wu



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

2016-08-08 Thread Joseph Wu

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

(Updated Aug. 8, 2016, 6:07 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
and Vinod Kone.


Changes
---

Rebased on the Process ID change by Gaston.


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 (updated)
-

  3rdparty/libprocess/include/process/reap.hpp 
1a9709c618c5ddc9d2b7492cc1855a11f1fc4fb9 
  3rdparty/libprocess/src/process.cpp 629f1644bc0a263972ec9efc41890c33f9406a34 
  3rdparty/libprocess/src/reap.cpp f8d2fdc3449744cfd6a974170484b676e8f3ac7b 

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


Testing
---

Tests done in a subsequent review.


Thanks,

Joseph Wu



Re: Review Request 50699: Added new TaskState values and PARTITION_AWARE capability.

2016-08-08 Thread Neil Conway


> On Aug. 6, 2016, 12:49 a.m., Vinod Kone wrote:
> > src/master/master.cpp, lines 6892-6893
> > 
> >
> > not yours, but lets not have a default case (have an explicit 
> > TASK_UNKNOWN case that is a no-op) here so that people don't forget to 
> > update this in the future when they add new statuses.

In this case, we deliberately want to ignore `TASK_UNKNOWN` (since we don't 
expect to see that status for a task the master knows about). Do you think we 
should do:

```
case TASK_UNKNOWN:
  CHECK(false); // Shouldn't happen.
```

Or is there a more idiomatic way to write this?


- Neil


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


On Aug. 9, 2016, 12:59 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50699/
> ---
> 
> (Updated Aug. 9, 2016, 12:59 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4049
> https://issues.apache.org/jira/browse/MESOS-4049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> TASK_DROPPED, TASK_UNREACHABLE, TASK_GONE, TASK_GONE_BY_OPERATOR, and
> TASK_UNKNOWN. These values are intended to replace the existing
> TASK_LOST state by offering more fine-grained information on the
> current state of a task. These states will only be sent to frameworks
> that opt into this new behavior via the PARTITION_AWARE capability.
> 
> Note that this commit doesn't add a master metric for the TASK_UNKNOWN
> status, because this is a "default" status reported when the master has
> no knowledge of a particular task/agent ID. Hence the number of
> "unknown" tasks at any given time is not a well-defined metric.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 6fd8363572ca157ef77004fa4be52dad400fe9c6 
>   include/mesos/v1/mesos.proto 85468f80379b42426df6a80b4b04075b983fd3ec 
>   src/common/protobuf_utils.cpp 8c4a7264080385789157703b05b42716fb0563b3 
>   src/examples/disk_full_framework.cpp 
> ad304fee94d443125a0dec2b2820267c69508621 
>   src/master/http.cpp 52dd80b856cf2317c0b73ba54bf501696786088d 
>   src/master/master.cpp 0bd1a3490a86fede86a3f5f62ce4745b65aae258 
>   src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
>   src/master/metrics.cpp 88a752dc2b4b73ccb919e99478b9ea2bd83842a0 
>   src/tests/master_tests.cpp 398164d09b8ef14f916122ed8780023c4a3cd0f6 
> 
> Diff: https://reviews.apache.org/r/50699/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 50699: Added new TaskState values and PARTITION_AWARE capability.

2016-08-08 Thread Neil Conway

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

(Updated Aug. 9, 2016, 12:59 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Tweak comments, per reviews.


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


Repository: mesos


Description
---

TASK_DROPPED, TASK_UNREACHABLE, TASK_GONE, TASK_GONE_BY_OPERATOR, and
TASK_UNKNOWN. These values are intended to replace the existing
TASK_LOST state by offering more fine-grained information on the
current state of a task. These states will only be sent to frameworks
that opt into this new behavior via the PARTITION_AWARE capability.

Note that this commit doesn't add a master metric for the TASK_UNKNOWN
status, because this is a "default" status reported when the master has
no knowledge of a particular task/agent ID. Hence the number of
"unknown" tasks at any given time is not a well-defined metric.


Diffs (updated)
-

  include/mesos/mesos.proto 6fd8363572ca157ef77004fa4be52dad400fe9c6 
  include/mesos/v1/mesos.proto 85468f80379b42426df6a80b4b04075b983fd3ec 
  src/common/protobuf_utils.cpp 8c4a7264080385789157703b05b42716fb0563b3 
  src/examples/disk_full_framework.cpp ad304fee94d443125a0dec2b2820267c69508621 
  src/master/http.cpp 52dd80b856cf2317c0b73ba54bf501696786088d 
  src/master/master.cpp 0bd1a3490a86fede86a3f5f62ce4745b65aae258 
  src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
  src/master/metrics.cpp 88a752dc2b4b73ccb919e99478b9ea2bd83842a0 
  src/tests/master_tests.cpp 398164d09b8ef14f916122ed8780023c4a3cd0f6 

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


Testing
---

make check


Thanks,

Neil Conway



Re: Review Request 50763: Updated Linux 'MountInfoTable' entries to be sorted as expected.

2016-08-08 Thread Kevin Klues

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

(Updated Aug. 9, 2016, 12:54 a.m.)


Review request for mesos, Benjamin Mahler and Jie Yu.


Changes
---

Addressed Avinash's comments.


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


Repository: mesos


Description
---

Many places in the codebase assume that the mountinfo table is sorted
according to the order: 'parent mount point < child mount point'.

However, in some cases this may not be true if (for example), a parent
mount point (say '/') is remounted to add some extra flags to it.
When this happens, the remounted file system will appear in the
mountinfo table at the point where it was remounted.

We actually encountered this problem in the wild for the case of '/'
being remounted after '/run' was mounted -- causing problems in the
'NvidiaVolume' which assumes the 'parent < child' ordering.

This commit fixes this problem by building the list of MountInfoTable
entries in sorted order when 'read()' is called. An optional flag can
be used to disable sorting produce the the original ordering.


Diffs (updated)
-

  src/linux/fs.hpp ec3b5b8cd6926b1f69ad499de1c13b989766a84e 
  src/linux/fs.cpp f57db80ad0d7235d47910e05d663c77e233f8228 
  src/tests/containerizer/fs_tests.cpp 4cfafad28daac6bed849992a254660117d7ff30b 

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


Testing
---

GTEST_FILTER="" make -j check
src/mesos-tests
sudo src/mesos-tests

Appeared to have one unrelated flaky test fail: 
`ResourceOffersTest.ResourcesGetReofferedAfterTaskInfoError`
Rerunning the tests a second time passed.


Thanks,

Kevin Klues



Re: Review Request 50763: Updated Linux 'MountInfoTable' entries to be sorted as expected.

2016-08-08 Thread Kevin Klues


> On Aug. 9, 2016, 12:48 a.m., Avinash sridharan wrote:
> > src/linux/fs.cpp, line 134
> > 
> >
> > The recursion is terminal since the entries in the `MountInfoTable` are 
> > guaranteed to have no cycles. An entry can have one and only one parent. 
> > Maybe document this? Ignore if you think this is self-evident.

Will do.


- Kevin


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


On Aug. 8, 2016, 11:55 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50763/
> ---
> 
> (Updated Aug. 8, 2016, 11:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-5969
> https://issues.apache.org/jira/browse/MESOS-5969
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Many places in the codebase assume that the mountinfo table is sorted
> according to the order: 'parent mount point < child mount point'.
> 
> However, in some cases this may not be true if (for example), a parent
> mount point (say '/') is remounted to add some extra flags to it.
> When this happens, the remounted file system will appear in the
> mountinfo table at the point where it was remounted.
> 
> We actually encountered this problem in the wild for the case of '/'
> being remounted after '/run' was mounted -- causing problems in the
> 'NvidiaVolume' which assumes the 'parent < child' ordering.
> 
> This commit fixes this problem by building the list of MountInfoTable
> entries in sorted order when 'read()' is called. An optional flag can
> be used to disable sorting produce the the original ordering.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp ec3b5b8cd6926b1f69ad499de1c13b989766a84e 
>   src/linux/fs.cpp f57db80ad0d7235d47910e05d663c77e233f8228 
>   src/tests/containerizer/fs_tests.cpp 
> 4cfafad28daac6bed849992a254660117d7ff30b 
> 
> Diff: https://reviews.apache.org/r/50763/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check
> src/mesos-tests
> sudo src/mesos-tests
> 
> Appeared to have one unrelated flaky test fail: 
> `ResourceOffersTest.ResourcesGetReofferedAfterTaskInfoError`
> Rerunning the tests a second time passed.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 50763: Updated Linux 'MountInfoTable' entries to be sorted as expected.

2016-08-08 Thread Avinash sridharan

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




src/linux/fs.cpp (line 134)


The recursion is terminal since the entries in the `MountInfoTable` are 
guaranteed to have no cycles. An entry can have one and only one parent. Maybe 
document this? Ignore if you think this is self-evident.



src/linux/fs.cpp (line 136)


We are doing a `std::move` on `*entry` and on the next line we are actually 
accessing the `id` stored in this buffer . This doesn't look safe, since the 
idea of `move` is not to access the "moved" buffer after an invocation of 
`std::move`?


- Avinash sridharan


On Aug. 8, 2016, 11:55 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50763/
> ---
> 
> (Updated Aug. 8, 2016, 11:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-5969
> https://issues.apache.org/jira/browse/MESOS-5969
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Many places in the codebase assume that the mountinfo table is sorted
> according to the order: 'parent mount point < child mount point'.
> 
> However, in some cases this may not be true if (for example), a parent
> mount point (say '/') is remounted to add some extra flags to it.
> When this happens, the remounted file system will appear in the
> mountinfo table at the point where it was remounted.
> 
> We actually encountered this problem in the wild for the case of '/'
> being remounted after '/run' was mounted -- causing problems in the
> 'NvidiaVolume' which assumes the 'parent < child' ordering.
> 
> This commit fixes this problem by building the list of MountInfoTable
> entries in sorted order when 'read()' is called. An optional flag can
> be used to disable sorting produce the the original ordering.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp ec3b5b8cd6926b1f69ad499de1c13b989766a84e 
>   src/linux/fs.cpp f57db80ad0d7235d47910e05d663c77e233f8228 
>   src/tests/containerizer/fs_tests.cpp 
> 4cfafad28daac6bed849992a254660117d7ff30b 
> 
> Diff: https://reviews.apache.org/r/50763/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check
> src/mesos-tests
> sudo src/mesos-tests
> 
> Appeared to have one unrelated flaky test fail: 
> `ResourceOffersTest.ResourcesGetReofferedAfterTaskInfoError`
> Rerunning the tests a second time passed.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



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

2016-08-08 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [50907, 50910]

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 Aug. 8, 2016, 10:10 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 8, 2016, 10:10 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>



Re: Review Request 50763: Updated Linux 'MountInfoTable' entries to be sorted as expected.

2016-08-08 Thread Kevin Klues

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

(Updated Aug. 8, 2016, 11:55 p.m.)


Review request for mesos, Benjamin Mahler and Jie Yu.


Changes
---

Updated based on Jie's comments.


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


Repository: mesos


Description
---

Many places in the codebase assume that the mountinfo table is sorted
according to the order: 'parent mount point < child mount point'.

However, in some cases this may not be true if (for example), a parent
mount point (say '/') is remounted to add some extra flags to it.
When this happens, the remounted file system will appear in the
mountinfo table at the point where it was remounted.

We actually encountered this problem in the wild for the case of '/'
being remounted after '/run' was mounted -- causing problems in the
'NvidiaVolume' which assumes the 'parent < child' ordering.

This commit fixes this problem by building the list of MountInfoTable
entries in sorted order when 'read()' is called. An optional flag can
be used to disable sorting produce the the original ordering.


Diffs (updated)
-

  src/linux/fs.hpp ec3b5b8cd6926b1f69ad499de1c13b989766a84e 
  src/linux/fs.cpp f57db80ad0d7235d47910e05d663c77e233f8228 
  src/tests/containerizer/fs_tests.cpp 4cfafad28daac6bed849992a254660117d7ff30b 

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


Testing
---

GTEST_FILTER="" make -j check
src/mesos-tests
sudo src/mesos-tests

Appeared to have one unrelated flaky test fail: 
`ResourceOffersTest.ResourcesGetReofferedAfterTaskInfoError`
Rerunning the tests a second time passed.


Thanks,

Kevin Klues



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

2016-08-08 Thread Kevin Klues

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

(Updated Aug. 8, 2016, 11:44 p.m.)


Review request for mesos, Haris Choudhary and Joseph Wu.


Changes
---

Removed the test hooks for the CLI. Those will be coming in a subsuquent commit.


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


Repository: mesos


Description
---

Added the infrastructure for a new python-based CLI.


Diffs (updated)
-

  configure.ac f82c6314d65a8ea3eb118bab8e110063cae46121 
  docs/getting-started.md ebe52705b8c8757d4a507ce3ae75f56d535a39d1 
  src/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
  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 
  support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 

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


Testing
---

../configure --enable-new-cli
make -C src mesos
src/mesos


Thanks,

Kevin Klues



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

2016-08-08 Thread Kevin Klues

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

Review request for mesos, Haris Choudhary and Joseph Wu.


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


Repository: mesos


Description
---

Added the infrastructure for a new python-based CLI.


Diffs
-

  configure.ac f82c6314d65a8ea3eb118bab8e110063cae46121 
  docs/getting-started.md ebe52705b8c8757d4a507ce3ae75f56d535a39d1 
  src/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
  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 
  support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 

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


Testing
---

../configure --enable-new-cli
make -C src mesos
src/mesos


Thanks,

Kevin Klues



Re: Review Request 50897: Added compiler information to the output of ./configure.

2016-08-08 Thread Kapil Arya

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



If you don't mind, could you include a sample output here?

Also, I am wondering if we should include a little more information about the 
environment (either as part of `./configure` or `make`) such as kernel, libc 
version, etc.!

- Kapil Arya


On Aug. 8, 2016, 11:57 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50897/
> ---
> 
> (Updated Aug. 8, 2016, 11:57 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Kapil Arya, 
> and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With this change, the output of `./configure` contains a message like:
> `configure: Using Clang 7.0.2`.
> 
> 
> Diffs
> -
> 
>   configure.ac f82c6314d65a8ea3eb118bab8e110063cae46121 
> 
> Diff: https://reviews.apache.org/r/50897/diff/
> 
> 
> Testing
> ---
> 
> Tested with:
> 
> `GCC 4.8.1, 4.8.4, 4.8.5, 4.9.2, 5.3.1`
> `Clang 7.0.2`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>



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

2016-08-08 Thread Haris Choudhary

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

Review request for mesos, Kevin Klues and Vinod Kone.


Bugs: Mesos-6006
https://issues.apache.org/jira/browse/Mesos-6006


Repository: mesos


Description
---

It currently doesn't run over any files in the code base, but we will
be adding the new python CLI in a subsequent commit, which will use
this new linter.


Diffs
-

  support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 

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


Testing
---


Thanks,

Haris Choudhary



Re: Review Request 50907: Abstracted mesos-style.py to wrap the cpp linter in a class.

2016-08-08 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [50907]

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 Aug. 8, 2016, 7:49 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50907/
> ---
> 
> (Updated Aug. 8, 2016, 7:49 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-6006
> https://issues.apache.org/jira/browse/MESOS-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, mesos-style.py was just a collection of functions that
> checked the style of relevant files in the mesos code base.  However,
> the script assumed that we always wanted to run cpplint over every
> file we were checking. Since we are planning on adding a python linter
> to the codebase soon, it makes sense to abstract the common
> functionality from this script into a class so that a cpp-based linter
> and a python-based linter can inherit the same set of common
> functionality.
> 
> This commit builds this abstraction and implements a 'CppLinter()' in
> terms of it.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50907/diff/
> 
> 
> Testing
> ---
> 
> Ran `support/mesos-style.py` over the whole code base.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 50763: Updated Linux 'MountInfoTable' entries to be sorted as expected.

2016-08-08 Thread Jie Yu

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


Fix it, then Ship it!





src/linux/fs.cpp (line 120)


Any reason use pointer here? Can you use the same move optimization as you 
did below?


- Jie Yu


On Aug. 5, 2016, 6:45 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50763/
> ---
> 
> (Updated Aug. 5, 2016, 6:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-5969
> https://issues.apache.org/jira/browse/MESOS-5969
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Many places in the codebase assume that the mountinfo table is sorted
> according to the order: 'parent mount point < child mount point'.
> 
> However, in some cases this may not be true if (for example), a parent
> mount point (say '/') is remounted to add some extra flags to it.
> When this happens, the remounted file system will appear in the
> mountinfo table at the point where it was remounted.
> 
> We actually encountered this problem in the wild for the case of '/'
> being remounted after '/run' was mounted -- causing problems in the
> 'NvidiaVolume' which assumes the 'parent < child' ordering.
> 
> This commit fixes this problem by building the list of MountInfoTable
> entries in sorted order when 'read()' is called. An optional flag can
> be used to disable sorting produce the the original ordering.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp ec3b5b8cd6926b1f69ad499de1c13b989766a84e 
>   src/linux/fs.cpp f57db80ad0d7235d47910e05d663c77e233f8228 
>   src/tests/containerizer/fs_tests.cpp 
> 4cfafad28daac6bed849992a254660117d7ff30b 
> 
> Diff: https://reviews.apache.org/r/50763/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check
> src/mesos-tests
> sudo src/mesos-tests
> 
> Appeared to have one unrelated flaky test fail: 
> `ResourceOffersTest.ResourcesGetReofferedAfterTaskInfoError`
> Rerunning the tests a second time passed.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 50897: Added compiler information to the output of ./configure.

2016-08-08 Thread Benjamin Mahler

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



It would be good to include a bit of context, I assume you're doing this 
because our output currently does not clearly contain this information?

Any reason this change isn't replicated in libprocess and stout configure.ac's?

- Benjamin Mahler


On Aug. 8, 2016, 3:57 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50897/
> ---
> 
> (Updated Aug. 8, 2016, 3:57 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Kapil Arya, 
> and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With this change, the output of `./configure` contains a message like:
> `configure: Using Clang 7.0.2`.
> 
> 
> Diffs
> -
> 
>   configure.ac f82c6314d65a8ea3eb118bab8e110063cae46121 
> 
> Diff: https://reviews.apache.org/r/50897/diff/
> 
> 
> Testing
> ---
> 
> Tested with:
> 
> `GCC 4.8.1, 4.8.4, 4.8.5, 4.9.2, 5.3.1`
> `Clang 7.0.2`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>



Re: Review Request 50901: Added new contributor (Gaojin CAO).

2016-08-08 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [50901]

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 Aug. 8, 2016, 4:34 p.m., Gaojin CAO wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50901/
> ---
> 
> (Updated Aug. 8, 2016, 4:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Guangya Liu, and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new contributor (Gaojin CAO).
> 
> 
> Diffs
> -
> 
>   docs/contributors.yaml 7114bd6f2b6fe0edf9786372da4dec288095d637 
> 
> Diff: https://reviews.apache.org/r/50901/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gaojin CAO
> 
>



Re: Review Request 50907: Abstracted mesos-style.py to wrap the cpp linter in a class.

2016-08-08 Thread Kevin Klues

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

(Updated Aug. 8, 2016, 7:49 p.m.)


Review request for mesos, Jie Yu and Vinod Kone.


Changes
---

Added the JIRA link


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


Repository: mesos


Description
---

Previously, mesos-style.py was just a collection of functions that
checked the style of relevant files in the mesos code base.  However,
the script assumed that we always wanted to run cpplint over every
file we were checking. Since we are planning on adding a python linter
to the codebase soon, it makes sense to abstract the common
functionality from this script into a class so that a cpp-based linter
and a python-based linter can inherit the same set of common
functionality.

This commit builds this abstraction and implements a 'CppLinter()' in
terms of it.


Diffs
-

  support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 

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


Testing
---

Ran `support/mesos-style.py` over the whole code base.


Thanks,

Kevin Klues



Review Request 50907: Abstracted mesos-style.py to wrap the cpp linter in a class.

2016-08-08 Thread Kevin Klues

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

Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

Previously, mesos-style.py was just a collection of functions that
checked the style of relevant files in the mesos code base.  However,
the script assumed that we always wanted to run cpplint over every
file we were checking. Since we are planning on adding a python linter
to the codebase soon, it makes sense to abstract the common
functionality from this script into a class so that a cpp-based linter
and a python-based linter can inherit the same set of common
functionality.

This commit builds this abstraction and implements a 'CppLinter()' in
terms of it.


Diffs
-

  support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 

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


Testing
---

Ran `support/mesos-style.py` over the whole code base.


Thanks,

Kevin Klues



Re: Review Request 50900: Removed unneeded extra space in libprocess code base.

2016-08-08 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [50887, 50899, 50900]

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 Aug. 8, 2016, 4:19 p.m., Gaojin CAO wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50900/
> ---
> 
> (Updated Aug. 8, 2016, 4:19 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Guangya Liu, and haosdent huang.
> 
> 
> Bugs: MESOS-5830
> https://issues.apache.org/jira/browse/MESOS-5830
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch is part of MESOS-5830, specially working on cleaning up
> unneeded space in project libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/async.hpp 
> e7d846288791221f4dfa03d7de89d9bde177f1ca 
>   3rdparty/libprocess/include/process/protobuf.hpp 
> 695c6e6572852d2c4afbaacc6b63771ed035fd65 
>   3rdparty/libprocess/include/process/reap.hpp 
> 1a9709c618c5ddc9d2b7492cc1855a11f1fc4fb9 
>   3rdparty/libprocess/include/process/sequence.hpp 
> c9a46f28b770023ca621696493521445762a3fd1 
>   3rdparty/libprocess/include/process/statistics.hpp 
> 13aa464329c2cc77b017868a1dcbf5b77c53635d 
>   3rdparty/libprocess/src/reap.cpp f8d2fdc3449744cfd6a974170484b676e8f3ac7b 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 9fac45e21bd03ccea19d0ef7d8bb2efc408e60b5 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 8a0d61eb57f9ae972eedf3481f131844b283abc5 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> 4ddd35b61f222ca674e41875a6d0ee43d12cbfbd 
>   3rdparty/libprocess/src/tests/reap_tests.cpp 
> 0cbe9f139d59ac154695d84638c551fbe098cb10 
>   3rdparty/libprocess/src/tests/shared_tests.cpp 
> 8f94cca107ebe1fdefc19fb1f65e4c205f96 
>   3rdparty/libprocess/src/tests/statistics_tests.cpp 
> db9817ff182f37b7a04358fb59b8b800b16a7e1e 
> 
> Diff: https://reviews.apache.org/r/50900/diff/
> 
> 
> Testing
> ---
> 
> Pass `make` and `make tests`.
> 
> 
> Thanks,
> 
> Gaojin CAO
> 
>



Re: Review Request 50899: Removed unneeded extra space in stout code base.

2016-08-08 Thread haosdent huang

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


Ship it!




Ship It!

- haosdent huang


On Aug. 8, 2016, 4:17 p.m., Gaojin CAO wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50899/
> ---
> 
> (Updated Aug. 8, 2016, 4:17 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Guangya Liu, and haosdent huang.
> 
> 
> Bugs: MESOS-5830
> https://issues.apache.org/jira/browse/MESOS-5830
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch is part of MESOS-5830, specially working on cleaning up
> unneeded space in project stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags.hpp 
> 169f959ab2770744d615cc235062ba2becc8af64 
>   3rdparty/stout/include/stout/interval.hpp 
> e6deeac39855488548678c59ed4c69646fc02602 
>   3rdparty/stout/include/stout/json.hpp 
> 51b1ada87846d4d25cbc8a6bbb97ba91e02a9cab 
>   3rdparty/stout/include/stout/linkedhashmap.hpp 
> 6e06260bcce2dbe0f9134f3d0fd7a2b47b0d3286 
>   3rdparty/stout/include/stout/os/osx.hpp 
> ca77bb0bbda0cee07d85435350dc4ccc176d4013 
>   3rdparty/stout/include/stout/os/sunos.hpp 
> ec8e1f7562d5747113631351e6d8ff8989cf0fed 
>   3rdparty/stout/include/stout/os/sysctl.hpp 
> 6db7bf6f139770ce978d171f451e932072cdaf5f 
>   3rdparty/stout/include/stout/proc.hpp 
> b543432c1a8419093c508c1d0e72b9757df04463 
>   3rdparty/stout/include/stout/protobuf.hpp 
> 91305e104c01d649bd435a27b15954036c27 
>   3rdparty/stout/include/stout/strings.hpp 
> 7f7f1cffcebfe16cb986917b1d90c1ae4a480989 
>   3rdparty/stout/tests/flags_tests.cpp 
> 77f3a6af110da1ffcdf2b7ab2b66431a6b5c91d3 
>   3rdparty/stout/tests/ip_tests.cpp 4d1f1c9a4dd35da2a21bef1349a662af44bb4ba1 
>   3rdparty/stout/tests/mac_tests.cpp 1ff60cf5506c1855547400aa20107ec5086d8431 
>   3rdparty/stout/tests/multimap_tests.cpp 
> 2f606eb16fbaa3c755e597a05e97cd30d3bfd9f4 
>   3rdparty/stout/tests/option_tests.cpp 
> 36abe137bdae1e6aaf65f39bf302d9e7a76fefdd 
>   3rdparty/stout/tests/os/process_tests.cpp 
> b536e664d22370e30c49a8f953189e92a1358709 
>   3rdparty/stout/tests/os_tests.cpp e67444077eae55fd25945b451164b5bcc37552b0 
>   3rdparty/stout/tests/proc_tests.cpp 
> 17b909368405d9ddb7e7127e89d0422a9a2baa11 
>   3rdparty/stout/tests/strings_tests.cpp 
> b54a9dbf162403310b8bba687442e184a473f5a6 
> 
> Diff: https://reviews.apache.org/r/50899/diff/
> 
> 
> Testing
> ---
> 
> Pass `make` and `make tests`.
> 
> 
> Thanks,
> 
> Gaojin CAO
> 
>



Re: Review Request 50901: Added new contributor (Gaojin CAO).

2016-08-08 Thread haosdent huang

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


Ship it!




Ship It!

- haosdent huang


On Aug. 8, 2016, 4:34 p.m., Gaojin CAO wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50901/
> ---
> 
> (Updated Aug. 8, 2016, 4:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Guangya Liu, and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new contributor (Gaojin CAO).
> 
> 
> Diffs
> -
> 
>   docs/contributors.yaml 7114bd6f2b6fe0edf9786372da4dec288095d637 
> 
> Diff: https://reviews.apache.org/r/50901/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gaojin CAO
> 
>



Re: Review Request 50897: Added compiler information to the output of ./configure.

2016-08-08 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [50897]

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 Aug. 8, 2016, 3:57 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50897/
> ---
> 
> (Updated Aug. 8, 2016, 3:57 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Kapil Arya, 
> and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With this change, the output of `./configure` contains a message like:
> `configure: Using Clang 7.0.2`.
> 
> 
> Diffs
> -
> 
>   configure.ac f82c6314d65a8ea3eb118bab8e110063cae46121 
> 
> Diff: https://reviews.apache.org/r/50897/diff/
> 
> 
> Testing
> ---
> 
> Tested with:
> 
> `GCC 4.8.1, 4.8.4, 4.8.5, 4.9.2, 5.3.1`
> `Clang 7.0.2`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>



Review Request 50901: Added new contributor (Gaojin CAO).

2016-08-08 Thread Gaojin CAO

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

Review request for mesos, Benjamin Bannier, Guangya Liu, and haosdent huang.


Repository: mesos


Description
---

Added new contributor (Gaojin CAO).


Diffs
-

  docs/contributors.yaml 7114bd6f2b6fe0edf9786372da4dec288095d637 

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


Testing
---


Thanks,

Gaojin CAO



Re: Review Request 50887: Trimmed unneeded extra space between right angle brackets.

2016-08-08 Thread Gaojin CAO


> On Aug. 8, 2016, 1:14 p.m., Benjamin Bannier wrote:
> > Great start!
> > 
> > I grepped the code base and still found many instances of these unneeded 
> > spaces (with `git grep '> >'`). Could you make a full sweep? Note that 
> > changes to libprocess and stout would need to be made in two separate 
> > patches. Feel free to add me as a reviewer.
> 
> Guangya Liu wrote:
> @Gaojin, per Ben's comments, you may want to craete two patches, one is 
> for https://github.com/apache/mesos/tree/master/3rdparty and the other is for 
> https://github.com/apache/mesos/tree/master/src

Done! I have split changes into 3 patches, this one and the other two(stout: 
https://reviews.apache.org/r/50899/, libprocess: 
https://reviews.apache.org/r/50900/). Each has been tested with `make` and 
`make tests`.


- Gaojin


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


On Aug. 8, 2016, 4:13 p.m., Gaojin CAO wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50887/
> ---
> 
> (Updated Aug. 8, 2016, 4:13 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Guangya Liu, and haosdent huang.
> 
> 
> Bugs: MESOS-5830
> https://issues.apache.org/jira/browse/MESOS-5830
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos codebase has been evolved and no more need an extra space
> between two right angle brackets, e.g. vector

Review Request 50900: Removed unneeded extra space in libprocess code base.

2016-08-08 Thread Gaojin CAO

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

Review request for mesos, Benjamin Bannier, Guangya Liu, and haosdent huang.


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


Repository: mesos


Description
---

This patch is part of MESOS-5830, specially working on cleaning up
unneeded space in project libprocess.


Diffs
-

  3rdparty/libprocess/include/process/async.hpp 
e7d846288791221f4dfa03d7de89d9bde177f1ca 
  3rdparty/libprocess/include/process/protobuf.hpp 
695c6e6572852d2c4afbaacc6b63771ed035fd65 
  3rdparty/libprocess/include/process/reap.hpp 
1a9709c618c5ddc9d2b7492cc1855a11f1fc4fb9 
  3rdparty/libprocess/include/process/sequence.hpp 
c9a46f28b770023ca621696493521445762a3fd1 
  3rdparty/libprocess/include/process/statistics.hpp 
13aa464329c2cc77b017868a1dcbf5b77c53635d 
  3rdparty/libprocess/src/reap.cpp f8d2fdc3449744cfd6a974170484b676e8f3ac7b 
  3rdparty/libprocess/src/tests/benchmarks.cpp 
9fac45e21bd03ccea19d0ef7d8bb2efc408e60b5 
  3rdparty/libprocess/src/tests/http_tests.cpp 
8a0d61eb57f9ae972eedf3481f131844b283abc5 
  3rdparty/libprocess/src/tests/metrics_tests.cpp 
4ddd35b61f222ca674e41875a6d0ee43d12cbfbd 
  3rdparty/libprocess/src/tests/reap_tests.cpp 
0cbe9f139d59ac154695d84638c551fbe098cb10 
  3rdparty/libprocess/src/tests/shared_tests.cpp 
8f94cca107ebe1fdefc19fb1f65e4c205f96 
  3rdparty/libprocess/src/tests/statistics_tests.cpp 
db9817ff182f37b7a04358fb59b8b800b16a7e1e 

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


Testing
---

Pass `make` and `make tests`.


Thanks,

Gaojin CAO



Review Request 50899: Removed unneeded extra space in stout code base.

2016-08-08 Thread Gaojin CAO

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

Review request for mesos, Benjamin Bannier, Guangya Liu, and haosdent huang.


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


Repository: mesos


Description
---

This patch is part of MESOS-5830, specially working on cleaning up
unneeded space in project stout.


Diffs
-

  3rdparty/stout/include/stout/flags.hpp 
169f959ab2770744d615cc235062ba2becc8af64 
  3rdparty/stout/include/stout/interval.hpp 
e6deeac39855488548678c59ed4c69646fc02602 
  3rdparty/stout/include/stout/json.hpp 
51b1ada87846d4d25cbc8a6bbb97ba91e02a9cab 
  3rdparty/stout/include/stout/linkedhashmap.hpp 
6e06260bcce2dbe0f9134f3d0fd7a2b47b0d3286 
  3rdparty/stout/include/stout/os/osx.hpp 
ca77bb0bbda0cee07d85435350dc4ccc176d4013 
  3rdparty/stout/include/stout/os/sunos.hpp 
ec8e1f7562d5747113631351e6d8ff8989cf0fed 
  3rdparty/stout/include/stout/os/sysctl.hpp 
6db7bf6f139770ce978d171f451e932072cdaf5f 
  3rdparty/stout/include/stout/proc.hpp 
b543432c1a8419093c508c1d0e72b9757df04463 
  3rdparty/stout/include/stout/protobuf.hpp 
91305e104c01d649bd435a27b15954036c27 
  3rdparty/stout/include/stout/strings.hpp 
7f7f1cffcebfe16cb986917b1d90c1ae4a480989 
  3rdparty/stout/tests/flags_tests.cpp 77f3a6af110da1ffcdf2b7ab2b66431a6b5c91d3 
  3rdparty/stout/tests/ip_tests.cpp 4d1f1c9a4dd35da2a21bef1349a662af44bb4ba1 
  3rdparty/stout/tests/mac_tests.cpp 1ff60cf5506c1855547400aa20107ec5086d8431 
  3rdparty/stout/tests/multimap_tests.cpp 
2f606eb16fbaa3c755e597a05e97cd30d3bfd9f4 
  3rdparty/stout/tests/option_tests.cpp 
36abe137bdae1e6aaf65f39bf302d9e7a76fefdd 
  3rdparty/stout/tests/os/process_tests.cpp 
b536e664d22370e30c49a8f953189e92a1358709 
  3rdparty/stout/tests/os_tests.cpp e67444077eae55fd25945b451164b5bcc37552b0 
  3rdparty/stout/tests/proc_tests.cpp 17b909368405d9ddb7e7127e89d0422a9a2baa11 
  3rdparty/stout/tests/strings_tests.cpp 
b54a9dbf162403310b8bba687442e184a473f5a6 

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


Testing
---

Pass `make` and `make tests`.


Thanks,

Gaojin CAO



Re: Review Request 50887: Trimmed unneeded extra space between right angle brackets.

2016-08-08 Thread Gaojin CAO

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

(Updated Aug. 8, 2016, 4:13 p.m.)


Review request for mesos, Benjamin Bannier, Guangya Liu, and haosdent huang.


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


Repository: mesos


Description
---

Mesos codebase has been evolved and no more need an extra space
between two right angle brackets, e.g. vector

Re: Review Request 50774: Added `-Wsign-compare` to global compiler flags.

2016-08-08 Thread Michael Park

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


Ship it!




Ship It!

- Michael Park


On Aug. 3, 2016, 11:57 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50774/
> ---
> 
> (Updated Aug. 3, 2016, 11:57 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos already builds with the `-Wall` and `-Werror` flags.
> However, different compilers have different sets of checks for `-Wall`.
> 
> This change explicitly adds the `-Wsign-compare` warning flag that is
> present in GCC's `-Wall`, but not in Clang.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
> 
> Diff: https://reviews.apache.org/r/50774/diff/
> 
> 
> Testing
> ---
> 
> Introduced a signed-unsigned comparison on OSX and checked that the build 
> warns (fails) due to the comparison.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 50898: Add new contributor (Will Rouesnel).

2016-08-08 Thread Anand Mazumdar

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


Ship it!




Ship It!

- Anand Mazumdar


On Aug. 8, 2016, 3:24 p.m., Will Rouesnel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50898/
> ---
> 
> (Updated Aug. 8, 2016, 3:24 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add new contributor (Will Rouesnel).
> 
> 
> Diffs
> -
> 
>   docs/contributors.yaml fdb21d44a1c2289c05fa6cf898ca364de8c8c040 
> 
> Diff: https://reviews.apache.org/r/50898/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Will Rouesnel
> 
>



Review Request 50898: Add new contributor (Will Rouesnel).

2016-08-08 Thread Will Rouesnel

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

Review request for mesos.


Repository: mesos


Description
---

Add new contributor (Will Rouesnel).


Diffs
-

  docs/contributors.yaml fdb21d44a1c2289c05fa6cf898ca364de8c8c040 

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


Testing
---


Thanks,

Will Rouesnel



Re: Review Request 50733: Removed CgroupsCpushareIsolatorProcess.

2016-08-08 Thread Qian Zhang

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


Fix it, then Ship it!





src/tests/containerizer/isolator_tests.cpp (line 531)


I would prefer to keep the original test name `ROOT_CGROUPS_RevocableCpu`.



src/tests/containerizer/isolator_tests.cpp (line 606)


Ditto.



src/tests/containerizer/isolator_tests.cpp (line 724)


Ditto



src/tests/containerizer/isolator_tests.cpp (line 816)


Ditto


- Qian Zhang


On Aug. 4, 2016, 5:18 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50733/
> ---
> 
> (Updated Aug. 4, 2016, 5:18 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5973
> https://issues.apache.org/jira/browse/MESOS-5973
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed CgroupsCpushareIsolatorProcess.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 1286ee08fe2d60867326a1f2585f054c20b52208 
>   src/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
>   src/docker/docker.cpp e8d2cb9662af34d75c9e2d822004f58fac76e7e0 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
>   src/slave/containerizer/mesos/isolators/cgroups/cpushare.hpp 
> 74982a610b6c0a74734165a0c6aa8c9f72f54deb 
>   src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp 
> 221e814a448c4b5df9dab98de597451a24e2b89c 
>   src/tests/containerizer/isolator_tests.cpp 
> 488747347f71a6a1bb6bc01477143d077d4fd3eb 
> 
> Diff: https://reviews.apache.org/r/50733/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Review Request 50897: Added compiler information to the output of ./configure.

2016-08-08 Thread Gastón Kleiman

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

Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Kapil Arya.


Repository: mesos


Description
---

With this change, the output of `./configure` contains a message like:
`configure: Using Clang 7.0.2`.


Diffs
-

  configure.ac f82c6314d65a8ea3eb118bab8e110063cae46121 

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


Testing
---

Tested with:

`GCC 4.8.1, 4.8.4, 4.8.5, 4.9.2, 5.3.1`
`Clang 7.0.2`


Thanks,

Gastón Kleiman



Re: Review Request 50887: Trimmed unneeded extra space between right angle brackets.

2016-08-08 Thread Guangya Liu


> On 八月 8, 2016, 1:14 p.m., Benjamin Bannier wrote:
> > Great start!
> > 
> > I grepped the code base and still found many instances of these unneeded 
> > spaces (with `git grep '> >'`). Could you make a full sweep? Note that 
> > changes to libprocess and stout would need to be made in two separate 
> > patches. Feel free to add me as a reviewer.

@Gaojin, per Ben's comments, you may want to craete two patches, one is for 
https://github.com/apache/mesos/tree/master/3rdparty and the other is for 
https://github.com/apache/mesos/tree/master/src


- Guangya


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


On 八月 8, 2016, 8:02 a.m., Gaojin CAO wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50887/
> ---
> 
> (Updated 八月 8, 2016, 8:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Guangya Liu, and haosdent huang.
> 
> 
> Bugs: MESOS-5830
> https://issues.apache.org/jira/browse/MESOS-5830
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos codebase has been evolved and no more need an extra space
> between two right angle brackets, e.g. vector

Re: Review Request 50887: Trimmed unneeded extra space between right angle brackets.

2016-08-08 Thread Benjamin Bannier

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



Great start!

I grepped the code base and still found many instances of these unneeded spaces 
(with `git grep '> >'`). Could you make a full sweep? Note that changes to 
libprocess and stout would need to be made in two separate patches. Feel free 
to add me as a reviewer.

- Benjamin Bannier


On Aug. 8, 2016, 10:02 a.m., Gaojin CAO wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50887/
> ---
> 
> (Updated Aug. 8, 2016, 10:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Guangya Liu, and haosdent huang.
> 
> 
> Bugs: MESOS-5830
> https://issues.apache.org/jira/browse/MESOS-5830
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos codebase has been evolved and no more need an extra space
> between two right angle brackets, e.g. vector

Re: Review Request 50853: Resolved C++11-related TODO in master/master.cpp.

2016-08-08 Thread Gastón Kleiman


> On Aug. 8, 2016, 6:45 a.m., Benjamin Mahler wrote:
> > Which compilers did you test with?

I tested with:

`gcc 4.8.1, 4.8.4, 4.8.5, 4.9.2, 5.3.1`
`clang 7.0.2`


- Gastón


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


On Aug. 5, 2016, 3:30 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50853/
> ---
> 
> (Updated Aug. 5, 2016, 3:30 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Resolved C++11-related TODO in master/master.cpp.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 92595097c8f26675aee122c8a11366262534db64 
> 
> Diff: https://reviews.apache.org/r/50853/diff/
> 
> 
> Testing
> ---
> 
> `make check` in OS X, Centos 7, Centos 6, Debian 8, Fedora 23, Ubuntu 14, 
> Ubuntu 12, Ubuntu 15, Ubuntu 16
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>



Re: Review Request 50889: Introduce parsing and stringification of `CapabilityInfo`s.

2016-08-08 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [50266, 50889]

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 Aug. 8, 2016, 11:03 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50889/
> ---
> 
> (Updated Aug. 8, 2016, 11:03 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-50501
> https://issues.apache.org/jira/browse/MESOS-50501
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp d0d23566f6262d8c4073c7870cb5546d39017e68 
>   src/common/parse.hpp 5dc795d7f54209abe64ad48360f538faac7616f0 
>   src/common/type_utils.cpp 7110d87ba8078a5cc00669c82f8cd36f103c14b3 
> 
> Diff: https://reviews.apache.org/r/50889/diff/
> 
> 
> Testing
> ---
> 
> `make check` (OS X, clang/trunk, not optimized)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 50812: Updated `HealthCheck` protobuf for HTTP and TCP health check.

2016-08-08 Thread Alexander Rukletsov


> On Aug. 8, 2016, 11:13 a.m., Gastón Kleiman wrote:
> > include/mesos/mesos.proto, line 337
> > 
> >
> > Ideally this should be an enum, but I guess that we want to be 
> > consistent with `message URL`.

That's right. Dropping the issue.


- Alexander


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


On Aug. 5, 2016, 4:19 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50812/
> ---
> 
> (Updated Aug. 5, 2016, 4:19 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Mahler, and 
> Gastón Kleiman.
> 
> 
> Bugs: MESOS-5987
> https://issues.apache.org/jira/browse/MESOS-5987
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `HealthCheck` protobuf for HTTP and TCP health check.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8c74d0bdc1d15074b55d1be84816307bb9478a38 
>   include/mesos/v1/mesos.proto 94b59dd75abfa9e8601e59f7a20dfd94bc88fa70 
> 
> Diff: https://reviews.apache.org/r/50812/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 50812: Updated `HealthCheck` protobuf for HTTP and TCP health check.

2016-08-08 Thread Gastón Kleiman

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


Fix it, then Ship it!




LGTM (assuming that Alex R's suggestions will be applied)


include/mesos/mesos.proto (line 337)


Ideally this should be an enum, but I guess that we want to be consistent 
with `message URL`.


- Gastón Kleiman


On Aug. 5, 2016, 4:19 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50812/
> ---
> 
> (Updated Aug. 5, 2016, 4:19 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Mahler, and 
> Gastón Kleiman.
> 
> 
> Bugs: MESOS-5987
> https://issues.apache.org/jira/browse/MESOS-5987
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `HealthCheck` protobuf for HTTP and TCP health check.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8c74d0bdc1d15074b55d1be84816307bb9478a38 
>   include/mesos/v1/mesos.proto 94b59dd75abfa9e8601e59f7a20dfd94bc88fa70 
> 
> Diff: https://reviews.apache.org/r/50812/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 50812: Updated `HealthCheck` protobuf for HTTP and TCP health check.

2016-08-08 Thread Alexander Rukletsov

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




include/mesos/mesos.proto (lines 373 - 374)


We don't need this anymore, right?


- Alexander Rukletsov


On Aug. 5, 2016, 4:19 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50812/
> ---
> 
> (Updated Aug. 5, 2016, 4:19 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Mahler, and 
> Gastón Kleiman.
> 
> 
> Bugs: MESOS-5987
> https://issues.apache.org/jira/browse/MESOS-5987
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `HealthCheck` protobuf for HTTP and TCP health check.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8c74d0bdc1d15074b55d1be84816307bb9478a38 
>   include/mesos/v1/mesos.proto 94b59dd75abfa9e8601e59f7a20dfd94bc88fa70 
> 
> Diff: https://reviews.apache.org/r/50812/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 50266: Introduced linux capabilities API.

2016-08-08 Thread Benjamin Bannier


> On July 22, 2016, 2:32 a.m., Jie Yu wrote:
> > src/linux/capabilities.cpp, lines 46-48
> > 
> >
> > No need for 'static' here if constexpr is used.
> > 
> > Also, I would prefer calling it:
> > ```
> > PROC_CAP_LAST_CAP
> > ```

I made all variables in this block `constexpr`. I am not sure how storage 
duration would be affected by using `constexpr` since this does not make these 
variable compile-time constants (i.e., the compiler might still allocate 
storage, but it could just as well optimize the variable away), but I also 
dropped the `static` specifiers.

Also, I use the name you suggested now.


> On July 22, 2016, 2:32 a.m., Jie Yu wrote:
> > src/linux/capabilities.cpp, line 47
> > 
> >
> > any reason this is not a constexpr?

`constexpr` now.


> On July 22, 2016, 2:32 a.m., Jie Yu wrote:
> > src/linux/capabilities.cpp, line 48
> > 
> >
> > Ditto here?

Done.


> On July 22, 2016, 2:32 a.m., Jie Yu wrote:
> > src/linux/capabilities.cpp, line 257
> > 
> >
> > s/processCaps/capabilities/

Renamed here and also for the case of the parameters of `doCapSet` and `set` 
below.


> On July 22, 2016, 2:32 a.m., Jie Yu wrote:
> > src/linux/capabilities.cpp, lines 34-39
> > 
> >
> > Any reason why we need to declare them here? Shouldn't 
> > `` already declared them?

These are libc function mapping to sys calls which are only exposed via libcap 
development headers. Declaring them here ourself allows us to not depend on the 
presence of the development header files. I agree this is not something we do 
often. Should we just introduce the dependency on the dev files and do away 
with our own decls of these functions here?


> On July 22, 2016, 2:32 a.m., Jie Yu wrote:
> > src/linux/capabilities.hpp, lines 203-212
> > 
> >
> > This does not belong here. We should put it in 
> > include/mesos/type_utils.hpp and src/common/type_utils.cpp. Please add 
> > those in a separate patch.

I moved the code and added it in https://reviews.apache.org/r/50889/.


> On July 22, 2016, 2:32 a.m., Jie Yu wrote:
> > src/linux/capabilities.hpp, lines 215-228
> > 
> >
> > Let's introduce this in a separate patch.

Moved to https://reviews.apache.org/r/50889/.


- Benjamin


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


On Aug. 8, 2016, 1:03 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50266/
> ---
> 
> (Updated Aug. 8, 2016, 1:03 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5051
> https://issues.apache.org/jira/browse/MESOS-5051
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces basic API for linux capabilities. This is not a
> comprehensive API but is strictly limited to the need for securing Mesos
> containers using linux capabilities.
> 
> This patch is based on the work in https://reviews.apache.org/r/46370/.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 1286ee08fe2d60867326a1f2585f054c20b52208 
>   src/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
>   src/linux/capabilities.hpp PRE-CREATION 
>   src/linux/capabilities.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50266/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Review Request 50889: Introduce parsing and stringification of `CapabilityInfo`s.

2016-08-08 Thread Benjamin Bannier

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  include/mesos/type_utils.hpp d0d23566f6262d8c4073c7870cb5546d39017e68 
  src/common/parse.hpp 5dc795d7f54209abe64ad48360f538faac7616f0 
  src/common/type_utils.cpp 7110d87ba8078a5cc00669c82f8cd36f103c14b3 

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


Testing
---

`make check` (OS X, clang/trunk, not optimized)


Thanks,

Benjamin Bannier



Re: Review Request 50266: Introduced linux capabilities API.

2016-08-08 Thread Benjamin Bannier

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

(Updated Aug. 8, 2016, 1:03 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Addressed some comments, still more work to come.


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


Repository: mesos


Description
---

This change introduces basic API for linux capabilities. This is not a
comprehensive API but is strictly limited to the need for securing Mesos
containers using linux capabilities.

This patch is based on the work in https://reviews.apache.org/r/46370/.


Diffs (updated)
-

  src/CMakeLists.txt 1286ee08fe2d60867326a1f2585f054c20b52208 
  src/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
  src/linux/capabilities.hpp PRE-CREATION 
  src/linux/capabilities.cpp PRE-CREATION 

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


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)


Thanks,

Benjamin Bannier



Re: Review Request 50812: Updated `HealthCheck` protobuf for HTTP and TCP health check.

2016-08-08 Thread Alexander Rukletsov

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




include/mesos/mesos.proto (line 321)


This is not true any more : )


- Alexander Rukletsov


On Aug. 5, 2016, 4:19 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50812/
> ---
> 
> (Updated Aug. 5, 2016, 4:19 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Mahler, and 
> Gastón Kleiman.
> 
> 
> Bugs: MESOS-5987
> https://issues.apache.org/jira/browse/MESOS-5987
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `HealthCheck` protobuf for HTTP and TCP health check.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8c74d0bdc1d15074b55d1be84816307bb9478a38 
>   include/mesos/v1/mesos.proto 94b59dd75abfa9e8601e59f7a20dfd94bc88fa70 
> 
> Diff: https://reviews.apache.org/r/50812/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 50812: Updated `HealthCheck` protobuf for HTTP and TCP health check.

2016-08-08 Thread Alexander Rukletsov

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


Fix it, then Ship it!




I'll address the outstanding issues and will commit shorty.


include/mesos/mesos.proto (lines 327 - 329)


I can't befriend this syntax : ). I'd suggest we omit `*_CHECK` suffixes 
here and rename the messages.



include/mesos/mesos.proto (line 336)


To avoid naming collisiong, let's call it `HTTPCheckInfo`. We usually add 
`*Info` suffix to proto messages to distinguish them from non-generated class 
names. Omitting "health" seems reasonable if we are to introduce other check 
types, e.g. readiness checks.



include/mesos/mesos.proto (line 337)


Let's mention we support either "http" or "https" now.



include/mesos/mesos.proto (line 347)


We already mention this in the beginning.



include/mesos/mesos.proto (lines 354 - 355)


How about:
Consider adding a flag to enable task's certificate validation for HTTPS 
health checks, see MESOS-5997.



include/mesos/mesos.proto (lines 361 - 362)


"..., i.e. based on establishing a TCP connection to the specified port."

for simplicity.



include/mesos/mesos.proto (line 364)


`TCPCheckInfo`? See reasoning above.


- Alexander Rukletsov


On Aug. 5, 2016, 4:19 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50812/
> ---
> 
> (Updated Aug. 5, 2016, 4:19 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Mahler, and 
> Gastón Kleiman.
> 
> 
> Bugs: MESOS-5987
> https://issues.apache.org/jira/browse/MESOS-5987
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `HealthCheck` protobuf for HTTP and TCP health check.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8c74d0bdc1d15074b55d1be84816307bb9478a38 
>   include/mesos/v1/mesos.proto 94b59dd75abfa9e8601e59f7a20dfd94bc88fa70 
> 
> Diff: https://reviews.apache.org/r/50812/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 50887: Trimmed unneeded extra space between right angle brackets.

2016-08-08 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [50887]

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 Aug. 8, 2016, 8:02 a.m., Gaojin CAO wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50887/
> ---
> 
> (Updated Aug. 8, 2016, 8:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Guangya Liu, and haosdent huang.
> 
> 
> Bugs: MESOS-5830
> https://issues.apache.org/jira/browse/MESOS-5830
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos codebase has been evolved and no more need an extra space
> between two right angle brackets, e.g. vector

Re: Review Request 50127: Added DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch.

2016-08-08 Thread Guangya Liu

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




src/tests/containerizer/docker_containerizer_tests.cpp (line 3759)


I think you need put the test case wrappered by `#ifdef __linux__`



src/tests/containerizer/docker_containerizer_tests.cpp (lines 3805 - 3806)


new line here



src/tests/containerizer/docker_containerizer_tests.cpp (lines 3871 - 3872)


s/.get()./->



src/tests/containerizer/docker_containerizer_tests.cpp (line 3874)


s/.get()./->



src/tests/containerizer/docker_containerizer_tests.cpp (line 3886)


s/.get()./->



src/tests/containerizer/docker_containerizer_tests.cpp (line 3889)


s/.get()./->



src/tests/containerizer/docker_containerizer_tests.cpp (line 3903)


s/.get()./->



src/tests/containerizer/docker_containerizer_tests.cpp (line 3906)


s/.get()./->



src/tests/containerizer/docker_containerizer_tests.cpp (lines 3912 - 3914)


s/.get()./->



src/tests/containerizer/docker_containerizer_tests.cpp (line 3919)


s/string name/const string name



src/tests/containerizer/docker_containerizer_tests.cpp (lines 3919 - 3920)


new line here



src/tests/containerizer/docker_containerizer_tests.cpp (line 3921)


kill this line



src/tests/containerizer/docker_containerizer_tests.cpp (lines 3929 - 3930)


new line here


- Guangya Liu


On 八月 5, 2016, 9:53 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50127/
> ---
> 
> (Updated 八月 5, 2016, 9:53 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This added a testing case for end-to-end GPU support for docker
> containerizer.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4e5b1fd1628504d346cced545f7911d6b6443773 
> 
> Diff: https://reviews.apache.org/r/50127/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch" make -j 
> check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>



Re: Review Request 50127: Added DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch.

2016-08-08 Thread Guangya Liu


> On 八月 4, 2016, 3:04 p.m., Guangya Liu wrote:
> > src/tests/containerizer/nvidia_gpu_isolator_tests.cpp, line 758
> > 
> >
> > Do you need this with docker containerizer?
> 
> Yubo Li wrote:
> Currently we need. When I remove this line, the test reports error:
> resources: '--nvidia_gpus_devices' can only be specified if the 
> `--isolation` flag contains 'gpu/nvidia'.
> 
> Actually, docker containerizer does not need `--isolation`, but currently 
> `--nvidia_gpus_devices` forces us to do this. We would talk with Ben and 
> Kevin to remove this retriction for docker containerizer soon.

I see, did you initate any discussion with them?


- Guangya


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


On 八月 5, 2016, 9:53 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50127/
> ---
> 
> (Updated 八月 5, 2016, 9:53 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This added a testing case for end-to-end GPU support for docker
> containerizer.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4e5b1fd1628504d346cced545f7911d6b6443773 
> 
> Diff: https://reviews.apache.org/r/50127/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch" make -j 
> check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>



Review Request 50887: Trimmed unneeded extra space between right angle brackets.

2016-08-08 Thread Gaojin CAO

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

Review request for mesos, Guangya Liu and haosdent huang.


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


Repository: mesos


Description
---

Mesos codebase has been evolved and no more need an extra space
between two right angle brackets, e.g. vector

Re: Review Request 49924: Added libprocess as a shared library.

2016-08-08 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [49688, 49862, 49863, 49870, 49874, 49921, 49924]

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 Aug. 8, 2016, 5:25 a.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49924/
> ---
> 
> (Updated Aug. 8, 2016, 5:25 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-5792
> https://issues.apache.org/jira/browse/MESOS-5792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch allow to build libprocess as shared library on OSX and Linux
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/CMakeLists.txt 
> d1547ef6a8762385f653d3824307727e4d0a7e71 
> 
> Diff: https://reviews.apache.org/r/49924/diff/
> 
> 
> Testing
> ---
> 
> cmake .. && make
> 
> With this patch and https://reviews.apache.org/r/49862,  Converted libmesos, 
> http_parser and libprocess to shared libraries and we are using libevent 
> shared library, zookeeper does not have a shared library in the 3rdparty (I 
> guess the code is compiled as relocatable) and did not have issues linking.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>



Re: Review Request 50853: Resolved C++11-related TODO in master/master.cpp.

2016-08-08 Thread Benjamin Mahler

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



Which compilers did you test with?

- Benjamin Mahler


On Aug. 5, 2016, 3:30 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50853/
> ---
> 
> (Updated Aug. 5, 2016, 3:30 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Resolved C++11-related TODO in master/master.cpp.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 92595097c8f26675aee122c8a11366262534db64 
> 
> Diff: https://reviews.apache.org/r/50853/diff/
> 
> 
> Testing
> ---
> 
> `make check` in OS X, Centos 7, Centos 6, Debian 8, Fedora 23, Ubuntu 14, 
> Ubuntu 12, Ubuntu 15, Ubuntu 16
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>



Re: Review Request 50125: Added mesos-docker-executor support for device control.

2016-08-08 Thread Guangya Liu

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




src/docker/executor.hpp (lines 78 - 79)


what about rename it as `devices`?

Ditto for the following.



src/docker/executor.cpp (line 163)


s/vector deviceList/const vector deviceList



src/docker/executor.cpp (line 164)


s/dev/device, we prefer a full name here for easy to understand.



src/docker/executor.cpp (lines 170 - 171)


kill this, the `parse` already logged the message.


- Guangya Liu


On 八月 5, 2016, 9:52 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50125/
> ---
> 
> (Updated 八月 5, 2016, 9:52 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a new flag '--device' to mesos-docker-executor, and gave its
> feature to control device exposition, isolation, and access permission.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.hpp 7b63d784d6b8685912598b77fb38cf6e70646ae3 
>   src/docker/executor.cpp 445628c9164facdd8bd812c5b45e3b2b886ebf0e 
>   src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 
> 
> Diff: https://reviews.apache.org/r/50125/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>



Re: Review Request 50599: Passed allocated GPUs to 'device' entry of 'docker::Flags'.

2016-08-08 Thread Guangya Liu

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




src/slave/containerizer/docker.cpp (lines 1344 - 1383)


This should be moved to a new function per the comments 
`DockerContainerizerProcess::_launchExecutorProcess` at 
https://reviews.apache.org/r/50841/ .


- Guangya Liu


On 八月 5, 2016, 9:51 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated 八月 5, 2016, 9:51 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 a new string entry 'device' to 'docker::Flags' to
> indicate host devices exposed into docker container, and
> passed allocated GPUs to it.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>