date:20161130


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



Bad patch!

Reviews applied: [54241, 54229, 53974, 53939, 54148, 54147, 54053]

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

Error:
2016-12-01 07:25:39 URL:https://reviews.apache.org/r/54148/diff/raw/ 
[13977/13977] -> "54148.patch" [1]
error: patch failed: src/slave/containerizer/mesos/io/switchboard.hpp:63
error: src/slave/containerizer/mesos/io/switchboard.hpp: patch does not apply
error: patch failed: src/slave/containerizer/mesos/io/switchboard.cpp:114
error: src/slave/containerizer/mesos/io/switchboard.cpp: patch does not apply

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

- Mesos ReviewBot


On Dec. 1, 2016, 4:13 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54241/
> ---
> 
> (Updated Dec. 1, 2016, 4:13 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6467
> https://issues.apache.org/jira/browse/MESOS-6467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated IOSwitchboard to block IO until connected for DEBUG containers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.hpp 
> d3ab52d98ade067ccec63f3539073c152840099f 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> ae7a1fe3e24d5dc18269d9da951493700ee957d0 
> 
> Diff: https://reviews.apache.org/r/54241/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check
> sudo src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 54196: Added API handler for LAUNCH_NESTED_CONTAINER_SESSION.


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

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


Review request for mesos and Anand Mazumdar.


Changes
---

updated for signature change.


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


Repository: mesos


Description
---

In addition to launching the nested container the API handler
ensures that the container is destroyed if the connection breaks.


Diffs (updated)
-

  src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
  src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 
  src/slave/validation.cpp 46e84361244394196ee5d431c1da34686d6c938a 
  src/tests/api_tests.cpp b2a70f3174824d3cb241a098d96e4d63a390f8bc 

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


Testing
---

make check

Added a basic test for now that tests the failure case. Will be adding more 
tests in subsequent reviews.


Thanks,

Vinod Kone

Re: Review Request 54193: Refactored `Slave::Http::launchNestedContainer()`.


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

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


Review request for mesos and Anand Mazumdar.


Changes
---

updated for signature change. NNFR.


Summary (updated)
-

Refactored `Slave::Http::launchNestedContainer()`.


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


Repository: mesos


Description
---

This so that code can be reused for `launchNestedContainerSession()`
that will be added later. No functional change.


Diffs (updated)
-

  src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
  src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 

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


Testing
---

make check


Thanks,

Vinod Kone

Review Request 54245: Updated signatures of v1 agent API handlers.


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

Review request for mesos and Anand Mazumdar.


Repository: mesos


Description
---

Moved the position of `contentType` argument in the handlers and
renamed it to `acceptType` to be consistent.


Diffs
-

  src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
  src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 

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


Testing
---

make check


Thanks,

Vinod Kone

Re: Review Request 54218: Made `serialize()`/`deserialize()` handle streaming content types.


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

(Updated Dec. 1, 2016, 6:39 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments, NNFR


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


Repository: mesos


Description
---

The `serialize()` helper can now encode a record in 'Record-IO'
format for streaming content type. Also, `deserialize()` now
handles a streaming content type by just parsing it the body
into a corresponding protobuf.


Diffs (updated)
-

  src/common/http.hpp 378208be58d0cd141ff60a6d25a173f2793870dc 
  src/common/http.cpp 42af33094150001fa6741effb6b0aee1cde3c109 

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


Testing
---

make check (these would be tested later in the chain)


Thanks,

Anand Mazumdar

Re: Review Request 54238: Added `getContainerIOSwitchboardAddress` helper to containerizer paths.


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



Bad patch!

Reviews applied: [54238, 54148, 54147, 54053]

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

Error:
2016-12-01 06:35:07 URL:https://reviews.apache.org/r/54148/diff/raw/ 
[13977/13977] -> "54148.patch" [1]
error: patch failed: src/slave/containerizer/mesos/io/switchboard.hpp:63
error: src/slave/containerizer/mesos/io/switchboard.hpp: patch does not apply
error: patch failed: src/slave/containerizer/mesos/io/switchboard.cpp:114
error: src/slave/containerizer/mesos/io/switchboard.cpp: patch does not apply

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

- Mesos ReviewBot


On Dec. 1, 2016, 1:45 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54238/
> ---
> 
> (Updated Dec. 1, 2016, 1:45 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6467
> https://issues.apache.org/jira/browse/MESOS-6467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `getContainerIOSwitchboardAddress` helper to containerizer paths.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/paths.hpp 
> 44df6c0b74e38140a9e6037c103a7f8c6d16 
>   src/slave/containerizer/mesos/paths.cpp 
> 422fb80ced8e6435a714d32e3ee9b72874e2034d 
> 
> Diff: https://reviews.apache.org/r/54238/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 54242: Added a test for the `ATTACH_CONTAINER_INPUT` call on the Agent API.


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


Ship it!




Ship It!

- Vinod Kone


On Dec. 1, 2016, 4:44 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54242/
> ---
> 
> (Updated Dec. 1, 2016, 4:44 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6472
> https://issues.apache.org/jira/browse/MESOS-6472
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test for the `ATTACH_CONTAINER_INPUT` call on the Agent API.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp b2a70f3174824d3cb241a098d96e4d63a390f8bc 
> 
> Diff: https://reviews.apache.org/r/54242/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 54243: Fixed the attach output container test to use the mock containerizer.


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


Ship it!




Thanks for taking care of this.

- Vinod Kone


On Dec. 1, 2016, 4:44 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54243/
> ---
> 
> (Updated Dec. 1, 2016, 4:44 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Otherwise, the test would fail when we add support to the
> containerizer to support `attach()`.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp b2a70f3174824d3cb241a098d96e4d63a390f8bc 
> 
> Diff: https://reviews.apache.org/r/54243/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 54221: Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.


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


Ship it!




Ship It!

- Vinod Kone


On Dec. 1, 2016, 5:43 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54221/
> ---
> 
> (Updated Dec. 1, 2016, 5:43 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: MESOS-6472
> https://issues.apache.org/jira/browse/MESOS-6472
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
>   src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 
> 
> Diff: https://reviews.apache.org/r/54221/diff/
> 
> 
> Testing
> ---
> 
> make check (tests would be added soon and can't be committed without them)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 54221: Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.



> On Dec. 1, 2016, 3:22 a.m., Vinod Kone wrote:
> > src/slave/http.cpp, line 2231
> > 
> >
> > This is different from how we prolong the lifetime of `connection` in 
> > `attachContainerOutput`. can we make them consistent?
> 
> Anand Mazumdar wrote:
> hmm, the scenarios looked pretty different and hence I did not make them 
> consistent. In the previous scenario, we had already received the `response` 
> from the `Connection` object and were streaming it back to the client. Hence, 
> it was fine to just capture it in `transform()` to prolong the life time. 
> However, here, the switch board would only _start_ streaming the response 
> after it has received the streaming request fully.
> 
> Vinod Kone wrote:
> I see the difference between the 2 scenarios, but I don't completely get 
> why `connection` cannot be captured in the `transform` lambda here too? Are 
> you saying that doesn't work?
> 
> Anand Mazumdar wrote:
> Yes, it won't work in this case. `transform()` would complete as soon as 
> we have completed streaming the `Request` to the switch board but before 
> waiting for the response. Hence, we need this alternate way to prolong its 
> lifetime.

aha. makes sense. thanks.


- Vinod


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


On Dec. 1, 2016, 5:43 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54221/
> ---
> 
> (Updated Dec. 1, 2016, 5:43 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: MESOS-6472
> https://issues.apache.org/jira/browse/MESOS-6472
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
>   src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 
> 
> Diff: https://reviews.apache.org/r/54221/diff/
> 
> 
> Testing
> ---
> 
> make check (tests would be added soon and can't be committed without them)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 53690: Add namespaces/ipc documentation.


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



Patch looks great!

Reviews applied: [53688, 53689, 53690]

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 Dec. 1, 2016, 12:25 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53690/
> ---
> 
> (Updated Dec. 1, 2016, 12:25 a.m.)
> 
> 
> Review request for mesos, Avinash Gautam and Jie Yu.
> 
> 
> Bugs: MESOS-6557
> https://issues.apache.org/jira/browse/MESOS-6557
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add namespaces/ipc documentation.
> 
> 
> Diffs
> -
> 
>   docs/mesos-containerizer.md 2bff35f6361f760a9001a249d2c01bbbc9e72932 
> 
> Diff: https://reviews.apache.org/r/53690/diff/
> 
> 
> Testing
> ---
> 
> Make check on Fedora 24.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 54221: Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.


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

(Updated Dec. 1, 2016, 5:43 a.m.)


Review request for mesos, Kevin Klues and Vinod Kone.


Changes
---

Review comments


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


Repository: mesos


Description
---

Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.


Diffs (updated)
-

  src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
  src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 

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


Testing
---

make check (tests would be added soon and can't be committed without them)


Thanks,

Anand Mazumdar

Re: Review Request 54115: Added an http::serve abstraction.

2016-11-30 Thread Benjamin Hindman


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




3rdparty/libprocess/src/http.cpp (lines 1558 - 1566)


Return an InternalServerError here instead.



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


Close the reader onAny after this stream, not above in the then.


- Benjamin Hindman


On Nov. 28, 2016, 6:20 a.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54115/
> ---
> 
> (Updated Nov. 28, 2016, 6:20 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an http::serve abstraction.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> a684e09c8353112a0656b7e899a469c1e022e93b 
>   3rdparty/libprocess/src/http.cpp 3f16f293a5c5cd0b31a85efe94cb6f8019543d45 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d41929a9c8b2469c10b9e31985c447076c1684dc 
> 
> Diff: https://reviews.apache.org/r/54115/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 54221: Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.


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

(Updated Dec. 1, 2016, 5:34 a.m.)


Review request for mesos, Kevin Klues and Vinod Kone.


Changes
---

Made the outer lambda mutable. Surprisingly, clang does not catch it but gcc 
barfs out.


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


Repository: mesos


Description
---

Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.


Diffs (updated)
-

  src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
  src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 

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


Testing
---

make check (tests would be added soon and can't be committed without them)


Thanks,

Anand Mazumdar

Re: Review Request 54221: Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.



> On Dec. 1, 2016, 3:22 a.m., Vinod Kone wrote:
> > src/slave/http.cpp, line 2231
> > 
> >
> > This is different from how we prolong the lifetime of `connection` in 
> > `attachContainerOutput`. can we make them consistent?
> 
> Anand Mazumdar wrote:
> hmm, the scenarios looked pretty different and hence I did not make them 
> consistent. In the previous scenario, we had already received the `response` 
> from the `Connection` object and were streaming it back to the client. Hence, 
> it was fine to just capture it in `transform()` to prolong the life time. 
> However, here, the switch board would only _start_ streaming the response 
> after it has received the streaming request fully.
> 
> Vinod Kone wrote:
> I see the difference between the 2 scenarios, but I don't completely get 
> why `connection` cannot be captured in the `transform` lambda here too? Are 
> you saying that doesn't work?

Yes, it won't work in this case. `transform()` would complete as soon as we 
have completed streaming the `Request` to the switch board but before waiting 
for the response. Hence, we need this alternate way to prolong its lifetime.


- Anand


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


On Dec. 1, 2016, 3:59 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54221/
> ---
> 
> (Updated Dec. 1, 2016, 3:59 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: MESOS-6472
> https://issues.apache.org/jira/browse/MESOS-6472
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
>   src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 
> 
> Diff: https://reviews.apache.org/r/54221/diff/
> 
> 
> Testing
> ---
> 
> make check (tests would be added soon and can't be committed without them)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 54221: Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.



> On Dec. 1, 2016, 3:22 a.m., Vinod Kone wrote:
> > src/slave/http.cpp, lines 2194-2195
> > 
> >
> > why do this here instead of inside the lambda?
> 
> Anand Mazumdar wrote:
> C++11 does not support moving objects when capturing them in a lambda. 
> So, it was done outside the lambda.

I see. Can you add a comment saying so for posterity?


> On Dec. 1, 2016, 3:22 a.m., Vinod Kone wrote:
> > src/slave/http.cpp, line 2231
> > 
> >
> > This is different from how we prolong the lifetime of `connection` in 
> > `attachContainerOutput`. can we make them consistent?
> 
> Anand Mazumdar wrote:
> hmm, the scenarios looked pretty different and hence I did not make them 
> consistent. In the previous scenario, we had already received the `response` 
> from the `Connection` object and were streaming it back to the client. Hence, 
> it was fine to just capture it in `transform()` to prolong the life time. 
> However, here, the switch board would only _start_ streaming the response 
> after it has received the streaming request fully.

I see the difference between the 2 scenarios, but I don't completely get why 
`connection` cannot be captured in the `transform` lambda here too? Are you 
saying that doesn't work?


- Vinod


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


On Dec. 1, 2016, 3:59 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54221/
> ---
> 
> (Updated Dec. 1, 2016, 3:59 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: MESOS-6472
> https://issues.apache.org/jira/browse/MESOS-6472
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
>   src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 
> 
> Diff: https://reviews.apache.org/r/54221/diff/
> 
> 
> Testing
> ---
> 
> make check (tests would be added soon and can't be committed without them)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 54242: Added a test for the `ATTACH_CONTAINER_INPUT` call on the Agent API.


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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Added a test for the `ATTACH_CONTAINER_INPUT` call on the Agent API.


Diffs
-

  src/tests/api_tests.cpp b2a70f3174824d3cb241a098d96e4d63a390f8bc 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 54243: Fixed the attach output container test to use the mock containerizer.


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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Otherwise, the test would fail when we add support to the
containerizer to support `attach()`.


Diffs
-

  src/tests/api_tests.cpp b2a70f3174824d3cb241a098d96e4d63a390f8bc 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 54229: Enabled the IO switchboard server by default for tests.


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

(Updated Dec. 1, 2016, 4:13 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Enabled the IO switchboard server by default for tests.


Diffs
-

  src/tests/mesos.cpp 787c9b636e0e2e712be62407cff69d854b8eecbe 

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


Testing
---

make check


Thanks,

Kevin Klues

Re: Review Request 54241: Updated IOSwitchboard to block IO until connected for DEBUG containers.


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

(Updated Dec. 1, 2016, 4:13 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Updated IOSwitchboard to block IO until connected for DEBUG containers.


Diffs
-

  src/slave/containerizer/mesos/io/switchboard.hpp 
d3ab52d98ade067ccec63f3539073c152840099f 
  src/slave/containerizer/mesos/io/switchboard.cpp 
ae7a1fe3e24d5dc18269d9da951493700ee957d0 

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


Testing
---

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


Thanks,

Kevin Klues

Review Request 54241: Updated IOSwitchboard to block IO until connected for DEBUG containers.


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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Updated IOSwitchboard to block IO until connected for DEBUG containers.


Diffs
-

  src/slave/containerizer/mesos/io/switchboard.hpp 
d3ab52d98ade067ccec63f3539073c152840099f 
  src/slave/containerizer/mesos/io/switchboard.cpp 
ae7a1fe3e24d5dc18269d9da951493700ee957d0 

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


Testing
---

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


Thanks,

Kevin Klues

Re: Review Request 54221: Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.



> On Dec. 1, 2016, 3:22 a.m., Vinod Kone wrote:
> > can you add a simple test for this change as discussed. comprehensive tests 
> > can come later.

yep, on it :-)


> On Dec. 1, 2016, 3:22 a.m., Vinod Kone wrote:
> > src/slave/http.cpp, line 2231
> > 
> >
> > This is different from how we prolong the lifetime of `connection` in 
> > `attachContainerOutput`. can we make them consistent?

hmm, the scenarios looked pretty different and hence I did not make them 
consistent. In the previous scenario, we had already received the `response` 
from the `Connection` object and were streaming it back to the client. Hence, 
it was fine to just capture it in `transform()` to prolong the life time. 
However, here, the switch board would only _start_ streaming the response after 
it has received the streaming request fully.


> On Dec. 1, 2016, 3:22 a.m., Vinod Kone wrote:
> > src/slave/http.cpp, lines 2194-2195
> > 
> >
> > why do this here instead of inside the lambda?

C++11 does not support moving objects when capturing them in a lambda. So, it 
was done outside the lambda.


> On Dec. 1, 2016, 3:22 a.m., Vinod Kone wrote:
> > src/slave/http.cpp, line 519
> > 
> >
> > why is `principal` not being passed?
> > 
> > also the signatures of the API handlers are a bit inconsistent now
> > 
> > ```
> > # most of the handlers are
> > foo(call, principal, contentType)
> > 
> > attachContainerInput(contentType, acceptType, call, reader)
> > 
> > attachContainerOutput(call, contentType, acceptType, principal)
> > 
> > launchContainerOutput(call, contentType, acceptType, principal)
> > 
> > ```
> > 
> > i propose we do
> > ```
> > # NOTE: s/contentType/acceptType/ for consistency
> > foo(call, acceptType, principal)
> > 
> > attachContainerInput(call, reader, contentType, acceptType, principal)
> > 
> > attachContainerOutput(call, contentType, acceptType, principal)
> > 
> > launchContainerOutput(call, contentType, acceptType, principal)
> > 
> > ```
> > 
> > i can do the change for `foo`. can you change `attachContainerInput` in 
> > this patch?

+1 to the proposal except don't pass `principal` when its not needed. Some 
calls might not be interested in AuthZ at all.

I did not pass `principal` since we did not have AuthZ handling for this 
handler. Would add it since we know that it's already being worked on.


- Anand


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


On Dec. 1, 2016, 3:59 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54221/
> ---
> 
> (Updated Dec. 1, 2016, 3:59 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: MESOS-6472
> https://issues.apache.org/jira/browse/MESOS-6472
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
>   src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 
> 
> Diff: https://reviews.apache.org/r/54221/diff/
> 
> 
> Testing
> ---
> 
> make check (tests would be added soon and can't be committed without them)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 54221: Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.


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

(Updated Dec. 1, 2016, 3:59 a.m.)


Review request for mesos, Kevin Klues and Vinod Kone.


Changes
---

Review comments


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


Repository: mesos


Description
---

Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.


Diffs (updated)
-

  src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
  src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 

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


Testing
---

make check (tests would be added soon and can't be committed without them)


Thanks,

Anand Mazumdar

Re: Review Request 54229: Enabled the IO switchboard server by default for tests.


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



Bad patch!

Reviews applied: [54229, 53974, 53939, 54148, 54147, 54053]

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

Error:
2016-12-01 03:48:55 URL:https://reviews.apache.org/r/54148/diff/raw/ 
[13977/13977] -> "54148.patch" [1]
error: patch failed: src/slave/containerizer/mesos/io/switchboard.hpp:63
error: src/slave/containerizer/mesos/io/switchboard.hpp: patch does not apply
error: patch failed: src/slave/containerizer/mesos/io/switchboard.cpp:114
error: src/slave/containerizer/mesos/io/switchboard.cpp: patch does not apply

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

- Mesos ReviewBot


On Nov. 30, 2016, 10:26 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54229/
> ---
> 
> (Updated Nov. 30, 2016, 10:26 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled the IO switchboard server by default for tests.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.cpp 787c9b636e0e2e712be62407cff69d854b8eecbe 
> 
> Diff: https://reviews.apache.org/r/54229/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 54218: Made `serialize()`/`deserialize()` handle streaming content types.


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


Fix it, then Ship it!





src/common/http.hpp (line 53)


s/the `message` argument/`message`/



src/common/http.hpp (line 62)


s/the `body` argument/`body`/


- Vinod Kone


On Dec. 1, 2016, 3:21 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54218/
> ---
> 
> (Updated Dec. 1, 2016, 3:21 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6472
> https://issues.apache.org/jira/browse/MESOS-6472
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `serialize()` helper can now encode a record in 'Record-IO'
> format for streaming content type. Also, `deserialize()` now
> handles a streaming content type by just parsing it the body
> into a corresponding protobuf.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 378208be58d0cd141ff60a6d25a173f2793870dc 
>   src/common/http.cpp 42af33094150001fa6741effb6b0aee1cde3c109 
> 
> Diff: https://reviews.apache.org/r/54218/diff/
> 
> 
> Testing
> ---
> 
> make check (these would be tested later in the chain)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 54220: Made the agent's `api/v1` handler handle request streaming.


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


Ship it!




Ship It!

- Vinod Kone


On Dec. 1, 2016, 3:13 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54220/
> ---
> 
> (Updated Dec. 1, 2016, 3:13 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6472
> https://issues.apache.org/jira/browse/MESOS-6472
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This lays the ground work for adding the attach container input
> call later.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 378208be58d0cd141ff60a6d25a173f2793870dc 
>   src/common/http.cpp 42af33094150001fa6741effb6b0aee1cde3c109 
>   src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
>   src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 
> 
> Diff: https://reviews.apache.org/r/54220/diff/
> 
> 
> Testing
> ---
> 
> make check (tests would be added later in the chain)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 54221: Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.


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



can you add a simple test for this change as discussed. comprehensive tests can 
come later.


src/slave/http.cpp (line 518)


why is `principal` not being passed?

also the signatures of the API handlers are a bit inconsistent now

```
# most of the handlers are
foo(call, principal, contentType)

attachContainerInput(contentType, acceptType, call, reader)

attachContainerOutput(call, contentType, acceptType, principal)

launchContainerOutput(call, contentType, acceptType, principal)

```

i propose we do
```
# NOTE: s/contentType/acceptType/ for consistency
foo(call, acceptType, principal)

attachContainerInput(call, reader, contentType, acceptType, principal)

attachContainerOutput(call, contentType, acceptType, principal)

launchContainerOutput(call, contentType, acceptType, principal)

```

i can do the change for `foo`. can you change `attachContainerInput` in 
this patch?



src/slave/http.cpp (line 2172)


s/reader/decoder/

we have been naming recordio::Reader as `decoder` when it conflicts with 
the naming of Pipe::Reder.



src/slave/http.cpp (line 2180)


if you change the above, this will be 

s/reader_/reader/



src/slave/http.cpp (line 2190)


Can you expand on why you need to write the first record manually, for 
posterity?



src/slave/http.cpp (lines 2193 - 2194)


why do this here instead of inside the lambda?



src/slave/http.cpp (line 2230)


This is different from how we prolong the lifetime of `connection` in 
`attachContainerOutput`. can we make them consistent?


- Vinod Kone


On Nov. 30, 2016, 10:09 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54221/
> ---
> 
> (Updated Nov. 30, 2016, 10:09 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: MESOS-6472
> https://issues.apache.org/jira/browse/MESOS-6472
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
>   src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 
> 
> Diff: https://reviews.apache.org/r/54221/diff/
> 
> 
> Testing
> ---
> 
> make check (tests would be added soon and can't be committed without them)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 54218: Made `serialize()`/`deserialize()` handle streaming content types.


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

(Updated Dec. 1, 2016, 3:21 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments


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


Repository: mesos


Description
---

The `serialize()` helper can now encode a record in 'Record-IO'
format for streaming content type. Also, `deserialize()` now
handles a streaming content type by just parsing it the body
into a corresponding protobuf.


Diffs (updated)
-

  src/common/http.hpp 378208be58d0cd141ff60a6d25a173f2793870dc 
  src/common/http.cpp 42af33094150001fa6741effb6b0aee1cde3c109 

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


Testing
---

make check (these would be tested later in the chain)


Thanks,

Anand Mazumdar

Re: Review Request 54220: Made the agent's `api/v1` handler handle request streaming.


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

(Updated Dec. 1, 2016, 3:13 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments.


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


Repository: mesos


Description
---

This lays the ground work for adding the attach container input
call later.


Diffs (updated)
-

  src/common/http.hpp 378208be58d0cd141ff60a6d25a173f2793870dc 
  src/common/http.cpp 42af33094150001fa6741effb6b0aee1cde3c109 
  src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
  src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 

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


Testing
---

make check (tests would be added later in the chain)


Thanks,

Anand Mazumdar

Re: Review Request 54197: Introduced Resource.AllocationInfo.

2016-11-30 Thread Jay Guo


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

(Updated Dec. 1, 2016, 2:47 a.m.)


Review request for mesos, Guangya Liu and Qiang Zhang.


Changes
---

addressed guangya's comment


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


Repository: mesos


Description (updated)
---

To determine which role resources are allocated to, we now need to
store allocation information within the Resources. This is part of
the effort to support multi-role frameworks. See MESOS-6633 for more
details.


Diffs (updated)
-

  include/mesos/mesos.proto cbfcd8a666e0b4a486f6dcd9e8356c9d5a1cea05 
  include/mesos/v1/mesos.proto e4f269b96ffc06d040604d1d6fead01e4aca2c13 

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


Testing
---


Thanks,

Jay Guo

Re: Review Request 54198: Added Resource.Allocation in Offer.

2016-11-30 Thread Jay Guo


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

(Updated Dec. 1, 2016, 2:47 a.m.)


Review request for mesos, Guangya Liu and Qian Zhang.


Changes
---

addressed guangya's comment


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


Repository: mesos


Description
---

Added Resource.Allocation in Offer protobuf message to indicate a
single role per offer. To preserve the offer semantics between a
single multi-role scheduler and multiple single-role schedulers,
we would like to ensure that an offer only contains resources
allocated to a single role.


Diffs (updated)
-

  include/mesos/mesos.proto cbfcd8a666e0b4a486f6dcd9e8356c9d5a1cea05 
  include/mesos/v1/mesos.proto e4f269b96ffc06d040604d1d6fead01e4aca2c13 

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


Testing
---


Thanks,

Jay Guo

Review Request 54069: Changed master to validate roles instead of just role.

2016-11-30 Thread Jay Guo


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

Review request for mesos.


Repository: mesos


Description
---

Master should check role, roles and MULTI_ROLE in FrameworkInfo and
reject subscription if it is invalid. A test case is added to ensure
this behavior.


Diffs
-


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


Testing
---


Thanks,

Jay Guo

Re: Review Request 54232: Shutdown tasks of completed frameworks on agent re-registration.


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



Patch looks great!

Reviews applied: [53885, 53886, 53887, 53888, 53889, 53890, 53891, 53892, 
53893, 53894, 53895, 53896, 53897, 54177, 54178, 54179, 54180, 54181, 54182, 
54183, 54232]

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 Nov. 30, 2016, 10:25 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54232/
> ---
> 
> (Updated Nov. 30, 2016, 10:25 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6602
> https://issues.apache.org/jira/browse/MESOS-6602
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, if a framework completed (e.g., due to a teardown operation
> or framework shutdown), any framework tasks running on partitioned
> agents would not be shutdown when the agent re-registered. For tasks
> that are not partition-aware, the task would be shutdown on agent
> re-registration anyway. But for partition-aware tasks, this could lead
> to orphan tasks.
> 
> Fix this by changing the master to shutdown such tasks when the agent
> reregisters.
> 
> Note that if the master fails over between the time the framework
> completes and a partitioned agent re-registers, any framework tasks
> running on the agent will NOT be shutdown. This is a known bug; fixing
> it requires persisting the framework shutdown operation to the registry
> (MESOS-1719).
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 877ca9010d0d6efc97f3d71fbd27272a255409d0 
>   src/master/master.cpp e03a2e8025943825a2902102c43dc0eb66bacb6a 
>   src/tests/partition_tests.cpp 5a0d4bd2de6a5aa0e9fdf0d34cd10d16fd4e34a1 
> 
> Diff: https://reviews.apache.org/r/54232/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 54167: Removed cyclic header dependency in libprocess.


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


Ship it!




Ship It!

- Benjamin Mahler


On Nov. 29, 2016, 3:59 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54167/
> ---
> 
> (Updated Nov. 29, 2016, 3:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Benjamin Mahler.
> 
> 
> Bugs: MESOS-6647
> https://issues.apache.org/jira/browse/MESOS-6647
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `executor.hpp` doesn't need anything from `defer.hpp` but defines a macro 
> that is used in it.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/executor.hpp 
> 48c1b34b6bd117653c1728321cf96b909e535964 
> 
> Diff: https://reviews.apache.org/r/54167/diff/
> 
> 
> Testing
> ---
> 
> ```
> make && make check
> ```
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 54115: Added an http::serve abstraction.


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


Ship it!




Modulo comments.

- Benjamin Mahler


On Nov. 28, 2016, 6:20 a.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54115/
> ---
> 
> (Updated Nov. 28, 2016, 6:20 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an http::serve abstraction.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> a684e09c8353112a0656b7e899a469c1e022e93b 
>   3rdparty/libprocess/src/http.cpp 3f16f293a5c5cd0b31a85efe94cb6f8019543d45 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d41929a9c8b2469c10b9e31985c447076c1684dc 
> 
> Diff: https://reviews.apache.org/r/54115/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 54115: Added an http::serve abstraction.


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




3rdparty/libprocess/src/http.cpp (lines 1623 - 1629)


How about a switch here so that this doesn't compile when someone adds or 
removes response types?


- Benjamin Mahler


On Nov. 28, 2016, 6:20 a.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54115/
> ---
> 
> (Updated Nov. 28, 2016, 6:20 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an http::serve abstraction.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> a684e09c8353112a0656b7e899a469c1e022e93b 
>   3rdparty/libprocess/src/http.cpp 3f16f293a5c5cd0b31a85efe94cb6f8019543d45 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d41929a9c8b2469c10b9e31985c447076c1684dc 
> 
> Diff: https://reviews.apache.org/r/54115/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 54238: Added `getContainerIOSwitchboardAddress` helper to containerizer paths.


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


Ship it!




Ship It!

- Jie Yu


On Dec. 1, 2016, 1:45 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54238/
> ---
> 
> (Updated Dec. 1, 2016, 1:45 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6467
> https://issues.apache.org/jira/browse/MESOS-6467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `getContainerIOSwitchboardAddress` helper to containerizer paths.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/paths.hpp 
> 44df6c0b74e38140a9e6037c103a7f8c6d16 
>   src/slave/containerizer/mesos/paths.cpp 
> 422fb80ced8e6435a714d32e3ee9b72874e2034d 
> 
> Diff: https://reviews.apache.org/r/54238/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 54115: Added an http::serve abstraction.

2016-11-30 Thread Benjamin Hindman


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




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


Shutdown READ_WRITE.


- Benjamin Hindman


On Nov. 28, 2016, 6:20 a.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54115/
> ---
> 
> (Updated Nov. 28, 2016, 6:20 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an http::serve abstraction.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> a684e09c8353112a0656b7e899a469c1e022e93b 
>   3rdparty/libprocess/src/http.cpp 3f16f293a5c5cd0b31a85efe94cb6f8019543d45 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d41929a9c8b2469c10b9e31985c447076c1684dc 
> 
> Diff: https://reviews.apache.org/r/54115/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 53939: Added implementation for containerizer 'attach()' call.


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

(Updated Dec. 1, 2016, 1:45 a.m.)


Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.


Changes
---

Updated based on Jie's comments.


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


Repository: mesos


Description
---

Added implementation for containerizer 'attach()' call.


Diffs (updated)
-

  src/slave/containerizer/composing.cpp 
e0818bce8a8c3561f2fdd1127cea9ce310ae7f4d 
  src/slave/containerizer/mesos/containerizer.cpp 
7dde6fc7c20ecd3543891e7d33230d0eaf9460a2 
  src/slave/containerizer/mesos/io/switchboard.hpp 
d3ab52d98ade067ccec63f3539073c152840099f 
  src/slave/containerizer/mesos/io/switchboard.cpp 
ae7a1fe3e24d5dc18269d9da951493700ee957d0 

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


Testing
---

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


Thanks,

Kevin Klues

Review Request 54238: Added `getContainerIOSwitchboardAddress` helper to containerizer paths.


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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added `getContainerIOSwitchboardAddress` helper to containerizer paths.


Diffs
-

  src/slave/containerizer/mesos/paths.hpp 
44df6c0b74e38140a9e6037c103a7f8c6d16 
  src/slave/containerizer/mesos/paths.cpp 
422fb80ced8e6435a714d32e3ee9b72874e2034d 

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


Testing
---

make check


Thanks,

Kevin Klues

Re: Review Request 54199: Minor style fix.

2016-11-30 Thread Guangya Liu


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


Ship it!




Ship It!

- Guangya Liu


On 十一月 30, 2016, 9:56 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54199/
> ---
> 
> (Updated 十一月 30, 2016, 9:56 a.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Minor style fix.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp b6c610b79aab146ece6b9be59be86f283d1c9ee8 
> 
> Diff: https://reviews.apache.org/r/54199/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 53974: Added support to handle ATTACH_CONTAINER_OUPUT in the io switchbaord.


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

(Updated Dec. 1, 2016, 1:32 a.m.)


Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.


Changes
---

Updated based on Jie's comments.


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


Repository: mesos


Description
---

Added support to handle ATTACH_CONTAINER_OUPUT in the io switchbaord.


Diffs (updated)
-

  src/slave/containerizer/mesos/io/switchboard.hpp 
d3ab52d98ade067ccec63f3539073c152840099f 
  src/slave/containerizer/mesos/io/switchboard.cpp 
ae7a1fe3e24d5dc18269d9da951493700ee957d0 
  src/slave/containerizer/mesos/io/switchboard_main.cpp PRE-CREATION 
  src/tests/containerizer/io_switchboard_tests.cpp PRE-CREATION 

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


Testing
---

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


Thanks,

Kevin Klues

Re: Review Request 54147: Added a server side component for the IOSwitchboard.


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

(Updated Dec. 1, 2016, 1:31 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Updated test to work for non root user.


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


Repository: mesos


Description
---

The 'IOSwitchboardServer' component encapsulates the server side logic
for redirecting the 'stdin/stdout/stderr' of a container to/from
multiple sources/targets. For now, we only redirect IO from a
container to the FDs supplied to us by the logger.  We also send the
stdout/stderr data to a simple HTTP server that we launch on a unix
domain socket set up by the agent.  Right now this server is just a
stub and doesn't do anything useful.

In future commits, we will expand this HTTP server to handle
'ATTACH_CONTAINER_INPUT' and 'ATTACH_CONTAINER_OUTPUT' calls on behalf
of a container. It will use the stdout/stderr messages passed to it to
and send that data over the response stream to any clients connected
with an 'ATTACH_CONTAINER_OUTPUT' call. Likewise, it will take any
input streamed in over a 'ATTACH_CONTAINER_INPUT' request and write it
to a container's stdin.

In 'local' mode, it will be run inside the agent itself. In
'non-local' mode, it will be run as an external process to survive
agent restarts.


Diffs (updated)
-

  src/Makefile.am 7750ed756d60aa61225667c129df35c6ec70f239 
  src/slave/containerizer/mesos/io/switchboard.hpp 
d3ab52d98ade067ccec63f3539073c152840099f 
  src/slave/containerizer/mesos/io/switchboard.cpp 
ae7a1fe3e24d5dc18269d9da951493700ee957d0 
  src/tests/containerizer/io_switchboard_tests.cpp PRE-CREATION 

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


Testing
---

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


Thanks,

Kevin Klues

Re: Review Request 54198: Added Resource.Allocation in Offer.

2016-11-30 Thread Guangya Liu


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


Fix it, then Ship it!




Ship It!


include/mesos/mesos.proto (line 1313)


s/represent/represents


- Guangya Liu


On 十一月 30, 2016, 9:56 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54198/
> ---
> 
> (Updated 十一月 30, 2016, 9:56 a.m.)
> 
> 
> Review request for mesos, Guangya Liu and Qian Zhang.
> 
> 
> Bugs: MESOS-6634
> https://issues.apache.org/jira/browse/MESOS-6634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Resource.Allocation in Offer protobuf message to indicate a
> single role per offer. To preserve the offer semantics between a
> single multi-role scheduler and multiple single-role schedulers,
> we would like to ensure that an offer only contains resources
> allocated to a single role.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cbfcd8a666e0b4a486f6dcd9e8356c9d5a1cea05 
>   include/mesos/v1/mesos.proto e4f269b96ffc06d040604d1d6fead01e4aca2c13 
> 
> Diff: https://reviews.apache.org/r/54198/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 54197: Introduced Resource.AllocationInfo.

2016-11-30 Thread Guangya Liu


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


Fix it, then Ship it!




There is a typo in `Description` section: s/This if/This is


include/mesos/mesos.proto (lines 787 - 793)


How about the following to make it less jagged?

```
  message AllocationInfo {
// If set, this resource is allocated to a role. Note that in the
// future, this may be unset and the scheduler may be responsible
// for allocating to one of its roles.
optional string role = 1;

// In the future, we may add additional fields here, e.g. priority
// tier, type of allocation (quota / fair share).
  }
```


- Guangya Liu


On 十一月 30, 2016, 9:58 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54197/
> ---
> 
> (Updated 十一月 30, 2016, 9:58 a.m.)
> 
> 
> Review request for mesos, Guangya Liu and Qiang Zhang.
> 
> 
> Bugs: MESOS-6633
> https://issues.apache.org/jira/browse/MESOS-6633
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To determine which role resources are allocated to, we now need to
> store allocation information within the Resources. This if part of
> the effort to support multi-role frameworks. See MESOS-6633 for more
> details.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cbfcd8a666e0b4a486f6dcd9e8356c9d5a1cea05 
>   include/mesos/v1/mesos.proto e4f269b96ffc06d040604d1d6fead01e4aca2c13 
> 
> Diff: https://reviews.apache.org/r/54197/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 54148: Updated the io switchboard to launch an external io switchboard server.


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

(Updated Dec. 1, 2016, 12:54 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Updated based on Jie's comments.


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


Repository: mesos


Description
---

We don't currently handle recovering access to the io switchboard
server process after agent restarts. We will add that in a subsequent
commit.


Diffs (updated)
-

  src/Makefile.am 7750ed756d60aa61225667c129df35c6ec70f239 
  src/slave/containerizer/mesos/io/switchboard.hpp 
d3ab52d98ade067ccec63f3539073c152840099f 
  src/slave/containerizer/mesos/io/switchboard.cpp 
ae7a1fe3e24d5dc18269d9da951493700ee957d0 
  src/slave/containerizer/mesos/io/switchboard_main.cpp PRE-CREATION 

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


Testing
---

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

[  FAILED  ] 
bool/UserContainerLoggerTest.ROOT_LOGROTATE_RotateWithSwitchUserTrueOrFalse/0, 
where GetParam() = true


Thanks,

Kevin Klues

Re: Review Request 53939: Added implementation for containerizer 'attach()' call.


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

(Updated Dec. 1, 2016, 12:55 a.m.)


Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.


Changes
---

Rebased with previous patches.


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


Repository: mesos


Description
---

Added implementation for containerizer 'attach()' call.


Diffs (updated)
-

  src/slave/containerizer/composing.cpp 
e0818bce8a8c3561f2fdd1127cea9ce310ae7f4d 
  src/slave/containerizer/mesos/containerizer.cpp 
7dde6fc7c20ecd3543891e7d33230d0eaf9460a2 
  src/slave/containerizer/mesos/io/switchboard.hpp 
d3ab52d98ade067ccec63f3539073c152840099f 
  src/slave/containerizer/mesos/io/switchboard.cpp 
ae7a1fe3e24d5dc18269d9da951493700ee957d0 

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


Testing
---

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


Thanks,

Kevin Klues

Re: Review Request 54147: Added a server side component for the IOSwitchboard.


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

(Updated Dec. 1, 2016, 12:53 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Updated to address Jie's newest comments.


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


Repository: mesos


Description
---

The 'IOSwitchboardServer' component encapsulates the server side logic
for redirecting the 'stdin/stdout/stderr' of a container to/from
multiple sources/targets. For now, we only redirect IO from a
container to the FDs supplied to us by the logger.  We also send the
stdout/stderr data to a simple HTTP server that we launch on a unix
domain socket set up by the agent.  Right now this server is just a
stub and doesn't do anything useful.

In future commits, we will expand this HTTP server to handle
'ATTACH_CONTAINER_INPUT' and 'ATTACH_CONTAINER_OUTPUT' calls on behalf
of a container. It will use the stdout/stderr messages passed to it to
and send that data over the response stream to any clients connected
with an 'ATTACH_CONTAINER_OUTPUT' call. Likewise, it will take any
input streamed in over a 'ATTACH_CONTAINER_INPUT' request and write it
to a container's stdin.

In 'local' mode, it will be run inside the agent itself. In
'non-local' mode, it will be run as an external process to survive
agent restarts.


Diffs (updated)
-

  src/Makefile.am 7750ed756d60aa61225667c129df35c6ec70f239 
  src/slave/containerizer/mesos/io/switchboard.hpp 
d3ab52d98ade067ccec63f3539073c152840099f 
  src/slave/containerizer/mesos/io/switchboard.cpp 
ae7a1fe3e24d5dc18269d9da951493700ee957d0 
  src/tests/containerizer/io_switchboard_tests.cpp PRE-CREATION 

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


Testing
---

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


Thanks,

Kevin Klues

Re: Review Request 54220: Made the agent's `api/v1` handler handle request streaming.


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




src/slave/http.cpp (line 340)


s/,/or/
s/,/or/

I think comma here might be confusing "content type should be 'a,b,c' or 
'd'"



src/slave/http.cpp (lines 365 - 378)


not sure why this block has anything to do with `requestStreaming`? why 
can't it just be singly nested 'if else' statements?



src/slave/http.cpp (lines 411 - 416)


i think this looks better if it's indented as

```
return _api(call.get(),
std::move(reader),
contentType,
acceptType.get(),
principal);
```

since `return _api(` is a short string.



src/slave/http.cpp (lines 420 - 432)


I would put this in an else block to make the semantics clear.

We typically do 
```
if (error) {
  // handle error.
  return;
}

if (some other error) {
  // handle error.
  return;
}

// handle normal case

```
when handling error cases.

but i don't think that style makes sense when both blocks are equally 
valid. in that case it should be `if else` or `switch`.



src/slave/http.cpp (line 443)


s/does/has/



src/slave/http.cpp (lines 450 - 451)


put "&&" on the above lien and indent "call.type()" with "!request"


- Vinod Kone


On Nov. 30, 2016, 7:18 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54220/
> ---
> 
> (Updated Nov. 30, 2016, 7:18 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6472
> https://issues.apache.org/jira/browse/MESOS-6472
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This lays the ground work for adding the attach container input
> call later.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 378208be58d0cd141ff60a6d25a173f2793870dc 
>   src/common/http.cpp 42af33094150001fa6741effb6b0aee1cde3c109 
>   src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
>   src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 
> 
> Diff: https://reviews.apache.org/r/54220/diff/
> 
> 
> Testing
> ---
> 
> make check (tests would be added later in the chain)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 54147: Added a server side component for the IOSwitchboard.


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




src/tests/containerizer/io_switchboard_tests.cpp (line 58)


close this and stderrFd as well?


- Jie Yu


On Nov. 30, 2016, 10:26 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54147/
> ---
> 
> (Updated Nov. 30, 2016, 10:26 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6639
> https://issues.apache.org/jira/browse/MESOS-6639
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The 'IOSwitchboardServer' component encapsulates the server side logic
> for redirecting the 'stdin/stdout/stderr' of a container to/from
> multiple sources/targets. For now, we only redirect IO from a
> container to the FDs supplied to us by the logger.  We also send the
> stdout/stderr data to a simple HTTP server that we launch on a unix
> domain socket set up by the agent.  Right now this server is just a
> stub and doesn't do anything useful.
> 
> In future commits, we will expand this HTTP server to handle
> 'ATTACH_CONTAINER_INPUT' and 'ATTACH_CONTAINER_OUTPUT' calls on behalf
> of a container. It will use the stdout/stderr messages passed to it to
> and send that data over the response stream to any clients connected
> with an 'ATTACH_CONTAINER_OUTPUT' call. Likewise, it will take any
> input streamed in over a 'ATTACH_CONTAINER_INPUT' request and write it
> to a container's stdin.
> 
> In 'local' mode, it will be run inside the agent itself. In
> 'non-local' mode, it will be run as an external process to survive
> agent restarts.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am d1cc1016b1642c504f52c27623bc12c7ddf07599 
>   src/slave/containerizer/mesos/io/switchboard.hpp 
> aaa3a35245b291f6003f519dbf8c0e1b82bc15fd 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 25cbf2447d197134f0753b062b6f4130821005b2 
>   src/tests/containerizer/io_switchboard_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54147/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check
> sudo src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 54193: Refactored `Slave::Http::launcNestedContainer()`.


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


Ship it!




Ship It!

- Anand Mazumdar


On Nov. 30, 2016, 3:53 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54193/
> ---
> 
> (Updated Nov. 30, 2016, 3:53 a.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Bugs: MESOS-6471
> https://issues.apache.org/jira/browse/MESOS-6471
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This so that code can be reused for `launchNestedContainerSession()`
> that will be added later. No functional change.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
>   src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 
> 
> Diff: https://reviews.apache.org/r/54193/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 53690: Add namespaces/ipc documentation.

2016-11-30 Thread James Peach


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

(Updated Dec. 1, 2016, 12:25 a.m.)


Review request for mesos, Avinash Gautam and Jie Yu.


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


Repository: mesos


Description
---

Add namespaces/ipc documentation.


Diffs (updated)
-

  docs/mesos-containerizer.md 2bff35f6361f760a9001a249d2c01bbbc9e72932 

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


Testing
---

Make check on Fedora 24.


Thanks,

James Peach

Re: Review Request 53688: Implement a namespace/ipc isolator.

2016-11-30 Thread James Peach


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

(Updated Dec. 1, 2016, 12:24 a.m.)


Review request for mesos, Avinash Gautam and Jie Yu.


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


Repository: mesos


Description
---

Implement a namespace/ipc isolator.


Diffs (updated)
-

  src/CMakeLists.txt ea6e399c40d3b2cda195091dc7e74230ff3f68fd 
  src/Makefile.am 7750ed756d60aa61225667c129df35c6ec70f239 
  src/slave/containerizer/mesos/containerizer.cpp 
7dde6fc7c20ecd3543891e7d33230d0eaf9460a2 
  src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp PRE-CREATION 
  src/tests/containerizer/isolator_tests.cpp 
da4627846730abd3a817c3d538ff5676c3c1ef45 

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


Testing
---

Make check on Fedora 24.


Thanks,

James Peach

Re: Review Request 53689: Use a common fixture for the PID namespace test.

2016-11-30 Thread James Peach


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

(Updated Dec. 1, 2016, 12:25 a.m.)


Review request for mesos, Avinash Gautam and Jie Yu.


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


Repository: mesos


Description
---

Use a common fixture for the PID namespace test.


Diffs (updated)
-

  src/tests/containerizer/isolator_tests.cpp 
da4627846730abd3a817c3d538ff5676c3c1ef45 

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


Testing
---

Make check in Fedora 24.


Thanks,

James Peach

Re: Review Request 54218: Made `serialize()`/`deserialize()` handle streaming content types.



> On Dec. 1, 2016, 12:14 a.m., Vinod Kone wrote:
> > not looking at subsequent reviews, it's not clear to me why the 
> > serialize/deserialize functions of streaming types are same as 
> > non-streaming types. i would've imagined the streaming types to use 
> > RecordIO format. maybe add a comment?

I can add a comment for posterity. It's not quite possible for us to re-use 
`deserialize()` if it supports "Record-IO" formatting with the 
`recordio::Reader` abstraction. This was the reason why `serialize()` supported 
"Record-IO" formatting in the initial version of this patch and was 
inconsistent with `deserialize()` and we made both of them consistent now.


> On Dec. 1, 2016, 12:14 a.m., Vinod Kone wrote:
> > src/common/http.cpp, lines 115-126
> > 
> >
> > instead of lambdas can you do this similar to `deserialize`?
> > 
> > ```
> > case ContentType::PROTOBUF:
> > case ContentType::STREAMING_PROTOBUF: {
> >   return message.SerializeAsString();
> > }
> > 
> > case ContentType::JSON:
> > case ContentType::STREAMING_JSON: {
> >   JSON::Object object = JSON::protobuf(message);
> >   return stringify(object);
> > }
> > 
> > ```

Yeah, we don't need the lambdas any more since it's no longer doing "Record-IO" 
as was the case previously. :-)


- Anand


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


On Nov. 30, 2016, 10:08 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54218/
> ---
> 
> (Updated Nov. 30, 2016, 10:08 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6472
> https://issues.apache.org/jira/browse/MESOS-6472
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `serialize()` helper can now encode a record in 'Record-IO'
> format for streaming content type. Also, `deserialize()` now
> handles a streaming content type by just parsing it the body
> into a corresponding protobuf.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 378208be58d0cd141ff60a6d25a173f2793870dc 
>   src/common/http.cpp 42af33094150001fa6741effb6b0aee1cde3c109 
> 
> Diff: https://reviews.apache.org/r/54218/diff/
> 
> 
> Testing
> ---
> 
> make check (these would be tested later in the chain)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 54229: Enabled the IO switchboard server by default for tests.


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


Ship it!




Ship It!

- Jie Yu


On Nov. 30, 2016, 10:26 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54229/
> ---
> 
> (Updated Nov. 30, 2016, 10:26 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled the IO switchboard server by default for tests.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.cpp 787c9b636e0e2e712be62407cff69d854b8eecbe 
> 
> Diff: https://reviews.apache.org/r/54229/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 53974: Added support to handle ATTACH_CONTAINER_OUPUT in the io switchbaord.


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




src/slave/containerizer/mesos/io/switchboard.cpp (line 509)


This can be Option failure.



src/slave/containerizer/mesos/io/switchboard.cpp (lines 654 - 663)


we should handle onDiscarded as well



src/slave/containerizer/mesos/io/switchboard.cpp (line 655)


`failure = Failure("Failed ...");`



src/slave/containerizer/mesos/io/switchboard.cpp (line 656)


Can you add some notes about why 'false' is used here?



src/slave/containerizer/mesos/io/switchboard.cpp (line 680)


`{` on the next line.



src/slave/containerizer/mesos/io/switchboard.cpp (line 739)


kill this line?



src/slave/containerizer/mesos/io/switchboard.cpp (line 746)


kill this line



src/slave/containerizer/mesos/io/switchboard.cpp (line 787)


const?



src/slave/containerizer/mesos/io/switchboard.cpp (line 796)


why not const?



src/slave/containerizer/mesos/io/switchboard.cpp (line 825)


iterator might be invalidated after insertion or erase. So I don't think 
this works.



src/tests/containerizer/io_switchboard_tests.cpp (line 204)


Is there constant for this?


- Jie Yu


On Nov. 30, 2016, 10:26 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53974/
> ---
> 
> (Updated Nov. 30, 2016, 10:26 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-6467
> https://issues.apache.org/jira/browse/MESOS-6467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support to handle ATTACH_CONTAINER_OUPUT in the io switchbaord.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.hpp 
> aaa3a35245b291f6003f519dbf8c0e1b82bc15fd 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 25cbf2447d197134f0753b062b6f4130821005b2 
>   src/slave/containerizer/mesos/io/switchboard_main.cpp PRE-CREATION 
>   src/tests/containerizer/io_switchboard_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53974/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check
> sudo src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 54218: Made `serialize()`/`deserialize()` handle streaming content types.


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



not looking at subsequent reviews, it's not clear to me why the 
serialize/deserialize functions of streaming types are same as non-streaming 
types. i would've imagined the streaming types to use RecordIO format. maybe 
add a comment?


src/common/http.cpp (lines 115 - 126)


instead of lambdas can you do this similar to `deserialize`?

```
case ContentType::PROTOBUF:
case ContentType::STREAMING_PROTOBUF: {
  return message.SerializeAsString();
}

case ContentType::JSON:
case ContentType::STREAMING_JSON: {
  JSON::Object object = JSON::protobuf(message);
  return stringify(object);
}

```


- Vinod Kone


On Nov. 30, 2016, 10:08 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54218/
> ---
> 
> (Updated Nov. 30, 2016, 10:08 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6472
> https://issues.apache.org/jira/browse/MESOS-6472
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `serialize()` helper can now encode a record in 'Record-IO'
> format for streaming content type. Also, `deserialize()` now
> handles a streaming content type by just parsing it the body
> into a corresponding protobuf.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 378208be58d0cd141ff60a6d25a173f2793870dc 
>   src/common/http.cpp 42af33094150001fa6741effb6b0aee1cde3c109 
> 
> Diff: https://reviews.apache.org/r/54218/diff/
> 
> 
> Testing
> ---
> 
> make check (these would be tested later in the chain)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 54219: Added validation for the attach container input call on the agent.


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


Ship it!




Ship It!

- Vinod Kone


On Nov. 30, 2016, 7:18 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54219/
> ---
> 
> (Updated Nov. 30, 2016, 7:18 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6472
> https://issues.apache.org/jira/browse/MESOS-6472
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added validation for the attach container input call on the agent.
> 
> 
> Diffs
> -
> 
>   src/slave/validation.cpp 46e84361244394196ee5d431c1da34686d6c938a 
> 
> Diff: https://reviews.apache.org/r/54219/diff/
> 
> 
> Testing
> ---
> 
> make check (tests would be added later in the chain)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 53939: Added implementation for containerizer 'attach()' call.


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


Fix it, then Ship it!





src/slave/containerizer/mesos/io/switchboard.cpp (lines 312 - 318)


probably add a paths function:
```
getContainerIOSwitchboardSocketPath
```


- Jie Yu


On Nov. 30, 2016, 6:55 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53939/
> ---
> 
> (Updated Nov. 30, 2016, 6:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-6467
> https://issues.apache.org/jira/browse/MESOS-6467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added implementation for containerizer 'attach()' call.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> e0818bce8a8c3561f2fdd1127cea9ce310ae7f4d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4a03f97d10a1b34a0a13600af5ac6fc3679694fd 
>   src/slave/containerizer/mesos/io/switchboard.hpp 
> aaa3a35245b291f6003f519dbf8c0e1b82bc15fd 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 25cbf2447d197134f0753b062b6f4130821005b2 
> 
> Diff: https://reviews.apache.org/r/53939/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check
> sudo src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 54226: Exposed IOSwitchboard to MesosContainerizer.


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



Patch looks great!

Reviews applied: [54226]

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 Nov. 30, 2016, 8:31 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54226/
> ---
> 
> (Updated Nov. 30, 2016, 8:31 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-6651
> https://issues.apache.org/jira/browse/MESOS-6651
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed IOSwitchboard to MesosContainerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 370a26686c62c973b3e9c06c4653b7657719c858 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 7dde6fc7c20ecd3543891e7d33230d0eaf9460a2 
>   src/slave/containerizer/mesos/io/switchboard.hpp 
> d3ab52d98ade067ccec63f3539073c152840099f 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> ae7a1fe3e24d5dc18269d9da951493700ee957d0 
> 
> Diff: https://reviews.apache.org/r/54226/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 50736: Added SSL support to libprocess HTTP request helpers.


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

(Updated Nov. 30, 2016, 11:16 p.m.)


Review request for mesos, Anand Mazumdar, Joseph Wu, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

In order to test libprocess HTTP functionality with SSL
both enabled and disabled, the HTTP helper functions that
libprocess uses in tests must be parametrized by SSL
configuration.


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp 
7894b3100a4d89974207d9953b2fd04abe746a33 
  3rdparty/libprocess/src/http.cpp 8a10dca08247d9873f70c1b0c7e9f4f6332678f4 

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


Testing
---

Find testing information in the subsequent patch in this chain.


Thanks,

Greg Mann

Re: Review Request 51065: Changed hostname used for SSL cert creation in tests.


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

(Updated Nov. 30, 2016, 11:16 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

This changes the SSL helpers in libprocess to generate
certs using a hostname determined using the same address
that is advertised by libprocess. This assures that
validation of the certificates will succeed. The test
`SSLTest.BasicSameProcess` is also updated to accommodate
this change.


Diffs (updated)
-

  3rdparty/libprocess/include/process/ssl/gtest.hpp 
2a19b3b1e09517151bddb43a128f45457da4ca8a 
  3rdparty/libprocess/src/tests/ssl_tests.cpp 
7fa46e4cc3e1225e9c9591fb5a1fb8d90c165c01 

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


Testing
---

Testing details are found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 50737: Parametrized libprocess HTTPTests by SSL configuration.


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

(Updated Nov. 30, 2016, 11:14 p.m.)


Review request for mesos, Anand Mazumdar, Joseph Wu, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

This patch parametrizes the HTTP tests in libprocess so
that they run with SSL both enabled and disabled when
the library was compiled with SSL support.


Diffs (updated)
-

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

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


Testing
---

Testing details are found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 53803: Added a new libprocess HTTP test.


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

(Updated Nov. 30, 2016, 11:14 p.m.)


Review request for mesos, Benjamin Mahler and Joseph Wu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

This patch adds HTTPTest.SocketEOF to verify that EOFs are
reliably received whether or not there is a pending recv()
request at the time the EOF is received.


Diffs (updated)
-

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

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


Testing
---

Testing details are at the end of this review chain.


Thanks,

Greg Mann

Re: Review Request 53802: Eliminated an EOF race condition in libprocess SSL socket.


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

(Updated Nov. 30, 2016, 11:12 p.m.)


Review request for mesos, Benjamin Mahler and Joseph Wu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Previously, it was possible for an SSL socket to either:
1) Fail to receive an EOF if the EOF event was received when
   there was no pending recv() request.
2) Fail to receive all data sent on the sending side if an
   EOF event was received before all sent data was read.

This patch eliminates these race conditions to ensure reliable
receipt of both sent data and EOFs.


Diffs (updated)
-

  3rdparty/libprocess/src/libevent_ssl_socket.hpp 
57eaf4f607d0628db466cc1a139772eeeaa51136 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
0e292a8b0d470f4e199db08f09a0c863d73c 

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


Testing
---

Testing details are found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 54148: Updated the io switchboard to launch an external io switchboard server.


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


Fix it, then Ship it!





src/slave/containerizer/mesos/io/switchboard.cpp (line 157)


is not supported



src/slave/containerizer/mesos/io/switchboard.cpp (line 196)


I would remove this, and s/endif/else/ above



src/slave/containerizer/mesos/io/switchboard.cpp (line 291)


Let's use /dev/null here.



src/slave/containerizer/mesos/io/switchboard.cpp (line 312)


NO need for this check.



src/slave/containerizer/mesos/io/switchboard.cpp (lines 314 - 315)


I would add a constructor to Info and use the following here.
```
infos.put(containerId, Owned(new Info(pid));
```



src/slave/containerizer/mesos/io/switchboard.cpp (line 346)


remove the extra space.



src/slave/containerizer/mesos/io/switchboard.cpp (lines 349 - 350)


This should be a TODO here.



src/slave/containerizer/mesos/io/switchboard.cpp (line 355)


I suggest we start the `reap` in `prepare` the moment we create Info, and 
registered a callback `reaped`. This way, it's easier for us to impl. 'watch' 
in the future.

```
struct Info {
  pid_t pid;
  Future

Re: Review Request 52544: Introduced `int_fd` class.

2016-11-30 Thread Daniel Pravat



> On Nov. 19, 2016, 3:28 a.m., Michael Park wrote:
> > 3rdparty/stout/include/stout/os/windows/filedescriptor.hpp, lines 381-384
> > 
> >
> > Do we actually need this? I think the `int` on the rhs should implicit 
> > convert to `WindowsFD` in which case this would be handled by:
> > 
> > ```
> > bool operator==(
> > const os::WindowsFileDescriptor& left,
> > const os::WindowsFileDescriptor& right);
> > ```
> 
> Daniel Pravat wrote:
> There are many conversion options and the compiler is not selecting bool 
> operator==(
> const os::WindowsFileDescriptor& left,
> const os::WindowsFileDescriptor& right);

The are other candidates for this conversion. The compilation fails.


> On Nov. 19, 2016, 3:28 a.m., Michael Park wrote:
> > 3rdparty/stout/include/stout/os/windows/filedescriptor.hpp, lines 318-334
> > 
> >
> > I feel like we should be able to answer this question (`is_socket`) for 
> > `WindowsFD` since we answer it for `int` in POSIX. (Beyond the fact that it 
> > seems bizzare for `SOCKET` to somehow __not__ be a socket...)

This is a temporary function. I will remove it asap.


- Daniel


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


On Nov. 30, 2016, 7:54 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52544/
> ---
> 
> (Updated Nov. 30, 2016, 7:54 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Alex Clemmer, Joseph Wu, and Michael 
> Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In POSIX the `socket`,`pipe` and the `filedescriptor` are
> represented by an int type. In Windows a socket is kept in a
> `SOCKET` type (64 bit wide), a pipe in a `HANDLE` (64 bit wide) and
> a file descriptor in an int. This class unifies all Windows types.
> In POSIX this class is an int.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os.hpp 
> bd085e4e29bbdb2d2baaaeff1d10c0bd95ca65ba 
>   3rdparty/stout/include/stout/os/filedescriptor.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/filedescriptor.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52544/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 54061: Augmented FrameworkInfo to support multiple roles.


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


Ship it!




We should include some warnings that this isn't implemented yet and that this 
feature will be experimental at first. I'll do this in a separate commit.

- Benjamin Mahler


On Nov. 30, 2016, 9:58 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54061/
> ---
> 
> (Updated Nov. 30, 2016, 9:58 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, and Qiang Zhang.
> 
> 
> Bugs: MESOS-6628
> https://issues.apache.org/jira/browse/MESOS-6628
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the initial patch to introduce multi-role frameworks.
> A new section `repeated string roles` is added to FrameworkInfo.
> Old `optional string role` is kept for backwards compatability.
> To distinguish single-role and multi-role frameworks, a new framework
> capability `MULTI_ROLE` is introduced. See MESOS-6628 for more info.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cbfcd8a666e0b4a486f6dcd9e8356c9d5a1cea05 
>   include/mesos/v1/mesos.proto e4f269b96ffc06d040604d1d6fead01e4aca2c13 
> 
> Diff: https://reviews.apache.org/r/54061/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Review Request 54229: Enabled the IO switchboard server by default for tests.


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

Review request for mesos.


Repository: mesos


Description
---

Enabled the IO switchboard server by default for tests.


Diffs
-

  src/tests/mesos.cpp 787c9b636e0e2e712be62407cff69d854b8eecbe 

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


Testing
---

make check


Thanks,

Kevin Klues

Re: Review Request 54147: Added a server side component for the IOSwitchboard.


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

(Updated Nov. 30, 2016, 10:26 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Updated based on Jie's comments.


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


Repository: mesos


Description
---

The 'IOSwitchboardServer' component encapsulates the server side logic
for redirecting the 'stdin/stdout/stderr' of a container to/from
multiple sources/targets. For now, we only redirect IO from a
container to the FDs supplied to us by the logger.  We also send the
stdout/stderr data to a simple HTTP server that we launch on a unix
domain socket set up by the agent.  Right now this server is just a
stub and doesn't do anything useful.

In future commits, we will expand this HTTP server to handle
'ATTACH_CONTAINER_INPUT' and 'ATTACH_CONTAINER_OUTPUT' calls on behalf
of a container. It will use the stdout/stderr messages passed to it to
and send that data over the response stream to any clients connected
with an 'ATTACH_CONTAINER_OUTPUT' call. Likewise, it will take any
input streamed in over a 'ATTACH_CONTAINER_INPUT' request and write it
to a container's stdin.

In 'local' mode, it will be run inside the agent itself. In
'non-local' mode, it will be run as an external process to survive
agent restarts.


Diffs (updated)
-

  src/Makefile.am d1cc1016b1642c504f52c27623bc12c7ddf07599 
  src/slave/containerizer/mesos/io/switchboard.hpp 
aaa3a35245b291f6003f519dbf8c0e1b82bc15fd 
  src/slave/containerizer/mesos/io/switchboard.cpp 
25cbf2447d197134f0753b062b6f4130821005b2 
  src/tests/containerizer/io_switchboard_tests.cpp PRE-CREATION 

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


Testing
---

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


Thanks,

Kevin Klues

Re: Review Request 53974: Added support to handle ATTACH_CONTAINER_OUPUT in the io switchbaord.


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

(Updated Nov. 30, 2016, 10:26 p.m.)


Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.


Changes
---

Updated based on changes to previous patches.


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


Repository: mesos


Description
---

Added support to handle ATTACH_CONTAINER_OUPUT in the io switchbaord.


Diffs (updated)
-

  src/slave/containerizer/mesos/io/switchboard.hpp 
aaa3a35245b291f6003f519dbf8c0e1b82bc15fd 
  src/slave/containerizer/mesos/io/switchboard.cpp 
25cbf2447d197134f0753b062b6f4130821005b2 
  src/slave/containerizer/mesos/io/switchboard_main.cpp PRE-CREATION 
  src/tests/containerizer/io_switchboard_tests.cpp PRE-CREATION 

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


Testing
---

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


Thanks,

Kevin Klues

Review Request 54232: Shutdown tasks of completed frameworks on agent re-registration.


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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Previously, if a framework completed (e.g., due to a teardown operation
or framework shutdown), any framework tasks running on partitioned
agents would not be shutdown when the agent re-registered. For tasks
that are not partition-aware, the task would be shutdown on agent
re-registration anyway. But for partition-aware tasks, this could lead
to orphan tasks.

Fix this by changing the master to shutdown such tasks when the agent
reregisters.

Note that if the master fails over between the time the framework
completes and a partitioned agent re-registers, any framework tasks
running on the agent will NOT be shutdown. This is a known bug; fixing
it requires persisting the framework shutdown operation to the registry
(MESOS-1719).


Diffs
-

  src/master/master.hpp 877ca9010d0d6efc97f3d71fbd27272a255409d0 
  src/master/master.cpp e03a2e8025943825a2902102c43dc0eb66bacb6a 
  src/tests/partition_tests.cpp 5a0d4bd2de6a5aa0e9fdf0d34cd10d16fd4e34a1 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 54215: Fixed duplicate image layer ids returned by docker store.

2016-11-30 Thread Zhitao Li


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




src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 263)


I took a quick look at `mesosphere/inky` and all repeating layers are 
empty, so this is fine for this case.

However, I'm not sure whether there is a legitemate case which same 
non-empty layer can repeat in the same images multiple times. If that's 
possible then this code could break such image provisioning (unfortunately I 
can't reproduce such a case myself...)


- Zhitao Li


On Nov. 30, 2016, 6:54 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54215/
> ---
> 
> (Updated Nov. 30, 2016, 6:54 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, Qian Zhang, and Zhitao 
> Li.
> 
> 
> Bugs: MESOS-6654
> https://issues.apache.org/jira/browse/MESOS-6654
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This issue is exposed by pulling the 'mesosphere/inky' docker
> image using registry puller. Due to the duplicate layer id
> from the manifest, there are duplicate layer pathes passed
> to the backend. The aufs backend cannot handle this case and
> returns 'invalid arguments' error. Ideally, we should make
> sure that layer paths that are passed to the backend are
> unique.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
> 
> Diff: https://reviews.apache.org/r/54215/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Tested by the unit test 
> `ROOT_CURL_INTERNET_DockerDefaultEntryptRegistryPuller`.
> 
> Manually tested using the `mesosphere/inky` image, which contains duplicate 
> layer ids.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 54221: Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.


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

(Updated Nov. 30, 2016, 10:09 p.m.)


Review request for mesos, Kevin Klues and Vinod Kone.


Changes
---

Changes as a result of `serialize()` not being "Record-IO" aware.


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


Repository: mesos


Description
---

Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.


Diffs (updated)
-

  src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
  src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 

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


Testing
---

make check (tests would be added soon and can't be committed without them)


Thanks,

Anand Mazumdar

Re: Review Request 54218: Made `serialize()`/`deserialize()` handle streaming content types.


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

(Updated Nov. 30, 2016, 10:08 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments


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


Repository: mesos


Description
---

The `serialize()` helper can now encode a record in 'Record-IO'
format for streaming content type. Also, `deserialize()` now
handles a streaming content type by just parsing it the body
into a corresponding protobuf.


Diffs (updated)
-

  src/common/http.hpp 378208be58d0cd141ff60a6d25a173f2793870dc 
  src/common/http.cpp 42af33094150001fa6741effb6b0aee1cde3c109 

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


Testing
---

make check (these would be tested later in the chain)


Thanks,

Anand Mazumdar

Re: Review Request 54074: Added isolator nested aware user doc.


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



Patch looks great!

Reviews applied: [54013, 54073, 54074]

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 Nov. 30, 2016, 7:11 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54074/
> ---
> 
> (Updated Nov. 30, 2016, 7:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, Qian 
> Zhang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6335
> https://issues.apache.org/jira/browse/MESOS-6335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a table for current semantics of isolators
> in Mesos, as well as the guidance of how the make a custom
> isolator module to be nested aware.
> 
> 
> Diffs
> -
> 
>   docs/mesos-containerizer.md 2bff35f6361f760a9001a249d2c01bbbc9e72932 
> 
> Diff: https://reviews.apache.org/r/54074/diff/
> 
> 
> Testing
> ---
> 
> Tested by gist view. Here is the link:
> 
> https://github.com/Gilbert88/mesos/blob/doc_pod/docs/mesos-containerizer.md#isolator-nested-aware
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 54218: Made `serialize()`/`deserialize()` handle streaming content types.


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




src/common/http.hpp (lines 66 - 75)


please make this consistent with `serialize()`



src/common/http.cpp (line 105)


s/protobufSerializeFn/protobufSerializer/



src/common/http.cpp (line 109)


s/jsonSerializeFn/jsonSerializer/


- Vinod Kone


On Nov. 30, 2016, 7:18 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54218/
> ---
> 
> (Updated Nov. 30, 2016, 7:18 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6472
> https://issues.apache.org/jira/browse/MESOS-6472
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `serialize()` helper can now encode a record in 'Record-IO'
> format for streaming content type. Also, `deserialize()` now
> handles a streaming content type by just parsing it the body
> into a corresponding protobuf.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 378208be58d0cd141ff60a6d25a173f2793870dc 
>   src/common/http.cpp 42af33094150001fa6741effb6b0aee1cde3c109 
> 
> Diff: https://reviews.apache.org/r/54218/diff/
> 
> 
> Testing
> ---
> 
> make check (these would be tested later in the chain)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 54147: Added a server side component for the IOSwitchboard.


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


Fix it, then Ship it!





src/slave/containerizer/mesos/io/switchboard.hpp (line 111)


No need for explicit. You need explicit only for single parameter 
constructor.



src/slave/containerizer/mesos/io/switchboard.cpp (lines 297 - 302)


Try if this works `{defer(self(), ::outputHook, ...)}`



src/slave/containerizer/mesos/io/switchboard.cpp (lines 309 - 314)


Ditto.



src/slave/containerizer/mesos/io/switchboard.cpp (line 337)


onXXX accepts a void function. No need to 'return'.



src/slave/containerizer/mesos/io/switchboard.cpp (line 339)


Ignore this if you already did that in the following patch. We should 
terminate the process if promise is set or failed.



src/slave/containerizer/mesos/io/switchboard.cpp (line 345)


No need to return



src/slave/containerizer/mesos/io/switchboard.cpp (line 363)


Binding `this` without a defer is not safe. socket.accept() might finish 
after IOSwitchboardServerProcess terminates and is deleted.



src/slave/containerizer/mesos/io/switchboard.cpp (line 366)


No need to return for onAny



src/slave/containerizer/mesos/io/switchboard.cpp (line 376)


Ditto. This should be a defer.



src/tests/containerizer/io_switchboard_tests.cpp (line 40)


Why MesosTest? This can just be a TemporaryDirectoryTest



src/tests/containerizer/io_switchboard_tests.cpp (line 46)


Do you need this in this test?



src/tests/containerizer/io_switchboard_tests.cpp (lines 59 - 60)


If this is a TemporaryDirectoryTest, you can simply use `sandbox.get()`



src/tests/containerizer/io_switchboard_tests.cpp (line 118)


This should be EXPECT



src/tests/containerizer/io_switchboard_tests.cpp (line 123)


Ditto. this should be EXPECT


- Jie Yu


On Nov. 30, 2016, 9:35 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54147/
> ---
> 
> (Updated Nov. 30, 2016, 9:35 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6639
> https://issues.apache.org/jira/browse/MESOS-6639
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The 'IOSwitchboardServer' component encapsulates the server side logic
> for redirecting the 'stdin/stdout/stderr' of a container to/from
> multiple sources/targets. For now, we only redirect IO from a
> container to the FDs supplied to us by the logger.  We also send the
> stdout/stderr data to a simple HTTP server that we launch on a unix
> domain socket set up by the agent.  Right now this server is just a
> stub and doesn't do anything useful.
> 
> In future commits, we will expand this HTTP server to handle
> 'ATTACH_CONTAINER_INPUT' and 'ATTACH_CONTAINER_OUTPUT' calls on behalf
> of a container. It will use the stdout/stderr messages passed to it to
> and send that data over the response stream to any clients connected
> with an 'ATTACH_CONTAINER_OUTPUT' call. Likewise, it will take any
> input streamed in over a 'ATTACH_CONTAINER_INPUT' request and write it
> to a container's stdin.
> 
> In 'local' mode, it will be run inside the agent itself. In
> 'non-local' mode, it will be run as an external process to survive
> agent restarts.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am d1cc1016b1642c504f52c27623bc12c7ddf07599 
>   src/slave/containerizer/mesos/io/switchboard.hpp 
> aaa3a35245b291f6003f519dbf8c0e1b82bc15fd 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 25cbf2447d197134f0753b062b6f4130821005b2 
>   src/tests/containerizer/io_switchboard_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54147/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check
> sudo src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 53974: Added support to handle ATTACH_CONTAINER_OUPUT in the io switchbaord.


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

(Updated Nov. 30, 2016, 9:36 p.m.)


Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.


Changes
---

Rebased on changes to previous patches.


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


Repository: mesos


Description
---

Added support to handle ATTACH_CONTAINER_OUPUT in the io switchbaord.


Diffs (updated)
-

  src/slave/containerizer/mesos/io/switchboard.hpp 
aaa3a35245b291f6003f519dbf8c0e1b82bc15fd 
  src/slave/containerizer/mesos/io/switchboard.cpp 
25cbf2447d197134f0753b062b6f4130821005b2 
  src/slave/containerizer/mesos/io/switchboard_main.cpp PRE-CREATION 
  src/tests/containerizer/io_switchboard_tests.cpp PRE-CREATION 

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


Testing
---

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


Thanks,

Kevin Klues

Re: Review Request 54147: Added a server side component for the IOSwitchboard.


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

(Updated Nov. 30, 2016, 9:35 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Updated to use a defer() in the redirect hooks, based on offline comments from 
Jie.


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


Repository: mesos


Description
---

The 'IOSwitchboardServer' component encapsulates the server side logic
for redirecting the 'stdin/stdout/stderr' of a container to/from
multiple sources/targets. For now, we only redirect IO from a
container to the FDs supplied to us by the logger.  We also send the
stdout/stderr data to a simple HTTP server that we launch on a unix
domain socket set up by the agent.  Right now this server is just a
stub and doesn't do anything useful.

In future commits, we will expand this HTTP server to handle
'ATTACH_CONTAINER_INPUT' and 'ATTACH_CONTAINER_OUTPUT' calls on behalf
of a container. It will use the stdout/stderr messages passed to it to
and send that data over the response stream to any clients connected
with an 'ATTACH_CONTAINER_OUTPUT' call. Likewise, it will take any
input streamed in over a 'ATTACH_CONTAINER_INPUT' request and write it
to a container's stdin.

In 'local' mode, it will be run inside the agent itself. In
'non-local' mode, it will be run as an external process to survive
agent restarts.


Diffs (updated)
-

  src/Makefile.am d1cc1016b1642c504f52c27623bc12c7ddf07599 
  src/slave/containerizer/mesos/io/switchboard.hpp 
aaa3a35245b291f6003f519dbf8c0e1b82bc15fd 
  src/slave/containerizer/mesos/io/switchboard.cpp 
25cbf2447d197134f0753b062b6f4130821005b2 
  src/tests/containerizer/io_switchboard_tests.cpp PRE-CREATION 

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


Testing
---

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


Thanks,

Kevin Klues

Re: Review Request 54217: Introduced streaming content types.


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


Ship it!




Ship It!

- Vinod Kone


On Nov. 30, 2016, 7:17 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54217/
> ---
> 
> (Updated Nov. 30, 2016, 7:17 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6472
> https://issues.apache.org/jira/browse/MESOS-6472
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a couple of streaming content types that can be
> used by handlers to support request streaming in the future.
> 
> 
> Diffs
> -
> 
>   include/mesos/http.hpp bd7e81f13d6bfa512995a49ef746e376fc89336e 
> 
> Diff: https://reviews.apache.org/r/54217/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 54223: Adjusted agent `usedResources` accounting at the master.


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


Ship it!




Ship It!

- Benjamin Mahler


On Nov. 30, 2016, 7:21 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54223/
> ---
> 
> (Updated Nov. 30, 2016, 7:21 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If an agent finished running all the tasks for a given framework, the
> previous coding resulted in leaving an empty `Resources` object in the
> Slave's `usedResources` hashmap.
> 
> Along the way, replace using `[]` to access a hashmap in a `CHECK` with
> `at`. Because `[]` can modify the map, we should generally prefer using
> `at` for accesses that are intended to be read-only.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp b6c610b79aab146ece6b9be59be86f283d1c9ee8 
> 
> Diff: https://reviews.apache.org/r/54223/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> By adding `LOG(INFO)` to `Master::finalize()`, I validated that when running 
> a test like `MasterTest.KillTask`, the previous implementation left a key 
> with an empty `Resources` object in the `usedResources` hashmap. After this 
> change, the `hashmap` is empty, as expected.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 54178: Added a fixed-capacity hashmap to stout.


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

(Updated Nov. 30, 2016, 9:03 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Update makefile.


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


Repository: mesos


Description
---

This is similar to the LinkedHashMap and Cache constructs.

Unlike LinkedHashMap, a BoundedHashMap has a fixed maximum capacity:
when the BoundedHashMap is at capacity, inserting another element
results in removing the oldest element still in the map (i.e., FIFO
order is used for eviction).

Unlike Cache, reading an element from a BoundedHashMap does not change
which entry is evicted next.


Diffs (updated)
-

  3rdparty/stout/Makefile.am 4e10ae25a6c8a2d01d7b98b04ccf06a216611938 
  3rdparty/stout/include/Makefile.am d5bc728ce378586e84173b98499b0d52047a83e1 
  3rdparty/stout/include/stout/boundedhashmap.hpp PRE-CREATION 
  3rdparty/stout/tests/boundedhashmap_tests.cpp PRE-CREATION 

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


Testing
---

`make check`

Note that this is based on the revised design of LinkedHashMap, which in a 
concurrent RR has been modified to support `foreachpair` and friends. See 
https://reviews.apache.org/r/54018/


Thanks,

Neil Conway

Re: Review Request 54183: Improved management of unreachable and completed tasks in master.


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

(Updated Nov. 30, 2016, 8:58 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Tweak comments.


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


Repository: mesos


Description
---

Before partition-awareness, when an agent failed health checks, the
master removed the agent from the registry, marked all of its tasks
TASK_LOST, and moved them to the `completedTasks` list in the master's
memory. Although "lost" tasks might still be running, partitioned agents
would only be allowed to re-register if the master failed over, in which
case the `completedTasks` map would be emptied.

When partition-awareness was introduced, we initially followed the same
scheme, with the only difference that partition-aware tasks are marked
TASK_UNREACHABLE, not TASK_LOST.

This scheme has a few shortcomings. First, partition-aware tasks might
resume running when the partitioned agent re-registers. Second, we
re-added non-partition aware tasks when the agent re-registered but then
marked them completed when the framework is shutdown, resulting in two
entries in `completedTasks`.

This commit introduces a separate bounded map, `unreachableTasks`. These
tasks are reported separately via the HTTP endpoints, because they have
different semantics (unlike completed tasks, unreachable tasks can
resume running). The size of this map is limited by a new master flag,
`--max_unreachable_tasks_per_framework`. This commit also changes the
master to omit re-adding non-partition-aware tasks on re-registering
agents (unless the master has failed over): those tasks will shortly be
shutdown anyway.

Finally, this commit fixes a minor bug in the previous code: the
previous coding neglected to shutdown non-partition-aware frameworks
running on pre-1.0 Mesos agents that re-register with the master after
a network partition.


Diffs (updated)
-

  docs/configuration.md efe3e9bd9d203a7ba44adf4ead24f14b8b577637 
  include/mesos/master/master.proto 3553c683c17004ac1831ec90271aa8584c950e53 
  include/mesos/v1/master/master.proto 022b491b7d5c49c5aeddf4ffc97c148f55629c95 
  src/master/constants.hpp 5dd0667f62d2c0617cc0d5aed8cc005bd8344c88 
  src/master/flags.hpp 6a17b763dc76daa10073394f416b049e97a44238 
  src/master/flags.cpp 9bfb40e22820c3ced40b128280ec63288fea8b41 
  src/master/http.cpp ac560d1fdd219d0de0c5d987a32a7112e149602f 
  src/master/master.hpp b6c610b79aab146ece6b9be59be86f283d1c9ee8 
  src/master/master.cpp e03a2e8025943825a2902102c43dc0eb66bacb6a 
  src/tests/partition_tests.cpp 5a0d4bd2de6a5aa0e9fdf0d34cd10d16fd4e34a1 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 52695: Harden libprocess

2016-11-30 Thread Aaron Wood

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

(Updated Nov. 30, 2016, 8:52 p.m.)

Review request for mesos, James Peach, Michael Park, and Neil Conway.

Changes
---

Don't warn when stack protection isn't used.

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

Repository: mesos

Description
---

Add hardened flags for libprocess.
Take compile flag macro at 391cb680171d3889965b1ead43d3a326c913bc25.
The macro at 1a869696e4129279f7b99c3f9052717354b79a86 requires autoconf 2.64
which breaks on CentOS 6.

Diffs (updated)
-

3rdparty/libprocess/Makefile.am 9d496b8
3rdparty/libprocess/configure.ac e65e5ca
3rdparty/libprocess/m4/ax_check_compile_flag.m4 PRE-CREATION

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

Testing
---

Compared the benchmarks with and without the flags being used. Also did a
comparsion with the flags being used with and without optimizations and without
the flags being used with and without optimizations. Overall the performance
hit was very small with a 3-8% overhead (optimizations brings this down
slightly). Most benchmarks were about 5% (or less) slower.

File Attachments

--enable-optimized with hardening applied

https://reviews.apache.org/media/uploaded/files/2016/11/02/875c9e6e-c73b-4e3c-8265-0f7c6dc00351__hardened-optimized.txt
Hardening applied but no --enable-optimized

https://reviews.apache.org/media/uploaded/files/2016/11/02/932d28a7-2d31-471a-b438-647841a6853c__hardened-unoptimized.txt
--enable-optimized with no hardening applied

https://reviews.apache.org/media/uploaded/files/2016/11/02/896944ea-9b31-4d62-b1b9-97fb4700a882__optimized.txt
No hardening applied and no --enable-optimized

https://reviews.apache.org/media/uploaded/files/2016/11/02/b32667ce-3e3b-4d2b-b4f8-4c2404a0fc1c__unoptimized.txt

Thanks,

Aaron Wood

Re: Review Request 52645: Harden Mesos

2016-11-30 Thread Aaron Wood


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

(Updated Nov. 30, 2016, 8:51 p.m.)


Review request for mesos, James Peach, Michael Park, and Neil Conway.


Changes
---

Don't warn when stack protection isn't used.


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


Repository: mesos


Description
---

Add hardened flags for Mesos.
Take compile flag macro at 391cb680171d3889965b1ead43d3a326c913bc25.
The macro at 1a869696e4129279f7b99c3f9052717354b79a86 requires autoconf 2.64 
which breaks on CentOS 6.


Diffs (updated)
-

  configure.ac 5380cbc 
  m4/ax_check_compile_flag.m4 PRE-CREATION 
  src/Makefile.am 7750ed7 

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


Testing
---

Compared the benchmarks with and without the flags being used. Also did a 
comparsion with the flags being used with and without optimizations and without 
the flags being used with and without optimizations. Overall the performance 
hit was very small with a 3-8% overhead (optimizations brings this down 
slightly). Most benchmarks were about 5% (or less) slower.


Thanks,

Aaron Wood

Re: Review Request 52696: Harden stout

2016-11-30 Thread Aaron Wood

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

(Updated Nov. 30, 2016, 8:50 p.m.)

Review request for mesos, James Peach, Michael Park, and Neil Conway.

Changes
---

Don't warn when stack protection isn't used.

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

Repository: mesos

Description
---

Add hardened flags for stout.
Take compile flag macro at 391cb680171d3889965b1ead43d3a326c913bc25.
The macro at 1a869696e4129279f7b99c3f9052717354b79a86 requires autoconf 2.64
which breaks on CentOS 6.

Diffs (updated)
-

3rdparty/stout/Makefile.am 4e10ae2
3rdparty/stout/configure.ac f071f61
3rdparty/stout/m4/ax_check_compile_flag.m4 PRE-CREATION

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

Testing
---

File Attachments

--enable-optimized with hardening applied

https://reviews.apache.org/media/uploaded/files/2016/11/02/18a2f590-75ad-49c5-a697-56b746f28cae__hardened-optimized.txt
Hardening applied but no --enable-optimized

https://reviews.apache.org/media/uploaded/files/2016/11/02/a6e07766-80cc-4bd7-856d-8952cac12562__hardened-unoptimized.txt
--enable-optimized with no hardening applied

https://reviews.apache.org/media/uploaded/files/2016/11/02/046b37a9-5aff-4543-b3bb-5ac60daaf498__optimized.txt
No hardening applied and no --enable-optimized

https://reviews.apache.org/media/uploaded/files/2016/11/02/3baa96cf-be05-4ac0-ad4c-ef571386e8f4__unoptimized.txt

Thanks,

Aaron Wood

Re: Review Request 54226: Exposed IOSwitchboard to MesosContainerizer.


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


Ship it!




Ship It!

- Kevin Klues


On Nov. 30, 2016, 8:31 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54226/
> ---
> 
> (Updated Nov. 30, 2016, 8:31 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-6651
> https://issues.apache.org/jira/browse/MESOS-6651
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed IOSwitchboard to MesosContainerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 370a26686c62c973b3e9c06c4653b7657719c858 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 7dde6fc7c20ecd3543891e7d33230d0eaf9460a2 
>   src/slave/containerizer/mesos/io/switchboard.hpp 
> d3ab52d98ade067ccec63f3539073c152840099f 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> ae7a1fe3e24d5dc18269d9da951493700ee957d0 
> 
> Diff: https://reviews.apache.org/r/54226/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 54053: Updated 'io::redirect()' to take an optional vector of callback hooks.


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


Ship it!




Ship It!

- Jie Yu


On Nov. 28, 2016, 9:18 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54053/
> ---
> 
> (Updated Nov. 28, 2016, 9:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-6639
> https://issues.apache.org/jira/browse/MESOS-6639
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These callback hooks will be invoked before passing any data read from
> the 'from' file descriptor on to the 'to' file descriptor.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/io.hpp 
> eec5efd7e6b71a783f2bb40826054d0488cee71f 
>   3rdparty/libprocess/src/io.cpp d930ceebc90468e082b984e41385549f906dc6ae 
>   3rdparty/libprocess/src/tests/io_tests.cpp 
> 66b610e35fa9f6a1738c77d181d76dca3921e6fb 
> 
> Diff: https://reviews.apache.org/r/54053/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j40 check
> GTEST_FILTER="IOTest.Redirect" 3rdparty/libprocess/libprocess-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Review Request 54226: Exposed IOSwitchboard to MesosContainerizer.


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

Review request for mesos and Kevin Klues.


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


Repository: mesos


Description
---

Exposed IOSwitchboard to MesosContainerizer.


Diffs
-

  src/slave/containerizer/mesos/containerizer.hpp 
370a26686c62c973b3e9c06c4653b7657719c858 
  src/slave/containerizer/mesos/containerizer.cpp 
7dde6fc7c20ecd3543891e7d33230d0eaf9460a2 
  src/slave/containerizer/mesos/io/switchboard.hpp 
d3ab52d98ade067ccec63f3539073c152840099f 
  src/slave/containerizer/mesos/io/switchboard.cpp 
ae7a1fe3e24d5dc18269d9da951493700ee957d0 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 54212: Fixed overlay backend provisioning multi images symlink.

2016-11-30 Thread Zhitao Li


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




src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 167)


Do we want to consider maintain a similar hierarchy as `scratch`? It seems 
asymmetrical if symlinks and scratch is at different file system level.

Maybe we should consider make this `path::join(backendDir, "links", 
rootfsId)`?


- Zhitao Li


On Nov. 30, 2016, 6:54 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54212/
> ---
> 
> (Updated Nov. 30, 2016, 6:54 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, Qian Zhang, and Zhitao 
> Li.
> 
> 
> Bugs: MESOS-6653
> https://issues.apache.org/jira/browse/MESOS-6653
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the fix of MESOS-6000, symlinks are used in overlayfs
> backend to shorten the arguments when mounting the rootfs.
> E.g., '.../backends/overlay/links' is the symlink created
> for a provisioned image. It becomes problematic if a
> container image is specified while some image volumes are
> specified for the same container. An unique symlink is
> needed for each image to be provisioned.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
> b71a31323aef376c9a28e1d52322a1802fb212ad 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> 727bf645dd9337a94f098ab0a816b7e0e0651083 
> 
> Diff: https://reviews.apache.org/r/54212/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Verified that the unit test `ROOT_ImageInVolumeWithRootFilesystem` passed.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 54211: Fixed overlay backend symlink error message.

2016-11-30 Thread Zhitao Li


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


Ship it!




Ship It!

- Zhitao Li


On Nov. 30, 2016, 6:54 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54211/
> ---
> 
> (Updated Nov. 30, 2016, 6:54 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, Qian Zhang, and Zhitao 
> Li.
> 
> 
> Bugs: MESOS-6653
> https://issues.apache.org/jira/browse/MESOS-6653
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed overlay backend symlink error message.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
> b71a31323aef376c9a28e1d52322a1802fb212ad 
> 
> Diff: https://reviews.apache.org/r/54211/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 54223: Adjusted agent `usedResources` accounting at the master.


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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

If an agent finished running all the tasks for a given framework, the
previous coding resulted in leaving an empty `Resources` object in the
Slave's `usedResources` hashmap.

Along the way, replace using `[]` to access a hashmap in a `CHECK` with
`at`. Because `[]` can modify the map, we should generally prefer using
`at` for accesses that are intended to be read-only.


Diffs
-

  src/master/master.hpp b6c610b79aab146ece6b9be59be86f283d1c9ee8 

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


Testing
---

`make check`

By adding `LOG(INFO)` to `Master::finalize()`, I validated that when running a 
test like `MasterTest.KillTask`, the previous implementation left a key with an 
empty `Resources` object in the `usedResources` hashmap. After this change, the 
`hashmap` is empty, as expected.


Thanks,

Neil Conway

Review Request 54221: Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.


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

Review request for mesos, Kevin Klues and Vinod Kone.


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


Repository: mesos


Description
---

Added the v1 `ATTACH_CONTAINER_INPUT` call handler on the agent.


Diffs
-

  src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
  src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 

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


Testing
---

make check (tests would be added soon and can't be committed without them)


Thanks,

Anand Mazumdar

Review Request 54220: Made the agent's `api/v1` handler handle request streaming.


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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This lays the ground work for adding the attach container input
call later.


Diffs
-

  src/common/http.hpp 378208be58d0cd141ff60a6d25a173f2793870dc 
  src/common/http.cpp 42af33094150001fa6741effb6b0aee1cde3c109 
  src/slave/http.cpp 87189dd6e2e099cb74faabd3ad26aaca11e5cef2 
  src/slave/slave.hpp 05865a09724f7a46e9c06b02e59779513fc532aa 

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


Testing
---

make check (tests would be added later in the chain)


Thanks,

Anand Mazumdar

Review Request 54219: Added validation for the attach container input call on the agent.


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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Added validation for the attach container input call on the agent.


Diffs
-

  src/slave/validation.cpp 46e84361244394196ee5d431c1da34686d6c938a 

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


Testing
---

make check (tests would be added later in the chain)


Thanks,

Anand Mazumdar

Review Request 54218: Made `serialize()`/`deserialize()` handle streaming content types.


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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

The `serialize()` helper can now encode a record in 'Record-IO'
format for streaming content type. Also, `deserialize()` now
handles a streaming content type by just parsing it the body
into a corresponding protobuf.


Diffs
-

  src/common/http.hpp 378208be58d0cd141ff60a6d25a173f2793870dc 
  src/common/http.cpp 42af33094150001fa6741effb6b0aee1cde3c109 

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


Testing
---

make check (these would be tested later in the chain)


Thanks,

Anand Mazumdar

Review Request 54217: Introduced streaming content types.