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

[Anand Mazumdar]

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


Fix it, then Ship it!




Would fix these minor style nits while committing.


src/master/http.cpp (line 2091)


Added new line before.



src/master/http.cpp (lines 2092 - 2093)


hmm, looks like this was still not fixed.
- 2 space indent before then
- 2 space indent before ->


- Anand Mazumdar


On July 6, 2016, 1:44 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49488/
> ---
> 
> (Updated July 6, 2016, 1:44 a.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 debedd4a4061034a3b55181e93443b9d5e676c52 
>   src/master/master.hpp 996ff425fc8e9234a5e02560460ad1233dca7061 
> 
> Diff: https://reviews.apache.org/r/49488/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

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

[Anand Mazumdar]

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


Fix it, then Ship it!




Would fix these minor style nits while committing.


src/master/http.cpp (line 1348)


Newline before.



src/master/http.cpp (line 1350)


4 space indent here



src/master/http.cpp (line 1383)



s/mesos::master::Response::GetFrameworks/Future


- Anand Mazumdar


On July 6, 2016, 1:45 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49489/
> ---
> 
> (Updated July 6, 2016, 1:45 a.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 debedd4a4061034a3b55181e93443b9d5e676c52 
>   src/master/master.hpp 996ff425fc8e9234a5e02560460ad1233dca7061 
> 
> Diff: https://reviews.apache.org/r/49489/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

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

[Anand Mazumdar]

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


Fix it, then Ship it!




Would fix this before committing.


include/mesos/master/master.proto (line 272)


I added a bit more meat to the comment.

// Contains full state of the master i.e. information about the tasks, 
agents, frameworks and executors running in the cluster.


- Anand Mazumdar


On July 6, 2016, 1:46 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49509/
> ---
> 
> (Updated July 6, 2016, 1:46 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
> ---
> 
> 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 49516: Refactor Master::Http::getExecutors into helper function.

[Anand Mazumdar]

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


Fix it, then Ship it!




Would fix these minor style nits while committing.


src/master/http.cpp (line 1426)


new line before to be consistent with `getTasks()`.



src/master/http.cpp (line 1466)



s/mesos::master::Response::GetExecutors/Future


- Anand Mazumdar


On July 6, 2016, 1:47 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49516/
> ---
> 
> (Updated July 6, 2016, 1:47 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
> ---
> 
> Helper function will be reused by `GetExecutors` and `GetState`.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp debedd4a4061034a3b55181e93443b9d5e676c52 
>   src/master/master.hpp 996ff425fc8e9234a5e02560460ad1233dca7061 
> 
> Diff: https://reviews.apache.org/r/49516/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

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

[Anand Mazumdar]

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


Fix it, then Ship it!




Would be committing it shortly with the following comment fix.


src/master/http.cpp (lines 597 - 604)


I removed the `NOTE` and merged it into the `TODO`

// TODO(zhitao): There is a possible race condition here: if an action like 
`taskUpdate()` is queued between `_getState()` and this continuation, neither 
the event will be sent to the subscriber (because the connection is not in 
subscribers yet), nor the effect of the change would be captured in the 
snapshot.


- Anand Mazumdar


On July 6, 2016, 3:58 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49518/
> ---
> 
> (Updated July 6, 2016, 3:58 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 debedd4a4061034a3b55181e93443b9d5e676c52 
>   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.

[Anand Mazumdar]

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


Fix it, then Ship it!




Would fix these minor style nits while committing.


src/master/http.cpp (line 1552)


2 space indent here.


- Anand Mazumdar


On July 6, 2016, 3:56 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49517/
> ---
> 
> (Updated July 6, 2016, 3:56 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 debedd4a4061034a3b55181e93443b9d5e676c52 
>   src/master/master.hpp 996ff425fc8e9234a5e02560460ad1233dca7061 
>   src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 
> 
> Diff: https://reviews.apache.org/r/49517/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49245: Implement READ_FILE for agent operator API.

[zhou xing]

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

(Updated 七月 6, 2016, 5:23 a.m.)


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


Changes
---

udpate code according to message changes


Bugs: mesos-5515
https://issues.apache.org/jira/browse/mesos-5515


Repository: mesos


Description
---

Implemented readFile method for agent operator API.


Diffs (updated)
-

  src/slave/http.cpp 67ed67e168faab692d59e077b2e9a927756c6704 
  src/slave/slave.hpp 484ba758b4c87935aabd2f76a0e654a3c6d09167 
  src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 

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


Testing
---

make
make check


Thanks,

zhou xing

Review Request 49688: Added cmake build for mesos tests.

[Srinivas Brahmaroutu]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description
---

WIP. Added cmake build for mesos tests.


Diffs
-

  CMakeLists.txt 31601a2280fa4a07df53e4e332a7e2fb0199079c 
  src/tests/CMakeLists.txt 3c530631d22aa1cfdc2c600112059601bba7d6b7 
  src/tests/cmake/TestsConfigure.cmake PRE-CREATION 

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


Testing
---

cmake ..
cmake check


Thanks,

Srinivas Brahmaroutu

Re: Review Request 49244: Implement READ_FILE for master operator API.

[zhou xing]

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

(Updated 七月 6, 2016, 5:22 a.m.)


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


Changes
---

update code according to message change


Bugs: mesos-5515
https://issues.apache.org/jira/browse/mesos-5515


Repository: mesos


Description
---

Implemented readFile method for master operator API.


Diffs (updated)
-

  src/master/http.cpp debedd4a4061034a3b55181e93443b9d5e676c52 
  src/master/master.hpp fbacd9271be4117467140b8405f4c94ca64e7188 
  src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 

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


Testing
---

make
make check


Thanks,

zhou xing

Re: Review Request 49243: Create readFile method in FilesProcess.

[zhou xing]

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

(Updated 七月 6, 2016, 5:21 a.m.)


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


Changes
---

update code per message changes


Bugs: mesos-5515
https://issues.apache.org/jira/browse/mesos-5515


Repository: mesos


Description
---

This method helps to readFiles based on offset, length
and path, which can be used for implementing master/agent's
READ_FILE operator v1 API.


Diffs (updated)
-

  src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
  src/files/files.cpp a5a1b86e14f63e5e3834a2900270252fbe16f586 

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


Testing
---

make
make check


Thanks,

zhou xing

Re: Review Request 49242: Add ReadFile protobuf message.

[zhou xing]

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

(Updated 七月 6, 2016, 5:19 a.m.)


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


Changes
---

update message


Bugs: mesos-5515
https://issues.apache.org/jira/browse/mesos-5515


Repository: mesos


Description
---

Add ReadFile message in the Response message of master.proto
, v1/master.proto, agent.proto, v1/agent.proto.


Diffs (updated)
-

  include/mesos/agent/agent.proto 538d12f71df1943f91bafb99650625aa910affaa 
  include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
  include/mesos/v1/agent/agent.proto 48f15173fe62b9ce7f648f6b54d74ec62f797c55 
  include/mesos/v1/master/master.proto b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 

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


Testing
---

make
make check


Thanks,

zhou xing

Review Request 49679: Updated v1 operator Call::ReadFile message.

[zhou xing]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


Bugs: mesos-5515
https://issues.apache.org/jira/browse/mesos-5515


Repository: mesos


Description
---

Updated the type of offset and length of Call::ReadFile
for v1 operator API.


Diffs
-

  include/mesos/agent/agent.proto 538d12f71df1943f91bafb99650625aa910affaa 
  include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
  include/mesos/v1/agent/agent.proto 48f15173fe62b9ce7f648f6b54d74ec62f797c55 
  include/mesos/v1/master/master.proto b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 

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


Testing
---

make
make check


Thanks,

zhou xing

Re: Review Request 49674: Fixed the docker executor to handle inspect failures.

[Kevin Klues]

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


Fix it, then Ship it!





src/docker/executor.cpp (lines 211 - 219)


We obviously need a better way of cleaning things up properly on failure, 
but at least printing the error message is better than nothing.


- Kevin Klues


On July 6, 2016, 12:34 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49674/
> ---
> 
> (Updated July 6, 2016, 12:34 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Joseph Wu, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the docker executor to handle inspect failures.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp c53a008bf4256a4507dfcc75dc297f133f4c3c6a 
> 
> Diff: https://reviews.apache.org/r/49674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 49673: Fixed a bug in JSON::Object.find: array subscript is dropped.

[Kevin Klues]

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




3rdparty/stout/tests/json_tests.cpp (line 340)


Maybe we should add a test here to explicitly test for the case wher the 
nested lookup is on a `null` value and it has a subscript, e.g.:

```
  EXPECT_ERROR(object.find("nested1.nested3[1].string"));
```


- Kevin Klues


On July 6, 2016, 12:34 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49673/
> ---
> 
> (Updated July 6, 2016, 12:34 a.m.)
> 
> 
> Review request for mesos, Joseph Wu and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a subscript is used on a non-array type, this was not being
> treated as an error. The tests introduced show some examples.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/json.hpp 
> 829838ba95702b536f6badc9ac14fb483668d016 
>   3rdparty/stout/tests/json_tests.cpp 
> 12eaf8e10ea5d1d661975e518ec7f30886b66c0a 
> 
> Diff: https://reviews.apache.org/r/49673/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 49673: Fixed a bug in JSON::Object.find: array subscript is dropped.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On July 6, 2016, 12:34 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49673/
> ---
> 
> (Updated July 6, 2016, 12:34 a.m.)
> 
> 
> Review request for mesos, Joseph Wu and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a subscript is used on a non-array type, this was not being
> treated as an error. The tests introduced show some examples.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/json.hpp 
> 829838ba95702b536f6badc9ac14fb483668d016 
>   3rdparty/stout/tests/json_tests.cpp 
> 12eaf8e10ea5d1d661975e518ec7f30886b66c0a 
> 
> Diff: https://reviews.apache.org/r/49673/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 49671: Minor cleanups in JsonTest.Find.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On July 6, 2016, 12:34 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49671/
> ---
> 
> (Updated July 6, 2016, 12:34 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Minor cleanups in JsonTest.Find.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/json_tests.cpp 
> 12eaf8e10ea5d1d661975e518ec7f30886b66c0a 
> 
> Diff: https://reviews.apache.org/r/49671/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 49670: Updated JSON::Value.find to return None when a Null is found.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On July 6, 2016, 12:34 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49670/
> ---
> 
> (Updated July 6, 2016, 12:34 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated JSON::Value.find to return None when a Null is found.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/json.hpp 
> 829838ba95702b536f6badc9ac14fb483668d016 
>   3rdparty/stout/tests/json_tests.cpp 
> 12eaf8e10ea5d1d661975e518ec7f30886b66c0a 
> 
> Diff: https://reviews.apache.org/r/49670/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 49678: Added unit test to verify GPU isolation inside a docker container.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On July 6, 2016, 3:55 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49678/
> ---
> 
> (Updated July 6, 2016, 3:55 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5401
> https://issues.apache.org/jira/browse/MESOS-5401
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unit test to verify GPU isolation inside a docker container.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
> 474040c89d69b50c051122d873c11a102b33c538 
> 
> Diff: https://reviews.apache.org/r/49678/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" 
> bin/mesos-tests.sh`
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49609: Added filtering for orphaned tasks in /state endpoint.

[Vinod Kone]

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




src/master/http.cpp (line 2511)


is this check safe in a new master old agent scenario?


- Vinod Kone


On July 5, 2016, 7:51 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49609/
> ---
> 
> (Updated July 5, 2016, 7:51 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5757
> https://issues.apache.org/jira/browse/MESOS-5757
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added filtering for orphaned tasks in /state endpoint.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
> 
> Diff: https://reviews.apache.org/r/49609/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 49685: Added filtering for orphaned executors in `GET_EXECUTORS` operator API.

[Vinod Kone]

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




src/master/http.cpp (line 1501)


don't think this check works for new master old agent case?


- Vinod Kone


On July 6, 2016, 3:43 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49685/
> ---
> 
> (Updated July 6, 2016, 3:43 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Joerg Schad, Vinod Kone, and Zhitao 
> Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added filtering for orphaned executors in `GET_EXECUTORS` operator API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp debedd4a4061034a3b55181e93443b9d5e676c52 
> 
> Diff: https://reviews.apache.org/r/49685/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49686: Added filtering for orphaned tasks in `GET_TASKS` operator API.

[Vinod Kone]

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




src/master/http.cpp (line 3491)


ditto. check is not safe i think.


- Vinod Kone


On July 6, 2016, 3:44 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49686/
> ---
> 
> (Updated July 6, 2016, 3:44 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Joerg Schad, Vinod Kone, and Zhitao 
> Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added filtering for orphaned tasks in `GET_TASKS` operator API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp debedd4a4061034a3b55181e93443b9d5e676c52 
> 
> Diff: https://reviews.apache.org/r/49686/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49681: Renamed `unsubscribed_frameworks` to `recovered_frameworks`.

[Vinod Kone]

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




src/master/http.cpp (line 1396)


we should also skip un-authorized frameworks?


- Vinod Kone


On July 6, 2016, 3:43 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49681/
> ---
> 
> (Updated July 6, 2016, 3:43 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Joerg Schad, Vinod Kone, and Zhitao 
> Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed `unsubscribed_frameworks` to `recovered_frameworks`.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
>   include/mesos/v1/master/master.proto 
> b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
>   src/master/http.cpp debedd4a4061034a3b55181e93443b9d5e676c52 
> 
> Diff: https://reviews.apache.org/r/49681/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

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 6, 2016, 3:58 a.m.)


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


Changes
---

Move pipe creation into continuation, and add comment and TODO for the 
continuation.


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 debedd4a4061034a3b55181e93443b9d5e676c52 
  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 49681: Renamed `unsubscribed_frameworks` to `recovered_frameworks`.

[Vinod Kone]

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




src/master/http.cpp (line 1396)


btw. does this check work when an old agent (pre-1.0) re-register with new 
master? cc @joerg


- Vinod Kone


On July 6, 2016, 3:43 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49681/
> ---
> 
> (Updated July 6, 2016, 3:43 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Joerg Schad, Vinod Kone, and Zhitao 
> Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed `unsubscribed_frameworks` to `recovered_frameworks`.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
>   include/mesos/v1/master/master.proto 
> b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
>   src/master/http.cpp debedd4a4061034a3b55181e93443b9d5e676c52 
> 
> Diff: https://reviews.apache.org/r/49681/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49681: Renamed `unsubscribed_frameworks` to `recovered_frameworks`.

[Vinod Kone]

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




include/mesos/master/master.proto (line 331)


s/FrameworkID/FrameworkInfo/



include/mesos/v1/master/master.proto (line 332)


ditto. see above.



src/master/http.cpp (line 1398)


->CopyFrom(master->frameworks.recovered[frameworkId]);


- Vinod Kone


On July 6, 2016, 3:43 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49681/
> ---
> 
> (Updated July 6, 2016, 3:43 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Joerg Schad, Vinod Kone, and Zhitao 
> Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed `unsubscribed_frameworks` to `recovered_frameworks`.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
>   include/mesos/v1/master/master.proto 
> b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
>   src/master/http.cpp debedd4a4061034a3b55181e93443b9d5e676c52 
> 
> Diff: https://reviews.apache.org/r/49681/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49678: Added unit test to verify GPU isolation inside a docker container.

[Kevin Klues]

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

(Updated July 6, 2016, 3:55 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Update to explicitly set the resources consumed in the agent flags, as well as 
reorder tests so that their offers are guaranteed to contain all the resources 
they need.


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


Repository: mesos


Description
---

Added unit test to verify GPU isolation inside a docker container.


Diffs (updated)
-

  src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
474040c89d69b50c051122d873c11a102b33c538 

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


Testing
---

`GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" 
bin/mesos-tests.sh`


Thanks,

Kevin Klues

Review Request 49686: Added filtering for orphaned tasks in `GET_TASKS` operator API.

[haosdent huang]

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

Review request for mesos, Anand Mazumdar, Joerg Schad, Vinod Kone, and Zhitao 
Li.


Repository: mesos


Description
---

Added filtering for orphaned tasks in `GET_TASKS` operator API.


Diffs
-

  src/master/http.cpp debedd4a4061034a3b55181e93443b9d5e676c52 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49681: Renamed `unsubscribed_frameworks` to `recovered_frameworks`.

[haosdent huang]

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

(Updated July 6, 2016, 3:43 a.m.)


Review request for mesos, Anand Mazumdar, Joerg Schad, Vinod Kone, and Zhitao 
Li.


Changes
---

Rebase.


Repository: mesos


Description
---

Renamed `unsubscribed_frameworks` to `recovered_frameworks`.


Diffs (updated)
-

  include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
  include/mesos/v1/master/master.proto b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
  src/master/http.cpp debedd4a4061034a3b55181e93443b9d5e676c52 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49685: Added filtering for orphaned executors in `GET_EXECUTORS` operator API.

[haosdent huang]

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

(Updated July 6, 2016, 3:43 a.m.)


Review request for mesos, Anand Mazumdar, Joerg Schad, Vinod Kone, and Zhitao 
Li.


Changes
---

Rebase.


Summary (updated)
-

Added filtering for orphaned executors in `GET_EXECUTORS` operator API.


Repository: mesos


Description (updated)
---

Added filtering for orphaned executors in `GET_EXECUTORS` operator API.


Diffs (updated)
-

  src/master/http.cpp debedd4a4061034a3b55181e93443b9d5e676c52 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49680: Adjust the format of allocator slack channel for working group.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On July 6, 2016, 1:46 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49680/
> ---
> 
> (Updated July 6, 2016, 1:46 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adjust the format of allocator slack channel for working group.
> 
> 
> Diffs
> -
> 
>   docs/working-groups.md d2b286f2e5b5f4dd9d2217ec004b4c77bd0279ca 
> 
> Diff: https://reviews.apache.org/r/49680/diff/
> 
> 
> Testing
> ---
> 
> https://github.com/gyliu513/mesos/blob/master/docs/working-groups.md
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 49681: Renamed `unsubscribed_frameworks` to `recovered_frameworks`.

[haosdent huang]

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

(Updated July 6, 2016, 3:31 a.m.)


Review request for mesos, Anand Mazumdar, Joerg Schad, Vinod Kone, and Zhitao 
Li.


Changes
---

Rebase.


Repository: mesos


Description
---

Renamed `unsubscribed_frameworks` to `recovered_frameworks`.


Diffs (updated)
-

  include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
  include/mesos/v1/master/master.proto b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
  src/master/http.cpp 6b4f85b8655b3f6fcab9e595bb9bd05493f4421a 

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


Testing
---


Thanks,

haosdent huang

Review Request 49685: Added filtering for orphaned executors in `master::Call::GET_EXECUTORS`.

[haosdent huang]

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

Review request for mesos, Anand Mazumdar, Joerg Schad, Vinod Kone, and Zhitao 
Li.


Repository: mesos


Description
---

Added filtering for orphaned executors in `master::Call::GET_EXECUTORS`.


Diffs
-

  src/master/http.cpp 6b4f85b8655b3f6fcab9e595bb9bd05493f4421a 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49600: Added authz to /files/debug endpoint.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49600]

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 5, 2016, 8:08 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49600/
> ---
> 
> (Updated July 5, 2016, 8:08 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, and 
> Jan Schlicht.
> 
> 
> Bugs: MESOS-5369
> https://issues.apache.org/jira/browse/MESOS-5369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authz to /files/debug endpoint.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md fb56cdba6f142171b3d49e2582b7921f211907e3 
>   docs/upgrades.md 255b5bd44c01bb7bfc8d9d7a11f393b2b2502084 
>   src/common/http.cpp fffa24cb07f73dd2bc8edc59fc276fdca5b86380 
>   src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
>   src/files/files.cpp a5a1b86e14f63e5e3834a2900270252fbe16f586 
>   src/master/main.cpp 84f3b0723fd3df02386c8072ded3cb42272cd065 
>   src/slave/main.cpp a7ed669a0b6861d6ce8546dfafac849044a77eec 
>   src/tests/files_tests.cpp 31337e280c6224a8c949c8868a53e5a785b4573f 
> 
> Diff: https://reviews.apache.org/r/49600/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> 
> sudo GTEST_FILTER="FilesTest.DebugTest" make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49223: Enhance value parsing.

[Klaus Ma]

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

(Updated July 6, 2016, 10:40 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Address comments.


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


Repository: mesos


Description
---

Enhance value parsing.


Diffs (updated)
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

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


Testing
---

make && make check


Thanks,

Klaus Ma

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/#review140927
---




src/master/http.cpp (lines 1551 - 1552)


I think @anandmazumdar means
```
.then([contentType](const mesos::master::Response::GetState& getState)
-> Future {
```


- haosdent huang


On July 6, 2016, 1:48 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49517/
> ---
> 
> (Updated July 6, 2016, 1:48 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 debedd4a4061034a3b55181e93443b9d5e676c52 
>   src/master/master.hpp 996ff425fc8e9234a5e02560460ad1233dca7061 
>   src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 
> 
> Diff: https://reviews.apache.org/r/49517/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Review Request 49681: Renamed `unsubscribed_frameworks` to `recovered_frameworks`.

[haosdent huang]

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

Review request for mesos, Anand Mazumdar, Vinod Kone, and Zhitao Li.


Repository: mesos


Description
---

Renamed `unsubscribed_frameworks` to `recovered_frameworks`.


Diffs
-

  include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
  include/mesos/v1/master/master.proto b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
  src/master/http.cpp 6b4f85b8655b3f6fcab9e595bb9bd05493f4421a 

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


Testing
---


Thanks,

haosdent huang

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 6, 2016, 1:49 a.m.)


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


Changes
---

Rebase.


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 debedd4a4061034a3b55181e93443b9d5e676c52 
  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 6, 2016, 1:48 a.m.)


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


Changes
---

Rebase and review comments.


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 debedd4a4061034a3b55181e93443b9d5e676c52 
  src/master/master.hpp 996ff425fc8e9234a5e02560460ad1233dca7061 
  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.

[Zhitao Li]

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

(Updated July 6, 2016, 1:47 a.m.)


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


Changes
---

Rebase and review comments.


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


Repository: mesos


Description
---

Helper function will be reused by `GetExecutors` and `GetState`.


Diffs (updated)
-

  src/master/http.cpp debedd4a4061034a3b55181e93443b9d5e676c52 
  src/master/master.hpp 996ff425fc8e9234a5e02560460ad1233dca7061 

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


Testing
---

make check


Thanks,

Zhitao Li

Review Request 49680: Adjust the format of allocator slack channel for working group.

[Guangya Liu]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Adjust the format of allocator slack channel for working group.


Diffs
-

  docs/working-groups.md d2b286f2e5b5f4dd9d2217ec004b4c77bd0279ca 

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


Testing
---

https://github.com/gyliu513/mesos/blob/master/docs/working-groups.md


Thanks,

Guangya Liu

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

[Zhitao Li]

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

(Updated July 6, 2016, 1:46 a.m.)


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


Changes
---

Rebase and review comments.


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


Repository: mesos


Description
---

Revised protobuf definition of GetState response.


Diffs (updated)
-

  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 49489: Refactor master::Http::getFrameworks to helper function.

[Zhitao Li]

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

(Updated July 6, 2016, 1:45 a.m.)


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


Changes
---

Rebase and review comments.


Repository: mesos


Description
---

The new helper function will be reused by `GET_FRAMEWORKS` and
`GET_STATE` calls.


Diffs (updated)
-

  src/master/http.cpp debedd4a4061034a3b55181e93443b9d5e676c52 
  src/master/master.hpp 996ff425fc8e9234a5e02560460ad1233dca7061 

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


Testing
---

make check


Thanks,

Zhitao Li

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

[Zhitao Li]

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

(Updated July 6, 2016, 1:44 a.m.)


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


Changes
---

Rebase and review comments.


Repository: mesos


Description
---

The new helper function will be reused by both `GET_AGENTS`
and `GET_STATE` calls.


Diffs (updated)
-

  src/master/http.cpp debedd4a4061034a3b55181e93443b9d5e676c52 
  src/master/master.hpp 996ff425fc8e9234a5e02560460ad1233dca7061 

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.

[Zhitao Li]

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

(Updated July 6, 2016, 1:43 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
---

Helper function will be also be reused for `GetState`.


Diffs (updated)
-

  src/master/http.cpp debedd4a4061034a3b55181e93443b9d5e676c52 
  src/master/master.hpp 996ff425fc8e9234a5e02560460ad1233dca7061 

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


Testing
---

make check


Thanks,

Zhitao Li

Re: Review Request 49616: Add suppression benchmark.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49616]

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 5, 2016, 9:48 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49616/
> ---
> 
> (Updated July 5, 2016, 9:48 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5781
> https://issues.apache.org/jira/browse/MESOS-5781
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Useful for high framework clusters which carry
>   many suppressed frameworks that are considered
>   during allocation.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0498cd5e54b0e4b87a767585a77699653aa52179 
> 
> Diff: https://reviews.apache.org/r/49616/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49657: Added ability to set framework capabilities in 'mesos-execute'.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On July 6, 2016, 1:22 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49657/
> ---
> 
> (Updated July 6, 2016, 1:22 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5787
> https://issues.apache.org/jira/browse/MESOS-5787
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added ability to set framework capabilities in 'mesos-execute'.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 18c2e3449bf5e50bea0bbd0a92efa20fc8b9032b 
> 
> Diff: https://reviews.apache.org/r/49657/diff/
> 
> 
> Testing
> ---
> 
> Start a master/ agent on a GPU machine as specified in:
> ```
> http://mesos.apache.org/documentation/latest/container-image/
> ```
> 
> Run execute:
> ```
> $ sudo bin/mesos-execute \
>   --master=:5050 \
>   --name=test \
>   --docker_image=alpine \
>   --command="pwd" \
>   --framework_capabilities="GPU_RESOURCES"
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49657: Added ability to set framework capabilities in 'mesos-execute'.

[Kevin Klues]

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

(Updated July 6, 2016, 1:22 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Upated to address (offline) comments.


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


Repository: mesos


Description
---

Added ability to set framework capabilities in 'mesos-execute'.


Diffs (updated)
-

  src/cli/execute.cpp 18c2e3449bf5e50bea0bbd0a92efa20fc8b9032b 

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


Testing
---

Start a master/ agent on a GPU machine as specified in:
```
http://mesos.apache.org/documentation/latest/container-image/
```

Run execute:
```
$ sudo bin/mesos-execute \
  --master=:5050 \
  --name=test \
  --docker_image=alpine \
  --command="pwd" \
  --framework_capabilities="GPU_RESOURCES"
```


Thanks,

Kevin Klues

Re: Review Request 49669: Integrated the 'NvidiaVolume' component into the 'NvidiaGpuIsolator'.

[Kevin Klues]

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

(Updated July 6, 2016, 1:22 a.m.)


Review request for mesos, Benjamin Mahler and Jie Yu.


Changes
---

Upated to address comments.


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


Repository: mesos


Description
---

Integrated the 'NvidiaVolume' component into the 'NvidiaGpuIsolator'.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/isolator.hpp 
ceb37950824e1f67f2f1c79846e6e5a9a83a5a89 
  src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
d407cab5362473e040a6e23d9c3a17af3459720b 

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


Testing
---

A unit test is forthcoming, but
I tested manually with a `master`, `agent` and `mesos-execute`

```
sudo ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos

sudo bin/mesos-agent.sh --master=127.0.0.1:5050 --ip=127.0.0.1 
--work_dir=/var/lib/mesos 
--isolation="docker/runtime,filesystem/lux,cgroups/devices,gpu/nvidia" 
--image_providers=docker --executor_environment_variables="{}"

src/mesos-execute --master=127.0.0.1:5050 
--framework_capabilities="GPU_RESOURCES" --resources="cpus:1;mem:128;gpus:3" 
--name=test --docker_image=nvidia/cuda --command="nvidia-smi"
```


Thanks,

Kevin Klues

Review Request 49678: Added unit test to verify GPU isolation inside a docker container.

[Kevin Klues]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Added unit test to verify GPU isolation inside a docker container.


Diffs
-

  src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
474040c89d69b50c051122d873c11a102b33c538 

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


Testing
---

`GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" 
bin/mesos-tests.sh`


Thanks,

Kevin Klues

Re: Review Request 49668: Added Nvidia devices as default devices to inject into every container.

[Kevin Klues]

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

(Updated July 6, 2016, 1:22 a.m.)


Review request for mesos, Benjamin Mahler and Jie Yu.


Changes
---

Upated to address comments.


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


Repository: mesos


Description
---

This is a temporary hack to make Nvidia devices available to a
container when file system isolation is enabled. Ideally, we would
have a generic mechanism of injecting new devices into a continaer
from an isolator, but this feature is currently lacking so we need to
resort to hard-coding them for now.

In essence, this commit adds all Nvidia devices to the default list of
devices on machines that have Nvidia GPUs. Without the 'gpu/nvidia'
isolator enabled, this means that containers have free reign to use
these devices however they want. However, with the `'gpu/nvidia'
isolator enabled, they will have restricted access to only those GPUs
they have been allocated (though they will be able to see all of them
on the file system). These are similar semantics to the current
support for Nvidia GPUs without filesystem isolation.

In the future, we will restrict injecting these devices to only occur
when the 'nvidia/gpu' isolator is enabled.


Diffs (updated)
-

  src/linux/fs.cpp 3fe820340efd8e81818e06a548575bca829a0bd1 

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


Testing
---

make check

A subesequent commit will contain an End-to-End unit test using these devices 
on a GPU capable machine.


Thanks,

Kevin Klues

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

[Anand Mazumdar]

> On July 6, 2016, 12:30 a.m., Anand Mazumdar wrote:
> > src/tests/api_tests.cpp, line 537
> > 
> >
> > Just execDriver->sendXX would suffice.
> > 
> > Would be a good idea to do a sweep across this file and remove the 
> > redundant `get()` in favor of the `->` operator.
> 
> Zhitao Li wrote:
> I think we need at least one `get()` here, because type of `execDriver` 
> is a Future?

Yep, my bad. Might be good to audit the other instances where they are not 
needed.


- Anand


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


On July 4, 2016, 4:05 a.m., Zhitao Li wrote:
> 
> ---
> 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.
> 
> 
> 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 49669: Integrated the 'NvidiaVolume' component into the 'NvidiaGpuIsolator'.

[Benjamin Mahler]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/gpu/isolator.cpp (lines 292 - 293)


Ideally we could print out the 'type' here but it looks like 
ContainerConfig doesn't have a type field :(

Mind adding a TODO there?

How about the following error message for now:

```
// TODO(klueska): Once ContainerConfig has a type, include that in the 
error message.
return Failure("Nvidia GPU Isolator does not support non-Docker images");
```



src/slave/containerizer/mesos/isolators/gpu/isolator.cpp (lines 299 - 304)


Move this below the check?



src/slave/containerizer/mesos/isolators/gpu/isolator.cpp (line 323)


How about the more readable long names?

```
--no-mtab
--read-only
```


- Benjamin Mahler


On July 6, 2016, 12:02 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49669/
> ---
> 
> (Updated July 6, 2016, 12:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-5401
> https://issues.apache.org/jira/browse/MESOS-5401
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Integrated the 'NvidiaVolume' component into the 'NvidiaGpuIsolator'.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.hpp 
> ceb37950824e1f67f2f1c79846e6e5a9a83a5a89 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
> d407cab5362473e040a6e23d9c3a17af3459720b 
> 
> Diff: https://reviews.apache.org/r/49669/diff/
> 
> 
> Testing
> ---
> 
> A unit test is forthcoming, but
> I tested manually with a `master`, `agent` and `mesos-execute`
> 
> ```
> sudo ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
> 
> sudo bin/mesos-agent.sh --master=127.0.0.1:5050 --ip=127.0.0.1 
> --work_dir=/var/lib/mesos 
> --isolation="docker/runtime,filesystem/lux,cgroups/devices,gpu/nvidia" 
> --image_providers=docker --executor_environment_variables="{}"
> 
> src/mesos-execute --master=127.0.0.1:5050 
> --framework_capabilities="GPU_RESOURCES" --resources="cpus:1;mem:128;gpus:3" 
> --name=test --docker_image=nvidia/cuda --command="nvidia-smi"
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

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

[Anand Mazumdar]

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


Ship it!




Similar cleanup comments to the getFrameworks review (r49489)

- Anand Mazumdar


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 49509: Revised protobuf definition of GetState response.

[Anand Mazumdar]

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


Fix it, then Ship it!





include/mesos/master/master.proto (line 272)


Can you add a short description for this similar to what we have for other 
protobuf calls?

Ditto for the versioned protobuf.


- Anand Mazumdar


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 49489: Refactor master::Http::getFrameworks to helper function.

[Anand Mazumdar]

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


Fix it, then Ship it!





src/master/http.cpp (line 1339)


s/Response/Future



src/master/http.cpp (line 1350)


Pass argument by const ref. Ditto for the declaration.



src/master/http.cpp (line 1371)


Future<> to be consistent.


- Anand Mazumdar


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 49517: Implement GetState V1 master API.

[Zhitao Li]

> On July 6, 2016, 12:30 a.m., Anand Mazumdar wrote:
> > src/tests/api_tests.cpp, line 537
> > 
> >
> > Just execDriver->sendXX would suffice.
> > 
> > Would be a good idea to do a sweep across this file and remove the 
> > redundant `get()` in favor of the `->` operator.

I think we need at least one `get()` here, because type of `execDriver` is a 
Future?


- Zhitao


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


On July 4, 2016, 4:05 a.m., Zhitao Li wrote:
> 
> ---
> 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.
> 
> 
> 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 49668: Added Nvidia devices as default devices to inject into every container.

[Benjamin Mahler]

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


Fix it, then Ship it!





src/linux/fs.cpp (line 523)


Include for glob?



src/linux/fs.cpp (line 525)


Include the error context?



src/linux/fs.cpp (line 528)


s/string &/string& /


- Benjamin Mahler


On July 6, 2016, 12:06 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49668/
> ---
> 
> (Updated July 6, 2016, 12:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-5793
> https://issues.apache.org/jira/browse/MESOS-5793
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a temporary hack to make Nvidia devices available to a
> container when file system isolation is enabled. Ideally, we would
> have a generic mechanism of injecting new devices into a continaer
> from an isolator, but this feature is currently lacking so we need to
> resort to hard-coding them for now.
> 
> In essence, this commit adds all Nvidia devices to the default list of
> devices on machines that have Nvidia GPUs. Without the 'gpu/nvidia'
> isolator enabled, this means that containers have free reign to use
> these devices however they want. However, with the `'gpu/nvidia'
> isolator enabled, they will have restricted access to only those GPUs
> they have been allocated (though they will be able to see all of them
> on the file system). These are similar semantics to the current
> support for Nvidia GPUs without filesystem isolation.
> 
> In the future, we will restrict injecting these devices to only occur
> when the 'nvidia/gpu' isolator is enabled.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.cpp 3fe820340efd8e81818e06a548575bca829a0bd1 
> 
> Diff: https://reviews.apache.org/r/49668/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> A subesequent commit will contain an End-to-End unit test using these devices 
> on a GPU capable machine.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

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

[Anand Mazumdar]

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


Fix it, then Ship it!





src/master/http.cpp (line 2081)


s/Response/Future to be consistent throughout

Also 
- 2 space indent before then instead of the 4 space indent.
- 2 space indent before ->



src/master/http.cpp (line 2093)


Capture by const ref. Ditto for the declaration.



src/master/http.cpp (line 2125)


Nit: Newline before


- Anand Mazumdar


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 49658: Updated elfio-3.1.patch to fix alignment bug properly.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On July 5, 2016, 9:25 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49658/
> ---
> 
> (Updated July 5, 2016, 9:25 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5767
> https://issues.apache.org/jira/browse/MESOS-5767
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous patch incorrectly fixed the alignment of an ELF note to 4
> bytes (even for 64 bit ELF files). This happened to be OK for our
> specific case of parsing '.note.ABI-tag' sections. However, the
> alignment for notes in a 64 bit ELF section should be 8-byte aligned
> (as the original code attempts to do). The problem with the original
> code, however, is that it performs the alignment only on the name
> string inside the note, rather than the entire note object itself.
> This commit fixes this alignment issue.
> 
> 
> Diffs
> -
> 
>   3rdparty/elfio-3.1.patch f8c20e92a0ec12e8a2f1d8e8ec54eac78ced707d 
> 
> Diff: https://reviews.apache.org/r/49658/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

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

[Zhitao Li]

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

(Updated July 6, 2016, 12:38 a.m.)


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


Changes
---

Removed dependency on `GET_EXECUTORS` since it's already committed


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 49487: Refactor Master::Http::getTasks into helper function.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


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

Review Request 49673: Fixed a bug in JSON::Object.find: array subscript is dropped.

[Benjamin Mahler]

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

Review request for mesos, Joseph Wu and Kevin Klues.


Repository: mesos


Description
---

If a subscript is used on a non-array type, this was not being
treated as an error. The tests introduced show some examples.


Diffs
-

  3rdparty/stout/include/stout/json.hpp 
829838ba95702b536f6badc9ac14fb483668d016 
  3rdparty/stout/tests/json_tests.cpp 12eaf8e10ea5d1d661975e518ec7f30886b66c0a 

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


Testing
---

make check


Thanks,

Benjamin Mahler

Review Request 49671: Minor cleanups in JsonTest.Find.

[Benjamin Mahler]

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

Review request for mesos and Kevin Klues.


Repository: mesos


Description
---

Minor cleanups in JsonTest.Find.


Diffs
-

  3rdparty/stout/tests/json_tests.cpp 12eaf8e10ea5d1d661975e518ec7f30886b66c0a 

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


Testing
---

make check


Thanks,

Benjamin Mahler

Review Request 49674: Fixed the docker executor to handle inspect failures.

[Benjamin Mahler]

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

Review request for mesos, Gilbert Song, Joseph Wu, and Kevin Klues.


Repository: mesos


Description
---

Fixed the docker executor to handle inspect failures.


Diffs
-

  src/docker/executor.cpp c53a008bf4256a4507dfcc75dc297f133f4c3c6a 

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


Testing
---

make check


Thanks,

Benjamin Mahler

Review Request 49670: Updated JSON::Value.find to return None when a Null is found.

[Benjamin Mahler]

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

Review request for mesos, Gilbert Song and Kevin Klues.


Repository: mesos


Description
---

Updated JSON::Value.find to return None when a Null is found.


Diffs
-

  3rdparty/stout/include/stout/json.hpp 
829838ba95702b536f6badc9ac14fb483668d016 
  3rdparty/stout/tests/json_tests.cpp 12eaf8e10ea5d1d661975e518ec7f30886b66c0a 

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


Testing
---

make check


Thanks,

Benjamin Mahler

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

[Anand Mazumdar]

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


Fix it, then Ship it!




Some minor style nits.

Also if you have not done so, can you run this test with `gtest_repeat=100` to 
be sure that it's not flaky?


src/master/http.cpp (line 1537)


2 space indent here for continuations please like you do in a later 
function in this change.



src/master/http.cpp (line 1538)


2 space indent here before `->`



src/master/http.cpp (line 1550)


pass argument by const ref. Ditto for the declaration in header



src/master/http.cpp (line 1558)


Do you need to capture anything here?



src/tests/api_tests.cpp (line 453)


Remove this. We tend to avoid being explicit about `Times(1)`.



src/tests/api_tests.cpp (line 467)


Extra space.



src/tests/api_tests.cpp (line 474)


New line before and capture by const ref.



src/tests/api_tests.cpp (lines 477 - 478)


Reorder expectation based on comment earlier.



src/tests/api_tests.cpp (lines 481 - 489)


Let's be less verbose and just use:

`createTask(offer, "", DEFAULT_EXECUTOR_ID)`



src/tests/api_tests.cpp (line 517)


Newline before and capture by const ref?



src/tests/api_tests.cpp (line 537)


Just execDriver->sendXX would suffice.

Would be a good idea to do a sweep across this file and remove the 
redundant `get()` in favor of the `->` operator.



src/tests/api_tests.cpp (line 552)


Newline before + capture by const ref?


- Anand Mazumdar


On July 4, 2016, 4:05 a.m., Zhitao Li wrote:
> 
> ---
> 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.
> 
> 
> 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 49672: Skipped terminated executors in statistics endpoints.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On July 6, 2016, 12:14 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49672/
> ---
> 
> (Updated July 6, 2016, 12:14 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gilbert Song, and Vinod Kone.
> 
> 
> Bugs: MESOS-5794
> https://issues.apache.org/jira/browse/MESOS-5794
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For /containers and /monitor/statistics endpoints. This is possible
> because the agent won't remove the executor from its list until it
> receives the ack for the last status update. For those executors, we
> know that they have terminated, therefore we should skip them.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 67ed67e168faab692d59e077b2e9a927756c6704 
>   src/slave/slave.cpp 066c0ee83fce58127a3c7a8e790655458122fcf9 
> 
> Diff: https://reviews.apache.org/r/49672/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

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 6, 2016, 12:27 a.m.)


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


Changes
---

s/Note/NOTE in comment.


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 debedd4a4061034a3b55181e93443b9d5e676c52 
  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 6, 2016, 12:26 a.m.)


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


Changes
---

Review comments from @anandm.


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 debedd4a4061034a3b55181e93443b9d5e676c52 
  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 49668: Added Nvidia devices as default devices to inject into every container.

[Jie Yu]

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


Fix it, then Ship it!




Please make sure to run a sudo make check ;)


src/linux/fs.cpp (line 523)


path::join here is not needed, just do
```
os::glob("/dev/nvidia*");
```


- Jie Yu


On July 6, 2016, 12:06 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49668/
> ---
> 
> (Updated July 6, 2016, 12:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-5793
> https://issues.apache.org/jira/browse/MESOS-5793
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a temporary hack to make Nvidia devices available to a
> container when file system isolation is enabled. Ideally, we would
> have a generic mechanism of injecting new devices into a continaer
> from an isolator, but this feature is currently lacking so we need to
> resort to hard-coding them for now.
> 
> In essence, this commit adds all Nvidia devices to the default list of
> devices on machines that have Nvidia GPUs. Without the 'gpu/nvidia'
> isolator enabled, this means that containers have free reign to use
> these devices however they want. However, with the `'gpu/nvidia'
> isolator enabled, they will have restricted access to only those GPUs
> they have been allocated (though they will be able to see all of them
> on the file system). These are similar semantics to the current
> support for Nvidia GPUs without filesystem isolation.
> 
> In the future, we will restrict injecting these devices to only occur
> when the 'nvidia/gpu' isolator is enabled.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.cpp 3fe820340efd8e81818e06a548575bca829a0bd1 
> 
> Diff: https://reviews.apache.org/r/49668/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> A subesequent commit will contain an End-to-End unit test using these devices 
> on a GPU capable machine.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49672: Skipped terminated executors in statistics endpoints.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 5, 2016, 5:14 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49672/
> ---
> 
> (Updated July 5, 2016, 5:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gilbert Song, and Vinod Kone.
> 
> 
> Bugs: MESOS-5794
> https://issues.apache.org/jira/browse/MESOS-5794
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For /containers and /monitor/statistics endpoints. This is possible
> because the agent won't remove the executor from its list until it
> receives the ack for the last status update. For those executors, we
> know that they have terminated, therefore we should skip them.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 67ed67e168faab692d59e077b2e9a927756c6704 
>   src/slave/slave.cpp 066c0ee83fce58127a3c7a8e790655458122fcf9 
> 
> Diff: https://reviews.apache.org/r/49672/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 49672: Skipped terminated executors in statistics endpoints.

[Jie Yu]

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

Review request for mesos, Benjamin Mahler, Gilbert Song, and Vinod Kone.


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


Repository: mesos


Description
---

For /containers and /monitor/statistics endpoints. This is possible
because the agent won't remove the executor from its list until it
receives the ack for the last status update. For those executors, we
know that they have terminated, therefore we should skip them.


Diffs
-

  src/slave/http.cpp 67ed67e168faab692d59e077b2e9a927756c6704 
  src/slave/slave.cpp 066c0ee83fce58127a3c7a8e790655458122fcf9 

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


Testing
---

make check


Thanks,

Jie Yu

Review Request 49668: Added Nvidia devices as default devices to inject into every container.

[Kevin Klues]

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

Review request for mesos, Benjamin Mahler and Jie Yu.


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


Repository: mesos


Description
---

This is a temporary hack to make Nvidia devices available to a
container when file system isolation is enabled. Ideally, we would
have a generic mechanism of injecting new devices into a continaer
from an isolator, but this feature is currently lacking so we need to
resort to hard-coding them for now.

In essence, this commit adds all Nvidia devices to the default list of
devices on machines that have Nvidia GPUs. Without the 'gpu/nvidia'
isolator enabled, this means that containers have free reign to use
these devices however they want. However, with the `'gpu/nvidia'
isolator enabled, they will have restricted access to only those GPUs
they have been allocated (though they will be able to see all of them
on the file system). These are similar semantics to the current
support for Nvidia GPUs without filesystem isolation.

In the future, we will restrict injecting these devices to only occur
when the 'nvidia/gpu' isolator is enabled.


Diffs
-

  src/linux/fs.cpp 3fe820340efd8e81818e06a548575bca829a0bd1 

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


Testing
---

make check

A subesequent commit will contain an End-to-End unit test using these devices 
on a GPU capable machine.


Thanks,

Kevin Klues

Re: Review Request 49660: Added #allocator slack channel to Resource Allocation group.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On July 5, 2016, 11:57 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49660/
> ---
> 
> (Updated July 5, 2016, 11:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added #allocator slack channel to Resource Allocation group.
> 
> 
> Diffs
> -
> 
>   docs/working-groups.md b1b696fa7907040374919a0e43c76b2deedfb2c3 
> 
> Diff: https://reviews.apache.org/r/49660/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 49669: Integrated the 'NvidiaVolume' component into the 'NvidiaGpuIsolator'.

[Kevin Klues]

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

Review request for mesos, Benjamin Mahler and Jie Yu.


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


Repository: mesos


Description
---

Integrated the 'NvidiaVolume' component into the 'NvidiaGpuIsolator'.


Diffs
-

  src/slave/containerizer/mesos/isolators/gpu/isolator.hpp 
ceb37950824e1f67f2f1c79846e6e5a9a83a5a89 
  src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
d407cab5362473e040a6e23d9c3a17af3459720b 

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


Testing
---

A unit test is forthcoming, but
I tested manually with a `master`, `agent` and `mesos-execute`

```
sudo ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos

sudo bin/mesos-agent.sh --master=127.0.0.1:5050 --ip=127.0.0.1 
--work_dir=/var/lib/mesos 
--isolation="docker/runtime,filesystem/lux,cgroups/devices,gpu/nvidia" 
--image_providers=docker --executor_environment_variables="{}"

src/mesos-execute --master=127.0.0.1:5050 
--framework_capabilities="GPU_RESOURCES" --resources="cpus:1;mem:128;gpus:3" 
--name=test --docker_image=nvidia/cuda --command="nvidia-smi"
```


Thanks,

Kevin Klues

Re: Review Request 49623: Fixed a no-longer-correct comment in `Master::shutdownSlave`.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On July 5, 2016, 12:22 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49623/
> ---
> 
> (Updated July 5, 2016, 12:22 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The comment was correct when written, but is no longer accurate.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 0d054d027ff2a381606d81062fe56249e0fcb839 
>   src/master/master.cpp 78a8889313179b4509a6657e6c61d9f6d3bb99c0 
> 
> Diff: https://reviews.apache.org/r/49623/diff/
> 
> 
> Testing
> ---
> 
> Also tweak another comment in `master.hpp`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49660: Added #allocator slack channel to Resource Allocation group.

[Guangya Liu]

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

(Updated 七月 5, 2016, 11:57 p.m.)


Review request for mesos, Benjamin Mahler and Vinod Kone.


Repository: mesos


Description
---

Added #allocator slack channel to Resource Allocation group.


Diffs (updated)
-

  docs/working-groups.md b1b696fa7907040374919a0e43c76b2deedfb2c3 

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


Testing
---


Thanks,

Guangya Liu

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

[Zhitao Li]

> On July 5, 2016, 11:41 p.m., Anand Mazumdar wrote:
> > src/master/http.cpp, line 598
> > 
> >
> > s/executod/executed
> > s/Note/NOTE
> > 
> > Also, this looks more like a TODO than a NOTE?

This is a note for futher maintainer so s/he does not accidentally move this 
into a `defer` and breaks the logic.


- Zhitao


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


On July 5, 2016, 11:08 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49518/
> ---
> 
> (Updated July 5, 2016, 11:08 p.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.

[Anand Mazumdar]

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



Looks very good! Just some minor style nit-picks.


include/mesos/master/master.proto (line 443)


The part about this event being forwarded by the master looks redundant?

How about making this consistent with similar comments in 
scheduler/executor.proto i.e.

// First event received when a client subscribes.



src/master/http.cpp (line 598)


s/executod/executed
s/Note/NOTE

Also, this looks more like a TODO than a NOTE?



src/tests/api_tests.cpp (lines 1424 - 1425)


Capture alias by const ref?



src/tests/api_tests.cpp (lines 1427 - 1434)


Nit: These individual multi line statements should all fit in one line, no?



src/tests/api_tests.cpp (lines 1433 - 1434)


Nit: Move this after L1428 to align with your comment i.e. expected 1 
framework/agent followed by 0 tasks/executors.


- Anand Mazumdar


On July 5, 2016, 11:08 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49518/
> ---
> 
> (Updated July 5, 2016, 11:08 p.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 49600: Added authz to /files/debug endpoint.

[Adam B]

> On July 5, 2016, 3:04 a.m., Adam B wrote:
> > src/files/files.hpp, line 65
> > 
> >
> > Shame we have to perpetuate these naked pointers. Any chance we could 
> > use a `shared_ptr`?
> 
> Abhishek Dasgupta wrote:
> It looks we have to change mesos::Authorizer* raw pointer to shared 
> pointer from all along..Only then it makes sense to use shared_ptr here..We 
> can raise another patch to conver all the mesos::Authorizer* raw pointer to 
> shared_ptr..Should we do that? I may raise another isuue to track it. What do 
> you say??

Go ahead and file a new JIRA for this, and we can commit this patch/JIRA 
without it. We can drop this RB issue once you have linked the new JIRA in the 
comment here.


- Adam


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


On July 5, 2016, 1:08 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49600/
> ---
> 
> (Updated July 5, 2016, 1:08 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, and 
> Jan Schlicht.
> 
> 
> Bugs: MESOS-5369
> https://issues.apache.org/jira/browse/MESOS-5369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authz to /files/debug endpoint.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md fb56cdba6f142171b3d49e2582b7921f211907e3 
>   docs/upgrades.md 255b5bd44c01bb7bfc8d9d7a11f393b2b2502084 
>   src/common/http.cpp fffa24cb07f73dd2bc8edc59fc276fdca5b86380 
>   src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
>   src/files/files.cpp a5a1b86e14f63e5e3834a2900270252fbe16f586 
>   src/master/main.cpp 84f3b0723fd3df02386c8072ded3cb42272cd065 
>   src/slave/main.cpp a7ed669a0b6861d6ce8546dfafac849044a77eec 
>   src/tests/files_tests.cpp 31337e280c6224a8c949c8868a53e5a785b4573f 
> 
> Diff: https://reviews.apache.org/r/49600/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> 
> sudo GTEST_FILTER="FilesTest.DebugTest" make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49600: Added authz to /files/debug endpoint.

[Adam B]

> On July 5, 2016, 12:03 a.m., Alexander Rojas wrote:
> > src/files/files.cpp, lines 729-731
> > 
> >
> > The lambda doesn't use any method nor attribute from `FilesProcess`, so 
> > you can save the `defer()` call as well as the `this` capture.
> 
> Abhishek Dasgupta wrote:
> Alexander, but in the future if someone adds something in the lambda 
> accesses the this pointer. This would lead to a potential race condition. So 
> I thought it is safe to wrap it in defer(). What do u say?? I will remove the 
> 'this' capture for now.
> 
> Alexander Rojas wrote:
> I understand your point, but I myself had been requested this change 
> [here](https://reviews.apache.org/r/49394/#comment205373). However I leave 
> this up to you and your shepherd to decide. `this` should go anyway though.
> 
> Abhishek Dasgupta wrote:
> And I myself was asked to use defer 
> [here](https://reviews.apache.org/r/49446/diff/1?file=1434166#file1434166line2793).Adam
>  , any comment??

In Alexander's example, `jsonp` is the only variable from local scope passed 
into the lambda, so the lambda has no chance of accessing any shared state.
In Abhishek's example, `[=]` is passed in, which includes all of `this` and 
hence it's dangerous to execute that lambda without a defer. If you could 
substitute individual immutable parameters in place of `[=]` you may be able to 
avoid a defer in that case too.
Since this patch no longer passes `this`, and only passes the immutable 
`object` and `jsonp`, we should be safe without the defer. If somebody in the 
future tries to add `this` back into the lambda parameters, they'll need to 
setup a defer then.


- Adam


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


On July 5, 2016, 1:08 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49600/
> ---
> 
> (Updated July 5, 2016, 1:08 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, and 
> Jan Schlicht.
> 
> 
> Bugs: MESOS-5369
> https://issues.apache.org/jira/browse/MESOS-5369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authz to /files/debug endpoint.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md fb56cdba6f142171b3d49e2582b7921f211907e3 
>   docs/upgrades.md 255b5bd44c01bb7bfc8d9d7a11f393b2b2502084 
>   src/common/http.cpp fffa24cb07f73dd2bc8edc59fc276fdca5b86380 
>   src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
>   src/files/files.cpp a5a1b86e14f63e5e3834a2900270252fbe16f586 
>   src/master/main.cpp 84f3b0723fd3df02386c8072ded3cb42272cd065 
>   src/slave/main.cpp a7ed669a0b6861d6ce8546dfafac849044a77eec 
>   src/tests/files_tests.cpp 31337e280c6224a8c949c8868a53e5a785b4573f 
> 
> Diff: https://reviews.apache.org/r/49600/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> 
> sudo GTEST_FILTER="FilesTest.DebugTest" make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49660: Added #allocator slack channel to Resource Allocation group.

[Benjamin Mahler]

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




docs/working-groups.md (lines 98 - 99)


Can you also link to the archives?

http://mesos.slackarchive.io/allocator/

I'll invite the archive bot to the room.


- Benjamin Mahler


On July 5, 2016, 9:49 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49660/
> ---
> 
> (Updated July 5, 2016, 9:49 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added #allocator slack channel to Resource Allocation group.
> 
> 
> Diffs
> -
> 
>   docs/working-groups.md b1b696fa7907040374919a0e43c76b2deedfb2c3 
> 
> Diff: https://reviews.apache.org/r/49660/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

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 5, 2016, 11:08 p.m.)


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


Changes
---

Rename field name from `snapshot` to `get_state` to be consistent.


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 49661: Ignore null entries for Docker inspect's 'HostConfig.Devices' field.

[Gilbert Song]

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




src/docker/docker.cpp (line 365)


This patch should work correctly, but using a `json.find` here seems a 
little not accurate to me.

The reason is that we do a `json.find` above and `JSON::Null` 
would be regarded as an error case, but actually it isn't. We just want to skip 
it.

More pratically, could we consider doing `json.find` first and 
then convert it to be a JSON array if it is not null.

```
Result devicesArray =
  json.find("HostConfig.Devices");
  
if (devicesArray.isError()) {
  return Error();
}

if (devicesArray.isSome() && !devicesArray.get().is()) {
  if (!devicesArray.is()) {
return Error();
  }
  
  const vector& values =
devicesArray.get().as().values;

  ...
}
```

Can follow examples on docker/docker.cpp `Docker::Image::create()`.


- Gilbert Song


On July 5, 2016, 3:15 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49661/
> ---
> 
> (Updated July 5, 2016, 3:15 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This now contains an empty array when no devices are present.
> However, it appears that Docker previously used null entries
> to represent the same scenario.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp aad5380f5848f1b49d5524032f3b5b18c2aa1c56 
> 
> Diff: https://reviews.apache.org/r/49661/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="*DockerTest.*"
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 49615: Implemented 'shouldInject()' in the 'NvidiaVolume' component.

[Benjamin Mahler]

> On July 5, 2016, 10:42 p.m., Benjamin Mahler wrote:
> > src/slave/containerizer/mesos/isolators/gpu/volume.cpp, lines 387-393
> > 
> >
> > You don't need either of these checks
> 
> Kevin Klues wrote:
> Why don't I need these checks? The fields in the protobuf are optional.

.config() without the check will return a default object which won't have any 
labels


- Benjamin


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


On July 4, 2016, 10:09 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49615/
> ---
> 
> (Updated July 4, 2016, 10:09 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5401
> https://issues.apache.org/jira/browse/MESOS-5401
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We use the the `com.nvidia.volumes.needed` label from
> nvidia-docker to decide if we should inject the volume or not:
> 
> https://github.com/NVIDIA/nvidia-docker/wiki/Image-inspection
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/volume.cpp 
> 9aac3cc8622c21014de87576f84180d30ae3fadd 
>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
> 474040c89d69b50c051122d873c11a102b33c538 
> 
> Diff: https://reviews.apache.org/r/49615/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="" make -j check && 
> GTEST_FILTER="*NVIDIA_GPU_VolumeShouldInject" bin/mesos-tests.sh`
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49615: Implemented 'shouldInject()' in the 'NvidiaVolume' component.

[Kevin Klues]

> On July 5, 2016, 10:42 p.m., Benjamin Mahler wrote:
> > src/slave/containerizer/mesos/isolators/gpu/volume.cpp, lines 387-393
> > 
> >
> > You don't need either of these checks

Why don't I need these checks? The fields in the protobuf are optional.


- Kevin


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


On July 4, 2016, 10:09 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49615/
> ---
> 
> (Updated July 4, 2016, 10:09 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5401
> https://issues.apache.org/jira/browse/MESOS-5401
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We use the the `com.nvidia.volumes.needed` label from
> nvidia-docker to decide if we should inject the volume or not:
> 
> https://github.com/NVIDIA/nvidia-docker/wiki/Image-inspection
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/volume.cpp 
> 9aac3cc8622c21014de87576f84180d30ae3fadd 
>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
> 474040c89d69b50c051122d873c11a102b33c538 
> 
> Diff: https://reviews.apache.org/r/49615/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="" make -j check && 
> GTEST_FILTER="*NVIDIA_GPU_VolumeShouldInject" bin/mesos-tests.sh`
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49661: Ignore null entries for Docker inspect's 'HostConfig.Devices' field.

[Joseph Wu]

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



Do you think we should add a failure handler to the docker executor?

i.e. 
https://github.com/apache/mesos/blob/def697929f5029832cd2895da38565e52a9882ec/src/docker/executor.cpp#L178

```
inspect = docker->inspect(containerName, DOCKER_INSPECT_DELAY)
  .onFailed([](const std::string& message) {
LOG(INFO) << "Inspect failed with : " << message;
  })
```


src/docker/docker.cpp (line 241)


It won't be possible/straightfoward to make a protobuf schema for this 
method afterwards :(  The type is Null || Array  ):


- Joseph Wu


On July 5, 2016, 3:15 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49661/
> ---
> 
> (Updated July 5, 2016, 3:15 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This now contains an empty array when no devices are present.
> However, it appears that Docker previously used null entries
> to represent the same scenario.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp aad5380f5848f1b49d5524032f3b5b18c2aa1c56 
> 
> Diff: https://reviews.apache.org/r/49661/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="*DockerTest.*"
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 49661: Ignore null entries for Docker inspect's 'HostConfig.Devices' field.

[Kevin Klues]

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


Ship it!




Did you double check with the Centos 6 CI that it passes with this fix?

- Kevin Klues


On July 5, 2016, 10:15 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49661/
> ---
> 
> (Updated July 5, 2016, 10:15 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This now contains an empty array when no devices are present.
> However, it appears that Docker previously used null entries
> to represent the same scenario.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp aad5380f5848f1b49d5524032f3b5b18c2aa1c56 
> 
> Diff: https://reviews.apache.org/r/49661/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="*DockerTest.*"
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 49615: Implemented 'shouldInject()' in the 'NvidiaVolume' component.

[Benjamin Mahler]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/gpu/volume.cpp (lines 381 - 384)


Why duplicate the comment from the header in the .cpp file?



src/slave/containerizer/mesos/isolators/gpu/volume.cpp (lines 387 - 393)


You don't need either of these checks



src/slave/containerizer/mesos/isolators/gpu/volume.cpp (line 395)


why auto instead of `const Label&`?



src/slave/containerizer/mesos/isolators/gpu/volume.cpp (line 396)


You don't need the has_key check here



src/slave/containerizer/mesos/isolators/gpu/volume.cpp (lines 397 - 399)


I'm a bit confused by the comment, what is it used for exactly? As 
discussed, let's say that it is used as the volume name, which is unnecessary 
for us because we use the host path directly.



src/tests/containerizer/nvidia_gpu_isolator_tests.cpp (lines 488 - 512)


Seems we also need a false case here?


- Benjamin Mahler


On July 4, 2016, 10:09 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49615/
> ---
> 
> (Updated July 4, 2016, 10:09 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5401
> https://issues.apache.org/jira/browse/MESOS-5401
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We use the the `com.nvidia.volumes.needed` label from
> nvidia-docker to decide if we should inject the volume or not:
> 
> https://github.com/NVIDIA/nvidia-docker/wiki/Image-inspection
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/volume.cpp 
> 9aac3cc8622c21014de87576f84180d30ae3fadd 
>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
> 474040c89d69b50c051122d873c11a102b33c538 
> 
> Diff: https://reviews.apache.org/r/49615/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="" make -j check && 
> GTEST_FILTER="*NVIDIA_GPU_VolumeShouldInject" bin/mesos-tests.sh`
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Review Request 49661: Ignore null entries for Docker inspect's 'HostConfig.Devices' field.

[Benjamin Mahler]

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

Review request for mesos, Gilbert Song and Kevin Klues.


Repository: mesos


Description
---

This now contains an empty array when no devices are present.
However, it appears that Docker previously used null entries
to represent the same scenario.


Diffs
-

  src/docker/docker.cpp aad5380f5848f1b49d5524032f3b5b18c2aa1c56 

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


Testing
---

sudo ./bin/mesos-tests.sh --gtest_filter="*DockerTest.*"


Thanks,

Benjamin Mahler

Re: Review Request 49649: Libprocess: Fixed 'make dist'.

[Till Toenshoff]

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


Ship it!




Ship It!

- Till Toenshoff


On July 5, 2016, 8:21 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49649/
> ---
> 
> (Updated July 5, 2016, 8:21 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-5416
> https://issues.apache.org/jira/browse/MESOS-5416
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 41b77787f67177ec2debb2c7f850b653efbb16db 
> 
> Diff: https://reviews.apache.org/r/49649/diff/
> 
> 
> Testing
> ---
> 
> make dist
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 49648: Stout: Fixed 'make dist'.

[Till Toenshoff]

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


Ship it!




Ship It!

- Till Toenshoff


On July 5, 2016, 8:21 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49648/
> ---
> 
> (Updated July 5, 2016, 8:21 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-5416
> https://issues.apache.org/jira/browse/MESOS-5416
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/Makefile.am 44ce5c49b37094a21b7503c3ca290f53db432852 
> 
> Diff: https://reviews.apache.org/r/49648/diff/
> 
> 
> Testing
> ---
> 
> make dist
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 49639: Added test for sending framework information after master failover.

[Vinod Kone]

> On July 5, 2016, 6:29 p.m., Vinod Kone wrote:
> > src/tests/master_slave_reconciliation_tests.cpp, line 628
> > 
> >
> > Is it possible to write a test that verifies that orphaned tasks are 
> > authorized?

i'll commmit this chain to avoid rebasing further. please write the new test in 
a follow up patch.


- Vinod


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


On July 5, 2016, 3:05 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49639/
> ---
> 
> (Updated July 5, 2016, 3:05 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5757
> https://issues.apache.org/jira/browse/MESOS-5757
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We test that the FrameworkInfos for running frameworks are send
>  from the agent to the master when an agent reregisters.
> 
> 
> Diffs
> -
> 
>   src/tests/master_slave_reconciliation_tests.cpp 
> fc02ca64c3efcc4d7b7f52d9208fe15a1d60ca44 
> 
> Diff: https://reviews.apache.org/r/49639/diff/
> 
> 
> Testing
> ---
> 
> (sudo) make check on various platforms
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 49607: Added support for recovered frameworks.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On July 5, 2016, 8:15 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49607/
> ---
> 
> (Updated July 5, 2016, 8:15 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously after a master failover while agents
> were reregistering we might not have access
> to all FrameworkInfo (which would first be available
> once the Framework reregistered). This patch
> adds support that once a framework reregisters
> we also keep track of recovered FrameworkInfos.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 
>   src/master/master.cpp 78a8889313179b4509a6657e6c61d9f6d3bb99c0 
>   src/messages/messages.proto 6d7eccc5501b3df010d981e09d0654e815da551f 
>   src/slave/slave.cpp da643e6e50b2f313705d2f862c961291aa5d2f22 
> 
> Diff: https://reviews.apache.org/r/49607/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:
> > 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.
> 
> Jiang Yan Xu wrote:
> Hey Jie I didn't realize this, but it seems like Environment already 
> takes care of this (you wrote this part): 
> https://github.com/apache/mesos/blob/ada24cb6b29cc74e30ce31550459a6b14d409c96/src/tests/environment.cpp#L847-L861
> 
> And `umount -l` does cover this case here: so we don't even need a 
> separate test fixture anymore.
> 
> Of course we should verify this.

Oh, right! Yeah, we probably can get rid of the test fixture! Can you test it?


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

Review Request 49660: Added #allocator slack channel to Resource Allocation group.

[Guangya Liu]

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

Review request for mesos, Benjamin Mahler and Vinod Kone.


Repository: mesos


Description
---

Added #allocator slack channel to Resource Allocation group.


Diffs
-

  docs/working-groups.md b1b696fa7907040374919a0e43c76b2deedfb2c3 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 49649: Libprocess: Fixed 'make dist'.

[Till Toenshoff]

> On July 5, 2016, 9:42 p.m., Till Toenshoff wrote:
> > I am still seeing `make distcheck` failing.
> > 
> > ```
> > $ make dist
> > make  dist-gzip am__post_remove_distdir='@:'
> > make[1]: Entering directory '/home/till/scratchpad/mesos/build'
> > if test -d "mesos-1.0.0"; then find "mesos-1.0.0" -type d ! -perm -200 
> > -exec chmod u+w {} ';' && rm -rf "mesos-1.0.0" || { sleep 5 && rm -rf 
> > "mesos-1.0.0"; }; else :; fi
> > test -d "mesos-1.0.0" || mkdir "mesos-1.0.0"
> >  (cd 3rdparty && make  top_distdir=../mesos-1.0.0 
> > distdir=../mesos-1.0.0/3rdparty \
> >  am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: 
> > distdir)
> > make[2]: Entering directory '/home/till/scratchpad/mesos/build/3rdparty'
> >  (cd stout && make  top_distdir=../../mesos-1.0.0 
> > distdir=../../mesos-1.0.0/3rdparty/stout \
> >  am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: 
> > distdir)
> > make[3]: Entering directory 
> > '/home/till/scratchpad/mesos/build/3rdparty/stout'
> >  (cd 3rdparty && make  top_distdir=../../../mesos-1.0.0 
> > distdir=../../../mesos-1.0.0/3rdparty/stout/3rdparty \
> >  am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: 
> > distdir)
> > /bin/sh: line 14: cd: 3rdparty: No such file or directory
> > Makefile:1542: recipe for target 'distdir' failed
> > make[3]: *** [distdir] Error 1
> > make[3]: Leaving directory 
> > '/home/till/scratchpad/mesos/build/3rdparty/stout'
> > Makefile:927: recipe for target 'distdir' failed
> > make[2]: *** [distdir] Error 1
> > make[2]: Leaving directory '/home/till/scratchpad/mesos/build/3rdparty'
> > Makefile:862: recipe for target 'distdir' failed
> > make[1]: *** [distdir] Error 1
> > make[1]: Leaving directory '/home/till/scratchpad/mesos/build'
> > Makefile:961: recipe for target 'dist' failed
> > make: *** [dist] Error 2
> > ```

Ow - actually forget this remark - nothing to see here :)


- Till


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


On July 5, 2016, 8:21 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49649/
> ---
> 
> (Updated July 5, 2016, 8:21 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-5416
> https://issues.apache.org/jira/browse/MESOS-5416
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 41b77787f67177ec2debb2c7f850b653efbb16db 
> 
> Diff: https://reviews.apache.org/r/49649/diff/
> 
> 
> Testing
> ---
> 
> make dist
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 49656: Added ability to stringify cgroups::devices::Entry::Selector::Type.

[Benjamin Mahler]

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


Ship it!





src/linux/cgroups.cpp (lines 2425 - 2439)


Since we're relying on the switch catching all cases (otherwise we should 
get a compiler error), perhaps we should leverage UNREACHABLE here?

```
  switch (type) {
case Entry::Selector::Type::ALL:   return stream << "a";
case Entry::Selector::Type::BLOCK: return stream << "b";
case Entry::Selector::Type::CHARACTER: return stream << "c";
// We omit the default case because we assume -Wswitch
// will trigger a compile-time error if a case is missed.
  }

  UNREACHABLE();
```


- Benjamin Mahler


On July 5, 2016, 9:14 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49656/
> ---
> 
> (Updated July 5, 2016, 9:14 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-3368
> https://issues.apache.org/jira/browse/MESOS-3368
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added ability to stringify cgroups::devices::Entry::Selector::Type.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 5f4010734ed9e3295dcc3a4390123e4f4ce99c16 
>   src/linux/cgroups.cpp 95ceb373ca4d961c402f9936f31cda1a25c60e87 
> 
> Diff: https://reviews.apache.org/r/49656/diff/
> 
> 
> Testing
> ---
> 
> make check (on linux)
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49448: Added testcases for LIST_FILES call.

[Anand Mazumdar]

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




src/tests/api_tests.cpp (line 537)


Kill this after separating this test into 2.



src/tests/api_tests.cpp (lines 546 - 551)


Indent incorrect.



src/tests/api_tests.cpp (lines 554 - 568)


It's a bit odd that the test header comment says that it tries to verify if 
it can retrieve the file listing. :-)

Let's move this into a separate test rather than merging it here.

Ditto for the agent test.


- Anand Mazumdar


On July 2, 2016, 7:50 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49448/
> ---
> 
> (Updated July 2, 2016, 7:50 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5514
> https://issues.apache.org/jira/browse/MESOS-5514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added testcases for LIST_FILES call.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp b3127b716480223a6f23b7908bf6bc1808120f80 
> 
> Diff: https://reviews.apache.org/r/49448/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo 
> GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
>  make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49658: Updated elfio-3.1.patch to fix alignment bug properly.

[Kevin Klues]

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

(Updated July 5, 2016, 9:25 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Added bug and testing section


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


Repository: mesos


Description
---

The previous patch incorrectly fixed the alignment of an ELF note to 4
bytes (even for 64 bit ELF files). This happened to be OK for our
specific case of parsing '.note.ABI-tag' sections. However, the
alignment for notes in a 64 bit ELF section should be 8-byte aligned
(as the original code attempts to do). The problem with the original
code, however, is that it performs the alignment only on the name
string inside the note, rather than the entire note object itself.
This commit fixes this alignment issue.


Diffs
-

  3rdparty/elfio-3.1.patch f8c20e92a0ec12e8a2f1d8e8ec54eac78ced707d 

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


Testing (updated)
---

make check


Thanks,

Kevin Klues