Re: Review Request 42113: WIP: Handle unreserve logic for dynamic reservation (2/3).

2016-01-13 Thread Guangya Liu

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

(Updated 一月 13, 2016, 9:03 a.m.)


Review request for mesos and Klaus Ma.


Summary (updated)
-

WIP: Handle unreserve logic for dynamic reservation (2/3).


Repository: mesos


Description
---

Handle unreserve logic for dynamic reservation with allocation slack.

This patch is halding the case when using updateAllocation to unreserve
some resources.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
  src/tests/hierarchical_allocator_tests.cpp 
e044f832c2c16e53e663c6ced5452649bb0dcb59 

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


Testing
---

make
make check


Thanks,

Guangya Liu



Re: Review Request 41876: Used std::list as opposed to List from stout.

2016-01-13 Thread Benjamin Bannier

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

(Updated Jan. 13, 2016, 11:02 a.m.)


Review request for mesos, Ben Mahler and Till Toenshoff.


Changes
---

Removed old and unused dep (which created a cycle here that made 
apply-reviews.py hang).


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


Repository: mesos


Description
---

List from stout provides no advantage over std::list now that C++11 is
available.

Review: https://reviews.apache.org/r/41876


Diffs
-

  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
8f0a31341eebb168f78bc74f8afbaef9ba98e847 
  3rdparty/libprocess/src/tests/timeseries_tests.cpp 
274598e63d28150aaa0ad06fa2d85e41e525c153 

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


Testing
---


Thanks,

Benjamin Bannier



Review Request 42246: Fixed support for non-HDFS URIs by Hadoop client.

2016-01-13 Thread Bernd Mathiske

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

Review request for mesos, haosdent huang, Jie Yu, and switched to 'mcypark'.


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


Repository: mesos


Description
---

Fixed support for non-HDFS URIs by Hadoop client.


Diffs
-

  src/hdfs/hdfs.cpp 51f016b049c6e947c4d27fbbbf79c53f9ec5a51e 

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


Testing
---

make check


Thanks,

Bernd Mathiske



Re: Review Request 41243: Updated how we find the .git directory in bootstrap.

2016-01-13 Thread Benjamin Bannier

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

Ship it!


Ship It!

- Benjamin Bannier


