Re: Review Request 49540: Used the argv version for command that launches the command executor.

[Gilbert Song]

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




src/slave/slave.cpp (line 3956)


Could you explain why we change the behavior here?

Old:
Only command task with rootfs has shell as false.

Now:
Command task with/without rootfs will defaultly have shell as false.



src/slave/slave.cpp (line 3958)


Hmm.. seems like we dont have `mesos-executor` as the first argv for a 
while. Just curious that is not supposed to work correctly, right?


- Gilbert Song


On July 1, 2016, 4:24 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49540/
> ---
> 
> (Updated July 1, 2016, 4:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Ian Downes, and Vinod Kone.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used the argv version for command that launches the command executor.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp da643e6e50b2f313705d2f862c961291aa5d2f22 
>   src/tests/slave_tests.cpp d800cb8c10b5730a711c8354802bdff6aaca8c01 
> 
> Diff: https://reviews.apache.org/r/49540/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49518: Initial snapshot for v1 master event stream.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49464, 49465, 49487, 49488, 49489, 49509, 49516, 49517, 49518]

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

- Mesos ReviewBot


On July 4, 2016, 4:06 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49518/
> ---
> 
> (Updated July 4, 2016, 4:06 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5498
> https://issues.apache.org/jira/browse/MESOS-5498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Initial snapshot for v1 master event stream.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
>   include/mesos/v1/master/master.proto 
> b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 
> 
> Diff: https://reviews.apache.org/r/49518/diff/
> 
> 
> Testing
> ---
> 
> Updated test and make check.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49518: Initial snapshot for v1 master event stream.

[Zhitao Li]

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

(Updated July 4, 2016, 4:06 a.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Changes
---

Ensure snapshot is sent before subscribe.


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


Repository: mesos


Description
---

Initial snapshot for v1 master event stream.


Diffs (updated)
-

  include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
  include/mesos/v1/master/master.proto b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
  src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
  src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 

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


Testing
---

Updated test and make check.


Thanks,

Zhitao Li

Re: Review Request 49517: Implement GetState V1 master API.

[Zhitao Li]

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

(Updated July 4, 2016, 4:05 a.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Changes
---

Adjust EXPECT_CALL location.


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


Repository: mesos


Description
---

The help function will also be used for snapshot of
event stream.


Diffs (updated)
-

  src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
  src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 
  src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 

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


Testing
---

make check


Thanks,

Zhitao Li

Re: Review Request 49518: Initial snapshot for v1 master event stream.

[Zhitao Li]

> On July 3, 2016, 2:38 a.m., haosdent huang wrote:
> > I think we should split the test cases for `SUBSCRIBE` instead of put all 
> > of them in `MasterAPITest.Subscribe`
> 
> Zhitao Li wrote:
> What do you mean here? `MasterAPITest.Subscribe` is testing `SUBSCRIBE` 
> to event stream, ans `Snapshot` is not yet filterable, so the client would 
> need to get it anyway. Can you elaborate on how you think we should separate 
> the test case?
> 
> haosdent huang wrote:
> Oh, @zhitao. I mean whether we could have a test case 
> `MasterAPITest.SubscribeAndSnapshot` only test `SNAPSHOT` event. Now in 
> `MasterAPITest.Subscribe`, we test `TASK_ADDED`, `TASK_UPDATED` and 
> `SNAPSHOT` events.

@haosdent, got it. I think that makes sense if/when we introduce some filter to 
skip snapshot event (this has been discussed but left out in the first 
version), but right now we need to capture the `Snapshot` event from client 
anyway, so `MasterAPITest.SubscribeAndSnapshot` would be just be a subset of 
`MasterAPITest.Subscribe` code, which I don't see really worth doing.

Please let me know if your idea here is different.


- Zhitao


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


On July 3, 2016, 7:35 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49518/
> ---
> 
> (Updated July 3, 2016, 7:35 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5498
> https://issues.apache.org/jira/browse/MESOS-5498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Initial snapshot for v1 master event stream.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
>   include/mesos/v1/master/master.proto 
> b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 
> 
> Diff: https://reviews.apache.org/r/49518/diff/
> 
> 
> Testing
> ---
> 
> Updated test and make check.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49518: Initial snapshot for v1 master event stream.

[Zhitao Li]

> On July 3, 2016, 11:35 p.m., Vinod Kone wrote:
> > src/master/http.cpp, line 596
> > 
> >
> > why `mutable` here?

Because `Connection::send` function is not `const`.


> On July 3, 2016, 11:35 p.m., Vinod Kone wrote:
> > src/master/http.cpp, line 598
> > 
> >
> > what's the guarantee that this is executed before any other streaming 
> > event is sent?

Good catch. I think we should move the line of `master->subscribe(http);` back 
after the send() call, so that if an event generating action like `taskUpdate` 
is queued before the `_getState()` call, we still guarantee that.

Also, the callback lambda must be executed _synchronously_, otherwise an event 
queued in the middle of `_getState()` execution and lamdba execution would not 
be captured in snapshot, or sent by stream. I'd like to leave a Note comment 
for this. Please let me know if my understanding is incorrect or you think it's 
not worth it.


- Zhitao


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


On July 3, 2016, 7:35 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49518/
> ---
> 
> (Updated July 3, 2016, 7:35 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5498
> https://issues.apache.org/jira/browse/MESOS-5498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Initial snapshot for v1 master event stream.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
>   include/mesos/v1/master/master.proto 
> b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 
> 
> Diff: https://reviews.apache.org/r/49518/diff/
> 
> 
> Testing
> ---
> 
> Updated test and make check.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49582: Used the `char` version of `strings::startsWith` in mesos.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49581, 49582]

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

- Mesos ReviewBot


On July 4, 2016, 12:39 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49582/
> ---
> 
> (Updated July 4, 2016, 12:39 a.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Bugs: MESOS-5692
> https://issues.apache.org/jira/browse/MESOS-5692
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/authentication/cram_md5/auxprop.cpp 
> 5912b01a80d30b5ad79d71b4a5041635ab8e1b84 
>   src/common/roles.cpp 65c71931a5645d346439c3cdff1c5c8cc3ee01a3 
>   src/slave/containerizer/fetcher.cpp 
> 15ff61ff54d72c951edbd591058ad04f8d1efb58 
> 
> Diff: https://reviews.apache.org/r/49582/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 49582: Used the `char` version of `strings::startsWith` in mesos.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On July 4, 2016, 8:39 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49582/
> ---
> 
> (Updated July 4, 2016, 8:39 a.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Bugs: MESOS-5692
> https://issues.apache.org/jira/browse/MESOS-5692
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/authentication/cram_md5/auxprop.cpp 
> 5912b01a80d30b5ad79d71b4a5041635ab8e1b84 
>   src/common/roles.cpp 65c71931a5645d346439c3cdff1c5c8cc3ee01a3 
>   src/slave/containerizer/fetcher.cpp 
> 15ff61ff54d72c951edbd591058ad04f8d1efb58 
> 
> Diff: https://reviews.apache.org/r/49582/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 49575: Fixed `delete` usage in authorization_test.cpp.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [49575, 49574, 49370, 49369]

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

Error:
2016-07-04 02:00:17 URL:https://reviews.apache.org/r/49369/diff/raw/ 
[22230/22230] -> "49369.patch" [1]
error: patch failed: src/common/http.hpp:139
error: src/common/http.hpp: patch does not apply
error: patch failed: src/common/http.cpp:713
error: src/common/http.cpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/14126/console

- Mesos ReviewBot


On July 3, 2016, 5:51 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49575/
> ---
> 
> (Updated July 3, 2016, 5:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Vinod Kone.
> 
> 
> Bugs: MESOS-5709
> https://issues.apache.org/jira/browse/MESOS-5709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Additionally this review fixes some minor
> formatting and typo issues.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp ddd1f6f8caff7b78746f7344c83b89305e94e203 
> 
> Diff: https://reviews.apache.org/r/49575/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 46825: Fully-typed all FlagsBase::add overloads.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46821, 46822, 46823, 46824, 46825]

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

- Mesos ReviewBot


On July 3, 2016, 3:22 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46825/
> ---
> 
> (Updated July 3, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Bugs: MESOS-3335
> https://issues.apache.org/jira/browse/MESOS-3335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fully-typed all FlagsBase::add overloads.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/flags.hpp 
> dd9362772d1fbd32638fc7e70126fd49d4a03c68 
>   3rdparty/stout/tests/flags_tests.cpp 
> 77f3a6af110da1ffcdf2b7ab2b66431a6b5c91d3 
>   3rdparty/stout/tests/subcommand_tests.cpp 
> 9213d6b9faec30b5be320ab37ca29c2406c964ac 
> 
> Diff: https://reviews.apache.org/r/46825/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X clang-trunk w/o optimizations)
> 
> The mesos tests suite do not execute at all with an optimizing clang-trunk 
> since copying of the `map` inside `FlagsBase` reference random memory leading 
> to `SEGFAULTs`. With these patches all of `stout-tests` and 
> `libprocess-tests` can be executed, but some remaining failures persist in 
> `mesos-tests` (their number is reduced though).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 49582: Used the `char` version of `strings::startsWith` in mesos.

[Michael Park]

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

Review request for mesos and Klaus Ma.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/authentication/cram_md5/auxprop.cpp 
5912b01a80d30b5ad79d71b4a5041635ab8e1b84 
  src/common/roles.cpp 65c71931a5645d346439c3cdff1c5c8cc3ee01a3 
  src/slave/containerizer/fetcher.cpp 15ff61ff54d72c951edbd591058ad04f8d1efb58 

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


Testing
---

`make check`


Thanks,

Michael Park

Review Request 49581: Used the `char` version of `strings::startsWith` in stout.

[Michael Park]

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

Review request for mesos and Klaus Ma.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/stout/include/stout/path.hpp 
3d06ca29ce9d4d133c3ec014aa99a51aa5289bc9 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 49580: Updated Docker::inspect to parse devices.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On July 3, 2016, 11:44 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49580/
> ---
> 
> (Updated July 3, 2016, 11:44 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Docker::inspect to parse devices.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 41443c882129c17b7020048a174d925a939922a5 
>   src/docker/docker.cpp c6ed312488425423180318b7f5dd16e9f42b2441 
>   src/tests/containerizer/docker_tests.cpp 
> 49bd7c252c342c8cb29ea916ad3f1f71638d2017 
> 
> Diff: https://reviews.apache.org/r/49580/diff/
> 
> 
> Testing
> ---
> 
> Added a test.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Review Request 49580: Updated Docker::inspect to parse devices.

[Benjamin Mahler]

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

Review request for mesos and Kevin Klues.


Repository: mesos


Description
---

Updated Docker::inspect to parse devices.


Diffs
-

  src/docker/docker.hpp 41443c882129c17b7020048a174d925a939922a5 
  src/docker/docker.cpp c6ed312488425423180318b7f5dd16e9f42b2441 
  src/tests/containerizer/docker_tests.cpp 
49bd7c252c342c8cb29ea916ad3f1f71638d2017 

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


Testing
---

Added a test.


Thanks,

Benjamin Mahler

Re: Review Request 49579: Added container path to Docker::Device.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On July 3, 2016, 11:35 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49579/
> ---
> 
> (Updated July 3, 2016, 11:35 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `docker inspect` allows both the host and container paths to be specified.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 41443c882129c17b7020048a174d925a939922a5 
>   src/docker/docker.cpp c6ed312488425423180318b7f5dd16e9f42b2441 
>   src/tests/containerizer/docker_tests.cpp 
> 49bd7c252c342c8cb29ea916ad3f1f71638d2017 
> 
> Diff: https://reviews.apache.org/r/49579/diff/
> 
> 
> Testing
> ---
> 
> make check GTEST_FILTER="DockerTest.ROOT_DOCKER_NVIDIA_GPU_DeviceAllow"
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Review Request 49579: Added container path to Docker::Device.

[Benjamin Mahler]

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

Review request for mesos and Kevin Klues.


Repository: mesos


Description
---

`docker inspect` allows both the host and container paths to be specified.


Diffs
-

  src/docker/docker.hpp 41443c882129c17b7020048a174d925a939922a5 
  src/docker/docker.cpp c6ed312488425423180318b7f5dd16e9f42b2441 
  src/tests/containerizer/docker_tests.cpp 
49bd7c252c342c8cb29ea916ad3f1f71638d2017 

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


Testing
---

make check GTEST_FILTER="DockerTest.ROOT_DOCKER_NVIDIA_GPU_DeviceAllow"


Thanks,

Benjamin Mahler

Re: Review Request 49518: Initial snapshot for v1 master event stream.

[Vinod Kone]

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




src/master/http.cpp (line 596)


why `mutable` here?



src/master/http.cpp (line 598)


what's the guarantee that this is executed before any other streaming event 
is sent?


- Vinod Kone


On July 3, 2016, 7:35 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49518/
> ---
> 
> (Updated July 3, 2016, 7:35 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5498
> https://issues.apache.org/jira/browse/MESOS-5498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Initial snapshot for v1 master event stream.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
>   include/mesos/v1/master/master.proto 
> b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 
> 
> Diff: https://reviews.apache.org/r/49518/diff/
> 
> 
> Testing
> ---
> 
> Updated test and make check.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49518: Initial snapshot for v1 master event stream.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49464, 49465, 49487, 49488, 49489, 49509, 49516, 49517, 49518]

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

- Mesos ReviewBot


On July 3, 2016, 7:35 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49518/
> ---
> 
> (Updated July 3, 2016, 7:35 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5498
> https://issues.apache.org/jira/browse/MESOS-5498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Initial snapshot for v1 master event stream.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
>   include/mesos/v1/master/master.proto 
> b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 
> 
> Diff: https://reviews.apache.org/r/49518/diff/
> 
> 
> Testing
> ---
> 
> Updated test and make check.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49517: Implement GetState V1 master API.

[Vinod Kone]

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


Fix it, then Ship it!





src/tests/api_tests.cpp (lines 560 - 561)


should be set before the driver.stop() ?


- Vinod Kone


On July 3, 2016, 7:34 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49517/
> ---
> 
> (Updated July 3, 2016, 7:34 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5489
> https://issues.apache.org/jira/browse/MESOS-5489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The help function will also be used for snapshot of
> event stream.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 
>   src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 
> 
> Diff: https://reviews.apache.org/r/49517/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49516: Refactor Master::Http::getExecutors into helper function.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On July 1, 2016, 11:45 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49516/
> ---
> 
> (Updated July 1, 2016, 11:45 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5489
> https://issues.apache.org/jira/browse/MESOS-5489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Helper function will be reused by `GetExecutors` and `GetState`.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 
> 
> Diff: https://reviews.apache.org/r/49516/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49489: Refactor master::Http::getFrameworks to helper function.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On July 1, 2016, 11:42 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49489/
> ---
> 
> (Updated July 1, 2016, 11:42 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new helper function will be reused by `GET_FRAMEWORKS` and
> `GET_STATE` calls.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 
> 
> Diff: https://reviews.apache.org/r/49489/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49509: Revised protobuf definition of GetState response.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On July 1, 2016, 11:43 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49509/
> ---
> 
> (Updated July 1, 2016, 11:43 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5489
> https://issues.apache.org/jira/browse/MESOS-5489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Revised protobuf definition of GetState response.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
>   include/mesos/v1/master/master.proto 
> b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
> 
> Diff: https://reviews.apache.org/r/49509/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49488: Refactor Master::Http::getAgents into helper function.

[Vinod Kone]

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


Ship it!




Looks like you fixed the issues. Not sure why you marked them as dropped? I 
re-opened and marked them as fixed for you.

- Vinod Kone


On July 1, 2016, 11:34 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49488/
> ---
> 
> (Updated July 1, 2016, 11:34 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new helper function will be reused by both `GET_AGENTS`
> and `GET_STATE` calls.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 
> 
> Diff: https://reviews.apache.org/r/49488/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49487: Refactor Master::Http::getTasks into helper function.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On July 1, 2016, 11:32 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49487/
> ---
> 
> (Updated July 1, 2016, 11:32 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5489
> https://issues.apache.org/jira/browse/MESOS-5489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Helper function will be also be reused for `GetState`.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 
> 
> Diff: https://reviews.apache.org/r/49487/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49377: Fixed allocator to update total resources in quota sorter.

[Michael Park]

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




src/master/allocator/mesos/hierarchical.cpp (lines 631 - 642)


Any particular reason why this moved?



src/master/allocator/mesos/hierarchical.cpp (lines 658 - 662)


I don't think I quite follow this. A framework can only be in 1 role `R`, 
and can only operate on resources to make them transition between `*` and `R`. 
I don't see what we would be missing here.


- Michael Park


On July 1, 2016, 9:47 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49377/
> ---
> 
> (Updated July 1, 2016, 9:47 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5698
> https://issues.apache.org/jira/browse/MESOS-5698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Each DRFSorter tracks the total resources in the cluster. This means
> that each sorter must be updated when the resources in the cluster have
> changed, e.g., due to the creation of a dynamic reservation or a
> persistent volume. In the previous implementation, the quota role sorter
> was not updated for non-quota roles when a reservation or persistent
> volume was created by a framework. This resulted in inconsistency
> between the total resources in the allocator and the quota role sorter.
> 
> This could cause several problems. First, removing a slave from the
> cluster would leak resources in the quota role sorter. Second, certain
> interleavings of slave removals and reserve/unreserve operations by
> frameworks and via HTTP endpoints could lead to CHECK failures.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 38381237fa6ceb3f21fd0d4b07d7c3787f0129df 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 967290d4d1100208900b4b724422c3218abc23cb 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 971a7b38d26b26c8543815e467f30cbb83ee412c 
> 
> Diff: https://reviews.apache.org/r/49377/diff/
> 
> 
> Testing
> ---
> 
> make check on OSX and recent Arch Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49376: Added assertions to DRFSorter.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On July 3, 2016, 8:36 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49376/
> ---
> 
> (Updated July 3, 2016, 8:36 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5698
> https://issues.apache.org/jira/browse/MESOS-5698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added assertions to DRFSorter.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 967290d4d1100208900b4b724422c3218abc23cb 
> 
> Diff: https://reviews.apache.org/r/49376/diff/
> 
> 
> Testing
> ---
> 
> These assertions DO NOT PASS. They are conceptually correct, however -- after 
> r/49377 they pass on `make check`.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49518: Initial snapshot for v1 master event stream.

[haosdent huang]

> On July 3, 2016, 2:38 a.m., haosdent huang wrote:
> > I think we should split the test cases for `SUBSCRIBE` instead of put all 
> > of them in `MasterAPITest.Subscribe`
> 
> Zhitao Li wrote:
> What do you mean here? `MasterAPITest.Subscribe` is testing `SUBSCRIBE` 
> to event stream, ans `Snapshot` is not yet filterable, so the client would 
> need to get it anyway. Can you elaborate on how you think we should separate 
> the test case?

Oh, @zhitao. I mean whether we could have a test case 
`MasterAPITest.SubscribeAndSnapshot` only test `SNAPSHOT` event. Now in 
`MasterAPITest.Subscribe`, we test `TASK_ADDED`, `TASK_UPDATED` and `SNAPSHOT` 
events.


- haosdent


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


On July 3, 2016, 7:35 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49518/
> ---
> 
> (Updated July 3, 2016, 7:35 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5498
> https://issues.apache.org/jira/browse/MESOS-5498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Initial snapshot for v1 master event stream.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
>   include/mesos/v1/master/master.proto 
> b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 
> 
> Diff: https://reviews.apache.org/r/49518/diff/
> 
> 
> Testing
> ---
> 
> Updated test and make check.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49571: Added a benchmark test for allocations.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45958, 45959, 48616, 45960, 45961, 45962, 45963, 45964, 
45966, 45967, 49571]

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

- Mesos ReviewBot


On July 3, 2016, 7:29 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49571/
> ---
> 
> (Updated July 3, 2016, 7:29 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5771
> https://issues.apache.org/jira/browse/MESOS-5771
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This parameterized test has the following configurations:
> (1) REGULAR: Offers from every slave have regular resources.
> (2) SHARED: Offers from every slave include a shared resource.
> (3) REGULAR: Offers from every alternate slave contain only regular
> resources; and offers from every other alternate slave contains
> a shared resource.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0498cd5e54b0e4b87a767585a77699653aa52179 
> 
> Diff: https://reviews.apache.org/r/49571/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 49575: Fixed `delete` usage in authorization_test.cpp.

[Vinod Kone]

> On July 3, 2016, 6:15 p.m., Vinod Kone wrote:
> > Ship It!

can you rebase?


- Vinod


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


On July 3, 2016, 5:51 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49575/
> ---
> 
> (Updated July 3, 2016, 5:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Vinod Kone.
> 
> 
> Bugs: MESOS-5709
> https://issues.apache.org/jira/browse/MESOS-5709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Additionally this review fixes some minor
> formatting and typo issues.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp ddd1f6f8caff7b78746f7344c83b89305e94e203 
> 
> Diff: https://reviews.apache.org/r/49575/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 49569: Added an option to the launch helper binary to unshare mount namespace.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [49569, 49568, 49549, 49548, 49542, 49541, 49540, 49523, 
49524, 49479, 49473, 49472, 49425, 49424, 49415]

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

Error:
2016-07-03 20:14:28 URL:https://reviews.apache.org/r/49541/diff/raw/ 
[1789/1789] -> "49541.patch" [1]
error: patch failed: src/launcher/executor.cpp:676
error: src/launcher/executor.cpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/14122/console

- Mesos ReviewBot


On July 3, 2016, 5:51 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49569/
> ---
> 
> (Updated July 3, 2016, 5:51 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gilbert Song, Ian Downes, and 
> Joshua Cohen.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows a custom executor to use this command to launch a command in
> a new root filesystem without worrying about creating a new mount
> namespace first. For example, the following command can be used to
> launch a command (`ls -al /`) using a root filesystem (`/tmp/alpine`).
> 
> `mesos-containerizer launch \
> --unshare_namespace_mnt \
> --rootfs=/tmp/alpine\
> --command='{"shell":true,"value":"ls -al /"}'`
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.hpp 
> c716e0396736d1f2f60ec31540f12f4f7597d081 
>   src/slave/containerizer/mesos/launch.cpp 
> 83f4d7f28c066a605aa84862eca9fde900ec96c6 
> 
> Diff: https://reviews.apache.org/r/49569/diff/
> 
> 
> Testing
> ---
> 
> Manually tested the command on CentOS7:
> ```
> [root@core-dev ~]# 
> /home/jie/workspace/dist/mesos/build/src/mesos-containerizer launch 
> --rootfs=/home/jie/alpine --unshare_namespace_mnt 
> --command='{"shell":true,"value":"ls -al /"}' --user=jie
> Changing root to /home/jie/alpine
> total 24
> drwxrwxr-x   17 1001 1002  4096 Jul  3 05:11 .
> drwxrwxr-x   17 1001 1002  4096 Jul  3 05:11 ..
> -rwxr-xr-x1 root root 0 Jul  3 05:09 .dockerenv
> drwxr-xr-x2 root root  4096 Apr  1 18:56 bin
> drwxr-xr-x4 root root   300 Jul  3 05:42 dev
> drwxr-xr-x   13 root root  4096 Jul  3 05:09 etc
> drwxr-xr-x2 root root 6 Apr  1 18:56 home
> drwxr-xr-x5 root root  4096 Apr  1 18:56 lib
> lrwxrwxrwx1 root root12 Apr  1 18:56 linuxrc -> 
> /bin/busybox
> drwxr-xr-x5 root root41 Apr  1 18:56 media
> drwxr-xr-x2 root root 6 Apr  1 18:56 mnt
> dr-xr-xr-x  685 root root 0 Jun 18 02:22 proc
> drwx--2 root root26 Jul  3 05:14 root
> drwxr-xr-x2 root root 6 Apr  1 18:56 run
> drwxr-xr-x2 root root  4096 Apr  1 18:56 sbin
> dr-xr-xr-x   13 root root 0 Jun 18 02:23 sys
> drwxrwxrwt2 root root 6 Jul  3 05:13 tmp
> drwxr-xr-x7 root root61 Apr  1 18:56 usr
> drwxr-xr-x   10 root root93 Apr  1 18:56 var
> ```
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49576: Fixed whitespace error.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On July 3, 2016, 6:54 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49576/
> ---
> 
> (Updated July 3, 2016, 6:54 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed whitespace error.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 86f0cb648ef442cc23d7e09550fc1c7882541577 
> 
> Diff: https://reviews.apache.org/r/49576/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49523: Moved MESOS_CONTAINERIZER to a separate constants file.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 1, 2016, 11:02 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49523/
> ---
> 
> (Updated July 1, 2016, 11:02 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Ian Downes, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved MESOS_CONTAINERIZER to a separate constants file.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am bdad9c2ae07585b53aac97341550f3ea0b852ae7 
>   src/slave/containerizer/mesos/constants.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 8e347735fad2301a2bcbc7d141efbf0f2b708435 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 63cf92217054fab43c843379c86e25ce7f07c7d9 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 1c50cf545392488cf1e2d51d8e03887bebef5e75 
> 
> Diff: https://reviews.apache.org/r/49523/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49479: Removed --sandbox flag from mesos-containerizer launch command.

[Gilbert Song]

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


Ship it!




Thanks for addressing the tech debt I left in Feb :)

- Gilbert Song


On June 30, 2016, 4:22 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49479/
> ---
> 
> (Updated June 30, 2016, 4:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gilbert Song, and Ian Downes.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This flag is unnecessary. The caller should calculate the current
> working directory for the command, instead of doing the calculation in
> this helper binary. This patch moved the calculation to the caller and
> simplified this helper binary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 63cf92217054fab43c843379c86e25ce7f07c7d9 
>   src/slave/containerizer/mesos/launch.hpp 
> c716e0396736d1f2f60ec31540f12f4f7597d081 
>   src/slave/containerizer/mesos/launch.cpp 
> 83f4d7f28c066a605aa84862eca9fde900ec96c6 
>   src/tests/containerizer/launch_tests.cpp 
> 3e36f2f7ab89b98de2c1a971e4ecca58c13ad642 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 1c50cf545392488cf1e2d51d8e03887bebef5e75 
> 
> Diff: https://reviews.apache.org/r/49479/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 49576: Fixed whitespace error.

[Kevin Klues]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Fixed whitespace error.


Diffs
-

  src/tests/containerizer/mesos_containerizer_tests.cpp 
86f0cb648ef442cc23d7e09550fc1c7882541577 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 49574: Refactored /role and getRoles endpoint code.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On July 3, 2016, 5:50 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49574/
> ---
> 
> (Updated July 3, 2016, 5:50 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5709
> https://issues.apache.org/jira/browse/MESOS-5709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch factors out common functionality of
> getting filtered/authorized roles which was
> previously duplicated between both methods.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 
> 
> Diff: https://reviews.apache.org/r/49574/diff/
> 
> 
> Testing
> ---
> 
> make (sudo) make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 49575: Fixed `delete` usage in authorization_test.cpp.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On July 3, 2016, 5:51 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49575/
> ---
> 
> (Updated July 3, 2016, 5:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Vinod Kone.
> 
> 
> Bugs: MESOS-5709
> https://issues.apache.org/jira/browse/MESOS-5709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Additionally this review fixes some minor
> formatting and typo issues.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp ddd1f6f8caff7b78746f7344c83b89305e94e203 
> 
> Diff: https://reviews.apache.org/r/49575/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 49575: Fixed `delete` usage in authorization_test.cpp.

[Joerg Schad]

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

(Updated July 3, 2016, 5:51 p.m.)


Review request for mesos, Alexander Rojas and Vinod Kone.


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


Repository: mesos


Description
---

Additionally this review fixes some minor
formatting and typo issues.


Diffs (updated)
-

  src/tests/authorization_tests.cpp ddd1f6f8caff7b78746f7344c83b89305e94e203 

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


Testing
---

sudo make check


Thanks,

Joerg Schad

Re: Review Request 49574: Refactored /role and getRoles endpoint code.

[Joerg Schad]

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

(Updated July 3, 2016, 5:50 p.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This patch factors out common functionality of
getting filtered/authorized roles which was
previously duplicated between both methods.


Diffs (updated)
-

  src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
  src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 

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


Testing
---

make (sudo) make check


Thanks,

Joerg Schad

Re: Review Request 49575: Fixed `delete` usage in authorization_test.cpp.

[Joerg Schad]

> On July 3, 2016, 5:34 p.m., Vinod Kone wrote:
> > src/tests/authorization_tests.cpp, line 2342
> > 
> >
> > hmm. why this change. these tests expect authorization creation to 
> > fail, so `create` will not tbe `Some`?

As we use EXPECT_SOME the test will still continue and hence .get() will fail. 
As discussed I updated the check to ASSERT_SOME


- Joerg


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


On July 3, 2016, 4:47 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49575/
> ---
> 
> (Updated July 3, 2016, 4:47 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Vinod Kone.
> 
> 
> Bugs: MESOS-5709
> https://issues.apache.org/jira/browse/MESOS-5709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Additionally this review fixes some minor
> formatting and typo issues.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp ddd1f6f8caff7b78746f7344c83b89305e94e203 
> 
> Diff: https://reviews.apache.org/r/49575/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 49575: Fixed `delete` usage in authorization_test.cpp.

[Vinod Kone]

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




src/tests/authorization_tests.cpp (line 2342)


hmm. why this change. these tests expect authorization creation to fail, so 
`create` will not tbe `Some`?


- Vinod Kone


On July 3, 2016, 4:47 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49575/
> ---
> 
> (Updated July 3, 2016, 4:47 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Vinod Kone.
> 
> 
> Bugs: MESOS-5709
> https://issues.apache.org/jira/browse/MESOS-5709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Additionally this review fixes some minor
> formatting and typo issues.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp ddd1f6f8caff7b78746f7344c83b89305e94e203 
> 
> Diff: https://reviews.apache.org/r/49575/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 49520: Fixed the flaky BusyMountPoint test.

[Jie Yu]

> On July 3, 2016, 5:32 p.m., Jie Yu wrote:
> >

Could you please run the test to make sure there's no mount leak?


- Jie


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


On July 1, 2016, 6:22 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49520/
> ---
> 
> (Updated July 1, 2016, 6:22 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5752
> https://issues.apache.org/jira/browse/MESOS-5752
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In ROOT_GarbageCollectorUndeletableFilesTest.BusyMountPoint there is
> a race between the task creating a file and the test looking for it
> which sometimes leads to assertion failure on the existence of the
> file.
> 
> 
> Diffs
> -
> 
>   src/tests/gc_tests.cpp 2e23f04ea908aadcefb21e11203b95b94ec3c60b 
> 
> Diff: https://reviews.apache.org/r/49520/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 49520: Fixed the flaky BusyMountPoint test.

[Jie Yu]

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




src/tests/gc_tests.cpp (line 919)


new line above



src/tests/gc_tests.cpp (line 932)


I prefer using `_mountPoint` than `mountPoint_`. For those guys used to 
google style, a suffix `_` indicates that it's a member field.



src/tests/gc_tests.cpp (line 955)


Ditto on using `_sandbox`



src/tests/gc_tests.cpp (line 963)


Is it possible that mountPoint is not set but test failed? Are we leaking 
mounts in that case?

I would suggest that we override `CreateSlaveFlags` and save slave's 
work_dir in the test fixture.

During teardown, you can call fs::unmountAll(workDir) to cleanup the mounts.



src/tests/gc_tests.cpp (line 968)


Please add `using process::Timeout` in the beginning of this file and use 
`Timeout` here.


- Jie Yu


On July 1, 2016, 6:22 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49520/
> ---
> 
> (Updated July 1, 2016, 6:22 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5752
> https://issues.apache.org/jira/browse/MESOS-5752
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In ROOT_GarbageCollectorUndeletableFilesTest.BusyMountPoint there is
> a race between the task creating a file and the test looking for it
> which sometimes leads to assertion failure on the existence of the
> file.
> 
> 
> Diffs
> -
> 
>   src/tests/gc_tests.cpp 2e23f04ea908aadcefb21e11203b95b94ec3c60b 
> 
> Diff: https://reviews.apache.org/r/49520/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 49574: Refactored /role and getRoles endpoint code.

[Vinod Kone]

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




src/master/http.cpp (line 2919)


s/getFilteredRoles/_roles/

we've been naming the common factored out functions as `_foo` in this api.



src/master/http.cpp (line 2994)


why do you need to pull this into a variable?

can't you just do 

```
return _roles(principal)
  .then()

```



src/master/http.cpp (lines 3038 - 3040)


ditto. just do

```
return _roles()
  .then()
```


- Vinod Kone


On July 3, 2016, 4:46 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49574/
> ---
> 
> (Updated July 3, 2016, 4:46 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5709
> https://issues.apache.org/jira/browse/MESOS-5709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch factors out common functionality of
> getting filtered/authorized roles which was
> previously duplicated between both methods.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 
> 
> Diff: https://reviews.apache.org/r/49574/diff/
> 
> 
> Testing
> ---
> 
> make (sudo) make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 49369: Introduced authorization based filtering for /roles.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On July 1, 2016, 2:39 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49369/
> ---
> 
> (Updated July 1, 2016, 2:39 p.m.)
> 
> 
> Review request for mesos, Adam B and Vinod Kone.
> 
> 
> Bugs: MESOS-5709
> https://issues.apache.org/jira/browse/MESOS-5709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously only /weights was filtered but /roles actually
> contains the same information and hence both endpoints
> should be filtered in the same way.
> As part of this work we renamed the GetWeight action to
> ViewRole (and same for the acls).
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 31d5c144f92749d685dbf75b65362bd633c54ea5 
>   include/mesos/authorizer/authorizer.proto 
> e22d3a491e0c5e7c384af8635a7f555e2194b939 
>   src/authorizer/local/authorizer.cpp 
> aadb7f6f0889514060744a7eca96cda00639eb4e 
>   src/common/http.hpp eb2d0156550b3ed90c7f72a2e1405efc45eab78a 
>   src/common/http.cpp 2ef264fc886be50d2091dbe24ab422c818b5ee0b 
>   src/master/http.cpp 528f01f2e00ddbd15da1cc4fca27b5347c9fc798 
>   src/master/weights_handler.cpp 5fc69c6a1c2663bad1774d9376b0aa9f90fa7275 
>   src/tests/authorization_tests.cpp c1e8ea64bb9008163ae8f0488306687d0f63a527 
>   src/tests/dynamic_weights_tests.cpp 
> c67ed75a050b9db5575ac2bb6100bcf01cfc04ff 
>   src/tests/master_authorization_tests.cpp 
> 0807469763b14f39b594769341d5eb9678d26946 
> 
> Diff: https://reviews.apache.org/r/49369/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 49323: Added tests that combine the two ways of creating volumes.

[Michael Park]

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




src/tests/persistent_volume_endpoints_tests.cpp (lines 1639 - 1647)


It seems like the final test here is that the offer is recinded and that 
the unreserve request has been accepted.

I'm curious as to why we don't check that the resources has been restored, 
like we do in the test below. In the test below, the last check is:

```
  AWAIT_READY(offers);

  ASSERT_EQ(1u, offers.get().size());
  offer = offers.get()[0];

  EXPECT_TRUE(Resources(offer.resources()).contains(unreserved));
```


- Michael Park


On July 1, 2016, 9:47 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49323/
> ---
> 
> (Updated July 1, 2016, 9:47 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5698
> https://issues.apache.org/jira/browse/MESOS-5698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests check that dynamic reservations and persistent volumes can
> be created via the offer API and then removed via the HTTP endpoints,
> and vice versa.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 971a7b38d26b26c8543815e467f30cbb83ee412c 
> 
> Diff: https://reviews.apache.org/r/49323/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> This test passes without any Mesos changes. i.e., it doesn't reproduce the 
> bug by itself, but having more test coverage for this scenario seems wise.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 49574: Refactored /role and getRoles endpoint code.

[Joerg Schad]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This patch factors out common functionality of
getting filtered/authorized roles which was
previously duplicated between both methods.


Diffs
-

  src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
  src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 

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


Testing
---

make (sudo) make check


Thanks,

Joerg Schad

Re: Review Request 46825: Fully-typed all FlagsBase::add overloads.

[Benjamin Bannier]

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

(Updated July 3, 2016, 5:22 p.m.)


Review request for mesos, Alexander Rukletsov and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Fully-typed all FlagsBase::add overloads.


Diffs (updated)
-

  3rdparty/stout/include/stout/flags/flags.hpp 
dd9362772d1fbd32638fc7e70126fd49d4a03c68 
  3rdparty/stout/tests/flags_tests.cpp 77f3a6af110da1ffcdf2b7ab2b66431a6b5c91d3 
  3rdparty/stout/tests/subcommand_tests.cpp 
9213d6b9faec30b5be320ab37ca29c2406c964ac 

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


Testing
---

make check (OS X clang-trunk w/o optimizations)

The mesos tests suite do not execute at all with an optimizing clang-trunk 
since copying of the `map` inside `FlagsBase` reference random memory leading 
to `SEGFAULTs`. With these patches all of `stout-tests` and `libprocess-tests` 
can be executed, but some remaining failures persist in `mesos-tests` (their 
number is reduced though).


Thanks,

Benjamin Bannier

Re: Review Request 46822: Avoided slicing of flags in subprocess.

[Benjamin Bannier]

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

(Updated July 3, 2016, 5:22 p.m.)


Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Michael 
Park, and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

While `FlagsBase` internally stores just maps of names and
flags, the functions stored in a `Flag` rely on the original type of
the `Flags` containing them (e.g., we perform dynamic casts to detect
their types).

Since `Option` stores `T` as a value (i.e., it cannot contain
reference types) any interface taking an `Option` cannot rely on
polymorphic behavior of `T`. To make use of polymorphism we should
instead store e.g., a pointer type to avoid slicing.

Here we change `Flags` arguments of `subprocess` to allow preserving
the original type so `Flag` function can work reliably. We do this by
passing a `Shared` so we do not restrict copying; not that it
would also be possible to use an `Owned`, but this would
require an audit of all sites where the arguments are used as
`Owned` should not be copied, but do not prevent that on their own.


Diffs (updated)
-

  src/slave/container_loggers/lib_logrotate.cpp 
d53847bbca83367927725f06e8f962ce7011f468 
  src/slave/containerizer/docker.cpp 915030bdbe5a5b55acc38042ee0475074a602ee0 
  src/slave/containerizer/mesos/containerizer.cpp 
a96b382f22886362a1159e1166dfe041072985ba 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
92b33111799cb4e1c8bc2051381e1254d701d95c 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
25d2a57d7a814d735db78bda8c6956199de5c390 
  src/slave/containerizer/mesos/launcher.hpp 
05320f462653c31fc2f093d6c67e2182e9c794fa 
  src/slave/containerizer/mesos/launcher.cpp 
ff675262af8947b89f8099828665e5e5d86491d8 
  src/slave/containerizer/mesos/linux_launcher.hpp 
89bb2958a41dffe4ade9c2492b9a7412f90a432d 
  src/slave/containerizer/mesos/linux_launcher.cpp 
5028854fa003615f158120e030866b7ec4402b66 
  src/tests/containerizer/launch_tests.cpp 
3e36f2f7ab89b98de2c1a971e4ecca58c13ad642 
  src/tests/containerizer/launcher.hpp c352634c4766d289706c7cc738677619d7d02ccd 
  src/tests/containerizer/port_mapping_tests.cpp 
1c50cf545392488cf1e2d51d8e03887bebef5e75 

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


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 46823: Fully qualified addresses of Flag members in add calls in stout.

[Benjamin Bannier]

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

(Updated July 3, 2016, 5:22 p.m.)


Review request for mesos, Alexander Rukletsov and Vinod Kone.


Changes
---

Rebased.


Summary (updated)
-

Fully qualified addresses of Flag members in add calls in stout.


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


Repository: mesos


Description
---

While right now we can technically `add` variables to `Flags` classes
which are not members, the in order to have correct copy semantics for
`Flags` only member variables should be used.

Here we changed all instances to a full pointer-to-member syntax in
the current code.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
3f3e21514bd5e2e388165eb64d540764097557ac 

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


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 46821: Avoided slicing of flags in subprocess in libprocess and stout.

[Benjamin Bannier]

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

(Updated July 3, 2016, 5:22 p.m.)


Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Michael 
Park, and Vinod Kone.


Changes
---

Rebased.


Summary (updated)
-

Avoided slicing of flags in subprocess in libprocess and stout.


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


Repository: mesos


Description
---

While `FlagsBase` internally stores just maps of names and
flags, the functions stored in a `Flag` rely on the original type of
the `Flags` containing them (e.g., we perform dynamic casts to detect
their types).

Since `Option` stores `T` as a value (i.e., it cannot contain
reference types) any interface taking an `Option` cannot rely on
polymorphic behavior of `T`. To make use of polymorphism we should
instead store e.g., a pointer type to avoid slicing.

Here we change `Flags` arguments of `subprocess` to allow preserving
the original type so `Flag` function can work reliably. We do this by
passing a `Shared` so we do not restrict copying; not that it
would also be possible to use an `Owned`, but this would
require an audit of all sites where the arguments are used as
`Owned` should not be copied, but do not prevent that on their own.


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess_base.hpp 
e0c6b2d930fbd2cfe011e6faf44843b83ab1db27 
  3rdparty/libprocess/src/subprocess.cpp 
44073146118b6c6e9e730b8c901852594080a3eb 
  3rdparty/libprocess/src/subprocess_windows.cpp 
d4713ec9d6cdc3caca6e62d882c9efcb754c2017 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
3f3e21514bd5e2e388165eb64d540764097557ac 

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


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 46824: Fully qualified addresses of Flag members in add calls.

[Benjamin Bannier]

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

(Updated July 3, 2016, 5:22 p.m.)


Review request for mesos, Alexander Rukletsov and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

While right now we can technically `add` variables to `Flags` classes
which are not members, the in order to have correct copy semantics for
`Flags` only member variables should be used.

Here we changed all instances to a full pointer-to-member syntax in
the current code.


Diffs (updated)
-

  src/cli/execute.cpp 18c2e3449bf5e50bea0bbd0a92efa20fc8b9032b 
  src/cli/resolve.cpp ac6be05e64c5e66703081068d992ebb0d6ca6c70 
  src/docker/executor.hpp 7b63d784d6b8685912598b77fb38cf6e70646ae3 
  src/examples/balloon_framework.cpp 5613ff0c61e2d2f84684a206debc97dcb8b2c0d3 
  src/examples/dynamic_reservation_framework.cpp 
850bb2a5b243dd5d5a8b6476570b4f943fde6185 
  src/examples/load_generator_framework.cpp 
5402e31b89b7ead1dc8fdc9065980b5b1c0d380c 
  src/examples/long_lived_framework.cpp 
7c57eb5e4a34208504475013690ae8e3cae74155 
  src/examples/no_executor_framework.cpp 
57425726aa5ca27c9579b0d8ecc0bb9eb9b26852 
  src/examples/persistent_volume_framework.cpp 
fe2837cfffb6dd251ccb9c93197f623d0c55fb36 
  src/examples/test_framework.cpp a83766a92617d50eadd92ec55113e049a7290d03 
  src/examples/test_http_framework.cpp cf6baed07c862ccade080618f33cc921fc09415d 
  src/launcher/executor.cpp 63010252ffd6b1dfbe3358a1f1f4447967b824d2 
  src/local/main.cpp 578b65efac1dd8ec201bfcc85de353ca6b867148 
  src/master/main.cpp 84f3b0723fd3df02386c8072ded3cb42272cd065 
  src/slave/container_loggers/lib_logrotate.hpp 
8c5602da3e5ff7bcf758da61723b7a0ea00a6a6e 
  src/slave/container_loggers/logrotate.hpp 
16d92322079a69d8e6bed7830623c62f345cd51c 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
92b33111799cb4e1c8bc2051381e1254d701d95c 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
25d2a57d7a814d735db78bda8c6956199de5c390 
  src/slave/containerizer/mesos/launch.cpp 
83f4d7f28c066a605aa84862eca9fde900ec96c6 
  src/slave/containerizer/mesos/mount.cpp 
dbd7853a43ee1402f2f91d933a657010efdd76b0 
  src/slave/main.cpp a7ed669a0b6861d6ce8546dfafac849044a77eec 

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


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 49442: Documented behavior when framework and master both failover.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On June 30, 2016, 1 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49442/
> ---
> 
> (Updated June 30, 2016, 1 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented behavior when framework and master both failover.
> 
> 
> Diffs
> -
> 
>   docs/high-availability-framework-guide.md 
> 0c73e1478682d2a26c369bcf88235dc962e6c834 
> 
> Diff: https://reviews.apache.org/r/49442/diff/
> 
> 
> Testing
> ---
> 
> Previewed in site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49503: Fixed usage of some GMock expectations.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On July 1, 2016, 10:12 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49503/
> ---
> 
> (Updated July 1, 2016, 10:12 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> EXPECT_CALL() should be invoked before the event that is expected to
> occur. In a few places, EXPECT_CALL() was used in a way that might
> theoretically race with the generation of the expected event.
> 
> 
> Diffs
> -
> 
>   src/tests/fault_tolerance_tests.cpp 
> 57b1503f508304412b3d247f6ff7d5e49756fc57 
>   src/tests/master_tests.cpp ddbab7a64ffac9bc706f17e8feb4b27a75ef97b0 
>   src/tests/persistent_volume_tests.cpp 
> 5125a8da44759d1235fddac26e9eb5436c3d037b 
>   src/tests/scheduler_event_call_tests.cpp 
> 2e46f2752a35d32ec805025f86954d6900060022 
>   src/tests/slave_recovery_tests.cpp 8e9254afcf798d5158ec8a2c2b4cddfe50fac3f1 
> 
> Diff: https://reviews.apache.org/r/49503/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49140: Added startsWith/endsWith to support char.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On June 26, 2016, 9:46 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49140/
> ---
> 
> (Updated June 26, 2016, 9:46 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-5692
> https://issues.apache.org/jira/browse/MESOS-5692
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added startsWith/endsWith to support char.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/strings.hpp 
> 162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 
> 
> Diff: https://reviews.apache.org/r/49140/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49246: Enhanced startsWith/endsWith's performance.

[Michael Park]

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




3rdparty/stout/include/stout/strings.hpp (lines 370 - 374)


I think it would be more efficient and readable to use `std::equal`.

```
return s.size() >= prefix.size() &&
   std::equal(prefix.begin(), prefix.end(), s.begin());
```



3rdparty/stout/include/stout/strings.hpp (lines 386 - 390)


```
return s.size() >= suffix.size() &&
   std::equal(suffix.rbegin(), suffix.rend(), s.rbegin());
```


- Michael Park


On June 27, 2016, 6:16 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49246/
> ---
> 
> (Updated June 27, 2016, 6:16 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-5715
> https://issues.apache.org/jira/browse/MESOS-5715
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhanced startsWith/endsWith's performance.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/strings.hpp 
> 162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 
> 
> Diff: https://reviews.apache.org/r/49246/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49376: Added assertions to DRFSorter.

[Neil Conway]

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

(Updated July 3, 2016, 8:36 a.m.)


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


Changes
---

Rename `removalQuantity` to `resourcesQuantity` per review.


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


Repository: mesos


Description
---

Added assertions to DRFSorter.


Diffs (updated)
-

  src/master/allocator/sorter/drf/sorter.cpp 
967290d4d1100208900b4b724422c3218abc23cb 

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


Testing
---

These assertions DO NOT PASS. They are conceptually correct, however -- after 
r/49377 they pass on `make check`.


Thanks,

Neil Conway

Re: Review Request 49377: Fixed allocator to update total resources in quota sorter.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On July 1, 2016, 9:47 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49377/
> ---
> 
> (Updated July 1, 2016, 9:47 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5698
> https://issues.apache.org/jira/browse/MESOS-5698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Each DRFSorter tracks the total resources in the cluster. This means
> that each sorter must be updated when the resources in the cluster have
> changed, e.g., due to the creation of a dynamic reservation or a
> persistent volume. In the previous implementation, the quota role sorter
> was not updated for non-quota roles when a reservation or persistent
> volume was created by a framework. This resulted in inconsistency
> between the total resources in the allocator and the quota role sorter.
> 
> This could cause several problems. First, removing a slave from the
> cluster would leak resources in the quota role sorter. Second, certain
> interleavings of slave removals and reserve/unreserve operations by
> frameworks and via HTTP endpoints could lead to CHECK failures.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 38381237fa6ceb3f21fd0d4b07d7c3787f0129df 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 967290d4d1100208900b4b724422c3218abc23cb 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 971a7b38d26b26c8543815e467f30cbb83ee412c 
> 
> Diff: https://reviews.apache.org/r/49377/diff/
> 
> 
> Testing
> ---
> 
> make check on OSX and recent Arch Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49376: Added assertions to DRFSorter.

[Alexander Rukletsov]

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


Fix it, then Ship it!





src/master/allocator/sorter/drf/sorter.cpp (line 307)


Can we call it `resourcesQuantity` for consistency? `removalQuantity` seems 
like a better name, but the same variable above is called `resourcesQuantity`.


- Alexander Rukletsov


On July 1, 2016, 9:47 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49376/
> ---
> 
> (Updated July 1, 2016, 9:47 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5698
> https://issues.apache.org/jira/browse/MESOS-5698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added assertions to DRFSorter.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 967290d4d1100208900b4b724422c3218abc23cb 
> 
> Diff: https://reviews.apache.org/r/49376/diff/
> 
> 
> Testing
> ---
> 
> These assertions DO NOT PASS. They are conceptually correct, however -- after 
> r/49377 they pass on `make check`.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49375: Simplified DRFSorter to not track per-slave total resources.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On July 1, 2016, 9:47 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49375/
> ---
> 
> (Updated July 1, 2016, 9:47 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5698
> https://issues.apache.org/jira/browse/MESOS-5698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The DRFSorter previously kept the total resources at each slave, along
> with the total quantity of resources in the cluster. The latter figure
> is what most of the clients of the sorter care about. In the few places
> where the previous coding was using the per-slave total resources, it is
> relatively easy to adjust the code to remove this dependency.
> 
> As part of this change, remove `total()` and `update(const SlaveID&
> slaveId, const Resources& resources)` from the Sorter interface. The
> former was unused; for the latter, code that used it can instead be
> rewritten to adjust the total resources in the cluster by first removing
> the previous resources at a slave and then adding the new resources.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 38381237fa6ceb3f21fd0d4b07d7c3787f0129df 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 0aa1a71da4501a3b469d07538a043b4c1d74d688 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 967290d4d1100208900b4b724422c3218abc23cb 
>   src/master/allocator/sorter/sorter.hpp 
> 5ce6bb82b0127257d97daf0cea6d1d0db405bf83 
>   src/tests/sorter_tests.cpp 6fc58829892dc0223140f1b47593a3e5853cace5 
> 
> Diff: https://reviews.apache.org/r/49375/diff/
> 
> 
> Testing
> ---
> 
> make check on OSX and recent Arch Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49323: Added tests that combine the two ways of creating volumes.

[Alexander Rukletsov]

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


Ship it!




In retrospect it seems unfortunate that we split volumes tests in two files. 
Now it's hard to say, where these two tests actually belong.

- Alexander Rukletsov


On July 1, 2016, 9:47 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49323/
> ---
> 
> (Updated July 1, 2016, 9:47 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5698
> https://issues.apache.org/jira/browse/MESOS-5698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests check that dynamic reservations and persistent volumes can
> be created via the offer API and then removed via the HTTP endpoints,
> and vice versa.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 971a7b38d26b26c8543815e467f30cbb83ee412c 
> 
> Diff: https://reviews.apache.org/r/49323/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> This test passes without any Mesos changes. i.e., it doesn't reproduce the 
> bug by itself, but having more test coverage for this scenario seems wise.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49517: Implement GetState V1 master API.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On July 3, 2016, 7:34 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49517/
> ---
> 
> (Updated July 3, 2016, 7:34 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5489
> https://issues.apache.org/jira/browse/MESOS-5489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The help function will also be used for snapshot of
> event stream.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 
>   src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 
> 
> Diff: https://reviews.apache.org/r/49517/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49518: Initial snapshot for v1 master event stream.

[Zhitao Li]

> On July 3, 2016, 2:38 a.m., haosdent huang wrote:
> > I think we should split the test cases for `SUBSCRIBE` instead of put all 
> > of them in `MasterAPITest.Subscribe`

What do you mean here? `MasterAPITest.Subscribe` is testing `SUBSCRIBE` to 
event stream, ans `Snapshot` is not yet filterable, so the client would need to 
get it anyway. Can you elaborate on how you think we should separate the test 
case?


- Zhitao


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


On July 3, 2016, 7:35 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49518/
> ---
> 
> (Updated July 3, 2016, 7:35 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5498
> https://issues.apache.org/jira/browse/MESOS-5498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Initial snapshot for v1 master event stream.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
>   include/mesos/v1/master/master.proto 
> b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 
> 
> Diff: https://reviews.apache.org/r/49518/diff/
> 
> 
> Testing
> ---
> 
> Updated test and make check.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49518: Initial snapshot for v1 master event stream.

[Zhitao Li]

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

(Updated July 3, 2016, 7:35 a.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Changes
---

Fix white line.


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


Repository: mesos


Description
---

Initial snapshot for v1 master event stream.


Diffs (updated)
-

  include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
  include/mesos/v1/master/master.proto b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
  src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
  src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 

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


Testing
---

Updated test and make check.


Thanks,

Zhitao Li

Re: Review Request 49517: Implement GetState V1 master API.

[Zhitao Li]

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

(Updated July 3, 2016, 7:34 a.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Changes
---

Fix indent.


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


Repository: mesos


Description
---

The help function will also be used for snapshot of
event stream.


Diffs (updated)
-

  src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
  src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 
  src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 

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


Testing
---

make check


Thanks,

Zhitao Li

Re: Review Request 49571: Added a benchmark test for allocations.

[Anindya Sinha]

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

(Updated July 3, 2016, 7:29 a.m.)


Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

This parameterized test has the following configurations:
(1) REGULAR: Offers from every slave have regular resources.
(2) SHARED: Offers from every slave include a shared resource.
(3) REGULAR: Offers from every alternate slave contain only regular
resources; and offers from every other alternate slave contains
a shared resource.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
0498cd5e54b0e4b87a767585a77699653aa52179 

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


Testing (updated)
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 49553: Fixed indentions of HealthCheck files in src/Makefile.am.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 2, 2016, 8:12 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49553/
> ---
> 
> (Updated July 2, 2016, 8:12 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
> Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5727
> https://issues.apache.org/jira/browse/MESOS-5727
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed indentions of HealthCheck files in src/Makefile.am.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 52d63f26e0455ef31411e964e497a509d96aaf4a 
> 
> Diff: https://reviews.apache.org/r/49553/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49554: Fixed HealthCheck typo in `launcher/executor.cpp`.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 2, 2016, 8:12 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49554/
> ---
> 
> (Updated July 2, 2016, 8:12 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
> Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5727
> https://issues.apache.org/jira/browse/MESOS-5727
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed HealthCheck typo in `launcher/executor.cpp`.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 63010252ffd6b1dfbe3358a1f1f4447967b824d2 
> 
> Diff: https://reviews.apache.org/r/49554/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 49571: Added a benchmark test for allocations.

[Anindya Sinha]

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

Review request for mesos.


Repository: mesos


Description
---

This parameterized test has the following configurations:
(1) REGULAR: Offers from every slave have regular resources.
(2) SHARED: Offers from every slave include a shared resource.
(3) REGULAR: Offers from every alternate slave contain only regular
resources; and offers from every other alternate slave contains
a shared resource.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
0498cd5e54b0e4b87a767585a77699653aa52179 

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


Testing
---


Thanks,

Anindya Sinha

Re: Review Request 45961: Support sharing of resources through reference counting of resources.

[Anindya Sinha]

> On June 14, 2016, 10:50 p.m., Jiang Yan Xu wrote:
> > src/master/allocator/sorter/drf/sorter.cpp, lines 457-467
> > 
> >
> > For this we need to
> > 
> > 1. Run existing benchmarks to see how much performance degradation is 
> > there for clusters (large number of agents or frameworks) that are not 
> > using shared resources. Ideally there should be negligible performance 
> > impact for such clusters.
> > 2. Create a benchmark that uses shared resources in a realistic way.
> > 
> > In implementing this patch we do have the options to store shared 
> > resources separately in a few places (e.g., store shared resources outside 
> > of `allocations[name].resources`) that can help with performance but the 
> > trade off is that it may increase code complexity. Benchmarks can help us 
> > address these tradeoffs.
> 
> Anindya Sinha wrote:
> Verified that changes in allocator and sorter does not affect run time 
> performance adversely for regular resources. I will add a benchmark for 
> shared resources in the next update.

Added a new benchmark test in https://reviews.apache.org/r/49571/, and verifies 
allocation times over a large number of agents and frameworks is not impacted 
adversely with this change.


- Anindya


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


On July 1, 2016, 10:28 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45961/
> ---
> 
> (Updated July 1, 2016, 10:28 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, Michael 
> Park, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> o Each shared resource is accouted via its share count. This count is
>   updated based on the resource operations (such as add and subtract)
>   in various scenarios such as task launch and terminate at multiple
>   modules such as master, allocator, sorter, etc.
> o Only allow DESTROY if there are no running or pending tasks using
>   the volume. However, if the volume is in a pending offer to one or
>   more frameworks, resind that offer before processing the DESTROY.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp f6ff92b591c15bc8e93fd85e1896349c3a7bb968 
>   src/common/resources_utils.cpp 8e881a09500b0966b577e514077460f724868a8d 
>   src/master/allocator/mesos/hierarchical.cpp 
> eca949e07abb00423a2f35a56eedc5d4287d81f3 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 0aa1a71da4501a3b469d07538a043b4c1d74d688 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 967290d4d1100208900b4b724422c3218abc23cb 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 
>   src/master/master.cpp 78a8889313179b4509a6657e6c61d9f6d3bb99c0 
>   src/master/validation.hpp 43d876b00511d4ef652c2685d4950c994b5eef31 
>   src/master/validation.cpp 50ba372d1301d16d1738a3e4f4882b51f9ce06cd 
>   src/tests/master_validation_tests.cpp 
> 9eb82a569f7c48caa96d4d54a93b199ccac74385 
>   src/tests/sorter_tests.cpp 6fc58829892dc0223140f1b47593a3e5853cace5 
>   src/v1/resources.cpp 8c3f2d1c1529915a59d47fe37bb3fc7a3267079a 
> 
> Diff: https://reviews.apache.org/r/45961/diff/
> 
> 
> Testing
> ---
> 
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>