Re: Review Request 38837: CMake: Disable slave build on Windows.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38825, 38826, 38827, 38837]

All tests passed.

- Mesos ReviewBot


On Sept. 29, 2015, 2:13 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38837/
> ---
> 
> (Updated Sept. 29, 2015, 2:13 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Disable slave build on Windows.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt c866f6d85d629ca9c3a987fa2c652bc63a7ff1be 
> 
> Diff: https://reviews.apache.org/r/38837/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38816: add test case for docker remotePuller

[Jojy Varghese]

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



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


Please add this along with others in the "../provisioner/docker/" block in 
alphabetical order.



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


We sort the block alphabetically.



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


You can re-use the RegistryClientTest fixture instead of creating a new one.



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


Wondering if you can refactor all of this so that SimpleGetBlob and this 
test can reuse the common code.



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


You will have to verify that the response blob saved in the file has the 
content you expected (clock::now)


- Jojy Varghese


On Sept. 28, 2015, 6:27 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38816/
> ---
> 
> (Updated Sept. 28, 2015, 6:27 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-3494
> https://issues.apache.org/jira/browse/MESOS-3494
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> add test case for docker remotePuller
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38816/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu 14.04) 
> 
> libevent and ssl enabled in configuration (e.g., '../configure --disable-java 
> --disable-python --enable-libevent --enable-ssl --enable-debug')
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 38810: poll_socket: fix a file descriptor leak under error condition.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38810]

All tests passed.

- Mesos ReviewBot


On Sept. 28, 2015, 5:28 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38810/
> ---
> 
> (Updated Sept. 28, 2015, 5:28 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> poll_socket: fix a file descriptor leak under error condition.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/poll_socket.cpp 
> 4da62b698ac9c5bbd4f1eee6188b6dcca03aa099 
> 
> Diff: https://reviews.apache.org/r/38810/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 37903: stout: Fix bug in IPNetwork::create() with zero prefix.

[Niklas Nielsen]

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


I'll get this in today :) Thanks Neil!

- Niklas Nielsen


On Sept. 24, 2015, 6:02 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37903/
> ---
> 
> (Updated Sept. 24, 2015, 6:02 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3328
> https://issues.apache.org/jira/browse/MESOS-3328
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous coding would try to shift a uint32_t value 32 bits to the left; 
> per
> C++ spec, this yields undefined behavior. On my machine, this resulted in
> treating a prefix of 0 as equivalent to a prefix of 32, which is obviously
> wrong.
> 
> Spotted via ubsan: see MESOS-3328.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
> 1ad119d54820e97497b1773518875be25ddbf98a 
>   3rdparty/libprocess/3rdparty/stout/tests/ip_tests.cpp 
> b0cbcb38cfcb923ec7c185bacf139ceb0a28924f 
> 
> Diff: https://reviews.apache.org/r/37903/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38819: Fixed a bug in cgroups test filter.

[Ben Mahler]

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

Ship it!


Much appreciated :)

- Ben Mahler


On Sept. 28, 2015, 7:46 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38819/
> ---
> 
> (Updated Sept. 28, 2015, 7:46 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Paul Brett.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a bug in cgroups test filter.
> 
> Otherwise, 
> CgroupsNoHierarchyTest.ROOT_CGROUPS_NOHIERARCHY_MountUnmountHierarchy will 
> run even if there are hierarchies being mounted.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp 000b2010831039f961c0ee9bb83cb1929da66197 
> 
> Diff: https://reviews.apache.org/r/38819/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38809: socket: refactor to use Option and fix file descriptor leaks.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38809]

All tests passed.

- Mesos ReviewBot


On Sept. 28, 2015, 5:25 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38809/
> ---
> 
> (Updated Sept. 28, 2015, 5:25 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3519
> https://issues.apache.org/jira/browse/MESOS-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> socket: refactor to use Option and fix file descriptor leaks.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/socket.hpp 
> a677e29b28711fc065134c11792b4f62fa3aa8b4 
>   3rdparty/libprocess/src/socket.cpp 5879423bd92c5805353a962cbd822a993b0568c5 
> 
> Diff: https://reviews.apache.org/r/38809/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 38161: Replaced a hard-coded number for registration backoff with a proper constant and extended comments.

[haosdent huang]

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

Ship it!


Ship It!

- haosdent huang


On Sept. 28, 2015, 5:08 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38161/
> ---
> 
> (Updated Sept. 28, 2015, 5:08 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/fault_tolerance_tests.cpp 
> c97bc4691f9bac4a8677e6d2247be96ee9674b57 
> 
> Diff: https://reviews.apache.org/r/38161/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 38161: Replaced a hard-coded number for registration backoff with a proper constant and extended comments.

[Alexander Rukletsov]

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

(Updated Sept. 28, 2015, 5:08 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Rebased.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/fault_tolerance_tests.cpp c97bc4691f9bac4a8677e6d2247be96ee9674b57 

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


Testing
---

make check (Mac OS 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 37275: [2/2]Generate make batch file to build project in windows.

[Joseph Wu]

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


Haosdent, are you going to close these two now that they have been 
(effectively) submitted?

- Joseph Wu


On Sept. 15, 2015, 8:51 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37275/
> ---
> 
> (Updated Sept. 15, 2015, 8:51 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Generate make batch file to build project in windows.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
> 
> Diff: https://reviews.apache.org/r/37275/diff/
> 
> 
> Testing
> ---
> 
> # Build steps:
> cmake ..  -DREBUNDLED=FALSE
> make
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38747: Adding digest utilities

[Jiang Yan Xu]

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


High-level comments:

- This implementation relies on USE_SSL_SOCKET which is tied to `--enable_ssl`. 
Conceptually the two should be decoupled. Ian Downes has an implementation 
which uses the system commands that are widely distributed on *nix boxes: 
https://reviews.apache.org/r/34138/. Can such an implemenation be used as a 
fallback at least?
- Can the implentation be put in a cpp file and leave only public (the 
recommended way of using this utility) methods in the header (e.g. 
`process::verify(...)` and `process::digest()`)?

- Jiang Yan Xu


On Sept. 25, 2015, 1:33 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> ---
> 
> (Updated Sept. 25, 2015, 1:33 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added SHA256, SHA512 implementation.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> a14b5b8fe22d3e75bef3c716ae7865ddaf132927 
>   3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38747/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Review Request 38819: Fixed a bug in cgroups test filter.

[Jie Yu]

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

Review request for mesos, Ben Mahler and Paul Brett.


Repository: mesos


Description
---

Fixed a bug in cgroups test filter.

Otherwise, 
CgroupsNoHierarchyTest.ROOT_CGROUPS_NOHIERARCHY_MountUnmountHierarchy will run 
even if there are hierarchies being mounted.


Diffs
-

  src/tests/environment.cpp 000b2010831039f961c0ee9bb83cb1929da66197 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 37853: Overlay filesystem provisioning backend

[Jie Yu]

> On Aug. 27, 2015, 10:42 p.m., Jie Yu wrote:
> > src/slave/containerizer/provisioners/backends/overlay.cpp, line 58
> > 
> >
> > You also want to check if overlay fs is supported or not. Not every 
> > linux kernel supports overlay fs.
> 
> Mei Wan wrote:
> How would I check this?

Well, i'll let you to figure this out :) Hint: google /proc/filesystems


> On Aug. 27, 2015, 10:42 p.m., Jie Yu wrote:
> > src/slave/containerizer/provisioners/backends/overlay.cpp, lines 110-114
> > 
> >
> > Could you please add a comment about the ordering the layers will be 
> > stacked.
> > 
> > I think we should add a document at the backend interface as well 
> > stating that layers are stacking in such a way that the front layer in the 
> > vector will be the bottom most layer (i.e., applied first).
> > 
> > The lowerdir here should actually reverse the order of the layers 
> > vector.
> > 
> > ```
> > The specified lower directories will be stacked beginning from the 
> > rightmost one and going left.  In the above example lower1 will be the top, 
> > lower2 the middle and lower3 the bottom layer.
> > ```
> 
> Mei Wan wrote:
> Where would I add this document? I could just add it to the flag?

You can just add that commments right above this code block.


> On Aug. 27, 2015, 10:42 p.m., Jie Yu wrote:
> > src/slave/containerizer/provisioners/backends/overlay.cpp, line 111
> > 
> >
> > I think you should be able to use strings::join here:
> > 
> > ```
> > strings::join(":", layers);
> > ```
> 
> Mei Wan wrote:
> The format would be dir1:dir2:dir3 etc. I'm just wondering is it possible 
> to have the semi colon only appear between the directories? If I'm thinking 
> about this correctly, strings::join will give me :dir1:dir2:dir3? Also since 
> I'm stacking the layers in reverse from the comment above, does this still 
> apply?

strings::join will give you dir1:dir2:dir3 (exactly what you want).


- Jie


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


On Aug. 27, 2015, 9:11 p.m., Mei Wan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37853/
> ---
> 
> (Updated Aug. 27, 2015, 9:11 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Jie Yu.
> 
> 
> Bugs: MESOS-2971
> https://issues.apache.org/jira/browse/MESOS-2971
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the overlay filesystem backend by layering the images as a 
> read-only filesystem.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7b620ff 
>   src/slave/containerizer/provisioners/backend.cpp 2f7c335 
>   src/slave/containerizer/provisioners/backends/overlay.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/backends/overlay.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37853/diff/
> 
> 
> Testing
> ---
> 
> I haven't done any official testing. When I was working off Ian's branch, I 
> tested it manually and the provisioning works.
> 
> 
> Thanks,
> 
> Mei Wan
> 
>

Review Request 38814: add test cases for sha512 digest verifier

[Gilbert Song]

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

Review request for mesos, Jojy Varghese and Timothy Chen.


Repository: mesos


Description
---

add test cases for sha512 digest verifier


Diffs
-

  3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 

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


Testing
---

make check (ubuntu 14.04)


Thanks,

Gilbert Song

Re: Review Request 38597: Added URL within http::Request.

[Ben Mahler]

> On Sept. 23, 2015, 9:10 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/decoder.hpp, line 157
> > 
> >
> > not needed anymore?

Yes not needed, not totally obvious but if you look at the ifdef'ed 
`HTTP_PARSER_VERSION_MAJOR` code, the url parts either come in:

(1) From separate callbacks: `on_path`, `on_query_string`, `on_fragment`, or:
(2) From the `on_url` callback, and we use `http_parser_parse_url` to parse the 
path, query, fragment.


> On Sept. 23, 2015, 9:10 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/include/process/http.hpp, line 180
> > 
> >
> > s/a//

Thanks!


- Ben


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


On Sept. 22, 2015, 6:18 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38597/
> ---
> 
> (Updated Sept. 22, 2015, 6:18 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now that URL is a URI-reference, we can implement the TODO to replace the raw 
> fields in Request with a URL.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/firewall.hpp 
> b1abfb29fe2db07d991e9a6f655345720faef863 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/include/process/system.hpp 
> 264d948c2c000c6cb176601735bd4ea0d6fcbc77 
>   3rdparty/libprocess/src/decoder.hpp 
> 67a5135f302153e376e8dfe8db82aa0b15449389 
>   3rdparty/libprocess/src/help.cpp d358b93005506dad53aac815908cca59e6a53118 
>   3rdparty/libprocess/src/logging.cpp 
> db76abe08bc74c4d7444e112f32a5d11a236d229 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> 943ba63b01fd982185bc33679bba5cc3fcc086fc 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 97c81b8e61a75771e5bf7d46505cec4e0c0f404a 
>   3rdparty/libprocess/src/tests/decoder_tests.cpp 
> 6994fa96d33209f9a367b8c3bb09b0d050023fad 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d0b9399d38fa284466a012a21080b1d9007af98b 
> 
> Diff: https://reviews.apache.org/r/38597/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 38160: Documented how to expedite event firing.

[haosdent huang]

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

Ship it!


Ship It!

- haosdent huang


On Sept. 28, 2015, 5:09 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38160/
> ---
> 
> (Updated Sept. 28, 2015, 5:09 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/mesos-testing-patterns.md d0d92ff569012677043a03412462cf4384096ba9 
> 
> Diff: https://reviews.apache.org/r/38160/diff/
> 
> 
> Testing
> ---
> 
> doc rendered in Markdown
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 37502: Add CMAKE_CXX_FLAGS to GMOCK_CONFIG_CMD in CMake.

[Joseph Wu]

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


Haosdent, are you going to close these two now that they have been 
(effectively) submitted?

- Joseph Wu


On Sept. 14, 2015, 1:08 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37502/
> ---
> 
> (Updated Sept. 14, 2015, 1:08 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, and Joseph Wu.
> 
> 
> Bugs: MESOS-3270
> https://issues.apache.org/jira/browse/MESOS-3270
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add CMAKE_CXX_FLAGS to GMOCK_CONFIG_CMD in CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> d13ba666740b4f2e382a0b1852724cfd519f8f64 
> 
> Diff: https://reviews.apache.org/r/37502/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Joseph Wu]

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

Ship it!



support/apply-reviews.py (line 8)


Not used.



support/apply-reviews.py (line 12)


Typo: Retutns.

Also, docstrings go inside the method body.

```
def review_url(id):
  """Returns a Review Board URL given a review ID."""
```

You can tell if it is correct if you do something like:
```
python
import apply-reviews
print review_url.__doc__
```



support/apply-reviews.py (line 18)


s/JSON-ifyed/JSON-ified/


- Joseph Wu


On Sept. 26, 2015, 7:02 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Sept. 26, 2015, 7:02 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

[Chris Chen]

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

Review request for mesos.


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


Repository: mesos


Description
---

Problem:
The Zookeeeper C client does makes certain assertions about the ordering
of ping packets that the Java client does not. An alternate implementation
of the Zookeeper server would then break the C client while working
correctly with the Java client.

Solution:
Port change from ZOOKEEPER-2253 to make C client handle pings similarly
to the Java client.

Result:
C client and Java clients behave the same when dealing with an out-of-order
ping packet.


Diffs
-

  3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
  3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 

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


Testing
---

Tested from upstream; Running mesos check suite currently.


Thanks,

Chris Chen

Review Request 38809: socket: refactor to use Option and fix file descriptor leaks.

[Chi Zhang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

socket: refactor to use Option and fix file descriptor leaks.


Diffs
-

  3rdparty/libprocess/include/process/socket.hpp 
a677e29b28711fc065134c11792b4f62fa3aa8b4 
  3rdparty/libprocess/src/socket.cpp 5879423bd92c5805353a962cbd822a993b0568c5 

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


Testing
---


Thanks,

Chi Zhang

Re: Review Request 38160: Documented how to expedite event firing.

[Alexander Rukletsov]

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

(Updated Sept. 28, 2015, 5:09 p.m.)


Review request for mesos, Bernd Mathiske and Ben Mahler.


Changes
---

Changed "naïve" to "simple" to avoid confusion.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  docs/mesos-testing-patterns.md d0d92ff569012677043a03412462cf4384096ba9 

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


Testing
---

doc rendered in Markdown


Thanks,

Alexander Rukletsov

Review Request 38810: poll_socket: fix a file descriptor leak under error condition.

[Chi Zhang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

poll_socket: fix a file descriptor leak under error condition.


Diffs
-

  3rdparty/libprocess/src/poll_socket.cpp 
4da62b698ac9c5bbd4f1eee6188b6dcca03aa099 

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


Testing
---


Thanks,

Chi Zhang

Re: Review Request 38160: Documented how to expedite event firing.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38161, 38160]

All tests passed.

- Mesos ReviewBot


On Sept. 28, 2015, 5:09 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38160/
> ---
> 
> (Updated Sept. 28, 2015, 5:09 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/mesos-testing-patterns.md d0d92ff569012677043a03412462cf4384096ba9 
> 
> Diff: https://reviews.apache.org/r/38160/diff/
> 
> 
> Testing
> ---
> 
> doc rendered in Markdown
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 38816: make changes corresponding to the change in remote_puller.cpp

[Gilbert Song]

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

Review request for mesos, Jojy Varghese and Timothy Chen.


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


Repository: mesos


Description
---

add test case for docker remotePuller


Diffs
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check (ubuntu 14.04) 

libevent and ssl enabled in configuration (e.g., '../configure --disable-java 
--disable-python --enable-libevent --enable-ssl --enable-debug')


Thanks,

Gilbert Song

Re: Review Request 38816: add test case for docker remotePuller

[Gilbert Song]

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

(Updated Sept. 28, 2015, 6:27 p.m.)


Review request for mesos, Jojy Varghese and Timothy Chen.


Summary (updated)
-

add test case for docker remotePuller


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


Repository: mesos


Description
---

add test case for docker remotePuller


Diffs
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check (ubuntu 14.04) 

libevent and ssl enabled in configuration (e.g., '../configure --disable-java 
--disable-python --enable-libevent --enable-ssl --enable-debug')


Thanks,

Gilbert Song

Re: Review Request 38809: socket: refactor to use Option and fix file descriptor leaks.

[Neil Conway]

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



3rdparty/libprocess/src/socket.cpp (line 40)


"socketfd.isNone()" would be more readable, IMHO.



3rdparty/libprocess/src/socket.cpp (line 77)


Why is this conditional on socketFd?



3rdparty/libprocess/src/socket.cpp (line 89)


Same as above.


- Neil Conway


On Sept. 28, 2015, 5:25 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38809/
> ---
> 
> (Updated Sept. 28, 2015, 5:25 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3519
> https://issues.apache.org/jira/browse/MESOS-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> socket: refactor to use Option and fix file descriptor leaks.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/socket.hpp 
> a677e29b28711fc065134c11792b4f62fa3aa8b4 
>   3rdparty/libprocess/src/socket.cpp 5879423bd92c5805353a962cbd822a993b0568c5 
> 
> Diff: https://reviews.apache.org/r/38809/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 38747: Adding digest utilities

[Neil Conway]

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



3rdparty/libprocess/include/process/digest.hpp (line 191)


I believe this leaks "fd".



3rdparty/libprocess/include/process/digest.hpp (line 205)


Probably "constexpr" is better than "static const".



3rdparty/libprocess/include/process/digest.hpp (line 219)


Seems like this duplicates some code above: can we refactor this into a 
function?



3rdparty/libprocess/include/process/digest.hpp (line 222)


Why do we initialize this to `{0}`?



3rdparty/libprocess/include/process/digest.hpp (line 225)


Need a space after "for".

Also, is sprintf() really the best choice here? There's std::hex + 
std::stringstream, although maybe that's not better overall.



3rdparty/libprocess/include/process/digest.hpp (line 237)


Typo



3rdparty/libprocess/include/process/digest.hpp (line 265)


Typo



3rdparty/libprocess/include/process/digest.hpp (line 315)


Typo


- Neil Conway


On Sept. 25, 2015, 8:33 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> ---
> 
> (Updated Sept. 25, 2015, 8:33 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added SHA256, SHA512 implementation.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> a14b5b8fe22d3e75bef3c716ae7865ddaf132927 
>   3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38747/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38747: Adding digest utilities

[Gilbert Song]

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



3rdparty/libprocess/include/process/digest.hpp (lines 210 - 226)


digest printed out is different between digest(const char* bytes, size_t 
size) and digest(const std::string& filePath) with a corresponding same string. 

Logic check is needed. (proprably is a logical error in my test case)



3rdparty/libprocess/include/process/digest.hpp (line 275)


This line is supposed to be deleted.


- Gilbert Song


On Sept. 25, 2015, 8:33 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> ---
> 
> (Updated Sept. 25, 2015, 8:33 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added SHA256, SHA512 implementation.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> a14b5b8fe22d3e75bef3c716ae7865ddaf132927 
>   3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38747/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

[Alex Clemmer]

> On Sept. 28, 2015, 8:43 p.m., Alex Clemmer wrote:
> > Can you add this comment to the CMake build system as well?
> > 
> > The file you need to change is here: 
> > https://github.com/apache/mesos/blob/master/3rdparty/CMakeLists.txt Should 
> > be line 40, above `PATCH_CMD`.
> 
> Chris Chen wrote:
> I don't see that file...

Hmm, I'm not 100% sure what you mean. Do you mean that you don't see it in 
apache/master? This file was checked in yesterday (along with ~20 other reviews 
relevant to the CMake stuff), perhaps it is the case that you haven't pulled 
since then?


- Alex


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


On Sept. 28, 2015, 6:17 p.m., Chris Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38815/
> ---
> 
> (Updated Sept. 28, 2015, 6:17 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3391
> https://issues.apache.org/jira/browse/MESOS-3391
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Problem:
> The Zookeeeper C client does makes certain assertions about the ordering
> of ping packets that the Java client does not. An alternate implementation
> of the Zookeeper server would then break the C client while working
> correctly with the Java client.
> 
> Solution:
> Port change from ZOOKEEPER-2253 to make C client handle pings similarly
> to the Java client.
> 
> Result:
> C client and Java clients behave the same when dealing with an out-of-order
> ping packet.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
> 
> Diff: https://reviews.apache.org/r/38815/diff/
> 
> 
> Testing
> ---
> 
> Tested from upstream; Running mesos check suite currently.
> 
> 
> Thanks,
> 
> Chris Chen
> 
>

Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

[Chris Chen]

> On Sept. 28, 2015, 8:43 p.m., Alex Clemmer wrote:
> > Can you add this comment to the CMake build system as well?
> > 
> > The file you need to change is here: 
> > https://github.com/apache/mesos/blob/master/3rdparty/CMakeLists.txt Should 
> > be line 40, above `PATCH_CMD`.
> 
> Chris Chen wrote:
> I don't see that file...
> 
> Alex Clemmer wrote:
> Hmm, I'm not 100% sure what you mean. Do you mean that you don't see it 
> in apache/master? This file was checked in yesterday (along with ~20 other 
> reviews relevant to the CMake stuff), perhaps it is the case that you haven't 
> pulled since then?

Lovely. My remote was set to the github copy. Reset origin to apache.org and I 
see it. Thx.


- Chris


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


On Sept. 28, 2015, 6:17 p.m., Chris Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38815/
> ---
> 
> (Updated Sept. 28, 2015, 6:17 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3391
> https://issues.apache.org/jira/browse/MESOS-3391
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Problem:
> The Zookeeeper C client does makes certain assertions about the ordering
> of ping packets that the Java client does not. An alternate implementation
> of the Zookeeper server would then break the C client while working
> correctly with the Java client.
> 
> Solution:
> Port change from ZOOKEEPER-2253 to make C client handle pings similarly
> to the Java client.
> 
> Result:
> C client and Java clients behave the same when dealing with an out-of-order
> ping packet.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
> 
> Diff: https://reviews.apache.org/r/38815/diff/
> 
> 
> Testing
> ---
> 
> Tested from upstream; Running mesos check suite currently.
> 
> 
> Thanks,
> 
> Chris Chen
> 
>

Re: Review Request 38809: socket: refactor to use Option and fix file descriptor leaks.

[Neil Conway]

> On Sept. 28, 2015, 6:03 p.m., Neil Conway wrote:
> > 3rdparty/libprocess/src/socket.cpp, line 77
> > 
> >
> > Why is this conditional on socketFd?
> 
> Chi Zhang wrote:
> The idea is I am only responsible for the resource I allocated. If a 
> socketfd is passed in, I leave it to the owner to free it; if a socketfd is 
> created by me, I will free it.

Duh, makes sense.


- Neil


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


On Sept. 28, 2015, 9:07 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38809/
> ---
> 
> (Updated Sept. 28, 2015, 9:07 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3519
> https://issues.apache.org/jira/browse/MESOS-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> socket: refactor to use Option and fix file descriptor leaks.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/socket.hpp 
> 817cb3711742871630a13b966563296644008f21 
>   3rdparty/libprocess/src/socket.cpp 5879423bd92c5805353a962cbd822a993b0568c5 
> 
> Diff: https://reviews.apache.org/r/38809/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 38823: io: fixed a file descriptor leak in io::redirect.

[Chi Zhang]

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

(Updated Sept. 28, 2015, 9:34 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

io: fixed a file descriptor leak in io::redirect.


Diffs
-

  3rdparty/libprocess/src/io.cpp d17ccf7713debc43f5c2ca1aeb9a1ca7ed03b4a6 

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


Testing
---


Thanks,

Chi Zhang

Re: Review Request 38823: io: fixed a file descriptor leak in io::redirect.

[Chi Zhang]

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

(Updated Sept. 28, 2015, 9:36 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

io: fixed a file descriptor leak in io::redirect.


Diffs
-

  3rdparty/libprocess/src/io.cpp d17ccf7713debc43f5c2ca1aeb9a1ca7ed03b4a6 

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


Testing (updated)
---

make check


Thanks,

Chi Zhang

Re: Review Request 38443: Added layerid information to ManifestResponse

[Timothy Chen]

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



src/slave/containerizer/provisioner/docker/registry_client.cpp (line 79)


tokenMgr -> tokenManager, I can fix myself



src/slave/containerizer/provisioner/docker/registry_client.cpp (line 505)


I think we need to say we failed because it's not the expected Object type. 
Same here and other placess you have the check

Also you use index: in other places, and just index here. Let's be 
consistent and use index:


- Timothy Chen


On Sept. 25, 2015, 6:33 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38443/
> ---
> 
> (Updated Sept. 25, 2015, 6:33 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added layerid information to ManifestResponse
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38443/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

[Chris Chen]

> On Sept. 28, 2015, 8:43 p.m., Alex Clemmer wrote:
> > Can you add this comment to the CMake build system as well?
> > 
> > The file you need to change is here: 
> > https://github.com/apache/mesos/blob/master/3rdparty/CMakeLists.txt Should 
> > be line 40, above `PATCH_CMD`.

I don't see that file...


- Chris


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


On Sept. 28, 2015, 6:17 p.m., Chris Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38815/
> ---
> 
> (Updated Sept. 28, 2015, 6:17 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3391
> https://issues.apache.org/jira/browse/MESOS-3391
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Problem:
> The Zookeeeper C client does makes certain assertions about the ordering
> of ping packets that the Java client does not. An alternate implementation
> of the Zookeeper server would then break the C client while working
> correctly with the Java client.
> 
> Solution:
> Port change from ZOOKEEPER-2253 to make C client handle pings similarly
> to the Java client.
> 
> Result:
> C client and Java clients behave the same when dealing with an out-of-order
> ping packet.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
> 
> Diff: https://reviews.apache.org/r/38815/diff/
> 
> 
> Testing
> ---
> 
> Tested from upstream; Running mesos check suite currently.
> 
> 
> Thanks,
> 
> Chris Chen
> 
>

Re: Review Request 38814: add test cases for sha512 digest verifier

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [38443]

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

Error:
 2015-09-28 20:39:14 URL:https://reviews.apache.org/r/38443/diff/raw/ 
[11158/11158] -> "38443.patch" [1]
error: patch failed: 
src/slave/containerizer/provisioner/docker/registry_client.cpp:89
error: src/slave/containerizer/provisioner/docker/registry_client.cpp: patch 
does not apply
Failed to apply patch

- Mesos ReviewBot


On Sept. 28, 2015, 6:06 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38814/
> ---
> 
> (Updated Sept. 28, 2015, 6:06 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> add test cases for sha512 digest verifier
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38814/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu 14.04)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

[Alex Clemmer]

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


Can you add this comment to the CMake build system as well?

The file you need to change is here: 
https://github.com/apache/mesos/blob/master/3rdparty/CMakeLists.txt Should be 
line 40, above `PATCH_CMD`.

- Alex Clemmer


On Sept. 28, 2015, 6:17 p.m., Chris Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38815/
> ---
> 
> (Updated Sept. 28, 2015, 6:17 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3391
> https://issues.apache.org/jira/browse/MESOS-3391
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Problem:
> The Zookeeeper C client does makes certain assertions about the ordering
> of ping packets that the Java client does not. An alternate implementation
> of the Zookeeper server would then break the C client while working
> correctly with the Java client.
> 
> Solution:
> Port change from ZOOKEEPER-2253 to make C client handle pings similarly
> to the Java client.
> 
> Result:
> C client and Java clients behave the same when dealing with an out-of-order
> ping packet.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
> 
> Diff: https://reviews.apache.org/r/38815/diff/
> 
> 
> Testing
> ---
> 
> Tested from upstream; Running mesos check suite currently.
> 
> 
> Thanks,
> 
> Chris Chen
> 
>

Re: Review Request 38774: state: fix file descriptor leak

[Chi Zhang]

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

(Updated Sept. 28, 2015, 8:42 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

state: fix file descriptor leak


Diffs (updated)
-

  src/slave/state.cpp 47c66dc80d57db86981769d404c2c8c7c972fec0 

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


Testing
---

make check


Thanks,

Chi Zhang

Re: Review Request 38809: socket: refactor to use Option and fix file descriptor leaks.

[Chi Zhang]

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

(Updated Sept. 28, 2015, 9:07 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

socket: refactor to use Option and fix file descriptor leaks.


Diffs (updated)
-

  3rdparty/libprocess/include/process/socket.hpp 
817cb3711742871630a13b966563296644008f21 
  3rdparty/libprocess/src/socket.cpp 5879423bd92c5805353a962cbd822a993b0568c5 

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


Testing (updated)
---

make check


Thanks,

Chi Zhang

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

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



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 210)


Add defer


- Timothy Chen


On Sept. 28, 2015, 8:39 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Sept. 28, 2015, 8:39 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First iteration with basic functionality.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 4a0b7d11f013941084571f2d89d835a4668a3d8b 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> cbb67686d45513f0395a0cf1bc5c43cb4935adae 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Review Request 38823: io: fixed a file descriptor leak in io::redirect.

[Chi Zhang]

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

Review request for mesos.


Repository: mesos


Description
---

io: fixed a file descriptor leak in io::redirect.


Diffs
-

  3rdparty/libprocess/src/io.cpp d17ccf7713debc43f5c2ca1aeb9a1ca7ed03b4a6 

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


Testing
---


Thanks,

Chi Zhang

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

[Klaus Ma]

> On Sept. 28, 2015, 9:11 a.m., Jan Schlicht wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 762
> > 
> >
> > Because this code has been changed from a constructor to a function, we 
> > can do better than calling `ABORT` here. We can change the function 
> > signature to return a `Try` and call `Error` here instead of 
> > `ABORT`. For example see the `write` function in line 55.
> > 
> > The function signature would change to
> > `inline Try protobuf(const google::protobuf::Message& message)`
> 
> Klaus Ma wrote:
> Got your point :).
> Just one concern on how to use it? We did not handle its result if failed 
> in `src`; personally, this's a kind of code practice to avoid undefined 
> protobuf field type. Anyway, I'll update accordingly if necessary.
> 
> Jan Schlicht wrote:
> Well, unfortunately this change means that the code that is using this 
> function has to be changed to handle the error cases. This means checking if 
> the `Try<>` contains a value or an error and react accordingly.

Did you check the code diff of #38335? In #38335, any suggestion on the 
function that return void; those function did not expect json parsing will 
fail. Personally, it's a bit overkill to re-implement them for json paring 
error handling which are not expect failed. 

@alex-mesos, @mcypark, what's your suggestion?


- Klaus


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


On Sept. 27, 2015, 1:34 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38342/
> ---
> 
> (Updated Sept. 27, 2015, 1:34 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 2285ce9 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 68328a2 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 8ebb798 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 34eb6d0 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 920f5c9 
> 
> Diff: https://reviews.apache.org/r/38342/diff/
> 
> 
> Testing
> ---
> 
> cd 3rdparty/libprocess/3rdparty/stout
> ./boostrap
> ./configure
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Sept. 28, 2015, 11:42 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Updated includes.


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


Repository: mesos


Description
---

Introduces the authenticator manager, which is a class which handles the actual 
authentication procedure during the execution of `ProcessManager::handle()` and 
it also takes care of the life cycle of instances of http::Authenticator.

No tests are added at this point since no public API is generated, the goal of 
this patch is to implement the manager and verify nothing breaks afterwards. 
Authenticator manager tests proper come in a latter patch.


Diffs (updated)
-

  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/include/process/http.hpp 
fbd6cf7967173495875a8ea90ed28ade88b982a2 
  3rdparty/libprocess/src/process.cpp c03fba47435505484704e6c8a61e56123859781f 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

[Jan Schlicht]

> On Sept. 25, 2015, 3:35 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 609
> > 
> >
> > 1. You can also use `inline`, that's what we usually do.
> > 2. `static_assert` can be replaced by pulling the type into the 
> > fucntion signature, not by `Try<>`.
> > 3. What I think Jan means instead is that instead of calling an 
> > `ABORT`, why not using a `Try<>`?
> 
> Klaus Ma wrote:
> 1. Yes, `inline` is fine according to the document; i'll have a try 
> (seems enhanced in C++11 :) ).
> 2. `static_assert` will issue a **compile-time** error if test failed, 
> `Try<>` will check the result at **run-time**; so prefer to `static_assert` 
> for `Array protobuf(...)`.

Changes are looking good! My apologies, I've made the mistake of addressing to 
seperate issues in one issue... The `Try<>` stuff doesn't relate to the 
`static_assert`. Let me try again at a diffent position :)


- Jan


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


On Sept. 27, 2015, 3:34 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38342/
> ---
> 
> (Updated Sept. 27, 2015, 3:34 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 2285ce9 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 68328a2 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 8ebb798 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 34eb6d0 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 920f5c9 
> 
> Diff: https://reviews.apache.org/r/38342/diff/
> 
> 
> Testing
> ---
> 
> cd 3rdparty/libprocess/3rdparty/stout
> ./boostrap
> ./configure
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37996: Added property manager

[Alexander Rojas]

> On Sept. 25, 2015, 11:12 a.m., Adam B wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/properties.hpp, line 67
> > 
> >
> > Why use a string instead of a stout::Path?

I created the issue MESOS-3531 to change `http::Request` and `http::Response` 
to use `Path`. I will create a similar dependent issue once this patch gets 
landed.


- Alexander


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


On Sept. 23, 2015, 1:17 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37996/
> ---
> 
> (Updated Sept. 23, 2015, 1:17 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the `PathInheritedProperties` class which allows to create a tree 
> where nodes can be tag with some property. This property is then inherited by 
> children nodes.
> 
> Two behaviors are implemented, overriding, i.e. Adding a property to the 
> child node of another node with a property already will result in the 
> ancestor property being lost. The second behavior, accumulating, takes a 
> function and accumulates
> properties of all ancestors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 76e1674e08bbe65a4fdf86731823a61f231d6d12 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/properties.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> baa648aacfd5204c33e102b58126862729fbb612 
>   3rdparty/libprocess/3rdparty/stout/tests/properties_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38747: Adding digest utilities

[Neil Conway]

> On Sept. 28, 2015, 6:32 p.m., Neil Conway wrote:
> > 3rdparty/libprocess/include/process/digest.hpp, line 222
> > 
> >
> > Why do we initialize this to `{0}`?
> 
> Jojy Varghese wrote:
> initialization of auto variables

IMO this is a little misleading: the array is actually initialized by the for 
loop that follows below. I think it would be a bit clearer to omit the 
initializer, and then explicitly NUL-terminate the array after the for loop.


- Neil


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


On Sept. 28, 2015, 10:14 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> ---
> 
> (Updated Sept. 28, 2015, 10:14 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added SHA256, SHA512 implementation.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> a14b5b8fe22d3e75bef3c716ae7865ddaf132927 
>   3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38747/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38817: Added a SIGPIPE handler for the libprocess tests.

[Cong Wang]

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



3rdparty/libprocess/src/tests/main.cpp (line 41)


Here you probably want a signal string (strsignal ()) instead of a raw 
value.


- Cong Wang


On Sept. 28, 2015, 7:29 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38817/
> ---
> 
> (Updated Sept. 28, 2015, 7:29 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We're currently experiencing SIGPIPEs on the build, this will give us a stack 
> trace.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/main.cpp 
> eec0c5f085dfed6dfeb3941528043dbd2eadcb59 
> 
> Diff: https://reviews.apache.org/r/38817/diff/
> 
> 
> Testing
> ---
> 
> Tested on CentOS and OS X:
> 
> On OS X, before:
> ```
> [ RUN  ] IOTest.Write
> make[5]: *** [check-local] Broken pipe: 13
> ```
> 
> After:
> ```
> [ RUN  ] IOTest.Write
> W0928 12:16:46.138653 190414848 main.cpp:52] RAW: Received signal SIGPIPE; 
> escalating to SIGABRT
> *** Aborted at 1443467806 (unix time) try "date -d @1443467806" if you are 
> using GNU date ***
> PC: @ 0x7fff827b5c82 __kill
> *** SIGABRT (@0x7fff827b5c82) received by PID 88406 (TID 0x10b598000) stack 
> trace: ***
> @ 0x7fff8f155f1a _sigtramp
> @ 0x7fff650e327c (unknown)
> @ 0x7fff8f155f1a _sigtramp
> @0x10b5960b0 (unknown)
> @ 0x7fff8fd3eb6f std::__1::mutex::lock()
> @0x10a47d439 
> _ZZ11synchronizeINSt3__15mutexEE12SynchronizedIT_EPS3_ENKUlPS1_E_clES6_
> @0x10a47d418 
> _ZZ11synchronizeINSt3__15mutexEE12SynchronizedIT_EPS3_ENUlPS1_E_8__invokeES6_
> @0x10a47d269 Synchronized<>::Synchronized()
> @0x10a47d1ed Synchronized<>::Synchronized()
> @0x10a3df30c synchronize<>()
> @0x10a3f1b8e Gate::open()
> @0x10a3d3a07 process::ProcessManager::enqueue()
> @0x10a3cde0a process::ProcessBase::enqueue()
> @0x10a3cec43 process::SocketManager::exited()
> @0x10a3d5626 process::ProcessManager::cleanup()
> @0x10a3d442b process::ProcessManager::resume()
> @0x10a3dbe3c 
> process::ProcessManager::init_threads()::$_1::operator()()
> @0x10a3dbac2 
> _ZNSt3__114__thread_proxyINS_5tupleIJNS_6__bindIZN7process14ProcessManager12init_threadsEvE3$_1JNS_17reference_wrapperIKNS_6atomicIbPvSD_
> @ 0x7fff8885105a _pthread_body
> @ 0x7fff88850fd7 _pthread_start
> @ 0x7fff8884e3ed thread_start
> ```
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

[Chris Chen]

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

(Updated Sept. 28, 2015, 11:45 p.m.)


Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

Problem:
The Zookeeeper C client does makes certain assertions about the ordering
of ping packets that the Java client does not. An alternate implementation
of the Zookeeper server would then break the C client while working
correctly with the Java client.

Solution:
Port change from ZOOKEEPER-2253 to make C client handle pings similarly
to the Java client.

Result:
C client and Java clients behave the same when dealing with an out-of-order
ping packet.


Diffs (updated)
-

  3rdparty/CMakeLists.txt ac5c25a8797a687e84384682975ab99fb3e30448 
  3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
  3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 

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


Testing
---

Tested from upstream; Running mesos check succeeds.


Thanks,

Chris Chen

Review Request 38828: docker: containerizer: fixed double-closing of STDIN in error handling.

[Chi Zhang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

The slave will destroy executors that are not launched successfully, which will
close all open file descriptors, thus the extra close is not needed.


Diffs
-

  src/slave/containerizer/docker.cpp efa37266368ee12fea9134b35ebc5047a2820f94 

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


Testing
---

make check


Thanks,

Chi Zhang

Re: Review Request 38443: Added layerid information to ManifestResponse

[Jojy Varghese]

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

(Updated Sept. 28, 2015, 10:50 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

review comments addressed.


Repository: mesos


Description
---

Added layerid information to ManifestResponse


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Review Request 38826: CMake: Added support for libevent for Unix builds.

[Alex Clemmer]

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

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


Repository: mesos


Description
---

CMake: Added support for libevent for Unix builds.


Diffs
-

  CMakeLists.txt c866f6d85d629ca9c3a987fa2c652bc63a7ff1be 
  src/CMakeLists.txt 891f951f3107ece29b7923b7a3cc414e2ea56983 
  src/slave/cmake/SlaveConfigure.cmake 230e574f3ddc6aa12cc74105de7201a33dd7cdab 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 38827: CMake: Expanded support for compiling much of the master.

[Alex Clemmer]

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

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


Repository: mesos


Description
---

CMake: Expanded support for compiling much of the master.


Diffs
-

  cmake/CompilationConfigure.cmake 98a08ee99299c873577b0b2a93a153f82ef75b29 
  src/CMakeLists.txt 891f951f3107ece29b7923b7a3cc414e2ea56983 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 38825: CMake: Plumb `ENABLE_LIBEVENT` through libprocess config scripts.

[Alex Clemmer]

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

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


Repository: mesos


Description
---

CMake: Plumb `ENABLE_LIBEVENT` through libprocess config scripts.


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
31d77d5629dba7ee0d1a4ceb7b60899f6047165f 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
9c369e2c94abcd2dfa463f98fb3b6527033064ab 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake 
076c8836d9d53b8bc69330a46a7e1d3039aa476b 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 38817: Added a SIGPIPE handler for the libprocess tests.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38817]

All tests passed.

- Mesos ReviewBot


On Sept. 28, 2015, 7:29 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38817/
> ---
> 
> (Updated Sept. 28, 2015, 7:29 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We're currently experiencing SIGPIPEs on the build, this will give us a stack 
> trace.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/main.cpp 
> eec0c5f085dfed6dfeb3941528043dbd2eadcb59 
> 
> Diff: https://reviews.apache.org/r/38817/diff/
> 
> 
> Testing
> ---
> 
> Tested on CentOS and OS X:
> 
> On OS X, before:
> ```
> [ RUN  ] IOTest.Write
> make[5]: *** [check-local] Broken pipe: 13
> ```
> 
> After:
> ```
> [ RUN  ] IOTest.Write
> W0928 12:16:46.138653 190414848 main.cpp:52] RAW: Received signal SIGPIPE; 
> escalating to SIGABRT
> *** Aborted at 1443467806 (unix time) try "date -d @1443467806" if you are 
> using GNU date ***
> PC: @ 0x7fff827b5c82 __kill
> *** SIGABRT (@0x7fff827b5c82) received by PID 88406 (TID 0x10b598000) stack 
> trace: ***
> @ 0x7fff8f155f1a _sigtramp
> @ 0x7fff650e327c (unknown)
> @ 0x7fff8f155f1a _sigtramp
> @0x10b5960b0 (unknown)
> @ 0x7fff8fd3eb6f std::__1::mutex::lock()
> @0x10a47d439 
> _ZZ11synchronizeINSt3__15mutexEE12SynchronizedIT_EPS3_ENKUlPS1_E_clES6_
> @0x10a47d418 
> _ZZ11synchronizeINSt3__15mutexEE12SynchronizedIT_EPS3_ENUlPS1_E_8__invokeES6_
> @0x10a47d269 Synchronized<>::Synchronized()
> @0x10a47d1ed Synchronized<>::Synchronized()
> @0x10a3df30c synchronize<>()
> @0x10a3f1b8e Gate::open()
> @0x10a3d3a07 process::ProcessManager::enqueue()
> @0x10a3cde0a process::ProcessBase::enqueue()
> @0x10a3cec43 process::SocketManager::exited()
> @0x10a3d5626 process::ProcessManager::cleanup()
> @0x10a3d442b process::ProcessManager::resume()
> @0x10a3dbe3c 
> process::ProcessManager::init_threads()::$_1::operator()()
> @0x10a3dbac2 
> _ZNSt3__114__thread_proxyINS_5tupleIJNS_6__bindIZN7process14ProcessManager12init_threadsEvE3$_1JNS_17reference_wrapperIKNS_6atomicIbPvSD_
> @ 0x7fff8885105a _pthread_body
> @ 0x7fff88850fd7 _pthread_start
> @ 0x7fff8884e3ed thread_start
> ```
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 38747: Adding digest utilities

[Jojy Varghese]

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

(Updated Sept. 28, 2015, 10:14 p.m.)


Review request for mesos, Gilbert Song and Timothy Chen.


Changes
---

review comments.


Repository: mesos


Description
---

Added SHA256, SHA512 implementation.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
a14b5b8fe22d3e75bef3c716ae7865ddaf132927 
  3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38747: Adding digest utilities

[Jojy Varghese]

> On Sept. 28, 2015, 7:17 p.m., Jiang Yan Xu wrote:
> > High-level comments:
> > 
> > - This implementation relies on USE_SSL_SOCKET which is tied to 
> > `--enable_ssl`. Conceptually the two should be decoupled. Ian Downes has an 
> > implementation which uses the system commands that are widely distributed 
> > on *nix boxes: https://reviews.apache.org/r/34138/. Can such an 
> > implemenation be used as a fallback at least?
> > - Can the implentation be put in a cpp file and leave only public (the 
> > recommended way of using this utility) methods in the header (e.g. 
> > `process::verify(...)` and `process::digest()`)?

Thanks for the suggestion and history.
- digest.hpp's process::digest and process::verify has default implementations 
(when USE_SSL_SOCKET is not available). So thats the fallback for now.
- Since this implementation is dependent on using traits/templates, the 
implementation is in hpp file
- I can work on adding Ian's implementation in this patch or let Ian add it as 
an extension to this patch.


- Jojy


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


On Sept. 28, 2015, 10:14 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> ---
> 
> (Updated Sept. 28, 2015, 10:14 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added SHA256, SHA512 implementation.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
>   3rdparty/libprocess/include/Makefile.am 
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
>   3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> a14b5b8fe22d3e75bef3c716ae7865ddaf132927 
>   3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38747/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38817: Added a SIGPIPE handler for the libprocess tests.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Sept. 28, 2015, 7:29 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38817/
> ---
> 
> (Updated Sept. 28, 2015, 7:29 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We're currently experiencing SIGPIPEs on the build, this will give us a stack 
> trace.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/main.cpp 
> eec0c5f085dfed6dfeb3941528043dbd2eadcb59 
> 
> Diff: https://reviews.apache.org/r/38817/diff/
> 
> 
> Testing
> ---
> 
> Tested on CentOS and OS X:
> 
> On OS X, before:
> ```
> [ RUN  ] IOTest.Write
> make[5]: *** [check-local] Broken pipe: 13
> ```
> 
> After:
> ```
> [ RUN  ] IOTest.Write
> W0928 12:16:46.138653 190414848 main.cpp:52] RAW: Received signal SIGPIPE; 
> escalating to SIGABRT
> *** Aborted at 1443467806 (unix time) try "date -d @1443467806" if you are 
> using GNU date ***
> PC: @ 0x7fff827b5c82 __kill
> *** SIGABRT (@0x7fff827b5c82) received by PID 88406 (TID 0x10b598000) stack 
> trace: ***
> @ 0x7fff8f155f1a _sigtramp
> @ 0x7fff650e327c (unknown)
> @ 0x7fff8f155f1a _sigtramp
> @0x10b5960b0 (unknown)
> @ 0x7fff8fd3eb6f std::__1::mutex::lock()
> @0x10a47d439 
> _ZZ11synchronizeINSt3__15mutexEE12SynchronizedIT_EPS3_ENKUlPS1_E_clES6_
> @0x10a47d418 
> _ZZ11synchronizeINSt3__15mutexEE12SynchronizedIT_EPS3_ENUlPS1_E_8__invokeES6_
> @0x10a47d269 Synchronized<>::Synchronized()
> @0x10a47d1ed Synchronized<>::Synchronized()
> @0x10a3df30c synchronize<>()
> @0x10a3f1b8e Gate::open()
> @0x10a3d3a07 process::ProcessManager::enqueue()
> @0x10a3cde0a process::ProcessBase::enqueue()
> @0x10a3cec43 process::SocketManager::exited()
> @0x10a3d5626 process::ProcessManager::cleanup()
> @0x10a3d442b process::ProcessManager::resume()
> @0x10a3dbe3c 
> process::ProcessManager::init_threads()::$_1::operator()()
> @0x10a3dbac2 
> _ZNSt3__114__thread_proxyINS_5tupleIJNS_6__bindIZN7process14ProcessManager12init_threadsEvE3$_1JNS_17reference_wrapperIKNS_6atomicIbPvSD_
> @ 0x7fff8885105a _pthread_body
> @ 0x7fff88850fd7 _pthread_start
> @ 0x7fff8884e3ed thread_start
> ```
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 38825: CMake: Plumb `ENABLE_LIBEVENT` through libprocess config scripts.

[Alex Clemmer]

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

(Updated Sept. 28, 2015, 10:58 p.m.)


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


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


Repository: mesos


Description
---

CMake: Plumb `ENABLE_LIBEVENT` through libprocess config scripts.


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
31d77d5629dba7ee0d1a4ceb7b60899f6047165f 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
9c369e2c94abcd2dfa463f98fb3b6527033064ab 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake 
076c8836d9d53b8bc69330a46a7e1d3039aa476b 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 38826: CMake: Added support for libevent for Unix builds.

[Alex Clemmer]

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

(Updated Sept. 28, 2015, 10:58 p.m.)


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


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


Repository: mesos


Description
---

CMake: Added support for libevent for Unix builds.


Diffs
-

  CMakeLists.txt c866f6d85d629ca9c3a987fa2c652bc63a7ff1be 
  src/CMakeLists.txt 891f951f3107ece29b7923b7a3cc414e2ea56983 
  src/slave/cmake/SlaveConfigure.cmake 230e574f3ddc6aa12cc74105de7201a33dd7cdab 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

[Mesos ReviewBot]

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


Bad review!

Reviews applied: []

Error:
 No reviewers specified. Please find a reviewer by asking on JIRA or the 
mailing list.

- Mesos ReviewBot


On Sept. 28, 2015, 8:59 p.m., Chris Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38815/
> ---
> 
> (Updated Sept. 28, 2015, 8:59 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3391
> https://issues.apache.org/jira/browse/MESOS-3391
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Problem:
> The Zookeeeper C client does makes certain assertions about the ordering
> of ping packets that the Java client does not. An alternate implementation
> of the Zookeeper server would then break the C client while working
> correctly with the Java client.
> 
> Solution:
> Port change from ZOOKEEPER-2253 to make C client handle pings similarly
> to the Java client.
> 
> Result:
> C client and Java clients behave the same when dealing with an out-of-order
> ping packet.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
> 
> Diff: https://reviews.apache.org/r/38815/diff/
> 
> 
> Testing
> ---
> 
> Tested from upstream; Running mesos check succeeds.
> 
> 
> Thanks,
> 
> Chris Chen
> 
>

Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

[Chris Chen]

> On Sept. 28, 2015, 11:18 p.m., Mesos ReviewBot wrote:
> > Bad review!
> > 
> > Reviews applied: []
> > 
> > Error:
> >  No reviewers specified. Please find a reviewer by asking on JIRA or the 
> > mailing list.

FINE.


- Chris


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


On Sept. 28, 2015, 11:24 p.m., Chris Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38815/
> ---
> 
> (Updated Sept. 28, 2015, 11:24 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3391
> https://issues.apache.org/jira/browse/MESOS-3391
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Problem:
> The Zookeeeper C client does makes certain assertions about the ordering
> of ping packets that the Java client does not. An alternate implementation
> of the Zookeeper server would then break the C client while working
> correctly with the Java client.
> 
> Solution:
> Port change from ZOOKEEPER-2253 to make C client handle pings similarly
> to the Java client.
> 
> Result:
> C client and Java clients behave the same when dealing with an out-of-order
> ping packet.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
> 
> Diff: https://reviews.apache.org/r/38815/diff/
> 
> 
> Testing
> ---
> 
> Tested from upstream; Running mesos check succeeds.
> 
> 
> Thanks,
> 
> Chris Chen
> 
>

Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

[Jiang Yan Xu]

> On Sept. 28, 2015, 4:18 p.m., Mesos ReviewBot wrote:
> > Bad review!
> > 
> > Reviews applied: []
> > 
> > Error:
> >  No reviewers specified. Please find a reviewer by asking on JIRA or the 
> > mailing list.
> 
> Chris Chen wrote:
> FINE.

Doesn't look like the change to 3rdparty/CMakeLists.txt is attached?


- Jiang Yan


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


On Sept. 28, 2015, 4:24 p.m., Chris Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38815/
> ---
> 
> (Updated Sept. 28, 2015, 4:24 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3391
> https://issues.apache.org/jira/browse/MESOS-3391
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Problem:
> The Zookeeeper C client does makes certain assertions about the ordering
> of ping packets that the Java client does not. An alternate implementation
> of the Zookeeper server would then break the C client while working
> correctly with the Java client.
> 
> Solution:
> Port change from ZOOKEEPER-2253 to make C client handle pings similarly
> to the Java client.
> 
> Result:
> C client and Java clients behave the same when dealing with an out-of-order
> ping packet.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
> 
> Diff: https://reviews.apache.org/r/38815/diff/
> 
> 
> Testing
> ---
> 
> Tested from upstream; Running mesos check succeeds.
> 
> 
> Thanks,
> 
> Chris Chen
> 
>

Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

[Jiang Yan Xu]

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

Ship it!


LGTM. Will commit it after reviewbot gives it a pass.

- Jiang Yan Xu


On Sept. 28, 2015, 11:17 a.m., Chris Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38815/
> ---
> 
> (Updated Sept. 28, 2015, 11:17 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3391
> https://issues.apache.org/jira/browse/MESOS-3391
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Problem:
> The Zookeeeper C client does makes certain assertions about the ordering
> of ping packets that the Java client does not. An alternate implementation
> of the Zookeeper server would then break the C client while working
> correctly with the Java client.
> 
> Solution:
> Port change from ZOOKEEPER-2253 to make C client handle pings similarly
> to the Java client.
> 
> Result:
> C client and Java clients behave the same when dealing with an out-of-order
> ping packet.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
> 
> Diff: https://reviews.apache.org/r/38815/diff/
> 
> 
> Testing
> ---
> 
> Tested from upstream; Running mesos check suite currently.
> 
> 
> Thanks,
> 
> Chris Chen
> 
>

Review Request 38817: Added a SIGPIPE handler for the libprocess tests.

[Ben Mahler]

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

Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

We're currently experiencing SIGPIPEs on the build, this will give us a stack 
trace.


Diffs
-

  3rdparty/libprocess/src/tests/main.cpp 
eec0c5f085dfed6dfeb3941528043dbd2eadcb59 

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


Testing
---

Tested on CentOS and OS X:

On OS X, before:
```
[ RUN  ] IOTest.Write
make[5]: *** [check-local] Broken pipe: 13
```

After:
```
[ RUN  ] IOTest.Write
W0928 12:16:46.138653 190414848 main.cpp:52] RAW: Received signal SIGPIPE; 
escalating to SIGABRT
*** Aborted at 1443467806 (unix time) try "date -d @1443467806" if you are 
using GNU date ***
PC: @ 0x7fff827b5c82 __kill
*** SIGABRT (@0x7fff827b5c82) received by PID 88406 (TID 0x10b598000) stack 
trace: ***
@ 0x7fff8f155f1a _sigtramp
@ 0x7fff650e327c (unknown)
@ 0x7fff8f155f1a _sigtramp
@0x10b5960b0 (unknown)
@ 0x7fff8fd3eb6f std::__1::mutex::lock()
@0x10a47d439 
_ZZ11synchronizeINSt3__15mutexEE12SynchronizedIT_EPS3_ENKUlPS1_E_clES6_
@0x10a47d418 
_ZZ11synchronizeINSt3__15mutexEE12SynchronizedIT_EPS3_ENUlPS1_E_8__invokeES6_
@0x10a47d269 Synchronized<>::Synchronized()
@0x10a47d1ed Synchronized<>::Synchronized()
@0x10a3df30c synchronize<>()
@0x10a3f1b8e Gate::open()
@0x10a3d3a07 process::ProcessManager::enqueue()
@0x10a3cde0a process::ProcessBase::enqueue()
@0x10a3cec43 process::SocketManager::exited()
@0x10a3d5626 process::ProcessManager::cleanup()
@0x10a3d442b process::ProcessManager::resume()
@0x10a3dbe3c 
process::ProcessManager::init_threads()::$_1::operator()()
@0x10a3dbac2 
_ZNSt3__114__thread_proxyINS_5tupleIJNS_6__bindIZN7process14ProcessManager12init_threadsEvE3$_1JNS_17reference_wrapperIKNS_6atomicIbPvSD_
@ 0x7fff8885105a _pthread_body
@ 0x7fff88850fd7 _pthread_start
@ 0x7fff8884e3ed thread_start
```


Thanks,

Ben Mahler

Re: Review Request 37703: Add docker exec command.

[haosdent huang]

> On Sept. 29, 2015, 1:24 a.m., Timothy Chen wrote:
> > src/docker/docker.cpp, line 1164
> > 
> >
> > We're now supporting optional docker socket (-H), if you look at other 
> > commands.
> > 
> > Perhaps we should make this into a helper function so we don't have to 
> > do this in every command.

Oh, thx. I would update and rebase this at this weekend.


- haosdent


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


On Sept. 9, 2015, 9:15 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37703/
> ---
> 
> (Updated Sept. 9, 2015, 9:15 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3291
> https://issues.apache.org/jira/browse/MESOS-3291
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add docker exec command.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp aaf8884a20901ad3a440d545d4b6316b87fc3403 
>   src/docker/docker.cpp 553e831029454d6d423842915b8bbfcaf19fa7f9 
>   src/tests/containerizer/docker_tests.cpp 
> babc7d8da4ed9d13b14bd69decd7f27fc7dfde89 
> 
> Diff: https://reviews.apache.org/r/37703/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38835: Ignore SIGPIPE when finalizing libevent.

[Joris Van Remoortere]

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

(Updated Sept. 29, 2015, 1:46 a.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

This fixes a Broken Pipe after unblocking SIGPIPE.


Diffs (updated)
-

  3rdparty/libprocess/src/libevent.cpp 00e0f08dd99fa60a92c1730c7ec899f2f7f55676 

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


Testing
---

make check with libevent and ssl enabled.


Thanks,

Joris Van Remoortere

Re: Review Request 38835: Ignore SIGPIPE when finalizing libevent.

[Ben Mahler]

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

Ship it!


Thanks!


3rdparty/libprocess/src/libevent.cpp (line 121)


Would be nice to keep this outside the suppress block, since we set to true 
outside the supress block above.


- Ben Mahler


On Sept. 29, 2015, 1:46 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38835/
> ---
> 
> (Updated Sept. 29, 2015, 1:46 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3540
> https://issues.apache.org/jira/browse/MESOS-3540
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fixes a Broken Pipe after unblocking SIGPIPE.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent.cpp 
> 00e0f08dd99fa60a92c1730c7ec899f2f7f55676 
> 
> Diff: https://reviews.apache.org/r/38835/diff/
> 
> 
> Testing
> ---
> 
> make check with libevent and ssl enabled.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Review Request 38837: CMake: Disable slave build on Windows.

[Alex Clemmer]

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

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


Repository: mesos


Description
---

CMake: Disable slave build on Windows.


Diffs
-

  CMakeLists.txt c866f6d85d629ca9c3a987fa2c652bc63a7ff1be 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 38827: CMake: Expanded support for compiling much of the master.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38825, 38826, 38827]

All tests passed.

- Mesos ReviewBot


On Sept. 28, 2015, 10:56 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38827/
> ---
> 
> (Updated Sept. 28, 2015, 10:56 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Expanded support for compiling much of the master.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 98a08ee99299c873577b0b2a93a153f82ef75b29 
>   src/CMakeLists.txt 891f951f3107ece29b7923b7a3cc414e2ea56983 
> 
> Diff: https://reviews.apache.org/r/38827/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38817: Added a SIGPIPE handler for the libprocess tests.

[Ben Mahler]

> On Sept. 28, 2015, 10:52 p.m., Cong Wang wrote:
> > 3rdparty/libprocess/src/tests/main.cpp, line 41
> > 
> >
> > Here you probably want a signal string (strsignal ()) instead of a raw 
> > value.

>From what I can tell, `strsignal` isn't listed as an async-signal-safe 
>function.


- Ben


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


On Sept. 28, 2015, 7:29 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38817/
> ---
> 
> (Updated Sept. 28, 2015, 7:29 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We're currently experiencing SIGPIPEs on the build, this will give us a stack 
> trace.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/main.cpp 
> eec0c5f085dfed6dfeb3941528043dbd2eadcb59 
> 
> Diff: https://reviews.apache.org/r/38817/diff/
> 
> 
> Testing
> ---
> 
> Tested on CentOS and OS X:
> 
> On OS X, before:
> ```
> [ RUN  ] IOTest.Write
> make[5]: *** [check-local] Broken pipe: 13
> ```
> 
> After:
> ```
> [ RUN  ] IOTest.Write
> W0928 12:16:46.138653 190414848 main.cpp:52] RAW: Received signal SIGPIPE; 
> escalating to SIGABRT
> *** Aborted at 1443467806 (unix time) try "date -d @1443467806" if you are 
> using GNU date ***
> PC: @ 0x7fff827b5c82 __kill
> *** SIGABRT (@0x7fff827b5c82) received by PID 88406 (TID 0x10b598000) stack 
> trace: ***
> @ 0x7fff8f155f1a _sigtramp
> @ 0x7fff650e327c (unknown)
> @ 0x7fff8f155f1a _sigtramp
> @0x10b5960b0 (unknown)
> @ 0x7fff8fd3eb6f std::__1::mutex::lock()
> @0x10a47d439 
> _ZZ11synchronizeINSt3__15mutexEE12SynchronizedIT_EPS3_ENKUlPS1_E_clES6_
> @0x10a47d418 
> _ZZ11synchronizeINSt3__15mutexEE12SynchronizedIT_EPS3_ENUlPS1_E_8__invokeES6_
> @0x10a47d269 Synchronized<>::Synchronized()
> @0x10a47d1ed Synchronized<>::Synchronized()
> @0x10a3df30c synchronize<>()
> @0x10a3f1b8e Gate::open()
> @0x10a3d3a07 process::ProcessManager::enqueue()
> @0x10a3cde0a process::ProcessBase::enqueue()
> @0x10a3cec43 process::SocketManager::exited()
> @0x10a3d5626 process::ProcessManager::cleanup()
> @0x10a3d442b process::ProcessManager::resume()
> @0x10a3dbe3c 
> process::ProcessManager::init_threads()::$_1::operator()()
> @0x10a3dbac2 
> _ZNSt3__114__thread_proxyINS_5tupleIJNS_6__bindIZN7process14ProcessManager12init_threadsEvE3$_1JNS_17reference_wrapperIKNS_6atomicIbPvSD_
> @ 0x7fff8885105a _pthread_body
> @ 0x7fff88850fd7 _pthread_start
> @ 0x7fff8884e3ed thread_start
> ```
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 38822: Added default upgrade steps to 0.24.X to 0.25.X section

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38822]

All tests passed.

- Mesos ReviewBot


On Sept. 28, 2015, 9 p.m., Niklas Nielsen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38822/
> ---
> 
> (Updated Sept. 28, 2015, 9 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, switched to 'mcypark', and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 8e33b9b90654a43a452f97fc66e969ecb0f3d6dd 
> 
> Diff: https://reviews.apache.org/r/38822/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>

Re: Review Request 38814: add test cases for sha512 digest verifier

[Jojy Varghese]

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



3rdparty/libprocess/src/tests/digest_tests.cpp (line 82)


You might have to use the TemporaryDrectoryTest fixture for this test and 
any test related to creating temporary file.



3rdparty/libprocess/src/tests/digest_tests.cpp (line 96)


This was a bug in my test. Please see my latest tests.



3rdparty/libprocess/src/tests/digest_tests.cpp (line 121)


same as above


- Jojy Varghese


On Sept. 28, 2015, 6:06 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38814/
> ---
> 
> (Updated Sept. 28, 2015, 6:06 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> add test cases for sha512 digest verifier
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38814/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu 14.04)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 38774: state: fix file descriptor leak

[Ben Mahler]

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

Ship it!


I'll get this committed shortly, thanks!


src/slave/state.cpp (line 655)


Ditto here



src/slave/state.cpp (line 731)


newline after this :)


- Ben Mahler


On Sept. 28, 2015, 8:42 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38774/
> ---
> 
> (Updated Sept. 28, 2015, 8:42 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> state: fix file descriptor leak
> 
> 
> Diffs
> -
> 
>   src/slave/state.cpp 47c66dc80d57db86981769d404c2c8c7c972fec0 
> 
> Diff: https://reviews.apache.org/r/38774/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 38828: docker: containerizer: fixed double-closing of STDIN in error handling.

[Ben Mahler]

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



src/slave/containerizer/docker.cpp 


I believe these were in place to terminate the subprocess, so perhaps a 
killtree with SIGTERM would be more appropriate to replace these with.. 
thoughts? Perhaps reach out to tim chen for this code.


- Ben Mahler


On Sept. 28, 2015, 10:50 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38828/
> ---
> 
> (Updated Sept. 28, 2015, 10:50 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The slave will destroy executors that are not launched successfully, which 
> will
> close all open file descriptors, thus the extra close is not needed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp efa37266368ee12fea9134b35ebc5047a2820f94 
> 
> Diff: https://reviews.apache.org/r/38828/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

[Alex Clemmer]

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



3rdparty/CMakeLists.txt (line 41)


Looks like the rest of this block is intented with 2 spaces. Can we do the 
same for this comment?


- Alex Clemmer


On Sept. 28, 2015, 11:45 p.m., Chris Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38815/
> ---
> 
> (Updated Sept. 28, 2015, 11:45 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3391
> https://issues.apache.org/jira/browse/MESOS-3391
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Problem:
> The Zookeeeper C client does makes certain assertions about the ordering
> of ping packets that the Java client does not. An alternate implementation
> of the Zookeeper server would then break the C client while working
> correctly with the Java client.
> 
> Solution:
> Port change from ZOOKEEPER-2253 to make C client handle pings similarly
> to the Java client.
> 
> Result:
> C client and Java clients behave the same when dealing with an out-of-order
> ping packet.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt ac5c25a8797a687e84384682975ab99fb3e30448 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
> 
> Diff: https://reviews.apache.org/r/38815/diff/
> 
> 
> Testing
> ---
> 
> Tested from upstream; Running mesos check succeeds.
> 
> 
> Thanks,
> 
> Chris Chen
> 
>

Re: Review Request 38809: socket: refactor to use Option and fix file descriptor leaks.

[Ben Mahler]

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



3rdparty/libprocess/include/process/socket.hpp (lines 64 - 65)


Can you fix the wrapping here? Also, I would suggest s/socketfd/s/ to keep 
the same as before, since `socketfd` is a bit unusual for our code base. Also 
the doxygen a few lines up refers to 's' still :)



3rdparty/libprocess/src/socket.cpp (lines 42 - 44)


Just as an aside, it's unfortunate the caller has to do this, we should 
consider doing the same thing that is done in os/posix/open.hpp with O_CLOEXEC:


https://github.com/apache/mesos/blob/5aa050bfaa7338b010b1a522406bde0f15015259/3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/open.hpp#L33



3rdparty/libprocess/src/socket.cpp (lines 77 - 79)


How about we store a bool for this at the top instead of having multiple 
variables?

```
// If the caller passed in a descriptor, we do
// not own its lifecycle and must not close it!
bool owned = s.isNone();

...
```


- Ben Mahler


On Sept. 28, 2015, 9:07 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38809/
> ---
> 
> (Updated Sept. 28, 2015, 9:07 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3519
> https://issues.apache.org/jira/browse/MESOS-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> socket: refactor to use Option and fix file descriptor leaks.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/socket.hpp 
> 817cb3711742871630a13b966563296644008f21 
>   3rdparty/libprocess/src/socket.cpp 5879423bd92c5805353a962cbd822a993b0568c5 
> 
> Diff: https://reviews.apache.org/r/38809/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

[Chris Chen]

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

(Updated Sept. 29, 2015, 12:50 a.m.)


Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

Problem:
The Zookeeeper C client does makes certain assertions about the ordering
of ping packets that the Java client does not. An alternate implementation
of the Zookeeper server would then break the C client while working
correctly with the Java client.

Solution:
Port change from ZOOKEEPER-2253 to make C client handle pings similarly
to the Java client.

Result:
C client and Java clients behave the same when dealing with an out-of-order
ping packet.


Diffs (updated)
-

  3rdparty/CMakeLists.txt ac5c25a8797a687e84384682975ab99fb3e30448 
  3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
  3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 

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


Testing
---

Tested from upstream; Running mesos check succeeds.


Thanks,

Chris Chen

Re: Review Request 38747: Adding digest utilities

[Jojy Varghese]

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

(Updated Sept. 29, 2015, 12:19 a.m.)


Review request for mesos, Gilbert Song and Timothy Chen.


Changes
---

Review comments, fixed bug in test


Repository: mesos


Description
---

Added SHA256, SHA512 implementation.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
a14b5b8fe22d3e75bef3c716ae7865ddaf132927 
  3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38823: io: fixed a file descriptor leak in io::redirect.

[Ben Mahler]

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

Ship it!


Ship It!

- Ben Mahler


On Sept. 28, 2015, 9:36 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38823/
> ---
> 
> (Updated Sept. 28, 2015, 9:36 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> io: fixed a file descriptor leak in io::redirect.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/io.cpp d17ccf7713debc43f5c2ca1aeb9a1ca7ed03b4a6 
> 
> Diff: https://reviews.apache.org/r/38823/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 38828: docker: containerizer: fixed double-closing of STDIN in error handling.

[Chi Zhang]

> On Sept. 29, 2015, 12:31 a.m., Ben Mahler wrote:
> > src/slave/containerizer/docker.cpp, lines 926-928
> > 
> >
> > I believe these were in place to terminate the subprocess, so perhaps a 
> > killtree with SIGTERM would be more appropriate to replace these with.. 
> > thoughts? Perhaps reach out to tim chen for this code.

Yeah, I checked the mesos containerizer code, which in case just returns a 
failure and waits for the slave to issue destroy when it gets the failure, but 
I will pull Tim in.


- Chi


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


On Sept. 28, 2015, 10:50 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38828/
> ---
> 
> (Updated Sept. 28, 2015, 10:50 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The slave will destroy executors that are not launched successfully, which 
> will
> close all open file descriptors, thus the extra close is not needed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp efa37266368ee12fea9134b35ebc5047a2820f94 
> 
> Diff: https://reviews.apache.org/r/38828/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 38828: docker: containerizer: fixed double-closing of STDIN in error handling.

[Chi Zhang]

> On Sept. 29, 2015, 12:31 a.m., Ben Mahler wrote:
> > src/slave/containerizer/docker.cpp, lines 926-928
> > 
> >
> > I believe these were in place to terminate the subprocess, so perhaps a 
> > killtree with SIGTERM would be more appropriate to replace these with.. 
> > thoughts? Perhaps reach out to tim chen for this code.
> 
> Chi Zhang wrote:
> Yeah, I checked the mesos containerizer code, which in case just returns 
> a failure and waits for the slave to issue destroy when it gets the failure, 
> but I will pull Tim in.

Hi Tim, could you take a look at this patch please?


- Chi


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


On Sept. 29, 2015, 12:50 a.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38828/
> ---
> 
> (Updated Sept. 29, 2015, 12:50 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The slave will destroy executors that are not launched successfully, which 
> will
> close all open file descriptors, thus the extra close is not needed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp efa37266368ee12fea9134b35ebc5047a2820f94 
> 
> Diff: https://reviews.apache.org/r/38828/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Review Request 38835: Ignore SIGPIPE when finalizing libevent.

[Joris Van Remoortere]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

This fixes a Broken Pipe after unblocking SIGPIPE.


Diffs
-

  3rdparty/libprocess/src/libevent.cpp 00e0f08dd99fa60a92c1730c7ec899f2f7f55676 

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


Testing
---

make check with libevent and ssl enabled.


Thanks,

Joris Van Remoortere

Re: Review Request 38828: docker: containerizer: fixed double-closing of STDIN in error handling.

[Timothy Chen]

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



src/slave/containerizer/docker.cpp 


Hey I think we should be doing the same behavior as MesosContainerizer 
since the executor behaves the same, and I think yes the extra close is not 
necessary.


- Timothy Chen


On Sept. 29, 2015, 12:50 a.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38828/
> ---
> 
> (Updated Sept. 29, 2015, 12:50 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The slave will destroy executors that are not launched successfully, which 
> will
> close all open file descriptors, thus the extra close is not needed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp efa37266368ee12fea9134b35ebc5047a2820f94 
> 
> Diff: https://reviews.apache.org/r/38828/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 37703: Add docker exec command.

[Timothy Chen]

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



src/docker/docker.cpp (line 1164)


We're now supporting optional docker socket (-H), if you look at other 
commands.

Perhaps we should make this into a helper function so we don't have to do 
this in every command.


- Timothy Chen


On Sept. 9, 2015, 9:15 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37703/
> ---
> 
> (Updated Sept. 9, 2015, 9:15 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3291
> https://issues.apache.org/jira/browse/MESOS-3291
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add docker exec command.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp aaf8884a20901ad3a440d545d4b6316b87fc3403 
>   src/docker/docker.cpp 553e831029454d6d423842915b8bbfcaf19fa7f9 
>   src/tests/containerizer/docker_tests.cpp 
> babc7d8da4ed9d13b14bd69decd7f27fc7dfde89 
> 
> Diff: https://reviews.apache.org/r/37703/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38809: socket: refactor to use Option and fix file descriptor leaks.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38809]

All tests passed.

- Mesos ReviewBot


On Sept. 28, 2015, 9:07 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38809/
> ---
> 
> (Updated Sept. 28, 2015, 9:07 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3519
> https://issues.apache.org/jira/browse/MESOS-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> socket: refactor to use Option and fix file descriptor leaks.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/socket.hpp 
> 817cb3711742871630a13b966563296644008f21 
>   3rdparty/libprocess/src/socket.cpp 5879423bd92c5805353a962cbd822a993b0568c5 
> 
> Diff: https://reviews.apache.org/r/38809/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

[Klaus Ma]

> On Sept. 28, 2015, 9:11 a.m., Jan Schlicht wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 762
> > 
> >
> > Because this code has been changed from a constructor to a function, we 
> > can do better than calling `ABORT` here. We can change the function 
> > signature to return a `Try` and call `Error` here instead of 
> > `ABORT`. For example see the `write` function in line 55.
> > 
> > The function signature would change to
> > `inline Try protobuf(const google::protobuf::Message& message)`

Got your point :).
Just one concern on how to use it? We did not handle its result if failed in 
`src`; personally, this's a kind of code practice to avoid undefined protobuf 
field type. Anyway, I'll update accordingly if necessary.


- Klaus


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


On Sept. 27, 2015, 1:34 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38342/
> ---
> 
> (Updated Sept. 27, 2015, 1:34 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 2285ce9 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 68328a2 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 8ebb798 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 34eb6d0 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 920f5c9 
> 
> Diff: https://reviews.apache.org/r/38342/diff/
> 
> 
> Testing
> ---
> 
> cd 3rdparty/libprocess/3rdparty/stout
> ./boostrap
> ./configure
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Review Request 38822: Added default upgrade steps to 0.24.X to 0.25.X section

[Niklas Nielsen]

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

Review request for mesos, Joris Van Remoortere, switched to 'mcypark', and 
Vinod Kone.


Repository: mesos


Description
---

See summary


Diffs
-

  docs/upgrades.md 8e33b9b90654a43a452f97fc66e969ecb0f3d6dd 

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


Testing
---


Thanks,

Niklas Nielsen

Re: Review Request 38580: Added docker registry RemotePuller

[Timothy Chen]

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



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 193)


We need to also insert into the tracker when we start a new download



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 196)


We don't need a dispatch here



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 218)


you can skip this whole .then and merge with the above onAny.



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 245)


You can call onAny here, then you don't need to return anything.



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 257)


kill the return



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 297)


We should return failure when timeout is <= 0 instead of just using the 
default and hide the bad input.



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 313)


Space between foreach and (



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 331)


No need to return



src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 338)


ditto


- Timothy Chen


On Sept. 28, 2015, 8:39 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38580/
> ---
> 
> (Updated Sept. 28, 2015, 8:39 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First iteration with basic functionality.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 4a0b7d11f013941084571f2d89d835a4668a3d8b 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
>   src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> cbb67686d45513f0395a0cf1bc5c43cb4935adae 
> 
> Diff: https://reviews.apache.org/r/38580/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

[Jan Schlicht]

> On Sept. 28, 2015, 11:11 a.m., Jan Schlicht wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 762
> > 
> >
> > Because this code has been changed from a constructor to a function, we 
> > can do better than calling `ABORT` here. We can change the function 
> > signature to return a `Try` and call `Error` here instead of 
> > `ABORT`. For example see the `write` function in line 55.
> > 
> > The function signature would change to
> > `inline Try protobuf(const google::protobuf::Message& message)`
> 
> Klaus Ma wrote:
> Got your point :).
> Just one concern on how to use it? We did not handle its result if failed 
> in `src`; personally, this's a kind of code practice to avoid undefined 
> protobuf field type. Anyway, I'll update accordingly if necessary.

Well, unfortunately this change means that the code that is using this function 
has to be changed to handle the error cases. This means checking if the `Try<>` 
contains a value or an error and react accordingly.


- Jan


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


On Sept. 27, 2015, 3:34 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38342/
> ---
> 
> (Updated Sept. 27, 2015, 3:34 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 2285ce9 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 68328a2 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 8ebb798 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 34eb6d0 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 920f5c9 
> 
> Diff: https://reviews.apache.org/r/38342/diff/
> 
> 
> Testing
> ---
> 
> cd 3rdparty/libprocess/3rdparty/stout
> ./boostrap
> ./configure
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>