On Jan. 13, 2016, 11:34 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41243/
> ---
> 
> (Updated Jan. 13, 2016, 11:34 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joseph Wu, 
> Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-4125
> https://issues.apache.org/jira/browse/MESOS-4125
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When building from git, bootstrap will (among other things) install
> pre-commit and post-rewrite hooks into the .git/hooks directory of the
> mesos tree. However the current implementation always assumes that .git
> exists in the same directory as the bootstrap file. This may not always
> be true.
> 
> Most notably, it is not true if the mesos tree is included as a
> submodule inside another project. When included as a submodule, .git is
> no longer a directory, but rather a file whose text contains a pointer
> back to the actual location of the .git folder inside the containing
> project. To get at this directory, we need to run 'git rev-parse
> --git-common-dir' instead of simply assuming that the local .git is the
> proper directory.
> 
> Review: https://reviews.apache.org/r/41243
> 
> 
> Diffs
> -
> 
>   bootstrap ea71ff22e42bf5e224483508d91c33ddfd350f52 
> 
> Diff: https://reviews.apache.org/r/41243/diff/
> 
> 
> Testing
> ---
> 
> Tested on native Mac OS X El Capitan
> Tested on vagrant boxes for bento/centos-7.1, ubuntu/trusty64, and 
> https://github.com/tommy-muehle/puppet-vagrant-boxes/releases/download/1.0.0/centos-6.6-x86_64.box
> Also tested on docker image centos:7
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 41882: Constrained types used in Flags instantiation.

2016-01-13 Thread Benjamin Bannier

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

(Updated Jan. 13, 2016, 11:43 a.m.)


Review request for mesos, Benjamin Hindman and Till Toenshoff.


Changes
---

Used an improved impl (that e.g., is less repetitive).


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


Repository: mesos


Description (updated)
---

To make the implementation less repetitive reimplemented `FlagsBase`
with a variadic template; this allows to now use an arbitrary number of
`FlagsBase` bases.

Since `Flags<>` does already inherit virtually from `FlagsBase` it
should be a safer base class than `FlagsBase` for users.

Review: https://reviews.apache.org/r/41882/


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
addef78ddeb0007cf1e1c79738381138a18a35b6 

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


Testing
---


Thanks,

Benjamin Bannier



Re: Review Request 42185: Added an example executor based on the new V1 API.

2016-01-13 Thread Qian Zhang

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



src/examples/test_http_executor.cpp (line 130)


Should the parameter be ```const queue& events"```?



src/examples/test_http_executor.cpp (line 140)


Why do we need the first `endl`?



src/examples/test_http_executor.cpp (line 148)


s/event.launch().task()/task



src/examples/test_http_executor.cpp (line 154)


Here do we need to set task state to `TASK_FINISHED` like test_executor.c?



src/examples/test_http_executor.cpp (line 187)


Suggest to add a `default` case in the end, like:
```
default: {
  EXIT(1) << "Received an UNKNOWN event";
}
```


- Qian Zhang


On Jan. 12, 2016, 5:23 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42185/
> ---
> 
> (Updated Jan. 12, 2016, 5:23 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4255
> https://issues.apache.org/jira/browse/MESOS-4255
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a custom executor based on the new executor library.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/examples/test_http_executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42185/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 42053: Added flags to set size of completed task/framework history.

2016-01-13 Thread Kevin Klues

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

(Updated Jan. 13, 2016, 10:42 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Missed one change from flags->masterFlags per Adam's previous comments


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


Repository: mesos


Description
---

The default size of the buffers used to hold the state of completed
tasks/frameworks is very large. However, many frameworks don't care much
about this information when requesting a master's state. Moreover, if a
large number of frameworks request this state simultaneously, the
master can quickly become overwhelmed because the process of generating
this state both blocks the master and takes up a lot of cycles. By
allowing the master to configure the size of the buffers used to hold
this state, we give it the power to significantly reduce the amount of
state it needs to maintain.

This change allows the master to limit the size of this state via
command line flags.

This commit is based on a pull request generated by Felix Bechstein at:
https://github.com/apache/mesos/pull/82

Review: https://reviews.apache.org/r/42053


Diffs (updated)
-

  docs/configuration.md 7e0eb9584f3cb24a388c44d7bc81cd87dbb4072e 
  src/master/constants.hpp ebab341e58035d4b579828add752c1ee37efeb95 
  src/master/constants.cpp 77dd31430776e4f24e6e074c1470edcb19e58449 
  src/master/flags.hpp d923b1b0444d7e9023f1db4cbc4f7d4b84c20ff5 
  src/master/flags.cpp 88909590ff421421659e6faac7f3444bdc57b630 
  src/master/master.hpp f02d165874fa8023675e545793de699aeecae29b 
  src/master/master.cpp c122c30d943813fc3ce9e7025783c7231809b022 

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


Testing
---

On Darwin I launched a master with:

./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos 
--max_completed_tasks_per_framework=2 --max_completed_frameworks=1

and a slave with:

./bin/mesos-slave.sh --master=127.0.0.1:5050

and then ran a bunch of instances of:

./src/test-framework --master=127.0.0.1:5050

each of which runs 5 tasks to completion

I then ran:

curl http://localhost:5050/tasks

and verified that only 1 framework and 2 of its completed tasks were given back 
to me in the json that was returned.  I repeated this for a number of other 
configurations with max_completed_frameworks=0..2 and 
max_completed_tasks_per_framework=0..5 and verified visually that the proper 
number of tasks/frameworks were being returned by the /tasks endpoint.

There is alos now a unit test for this as reviewed here:
https://reviews.apache.org/r/42053/


Thanks,

Kevin Klues



Re: Review Request 41876: Used std::list as opposed to List from stout.

2016-01-13 Thread Alexander Rojas

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

Ship it!


Ship It!

- Alexander Rojas


On Jan. 13, 2016, 11:02 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41876/
> ---
> 
> (Updated Jan. 13, 2016, 11:02 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Till Toenshoff.
> 
> 
> Bugs: MESOS-4273
> https://issues.apache.org/jira/browse/MESOS-4273
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> List from stout provides no advantage over std::list now that C++11 is
> available.
> 
> Review: https://reviews.apache.org/r/41876
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> 8f0a31341eebb168f78bc74f8afbaef9ba98e847 
>   3rdparty/libprocess/src/tests/timeseries_tests.cpp 
> 274598e63d28150aaa0ad06fa2d85e41e525c153 
> 
> Diff: https://reviews.apache.org/r/41876/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 41243: Updated how we find the .git directory in bootstrap.

2016-01-13 Thread Till Toenshoff

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



bootstrap (line 15)


This would introduce Python as a hard dependency of the Mesos build 
environment.

We should avoid this.


- Till Toenshoff


On Jan. 11, 2016, 8:12 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41243/
> ---
> 
> (Updated Jan. 11, 2016, 8:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joseph Wu, 
> Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-4125
> https://issues.apache.org/jira/browse/MESOS-4125
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When building from git, bootstrap will (among other things) install
> pre-commit and post-rewrite hooks into the .git/hooks directory of the
> mesos tree. However the current implementation always assumes that .git
> exists in the same directory as the bootstrap file. This may not always
> be true.
> 
> Most notably, it is not true if the mesos tree is included as a
> submodule inside another project. When included as a submodule, .git is
> no longer a directory, but rather a file whose text contains a pointer
> back to the actual location of the .git folder inside the containing
> project. To get at this directory, we need to run 'git rev-parse
> --git-common-dir' instead of simply assuming that the local .git is the
> proper directory.
> 
> Review: https://reviews.apache.org/r/41243
> 
> 
> Diffs
> -
> 
>   bootstrap ea71ff22e42bf5e224483508d91c33ddfd350f52 
> 
> Diff: https://reviews.apache.org/r/41243/diff/
> 
> 
> Testing
> ---
> 
> Tested on native Mac OS X El Capitan
> Tested on vagrant boxes for bento/centos-7.1, ubuntu/trusty64, and 
> https://github.com/tommy-muehle/puppet-vagrant-boxes/releases/download/1.0.0/centos-6.6-x86_64.box
> Also tested on docker image centos:7
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 42053: Added flags to set size of completed task/framework history.

2016-01-13 Thread Kevin Klues

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

(Updated Jan. 13, 2016, 10:43 a.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

The default size of the buffers used to hold the state of completed
tasks/frameworks is very large. However, many frameworks don't care much
about this information when requesting a master's state. Moreover, if a
large number of frameworks request this state simultaneously, the
master can quickly become overwhelmed because the process of generating
this state both blocks the master and takes up a lot of cycles. By
allowing the master to configure the size of the buffers used to hold
this state, we give it the power to significantly reduce the amount of
state it needs to maintain.

This change allows the master to limit the size of this state via
command line flags.

This commit is based on a pull request generated by Felix Bechstein at:
https://github.com/apache/mesos/pull/82

Review: https://reviews.apache.org/r/42053


Diffs
-

  docs/configuration.md 7e0eb9584f3cb24a388c44d7bc81cd87dbb4072e 
  src/master/constants.hpp ebab341e58035d4b579828add752c1ee37efeb95 
  src/master/constants.cpp 77dd31430776e4f24e6e074c1470edcb19e58449 
  src/master/flags.hpp d923b1b0444d7e9023f1db4cbc4f7d4b84c20ff5 
  src/master/flags.cpp 88909590ff421421659e6faac7f3444bdc57b630 
  src/master/master.hpp f02d165874fa8023675e545793de699aeecae29b 
  src/master/master.cpp c122c30d943813fc3ce9e7025783c7231809b022 

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


Testing
---

On Darwin I launched a master with:

./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos 
--max_completed_tasks_per_framework=2 --max_completed_frameworks=1

and a slave with:

./bin/mesos-slave.sh --master=127.0.0.1:5050

and then ran a bunch of instances of:

./src/test-framework --master=127.0.0.1:5050

each of which runs 5 tasks to completion

I then ran:

curl http://localhost:5050/tasks

and verified that only 1 framework and 2 of its completed tasks were given back 
to me in the json that was returned.  I repeated this for a number of other 
configurations with max_completed_frameworks=0..2 and 
max_completed_tasks_per_framework=0..5 and verified visually that the proper 
number of tasks/frameworks were being returned by the /tasks endpoint.

There is alos now a unit test for this as reviewed here:
https://reviews.apache.org/r/42053/


Thanks,

Kevin Klues



Re: Review Request 42165: Removed `hasPrincipal` parameter from unreserve validation.

2016-01-13 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [42164, 42165]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 11, 2016, 10:40 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42165/
> ---
> 
> (Updated Jan. 11, 2016, 10:40 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3940
> https://issues.apache.org/jira/browse/MESOS-3940
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed `hasPrincipal` parameter from unreserve validation.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d 
>   src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
>   src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/tests/master_validation_tests.cpp 
> fbf8fadbc04a7cbc60ee6091e0224339389b400f 
> 
> Diff: https://reviews.apache.org/r/42165/diff/
> 
> 
> Testing
> ---
> 
> Several instances of the relevant validation function were altered in the 
> master validation tests.
> 
> `make check` was used to test on both OSX and Ubuntu 14.04
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 42234: WIP: Added test case on re-using evicting executors.

2016-01-13 Thread Mesos ReviewBot

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


Bad patch!

Reviews applied: [40375, 41334, 41333, 40529, 41772, 40339, 40632]

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

Error:
 2016-01-13 10:11:13 URL:https://reviews.apache.org/r/40632/diff/raw/ 
[14855/14855] -> "40632.patch" [1]
error: patch failed: src/master/allocator/mesos/hierarchical.cpp:434
error: src/master/allocator/mesos/hierarchical.cpp: patch does not apply

- Mesos ReviewBot


On Jan. 13, 2016, 6:14 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42234/
> ---
> 
> (Updated Jan. 13, 2016, 6:14 a.m.)
> 
> 
> Review request for mesos, Guangya Liu and Jian Qiu.
> 
> 
> Bugs: MESOS-3897
> https://issues.apache.org/jira/browse/MESOS-3897
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test case on re-using evicting executors.
> 
> 
> Diffs
> -
> 
>   src/tests/oversubscription_tests.cpp 
> 7a75fb38e0177e33cf0e7cb82b4b9ebf8f05fe0a 
> 
> Diff: https://reviews.apache.org/r/42234/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 41882: Constrained types used in Flags instantiation.

2016-01-13 Thread Alexander Rojas

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

Ship it!


Ship It!

- Alexander Rojas


On Jan. 13, 2016, 11:43 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41882/
> ---
> 
> (Updated Jan. 13, 2016, 11:43 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Bugs: MESOS-4278
> https://issues.apache.org/jira/browse/MESOS-4278
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To make the implementation less repetitive reimplemented `FlagsBase`
> with a variadic template; this allows to now use an arbitrary number of
> `FlagsBase` bases.
> 
> Since `Flags<>` does already inherit virtually from `FlagsBase` it
> should be a safer base class than `FlagsBase` for users.
> 
> Review: https://reviews.apache.org/r/41882/
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
> addef78ddeb0007cf1e1c79738381138a18a35b6 
> 
> Diff: https://reviews.apache.org/r/41882/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 42221: Removed references to wDRF from allocator.

2016-01-13 Thread Alexander Rukletsov


> On Jan. 13, 2016, 1:52 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1185
> > 
> >
> > s/framwork/frameworks

Why do you want plural here?


- Alexander


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


On Jan. 12, 2016, 11:06 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42221/
> ---
> 
> (Updated Jan. 12, 2016, 11:06 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since wDRF is encapsulated in the choice of sorter, it should not be
> mentioned in the allocator code.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
> 
> Diff: https://reviews.apache.org/r/42221/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Re: Review Request 42221: Removed references to wDRF from allocator.

2016-01-13 Thread Alexander Rukletsov


> On Jan. 13, 2016, 1:52 a.m., Guangya Liu wrote:
> > Since the comments are mainly to developers, the `second stage` is not very 
> > clear to developers, but the `WDRF` seems more clear. If want to update, 
> > what about "fair share stage"?

The fairness of the "second" stage is determined by sorters as well. For 
example, one may write a sorter that favors roles starting with "alex-" : ). 
What allocator knows is that there are two stages: the first where only 
quota'ed frameworks are processed and the second for all frameworks.


- Alexander


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


On Jan. 12, 2016, 11:06 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42221/
> ---
> 
> (Updated Jan. 12, 2016, 11:06 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since wDRF is encapsulated in the choice of sorter, it should not be
> mentioned in the allocator code.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
> 
> Diff: https://reviews.apache.org/r/42221/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Re: Review Request 41243: Updated how we find the .git directory in bootstrap.

2016-01-13 Thread Till Toenshoff


> On Jan. 13, 2016, 10:04 a.m., Till Toenshoff wrote:
> > bootstrap, line 15
> > 
> >
> > This would introduce Python as a hard dependency of the Mesos build 
> > environment.
> > 
> > We should avoid this.
> 
> Kevin Klues wrote:
> If you look back at revision 1, I had a pure bash implemetation of 
> _relpath, but Benjamin Bannier suggested I change it to python since it would 
> be more maintainable. His comments on the matter can be seen above.

Indeed - my bad - I should have commented on that suggestion. Lets go back to 
bash as bootstrap should aim for minimized depedencies.


- Till


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


On Jan. 11, 2016, 8:12 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41243/
> ---
> 
> (Updated Jan. 11, 2016, 8:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joseph Wu, 
> Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-4125
> https://issues.apache.org/jira/browse/MESOS-4125
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When building from git, bootstrap will (among other things) install
> pre-commit and post-rewrite hooks into the .git/hooks directory of the
> mesos tree. However the current implementation always assumes that .git
> exists in the same directory as the bootstrap file. This may not always
> be true.
> 
> Most notably, it is not true if the mesos tree is included as a
> submodule inside another project. When included as a submodule, .git is
> no longer a directory, but rather a file whose text contains a pointer
> back to the actual location of the .git folder inside the containing
> project. To get at this directory, we need to run 'git rev-parse
> --git-common-dir' instead of simply assuming that the local .git is the
> proper directory.
> 
> Review: https://reviews.apache.org/r/41243
> 
> 
> Diffs
> -
> 
>   bootstrap ea71ff22e42bf5e224483508d91c33ddfd350f52 
> 
> Diff: https://reviews.apache.org/r/41243/diff/
> 
> 
> Testing
> ---
> 
> Tested on native Mac OS X El Capitan
> Tested on vagrant boxes for bento/centos-7.1, ubuntu/trusty64, and 
> https://github.com/tommy-muehle/puppet-vagrant-boxes/releases/download/1.0.0/centos-6.6-x86_64.box
> Also tested on docker image centos:7
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 41243: Updated how we find the .git directory in bootstrap.

2016-01-13 Thread Kevin Klues

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

(Updated Jan. 13, 2016, 10:34 a.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joseph Wu, Till 
Toenshoff, and Vinod Kone.


Changes
---

Reverted back to using the sh implementation of relPath rather than the python 
one.


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


Repository: mesos


Description
---

When building from git, bootstrap will (among other things) install
pre-commit and post-rewrite hooks into the .git/hooks directory of the
mesos tree. However the current implementation always assumes that .git
exists in the same directory as the bootstrap file. This may not always
be true.

Most notably, it is not true if the mesos tree is included as a
submodule inside another project. When included as a submodule, .git is
no longer a directory, but rather a file whose text contains a pointer
back to the actual location of the .git folder inside the containing
project. To get at this directory, we need to run 'git rev-parse
--git-common-dir' instead of simply assuming that the local .git is the
proper directory.

Review: https://reviews.apache.org/r/41243


Diffs (updated)
-

  bootstrap ea71ff22e42bf5e224483508d91c33ddfd350f52 

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


Testing
---

Tested on native Mac OS X El Capitan
Tested on vagrant boxes for bento/centos-7.1, ubuntu/trusty64, and 
https://github.com/tommy-muehle/puppet-vagrant-boxes/releases/download/1.0.0/centos-6.6-x86_64.box
Also tested on docker image centos:7


Thanks,

Kevin Klues



Re: Review Request 42246: Fixed support for non-HDFS URIs by Hadoop client.

2016-01-13 Thread haosdent huang

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

Ship it!


Ship It!

- haosdent huang


On Jan. 13, 2016, 10:07 a.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42246/
> ---
> 
> (Updated Jan. 13, 2016, 10:07 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and switched to 'mcypark'.
> 
> 
> Bugs: MESOS-4304
> https://issues.apache.org/jira/browse/MESOS-4304
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed support for non-HDFS URIs by Hadoop client.
> 
> 
> Diffs
> -
> 
>   src/hdfs/hdfs.cpp 51f016b049c6e947c4d27fbbbf79c53f9ec5a51e 
> 
> Diff: https://reviews.apache.org/r/42246/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>



Re: Review Request 42222: Added a comment on allocator recovery.

2016-01-13 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [42221, 4]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 12, 2016, 11:06 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4/
> ---
> 
> (Updated Jan. 12, 2016, 11:06 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
> 
> Diff: https://reviews.apache.org/r/4/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Re: Review Request 42194: WIP: Handle unreserve logic for dynamic reservation (3/3).

2016-01-13 Thread Guangya Liu

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

(Updated 一月 13, 2016, 9:58 a.m.)


Review request for mesos and Klaus Ma.


Summary (updated)
-

WIP: Handle unreserve logic for dynamic reservation (3/3).


Repository: mesos


Description
---

Handle unreserve logic for dynamic reservation with allocation slack.

This patch is halding the case when using updateAvailable to unreserve
some resources.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
  src/tests/hierarchical_allocator_tests.cpp 
e044f832c2c16e53e663c6ced5452649bb0dcb59 

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


Testing
---

make
make check


Thanks,

Guangya Liu



Re: Review Request 41243: Updated how we find the .git directory in bootstrap.

2016-01-13 Thread Kevin Klues


> On Jan. 13, 2016, 10:04 a.m., Till Toenshoff wrote:
> > bootstrap, line 15
> > 
> >
> > This would introduce Python as a hard dependency of the Mesos build 
> > environment.
> > 
> > We should avoid this.

If you look back at revision 1, I had a pure bash implemetation of _relpath, 
but Benjamin Bannier suggested I change it to python since it would be more 
maintainable. His comments on the matter can be seen above.


- Kevin


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


On Jan. 11, 2016, 8:12 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41243/
> ---
> 
> (Updated Jan. 11, 2016, 8:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joseph Wu, 
> Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-4125
> https://issues.apache.org/jira/browse/MESOS-4125
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When building from git, bootstrap will (among other things) install
> pre-commit and post-rewrite hooks into the .git/hooks directory of the
> mesos tree. However the current implementation always assumes that .git
> exists in the same directory as the bootstrap file. This may not always
> be true.
> 
> Most notably, it is not true if the mesos tree is included as a
> submodule inside another project. When included as a submodule, .git is
> no longer a directory, but rather a file whose text contains a pointer
> back to the actual location of the .git folder inside the containing
> project. To get at this directory, we need to run 'git rev-parse
> --git-common-dir' instead of simply assuming that the local .git is the
> proper directory.
> 
> Review: https://reviews.apache.org/r/41243
> 
> 
> Diffs
> -
> 
>   bootstrap ea71ff22e42bf5e224483508d91c33ddfd350f52 
> 
> Diff: https://reviews.apache.org/r/41243/diff/
> 
> 
> Testing
> ---
> 
> Tested on native Mac OS X El Capitan
> Tested on vagrant boxes for bento/centos-7.1, ubuntu/trusty64, and 
> https://github.com/tommy-muehle/puppet-vagrant-boxes/releases/download/1.0.0/centos-6.6-x86_64.box
> Also tested on docker image centos:7
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 42212: Added unit test for framework/task history flags.

2016-01-13 Thread Kevin Klues

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

(Updated Jan. 13, 2016, 11:01 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Change EXPECT_LE to EXPECT_EQ for comparing maxFrameworks/numFrameworks and 
maxTasksPerFramework/numTaskPerFramework. This change is OK, and actually more 
correct given the setup of the test (i.e. we will always be limited by max*). 
Also fix up some comments.


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


Repository: mesos


Description
---

This commit adds a unit test to verify that the the max_frameworks and
max_tasks_per_frameworks flags for master work properly. Specifically,
we test to verify that the proper amount of history is maintained for
both 0 values to these flags as well as positive values <= to the total
number frameworks and tasks per framework actually launched.


Diffs (updated)
-

  src/tests/master_tests.cpp 223b9d20a3a8a8194a3a6a605ec2394c37ab5957 

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


Testing
---

This is a unit test.  I ran it on my mac and on ubuntu 14.04.

GTEST_FILTER="MasterTest.FrameworksTasksCompletedFlags" make check -j 7


Thanks,

Kevin Klues



Re: Review Request 42194: WIP: Handle unreserve logic for dynamic reservation (3/3).

2016-01-13 Thread Mesos ReviewBot

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


Bad patch!

Reviews applied: [40375, 41334, 41333, 40529, 41772, 40339, 40632]

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

Error:
 2016-01-13 11:25:26 URL:https://reviews.apache.org/r/40632/diff/raw/ 
[14855/14855] -> "40632.patch" [1]
error: patch failed: src/master/allocator/mesos/hierarchical.cpp:434
error: src/master/allocator/mesos/hierarchical.cpp: patch does not apply

- Mesos ReviewBot


On Jan. 13, 2016, 9:58 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42194/
> ---
> 
> (Updated Jan. 13, 2016, 9:58 a.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Handle unreserve logic for dynamic reservation with allocation slack.
> 
> This patch is halding the case when using updateAvailable to unreserve
> some resources.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/42194/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2016-01-13 Thread Klaus Ma

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


ping @mcypark :).

- Klaus Ma


On Dec. 13, 2015, 11:28 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated Dec. 13, 2015, 11:28 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9762f85 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 5211f54 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 42248: Fixed markdown parsing in markdown-styleguide.

2016-01-13 Thread Joerg Schad

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

(Updated Jan. 13, 2016, 12:42 p.m.)


Review request for mesos, Joerg Schad and Till Toenshoff.


Repository: mesos


Description (updated)
---

As the website markdown parser supports fewer syntax marker (compared to 
github), the rendered markdown-styleguide was broken.


Diffs
-

  docs/markdown-style-guide.md fe9ffefbbb2c0ce9ade39cec1de073163a9a03b2 

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


Testing
---

Viewed via docker webseite generator.


Thanks,

Joerg Schad



Re: Review Request 41791: Updated allocation slack for dynamic reserve (1/3).

2016-01-13 Thread Guangya Liu

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

(Updated 一月 13, 2016, 12:58 p.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, Klaus Ma, and Jian Qiu.


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


Repository: mesos


Description
---

Update allocation slack resources if reserve some
new stateless reserved resources. For such case,
the allocation slack resources will be increased.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
  src/tests/hierarchical_allocator_tests.cpp 
e044f832c2c16e53e663c6ced5452649bb0dcb59 

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


Testing
---

make
make check


Thanks,

Guangya Liu



Re: Review Request 39782: Add a comment for os::libraries::setPaths.

2016-01-13 Thread Till Toenshoff

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

Ship it!


Ship It!

- Till Toenshoff


On Jan. 12, 2016, 10:14 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39782/
> ---
> 
> (Updated Jan. 12, 2016, 10:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Kapil Arya, Niklas Nielsen, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-3725
> https://issues.apache.org/jira/browse/MESOS-3725
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a comment for os::libraries::setPaths.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758 
> 
> Diff: https://reviews.apache.org/r/39782/diff/
> 
> 
> Testing
> ---
> 
> No code changes.
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 42194: Handle unreserve logic for dynamic reservation (3/3).

2016-01-13 Thread Guangya Liu

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

(Updated 一月 13, 2016, 1:10 p.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, Klaus Ma, and Jian Qiu.


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


Repository: mesos


Description
---

Handle unreserve logic for dynamic reservation with allocation slack.

This patch is halding the case when using updateAvailable to unreserve
some resources.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
  src/tests/hierarchical_allocator_tests.cpp 
e044f832c2c16e53e663c6ced5452649bb0dcb59 

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


Testing
---

make
make check


Thanks,

Guangya Liu



Re: Review Request 41769: Made allocator traverse all roles for quota allocation.

2016-01-13 Thread Alexander Rukletsov


> On Jan. 13, 2016, 11:41 a.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1164-1166
> > 
> >
> > This looks like a bug to me. I can't remember why we have `break` here 
> > in the first place. Could you please elaborate why do you think `break` is 
> > fine here?
> 
> Guangya Liu wrote:
> It is a bit tricky here so I add some comments here even though I prefer 
> we use `continue` here.
> 
> The reason that it is OK to use `break` is because all of the quota roles 
> are sorted based on starvation, if the roles in quota role sorter is 
> satisfied, we can assume that all roles behind current satisfied role is also 
> satisfied, that's why `break` here. Comments?

I'm not sure it's right. Roles are sorted according to their allocation, hence 
a role with quota 5 and allocation 5 will come before role with quota 50 and 
allocation 10. I believe `continue` should be used here. Also imagine a custom 
sorter is used, which does not guarantee any ordering.

Anyway, I've checked all the tests we have and it seems there is no test where 
we set quotas for multiple roles. Do you want to add such test or expand an 
existing one? Also we may want to add some tests around implicit roles and 
cover the other issue you fix in this test. There is a ticket for this already: 
MESOS-4292. We have to figure out what exactly we want to test and sync with 
NeilC.

I'd be happy to help out and review, this looks like a bug! 

Could you please also keep Joris and Qian Zhang in the loop? Thanks!


- Alexander


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


On Dec. 31, 2015, 11:34 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> ---
> 
> (Updated Dec. 31, 2015, 11:34 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Klaus Ma, and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made allocator traverse all roles for quota allocation.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 7f900c4e024485704d79e57ae22407557598fe6c 
> 
> Diff: https://reviews.apache.org/r/41769/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 41438: Added documentation on using network proxy for mesos fetcher

2016-01-13 Thread Joerg Schad

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



docs/fetcher.md (line 226)


s/mesos/Mesos (or otherwise consistent with the rest of the file)



docs/fetcher.md (line 231)


does protocol here has the same limitation as below in line 241 
(socks4/socks5/http/https)?



docs/fetcher.md (line 232)


s/uri/URI or consistent with the rest of the file...



docs/fetcher.md (line 260)


Any reason for two blank lines here?


- Joerg Schad


On Dec. 24, 2015, 1:22 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41438/
> ---
> 
> (Updated Dec. 24, 2015, 1:22 p.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Bugs: MESOS-4130
> https://issues.apache.org/jira/browse/MESOS-4130
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation on using network proxy for mesos fetcher
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md ec7372293491ed9ee4e20dda3c2def72a77a3f84 
> 
> Diff: https://reviews.apache.org/r/41438/diff/
> 
> 
> Testing
> ---
> 
> - Set `https_proxy=http://localhost:3128` (a local squid server) in 
> `/etc/default/mesos-slave`, restart mesos-slave, launch a new marathon task 
> with a `https://` uri, and through the following sysdig command I can see 
> there is data exchanging between mesos-fetcher process and port 3128.
> 
> ```
> sudo sysdig -A -c echo_fds proc.name=mesos-fetcher and fd.port=3128
> ```
> 
> - Stop the local squid server, try to restart the marathon task, the task 
> would fail repeatly, from slave logs there are error messages that fetcher 
> failed to fetch the uri.
> ```
> Dec 16 15:16:35 lin-E400 mesos-slave[24247]: E1216 15:16:35.678032 24283 
> slave.cpp:3342] Container '45c14132-c56a-4cff-a6b5-f57ba2670643' for executor 
> 'testapp_web.f0ef72d2-a3c4-11e5-af60-56847afe9799' of framework 
> 'b52179fd-8968-4
> bf8-baf0-dddc8a38c903-' failed to start: Failed to fetch all URIs for 
> container '45c14132-c56a-4cff-a6b5-f57ba2670643' with exit status: 2
> ```
> 
> - Restart the squid server, the task would start without a problem.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>



Re: Review Request 41438: Added documentation on using network proxy for mesos fetcher

2016-01-13 Thread Joerg Schad

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

Ship it!


(after the issues in my previous review are fixed)


docs/fetcher.md (line 239)


s/is of the format/has the format:?


- Joerg Schad


On Dec. 24, 2015, 1:22 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41438/
> ---
> 
> (Updated Dec. 24, 2015, 1:22 p.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Bugs: MESOS-4130
> https://issues.apache.org/jira/browse/MESOS-4130
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation on using network proxy for mesos fetcher
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md ec7372293491ed9ee4e20dda3c2def72a77a3f84 
> 
> Diff: https://reviews.apache.org/r/41438/diff/
> 
> 
> Testing
> ---
> 
> - Set `https_proxy=http://localhost:3128` (a local squid server) in 
> `/etc/default/mesos-slave`, restart mesos-slave, launch a new marathon task 
> with a `https://` uri, and through the following sysdig command I can see 
> there is data exchanging between mesos-fetcher process and port 3128.
> 
> ```
> sudo sysdig -A -c echo_fds proc.name=mesos-fetcher and fd.port=3128
> ```
> 
> - Stop the local squid server, try to restart the marathon task, the task 
> would fail repeatly, from slave logs there are error messages that fetcher 
> failed to fetch the uri.
> ```
> Dec 16 15:16:35 lin-E400 mesos-slave[24247]: E1216 15:16:35.678032 24283 
> slave.cpp:3342] Container '45c14132-c56a-4cff-a6b5-f57ba2670643' for executor 
> 'testapp_web.f0ef72d2-a3c4-11e5-af60-56847afe9799' of framework 
> 'b52179fd-8968-4
> bf8-baf0-dddc8a38c903-' failed to start: Failed to fetch all URIs for 
> container '45c14132-c56a-4cff-a6b5-f57ba2670643' with exit status: 2
> ```
> 
> - Restart the squid server, the task would start without a problem.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>



Re: Review Request 42040: Added Quota Operator Documentation.

2016-01-13 Thread Joerg Schad

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

(Updated Jan. 13, 2016, 1:53 p.m.)


Review request for mesos, Alexander Rukletsov, Bernd Mathiske, Joris Van 
Remoortere, and Neil Conway.


Changes
---

Changed anchors as they were incorrectly rendered by website markdown parser.


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


Repository: mesos


Description
---

Added Quota Operator Documentation.


Diffs (updated)
-

  docs/home.md 344c5617dfb003453fa17974500d44a8f766cb16 
  docs/quota.md PRE-CREATION 

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


Testing
---

Rendered version: https://gist.github.com/joerg84/a2c32e25d91e33045b56


Thanks,

Joerg Schad



Re: Review Request 42209: Updated getting started page to install Java on OSX.

2016-01-13 Thread Kevin Klues


> On Jan. 12, 2016, 10:04 p.m., Kevin Klues wrote:
> > Can you veirfy that this doesn't break things for Yosemite?
> 
> Vinod Kone wrote:
> I don't have yosemite handy to test it :) but from looking online it 
> looks like cask and caskroom have existed for a while, pre-yosemite.
> 
> Joseph Wu wrote:
> Maybe just add a "NOTE: For El Capitan...".
> The command runs just fine on Yosemite though.

I think so long as it works for Yosemite then we should add it without a 
comment since it really is a hard dependency and Yosemite doesn't come 
preinstalled with java either.


- Kevin


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


On Jan. 12, 2016, 9:38 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42209/
> ---
> 
> (Updated Jan. 12, 2016, 9:38 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Found this issue when following the getting started instructions on my new 
> mac that came with el capitan.
> 
> 
> Diffs
> -
> 
>   docs/getting-started.md 0982d9038d87cec6eaa72b5584e0877fc2b9f04c 
> 
> Diff: https://reviews.apache.org/r/42209/diff/
> 
> 
> Testing
> ---
> 
> checked the markdown locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 42209: Updated getting started page to install Java on OSX.

2016-01-13 Thread Kevin Klues

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

Ship it!


Ship It!

- Kevin Klues


On Jan. 12, 2016, 9:38 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42209/
> ---
> 
> (Updated Jan. 12, 2016, 9:38 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Found this issue when following the getting started instructions on my new 
> mac that came with el capitan.
> 
> 
> Diffs
> -
> 
>   docs/getting-started.md 0982d9038d87cec6eaa72b5584e0877fc2b9f04c 
> 
> Diff: https://reviews.apache.org/r/42209/diff/
> 
> 
> Testing
> ---
> 
> checked the markdown locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 41769: Made allocator traverse all roles for quota allocation.

2016-01-13 Thread Guangya Liu


> On 一月 13, 2016, 11:41 a.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1164-1166
> > 
> >
> > This looks like a bug to me. I can't remember why we have `break` here 
> > in the first place. Could you please elaborate why do you think `break` is 
> > fine here?

It is a bit tricky here so I add some comments here even though I prefer we use 
`continue` here.

The reason that it is OK to use `break` is because all of the quota roles are 
sorted based on starvation, if the roles in quota role sorter is satisfied, we 
can assume that all roles behind current satisfied role is also satisfied, 
that's why `break` here. Comments?


- Guangya


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


On 十二月 31, 2015, 11:34 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> ---
> 
> (Updated 十二月 31, 2015, 11:34 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Klaus Ma, and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made allocator traverse all roles for quota allocation.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 7f900c4e024485704d79e57ae22407557598fe6c 
> 
> Diff: https://reviews.apache.org/r/41769/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 41243: Updated how we find the .git directory in bootstrap.

2016-01-13 Thread Till Toenshoff

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


This recent change breaks the functionality - it hangs now.

- Till Toenshoff


On Jan. 13, 2016, 10:34 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41243/
> ---
> 
> (Updated Jan. 13, 2016, 10:34 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joseph Wu, 
> Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-4125
> https://issues.apache.org/jira/browse/MESOS-4125
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When building from git, bootstrap will (among other things) install
> pre-commit and post-rewrite hooks into the .git/hooks directory of the
> mesos tree. However the current implementation always assumes that .git
> exists in the same directory as the bootstrap file. This may not always
> be true.
> 
> Most notably, it is not true if the mesos tree is included as a
> submodule inside another project. When included as a submodule, .git is
> no longer a directory, but rather a file whose text contains a pointer
> back to the actual location of the .git folder inside the containing
> project. To get at this directory, we need to run 'git rev-parse
> --git-common-dir' instead of simply assuming that the local .git is the
> proper directory.
> 
> Review: https://reviews.apache.org/r/41243
> 
> 
> Diffs
> -
> 
>   bootstrap ea71ff22e42bf5e224483508d91c33ddfd350f52 
> 
> Diff: https://reviews.apache.org/r/41243/diff/
> 
> 
> Testing
> ---
> 
> Tested on native Mac OS X El Capitan
> Tested on vagrant boxes for bento/centos-7.1, ubuntu/trusty64, and 
> https://github.com/tommy-muehle/puppet-vagrant-boxes/releases/download/1.0.0/centos-6.6-x86_64.box
> Also tested on docker image centos:7
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 42246: Fixed support for non-HDFS URIs by Hadoop client.

2016-01-13 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [42246]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 13, 2016, 10:07 a.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42246/
> ---
> 
> (Updated Jan. 13, 2016, 10:07 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and switched to 'mcypark'.
> 
> 
> Bugs: MESOS-4304
> https://issues.apache.org/jira/browse/MESOS-4304
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed support for non-HDFS URIs by Hadoop client.
> 
> 
> Diffs
> -
> 
>   src/hdfs/hdfs.cpp 51f016b049c6e947c4d27fbbbf79c53f9ec5a51e 
> 
> Diff: https://reviews.apache.org/r/42246/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>



Re: Review Request 42113: Handle unreserve logic for dynamic reservation (2/3).

2016-01-13 Thread Guangya Liu

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

(Updated 一月 13, 2016, 1:01 p.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, Klaus Ma, and Jian Qiu.


Summary (updated)
-

Handle unreserve logic for dynamic reservation (2/3).


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


Repository: mesos


Description
---

Handle unreserve logic for dynamic reservation with allocation slack.

This patch is halding the case when using updateAllocation to unreserve
some resources.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
  src/tests/hierarchical_allocator_tests.cpp 
e044f832c2c16e53e663c6ced5452649bb0dcb59 

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


Testing
---

make
make check


Thanks,

Guangya Liu



Re: Review Request 42194: Handle unreserve logic for dynamic reservation (3/3).

2016-01-13 Thread Guangya Liu

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

(Updated 一月 13, 2016, 1:05 p.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, Klaus Ma, and Jian Qiu.


Summary (updated)
-

Handle unreserve logic for dynamic reservation (3/3).


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


Repository: mesos


Description
---

Handle unreserve logic for dynamic reservation with allocation slack.

This patch is halding the case when using updateAvailable to unreserve
some resources.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
  src/tests/hierarchical_allocator_tests.cpp 
e044f832c2c16e53e663c6ced5452649bb0dcb59 

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


Testing
---

make
make check


Thanks,

Guangya Liu



Re: Review Request 42212: Added unit test for framework/task history flags.

2016-01-13 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [42053, 42212]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 13, 2016, 11:01 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42212/
> ---
> 
> (Updated Jan. 13, 2016, 11:01 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3307
> https://issues.apache.org/jira/browse/MESOS-3307
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adds a unit test to verify that the the max_frameworks and
> max_tasks_per_frameworks flags for master work properly. Specifically,
> we test to verify that the proper amount of history is maintained for
> both 0 values to these flags as well as positive values <= to the total
> number frameworks and tasks per framework actually launched.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 223b9d20a3a8a8194a3a6a605ec2394c37ab5957 
> 
> Diff: https://reviews.apache.org/r/42212/diff/
> 
> 
> Testing
> ---
> 
> This is a unit test.  I ran it on my mac and on ubuntu 14.04.
> 
> GTEST_FILTER="MasterTest.FrameworksTasksCompletedFlags" make check -j 7
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Review Request 42250: CMake: Add sasl and dl link flags, add curl link library and add protobuf library directory.

2016-01-13 Thread Diana Arroyo

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

Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

CMake: Add sasl and dl link flags, add curl link library and add protobuf 
library directory.


Diffs
-

  src/slave/cmake/FindCurl.cmake PRE-CREATION 
  src/slave/cmake/SlaveConfigure.cmake fbdfdaa27fbd8c7429861eea5baf401a221f748b 

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


Testing
---

originally https://reviews.apache.org/r/41108/.  When attempting to address 
final review this review was created instead of modifiying original.  See 
https://reviews.apache.org/r/41108/ for testing comments.


Thanks,

Diana Arroyo



Re: Review Request 41108: CMake: Add sasl and dl link flags, add curl link library and add protobuf library directory.

2016-01-13 Thread Diana Arroyo


> On Jan. 8, 2016, 4:43 p.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [41096, 41185, 41108]
> > 
> > Failed command: ./support/apply-review.sh -n -r 41108
> > 
> > Error:
> >  2016-01-08 16:43:43 URL:https://reviews.apache.org/r/41108/diff/raw/ 
> > [4052/4052] -> "41108.patch" [1]
> > error: patch failed: src/slave/cmake/FindCurl.cmake:31
> > error: src/slave/cmake/FindCurl.cmake: patch does not apply
> 
> Alex Clemmer wrote:
> It looks like you need to update your master and rebase this commit to 
> come after the last commit on the master branch? Ping me if you need help 
> with this.

Please see: https://reviews.apache.org/r/42250/


- Diana


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


On Jan. 8, 2016, 3:29 p.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41108/
> ---
> 
> (Updated Jan. 8, 2016, 3:29 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/cmake/FindCurl.cmake PRE-CREATION 
>   src/slave/cmake/SlaveConfigure.cmake 
> fbdfdaa27fbd8c7429861eea5baf401a221f748b 
> 
> Diff: https://reviews.apache.org/r/41108/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu and OSX.
> Tested if and else path of new logic added to FindCurl.cmake.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>



Re: Review Request 40632: Enabled oversubscribed resources for reservations in allocator.

2016-01-13 Thread Guangya Liu

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

(Updated 一月 13, 2016, 12:39 p.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, Klaus Ma, and Jian Qiu.


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


Repository: mesos


Description (updated)
---

Enabled oversubscribed resources for reservations in allocator.

There are five patches handling the allocator part:
1) This patch handles send offer, add slave and quota integration test.
2) https://reviews.apache.org/r/41847/ Handles the case of update slave.
3) https://reviews.apache.org/r/41791/ Handles the case of dynamic reservations 
(1/3).
4) https://reviews.apache.org/r/42113/ Handle unreserve logic for dynamic 
reservation (2/3)
5) https://reviews.apache.org/r/42194/ Handle unreserve logic for dynamic 
reservation (3/3)


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
  src/tests/hierarchical_allocator_tests.cpp 
e044f832c2c16e53e663c6ced5452649bb0dcb59 

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


Testing
---

make
make check
GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="HierarchicalAllocatorTest.*" 
--verbose


Thanks,

Guangya Liu



Re: Review Request 41308: MESOS-1718: Unit Test for moving getExecutorInfo from slave to master

2016-01-13 Thread Klaus Ma

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


@Joseph/Vinod, would you help to review those patches move executor from slave 
to master?

- Klaus Ma


On Dec. 12, 2015, 5:58 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41308/
> ---
> 
> (Updated Dec. 12, 2015, 5:58 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
> Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-1718
> https://issues.apache.org/jira/browse/MESOS-1718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-1718: Unit Test for moving getExecutorInfo from slave to master
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 3f199e6 
>   src/tests/master_tests.cpp 865fa4a 
>   src/tests/master_validation_tests.cpp fbf8fad 
>   src/tests/monitor_tests.cpp a848d14 
>   src/tests/reservation_endpoints_tests.cpp d5d2aa7 
>   src/tests/slave_recovery_tests.cpp c0e4ff7 
>   src/tests/slave_tests.cpp 4975bea 
> 
> Diff: https://reviews.apache.org/r/41308/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 41847: Updated allocation slack when slave was updated.

2016-01-13 Thread Guangya Liu

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

(Updated 一月 13, 2016, 12:55 p.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, Klaus Ma, and Jian Qiu.


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


Repository: mesos


Description
---

Updated allocation slack when slave was updated.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
  src/tests/hierarchical_allocator_tests.cpp 
e044f832c2c16e53e663c6ced5452649bb0dcb59 

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


Testing
---

make
make check
GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="HierarchicalAllocatorTest.*" 
--verbose


Thanks,

Guangya Liu



Review Request 42196: Added cgroup instructions for CentOS 6.6.

2016-01-13 Thread Jan Schlicht

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

Review request for mesos and Till Toenshoff.


Bugs: MESOS-4035 and MESOS-4038
https://issues.apache.org/jira/browse/MESOS-4035
https://issues.apache.org/jira/browse/MESOS-4038


Repository: mesos


Description
---

By default cgroup management is not enabled on CentOS 6.6. As process isolation
can use cgroups, documentation on how to activate cgroup management using
cgconfig has been added. Furthermore the Mesos tests need the 'perf_event'
cgroup subsystem to be present. Instructions on how to do this have been added
as well.


Diffs
-

  docs/getting-started.md 0982d9038d87cec6eaa72b5584e0877fc2b9f04c 

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


Testing
---


Thanks,

Jan Schlicht



Re: Review Request 37531: Fix master CHECK failure if a framework uses duplicated task id.

2016-01-13 Thread Klaus Ma

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

(Updated Jan. 13, 2016, 10:06 p.m.)


Review request for mesos, Jie Yu and Vinod Kone.


Changes
---

rebase and ping Vinod :).


Summary (updated)
-

Fix master CHECK failure if a framework uses duplicated task id.


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


Repository: mesos


Description
---

__Phenomenon:__
The master crash because of duplicated task id

__Root Cause:__
The task id are stored in slave agent; if master failover, there's a time 
window that new slave lanched a task with same task id; so if the old task 
re-registered back, the master will crash because of duplicated task id.

__Solution:__
Stores tasks info in Master::Framework by SlaveID to avoid duplicated issue.


Diffs (updated)
-

  src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d 
  src/master/master.hpp f02d165874fa8023675e545793de699aeecae29b 
  src/master/master.cpp c122c30d943813fc3ce9e7025783c7231809b022 
  src/tests/master_tests.cpp 223b9d20a3a8a8194a3a6a605ec2394c37ab5957 

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


Testing
---

make
make check


Thanks,

Klaus Ma



Review Request 42197: Added a filter for tests using 'perf' hardware events.

2016-01-13 Thread Jan Schlicht

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

Review request for mesos and Till Toenshoff.


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


Repository: mesos


Description
---

Virtual machines may not always support 'CPU performance counters'. Tests
trying to collect to 'cycle' value of 'perf' will fail under these
circumstances. This will be avoided by disabling these tests if 'perf' hardware
events are not available.


Diffs
-

  src/tests/environment.cpp 20218a086baefcefb310eb45ed9024e5425ce787 

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


Testing
---

`sudo ./bin/mesos-tests.sh 
--gtest_filter="-*ROOT_CGROUPS_Perf:*ROOT_CGROUPS_Sample:*ROOT_CGROUPS_UserCgroup:*CGROUPS_ROOT_PerfRollForward:*ROOT_Sample"`
 in a VM without 'CPU performance counters'. This doesn't run any tests, as 
expected.


Thanks,

Jan Schlicht



Re: Review Request 41769: Made allocator traverse all roles for quota allocation.

2016-01-13 Thread Alexander Rukletsov

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



src/master/allocator/mesos/hierarchical.cpp (lines 1164 - 1166)


This looks like a bug to me. I can't remember why we have `break` here in 
the first place. Could you please elaborate why do you think `break` is fine 
here?


- Alexander Rukletsov


On Dec. 31, 2015, 11:34 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> ---
> 
> (Updated Dec. 31, 2015, 11:34 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Klaus Ma, and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made allocator traverse all roles for quota allocation.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 7f900c4e024485704d79e57ae22407557598fe6c 
> 
> Diff: https://reviews.apache.org/r/41769/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 41772: Added helper function to flatten resources.

2016-01-13 Thread Guangya Liu

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

(Updated 一月 13, 2016, 12:16 p.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, and Klaus Ma.


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


Repository: mesos


Description
---

Added two new helper functions to flatten resources.
1) Flatten reserved resources.
2) Flatten allocation slack revocable resources.


Diffs (updated)
-

  include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
  include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
  src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
  src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
  src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 

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


Testing (updated)
---

make
make check

GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="ResourcesOperationTest.*" 
--verbose


Thanks,

Guangya Liu



Re: Review Request 41882: Constrained types used in Flags instantiation.

2016-01-13 Thread Till Toenshoff

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

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp (lines 242 - 
257)


Much slicker now, thanks for the suggestion Alexander!

The comment helps understanding the ghist, gj!


- Till Toenshoff


On Jan. 13, 2016, 10:43 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41882/
> ---
> 
> (Updated Jan. 13, 2016, 10:43 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Bugs: MESOS-4278
> https://issues.apache.org/jira/browse/MESOS-4278
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To make the implementation less repetitive reimplemented `FlagsBase`
> with a variadic template; this allows to now use an arbitrary number of
> `FlagsBase` bases.
> 
> Since `Flags<>` does already inherit virtually from `FlagsBase` it
> should be a safer base class than `FlagsBase` for users.
> 
> Review: https://reviews.apache.org/r/41882/
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
> addef78ddeb0007cf1e1c79738381138a18a35b6 
> 
> Diff: https://reviews.apache.org/r/41882/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 42248: Fixed markdown parsing in markdown-styleguide.

2016-01-13 Thread Joerg Schad

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

(Updated Jan. 13, 2016, 12:47 p.m.)


Review request for mesos, Joerg Schad and Till Toenshoff.


Changes
---

Removed not needed empty line in example.


Repository: mesos


Description
---

As the website markdown parser supports fewer syntax marker (compared to 
github), the rendered markdown-styleguide was broken.


Diffs (updated)
-

  docs/markdown-style-guide.md fe9ffefbbb2c0ce9ade39cec1de073163a9a03b2 

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


Testing
---

Viewed via docker webseite generator.


Thanks,

Joerg Schad



Re: Review Request 42248: Fixed markdown parsing in markdown-styleguide.

2016-01-13 Thread Joerg Schad

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

(Updated Jan. 13, 2016, 1:01 p.m.)


Review request for mesos, Joerg Schad and Till Toenshoff.


Repository: mesos


Description
---

As the website markdown parser supports fewer syntax marker (compared to 
github), the rendered markdown-styleguide was broken.


Diffs
-

  docs/markdown-style-guide.md fe9ffefbbb2c0ce9ade39cec1de073163a9a03b2 

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


Testing
---

Viewed via docker webseite generator.


Thanks,

Joerg Schad



Re: Review Request 42130: Added tests for `ALLOCATION_SLACK` helper functions.

2016-01-13 Thread Klaus Ma

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

(Updated Jan. 13, 2016, 9:31 p.m.)


Review request for mesos, Guangya Liu and Jian Qiu.


Summary (updated)
-

Added tests for `ALLOCATION_SLACK` helper functions.


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


Repository: mesos


Description
---

Added tests for `ALLOCATION_SLACK` helper functions.


Diffs
-

  src/tests/cluster.hpp 576dcb8b7e27d1905425aa0f7cb319c19c17c15c 
  src/tests/oversubscription_tests.cpp 7a75fb38e0177e33cf0e7cb82b4b9ebf8f05fe0a 

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


Testing
---

make
make check GTEST_FILTER="OversubscriptionTest.*"


Thanks,

Klaus Ma



Re: Review Request 41438: Added documentation on using network proxy for mesos fetcher

2016-01-13 Thread Joerg Schad

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



docs/fetcher.md (line 258)


s/check/check the


- Joerg Schad


On Dec. 24, 2015, 1:22 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41438/
> ---
> 
> (Updated Dec. 24, 2015, 1:22 p.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Bugs: MESOS-4130
> https://issues.apache.org/jira/browse/MESOS-4130
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation on using network proxy for mesos fetcher
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md ec7372293491ed9ee4e20dda3c2def72a77a3f84 
> 
> Diff: https://reviews.apache.org/r/41438/diff/
> 
> 
> Testing
> ---
> 
> - Set `https_proxy=http://localhost:3128` (a local squid server) in 
> `/etc/default/mesos-slave`, restart mesos-slave, launch a new marathon task 
> with a `https://` uri, and through the following sysdig command I can see 
> there is data exchanging between mesos-fetcher process and port 3128.
> 
> ```
> sudo sysdig -A -c echo_fds proc.name=mesos-fetcher and fd.port=3128
> ```
> 
> - Stop the local squid server, try to restart the marathon task, the task 
> would fail repeatly, from slave logs there are error messages that fetcher 
> failed to fetch the uri.
> ```
> Dec 16 15:16:35 lin-E400 mesos-slave[24247]: E1216 15:16:35.678032 24283 
> slave.cpp:3342] Container '45c14132-c56a-4cff-a6b5-f57ba2670643' for executor 
> 'testapp_web.f0ef72d2-a3c4-11e5-af60-56847afe9799' of framework 
> 'b52179fd-8968-4
> bf8-baf0-dddc8a38c903-' failed to start: Failed to fetch all URIs for 
> container '45c14132-c56a-4cff-a6b5-f57ba2670643' with exit status: 2
> ```
> 
> - Restart the squid server, the task would start without a problem.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>



Re: Review Request 42197: Added a filter for tests using 'perf' hardware events.

2016-01-13 Thread Jan Schlicht

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

(Updated Jan. 13, 2016, 2:32 p.m.)


Review request for mesos and Till Toenshoff.


Changes
---

Fixed mistake in gtest filter.


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


Repository: mesos


Description
---

Virtual machines may not always support 'CPU performance counters'. Tests
trying to collect to 'cycle' value of 'perf' will fail under these
circumstances. This will be avoided by disabling these tests if 'perf' hardware
events are not available.


Diffs
-

  src/tests/environment.cpp 20218a086baefcefb310eb45ed9024e5425ce787 

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


Testing (updated)
---

`sudo ./bin/mesos-tests.sh 
--gtest_filter="*ROOT_CGROUPS_Perf:*ROOT_CGROUPS_Sample:*ROOT_CGROUPS_UserCgroup:*CGROUPS_ROOT_PerfRollForward:*ROOT_Sample"`
 in a VM without 'CPU performance counters'. This doesn't run any tests, as 
expected.


Thanks,

Jan Schlicht



Re: Review Request 41108: CMake: Add sasl and dl link flags, add curl link library and add protobuf library directory.

2016-01-13 Thread Diana Arroyo

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

(Updated Jan. 13, 2016, 6:09 p.m.)


Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/slave/cmake/FindCurl.cmake PRE-CREATION 
  src/slave/cmake/SlaveConfigure.cmake fbdfdaa27fbd8c7429861eea5baf401a221f748b 

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


Testing
---

Tested on Ubuntu and OSX.
Tested if and else path of new logic added to FindCurl.cmake.


Thanks,

Diana Arroyo



Re: Review Request 42143: Removed HTTPTest.Auth test.

2016-01-13 Thread Alexander Rojas

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

(Updated Jan. 13, 2016, 6:13 p.m.)


Review request for mesos, Benjamin Bannier, Ben Mahler, Jan Schlicht, and Till 
Toenshoff.


Changes
---

Fixed description.


Repository: mesos


Description (updated)
---

Manual HTTP authentication is being phased out in favor of the HTTP 
authentication support built-in into libprocess, hence this test is no longer 
necessary.


Diffs
-

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

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


Testing
---

make check


Thanks,

Alexander Rojas



Re: Review Request 41819: Added ContainerConfig to all isolators.

2016-01-13 Thread Jie Yu

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

Ship it!



src/slave/containerizer/mesos/isolators/filesystem/linux.cpp (line 281)


You can just do (no need to initailize it to None as that's the default 
behavior).

```
Option user;
```


- Jie Yu


On Jan. 6, 2016, 11:41 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41819/
> ---
> 
> (Updated Jan. 6, 2016, 11:41 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4240
> https://issues.apache.org/jira/browse/MESOS-4240
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added ContainerConfig to all isolators.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f3c370aeb331beb6202fd30cd0278877da0b42e0 
>   src/slave/containerizer/mesos/isolator.cpp 
> 493b5dd26cf0e8f986381a502cfa6d1dde6573d4 
>   src/slave/containerizer/mesos/isolators/cgroups/cpushare.hpp 
> 123b9ed3ccaebcd5da24fc62ff7a92d4a81ed760 
>   src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp 
> 3b95e195ad704f163c245175390d9a26bde7e17c 
>   src/slave/containerizer/mesos/isolators/cgroups/mem.hpp 
> 09952369c72d3c6322ae7a1c73cd68226d452ad2 
>   src/slave/containerizer/mesos/isolators/cgroups/mem.cpp 
> 2ddb9f4adbb879682cd39966ab974cf3fa32209c 
>   src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp 
> 5eaf49f1f35c93ad4465adb6c9c9cf57b3a2c6ee 
>   src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
> 4d82c2b2f231c59cbb600869a0f2b716c1e55f5e 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
> a29f9d184a0d1088577b3168a12bc8c06493a477 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 8af2cecdca947e55d8d39e26d2fc3d42212f36c3 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
> c3544aa313cbb185efb03bba59961cdf2b616a37 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
> 00ff84b6cd0aa29fa5a7918d7f88d480af8752ca 
>   src/slave/containerizer/mesos/isolators/filesystem/shared.hpp 
> 2e457015a0348a457581edf493877b71fab17090 
>   src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
> 361ed6561bd5e2f75d026922def01f42b43d61c2 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
> c2d1455249618f9cd2e17dc2244b184d52b32eaf 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
> d65c1593b44f4b21237581147e57e441ebf3160d 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> e3766c313a1bd2a838a73730c62c74c5ee8e1a4c 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 22a8428427b758bae4a0518356d7933c4110cd9f 
>   src/slave/containerizer/mesos/isolators/posix.hpp 
> 7e1ebc2fada5a5e291e84c7044bdba9a71f4b42c 
>   src/slave/containerizer/mesos/isolators/posix/disk.hpp 
> 31808c1e8199fbf2cea36c273860fdbf0a2388f8 
>   src/slave/containerizer/mesos/isolators/posix/disk.cpp 
> 248c34adb63907911d89bed5b1519682a852bb2d 
> 
> Diff: https://reviews.apache.org/r/41819/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>



Re: Review Request 37531: Fix master CHECK failure if a framework uses duplicated task id.

2016-01-13 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [37531]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 13, 2016, 2:06 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37531/
> ---
> 
> (Updated Jan. 13, 2016, 2:06 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-3070
> https://issues.apache.org/jira/browse/MESOS-3070
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> __Phenomenon:__
> The master crash because of duplicated task id
> 
> __Root Cause:__
> The task id are stored in slave agent; if master failover, there's a time 
> window that new slave lanched a task with same task id; so if the old task 
> re-registered back, the master will crash because of duplicated task id.
> 
> __Solution:__
> Stores tasks info in Master::Framework by SlaveID to avoid duplicated issue.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d 
>   src/master/master.hpp f02d165874fa8023675e545793de699aeecae29b 
>   src/master/master.cpp c122c30d943813fc3ce9e7025783c7231809b022 
>   src/tests/master_tests.cpp 223b9d20a3a8a8194a3a6a605ec2394c37ab5957 
> 
> Diff: https://reviews.apache.org/r/37531/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 42035: Windows: Removed the `--switch_user` flag in Windows.

2016-01-13 Thread Daniel Pravat

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



src/slave/flags.cpp (line 196)


There is a reason why this change is so extensive. This block should be 
replaced with a reasonable default for switch_user in Windows and it should be 
enough.


- Daniel Pravat


On Jan. 7, 2016, 9:47 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42035/
> ---
> 
> (Updated Jan. 7, 2016, 9:47 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Jie 
> Yu, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4310
> https://issues.apache.org/jira/browse/MESOS-4310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Mesos agent provides a flag, `--[no-]switch_user`, which instructs
> the agent to run tasks as the user who submitted the task instead of the
> user that is running the agent process itself.
> 
> On POSIX, this requires the usual suspects (`su`, `chown`, and `chroot`,
> for example to do things like make sure the correct user owns the
> executor directory, and things like that.
> 
> On Windows, many of these operations have very different semantics. To
> use one example, to `chown` on Windows, you normally have to give
> *permission* for something to `chown` an object, and then that thing
> actually has to use that permission to `chown` it, on its own.
> 
> Clearly a semantic mismatch of this magnitude will require some work to
> formally bridge, and the result is that we will simply disable the
> functionality entirely on the Windows agent until we are prepared to go
> back and support it formally.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md fb6f6784e5d11850ba0bafaeafa3213a1038e6b4 
>   src/slave/flags.hpp 6857fde027fd57b4934cb43ddf435d12900e0b87 
>   src/slave/flags.cpp 19c2996c4572b992030f8824380f3979ced7e526 
>   src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 
> 
> Diff: https://reviews.apache.org/r/42035/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 41108: CMake: Add sasl and dl link flags, add curl link library and add protobuf library directory.

2016-01-13 Thread Diana Arroyo

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

(Updated Jan. 13, 2016, 6:31 p.m.)


Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/slave/cmake/FindCurl.cmake df81fd538b06bf0c3e842a502bd5ef4083d897b5 
  src/slave/cmake/SlaveConfigure.cmake cf378a27297474b2a9f338e0c832612370f7302a 

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


Testing
---

Tested on Ubuntu and OSX.
Tested if and else path of new logic added to FindCurl.cmake.


Thanks,

Diana Arroyo



Re: Review Request 42235: Added protobuf for docker ImageReference and the parsing function.

2016-01-13 Thread Gilbert Song

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

Ship it!


Verified that the logic in `parseImageReference` is generally the same as 
`parseImageName`.


src/docker/spec.cpp (line 62)


We remove setting default tag as `latest`, is it because not necessary to 
set (if no tag is specified, it will pull the latest-tagged image)?



src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp (lines 23 
- 26)


move them under `#include `



src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp (line 32)


A little cleanup here:
move this header above `#include `



src/tests/containerizer/docker_spec_tests.cpp (line 22)


ditto.



src/tests/containerizer/provisioner_docker_tests.cpp (line 42)


ditto.


- Gilbert Song


On Jan. 12, 2016, 10:21 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42235/
> ---
> 
> (Updated Jan. 12, 2016, 10:21 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Gilbert Song, Jojy Varghese, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-4296
> https://issues.apache.org/jira/browse/MESOS-4296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The goal here is to replace Image::Name with ImageReference and the
> corresponding parsing method, which will be done in a subsequent patch.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.proto PRE-CREATION 
>   src/CMakeLists.txt 39a23df3227a4f524ea0d408dc894fa5bbab7d10 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/docker/spec.hpp 822b2383c6d6bd691cb1fdbfcd699ae3ae3585bd 
>   src/docker/spec.cpp 0188078037b3528acefa42d39f05bd60789b19b2 
>   src/slave/containerizer/mesos/provisioner/docker/message.hpp 
> 162e4c689bba832e523ff6b7d4e1e3c8e6713803 
>   src/slave/containerizer/mesos/provisioner/docker/message.proto 
> 2b2ed056bd9acd341de0e5cbdb4eed711f3a0fe5 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> bd6dace826b3694748e4209dac5aa73256356f41 
>   src/tests/containerizer/docker_spec_tests.cpp 
> aa4faf92e82acd739efc1da15521261da0092afa 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> f81f0039dc12d09d85fda0be345d1509b4c6a664 
> 
> Diff: https://reviews.apache.org/r/42235/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 42211: Added filters to disable NetClsIsolator tests in case cgroups is not present or the net_cls subsystem is disable.

2016-01-13 Thread Mesos ReviewBot

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


Bad patch!

Reviews applied: [42047]

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

Error:
 2016-01-13 20:49:34 URL:https://reviews.apache.org/r/42047/diff/raw/ 
[9619/9619] -> "42047.patch" [1]
Total errors found: 0
Checking 2 files
Error: Commit message summary (the first line) must not exceed 72 characters.

- Mesos ReviewBot


On Jan. 13, 2016, 5:52 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42211/
> ---
> 
> (Updated Jan. 13, 2016, 5:52 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4262
> https://issues.apache.org/jira/browse/MESOS-4262
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added filters to disable NetClsIsolator tests in case cgroups is not present 
> or the net_cls subsystem is disable.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp 36f0ad0b739559e5c883d72585731944da4283cf 
> 
> Diff: https://reviews.apache.org/r/42211/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> * Tested the filter by disabling cls_group.ko (module for net_cls cgroup). 
> The NetClsIsolator tests were correctly disabled when I ran make check with 
> the filter set to NetClsIsolatorTest.ROOT_CGROUPS_NetClsIsolate
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 42215: Added expectations for call to mocked executor shutdown.

2016-01-13 Thread Timothy Chen

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

Ship it!


Ship It!

- Timothy Chen


On Jan. 13, 2016, 12:05 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42215/
> ---
> 
> (Updated Jan. 13, 2016, 12:05 a.m.)
> 
> 
> Review request for mesos, Neil Conway and Timothy Chen.
> 
> 
> Bugs: MESOS-4347
> https://issues.apache.org/jira/browse/MESOS-4347
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added expectations for call to mocked executor shutdown. The mock executor's 
> shutdown method sometimes gets executed before the test exits, so it should 
> be explicitly expected.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 61d275c47fbb02baf22e4ad8f9b1580886da51d9 
> 
> Diff: https://reviews.apache.org/r/42215/diff/
> 
> 
> Testing
> ---
> 
> I used `GTEST_FILTER="ReservationTest.*" bin/mesos-tests.sh 
> --gtest_repeat=1 --gtest_break_on_failure=1 | grep -B 3 -A 6 WARNING` to 
> reproduce the warnings, and then to verify that they were gone after the fix. 
> Note that when running this command after the patch, other warning may be 
> seen due to https://issues.apache.org/jira/browse/MESOS-4350
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Review Request 42264: Fixed a GMock warning in RoleTest.ImplicitRoleStaticReservation.

2016-01-13 Thread Neil Conway

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

Review request for mesos, Joris Van Remoortere and Timothy Chen.


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


Repository: mesos


Description
---

Fixed a GMock warning in RoleTest.ImplicitRoleStaticReservation.


Diffs
-

  src/tests/role_tests.cpp 373ae267b85588fe491ab0a0ce8aa195f971aac3 

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


Testing
---

Ran the test a few hundred times with and without the change; without the 
change, a warning is observed. With the change, no warning is observed.


Thanks,

Neil Conway



Re: Review Request 40936: Windows: Unified POSIX and Windows implementation of shell.hpp

2016-01-13 Thread Daniel Pravat

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

(Updated Jan. 13, 2016, 7:14 p.m.)


Review request for mesos, Alex Naparu, Alex Clemmer, and M Lawindi.


Repository: mesos


Description
---

Windows: Unified POSIX and Windows implementation of shell.hpp


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp 
b18cae1118302e18d2cfb7ce4089ab5079a01d1a 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp 
bf737c87b8a337cc46e6c16d6fec2eef61e6ea05 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99 

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


Testing
---


Thanks,

Daniel Pravat



Re: Review Request 41870: [stout] Added ref-qualifiers to Option::get().

2016-01-13 Thread Michael Park

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

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp (line 108)


Space between `const` and `&`.


- Michael Park


On Jan. 7, 2016, 4:47 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41870/
> ---
> 
> (Updated Jan. 7, 2016, 4:47 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This trivial change adds ref-qualifiers introduced in C++11 to Option::get(). 
> This is in sync with what boost/folly `optional` already have.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 
> b58350b312c1969aa86ec679e95af0f97846c141 
> 
> Diff: https://reviews.apache.org/r/41870/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Verified this enables operations like the following to incur a move instead 
> of a copy:
> 
> ```
> Option create();
> T t = create().get();
> ```
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 42186: Added tests for recovery for HTTP based executors.

2016-01-13 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [41275, 41277, 41280, 41281, 41282, 41283, 41285, 41288, 
41290, 41291, 42181, 42182, 42183, 42184, 42185, 42186]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 13, 2016, 5:27 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42186/
> ---
> 
> (Updated Jan. 13, 2016, 5:27 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4255
> https://issues.apache.org/jira/browse/MESOS-4255
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp c0e4ff75b35c9e806741aab5696771e66d2c2ea8 
> 
> Diff: https://reviews.apache.org/r/42186/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 42262: Improved links at containerizer.md.

2016-01-13 Thread Joerg Schad

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

(Updated Jan. 13, 2016, 8:38 p.m.)


Review request for mesos, Joerg Schad and Jojy Varghese.


Repository: mesos


Description (updated)
---

Improved links in containerizer.md.


Diffs
-

  docs/containerizer.md bf0b9279703f341c136d1d5b3999a9fef0f25911 

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


Testing (updated)
---

Viewed via docker website generator.


Thanks,

Joerg Schad



Re: Review Request 42262: Improved links at containerizer.md.

2016-01-13 Thread Neil Conway

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

Ship it!


Ship It!

- Neil Conway


On Jan. 13, 2016, 8:38 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42262/
> ---
> 
> (Updated Jan. 13, 2016, 8:38 p.m.)
> 
> 
> Review request for mesos, Joerg Schad and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved links in containerizer.md.
> 
> 
> Diffs
> -
> 
>   docs/containerizer.md bf0b9279703f341c136d1d5b3999a9fef0f25911 
> 
> Diff: https://reviews.apache.org/r/42262/diff/
> 
> 
> Testing
> ---
> 
> Viewed via docker website generator.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 41806: Cleaned up assertions in test cases around JSON HTTP responses.

2016-01-13 Thread Neil Conway


> On Jan. 12, 2016, 10:56 p.m., Adam B wrote:
> > src/tests/executor_http_api_tests.cpp, line 394
> > 
> >
> > Do you think there's any test overhead in doing another AWAIT for a 
> > `response` that has already been awaited? I'd think it would be a near 
> > instantaneous noop, but if not, the EXPECT_SOME_EQ could be faster.
> > Maybe you could time a few test runs before/after this patch to make 
> > sure we're not noticeably slowing things down.

Seems like it doesn't make a noticeable performance difference. I ran `time 
./src/mesos-tests --gtest_filter="MasterTest.SlavesEndpointWithoutSlaves" 
--gtest_repeat=100`. With the patch applied, I got the following elapsed times:

```
5.269
4.663
4.803
4.672
```

(The first time probably suffered from cold cache/warmup effects.) Without the 
patch applied, I get:

```
4.867
4.989
4.728
4.756
```


- Neil


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


On Jan. 12, 2016, 8:47 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41806/
> ---
> 
> (Updated Jan. 12, 2016, 8:47 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used `AWAIT_EXPECT_RESPONSE_HEADER_EQ()` to check the "Content-Type" of
> the response, rather than accessing the "headers" field directly, and
> used the symbol `APPLICATION_JSON` rather than a string literal. Also
> added "Content-Type" checks to a few places that had neglected to make
> them, and cleaned up some whitespace style.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp 
> 952aacb720715ad26f91fa08bc386b242b7e007b 
>   src/tests/fault_tolerance_tests.cpp 
> fd1c3c90101eed9ef9352511e0c72a463cca965c 
>   src/tests/master_tests.cpp 223b9d20a3a8a8194a3a6a605ec2394c37ab5957 
>   src/tests/metrics_tests.cpp f081fb9b68f25c6d6005f195c34253fba8abc146 
>   src/tests/monitor_tests.cpp a848d14ebb6cab79c06bcf55bd39f044b41a006e 
>   src/tests/repair_tests.cpp 63ec889c4954c2c60d3466952551aa25b3284ddf 
>   src/tests/scheduler_driver_tests.cpp 
> 1365d21ccad87923b862fb4942f1fd51630a62b7 
>   src/tests/scheduler_http_api_tests.cpp 
> 4d23a5a8368e0ed126469fa4a90a889b339ad004 
>   src/tests/slave_tests.cpp e4fb490a1d877547fe883c22dbc47bb4969ecef6 
>   src/tests/status_update_manager_tests.cpp 
> bd34b97a3559a5fea9a7a253a89e0ac3029f4a33 
>   src/tests/utils.cpp 877139e97249761658dce3b1058cdc2e2a52367b 
> 
> Diff: https://reviews.apache.org/r/41806/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 40936: Windows: Unified POSIX and Windows implementation of shell.hpp

2016-01-13 Thread Daniel Pravat


> On Jan. 6, 2016, 2:23 a.m., Alex Clemmer wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp, line 
> > 73
> > 
> >
> > How semantically similar is this to the POSIX code? If it's very 
> > similar, does it make sense to transition the POSIX code to use the POSIX 
> > equivalent too?

Posix equivalne uses ::fork(). We cannot use that call. In theroy should have 
the same bahaviour.


> On Jan. 6, 2016, 2:23 a.m., Alex Clemmer wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp, line 
> > 68
> > 
> >
> > The comments on the POSIX version of this function claims that it's 
> > async signal safe. Is that true of this version as well? Does that even 
> > make sense in Windows?

We don't support POSIX signals in Windows. The comment is not need it.


> On Jan. 6, 2016, 2:23 a.m., Alex Clemmer wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp, 
> > lines 25-28
> > 
> >
> > Historically this stuff has gone in `stout/windows.hpp`, so that if 
> > another function needs them, because (1) they don't all have to redefine 
> > the symbols, and (2) we wanted to keep all the Windows-compat stuff in one 
> > place until we had a clearer picture of what we wanted to do with this 
> > stuff in the long term.
> > 
> > In the long term, I think the answer is that we've already started 
> > moving toward a model where each compat header gets its own file (_e.g._, 
> > `dirent.h` gets `stout/internal/windows/dirent.hpp`), so I think we'll 
> > eventually come back and refactor all of `windows.hpp` to fit this model.
> > 
> > For the time being, though, I think it makes sense to just move this 
> > there.

I see similar file with platform defines in libprocess/src/config.hpp. Maybe we 
should use a single file with all the types/define missing in windows.  I will 
keep this in mind for further refactoring.


> On Jan. 6, 2016, 2:23 a.m., Alex Clemmer wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp, line 
> > 120
> > 
> >
> > Sorry, template noob here. Why the variadic template here? Why not:
> > 
> > ```
> > int execlp(const char* path, const char* arg, ...)
> > {
> >   return ::execlp(path, arg);
> > }
> > ```
> > 
> > My understanding here is that `T` could be any number of type 
> > arguments, and the result of `const T*... t` is that you could pass in a 
> > bunch of different types into the `os::execlp` function.
> > 
> > Is that all correct?
> > 
> > If so, then I don't really understand what this freedom buys us, since 
> > it seems like in practice we'll only call it with `const char*`.

Removed the code. This is not requered


> On Jan. 6, 2016, 2:23 a.m., Alex Clemmer wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp, line 
> > 110
> > 
> >
> > `arg0` and `arg1` seem like they might not be the best names for these. 
> > Perhaps something more descriptive?

The code has been removed. arg0 and arg1 follows the convention on the API 
param naming.


> On Jan. 6, 2016, 2:23 a.m., Alex Clemmer wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp, lines 
> > 104-118
> > 
> >
> > Looks like `shell_const` is used later, for commit of launch.cpp, right?
> > 
> > I'm fine with that, but I have a couple suggestions:
> > 
> > * Can we please indent the static members according to the style we use 
> > for structs?
> > * Brief search of the codebase reveals that calls to `exec*("sh", "sh", 
> > ...)` are not that numerous. By my count, there are a few in fork.hpp, 
> > launch.cpp, executor.cpp, and here in shell.hpp, on line 144. It seems like 
> > it's worth just transitioning them all right now, since they're not that 
> > numerous, and since it seems bad to have the code in an inconsistent state. 
> > What do you think?

We should mmake the transition when we add the files.


- Daniel


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


On Jan. 13, 2016, 7:14 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40936/
> ---
> 
> 

Review Request 42265: Fixed more tests that didn't set a shutdown expect for MockExecutor.

2016-01-13 Thread Neil Conway

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

Review request for mesos, Joris Van Remoortere and Timothy Chen.


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


Repository: mesos


Description
---

Specifically, the following tests:

MasterTest.OfferNotRescindedOnceUsed
OversubscriptionTest.FetchResourceUsageFromMonitor
OversubscriptionTest.QoSFetchResourceUsageFromMonitor
SlaveTest.ContainerUpdatedBeforeTaskReachesExecutor


Diffs
-

  src/tests/master_tests.cpp 223b9d20a3a8a8194a3a6a605ec2394c37ab5957 
  src/tests/oversubscription_tests.cpp 7a75fb38e0177e33cf0e7cb82b4b9ebf8f05fe0a 
  src/tests/slave_tests.cpp eb47b5319b16601b8600c4d53035b8337382070c 

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


Testing
---

Ran each test a few hundred times with and without the changes; without the 
changes, a warning is observed. With the changes, no warning is observed.


Thanks,

Neil Conway



Re: Review Request 42196: Added cgroup instructions for CentOS 6.6.

2016-01-13 Thread Joerg Schad

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



docs/getting-started.md (line 117)


Is that true in general? Process isolation might rely on cgroups, but not 
necessarly or?


- Joerg Schad


On Jan. 13, 2016, 3:18 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42196/
> ---
> 
> (Updated Jan. 13, 2016, 3:18 p.m.)
> 
> 
> Review request for mesos, Joerg Schad and Till Toenshoff.
> 
> 
> Bugs: MESOS-4035 and MESOS-4038
> https://issues.apache.org/jira/browse/MESOS-4035
> https://issues.apache.org/jira/browse/MESOS-4038
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By default cgroup management is not enabled on CentOS 6.6. As process 
> isolation
> can use cgroups, documentation on how to activate cgroup management using
> cgconfig has been added. Furthermore the Mesos tests need the 'perf_event'
> cgroup subsystem to be present. Instructions on how to do this have been added
> as well.
> 
> 
> Diffs
> -
> 
>   docs/getting-started.md 0982d9038d87cec6eaa72b5584e0877fc2b9f04c 
> 
> Diff: https://reviews.apache.org/r/42196/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>



Re: Review Request 42236: Added a utility function to create https URI.

2016-01-13 Thread Timothy Chen

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

Ship it!


Ship It!


src/uri/schemes/http.hpp (line 54)


Should we provide the default port for https here?


- Timothy Chen


On Jan. 13, 2016, 6:22 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42236/
> ---
> 
> (Updated Jan. 13, 2016, 6:22 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Gilbert Song, Jojy Varghese, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-4296
> https://issues.apache.org/jira/browse/MESOS-4296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a utility function to create https URI.
> 
> 
> Diffs
> -
> 
>   src/uri/schemes/http.hpp 84a80b91ee664c29eed69f94d505aad45be326cf 
> 
> Diff: https://reviews.apache.org/r/42236/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 42237: Added docker image manifest parsing functions for strings.

2016-01-13 Thread Timothy Chen

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

Ship it!


Ship It!

- Timothy Chen


On Jan. 13, 2016, 6:23 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42237/
> ---
> 
> (Updated Jan. 13, 2016, 6:23 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Gilbert Song, Jojy Varghese, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-4296
> https://issues.apache.org/jira/browse/MESOS-4296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added docker image manifest parsing functions for strings.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp 822b2383c6d6bd691cb1fdbfcd699ae3ae3585bd 
>   src/docker/spec.cpp 0188078037b3528acefa42d39f05bd60789b19b2 
> 
> Diff: https://reviews.apache.org/r/42237/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 42263: Added more structure for containerizer related subpages in home.md.

2016-01-13 Thread Joerg Schad

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

(Updated Jan. 13, 2016, 8:38 p.m.)


Review request for mesos, Joerg Schad and Jojy Varghese.


Repository: mesos


Description
---

Added more structure for containerizer related subpages in home.md.


Diffs
-

  docs/home.md 344c5617dfb003453fa17974500d44a8f766cb16 

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


Testing (updated)
---

Viewed via Docker website generator.


Thanks,

Joerg Schad



Re: Review Request 42216: Fixed gmock warnings in hook tests.

2016-01-13 Thread Neil Conway

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

Ship it!


Ship It!

- Neil Conway


On Jan. 12, 2016, 10:27 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42216/
> ---
> 
> (Updated Jan. 12, 2016, 10:27 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Neil Conway.
> 
> 
> Bugs: MESOS-4348
> https://issues.apache.org/jira/browse/MESOS-4348
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We did not have an expectation on the `shutdown` method of the 
> `MockExecutor`. This led to the gmock warning being emitted in some runs.
> 
> 
> Diffs
> -
> 
>   src/tests/hook_tests.cpp 715b0b07e1befdca5b09172a439edccd7546d77f 
> 
> Diff: https://reviews.apache.org/r/42216/diff/
> 
> 
> Testing
> ---
> 
> Loop the tests for 1000 times.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 41820: Pulled out provisioner from linux filesystem isolator.

2016-01-13 Thread Timothy Chen

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

Ship it!


Ship It!

- Timothy Chen


On Jan. 11, 2016, 8:15 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41820/
> ---
> 
> (Updated Jan. 11, 2016, 8:15 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4240
> https://issues.apache.org/jira/browse/MESOS-4240
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Pulled out provisioner from linux filesystem isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 89d18624b08f2c953b9fa21663aeda694d4aac12 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f3c370aeb331beb6202fd30cd0278877da0b42e0 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
> a29f9d184a0d1088577b3168a12bc8c06493a477 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 8af2cecdca947e55d8d39e26d2fc3d42212f36c3 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 5bb85034c22caef64054c1629f6fd55d227e48b1 
> 
> Diff: https://reviews.apache.org/r/41820/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>



Re: Review Request 41820: Pulled out provisioner from linux filesystem isolator.

2016-01-13 Thread Gilbert Song

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

(Updated Jan. 13, 2016, 1:19 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Pulled out provisioner from linux filesystem isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
89d18624b08f2c953b9fa21663aeda694d4aac12 
  src/slave/containerizer/mesos/containerizer.cpp 
f3c370aeb331beb6202fd30cd0278877da0b42e0 
  src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
a29f9d184a0d1088577b3168a12bc8c06493a477 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
8af2cecdca947e55d8d39e26d2fc3d42212f36c3 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
5bb85034c22caef64054c1629f6fd55d227e48b1 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song



Re: Review Request 41820: Pulled out provisioner from linux filesystem isolator.

2016-01-13 Thread Jie Yu

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

Ship it!


Thanks! THis is great!


src/slave/containerizer/mesos/containerizer.cpp (line 1365)


We are not destroying the 'provisioner'. So
```
Failed to destroy the provisioned filesystem when...
```


- Jie Yu


On Jan. 11, 2016, 8:15 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41820/
> ---
> 
> (Updated Jan. 11, 2016, 8:15 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4240
> https://issues.apache.org/jira/browse/MESOS-4240
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Pulled out provisioner from linux filesystem isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 89d18624b08f2c953b9fa21663aeda694d4aac12 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f3c370aeb331beb6202fd30cd0278877da0b42e0 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
> a29f9d184a0d1088577b3168a12bc8c06493a477 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 8af2cecdca947e55d8d39e26d2fc3d42212f36c3 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 5bb85034c22caef64054c1629f6fd55d227e48b1 
> 
> Diff: https://reviews.apache.org/r/41820/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>



Re: Review Request 42237: Added docker image manifest parsing functions for strings.

2016-01-13 Thread Gilbert Song

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

Ship it!


LGTM!

- Gilbert Song


On Jan. 12, 2016, 10:23 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42237/
> ---
> 
> (Updated Jan. 12, 2016, 10:23 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Gilbert Song, Jojy Varghese, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-4296
> https://issues.apache.org/jira/browse/MESOS-4296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added docker image manifest parsing functions for strings.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp 822b2383c6d6bd691cb1fdbfcd699ae3ae3585bd 
>   src/docker/spec.cpp 0188078037b3528acefa42d39f05bd60789b19b2 
> 
> Diff: https://reviews.apache.org/r/42237/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 41870: Added ref-qualifiers to Option::get().

2016-01-13 Thread Anand Mazumdar

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

(Updated Jan. 13, 2016, 8:12 p.m.)


Review request for mesos, Joris Van Remoortere and Michael Park.


Changes
---

Removed [stout] prefix.


Summary (updated)
-

Added ref-qualifiers to Option::get().


Repository: mesos


Description
---

This trivial change adds ref-qualifiers introduced in C++11 to Option::get(). 
This is in sync with what boost/folly `optional` already have.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 
b58350b312c1969aa86ec679e95af0f97846c141 

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


Testing
---

make check

Verified this enables operations like the following to incur a move instead of 
a copy:

```
Option create();
T t = create().get();
```


Thanks,

Anand Mazumdar



Re: Review Request 41617: Added a new category called whitespace/mesos-comments to capture missing, leading, white-space in comments

2016-01-13 Thread Avinash sridharan

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

(Updated Jan. 13, 2016, 8:15 p.m.)


Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Added a new category called whitespace/mesos-comments to capture missing, 
leading, white-space in comments


Diffs
-

  support/cpplint.py 6890e27f92603b025e25e4db01decf351c33c9a1 
  support/mesos-style.py 5ee17593153df7815426918a1da6e25cb1188bca 

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


Testing
---

Ran the modified mesos-style.py on the existing code-base and got the following 
errors:
rc/authentication/cram_md5/authenticatee.hpp:60:  Should have a space between 
// and comment  [whitespace/mesos-comments] [4]
  2 src/authentication/cram_md5/authenticator.hpp:60:  Should have a space 
between // and comment  [whitespace/mesos-comments] [4]
  3 src/authorizer/local/authorizer.hpp:82:  Should have a space between // and 
comment  [whitespace/mesos-comments] [4]
  4 src/tests/containerizer/docker_containerizer_tests.cpp:921:  Should have a 
space between // and comment  [whitespace/mesos-comments] [4]
  5 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp:104:  Should have a 
space between // and comment  [whitespace/mesos-comments] [4]
  6 Total errors found: 1
  
  
  Doesn't seem to give any false-positives. All the above errors do have a 
missing leading white-space. 
  
  Tried a commit for the following patch:
  diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp  
index 90d56b9..17fcbed 100644  
--- a/src/tests/slave_tests.cpp  
+++ b/src/tests/slave_tests.cpp  
@@ -2238,7 +2238,7 @@ TEST_F(SlaveTest, DiscoveryInfoAndPorts)  
   EXPECT_SOME(portResult1);  
   EXPECT_SOME(portResult2);  
  
-  // Verify that the ports retrieved from state.json are the ones that were 
set.  
+  //Verify that the ports retrieved from state.json are the ones that were 
set.  
   EXPECT_EQ(JSON::Object(JSON::protobuf(*port1)), portResult1.get());  
   EXPECT_EQ(JSON::Object(JSON::protobuf(*port2)), portResult2.get());  


And the git commit hook correctly failed it:
git commit -m "Checking the new hook"
Checking 1 files
src/tests/slave_tests.cpp:2241:  Should have a space between // and comment  
[whitespace/mesos-comments] [4]
Total errors found: 1


Thanks,

Avinash sridharan



Re: Review Request 42216: Fixed gmock warnings in hook tests.

2016-01-13 Thread Neil Conway

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


Can you include the names of the tests in the commit message?

- Neil Conway


On Jan. 12, 2016, 10:27 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42216/
> ---
> 
> (Updated Jan. 12, 2016, 10:27 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Neil Conway.
> 
> 
> Bugs: MESOS-4348
> https://issues.apache.org/jira/browse/MESOS-4348
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We did not have an expectation on the `shutdown` method of the 
> `MockExecutor`. This led to the gmock warning being emitted in some runs.
> 
> 
> Diffs
> -
> 
>   src/tests/hook_tests.cpp 715b0b07e1befdca5b09172a439edccd7546d77f 
> 
> Diff: https://reviews.apache.org/r/42216/diff/
> 
> 
> Testing
> ---
> 
> Loop the tests for 1000 times.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 42215: Added expectations for call to mocked executor shutdown.

2016-01-13 Thread Neil Conway

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

Ship it!


Can you include the names of the tests in the commit message?

- Neil Conway


On Jan. 13, 2016, 12:05 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42215/
> ---
> 
> (Updated Jan. 13, 2016, 12:05 a.m.)
> 
> 
> Review request for mesos, Neil Conway and Timothy Chen.
> 
> 
> Bugs: MESOS-4347
> https://issues.apache.org/jira/browse/MESOS-4347
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added expectations for call to mocked executor shutdown. The mock executor's 
> shutdown method sometimes gets executed before the test exits, so it should 
> be explicitly expected.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 61d275c47fbb02baf22e4ad8f9b1580886da51d9 
> 
> Diff: https://reviews.apache.org/r/42215/diff/
> 
> 
> Testing
> ---
> 
> I used `GTEST_FILTER="ReservationTest.*" bin/mesos-tests.sh 
> --gtest_repeat=1 --gtest_break_on_failure=1 | grep -B 3 -A 6 WARNING` to 
> reproduce the warnings, and then to verify that they were gone after the fix. 
> Note that when running this command after the patch, other warning may be 
> seen due to https://issues.apache.org/jira/browse/MESOS-4350
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 41243: Updated how we find the .git directory in bootstrap.

2016-01-13 Thread Kevin Klues

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

(Updated Jan. 13, 2016, 6:41 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joseph Wu, Till 
Toenshoff, and Vinod Kone.


Changes
---

Updated the relPath() function to always operate on absolute paths as well as 
cleaned up the logic / variable names to make it a little more 
readable/maintainable.


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


Repository: mesos


Description
---

When building from git, bootstrap will (among other things) install
pre-commit and post-rewrite hooks into the .git/hooks directory of the
mesos tree. However the current implementation always assumes that .git
exists in the same directory as the bootstrap file. This may not always
be true.

Most notably, it is not true if the mesos tree is included as a
submodule inside another project. When included as a submodule, .git is
no longer a directory, but rather a file whose text contains a pointer
back to the actual location of the .git folder inside the containing
project. To get at this directory, we need to run 'git rev-parse
--git-common-dir' instead of simply assuming that the local .git is the
proper directory.

Review: https://reviews.apache.org/r/41243


Diffs (updated)
-

  bootstrap ea71ff22e42bf5e224483508d91c33ddfd350f52 

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


Testing
---

Tested on native Mac OS X El Capitan
Tested on vagrant boxes for bento/centos-7.1, ubuntu/trusty64, and 
https://github.com/tommy-muehle/puppet-vagrant-boxes/releases/download/1.0.0/centos-6.6-x86_64.box
Also tested on docker image centos:7


Thanks,

Kevin Klues



Re: Review Request 42236: Added a utility function to create https URI.

2016-01-13 Thread Gilbert Song

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

Ship it!


LGTM! Just consider whether or not we should put it as `https.hpp` under 
`schemes` directory.

- Gilbert Song


On Jan. 12, 2016, 10:22 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42236/
> ---
> 
> (Updated Jan. 12, 2016, 10:22 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Gilbert Song, Jojy Varghese, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-4296
> https://issues.apache.org/jira/browse/MESOS-4296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a utility function to create https URI.
> 
> 
> Diffs
> -
> 
>   src/uri/schemes/http.hpp 84a80b91ee664c29eed69f94d505aad45be326cf 
> 
> Diff: https://reviews.apache.org/r/42236/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 42059: Updated ContainerLogger to use Subprocess::IO type.

2016-01-13 Thread Benjamin Hindman

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

Ship it!



src/slave/containerizer/mesos/containerizer.cpp (lines 829 - 832)


Subprocess::IO out = Subprocess::FD(STDOUT_FILENO);
Subprocess::IO err = Subprocess::FD(STDERR_FILENO);

if (!local) {
  out = subprocessInfo.out;
  err = subprocessInfo.err;
}


- Benjamin Hindman


On Jan. 8, 2016, 6:28 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42059/
> ---
> 
> (Updated Jan. 8, 2016, 6:28 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update ContainerLogger to use Subprocess::IO type
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/container_logger.hpp 
> a2362070ead0afcef1e6c2ca784083ddb01ba51a 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f3c370aeb331beb6202fd30cd0278877da0b42e0 
> 
> Diff: https://reviews.apache.org/r/42059/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 41783: Logger Module: Implement the rotating container logger module.

2016-01-13 Thread Benjamin Hindman

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



src/slave/container_loggers/rotate.hpp (line 36)


{ on newline please.



src/slave/container_loggers/rotate.hpp (lines 46 - 50)


This should be `Bytes` rather than force the reader/user to write this in 
bytes (even if it's code that is generating this) and force you to use it as a 
`size_t`.

Same for other flags you have here and below too please.



src/slave/container_loggers/rotate.hpp (line 54)


How about s/of log/of rotated log/ here?

Also, is 'head' better than 'current'? Just a suggestion, given that head 
sounds like "first", I thought by head you meant the oldest log file, not the 
newest.



src/slave/container_loggers/rotate.cpp (lines 35 - 37)


These pulled out?



src/slave/container_loggers/rotate.cpp (lines 57 - 61)


Can you send me another review which updates the version of `io::read` that 
you're using to `dup`, `os::cloexec`, and `os::nonblock` the file descriptor so 
we don't have to? Then we can kill this (and other places that use that version 
of `io::read` too!).



src/slave/container_loggers/rotate.cpp (lines 207 - 218)


Want to do this validation in the flags themselves?



src/slave/container_loggers/rotating.hpp (line 49)


How about:

s/of stdout/of rotated stdout (i.e., stdout.1, stdout.2, ...)/


- Benjamin Hindman


On Jan. 8, 2016, 8:06 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41783/
> ---
> 
> (Updated Jan. 8, 2016, 8:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4136
> https://issues.apache.org/jira/browse/MESOS-4136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds a non-default ContainerLogger that constrains total log size by rotating 
> logs (i.e. renaming the head log file).
> 
> 
> Diffs
> -
> 
>   src/slave/container_loggers/rotate.hpp PRE-CREATION 
>   src/slave/container_loggers/rotate.cpp PRE-CREATION 
>   src/slave/container_loggers/rotating.hpp PRE-CREATION 
>   src/slave/container_loggers/rotating.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41783/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 42254: Checked whether the remaining cluster resources is allocatable.

2016-01-13 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [42221, 42254]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 13, 2016, 3:29 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42254/
> ---
> 
> (Updated Jan. 13, 2016, 3:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Guangya Liu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Checked whether the remaining cluster resources is allocatable.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
> 
> Diff: https://reviews.apache.org/r/42254/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 41870: [stout] Added ref-qualifiers to Option::get().

2016-01-13 Thread Anand Mazumdar

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

(Updated Jan. 13, 2016, 8:11 p.m.)


Review request for mesos, Joris Van Remoortere and Michael Park.


Changes
---

Review comment


Repository: mesos


Description
---

This trivial change adds ref-qualifiers introduced in C++11 to Option::get(). 
This is in sync with what boost/folly `optional` already have.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 
b58350b312c1969aa86ec679e95af0f97846c141 

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


Testing
---

make check

Verified this enables operations like the following to incur a move instead of 
a copy:

```
Option create();
T t = create().get();
```


Thanks,

Anand Mazumdar



Re: Review Request 42263: Added more structure for containerizer related subpages in home.md.

2016-01-13 Thread Jojy Varghese

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

Ship it!


Ship It!

- Jojy Varghese


On Jan. 13, 2016, 8:38 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42263/
> ---
> 
> (Updated Jan. 13, 2016, 8:38 p.m.)
> 
> 
> Review request for mesos, Joerg Schad and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added more structure for containerizer related subpages in home.md.
> 
> 
> Diffs
> -
> 
>   docs/home.md 344c5617dfb003453fa17974500d44a8f766cb16 
> 
> Diff: https://reviews.apache.org/r/42263/diff/
> 
> 
> Testing
> ---
> 
> Viewed via Docker website generator.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 41243: Updated how we find the .git directory in bootstrap.

2016-01-13 Thread Kevin Klues


> On Jan. 13, 2016, 12:26 p.m., Till Toenshoff wrote:
> > This recent change breaks the functionality - it hangs now.

Looks like 'git rev-parse --git-common-dir' sometimes returns the absolute path 
of the .git directory, and other times it returns a relative path.  My 
relPath() function assumes both paths are absolute. Patching up to force both 
to be absolute.


- Kevin


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


On Jan. 13, 2016, 6:41 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41243/
> ---
> 
> (Updated Jan. 13, 2016, 6:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joseph Wu, 
> Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-4125
> https://issues.apache.org/jira/browse/MESOS-4125
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When building from git, bootstrap will (among other things) install
> pre-commit and post-rewrite hooks into the .git/hooks directory of the
> mesos tree. However the current implementation always assumes that .git
> exists in the same directory as the bootstrap file. This may not always
> be true.
> 
> Most notably, it is not true if the mesos tree is included as a
> submodule inside another project. When included as a submodule, .git is
> no longer a directory, but rather a file whose text contains a pointer
> back to the actual location of the .git folder inside the containing
> project. To get at this directory, we need to run 'git rev-parse
> --git-common-dir' instead of simply assuming that the local .git is the
> proper directory.
> 
> Review: https://reviews.apache.org/r/41243
> 
> 
> Diffs
> -
> 
>   bootstrap ea71ff22e42bf5e224483508d91c33ddfd350f52 
> 
> Diff: https://reviews.apache.org/r/41243/diff/
> 
> 
> Testing
> ---
> 
> Tested on native Mac OS X El Capitan
> Tested on vagrant boxes for bento/centos-7.1, ubuntu/trusty64, and 
> https://github.com/tommy-muehle/puppet-vagrant-boxes/releases/download/1.0.0/centos-6.6-x86_64.box
> Also tested on docker image centos:7
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 41819: Added ContainerConfig to all isolators.

2016-01-13 Thread Gilbert Song

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

(Updated Jan. 13, 2016, 1:19 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Added ContainerConfig to all isolators.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
f3c370aeb331beb6202fd30cd0278877da0b42e0 
  src/slave/containerizer/mesos/isolator.cpp 
493b5dd26cf0e8f986381a502cfa6d1dde6573d4 
  src/slave/containerizer/mesos/isolators/cgroups/cpushare.hpp 
123b9ed3ccaebcd5da24fc62ff7a92d4a81ed760 
  src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp 
3b95e195ad704f163c245175390d9a26bde7e17c 
  src/slave/containerizer/mesos/isolators/cgroups/mem.hpp 
09952369c72d3c6322ae7a1c73cd68226d452ad2 
  src/slave/containerizer/mesos/isolators/cgroups/mem.cpp 
2ddb9f4adbb879682cd39966ab974cf3fa32209c 
  src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp 
5eaf49f1f35c93ad4465adb6c9c9cf57b3a2c6ee 
  src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
4d82c2b2f231c59cbb600869a0f2b716c1e55f5e 
  src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
a29f9d184a0d1088577b3168a12bc8c06493a477 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
8af2cecdca947e55d8d39e26d2fc3d42212f36c3 
  src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
c3544aa313cbb185efb03bba59961cdf2b616a37 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
4d6100e49aa1e4dcc78900a51826de0658c540d6 
  src/slave/containerizer/mesos/isolators/filesystem/shared.hpp 
2e457015a0348a457581edf493877b71fab17090 
  src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
361ed6561bd5e2f75d026922def01f42b43d61c2 
  src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
c2d1455249618f9cd2e17dc2244b184d52b32eaf 
  src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
d65c1593b44f4b21237581147e57e441ebf3160d 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
e3766c313a1bd2a838a73730c62c74c5ee8e1a4c 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
22a8428427b758bae4a0518356d7933c4110cd9f 
  src/slave/containerizer/mesos/isolators/posix.hpp 
7e1ebc2fada5a5e291e84c7044bdba9a71f4b42c 
  src/slave/containerizer/mesos/isolators/posix/disk.hpp 
31808c1e8199fbf2cea36c273860fdbf0a2388f8 
  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
248c34adb63907911d89bed5b1519682a852bb2d 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song



Re: Review Request 41595: Updated `Master::Http::state` to use `jsonify` in mesos.

2016-01-13 Thread Alexander Rojas

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


I was wondering what the results of the benchmark would be with an optimized 
build.

- Alexander Rojas


On Jan. 6, 2016, 3:48 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41595/
> ---
> 
> (Updated Jan. 6, 2016, 3:48 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 4f4cbf6e53588b72204f9628dea5696c71eb66a5 
>   src/common/http.cpp 7165551321bedb8a4d711a64d0d6d8fd15215424 
>   src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5 
> 
> Diff: https://reviews.apache.org/r/41595/diff/
> 
> 
> Testing
> ---
> 
> # Some preliminery numbers
> 
> These numbers are from my Ubuntu VM on my Mac OS X with @vinodkone's 
> benchmark test: [r40844](https://reviews.apache.org/r/40844/).
> 
> ### Before
> 
> ```
> [==] Running 36 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 36 tests from SlaveAndFrameworkCount/MasterState_BENCHMARK_Test
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/0
> Added 1000 slaves and 1 frameworks
> Received state.json response in 683.138216ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/0 (1436 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/1
> Added 1000 slaves and 50 frameworks
> Received state.json response in 550.636939ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/1 (1474 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/2
> Added 1000 slaves and 100 frameworks
> Received state.json response in 632.835236ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/2 (1491 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/3
> Added 1000 slaves and 200 frameworks
> Received state.json response in 584.035771ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/3 (1431 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/4
> Added 1000 slaves and 500 frameworks
> Received state.json response in 688.404348ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/4 (1586 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/5
> Added 1000 slaves and 1000 frameworks
> Received state.json response in 666.713683ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/5 (1590 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/6
> Added 5000 slaves and 1 frameworks
> Received state.json response in 3.916201532secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/6 (7852 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/7
> Added 5000 slaves and 50 frameworks
> Received state.json response in 3.362618796secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/7 (8315 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/8
> Added 5000 slaves and 100 frameworks
> Received state.json response in 3.126815189secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/8 (7153 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/9
> Added 5000 slaves and 200 frameworks
> Received state.json response in 3.079956539secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/9 (7534 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/10
> Added 5000 slaves and 500 frameworks
> Received state.json response in 3.222014521secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/10 (8129 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/11
> Added 5000 slaves and 1000 frameworks
> Received state.json response in 3.286657158secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/11 (8133 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/12
> Added 1 slaves and 1 frameworks
> Received state.json response in 7.332592151secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/12 
> (15639 ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/13
> Added 1 slaves and 50 frameworks
> Received state.json response in 6.998751033secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/13 
> (17175 ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/14
> Added 

Re: Review Request 42172: Add documentation for logging and ContainerLogger.

2016-01-13 Thread Benjamin Hindman

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

Ship it!


Ship It!

- Benjamin Hindman


On Jan. 12, 2016, 7:24 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42172/
> ---
> 
> (Updated Jan. 12, 2016, 7:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-4206
> https://issues.apache.org/jira/browse/MESOS-4206
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moves (and updates) logging flags from `configuration.md` into the new 
> `logging.md`.
> Adds documentation about logging in Master/Agent/Framework and the 
> `ContainerLogger`.
> 
> Allows the doc-linking pattern of `[link text](other_doc.md#Link-to-text)`.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md cbe7f5a338a0fc350c4b6c0e2f1f48bd0869ac34 
>   docs/home.md 344c5617dfb003453fa17974500d44a8f766cb16 
>   docs/logging.md PRE-CREATION 
>   docs/modules.md 1aad8e0958554c0219a287dffd6c6fb925edb025 
>   site/Rakefile 94833488c8e29f9b07e3cab3c8adce223f685679 
> 
> Diff: https://reviews.apache.org/r/42172/diff/
> 
> 
> Testing
> ---
> 
> Previewed on GitHub and via:
> ```
> docker build -t mesos/website support/site-docker
> docker run -it --rm -p 4567:4567 -v /path/to/mesos:/mesos mesos/website
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 42273: Modified scheduler library to move the queue contents before `async`.

2016-01-13 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [42273]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 13, 2016, 11:06 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42273/
> ---
> 
> (Updated Jan. 13, 2016, 11:06 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This trivial fix invokes `std::move` on the `queue` object to not perform a 
> copy before invoking the `async(..)` call.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp a17872b46ec600e0fae6c43247ccb63f5ee55ac0 
> 
> Diff: https://reviews.apache.org/r/42273/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 42164: Allowed (un)reserve operations without a principal.

2016-01-13 Thread Guangya Liu


> On 一月 13, 2016, 5:22 a.m., Michael Park wrote:
> > src/master/validation.cpp, line 703
> > 
> >
> > First thing here is that the `principal` field in `ReservationInfo` is 
> > still marked `required`. What's our plan for changing it to `optional`?
> > 
> > Second thing is that even if `principal.isNone()`, if is authorization 
> > is turned off we don't care, right? That is, if authorization is turned 
> > off, even frameworks without a principal can reserve any resources?
> 
> Greg Mann wrote:
> Regarding your first question: per our discussion, we do indeed need a 
> good plan for migrating to an `optional` principal within `ReservationInfo`. 
> It would seem that we have to choose between two less-than-ideal situations: 
> either we tolerate HTTP endpoints that don't work when authentication is 
> disabled, or we enforce that frameworks cannot register with `principal == 
> ""`, so that we can use the empty string to represent a null principal while 
> we migrate to an `optional` principal over a deprecation cycle.
> 
> To your second point: again per our discussion, semantically the 
> principal within `ReservationInfo` represents the principal (or lack thereof) 
> which reserved the resources, so we should enforce that it matches the 
> principal of the framework/operator, even if authorization is not enabled.
> 
> Guangya Liu wrote:
> If authorization is not enabled, then it is not a must to set principal 
> for a framework, so how can we enforce the match of principal?
> 
> Greg Mann wrote:
> We should enforce that the principal (or lack of a principal) used for 
> authorization is the same as the principal in the `ReservationInfo`. So if no 
> principal is provided (i.e., authorization disabled), then there should be no 
> principal in `ReservationInfo`. Since the principal in `ReservationInfo` is 
> currently a required field, we would need to use something like the empty 
> string `""` to represent the case of no principal. Ideally, the principal 
> field in `ReservationInfo` should be an optional field so that it can be 
> omitted when no principal is used. The plan is to change this field to 
> optional, but we're debating how and when it will be best to do so. Any 
> thoughts on how best to do this?

Compared with `HTTP endpoints that don't work when authentication is disabled`, 
I think that `enforce that frameworks cannot register with principal == ""` 
might be better as it will only impact some framework logic and more simple, 
but authorization require the whole cluster and all frameworks must enable 
authorization.


- Guangya


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


On 一月 13, 2016, 2:45 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42164/
> ---
> 
> (Updated 一月 13, 2016, 2:45 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3940
> https://issues.apache.org/jira/browse/MESOS-3940
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allowed (un)reserve operations without a principal.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/tests/master_validation_tests.cpp 
> fbf8fadbc04a7cbc60ee6091e0224339389b400f 
>   src/tests/reservation_endpoints_tests.cpp 
> b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
> 
> Diff: https://reviews.apache.org/r/42164/diff/
> 
> 
> Testing
> ---
> 
> Two master validation tests were removed which tested to make sure that 
> reserve and unreserve operations would not succeed without a principal.
> 
> `make check` was used to test on both OSX and Ubuntu 14.04
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



  1   2   3   >