Re: Review Request 41789: Expose the http::internal::request function.

[Joerg Schad]

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



3rdparty/libprocess/src/http.cpp (line 1280)


Would it make sense to add a test (e.g. testing DELETE)?


- Joerg Schad


On Jan. 15, 2016, 7:50 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41789/
> ---
> 
> (Updated Jan. 15, 2016, 7:50 a.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil 
> Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-3763
> https://issues.apache.org/jira/browse/MESOS-3763
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Expose the internal::http::request function in the header and not add an 
> additional method/overload for put function.
> 
> (TODO): Clean the other instances of post/get to use the http::request method.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> d0f820e0850955c01c5149fef2f05a1becb4bd32 
>   3rdparty/libprocess/src/http.cpp aabfd5dd1f93c190edce6d54e977dc56034c1ac9 
> 
> Diff: https://reviews.apache.org/r/41789/diff/
> 
> 
> Testing
> ---
> 
> Make & Make check successfully.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 41333: Added helper functions to filter usage slack resources.

[Guangya Liu]

> On 一月 15, 2016, 2:36 a.m., Joseph Wu wrote:
> > include/mesos/resources.hpp, line 174
> > 
> >
> > You should briefly define usage slack here.
> > 
> > Suggestion: s/revocable/, revocable resources that are allocated but 
> > under-utilized/
> > 
> > Ditto below.

Thanks Jospeh, will update this if we reach some agreement on its parenent 
patch for allocation slack ;-)


- Guangya


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


On 一月 8, 2016, 6:03 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41333/
> ---
> 
> (Updated 一月 8, 2016, 6:03 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
> https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper functions to filter usage slack resources. This helper function 
> will be used to validate if the task is using usage slack resource or not so 
> as to make sure that one task or executor can only use same kind of resources.
> 
> 
> Diffs
> -
> 
>   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/41333/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41789: Expose the http::internal::request function.

[Yongqiao Wang]

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

(Updated Jan. 14, 2016, 11:50 p.m.)


Review request for mesos, Adam B, Joerg Schad, Joris Van Remoortere, Neil 
Conway, and Qian Zhang.


Changes
---

Changed linked JIRA from MESOS-4200 (dynamic weights tests) to MESOS-3763 
(http::put)


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


Repository: mesos


Description
---

Expose the internal::http::request function in the header and not add an 
additional method/overload for put function.

(TODO): Clean the other instances of post/get to use the http::request method.


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
d0f820e0850955c01c5149fef2f05a1becb4bd32 
  3rdparty/libprocess/src/http.cpp aabfd5dd1f93c190edce6d54e977dc56034c1ac9 

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


Testing
---

Make & Make check successfully.


Thanks,

Yongqiao Wang

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

[Michael Park]

> On Jan. 14, 2016, 4:25 a.m., Benjamin Hindman wrote:
> > src/master/http.cpp, lines 135-146
> > 
> >
> > A basic comment that implies that a `Summary` representation is for the 
> > '/state-summary' endpoints and `Full` representation is for the '/state' 
> > endpoints (or maybe in the future for any "summary" endpoint) would be 
> > great. ;-)

Added comments over both of these.


> On Jan. 14, 2016, 4:25 a.m., Benjamin Hindman wrote:
> > src/master/http.cpp, line 175
> > 
> >
> > I can't remember what the outcome of our converation about this was, 
> > but do you even need the `Full` representation? Could you just have a `void 
> > json(JSON::ObjectWriter* writer, const Slave& slave)` work without needing 
> > to explicitly use `Full`? If we need `Full` in addition to `Summary` a 
> > comment above over `Full` that explains that for others would be great!

We don't "need" it per se, but for now I think it makes sense to be explicit 
about which one at the callsite. If we were to get rid of the summary endpoints 
later on, it would make sense at that point to get rid of `Summary` and 
collapse `Full` into `T`.


- Michael


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


On Jan. 15, 2016, 7:41 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41595/
> ---
> 
> (Updated Jan. 15, 2016, 7:41 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 bcafc7aff89659a68352f3876ce6042f8b34bd5d 
> 
> 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

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

[Michael Park]

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

(Updated Jan. 15, 2016, 7:41 a.m.)


Review request for mesos and Benjamin Hindman.


Changes
---

Addressed BenH's comments.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/common/http.hpp 4f4cbf6e53588b72204f9628dea5696c71eb66a5 
  src/common/http.cpp 7165551321bedb8a4d711a64d0d6d8fd15215424 
  src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d 

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 1 slaves and 100 frameworks
Received state.json response in 6.807456891secs
[   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/14 (15974 
ms)
[ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/15
Added 1 slaves and 200 frameworks
Received state.json response in 6.371190971secs
[   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/15 (15772 
ms)
[ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/16
Added 1 slaves and 500 frameworks
Received state.json response in 6.613739531secs
[   OK ]

Re: Review Request 41594: Added support for `jsonify` result to `OK` response in libprocess.

[Michael Park]

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

(Updated Jan. 15, 2016, 7:40 a.m.)


Review request for mesos and Benjamin Hindman.


Changes
---

Rebased.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/Makefile.am 
084475ac4ef58c60da67a8d50ac031a5591eb335 
  3rdparty/libprocess/include/process/http.hpp 
d0f820e0850955c01c5149fef2f05a1becb4bd32 
  3rdparty/libprocess/src/http.cpp aabfd5dd1f93c190edce6d54e977dc56034c1ac9 

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


Testing
---


Thanks,

Michael Park

Re: Review Request 42241: Speed up HookTest.VerifySlaveLaunchExecutorHook.

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 一月 15, 2016, 5:10 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42241/
> ---
> 
> (Updated 一月 15, 2016, 5:10 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Timothy Chen.
> 
> 
> Bugs: MESOS-4174
> https://issues.apache.org/jira/browse/MESOS-4174
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add Clock::advance to fasten the speed of executor to be shutdown.
> 
> 
> Diffs
> -
> 
>   src/tests/hook_tests.cpp 715b0b07e1befdca5b09172a439edccd7546d77f 
> 
> Diff: https://reviews.apache.org/r/42241/diff/
> 
> 
> Testing
> ---
> 
> Before
> HookTest.VerifySlaveLaunchExecutorHook (5061 ms)
> 
> After
> HookTest.VerifySlaveLaunchExecutorHook (132 ms)
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

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

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [41302]

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

Error:
 2016-01-15 07:25:53 URL:https://reviews.apache.org/r/41302/diff/raw/ 
[1701/1701] -> "41302.patch" [1]
No files to lint

Error: Commit message summary (the first line) must end in a period.

- Mesos ReviewBot


On Jan. 15, 2016, 5:19 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41308/
> ---
> 
> (Updated Jan. 15, 2016, 5:19 a.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/master/master.cpp 863a11c82d322f56db1ccf25b73a41304a46 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> cb58b7183c36d96b9ac4803c63980c278a50c97b 
>   src/tests/master_tests.cpp 9638fb867b07f85a02d3b78f8282843d8871cabe 
>   src/tests/master_validation_tests.cpp 
> fbf8fadbc04a7cbc60ee6091e0224339389b400f 
>   src/tests/monitor_tests.cpp 2226458b59b4a279a92e1353bd61457a0018d2a9 
>   src/tests/reservation_endpoints_tests.cpp 
> b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
>   src/tests/slave_recovery_tests.cpp 6683a081ac231f1e275a3cdb8ee841da430a9f66 
>   src/tests/slave_tests.cpp 076660daf025d6fd5065cd0c1930f17ecc5ca5aa 
> 
> Diff: https://reviews.apache.org/r/41308/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

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

[Ben Mahler]

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

Ship it!


Very nice! I'll make some minor adjustments based on the comments and get this 
committed for you.


src/tests/master_tests.cpp (lines 3921 - 3939)


I tried to consolidate these comments by pulling down the parts of the test 
level comment into the test body:

```
// Test the max_completed_framework flag for master.
TEST_F(MasterTest, MaxCompletedTasksPerFrameworkFlag)
{
  // In order to verify that the proper amount of history
  // is maintained, we launch exactly 2 frameworks when
  // 'max_completed_frameworks' is set to 0, 1 and 2. This
  // covers the cases of maintaining 0 history, some history
  // less than the total number of frameworks launched, and
  // history equal to the total number of frameworks launched.
  const size_t totalFrameworks = 2;
  const size_t maxFrameworksArray[] = {0, 1, 2};
```



src/tests/master_tests.cpp (lines 3993 - 4011)


Tried to consolidate here as well:

```
// Test the max_completed_tasks_per_framework flag for master.
TEST_F(MasterTest, MaxCompletedTasksPerFrameworkFlag)
{
  // We verify that the proper amount of history is maintained
  // by launching a single framework with exactly 2 tasks. We
  // do this when setting `max_completed_tasks_per_framework`
  // to 0, 1, and 2. This covers the cases of maintaining no
  // history, some history less than the total number of tasks
  // launched, and history equal to the total number of tasks
  // launched.
  const size_t totalTasksPerFramework = 2;
  const size_t maxTasksPerFrameworkArray[] = {0, 1, 2};
```



src/tests/master_tests.cpp (line 4011)


whoops: s/{ /{/



src/tests/master_tests.cpp (line 4085)


Perhaps:

```
// There should be only 1 completed framework.
```

Specifically, not sure if we need to describe that we "count" here?


- Ben Mahler


On Jan. 15, 2016, 12:49 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42212/
> ---
> 
> (Updated Jan. 15, 2016, 12:49 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 of frameworks and tasks per framework actually launched.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 9638fb867b07f85a02d3b78f8282843d8871cabe 
> 
> 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.MaxCompletedTasksPerFrameworkFlag:MasterTest.MaxCompletedFrameworksFlag"
>  make check -j 7
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Review Request 42342: Added a new test cases for revive offer.

[Guangya Liu]

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

Review request for mesos, Ben Mahler and Vinod Kone.


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


Repository: mesos


Description
---

Added a new test cases for revive offer.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
e044f832c2c16e53e663c6ced5452649bb0dcb59 

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


Testing
---

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


Thanks,

Guangya Liu

Re: Review Request 42241: Speed up HookTest.VerifySlaveLaunchExecutorHook.

[haosdent huang]

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

Ship it!


Ship It!

- haosdent huang


On Jan. 15, 2016, 5:10 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42241/
> ---
> 
> (Updated Jan. 15, 2016, 5:10 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Timothy Chen.
> 
> 
> Bugs: MESOS-4174
> https://issues.apache.org/jira/browse/MESOS-4174
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add Clock::advance to fasten the speed of executor to be shutdown.
> 
> 
> Diffs
> -
> 
>   src/tests/hook_tests.cpp 715b0b07e1befdca5b09172a439edccd7546d77f 
> 
> Diff: https://reviews.apache.org/r/42241/diff/
> 
> 
> Testing
> ---
> 
> Before
> HookTest.VerifySlaveLaunchExecutorHook (5061 ms)
> 
> After
> HookTest.VerifySlaveLaunchExecutorHook (132 ms)
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 39628: Cleared the suppressed flag when deactive a framework.

[Guangya Liu]

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

(Updated 一月 15, 2016, 7:16 a.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


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


Repository: mesos


Description
---

Cleared the suppressed flag when deactive a framework.


Diffs (updated)
-

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

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


Testing
---

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


Thanks,

Guangya Liu

Re: Review Request 39628: Cleared the suppressed flag when deactive a framework.

[Guangya Liu]

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

(Updated 一月 15, 2016, 7:14 a.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


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


Repository: mesos


Description
---

Cleared the suppressed flag when deactive a framework.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
  src/tests/hierarchical_allocator_tests.cpp 
e044f832c2c16e53e663c6ced5452649bb0dcb59 
  src/tests/hierarchical_allocator_tests.cpp.orig PRE-CREATION 

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


Testing
---

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


Thanks,

Guangya Liu

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

[Ben Mahler]

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


Thanks Alex! I ended up going over the structure of the recover code and left 
some higher level comments. There also appears to be a bug that will crash the 
master that I marked as an issue :)

Have we convinced ourselves that we want to act on partial state when quota is 
*not* set? It seems to complicate things here, and acting on partial state w/o 
quota has some issues as well?


src/master/allocator/mesos/hierarchical.cpp (line 149)


It now seems odd to start the allocation loop within initialize rather than 
within recover, we'll do the following:

intialize -> unpauses, starts allocation loop but the allocation loop 
should do nothing
recover -> pauses allocations, waits for condition to be met, resumes 
allocation.

It seems a simpler way to think about this would be to just start 
allocating after the recovery condition is met. This would consist of (1) 
started = true* and (2) the initial delay to 'batch' that starts the delay loop.

* Although keeping the pause concept may be useful if we want to support 
pausing/resuming allocation from endpoints.



src/master/allocator/mesos/hierarchical.cpp (lines 153 - 163)


It seems a little odd to document at the function level when this interface 
function is already documented:

```
  /**
   * Informs the allocator of the recovered state from the master.
   *
   * Because it is hard to define recovery for a running allocator, this
   * method should be called after `initialize()`, but before actual
   * allocation starts (i.e. `addSlave()` is called).
   *
   * TODO(alexr): Consider extending the signature with expected
   * frameworks count once it is available upon the master failover.
   *
   * @param quotas A (possibly empty) collection of quotas, keyed by
   * their role, known to the master.
   */
  virtual void recover(
  const int expectedAgentCount,
  const hashmap& quotas) = 0;
```

How about moving it in to the body after the checks, with some slight 
rephrasing:

```
void HierarchicalAllocatorProcess::recover(
const int _expectedAgentCount,
const hashmap& quotas)
{
  // Recovery should start before actual allocation starts.
  CHECK(initialized);
  CHECK_EQ(0u, slaves.size());
  CHECK_EQ(0, quotaRoleSorter->count());
  CHECK(_expectedAgentCount >= 0);

  // If there is no quota, recovery is a no-op. Otherwise, we need
  // to delay allocations while agents are re-registering because 
  // otherwise we perform allocations on a partial view of resources!
  // We would consequently perform unnecessary allocations to satisfy
  // quota constraints, which can over-allocate non-revocable resources
  // to roles using quota. Then, frameworks in roles without quota can
  // be unnecessarily deprived of resources. We may also be unable to
  // satisfy all of the quota constraints. Repeated master failovers
  // exacerbate the issue.
  
  if (quotas.empty()) {
VLOG(1) << "Skipping recovery of hierarchical allocator: "
<< "nothing to recover";

return;
  }
  
  ...
  
}
```

It's great that we went through the exercise of writing this comment 
because it seems we should consider whether we want to avoid acting on partial 
state even when no quota is set.



src/master/allocator/mesos/hierarchical.cpp (line 165)


I wonder if we can use a better word than "expected" here; something that 
conveys that this represents the count of agents that were registered prior to 
failover / persisted in the registry. For example, the following would be 
really clear:

```
void HierarchicalAllocatorProcess::recover(
const registry::Slaves& recoveredSlaves,
const hashmap& quotas)
```



src/master/allocator/mesos/hierarchical.cpp (line 168)


Should this comment be moved down?

I'm also not quite sure what it's expressing, is it a re-hash of the 
lifecycle documentation?

```
   * Because it is hard to define recovery for a running allocator, this
   * method should be called after `initialize()`, but before actual
   * allocation starts (i.e. `addSlave()` is called).
```

For example:

```
CHECK(initialized);

// Recovery should be called after initialize and before other calls.
CHECK_EQ(0u, slaves.size());
CHECK_EQ(0, quotaRoleSorter->c

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

[Michael Park]

> On Jan. 14, 2016, 4:25 a.m., Benjamin Hindman wrote:
> > src/master/http.cpp, line 112
> > 
> >
> > s/Type_Name/Type::Name/ here?

Double checked, it's not one of those unfortunately: 
https://developers.google.com/protocol-buffers/docs/reference/cpp-generated?hl=en#enum


- Michael


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


On Jan. 6, 2016, 2: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, 2: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

Re: Review Request 41600: Speed up SlaveTest.CommandExecutorWithOverride.

[haosdent huang]

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

Ship it!


Ship It!

- haosdent huang


On Jan. 15, 2016, 5:09 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41600/
> ---
> 
> (Updated Jan. 15, 2016, 5:09 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Timothy Chen.
> 
> 
> Bugs: MESOS-4161
> https://issues.apache.org/jira/browse/MESOS-4161
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the reason for the test case being slow is MESOS-4111, we will add 
> comment in the test case, and remove it when MESOS-4111 is fixed.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 328c853a338145256dadfc9364b9c2a0e6c356cb 
> 
> Diff: https://reviews.apache.org/r/41600/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 39628: Cleared the suppressed flag when deactive a framework.

[Guangya Liu]

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

(Updated 一月 15, 2016, 6:36 a.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


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


Repository: mesos


Description
---

Cleared the suppressed flag when deactive a framework.


Diffs (updated)
-

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

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


Testing
---

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


Thanks,

Guangya Liu

Re: Review Request 41946: Added a `Representation` abstraction to stout.

[Michael Park]

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

(Updated Jan. 15, 2016, 6:36 a.m.)


Review request for mesos and Benjamin Hindman.


Changes
---

Addressed BenH's comments.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
f32493f90f3bc2a62119dcea49eb5f4966388bf5 
  3rdparty/libprocess/3rdparty/stout/include/stout/representation.hpp 
PRE-CREATION 

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


Testing
---


Thanks,

Michael Park

Re: Review Request 39628: Cleared the suppressed flag when deactive a framework.

[Guangya Liu]

> On 一月 14, 2016, 10:55 p.m., Vinod Kone wrote:
> > src/tests/hierarchical_allocator_tests.cpp, lines 1843-1844
> > 
> >
> > How about
> > 
> > // This test checks that if a framework suppresses offers, disconnects 
> > and reconnects again,
> > // it will starting receiving resource offers again.

Minor change: // it will `start` receiving resource offers again.


- Guangya


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


On 一月 14, 2016, 9:53 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39628/
> ---
> 
> (Updated 一月 14, 2016, 9:53 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3802
> https://issues.apache.org/jira/browse/MESOS-3802
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleared the suppressed flag when deactive a framework.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/39628/diff/
> 
> 
> Testing
> ---
> 
> Ubuntu 14.04
> make 
> make check
> GLOG_v=2 ./bin/mesos-tests.sh  
> --gtest_filter="HierarchicalAllocatorTest.DeactivateActivateFramework" 
> --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41593: Added `jsonify` function to stout.

[Michael Park]

> On Jan. 14, 2016, 4:14 a.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 659
> > 
> >
> > Why do you need to pull this variable out?

This is consistent from the original code.


> On Jan. 14, 2016, 4:14 a.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 637
> > 
> >
> > Are you pulling this variable out to optimize away the calls to 
> > `field_count()`?

It looks like it. Synced with BenH offline, and we agreed this is ok.


- Michael


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


On Jan. 15, 2016, 4 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41593/
> ---
> 
> (Updated Jan. 15, 2016, 4 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 5ab7bc4966fe32eaddd573a4dbfd997f98b5d481 
>   3rdparty/libprocess/3rdparty/stout/README.md 
> a2a3dc6120b10c8fe82088b9f0ebecfa5642f945 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> f32493f90f3bc2a62119dcea49eb5f4966388bf5 
>   3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 017cef466a30e73af36d72332514a4dbb2beb6bc 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 62ad461eb228b688f1ceac16cfb003561ed5a806 
>   3rdparty/libprocess/3rdparty/stout/tests/jsonify_tests.cpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
> 34bb2523ebf3d6d390cbfe1623b89fae0053b712 
> 
> Diff: https://reviews.apache.org/r/41593/diff/
> 
> 
> Testing
> ---
> 
> Added tests to ``
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 42047: Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to isolate a mesos container using the net_cls cgroup subsystem.

[Cong Wang]

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


Why do we need netcls to regulate framework traffic on a per-container basis? 
Given the fact that a) the port range based filters already work and the code 
(see egress fq_codel) already exists b) we only have port range based network 
isolation so far.

I see no point of this. Please describe your use case with details, just 
pointing to netcls kernel doc doesn't help at all.

- Cong Wang


On Jan. 15, 2016, 5:44 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42047/
> ---
> 
> (Updated Jan. 15, 2016, 5:44 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4262
> https://issues.apache.org/jira/browse/MESOS-4262
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to 
> isolate a mesos container using the net_cls cgroup subsystem.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 39a23df3227a4f524ea0d408dc894fa5bbab7d10 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42047/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42048: Defined of the CgroupNetClsIsolatorProcess.

[Avinash sridharan]

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

(Updated Jan. 15, 2016, 5:55 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Defined of the CgroupNetClsIsolatorProcess.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp PRE-CREATION 

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


Testing
---

make check . Added a test case for CgroupNetClsIsolatorProcess.


Thanks,

Avinash sridharan

Re: Review Request 42047: Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to isolate a mesos container using the net_cls cgroup subsystem.

[Avinash sridharan]

> On Jan. 14, 2016, 11:06 p.m., Jie Yu wrote:
> > src/Makefile.am, line 819
> > 
> >
> > Ditto.

There headers:
slave/containerizer/mesos/isolators/cgroups/perf_event.hpp  \
  slave/containerizer/mesos/isolators/filesystem/linux.hpp  \
  
  We not in lexical order to start with, so leaving as is. Trying to maintain 
the order with the net_cls.hpp header. Should I fix the lexical order?


- Avinash


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


On Jan. 15, 2016, 5:44 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42047/
> ---
> 
> (Updated Jan. 15, 2016, 5:44 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4262
> https://issues.apache.org/jira/browse/MESOS-4262
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to 
> isolate a mesos container using the net_cls cgroup subsystem.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 39a23df3227a4f524ea0d408dc894fa5bbab7d10 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42047/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42047: Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to isolate a mesos container using the net_cls cgroup subsystem.

[Avinash sridharan]

> On Jan. 14, 2016, 11:05 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp, line 88
> > 
> >
> > Do you still need `_containerId` here?

Removed _containerId . Thanks.


> On Jan. 14, 2016, 11:05 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp, line 96
> > 
> >
> > Can we use Owned here?

Storing the Info objects directly in the Hashmap. The only catch here is that 
since the only field in 'struct Info' is a const string the copy assignment 
operator is implicitly deteled. This is not a problem since we can use emplace 
when initializing a key,value in the hashmap.


- Avinash


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


On Jan. 15, 2016, 5:44 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42047/
> ---
> 
> (Updated Jan. 15, 2016, 5:44 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4262
> https://issues.apache.org/jira/browse/MESOS-4262
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to 
> isolate a mesos container using the net_cls cgroup subsystem.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 39a23df3227a4f524ea0d408dc894fa5bbab7d10 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42047/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42047: Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to isolate a mesos container using the net_cls cgroup subsystem.

[Avinash sridharan]

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

(Updated Jan. 15, 2016, 5:44 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to 
isolate a mesos container using the net_cls cgroup subsystem.


Diffs (updated)
-

  src/CMakeLists.txt 39a23df3227a4f524ea0d408dc894fa5bbab7d10 
  src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp PRE-CREATION 

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


Testing
---


Thanks,

Avinash sridharan

Re: Review Request 42182: Fixed check thereby allowing HTTP executors to reconnect.

[Anand Mazumdar]

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

(Updated Jan. 15, 2016, 5:36 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Fixed a missing condition check for `Subscribe`


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


Repository: mesos


Description
---

Previously, we used to return a `ServiceUnavailable` when slave was recovering. 
However, we only need to do so, for all other calls and let `Subscribe` pass 
through.


Diffs (updated)
-

  src/slave/http.cpp 66abdcb12d670bc4b77fa5b5f8ce35220f9aa698 
  src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 41491: Exposed docker/appc image manifest to mesos containerizer.

[Gilbert Song]

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

(Updated Jan. 14, 2016, 9:30 p.m.)


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


Summary (updated)
-

Exposed docker/appc image manifest to mesos containerizer.


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


Repository: mesos


Description (updated)
---

Exposed docker/appc image manifest to mesos containerizer.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
03425daaf015cf231920b7eda1d5424895a41286 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
4202f5552174ad3ab595ab3f16ab91d0854951f0 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
023f7363f33c4a927fae11037a408f6747fc3c39 
  src/slave/containerizer/mesos/provisioner/store.hpp 
aec725f789f7aeb92abfcc6718c2e6e2f1f37981 
  src/tests/containerizer/provisioner_docker_tests.cpp 
f81f0039dc12d09d85fda0be345d1509b4c6a664 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 42341: Redirected requests to leading master for /scheduler endpoint.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This is a followup patch addressing comments on r41052. It would be good to fix 
this before 0.27 goes out.

The current patch redirects to the current master, if a leader has been 
elected. If a leader has not been elected yet, it returns `ServiceUnavailable` 
as before.


Diffs
-

  src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d 

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


Testing
---

make check + curl. Due to MESOS-2976, spawning multiple masters is not possible.


Thanks,

Anand Mazumdar

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

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [42325, 42326, 41617]

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

Error:
 2016-01-15 05:20:35 URL:https://reviews.apache.org/r/41617/diff/raw/ 
[1972/1972] -> "41617.patch" [1]
No files to lint

Error: Commit message summary (the first line) must not exceed 72 characters.

- Mesos ReviewBot


On Jan. 15, 2016, 3:35 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41617/
> ---
> 
> (Updated Jan. 15, 2016, 3:35 a.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 support/mesos-styles.py after this change and found zero errors.
> 
> Tried a commit for the following patch:
>   diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
> index eb47b53..e579d20 100644
> --- a/src/tests/slave_tests.cpp
> +++ b/src/tests/slave_tests.cpp
> @@ -2228,7 +2228,7 @@ TEST_F(SlaveTest, DiscoveryInfoAndPorts)
> 
>AWAIT_READY(launchTask);
> 
> -  // Verify label key and value in slave state.json.
> +  //Verify label key and value in slave state.json.
>Future response =
>  process::http::get(slave.get(), "state.json");
> 
> And the git commit hook correctly failed it:
> $ git commit -m "test"
> Checking 1 files
> src/tests/slave_tests.cpp:2231:  Should have a space between // and comment  
> [whitespace/comments] [4]
> Total errors found: 1
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

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

[Klaus Ma]

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

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


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

rebase


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

  src/master/master.cpp 863a11c82d322f56db1ccf25b73a41304a46 
  src/tests/containerizer/docker_containerizer_tests.cpp 
cb58b7183c36d96b9ac4803c63980c278a50c97b 
  src/tests/master_tests.cpp 9638fb867b07f85a02d3b78f8282843d8871cabe 
  src/tests/master_validation_tests.cpp 
fbf8fadbc04a7cbc60ee6091e0224339389b400f 
  src/tests/monitor_tests.cpp 2226458b59b4a279a92e1353bd61457a0018d2a9 
  src/tests/reservation_endpoints_tests.cpp 
b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
  src/tests/slave_recovery_tests.cpp 6683a081ac231f1e275a3cdb8ee841da430a9f66 
  src/tests/slave_tests.cpp 076660daf025d6fd5065cd0c1930f17ecc5ca5aa 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41305: MESOS-1718: move getExecutorInfo from slave to master

[Klaus Ma]

> On Dec. 14, 2015, 2:31 p.m., Qian Zhang wrote:
> > src/slave/slave.cpp, line 1370
> > 
> >
> > Before trying to get task's executor via `task.executor()`, suggest to 
> > add a CHECK to ensure the task always has an executor.

It's not necessary, executor is set in master.


- Klaus


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


On Jan. 15, 2016, 1:17 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41305/
> ---
> 
> (Updated Jan. 15, 2016, 1:17 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: move getExecutorInfo from slave to master
> 
> 
> Diffs
> -
> 
>   src/master/constants.hpp ebab341e58035d4b579828add752c1ee37efeb95 
>   src/master/master.hpp f02d165874fa8023675e545793de699aeecae29b 
>   src/master/master.cpp 863a11c82d322f56db1ccf25b73a41304a46 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
>   src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 
> 
> Diff: https://reviews.apache.org/r/41305/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 41306: MESOS-1718: use command line executor to launch tasks

[Klaus Ma]

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

(Updated Jan. 15, 2016, 1:18 p.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

rebase


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


Repository: mesos


Description
---

MESOS-1718: use command line executor to launch tasks


Diffs (updated)
-

  src/master/validation.cpp c7cf56815fc743ff52ef423b23d78398ad1b35a3 
  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41305: MESOS-1718: move getExecutorInfo from slave to master

[Klaus Ma]

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

(Updated Jan. 15, 2016, 1:17 p.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

rebase


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


Repository: mesos


Description
---

MESOS-1718: move getExecutorInfo from slave to master


Diffs (updated)
-

  src/master/constants.hpp ebab341e58035d4b579828add752c1ee37efeb95 
  src/master/master.hpp f02d165874fa8023675e545793de699aeecae29b 
  src/master/master.cpp 863a11c82d322f56db1ccf25b73a41304a46 
  src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 

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


Testing
---


Thanks,

Klaus Ma

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

[Till Toenshoff]

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


Right now I am not convinced the criterea used are actually defined close 
enough to resemble the demands of the tests. Seems we might need to go back to 
the drawing board...


src/tests/environment.cpp (line 322)


This grepping for `cycles` seems dangerous assuming that we need 
`cpu-cycles` support (aka `cycles`) when considering that there are also things 
like e.g. `ref-cycles` which I believe we are not relying on. 

Test-output of a box I tried:
```
$ perf list hw
  cpu-cycles OR cycles   [Hardware event]
  instructions   [Hardware event]
  cache-references   [Hardware event]
  cache-misses   [Hardware event]
  branch-instructions OR branches[Hardware event]
  branch-misses  [Hardware event]
  bus-cycles [Hardware event]
  ref-cycles [Hardware event]
```

Test-output of a VM I tried:
```
$ perf list hw
  ref-cycles [Hardware event]
```

More on that on 
https://issues.apache.org/jira/browse/MESOS-3082?focusedCommentId=15101208&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15101208


- Till Toenshoff


On Jan. 14, 2016, 3:28 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42197/
> ---
> 
> (Updated Jan. 14, 2016, 3:28 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier 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
> ---
> 
> Running `sudo ./bin/mesos-tests.sh 
> --gtest_filter="*ROOT_CGROUPS_Perf:*ROOT_CGROUPS_Sample:*ROOT_CGROUPS_UserCgroup:*CGROUPS_ROOT_PerfRollForward:*ROOT_Sample"`
> 
> 1) In a Linux VM without 'CPU performance counters': This doesn't run the 
> tests and shows an error message that 'cycles' is not available, as expected
> 2) In a Linux VM without 'perf': This doesn't run the tests and shows an 
> error message that 'perf' is not installed, as expected
> 3) On OSX: This doesn't run the tests and shows an error message that 'perf' 
> is not available, as expected
> 4) NOT TESTED YET!!! On native Linux (i.e. with 'CPU performance counters'): 
> This runs the tests.
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 39628: Cleared the suppressed flag when deactive a framework.

[Vinod Kone]

> On Jan. 14, 2016, 10:55 p.m., Vinod Kone wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 1914
> > 
> >
> > I'm not sure why you needed to have framework2 and slave2 in this test 
> > at all?
> > 
> > can't you just do
> > 
> > // framework1 suppresses offers
> > 
> > // ensure no offers are received. i.e., 'allocaiton' is pending.
> > 
> > // framework1 is deactivated and reactivated.
> > 
> > // ensure offer for slave1 is received.
> 
> Guangya Liu wrote:
> @Vinod, does there are any helper functions which can help if there are 
> pending allocations? I have some problem in checking `ensure no offers are 
> received. i.e., 'allocaiton' is pending.`
> 
> Did not find any helper functions so just using a complex to varify the 
> test cases.

If you do Clock::settle(), any pending events should be acted upon. So, if we 
ensure `allocation` future is still pending, that should be good enough.


- Vinod


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


On Jan. 14, 2016, 9:53 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39628/
> ---
> 
> (Updated Jan. 14, 2016, 9:53 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3802
> https://issues.apache.org/jira/browse/MESOS-3802
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleared the suppressed flag when deactive a framework.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/39628/diff/
> 
> 
> Testing
> ---
> 
> Ubuntu 14.04
> make 
> make check
> GLOG_v=2 ./bin/mesos-tests.sh  
> --gtest_filter="HierarchicalAllocatorTest.DeactivateActivateFramework" 
> --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41302: MESOS-1718: add slave's configuration into SlaveInfo

[Klaus Ma]

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

(Updated Jan. 15, 2016, 1:11 p.m.)


Review request for mesos, Ben Mahler, Ian Downes, Joseph Wu, and Vinod Kone.


Changes
---

rebase


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


Repository: mesos


Description
---

MESOS-1718: add slave's configuration into SlaveInfo


Diffs (updated)
-

  include/mesos/mesos.proto b12e0f3eff44d90ec01360fc08bf9e597d7ed9dd 
  include/mesos/v1/mesos.proto fa7e82e03b11cf6619a4f16e8e0fbbf755bf210c 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 42314: Allow schemes in HDFS URI fetcher plugin to be configurable.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [42314]

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

- Mesos ReviewBot


On Jan. 15, 2016, 2:31 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42314/
> ---
> 
> (Updated Jan. 15, 2016, 2:31 a.m.)
> 
> 
> Review request for mesos, Adam B and Jie Yu.
> 
> 
> Bugs: MESOS-4375
> https://issues.apache.org/jira/browse/MESOS-4375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow schemes in HDFS URI fetcher plugin to be configurable.
> 
> 
> Diffs
> -
> 
>   src/tests/uri_fetcher_tests.cpp ca1710cc852ebfd14dd180a245da67e6c98f148f 
>   src/uri/fetchers/hadoop.hpp 066568487dd3d219f87d12cd0fc82113d3a2a28e 
>   src/uri/fetchers/hadoop.cpp 69da5b5ad3c4d085d73ffbeca7c146e421b5dbc1 
> 
> Diff: https://reviews.apache.org/r/42314/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 35711: Disallow special characters in role name.

[Adam B]

> On Jan. 14, 2016, 1:33 a.m., Adam B wrote:
> > Looks great! Just a few minor items that I can fix myself. I'll commit this 
> > shortly.
> 
> haosdent huang wrote:
> Thank you very much!

No sir, thank you!


- Adam


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


On Jan. 13, 2016, 10 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35711/
> ---
> 
> (Updated Jan. 13, 2016, 10 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, and Michael Park.
> 
> 
> Bugs: MESOS-2210
> https://issues.apache.org/jira/browse/MESOS-2210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Disallow special characters in role name.
> 
> 
> Diffs
> -
> 
>   docs/roles.md af1adad7ec8122fd10f7de44848014b850416bcd 
>   include/mesos/roles.hpp PRE-CREATION 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
>   src/common/roles.cpp PRE-CREATION 
>   src/master/master.cpp c122c30d943813fc3ce9e7025783c7231809b022 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/tests/role_tests.cpp 373ae267b85588fe491ab0a0ce8aa195f971aac3 
> 
> Diff: https://reviews.apache.org/r/35711/diff/
> 
> 
> Testing
> ---
> 
> make -j8 check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

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

[Michael Park]

> On Jan. 14, 2016, 5:08 a.m., Alexander Rojas wrote:
> > I was wondering what the results of the benchmark would be with an 
> > optimized build.

The tests are already performed with an optimized build.


- Michael


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


On Jan. 6, 2016, 2: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, 2: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

Re: Review Request 41593: Added `jsonify` function to stout.

[Michael Park]

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

(Updated Jan. 15, 2016, 4 a.m.)


Review request for mesos and Benjamin Hindman.


Changes
---

Addressed some of BenH's comments.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/Makefile.am 
5ab7bc4966fe32eaddd573a4dbfd997f98b5d481 
  3rdparty/libprocess/3rdparty/stout/README.md 
a2a3dc6120b10c8fe82088b9f0ebecfa5642f945 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
f32493f90f3bc2a62119dcea49eb5f4966388bf5 
  3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
017cef466a30e73af36d72332514a4dbb2beb6bc 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
62ad461eb228b688f1ceac16cfb003561ed5a806 
  3rdparty/libprocess/3rdparty/stout/tests/jsonify_tests.cpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
34bb2523ebf3d6d390cbfe1623b89fae0053b712 

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


Testing
---

Added tests to ``


Thanks,

Michael Park

Re: Review Request 39628: Cleared the suppressed flag when deactive a framework.

[Guangya Liu]

> On 一月 14, 2016, 10:55 p.m., Vinod Kone wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 1914
> > 
> >
> > I'm not sure why you needed to have framework2 and slave2 in this test 
> > at all?
> > 
> > can't you just do
> > 
> > // framework1 suppresses offers
> > 
> > // ensure no offers are received. i.e., 'allocaiton' is pending.
> > 
> > // framework1 is deactivated and reactivated.
> > 
> > // ensure offer for slave1 is received.

@Vinod, does there are any helper functions which can help if there are pending 
allocations? I have some problem in checking `ensure no offers are received. 
i.e., 'allocaiton' is pending.`

Did not find any helper functions so just using a complex to varify the test 
cases.


- Guangya


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


On 一月 14, 2016, 9:53 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39628/
> ---
> 
> (Updated 一月 14, 2016, 9:53 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3802
> https://issues.apache.org/jira/browse/MESOS-3802
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleared the suppressed flag when deactive a framework.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/39628/diff/
> 
> 
> Testing
> ---
> 
> Ubuntu 14.04
> make 
> make check
> GLOG_v=2 ./bin/mesos-tests.sh  
> --gtest_filter="HierarchicalAllocatorTest.DeactivateActivateFramework" 
> --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

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

[Avinash sridharan]

> On Jan. 14, 2016, 11:02 p.m., Michael Park wrote:
> > support/cpplint.py, line 2641
> > 
> >
> > Sorry for the follow-up review after a `ShipIt`, but it looks like we 
> > missed something here. The comment says: `# Allow one space for new scopes, 
> > two spaces otherwise:`. The "new scope" part seems to be in this line: 
> > `Match(r'^\s*{ //', line)`. Can we just remove this now?

Ran the support/mesos-styles.py after this change and found zero errors.


- Avinash


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


On Jan. 15, 2016, 3:34 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41617/
> ---
> 
> (Updated Jan. 15, 2016, 3:34 a.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
> ---
> 
> Tried a commit for the following patch:
>   diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
> index eb47b53..e579d20 100644
> --- a/src/tests/slave_tests.cpp
> +++ b/src/tests/slave_tests.cpp
> @@ -2228,7 +2228,7 @@ TEST_F(SlaveTest, DiscoveryInfoAndPorts)
> 
>AWAIT_READY(launchTask);
> 
> -  // Verify label key and value in slave state.json.
> +  //Verify label key and value in slave state.json.
>Future response =
>  process::http::get(slave.get(), "state.json");
> 
> And the git commit hook correctly failed it:
> $ git commit -m "test"
> Checking 1 files
> src/tests/slave_tests.cpp:2231:  Should have a space between // and comment  
> [whitespace/comments] [4]
> Total errors found: 1
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

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

[Avinash sridharan]

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

(Updated Jan. 15, 2016, 3:35 a.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 (updated)
---

Ran the support/mesos-styles.py after this change and found zero errors.

Tried a commit for the following patch:
  diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index eb47b53..e579d20 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -2228,7 +2228,7 @@ TEST_F(SlaveTest, DiscoveryInfoAndPorts)

   AWAIT_READY(launchTask);

-  // Verify label key and value in slave state.json.
+  //Verify label key and value in slave state.json.
   Future response =
 process::http::get(slave.get(), "state.json");

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


Thanks,

Avinash sridharan

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

[Avinash sridharan]

> On Jan. 14, 2016, 8:46 p.m., Michael Park wrote:
> > Please update the `Summary` to be < 72 chars to make Reviewbot happy. Could 
> > you also submit a patch for the places that violate the rule we're 
> > enforcing here and make this patch depend on it? Again, so that this way 
> > Reviewbot will be happy and our CI will continue to pass.

Re-indented to make comments fit in 72 lines.


- Avinash


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


On Jan. 15, 2016, 3:34 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41617/
> ---
> 
> (Updated Jan. 15, 2016, 3:34 a.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
> ---
> 
> Tried a commit for the following patch:
>   diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
> index eb47b53..e579d20 100644
> --- a/src/tests/slave_tests.cpp
> +++ b/src/tests/slave_tests.cpp
> @@ -2228,7 +2228,7 @@ TEST_F(SlaveTest, DiscoveryInfoAndPorts)
> 
>AWAIT_READY(launchTask);
> 
> -  // Verify label key and value in slave state.json.
> +  //Verify label key and value in slave state.json.
>Future response =
>  process::http::get(slave.get(), "state.json");
> 
> And the git commit hook correctly failed it:
> $ git commit -m "test"
> Checking 1 files
> src/tests/slave_tests.cpp:2231:  Should have a space between // and comment  
> [whitespace/comments] [4]
> Total errors found: 1
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

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

[Avinash sridharan]

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

(Updated Jan. 15, 2016, 3:34 a.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 (updated)
-

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

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


Testing
---

Tried a commit for the following patch:
  diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index eb47b53..e579d20 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -2228,7 +2228,7 @@ TEST_F(SlaveTest, DiscoveryInfoAndPorts)

   AWAIT_READY(launchTask);

-  // Verify label key and value in slave state.json.
+  //Verify label key and value in slave state.json.
   Future response =
 process::http::get(slave.get(), "state.json");

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


Thanks,

Avinash sridharan

Re: Review Request 42305: Updated comments around sorters in the allocator.

[Guangya Liu]

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



src/master/allocator/mesos/hierarchical.hpp (line 388)


Can you please add more detail for `remaining resources`? It  would be 
great if we can clarify that the `remaining resources` do not include resources 
which are reserved by Quota but not allocated.


- Guangya Liu


On 一月 14, 2016, 3:39 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42305/
> ---
> 
> (Updated 一月 14, 2016, 3:39 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4128
> https://issues.apache.org/jira/browse/MESOS-4128
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 5f08b6e7d18766d7d0f8350a280ec577b4895fb9 
> 
> Diff: https://reviews.apache.org/r/42305/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

[Michael Park]

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



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
 (line 108)


Remove newline.



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
 (line 123)


Remove newline.



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
 (lines 128 - 147)


I would suggest something like the following:

```
  const WCHAR* substituteName =
data.SymbolicLinkReparseBuffer.PathBuffer +
data.SymbolicLinkReparseBuffer.SubstituteNameOffset / sizeof(WCHAR);
  const size_t substituteNameLength =
data.SymbolicLinkReparseBuffer.SubstituteNameLength / sizeof(WCHAR);

  const WCHAR* printName =
data.SymbolicLinkReparseBuffer.PathBuffer +
data.SymbolicLinkReparseBuffer.PrintNameOffset / sizeof(WCHAR);
  const size_t printNameLength =
data.SymbolicLinkReparseBuffer.PrintNameLength / sizeof(WCHAR);

  return SymbolicLink{
  std::wstring(substituteName, substituteNameLength),
  std::wstring(printName, printNameLength),
  data.SymbolicLinkReparseBuffer.Flags};
```

The parts I care about are: (1) `s/int/size_t` for indices/lengths, (2) 
removing the `struct` in `struct SymbolicLink`, and (3) constructing 
`SymbolicLink` in-place.

I don't think the index is worth saving.

But this is probably more of a personal taste. So just take parts of it 
that you like.



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
 (lines 190 - 191)


```
return WindowsError(
"Could not open handle for symlink at path '" + absolutePath + "'");
```



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp (line 27)


`s/time_t/`time_t`/`



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp (lines 40 
- 41)


Add new line.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp (lines 51 
- 52)


Add new line.



3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp (lines 166 - 169)


Is this supposed to return `int` like the ones below? or are they supposed 
to return `bool`?


- Michael Park


On Jan. 15, 2016, 1:58 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39803/
> ---
> 
> (Updated Jan. 15, 2016, 1:58 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented stout/os/stat.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> ec851dcb08d938defeb6066810011e003d594b29 
>   
> 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
>  PRE-CREATION 
>   
> 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/symlink.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/stat.hpp 
> ffe074830ef90f492990bf695e686c885b9a155c 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp 
> 5b38b9af654d7d1c574f0cc573083b66693ced1d 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 27edf1b4f8647a2720f2962eafa110d694b3baed 
> 
> Diff: https://reviews.apache.org/r/39803/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 41285: Initialized `pid` to None() instead of `UPID()` in Slave.

[Anand Mazumdar]

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

(Updated Jan. 15, 2016, 3:15 a.m.)


Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.


Changes
---

Updated description.


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


Repository: mesos


Description (updated)
---

This was earlier pointed out by Jie on https://reviews.apache.org/r/38874.

Currently, most of our logic for finding the executor type is based on the 
fields `pid`/`http` in the `Executor` struct. We were erroneously setting the 
initial `pid` value to be `UPID()` instead of being `None()`.

The value of `pid` being `UPID()` is only possible in this scenario:

- We were unable to find the type of executor upon agent recovery i.e. no 
pid/http marker file was found. We then mark this special case as `pid=UPID()`.

This special case helps us while recovery in the following ways:

- If the agent crashed before checkpointing the pid file, both `executor->pid` 
and `executor->http` would be `None()`. This is similar to the case for HTTP 
based executors (pid/http being `None`). In order, to distinguish between these 
two cases, we set the `pid=UPID()`. This helps us in not shutting the HTTP 
executor accidently by destroying the container when the agent is recovered 
using `recovery=cleanup`
- This special value also helps us to do better logging by correctly 
distinguishing between HTTP based executors and agent dying before 
checkpointing the pid/http marker file. ( Both have `pid/http` as `None`)


Diffs
-

  src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 

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


Testing
---

make check


Thanks,

Anand Mazumdar

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [42264]

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

- Mesos ReviewBot


On Jan. 13, 2016, 9:23 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42264/
> ---
> 
> (Updated Jan. 13, 2016, 9:23 p.m.)
> 
> 
> 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 42182: Fixed check thereby allowing HTTP executors to reconnect.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Jan. 15, 2016, 2:30 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42182/
> ---
> 
> (Updated Jan. 15, 2016, 2:30 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4255
> https://issues.apache.org/jira/browse/MESOS-4255
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, we used to return a `ServiceUnavailable` when slave was 
> recovering. However, we only need to do so, for all other calls and let 
> `Subscribe` pass through.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 66abdcb12d670bc4b77fa5b5f8ce35220f9aa698 
>   src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 
> 
> Diff: https://reviews.apache.org/r/42182/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

[Michael Park]

> On Jan. 14, 2016, midnight, Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp,
> >  lines 95-96
> > 
> >
> > I don't quite understand why these are `std::wstring`. It seems like we 
> > do some `x / sizeof(WCHAR)` to make this work? Could you elaborate on this 
> > a little bit please?
> 
> Alex Clemmer wrote:
> It sounds like there might be two questions here: (1) why are we dividing 
> by `sizeof(WCHAR)` to calculate the index of the beginning/ending offsets, 
> and (2) why does the `SymbolicLink` data structure use `wchar`. Let me 
> address both of them.
> 
> First, the division by `sizeof(WCHAR)` is because the offsets in the 
> `REPARSE_DATA_BUFFER` are actually _byte_ offsets, not array index offsets. 
> Thus, to calculate the actual array index of the beginning and ending of the 
> `printName` and `substituteName`, we need to divide by `sizeof(WCHAR)`. This 
> converts the byte offset into an array index offset.
> 
> Second, the `SymbolicLink` data structure uses `wstring` because (1) the 
> goal is for `SymbolicLink` to be a convenient, internal-only way to interact 
> with the data in a `REPARSE_DATA_BUFFER`, and (2) converting to `string` is 
> really hard and not necessary for this goal.
> 
> I think part of the confusion is that it seemed like the division by 
> `sizeof(WCHAR)` might mean that these fields aren't really `wstring`, which 
> is not the case.
> 
> Does this clarify things?

Yep. As far as I understand, the crux of the funky comes from the fact that 
`REPARSE_DATA_BUFFER` stores wide chars but specifies byte offsets.


- Michael


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


On Jan. 15, 2016, 1:58 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39803/
> ---
> 
> (Updated Jan. 15, 2016, 1:58 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented stout/os/stat.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> ec851dcb08d938defeb6066810011e003d594b29 
>   
> 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
>  PRE-CREATION 
>   
> 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/symlink.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/stat.hpp 
> ffe074830ef90f492990bf695e686c885b9a155c 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp 
> 5b38b9af654d7d1c574f0cc573083b66693ced1d 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 27edf1b4f8647a2720f2962eafa110d694b3baed 
> 
> Diff: https://reviews.apache.org/r/39803/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 42331: Update slave isolator prepare function using ContainerLaunchInfo.

[Gilbert Song]

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

(Updated Jan. 14, 2016, 6:42 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Update slave isolator prepare function using ContainerLaunchInfo.


Diffs (updated)
-

  include/mesos/slave/isolator.hpp 9c46a85680840ac26fe8f3f687a7d35c7eac1782 
  include/mesos/slave/isolator.proto f4f93ca4957fa137c8132ee07929623c93ef380a 
  src/slave/containerizer/mesos/containerizer.hpp 
f5303550d592b65717246e4d75c4355db9799073 
  src/slave/containerizer/mesos/containerizer.cpp 
0639324a0bf489eed8b34bacee4f9f8d0872 
  src/slave/containerizer/mesos/isolator.hpp 
6192b04c09d7b39352d3c3734ef4621a8acc629d 
  src/slave/containerizer/mesos/isolator.cpp 
e49ecfa0034e2f22d1a501760961ab3f91841105 
  src/slave/containerizer/mesos/isolators/cgroups/cpushare.hpp 
d8310e6cf37de9ae4c10703be1d4f7155121ef2a 
  src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp 
f37a3ef84f59ec9e4d0b68d8c192314a75398fdc 
  src/slave/containerizer/mesos/isolators/cgroups/mem.hpp 
e17bb837b36682c8a754294d135282a9639cd819 
  src/slave/containerizer/mesos/isolators/cgroups/mem.cpp 
b4a53fed7d370b0c62926ba6fbc1ab3e2be81c91 
  src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp 
9e084d812f686c99d5e65ce16a0126208b526a13 
  src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
ed6697c971014372e60e496e48fe546cd522b0a8 
  src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
bdc9271ba751b7eced88ea7e11c87eae18fa5a61 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
39850aea516c12c1e2bfe233e758739ceddd3d6f 
  src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
19b0287889a416588ea7d5d5848a9dca3a369b6a 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
6d141350aa8978b11a774aba3f571904695d1856 
  src/slave/containerizer/mesos/isolators/filesystem/shared.hpp 
186b5d1306f274b6428c520b0780bf3a6788fc80 
  src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
e052da0442978a662375728863a67d3c8eb4b8ed 
  src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
6d611554de7dab47b89dea9293d470c9668c1ed6 
  src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
36b9dd84ba1af211c3b7491edc99a924830993eb 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
b4097b5562aa30a0c190f5faa295b1ef473e7763 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
efc87e07bd7d3852d54531eed723a3d207a25b01 
  src/slave/containerizer/mesos/isolators/posix.hpp 
3d628aa3cef33e4bf8d87629aae508458ed7dfa1 
  src/slave/containerizer/mesos/isolators/posix/disk.hpp 
cbb4f39db80898199df3361b5f2549a63aff3f6e 
  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
075a4ec9714a71459127eb9b63c9b6aa410aa8fa 
  src/tests/containerizer/isolator.hpp 876484688abe0fc05d484fd65aaf4d6f2d6e40aa 
  src/tests/containerizer/isolator_tests.cpp 
9f67a2af5830dfffe6302268d08514b06ee81e61 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
00ae7398b9b763c54ba38d19e1275c1dace3f0eb 
  src/tests/containerizer/port_mapping_tests.cpp 
582df8a9d2b7fe80cd6b0c15ac33bc7d45536a61 

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


Testing
---

make check
sudo ./bin/mesos-test.sh


Thanks,

Gilbert Song

Re: Review Request 41334: Added helper functions to filter allocation slack resources.

[Guangya Liu]

> On 一月 15, 2016, 2:34 a.m., Joseph Wu wrote:
> > src/common/resources.cpp, lines 715-719
> > 
> >
> > I'm going to revive this discussion because:
> > 
> > 1) This helper has essentially the opposite semantics of other helpers 
> > that have an `Option& role` argument.  And it's not obvious why.
> > 2) This is inconsistent with `Resources::isUsageSlack` added in the 
> > next review.  Why do some revocable resources get filtered by roles and 
> > other not?
> > 3) General question: Why is it so important to filter out one role?  
> > Does it greatly simplify your implementation?  Or will you end up added 
> > lots of conditionals to deal with this case?

Actually, this is because we have one open issue for this: Do we need to offer 
the allocation slack to the role who offered those allocation slack resources, 
from the previous discussion, seems we can as the current role may also want to 
run some revocable tasks, so I think that we can remove the `role` parameter 
from here, comments?

A case is as this:
agent1: 
cpus(r1):100;mem(r1):100;cpus(*){ALLOCATION_SLACK}:100;mem(*){ALLOCATION_SLACK}:100

so when the offer from  agent1 send to framework with role `r1`, it will get 
all resources including both reserved resources and allocation slack resources, 
then the framework can select and decline resources back to mesos master based 
on task requirement.


- Guangya


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


On 一月 8, 2016, 1:12 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41334/
> ---
> 
> (Updated 一月 8, 2016, 1:12 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
> https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This helper function is used to filter out allocation slack resources.
> 
> 
> Diffs
> -
> 
>   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/41334/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41333: Added helper functions to filter usage slack resources.

[Joseph Wu]

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

Ship it!


Nice and simple :)


include/mesos/resources.hpp (line 174)


You should briefly define usage slack here.

Suggestion: s/revocable/, revocable resources that are allocated but 
under-utilized/

Ditto below.


- Joseph Wu


On Jan. 7, 2016, 10:03 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41333/
> ---
> 
> (Updated Jan. 7, 2016, 10:03 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
> https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper functions to filter usage slack resources. This helper function 
> will be used to validate if the task is using usage slack resource or not so 
> as to make sure that one task or executor can only use same kind of resources.
> 
> 
> Diffs
> -
> 
>   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/41333/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41334: Added helper functions to filter allocation slack resources.

[Joseph Wu]

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



src/common/resources.cpp (lines 715 - 719)


I'm going to revive this discussion because:

1) This helper has essentially the opposite semantics of other helpers that 
have an `Option& role` argument.  And it's not obvious why.
2) This is inconsistent with `Resources::isUsageSlack` added in the next 
review.  Why do some revocable resources get filtered by roles and other not?
3) General question: Why is it so important to filter out one role?  Does 
it greatly simplify your implementation?  Or will you end up added lots of 
conditionals to deal with this case?


- Joseph Wu


On Jan. 7, 2016, 5:12 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41334/
> ---
> 
> (Updated Jan. 7, 2016, 5:12 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
> https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This helper function is used to filter out allocation slack resources.
> 
> 
> Diffs
> -
> 
>   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/41334/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42314: Allow schemes in HDFS URI fetcher plugin to be configurable.

[haosdent huang]

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

(Updated Jan. 15, 2016, 2:31 a.m.)


Review request for mesos, Adam B and Jie Yu.


Changes
---

Update test case.


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


Repository: mesos


Description
---

Allow schemes in HDFS URI fetcher plugin to be configurable.


Diffs (updated)
-

  src/tests/uri_fetcher_tests.cpp ca1710cc852ebfd14dd180a245da67e6c98f148f 
  src/uri/fetchers/hadoop.hpp 066568487dd3d219f87d12cd0fc82113d3a2a28e 
  src/uri/fetchers/hadoop.cpp 69da5b5ad3c4d085d73ffbeca7c146e421b5dbc1 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 42182: Fixed check thereby allowing HTTP executors to reconnect.

[Anand Mazumdar]

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

(Updated Jan. 15, 2016, 2:30 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comment


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


Repository: mesos


Description
---

Previously, we used to return a `ServiceUnavailable` when slave was recovering. 
However, we only need to do so, for all other calls and let `Subscribe` pass 
through.


Diffs (updated)
-

  src/slave/http.cpp 66abdcb12d670bc4b77fa5b5f8ce35220f9aa698 
  src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 42314: Allow schemes in HDFS URI fetcher plugin to be configurable.

[haosdent huang]

> On Jan. 14, 2016, 11:35 p.m., Timothy Chen wrote:
> > src/uri/fetchers/hadoop.cpp, line 59
> > 
> >
> > This seems unncessary, even if there is duplicate I think we simply 
> > just override the entry when we register the scheme.

Yes, but because we define the interface to `virtual std::set 
schemes() = 0;` in `../../include/mesos/uri/fetcher.hpp`. I think pass set here 
is better.


- haosdent


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


On Jan. 14, 2016, 7:09 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42314/
> ---
> 
> (Updated Jan. 14, 2016, 7:09 p.m.)
> 
> 
> Review request for mesos, Adam B and Jie Yu.
> 
> 
> Bugs: MESOS-4375
> https://issues.apache.org/jira/browse/MESOS-4375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow schemes in HDFS URI fetcher plugin to be configurable.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/hadoop.hpp 066568487dd3d219f87d12cd0fc82113d3a2a28e 
>   src/uri/fetchers/hadoop.cpp 69da5b5ad3c4d085d73ffbeca7c146e421b5dbc1 
> 
> Diff: https://reviews.apache.org/r/42314/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

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

[Klaus Ma]

> On Jan. 13, 2016, 8:39 p.m., Klaus Ma wrote:
> > @Joseph/Vinod, would you help to review those patches move executor from 
> > slave to master?
> 
> Joseph Wu wrote:
> There are still several open issues open on each prior review.  Can you 
> go through and make sure those are addressed?

Honestly, I'd like you to help review the code diff about executor's resources: 
where should get command line executor's resources? cut from tasks or add 
additional resources to it.


- Klaus


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


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 41491: Unified Container: Exposed docker/appc image manifest to mesos containerizer.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [41491]

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

Error:
 2016-01-15 02:03:02 URL:https://reviews.apache.org/r/41491/diff/raw/ 
[7645/7645] -> "41491.patch" [1]
Total errors found: 0
Checking 5 files
Error: Commit message summary (the first line) must not exceed 72 characters.

- Mesos ReviewBot


On Jan. 15, 2016, 1:32 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41491/
> ---
> 
> (Updated Jan. 15, 2016, 1:32 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4225
> https://issues.apache.org/jira/browse/MESOS-4225
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified Container: Exposed docker/appc image manifest to mesos containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 03425daaf015cf231920b7eda1d5424895a41286 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 4202f5552174ad3ab595ab3f16ab91d0854951f0 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 023f7363f33c4a927fae11037a408f6747fc3c39 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> aec725f789f7aeb92abfcc6718c2e6e2f1f37981 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> f81f0039dc12d09d85fda0be345d1509b4c6a664 
> 
> Diff: https://reviews.apache.org/r/41491/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

[Alex Clemmer]

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

(Updated Jan. 15, 2016, 1:58 a.m.)


Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
Remoortere, and Joseph Wu.


Repository: mesos


Description
---

Windows: Implemented stout/os/stat.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
ec851dcb08d938defeb6066810011e003d594b29 
  
3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
 PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/symlink.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/stat.hpp 
ffe074830ef90f492990bf695e686c885b9a155c 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp 
5b38b9af654d7d1c574f0cc573083b66693ced1d 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
27edf1b4f8647a2720f2962eafa110d694b3baed 

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


Testing
---

`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.


Thanks,

Alex Clemmer

Re: Review Request 42331: Update slave isolator prepare function using ContainerLaunchInfo.

[Jie Yu]

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

Ship it!



src/slave/containerizer/mesos/containerizer.hpp (line 218)


no yours, why this is called 'scripts'?



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


instead of 'launchInfo.get().has_rootfs()', you can do 
'launchInfo->has_rootfs()'



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


Ditto. You can do launchInfo->rootfs().



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


launchInfo->commands()



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


Ditto.



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


ditto.



src/slave/containerizer/mesos/containerizer.cpp (lines 912 - 913)


Ditto.



src/tests/containerizer/port_mapping_tests.cpp (line 320)


launchInfo->commands()



src/tests/containerizer/port_mapping_tests.cpp (line 326)


Ditto.


- Jie Yu


On Jan. 15, 2016, 1:52 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42331/
> ---
> 
> (Updated Jan. 15, 2016, 1:52 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4282
> https://issues.apache.org/jira/browse/MESOS-4282
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update slave isolator prepare function using ContainerLaunchInfo.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp 9c46a85680840ac26fe8f3f687a7d35c7eac1782 
>   include/mesos/slave/isolator.proto f4f93ca4957fa137c8132ee07929623c93ef380a 
>   src/slave/containerizer/mesos/containerizer.hpp 
> f5303550d592b65717246e4d75c4355db9799073 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 0639324a0bf489eed8b34bacee4f9f8d0872 
>   src/slave/containerizer/mesos/isolator.hpp 
> 6192b04c09d7b39352d3c3734ef4621a8acc629d 
>   src/slave/containerizer/mesos/isolator.cpp 
> e49ecfa0034e2f22d1a501760961ab3f91841105 
>   src/slave/containerizer/mesos/isolators/cgroups/cpushare.hpp 
> d8310e6cf37de9ae4c10703be1d4f7155121ef2a 
>   src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp 
> f37a3ef84f59ec9e4d0b68d8c192314a75398fdc 
>   src/slave/containerizer/mesos/isolators/cgroups/mem.hpp 
> e17bb837b36682c8a754294d135282a9639cd819 
>   src/slave/containerizer/mesos/isolators/cgroups/mem.cpp 
> b4a53fed7d370b0c62926ba6fbc1ab3e2be81c91 
>   src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp 
> 9e084d812f686c99d5e65ce16a0126208b526a13 
>   src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
> ed6697c971014372e60e496e48fe546cd522b0a8 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
> bdc9271ba751b7eced88ea7e11c87eae18fa5a61 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 39850aea516c12c1e2bfe233e758739ceddd3d6f 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
> 19b0287889a416588ea7d5d5848a9dca3a369b6a 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
> 6d141350aa8978b11a774aba3f571904695d1856 
>   src/slave/containerizer/mesos/isolators/filesystem/shared.hpp 
> 186b5d1306f274b6428c520b0780bf3a6788fc80 
>   src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
> e052da0442978a662375728863a67d3c8eb4b8ed 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
> 6d611554de7dab47b89dea9293d470c9668c1ed6 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
> 36b9dd84ba1af211c3b7491edc99a924830993eb 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> b4097b5562aa30a0c190f5faa295b1ef473e7763 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> efc87e07bd7d3852d54531eed723a3d207a25b01 
>   src/slave/containerizer/mesos/isolators/posix.hpp 
> 3d628aa3cef33e4bf8d87629aae508458ed7dfa1 
>   src/slave/containerizer/mesos/isolators/posix/disk.hpp 
> cbb4f39db80898199df3361b5f2549a63aff3f6e 
>   src/slave/containerizer/mesos/isolators/posix/disk.cpp 
> 075a4ec9714a71459127eb9b63c9b6aa410aa8fa 
>   src/tests/containerizer/isolator.hpp 
> 876484688abe0fc05d484fd65aaf4d6f2d6e40aa 
>   src/tests/containerizer/isolator_tests.cpp 
> 9f67a2af5830dfffe6302268d08514b06ee81e61 
>   src/tests/containerizer/mesos_containerizer_tests.cpp

Re: Review Request 42331: Update slave isolator prepare function using ContainerLaunchInfo.

[Gilbert Song]

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

(Updated Jan. 14, 2016, 5:52 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Update slave isolator prepare function using ContainerLaunchInfo.


Diffs (updated)
-

  include/mesos/slave/isolator.hpp 9c46a85680840ac26fe8f3f687a7d35c7eac1782 
  include/mesos/slave/isolator.proto f4f93ca4957fa137c8132ee07929623c93ef380a 
  src/slave/containerizer/mesos/containerizer.hpp 
f5303550d592b65717246e4d75c4355db9799073 
  src/slave/containerizer/mesos/containerizer.cpp 
0639324a0bf489eed8b34bacee4f9f8d0872 
  src/slave/containerizer/mesos/isolator.hpp 
6192b04c09d7b39352d3c3734ef4621a8acc629d 
  src/slave/containerizer/mesos/isolator.cpp 
e49ecfa0034e2f22d1a501760961ab3f91841105 
  src/slave/containerizer/mesos/isolators/cgroups/cpushare.hpp 
d8310e6cf37de9ae4c10703be1d4f7155121ef2a 
  src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp 
f37a3ef84f59ec9e4d0b68d8c192314a75398fdc 
  src/slave/containerizer/mesos/isolators/cgroups/mem.hpp 
e17bb837b36682c8a754294d135282a9639cd819 
  src/slave/containerizer/mesos/isolators/cgroups/mem.cpp 
b4a53fed7d370b0c62926ba6fbc1ab3e2be81c91 
  src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp 
9e084d812f686c99d5e65ce16a0126208b526a13 
  src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
ed6697c971014372e60e496e48fe546cd522b0a8 
  src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
bdc9271ba751b7eced88ea7e11c87eae18fa5a61 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
39850aea516c12c1e2bfe233e758739ceddd3d6f 
  src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
19b0287889a416588ea7d5d5848a9dca3a369b6a 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
6d141350aa8978b11a774aba3f571904695d1856 
  src/slave/containerizer/mesos/isolators/filesystem/shared.hpp 
186b5d1306f274b6428c520b0780bf3a6788fc80 
  src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
e052da0442978a662375728863a67d3c8eb4b8ed 
  src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
6d611554de7dab47b89dea9293d470c9668c1ed6 
  src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
36b9dd84ba1af211c3b7491edc99a924830993eb 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
b4097b5562aa30a0c190f5faa295b1ef473e7763 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
efc87e07bd7d3852d54531eed723a3d207a25b01 
  src/slave/containerizer/mesos/isolators/posix.hpp 
3d628aa3cef33e4bf8d87629aae508458ed7dfa1 
  src/slave/containerizer/mesos/isolators/posix/disk.hpp 
cbb4f39db80898199df3361b5f2549a63aff3f6e 
  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
075a4ec9714a71459127eb9b63c9b6aa410aa8fa 
  src/tests/containerizer/isolator.hpp 876484688abe0fc05d484fd65aaf4d6f2d6e40aa 
  src/tests/containerizer/isolator_tests.cpp 
9f67a2af5830dfffe6302268d08514b06ee81e61 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
00ae7398b9b763c54ba38d19e1275c1dace3f0eb 
  src/tests/containerizer/port_mapping_tests.cpp 
582df8a9d2b7fe80cd6b0c15ac33bc7d45536a61 

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


Testing
---

make check
sudo ./bin/mesos-test.sh


Thanks,

Gilbert Song

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

[Mesos ReviewBot]

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


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. 15, 2016, 12:49 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42212/
> ---
> 
> (Updated Jan. 15, 2016, 12:49 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 of frameworks and tasks per framework actually launched.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 9638fb867b07f85a02d3b78f8282843d8871cabe 
> 
> 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.MaxCompletedTasksPerFrameworkFlag:MasterTest.MaxCompletedFrameworksFlag"
>  make check -j 7
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 42309: Fallback to --git-dir when --git-common-dir not available.

[Till Toenshoff]

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

Ship it!


Thanks for promptly following up on this issue, Kevin!


support/post-reviews.py (line 111)


Minor nit, missing '.' - fixing this while committing.


- Till Toenshoff


On Jan. 15, 2016, 1:04 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42309/
> ---
> 
> (Updated Jan. 15, 2016, 1:04 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Bugs: MESOS-4374
> https://issues.apache.org/jira/browse/MESOS-4374
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The newly introduced way of finding the .git directory via
> --git-common-dir is only supported on newer versions of git. Fallback to
> the older --git-dir method if --git-common-dir is not supported.
> 
> 
> Diffs
> -
> 
>   bootstrap 519090d696ae09dac70318e91ade583b38f44048 
>   support/post-reviews.py ea361018ee77f55953af55038109148ed8057e44 
> 
> Diff: https://reviews.apache.org/r/42309/diff/
> 
> 
> Testing
> ---
> 
> Tested on mac el capitan and ubuntu 14.04 with both bash and dash and git 
> 2.6.3 and git 2.2.0 repsectively.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

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

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On Jan. 13, 2016, 9:23 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42264/
> ---
> 
> (Updated Jan. 13, 2016, 9:23 p.m.)
> 
> 
> 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 41252: RegistryClientTests: added explicit content-length header.

[Timothy Chen]

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


I think this is no longer relevant as we're removing registry client.

- Timothy Chen


On Dec. 11, 2015, 12:40 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41252/
> ---
> 
> (Updated Dec. 11, 2015, 12:40 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added explicit content-length header so that the client can close the
> connection.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 31e7a31a23397eedf22d1360e37bde8339a3c7b9 
> 
> Diff: https://reviews.apache.org/r/41252/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39172: Add Blue Yonder to the powered by Mesos list.

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On Dec. 20, 2015, 4:20 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39172/
> ---
> 
> (Updated Dec. 20, 2015, 4:20 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Dave Lester, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add Blue Yonder to the powered by Mesos list.
> 
> 
> Diffs
> -
> 
>   docs/powered-by-mesos.md bbcac44570506b7e2bdd06ac0ab2204b0417314e 
> 
> Diff: https://reviews.apache.org/r/39172/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 41491: Unified Container: Exposed docker/appc image manifest to mesos containerizer.

[Gilbert Song]

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

(Updated Jan. 14, 2016, 5:32 p.m.)


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


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


Repository: mesos


Description
---

Unified Container: Exposed docker/appc image manifest to mesos containerizer.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
03425daaf015cf231920b7eda1d5424895a41286 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
4202f5552174ad3ab595ab3f16ab91d0854951f0 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
023f7363f33c4a927fae11037a408f6747fc3c39 
  src/slave/containerizer/mesos/provisioner/store.hpp 
aec725f789f7aeb92abfcc6718c2e6e2f1f37981 
  src/tests/containerizer/provisioner_docker_tests.cpp 
f81f0039dc12d09d85fda0be345d1509b4c6a664 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 42182: Fixed check thereby allowing HTTP executors to reconnect.

[Vinod Kone]

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



src/slave/http.cpp (lines 267 - 272)


you can just pull this down to #290 and remove the  call type check in the 
if statement.


- Vinod Kone


On Jan. 12, 2016, 5:46 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42182/
> ---
> 
> (Updated Jan. 12, 2016, 5:46 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4255
> https://issues.apache.org/jira/browse/MESOS-4255
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, we used to return a `ServiceUnavailable` when slave was 
> recovering. However, we only need to do so, for all other calls and let 
> `Subscribe` pass through.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 66abdcb12d670bc4b77fa5b5f8ce35220f9aa698 
>   src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 
> 
> Diff: https://reviews.apache.org/r/42182/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

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

[Joseph Wu]

> On Jan. 13, 2016, 4:39 a.m., Klaus Ma wrote:
> > @Joseph/Vinod, would you help to review those patches move executor from 
> > slave to master?

There are still several open issues open on each prior review.  Can you go 
through and make sure those are addressed?


- Joseph


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


On Dec. 12, 2015, 1:58 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41308/
> ---
> 
> (Updated Dec. 12, 2015, 1:58 a.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 42309: Fallback to --git-dir when --git-common-dir not available.

[Kevin Klues]

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

(Updated Jan. 15, 2016, 1:04 a.m.)


Review request for mesos, Benjamin Bannier and Till Toenshoff.


Changes
---

Add comment about why we even need to use --git-common-dir instead of just 
--git-dir


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


Repository: mesos


Description
---

The newly introduced way of finding the .git directory via
--git-common-dir is only supported on newer versions of git. Fallback to
the older --git-dir method if --git-common-dir is not supported.


Diffs (updated)
-

  bootstrap 519090d696ae09dac70318e91ade583b38f44048 
  support/post-reviews.py ea361018ee77f55953af55038109148ed8057e44 

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


Testing
---

Tested on mac el capitan and ubuntu 14.04 with both bash and dash and git 2.6.3 
and git 2.2.0 repsectively.


Thanks,

Kevin Klues

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

[Kevin Klues]

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

(Updated Jan. 15, 2016, 12:49 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Address bmahler's 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 of frameworks and tasks per framework actually launched.


Diffs (updated)
-

  src/tests/master_tests.cpp 9638fb867b07f85a02d3b78f8282843d8871cabe 

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.MaxCompletedTasksPerFrameworkFlag:MasterTest.MaxCompletedFrameworksFlag"
 make check -j 7


Thanks,

Kevin Klues

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

[Kevin Klues]

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



src/tests/master_tests.cpp (line 4032)


One more comment from bmahler.  In general, EXPECT_* and ASSERT_* lines 
tend to be separated from the logic that precedes them in order to make them 
pop more.  Exceptions exist when it's clear that we are asserting the existence 
of something just after creating it, but for most things, we should be more 
liberal with our white space.


- Kevin Klues


On Jan. 14, 2016, 4:11 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42212/
> ---
> 
> (Updated Jan. 14, 2016, 4:11 p.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 of 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.MaxCompletedTasksPerFrameworkFlag:MasterTest.MaxCompletedFrameworksFlag"
>  make check -j 7
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 42274: Added common command utils file.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [41958, 41959, 42156, 42157]

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

Error:
 2016-01-15 00:08:14 URL:https://reviews.apache.org/r/42157/diff/raw/ 
[3978/3978] -> "42157.patch" [1]
error: patch failed: src/tests/containerizer/provisioner_appc_tests.cpp:253
error: src/tests/containerizer/provisioner_appc_tests.cpp: patch does not apply

- Mesos ReviewBot


On Jan. 14, 2016, 11:06 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42274/
> ---
> 
> (Updated Jan. 14, 2016, 11:06 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This common file is a good place to add common command line utilities like 
> tar,
> digests(sha256, sha512, etc). Currently this functionality is spread in the 
> code
> base and this change would enable all those call sites to be replaced with a
> common code.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 39a23df3227a4f524ea0d408dc894fa5bbab7d10 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/common/command_utils.hpp PRE-CREATION 
>   src/common/command_utils.cpp PRE-CREATION 
>   src/tests/common/command_utils_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42274/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41892: DockerContinerizer infers hostPath for persistent volumes.

[Zhitao Li]

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



src/docker/docker.cpp (lines 410 - 420)


(Sorry I just got time to come back to this).

I don't exactly understand your suggestion about "add additional 
containerInfo.volumes() in DockerContainerizerProcess::Container::create()", 
because the latter function actually does not pass a `ContainerInfo`.

Because the volume is actually included in 
`TaskInfo::ContainerInfo::volumes`, we would need to mutate the `TaskInfo` 
before passing it to the `DockerExecutorProcess::launchTask()` function, which 
I don't really know how to do it, and I even doubt whether it's a good idea for 
logging/clarity purpose.

Please let me know if I didn't understand your suggestion, or if you think 
we should explore the other alternative (passing `hostPath` earlier in resource 
offer).


- Zhitao Li


On Jan. 4, 2016, 9:15 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41892/
> ---
> 
> (Updated Jan. 4, 2016, 9:15 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Bugs: MESOS-3413
> https://issues.apache.org/jira/browse/MESOS-3413
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This diff makes DockerContainerizer infers hostPath for persistent volumes 
> from resources passed-in.
> 
> This allows current DockerContainerizer to use persistent volumes w/o hard 
> code slave's work_dir. I also checked that the inferred directory actually 
> exists to avoid framework messing up role or persistent id.
> 
> Note that some validation on the resource part should be done before we start 
> the container, which seems to belong to slave.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp dde2b29deda7e40929e0169935a5bafdd43136b1 
>   src/docker/docker.cpp e93280735f1c5f66c765fcbabfd3e50b46c024e8 
>   src/docker/executor.hpp abbc419533ab40312e917931a2fc2ce78b38da41 
>   src/docker/executor.cpp 7512d07de6c8324340e6b5f3e5162ef00efc47fc 
>   src/slave/containerizer/docker.cpp aacf90f2cb6c08f94340936d29b2df513ac9825a 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> cb58b7183c36d96b9ac4803c63980c278a50c97b 
>   src/tests/containerizer/docker_tests.cpp 
> 83eceacaddc38d0ccfc42e65e700a09406d8df36 
>   src/tests/mesos.hpp a4811b0d8dee33ff2ca4968f532ce64b7ea95249 
>   src/tests/mesos.cpp f4b0f82449c4b6a2b4b7b7f14518714485d5a13a 
> 
> Diff: https://reviews.apache.org/r/41892/diff/
> 
> 
> Testing
> ---
> 
> New unit test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

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

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [42325, 42326, 41617]

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

Error:
 2016-01-14 23:46:44 URL:https://reviews.apache.org/r/41617/diff/raw/ 
[1943/1943] -> "41617.patch" [1]
No files to lint

Error: Commit message summary (the first line) must not exceed 72 characters.

- Mesos ReviewBot


On Jan. 14, 2016, 10:51 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41617/
> ---
> 
> (Updated Jan. 14, 2016, 10:51 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
> ---
> 
> Tried a commit for the following patch:
>   diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
> index eb47b53..e579d20 100644
> --- a/src/tests/slave_tests.cpp
> +++ b/src/tests/slave_tests.cpp
> @@ -2228,7 +2228,7 @@ TEST_F(SlaveTest, DiscoveryInfoAndPorts)
> 
>AWAIT_READY(launchTask);
> 
> -  // Verify label key and value in slave state.json.
> +  //Verify label key and value in slave state.json.
>Future response =
>  process::http::get(slave.get(), "state.json");
> 
> And the git commit hook correctly failed it:
> $ git commit -m "test"
> Checking 1 files
> src/tests/slave_tests.cpp:2231:  Should have a space between // and comment  
> [whitespace/comments] [4]
> Total errors found: 1
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42314: Allow schemes in HDFS URI fetcher plugin to be configurable.

[Timothy Chen]

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



src/uri/fetchers/hadoop.cpp (line 59)


This seems unncessary, even if there is duplicate I think we simply just 
override the entry when we register the scheme.


- Timothy Chen


On Jan. 14, 2016, 7:09 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42314/
> ---
> 
> (Updated Jan. 14, 2016, 7:09 p.m.)
> 
> 
> Review request for mesos, Adam B and Jie Yu.
> 
> 
> Bugs: MESOS-4375
> https://issues.apache.org/jira/browse/MESOS-4375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow schemes in HDFS URI fetcher plugin to be configurable.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/hadoop.hpp 066568487dd3d219f87d12cd0fc82113d3a2a28e 
>   src/uri/fetchers/hadoop.cpp 69da5b5ad3c4d085d73ffbeca7c146e421b5dbc1 
> 
> Diff: https://reviews.apache.org/r/42314/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 42047: Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to isolate a mesos container using the net_cls cgroup subsystem.

[Avinash sridharan]

> On Jan. 14, 2016, 11:05 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp, lines 17-33
> > 
> >
> > No need for all the headers here.

Ok, will move it to the next commit.


> On Jan. 14, 2016, 11:05 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp, lines 38-43
> > 
> >
> > I prefer to use 70 char width if possible. But up to you.

I have been using 80 till now. It shouldn't cause any stylistic diversity right 
?


- Avinash


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


On Jan. 12, 2016, 6:26 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42047/
> ---
> 
> (Updated Jan. 12, 2016, 6:26 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4262
> https://issues.apache.org/jira/browse/MESOS-4262
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to 
> isolate a mesos container using the net_cls cgroup subsystem.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 81afdc6c3e9b062efb181b2f92a9185bdd4acfb1 
>   src/Makefile.am 865926c5b46e42c5e29d3645a700c4ad20c1f11d 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42047/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42098: Added unit test-case for CgroupsNetClsIsolatorProcess.

[Avinash sridharan]

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

(Updated Jan. 14, 2016, 11:29 p.m.)


Review request for mesos, Jie Yu and Joseph Wu.


Repository: mesos


Description
---

Added unit test-case for CgroupsNetClsIsolatorProcess.


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
91178b69ccbf5b6cbf64421e5602e6d554fc34ca 

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


Testing (updated)
---

Enabled net_cls cgroup on ubuntu 14.04 and ran make check.


Thanks,

Avinash sridharan

Re: Review Request 42098: Added unit test-case for CgroupsNetClsIsolatorProcess.

[Avinash sridharan]

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

(Updated Jan. 14, 2016, 11:29 p.m.)


Review request for mesos, Jie Yu and Joseph Wu.


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


Repository: mesos


Description
---

Added unit test-case for CgroupsNetClsIsolatorProcess.


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
91178b69ccbf5b6cbf64421e5602e6d554fc34ca 

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


Testing
---

Enabled net_cls cgroup on ubuntu 14.04 and ran make check.


Thanks,

Avinash sridharan

Re: Review Request 42098: Added unit test-case for CgroupsNetClsIsolatorProcess.

[Avinash sridharan]

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

(Updated Jan. 14, 2016, 11:28 p.m.)


Review request for mesos, Jie Yu and Joseph Wu.


Repository: mesos


Description
---

Added unit test-case for CgroupsNetClsIsolatorProcess.


Diffs (updated)
-

  src/tests/containerizer/isolator_tests.cpp 
91178b69ccbf5b6cbf64421e5602e6d554fc34ca 

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


Testing
---


Thanks,

Avinash sridharan

Re: Review Request 42324: Fixed a misleading log message when accepting inverse offers.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [42324]

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

- Mesos ReviewBot


On Jan. 14, 2016, 10:41 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42324/
> ---
> 
> (Updated Jan. 14, 2016, 10:41 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4301
> https://issues.apache.org/jira/browse/MESOS-4301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Accepting an inverse offer will result in the master printing out "ACCEPT 
> call used invalid offers ...", even if all inverse offers are actually valid. 
>  We silence the logs in this case.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 863a11c82d322f56db1ccf25b73a41304a46 
> 
> Diff: https://reviews.apache.org/r/42324/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> # Check the following for blank output
> bin/mesos-tests.sh --gtest_filter="*Inverse*Offer*" --verbose 2>&1 /dev/null 
> |  grep "ACCEPT call used invalid offers"
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

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

[Kevin Klues]

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

(Updated Jan. 14, 2016, 11:25 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Update based on bmahlers whitespace issue


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


Repository: mesos


Description (updated)
---

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


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 863a11c82d322f56db1ccf25b73a41304a46 

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 42212: Added unit test for framework/task history flags.

[Kevin Klues]

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


These issues ae based on offline discussions with bmahler.


src/tests/master_tests.cpp (line 23)


Remove in favor of stringify.hpp in stout



src/tests/master_tests.cpp (line 3964)


The array is now below, not above.



src/tests/master_tests.cpp (line 3965)


The only real interesting cases here are with maxFrameworksArray = {0, 1, 
and 2}, with totalFrameworks = 2. This covers the case of maxFramworks == 0, 
something <= to the totalFrameworks launched, and something == to the total 
frameworks launched.  There is no need to go all the way up to the default, 
which may be raised in the future and cause things to slow down.



src/tests/master_tests.cpp (line 3987)


Remove these obvious comments. Only add comments where the code itself is a 
little unclear in what it's doing.



src/tests/master_tests.cpp (line 4023)


replace get() with -> as before.



src/tests/master_tests.cpp (line 4052)


Array is now below.



src/tests/master_tests.cpp (line 4059)


Also, change this to only cover 0,1,2 cases as above.



src/tests/master_tests.cpp (line 4148)


ditto as above


- Kevin Klues


On Jan. 14, 2016, 4:11 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42212/
> ---
> 
> (Updated Jan. 14, 2016, 4:11 p.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 of 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.MaxCompletedTasksPerFrameworkFlag:MasterTest.MaxCompletedFrameworksFlag"
>  make check -j 7
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

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

[Ben Mahler]

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

Ship it!



docs/configuration.md (lines 487 - 488)


Could you avoid the tab character here?


- Ben Mahler


On Jan. 13, 2016, 10:43 a.m., Kevin Klues wrote:
> 
> ---
> 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 42274: Added common command utils file.

[Jojy Varghese]

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

(Updated Jan. 14, 2016, 11:06 p.m.)


Review request for mesos and Jie Yu.


Changes
---

minor changes


Repository: mesos


Description
---

This common file is a good place to add common command line utilities like tar,
digests(sha256, sha512, etc). Currently this functionality is spread in the code
base and this change would enable all those call sites to be replaced with a
common code.


Diffs (updated)
-

  src/CMakeLists.txt 39a23df3227a4f524ea0d408dc894fa5bbab7d10 
  src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
  src/common/command_utils.hpp PRE-CREATION 
  src/common/command_utils.cpp PRE-CREATION 
  src/tests/common/command_utils_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 42047: Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to isolate a mesos container using the net_cls cgroup subsystem.

[Jie Yu]

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



src/CMakeLists.txt (line 123)


Please sort them alphebetically.



src/Makefile.am (line 802)


Ditto.



src/Makefile.am (line 819)


Ditto.


- Jie Yu


On Jan. 12, 2016, 6:26 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42047/
> ---
> 
> (Updated Jan. 12, 2016, 6:26 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4262
> https://issues.apache.org/jira/browse/MESOS-4262
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to 
> isolate a mesos container using the net_cls cgroup subsystem.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 81afdc6c3e9b062efb181b2f92a9185bdd4acfb1 
>   src/Makefile.am 865926c5b46e42c5e29d3645a700c4ad20c1f11d 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42047/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42047: Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to isolate a mesos container using the net_cls cgroup subsystem.

[Jie Yu]

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

Ship it!



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 17)


I would use `__CGROUPS_NET_CLS_ISOLATOR_HPP__` here. We should probably fix 
others as well (not in this patch).



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 20)


Ditto. not needed. Please include  though because that's used in 
Info, which is privite to this class.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 22)


The rule I used for includes is that if the parent class already have that 
symbol in its interface, we don't have to include it again in the derived class.

Since 'Future' is already part of the interface MesosIsolatorProcess, we 
don't need to include future.hpp again here.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 25 - 26)


YOu don't need them.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 28)


This is not needed.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 30)


This is not needed as it's already in the interface of the parent class.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 38 - 43)


I prefer to use 70 char width if possible. But up to you.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 84)


Let's put that below 'struct Info' right above 'hierarchy'.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 88)


Do you still need `_containerId` here?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 96)


Can we use Owned here?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (lines 17 - 33)


No need for all the headers here.


- Jie Yu


On Jan. 12, 2016, 6:26 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42047/
> ---
> 
> (Updated Jan. 12, 2016, 6:26 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4262
> https://issues.apache.org/jira/browse/MESOS-4262
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to 
> isolate a mesos container using the net_cls cgroup subsystem.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 81afdc6c3e9b062efb181b2f92a9185bdd4acfb1 
>   src/Makefile.am 865926c5b46e42c5e29d3645a700c4ad20c1f11d 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42047/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

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

[Michael Park]

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

Ship it!



support/cpplint.py (lines 32 - 33)


I think we can highlight the __single__ rather than the 'at least', since 
'at least' is not the modified part.



support/cpplint.py (line 2641)


Sorry for the follow-up review after a `ShipIt`, but it looks like we 
missed something here. The comment says: `# Allow one space for new scopes, two 
spaces otherwise:`. The "new scope" part seems to be in this line: 
`Match(r'^\s*{ //', line)`. Can we just remove this now?


- Michael Park


On Jan. 14, 2016, 10:51 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41617/
> ---
> 
> (Updated Jan. 14, 2016, 10:51 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
> ---
> 
> Tried a commit for the following patch:
>   diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
> index eb47b53..e579d20 100644
> --- a/src/tests/slave_tests.cpp
> +++ b/src/tests/slave_tests.cpp
> @@ -2228,7 +2228,7 @@ TEST_F(SlaveTest, DiscoveryInfoAndPorts)
> 
>AWAIT_READY(launchTask);
> 
> -  // Verify label key and value in slave state.json.
> +  //Verify label key and value in slave state.json.
>Future response =
>  process::http::get(slave.get(), "state.json");
> 
> And the git commit hook correctly failed it:
> $ git commit -m "test"
> Checking 1 files
> src/tests/slave_tests.cpp:2231:  Should have a space between // and comment  
> [whitespace/comments] [4]
> Total errors found: 1
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 39628: Cleared the suppressed flag when deactive a framework.

[Vinod Kone]

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



src/master/allocator/mesos/hierarchical.cpp (line 366)


s/can get resource/can be offered resources/

s/active/getting activated/



src/tests/hierarchical_allocator_tests.cpp (line 1842)


2 blank lines between outer elements.



src/tests/hierarchical_allocator_tests.cpp (lines 1843 - 1844)


How about

// This test checks that if a framework suppresses offers, disconnects and 
reconnects again,
// it will starting receiving resource offers again.



src/tests/hierarchical_allocator_tests.cpp (line 1891)


// Total cluster resources will become cpus=4, mem=2048.



src/tests/hierarchical_allocator_tests.cpp (line 1914)


I'm not sure why you needed to have framework2 and slave2 in this test at 
all?

can't you just do

// framework1 suppresses offers

// ensure no offers are received. i.e., 'allocaiton' is pending.

// framework1 is deactivated and reactivated.

// ensure offer for slave1 is received.


- Vinod Kone


On Jan. 14, 2016, 9:53 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39628/
> ---
> 
> (Updated Jan. 14, 2016, 9:53 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3802
> https://issues.apache.org/jira/browse/MESOS-3802
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleared the suppressed flag when deactive a framework.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/39628/diff/
> 
> 
> Testing
> ---
> 
> Ubuntu 14.04
> make 
> make check
> GLOG_v=2 ./bin/mesos-tests.sh  
> --gtest_filter="HierarchicalAllocatorTest.DeactivateActivateFramework" 
> --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 42325: Fixed non-conformance of whitespace in comments, for mesos.

[Avinash sridharan]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Fixed non-conformance of whitespace in comments, for mesos.


Diffs
-

  src/authentication/cram_md5/authenticatee.hpp 
8efa6ff29a9cc8d49c435120e361e1601c0c792d 
  src/authentication/cram_md5/authenticator.hpp 
901e1519272700459d7431ff6383232eb4c99c85 
  src/authorizer/local/authorizer.hpp 586f0da19c050e75e20902c376627c8f0b4bf272 
  src/tests/containerizer/docker_containerizer_tests.cpp 
cb58b7183c36d96b9ac4803c63980c278a50c97b 

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


Testing
---

make and make check


Thanks,

Avinash sridharan

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

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On Jan. 13, 2016, 10:20 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42216/
> ---
> 
> (Updated Jan. 13, 2016, 10:20 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Neil Conway, and Timothy Chen.
> 
> 
> 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. The 
> tests that are being fixed are:
> 
> - `HookTest.VerifyMasterLaunchTaskHook`
> - `HookTest.VerifySlaveRunTaskHook`
> - `HookTest.VerifySlaveTaskStatusDecorator`
> 
> 
> 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
> 
>

Review Request 42326: Fixed non-conformance of whitespace in comments, for stout.

[Avinash sridharan]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Fixed non-conformance of whitespace in comments, for stout.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp 
27c5d5df7db54bcbb8b110512e5398f12a34a28b 
  3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp 
79cda84aa09504aa29a24af5f5350991cba7a2e0 

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


Testing
---

make and make check


Thanks,

Avinash sridharan

Re: Review Request 42098: Added unit test-case for CgroupsNetClsIsolatorProcess.

[Joseph Wu]

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



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


Nit: >>



src/tests/containerizer/isolator_tests.cpp (lines 917 - 918)


Add a `EXPECT_LE(1, pids.size())` here?
(Right now, a string like " " would pass the check.)

Maybe along with a comment like:
// These are pids for the launcher, command executor, `sh`, and the sleep.



src/tests/containerizer/isolator_tests.cpp (lines 922 - 925)


This isn't necessary.


- Joseph Wu


On Jan. 13, 2016, 6:42 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42098/
> ---
> 
> (Updated Jan. 13, 2016, 6:42 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unit test-case for CgroupsNetClsIsolatorProcess.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> 91178b69ccbf5b6cbf64421e5602e6d554fc34ca 
> 
> Diff: https://reviews.apache.org/r/42098/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

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

[Avinash sridharan]

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

(Updated Jan. 14, 2016, 10:51 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 (updated)
-

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

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


Testing (updated)
---

Tried a commit for the following patch:
  diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index eb47b53..e579d20 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -2228,7 +2228,7 @@ TEST_F(SlaveTest, DiscoveryInfoAndPorts)

   AWAIT_READY(launchTask);

-  // Verify label key and value in slave state.json.
+  //Verify label key and value in slave state.json.
   Future response =
 process::http::get(slave.get(), "state.json");

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


Thanks,

Avinash sridharan

Review Request 42324: Fixed a misleading log message when accepting inverse offers.

[Joseph Wu]

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

Review request for mesos, Ben Mahler, Artem Harutyunyan, and Joris Van 
Remoortere.


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


Repository: mesos


Description
---

Accepting an inverse offer will result in the master printing out "ACCEPT call 
used invalid offers ...", even if all inverse offers are actually valid.  We 
silence the logs in this case.


Diffs
-

  src/master/master.cpp 863a11c82d322f56db1ccf25b73a41304a46 

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


Testing
---

make check

# Check the following for blank output
bin/mesos-tests.sh --gtest_filter="*Inverse*Offer*" --verbose 2>&1 /dev/null |  
grep "ACCEPT call used invalid offers"


Thanks,

Joseph Wu

Re: Review Request 41283: Introduced an Executor Library based on the new executor HTTP API.

[Vinod Kone]

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



src/executor/executor.cpp (lines 20 - 21)


reorder alphabetically.



src/executor/executor.cpp (line 123)


The name of this struct is weird considering it is encapsulating two 
connections.

s/HttpConnection/connections/ ?

Also, do we really need a struct?



src/executor/executor.cpp (line 131)


s/'Other'/other/

also `other` seems to be a weird name for the variable. how about calling 
it `nonSubscribe`?



src/executor/executor.cpp (lines 179 - 183)


is mesos_slave_pid the only way an executor is going to know about the http 
endpoint to connect? i think we need to have a better env variable because 
'pid' is a relic of the old world, which doesn't make sense in the http world.

wasn't there an MESOS_AGENT_ENDPOINT?



src/executor/executor.cpp (line 205)


s/Cannot/Failed to/



src/executor/executor.cpp (line 220)


s/Cannot/Failed to/



src/executor/executor.cpp (lines 282 - 287)


This definitely needs a comment here. IIUC, you are opening up two 
connections here one for subscribe and another for everything else.

More importantly, why are you opening both connections here? I would've 
expected to only open the subscribe connection if it's a subscribe calla and 
open a non-subscribe one if/when it's a nonsubscribe call.



src/executor/executor.cpp (lines 309 - 313)


I dont think the executor need to be informed if this connection failed. In 
the design doc we said that subscription connection is the only one the server 
(master/agent) keeps track of. For example thats the only connection master 
sends HEARTBEATS on.



src/executor/executor.cpp (line 334)


why do we wait for `recoveryTimeout` if the agent is disconnected and we 
are not checkpointing?



src/executor/executor.cpp (line 345)


This seems like a comment for the else block.

How about

// Backoff and reconnect if checkpointing is enabled.



src/executor/executor.cpp (line 382)


s/been/seen/ ?



src/executor/executor.cpp (line 415)


s/causing/caused/


- Vinod Kone


On Jan. 12, 2016, 9:21 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41283/
> ---
> 
> (Updated Jan. 12, 2016, 9:21 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3550
> https://issues.apache.org/jira/browse/MESOS-3550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces the implementation for the executor library. 
> 
> This uses the new HTTP Connection interface to ensure calls are properly 
> pipelined.
> 
> `connected` -> Callback invoked when a TCP connection is established with the 
> agent.
> `disconnected` -> When the TCP connection is interrupted possibly due to an 
> agent restart.
> `received` -> When the executor receives events from the agent upon 
> subscribing.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/executor/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41283/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 42320: Replaced deprecated Docker flag "-c" with "--cpu-shares"

[Ben Mahler]

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

Ship it!


Ship It!

- Ben Mahler


On Jan. 14, 2016, 10:06 p.m., Felix Abecassis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42320/
> ---
> 
> (Updated Jan. 14, 2016, 10:06 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced "-m" with "--memory" for consistency ("-m" is not deprecated).
> 
> The short option "-c" is targeted for removal in version 1.11 of Docker:
> https://github.com/docker/docker/blob/851fe00c64ff/docs/misc/deprecated.md#command-line-short-variant-options
> 
> The long options are available since Docker 0.8.0:
> https://github.com/docker/docker/commit/e71dbf4ee5add5736f595948fc20bd01af56a744
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp e93280735f1c5f66c765fcbabfd3e50b46c024e8 
> 
> Diff: https://reviews.apache.org/r/42320/diff/
> 
> 
> Testing
> ---
> 
> `make check` on my local desktop, the Docker tests did run:
> [--] 7 tests from DockerTest
> [ RUN  ] DockerTest.ROOT_DOCKER_interface
> [   OK ] DockerTest.ROOT_DOCKER_interface (8897 ms)
> 
> Also checked that if I replace "-c" by a bogus value like "--foobar", the 
> tests failed.
> 
> 
> Thanks,
> 
> Felix Abecassis
> 
>

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

[Alex Clemmer]

> On Jan. 13, 2016, 6:01 a.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp, lines 37-40
> > 
> >
> > We typically don't use typedefs in Mesos. Can we just use 
> > `std::shared_ptr`?
> 
> Alex Clemmer wrote:
> Per our Slack conversation: (1) I'm all for judicious use of `typedef`, 
> and in this case I would really prefer to keep `void *` out of the codebase 
> where possible. In Windows we expect to make prodigious use of 
> `shared_handle`, and it is (IMHO) dramatically clearer than 
> `shared_ptr`, so I'd humbly like to suggest we take an exception here. 
> And (2) regardless, we typedef all over the place in `windows.hpp` anyway.
> 
> I'm really hoping peopl will agree that this is ok after all. :)

mpark and I have gone back and forth over this a few times today. After some 
consideration, there are a few options on the table: (1) write a SharedHandle 
class, (2) inherit from `shared_ptr`, (3) use the `typedef` above, (4) use 
`shared_ptr`. I think (2) is the best of these approaches, and if I can't 
have that, I would like (3). Let's go through each in turn.

(1) A `SharedHandle` class might look like this:

```
// An RAII `HANDLE` based on `shared_ptr`.
struct SharedHandle {
  template 
  SharedHandle(HANDLE handle, Deleter deleter) : handle_(handle, deleter) {}
  std::shared_ptr handle_;
};
```

The benefits are that it restricts the `shared_ptr` constructor to just the 
case we want (_i.e._, we always want to pass the close function in, like 
`SharedHandle h(handle, FindClose)`, and it fully covers the zoo of functions 
that close different types of `HANDLE`. The downside is that none of the 
`shared_ptr` API is taken with us -- we don't get to use stuff like the `->` 
operator.

(2) Having `SharedHandle` inherit from `shared_ptr` and pulling in the APIs we 
want using declarations, mitigates the problem of `SharedHandle` not having the 
`shared_ptr` API. This is kind of annoying, though, and it's a bit overkill. 
The `shared_ptr` API does not seem so dangerous, and this is the core use case 
of its API. IMHO this danger does not warrant maintaining this complexity. In 
fact, I'd rather have the simplicity of `shared_ptr` than this option.

(3) and (4) Having a typedef `shared_handle` is a natural extension of the 
Windows API's `HANDLE` which itself is a typedef of `void*`. Passing around a 
`shared_ptr` introduces the user to this implementation detail of the 
`HANDLE`, that is simply unnecessary. It is also by far the simplest of the 
solutions I consider "good". And, since there is precedent for this choice in 
the Windows API, and since there is precedent even here in `windows.hpp`, I 
propose we just use option (3). If I can't have (3) I prefer the simplicity of 
(4).


- Alex


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


On Jan. 14, 2016, 10:08 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39803/
> ---
> 
> (Updated Jan. 14, 2016, 10:08 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented stout/os/stat.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> ec851dcb08d938defeb6066810011e003d594b29 
>   
> 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
>  PRE-CREATION 
>   
> 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/symlink.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/stat.hpp 
> ffe074830ef90f492990bf695e686c885b9a155c 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp 
> 5b38b9af654d7d1c574f0cc573083b66693ced1d 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 27edf1b4f8647a2720f2962eafa110d694b3baed 
> 
> Diff: https://reviews.apache.org/r/39803/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>