Re: Review Request 38102: MESOS-3046

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38102]

All tests passed.

- Mesos ReviewBot


On Sept. 3, 2015, 2:01 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38102/
> ---
> 
> (Updated Sept. 3, 2015, 2:01 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3046
> https://issues.apache.org/jira/browse/MESOS-3046
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3046 (Stout's UUID re-seeds a new random generator during each call to 
> UUID::random)
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp 
> e90dabb0c572923a50490ecb17867dc50c6d161d 
> 
> Diff: https://reviews.apache.org/r/38102/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38050: Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;

[Vinod Kone]

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


looks good overall. some minor issues.


src/master/master.cpp (line 146)


"Remove this handler in 0.26.0".



src/master/master.cpp (lines 149 - 150)


this fits on one line (within 80 chars)? why wrap it?



src/master/master.cpp (lines 181 - 189)


any reason to not just call pong() here?



src/tests/partition_tests.cpp (line 79)


why the change to process::Message here and everywhere else?


- Vinod Kone


On Sept. 3, 2015, 7:46 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38050/
> ---
> 
> (Updated Sept. 3, 2015, 7:46 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-1831
> https://issues.apache.org/jira/browse/MESOS-1831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master should send PingSlaveMessage instead of "PING"; Slave should accept 
> PingSlaveMessage but not "PING" message;
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/partition_tests.cpp b7030adcfb1bfb128040aff20f12461412b5382c 
>   src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23 
>   src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c 
> 
> Diff: https://reviews.apache.org/r/38050/diff/
> 
> 
> Testing
> ---
> 
> Test2: Start master and slave to debug the PingSlaveMessage. Passed!
> Test1: Run make check successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 37699: Removed remnants of LIBPROCESS_STATISTICS_WINDOW.

[Greg Mann]

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

(Updated Sept. 3, 2015, 3:08 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed remnants of LIBPROCESS_STATISTICS_WINDOW.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp 755187c8761137cb2bf2f7295b29a63f63c68bc6 

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


Testing
---

make check


Thanks,

Greg Mann

Re: Review Request 38102: MESOS-3046

[Klaus Ma]

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

(Updated Sept. 3, 2015, 2:01 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

MESOS-3046 (Stout's UUID re-seeds a new random generator during each call to 
UUID::random)


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp 
e90dabb0c572923a50490ecb17867dc50c6d161d 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Review Request 38102: MESOS-3046

[Klaus Ma]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

MESOS-3046 (Stout's UUID re-seeds a new random generator during each call to 
UUID::random)


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp 
e90dabb0c572923a50490ecb17867dc50c6d161d 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 37197: Docker image store.

[Jojy Varghese]

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



src/slave/containerizer/provisioners/docker/store.hpp (line 49)


Is this intended to be an abstract class? If so, why this factory method?



src/slave/containerizer/provisioners/docker/store.hpp (line 51)


What is the ownership model for fetcher pointer. By ownership I mean how is 
the lifecycle of the pointer managed?



src/slave/containerizer/provisioners/docker/store.hpp (line 55)


I think all new code needs to follow mesos doxygen guidelines.



src/slave/containerizer/provisioners/docker/store.hpp (line 93)


According to 
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Declaration_Order,
 the non-copyable/non-assignable section should be last in the declaration 
order.



src/slave/containerizer/provisioners/docker/store.cpp (line 150)


calls for a static constant?



src/slave/flags.hpp (line 52)


Wondering we should stuff everything in Flags class or if we can use 
inheritance to make  feature specfic flags.


- Jojy Varghese


On Aug. 27, 2015, 11:39 p.m., Lily Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37197/
> ---
> 
> (Updated Aug. 27, 2015, 11:39 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Till Toenshoff, 
> and Timothy Chen.
> 
> 
> Bugs: MESOS-2849
> https://issues.apache.org/jira/browse/MESOS-2849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stored images currently kept indefinitely.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION 
>   src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 
>   src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 
> 
> Diff: https://reviews.apache.org/r/37197/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Lily Chen
> 
>

Re: Review Request 37197: Docker image store.

[Jojy Varghese]

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



src/slave/containerizer/provisioners/docker/store.hpp (line 100)


Does this need to be in the hpp file?



src/slave/containerizer/provisioners/docker/store.hpp (line 146)


I would rethink the lifecycle management of this poiner.



src/slave/containerizer/provisioners/docker/store.cpp (line 49)


Ususally if you want to provide a factory class that hands out 
implementations, you use Factory class (commonly known as Factory design 
pattern).



src/slave/containerizer/provisioners/docker/store.cpp (line 139)


magic numbers (7) is not recommended.



src/slave/containerizer/provisioners/docker/store.cpp (line 146)


What is the workflow for creating these directories?



src/slave/containerizer/provisioners/docker/store.cpp (line 238)


const?


- Jojy Varghese


On Aug. 27, 2015, 11:39 p.m., Lily Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37197/
> ---
> 
> (Updated Aug. 27, 2015, 11:39 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Till Toenshoff, 
> and Timothy Chen.
> 
> 
> Bugs: MESOS-2849
> https://issues.apache.org/jira/browse/MESOS-2849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stored images currently kept indefinitely.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION 
>   src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 
>   src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 
> 
> Diff: https://reviews.apache.org/r/37197/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Lily Chen
> 
>

Re: Review Request 37699: Removed remnants of LIBPROCESS_STATISTICS_WINDOW.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37699]

All tests passed.

- Mesos ReviewBot


On Sept. 3, 2015, 3:08 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37699/
> ---
> 
> (Updated Sept. 3, 2015, 3:08 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3304
> https://issues.apache.org/jira/browse/MESOS-3304
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed remnants of LIBPROCESS_STATISTICS_WINDOW.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 755187c8761137cb2bf2f7295b29a63f63c68bc6 
> 
> Diff: https://reviews.apache.org/r/37699/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37873: Add quiesce logic in allocator

[Alexander Rukletsov]

> On Aug. 30, 2015, 5:10 p.m., Robert Lacroix wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 153
> > 
> >
> > Are we quiescing resources or rather offers? I think it should be 
> > called `quiesceOffers` so it's symmetrical to `reviveOffers`.

Technically, allocator does not know anything about offers, only about 
allocations. This may change soon (we think about pulling offers management 
into allocator), but until then I find this naming misleading.


- Alexander


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


On Aug. 31, 2015, 5:49 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37873/
> ---
> 
> (Updated Aug. 31, 2015, 5:49 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add quiesce logic in allocator
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp 
> aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 
> 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/mesos.hpp b2160f50db3e0b0d04b0625e7589004016c0f746 
> 
> Diff: https://reviews.apache.org/r/37873/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 37669: Ignore overflow docker components in version parsing.

[Jan Schlicht]

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

Ship it!



src/tests/containerizer/docker_tests.cpp (line 229)


This is a test of the special case with a "1.7.1.fc22" version string. How 
about an additional test of the general case like "1.7.1"?


- Jan Schlicht


On Sept. 3, 2015, 1:16 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37669/
> ---
> 
> (Updated Sept. 3, 2015, 1:16 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Timothy Chen.
> 
> 
> Bugs: MESOS-2986
> https://issues.apache.org/jira/browse/MESOS-2986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignore overflow docker components in version parsing.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 2e17cedb3e29548bdd2d10bd87bd0f61bf123be8 
>   src/tests/containerizer/docker_tests.cpp 
> cfad36850b40ade7a41f5b92255320ca1ed1bf93 
> 
> Diff: https://reviews.apache.org/r/37669/diff/
> 
> 
> Testing
> ---
> 
> # Add new test case
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="DockerTest.ROOT_DOCKER_parsing_version" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37826: Introduced conversion of JSON arrays to repeated protobufs.

[Joseph Wu]

> On Sept. 1, 2015, 9:46 a.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, lines 553-555
> > 
> >
> > Can you explain why this is a struct rather than a function?
> 
> Alexander Rukletsov wrote:
> It's because we cannot partially specialize function templates and 
> overload won't work since we take the same argument. Do you think a comment 
> should be expanded?
> 
> Alexander Rukletsov wrote:
> http://www.gotw.ca/publications/mill17.htm
> 
> Joseph Wu wrote:
> Yes, I think a comment would be good regardless of how it ends up.
> 
> What about using some C++11 type traits?  (But I'm not sure if this will 
> work.)
> Something like:
> ```
> // Specialization for non-repeated fields.
> template 
> Try parse(const JSON::Value& value,
> typename std::enable_if google::protobuf::Message>, int>::type = 0) {...}
> ```
> 
> Note: This file uses some similar Boost type traits further up.
> 
> Alexander Rukletsov wrote:
> I'm not sure how we can use type traits here. IIUC type traits facilitate 
> compile-time checks, and not compile-time dispatch. If we want to keep the 
> same function signature, we should rely on partial specialization. However, 
> we can still add these checks into `Parse<>::operator()` methods.
> 
> Alexander Rukletsov wrote:
> Which boils down to replacing
> ```
> { google::protobuf::Message* message = (T*) NULL; (void) message; }
> ```
> with
> ```
> static_assert(std::is_base_of::value,
>   "T must be a protobuf message");
> ```
> Does it make sense?

That part makes sense.  

And just to make sure, you can't use the same pattern found here?  (Which looks 
a lot like compile-time dispatch.)
https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp#L165-L177


- Joseph


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


On Sept. 3, 2015, 6:32 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37826/
> ---
> 
> (Updated Sept. 3, 2015, 6:32 a.m.)
> 
> 
> Review request for mesos, Joseph Wu and Michael Park.
> 
> 
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 57d5fdf45273c620655b44b5f5572290cffa4bf6 
> 
> Diff: https://reviews.apache.org/r/37826/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 37827: Added a test for converting JSON arrays to repeated protobufs.

[Michael Park]

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


Perhaps a dumb question, but may I ask why you needed to introduce 
`SimpleMessage`?

- Michael Park


On Sept. 3, 2015, 1:33 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37827/
> ---
> 
> (Updated Sept. 3, 2015, 1:33 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
> c56d6a3098293eb3659b3066f10e875927ec3ac3 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 
> cfc2803e42284f641879fb24bce1282215c8ea52 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 
> a1d4084661345f9367c75f9db61279f032b93e69 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 
> bbd36d39e9588eb8eea6d739451ad3bab029ca08 
> 
> Diff: https://reviews.apache.org/r/37827/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> **NOTE**: Filed 
> [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up 
> protobuf generation.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 38000: WIP: Added an API for libprocess users to interact with http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Sept. 3, 2015, 10:40 a.m.)


Review request for mesos and Benjamin Hindman.


Changes
---

Adding using clause.


Repository: mesos


Description
---

WIP

Adds functions which allows libprocess users to add handlers which require 
authentication as well as functions to install and remove authenticators.

Includes tests.


Diffs (updated)
-

  3rdparty/libprocess/include/process/process.hpp 
009f7c4167fa379ac6b1c267e1b4d5fcdf28d697 
  3rdparty/libprocess/src/process.cpp 755187c8761137cb2bf2f7295b29a63f63c68bc6 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d0b9399d38fa284466a012a21080b1d9007af98b 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38050: Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38050]

All tests passed.

- Mesos ReviewBot


On Sept. 3, 2015, 7:46 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38050/
> ---
> 
> (Updated Sept. 3, 2015, 7:46 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-1831
> https://issues.apache.org/jira/browse/MESOS-1831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master should send PingSlaveMessage instead of "PING"; Slave should accept 
> PingSlaveMessage but not "PING" message;
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/partition_tests.cpp b7030adcfb1bfb128040aff20f12461412b5382c 
>   src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23 
>   src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c 
> 
> Diff: https://reviews.apache.org/r/38050/diff/
> 
> 
> Testing
> ---
> 
> Test2: Start master and slave to debug the PingSlaveMessage. Passed!
> Test1: Run make check successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38050: Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;

[Yong Qiao Wang]

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

(Updated 九月 3, 2015, 7:46 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Master should send PingSlaveMessage instead of "PING"; Slave should accept 
PingSlaveMessage but not "PING" message;


Diffs
-

  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/tests/partition_tests.cpp b7030adcfb1bfb128040aff20f12461412b5382c 
  src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23 
  src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c 

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


Testing
---

Test2: Start master and slave to debug the PingSlaveMessage. Passed!
Test1: Run make check successfully!


Thanks,

Yong Qiao Wang

Re: Review Request 38050: Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;

[Yong Qiao Wang]

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

(Updated 九月 3, 2015, 7:45 a.m.)


Review request for mesos and Vinod Kone.


Bugs: MESOS-1831 and MESOS-1832
https://issues.apache.org/jira/browse/MESOS-1831
https://issues.apache.org/jira/browse/MESOS-1832


Repository: mesos


Description
---

Master should send PingSlaveMessage instead of "PING"; Slave should accept 
PingSlaveMessage but not "PING" message;


Diffs (updated)
-

  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/tests/partition_tests.cpp b7030adcfb1bfb128040aff20f12461412b5382c 
  src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23 
  src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c 

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


Testing (updated)
---

Test2: Start master and slave to debug the PingSlaveMessage. Passed!
Test1: Run make check successfully!


Thanks,

Yong Qiao Wang

Re: Review Request 37669: Ignore overflow docker components in version parsing.

[Bernd Mathiske]

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

Ship it!


Minor fixes can be dealt with when committing.


src/docker/docker.cpp (line 201)


s/docker/Docker
s/is/does



src/docker/docker.cpp (line 204)


s/to/to a


- Bernd Mathiske


On Sept. 2, 2015, 8:12 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37669/
> ---
> 
> (Updated Sept. 2, 2015, 8:12 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Timothy Chen.
> 
> 
> Bugs: MESOS-2986
> https://issues.apache.org/jira/browse/MESOS-2986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignore overflow docker components in version parsing.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 2e17cedb3e29548bdd2d10bd87bd0f61bf123be8 
>   src/tests/containerizer/docker_tests.cpp 
> cfad36850b40ade7a41f5b92255320ca1ed1bf93 
> 
> Diff: https://reviews.apache.org/r/37669/diff/
> 
> 
> Testing
> ---
> 
> # Add new test case
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="DockerTest.ROOT_DOCKER_parsing_version" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37669: Ignore overflow docker components in version parsing.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37669]

All tests passed.

- Mesos ReviewBot


On Sept. 3, 2015, 11:16 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37669/
> ---
> 
> (Updated Sept. 3, 2015, 11:16 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Timothy Chen.
> 
> 
> Bugs: MESOS-2986
> https://issues.apache.org/jira/browse/MESOS-2986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignore overflow docker components in version parsing.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 2e17cedb3e29548bdd2d10bd87bd0f61bf123be8 
>   src/tests/containerizer/docker_tests.cpp 
> cfad36850b40ade7a41f5b92255320ca1ed1bf93 
> 
> Diff: https://reviews.apache.org/r/37669/diff/
> 
> 
> Testing
> ---
> 
> # Add new test case
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="DockerTest.ROOT_DOCKER_parsing_version" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37669: Ignore overflow docker components in version parsing.

[haosdent huang]

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

(Updated Sept. 3, 2015, 11:16 a.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Timothy Chen.


Changes
---

Update according Bernd opinions.


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


Repository: mesos


Description
---

Ignore overflow docker components in version parsing.


Diffs (updated)
-

  src/docker/docker.cpp 2e17cedb3e29548bdd2d10bd87bd0f61bf123be8 
  src/tests/containerizer/docker_tests.cpp 
cfad36850b40ade7a41f5b92255320ca1ed1bf93 

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


Testing
---

# Add new test case
sudo ./bin/mesos-tests.sh 
--gtest_filter="DockerTest.ROOT_DOCKER_parsing_version" --verbose


Thanks,

haosdent huang

Re: Review Request 37929: Changed provisioner Store API and implementation so it works as a read-through cache.

[Jiang Yan Xu]

> On Sept. 2, 2015, 5:53 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioners/appc/store.cpp, line 235
> > 
> >
> > The wording sounds wierd, how about:
> > 
> > "Cannot match any image in store for '" + image.name()

Changed to:

```
return Failure("No image named '" + image.name() +
   "' can match the requirements");
```

Note that it's not about local availability anymore since the store is also 
responsible for retrieving it.


- Jiang Yan


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


On Sept. 2, 2015, 5:07 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37929/
> ---
> 
> (Updated Sept. 2, 2015, 5:07 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - i.e., It fetches images transparently when it's not in the local cache.
> - This way, the store doesn't have the sense of "localness" anymore but 
> rather it's an abstraction that provides access to all discoverable images, 
> no matter where they can be found.
> 
> Some context for motivation of this change can be found at: 
> https://reviews.apache.org/r/37881/
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioners/appc/store.hpp 
> e48d91be06410bfc028a7b2ed88218e13adbffee 
>   src/slave/containerizer/provisioners/appc/store.cpp 
> fbd1c535d398a4d37c30ba23f5408095c7d35b65 
>   src/tests/containerizer/appc_provisioner_tests.cpp 
> 47b66b9c30cefe8f9a8e2c1c1341776c2d235020 
> 
> Diff: https://reviews.apache.org/r/37929/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 37929: Changed provisioner Store API and implementation so it works as a read-through cache.

[Jiang Yan Xu]

> On Aug. 31, 2015, 2:42 p.m., Chi Zhang wrote:
> > src/slave/containerizer/provisioners/appc/store.hpp, lines 76-81
> > 
> >
> > Maybe not yet the guarantee on ordering since we don't support 
> > dependency in images.
> > 
> > Mainly I think if we don't have to worry about ordering, it is simpler.
> > 
> > If backends can't get away from that, we should then be more explicit 
> > on what kind of ordering we provie. BFS, first order traversal or sth else?
> 
> Jiang Yan Xu wrote:
> This comment is about what the method SHOULD do. And the TODO below says: 
> OK we are actually not doing it just yet. :)
> 
> It's important to be clear about what we WILL do in this case because 
> this is the contract between the provisioner and the store. We need to 
> determine right now where the logic should go in order to make sure the 
> interface is future proof.
> 
> The backend is told about the ordering because it itself doesn't have 
> enough information to otherwise detect it.
> 
> The ordering is basically "what overwites what in the fact of conflict" 
> so it's not BFS or how we RESOLVE the dependencies.
> 
> Chi Zhang wrote:
> I guess I wasn't clear if C, B, D, A and C, D, B, A are both acceptable 
> in your example, afte reading the comment?
> 
> Jiang Yan Xu wrote:
> Thanks. You are right I could have been more clear about this. In fact B 
> needs to go before D so I added the following comment: 
> (B before D in this example is decided by the order in which A specifies 
> its dependencies)
> 
> Chi Zhang wrote:
> is it true that the order at which A speicifies dependecies matters?

If image A depends on [B, D], it may intentionally want some files in D to 
overwrite the ones in B, right?
I am not saying it's a good practice but it's the intention of the spec to 
allow it.


- Jiang Yan


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


On Sept. 2, 2015, 5:07 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37929/
> ---
> 
> (Updated Sept. 2, 2015, 5:07 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - i.e., It fetches images transparently when it's not in the local cache.
> - This way, the store doesn't have the sense of "localness" anymore but 
> rather it's an abstraction that provides access to all discoverable images, 
> no matter where they can be found.
> 
> Some context for motivation of this change can be found at: 
> https://reviews.apache.org/r/37881/
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioners/appc/store.hpp 
> e48d91be06410bfc028a7b2ed88218e13adbffee 
>   src/slave/containerizer/provisioners/appc/store.cpp 
> fbd1c535d398a4d37c30ba23f5408095c7d35b65 
>   src/tests/containerizer/appc_provisioner_tests.cpp 
> 47b66b9c30cefe8f9a8e2c1c1341776c2d235020 
> 
> Diff: https://reviews.apache.org/r/37929/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 38050: Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;

[Yong Qiao Wang]

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

(Updated 九月 4, 2015, 1:04 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Master should send PingSlaveMessage instead of "PING"; Slave should accept 
PingSlaveMessage but not "PING" message;


Diffs (updated)
-

  3rdparty/libprocess/include/process/collect.hpp 
d07b6860a5b5cbed910b2e9e2ab2429bd70a3999 
  3rdparty/libprocess/src/tests/process_tests.cpp 
435663b10c1bfce07e8e84719aa14b5867c651c6 
  src/Makefile.am 4b643a32eb68dbd52714e9d66451dc2b62576f6d 
  src/linux/perf.hpp d10968ca670eb516bae08385fd0ddde8e8ad83b5 
  src/linux/perf.cpp 0011482cf9d920485728798518d32af0e9627724 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/slave/containerizer/isolators/cgroups/perf_event.hpp 
1f722ef3ef7ab7fce5542d4affae961d6cec2406 
  src/slave/containerizer/isolators/cgroups/perf_event.cpp 
7dc8b7a99074b74ade019ef4df296780650a2e4e 
  src/slave/containerizer/provisioners/backend.cpp 
6560ece14d8618878a35d1bfe27db3958da64358 
  src/slave/containerizer/provisioners/backends/copy.hpp 
2abca37ed2479d42c634c23cac8e40d515249988 
  src/slave/containerizer/provisioners/backends/copy.cpp 
b56946562525e79bef3a7387cd71f39fd0690683 
  src/tests/containerizer/perf_tests.cpp 
bef475e4b573f02b69f2c4dd6d1c6a6e10f34c38 
  src/tests/containerizer/provisioner_backend_tests.cpp 
f2498b109c910fbf753a53b4b36a88b8d779aa69 
  src/tests/partition_tests.cpp b7030adcfb1bfb128040aff20f12461412b5382c 
  src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23 
  src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c 

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


Testing
---

Test2: Start master and slave to debug the PingSlaveMessage. Passed!
Test1: Run make check successfully!


Thanks,

Yong Qiao Wang

Re: Review Request 37880: Changed the Appc provisioner directory to identify a rootfs by a rootfs_id (UUID).

[Chi Zhang]

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

Ship it!


Ship It!

- Chi Zhang


On Aug. 31, 2015, 5:44 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37880/
> ---
> 
> (Updated Aug. 31, 2015, 5:44 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2796
> https://issues.apache.org/jira/browse/MESOS-2796
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - So the user can provision multiple identical rootfses in the volumes of one 
> container, a case we don't need to disallow.
> - Previously a rootfs is identified by the 'image_id' of its topmost file 
> system layer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioners/appc/paths.hpp 
> 41e3bf79da0854406c488855f953111e67353829 
>   src/slave/containerizer/provisioners/appc/paths.cpp 
> 3113c84b9526dd9e9e89fb9aa4ec75ed66a996c7 
> 
> Diff: https://reviews.apache.org/r/37880/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 38050: Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;

[Yong Qiao Wang]

> On 九月 3, 2015, 3:45 p.m., Vinod Kone wrote:
> > src/tests/partition_tests.cpp, line 79
> > 
> >
> > why the change to process::Message here and everywhere else?

Actually, namespace "using process::Message;" has been used in the header of 
this file, so it does not need to change to process::Message. I will update to 
roolback these changes.


> On 九月 3, 2015, 3:45 p.m., Vinod Kone wrote:
> > src/master/master.cpp, lines 187-195
> > 
> >
> > any reason to not just call pong() here?

Update the pongOld function to just call pong().


- Yong Qiao


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


On 九月 3, 2015, 7:46 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38050/
> ---
> 
> (Updated 九月 3, 2015, 7:46 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-1831
> https://issues.apache.org/jira/browse/MESOS-1831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master should send PingSlaveMessage instead of "PING"; Slave should accept 
> PingSlaveMessage but not "PING" message;
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/partition_tests.cpp b7030adcfb1bfb128040aff20f12461412b5382c 
>   src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23 
>   src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c 
> 
> Diff: https://reviews.apache.org/r/38050/diff/
> 
> 
> Testing
> ---
> 
> Test2: Start master and slave to debug the PingSlaveMessage. Passed!
> Test1: Run make check successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 37826: Introduced conversion of JSON arrays to repeated protobufs.

[Michael Park]

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



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 592)


Do we want to support the following?

```cpp
using google::protobuf::RepeatedPtrField;

auto parse = 
protobuf::parse>(array_of_array);
```

I think we might want to disallow it since it seems that protobuf 
definitions cannot express "array of array" (maybe there actually is a way to 
do that?). If we do want to disallow it, we should include the following check 
at the top.

```cpp
{ google::protobuf::Message* message = (T*) NULL; (void) message; }
```



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 601)


`s/proagate/propagate/`



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 604)


Remove this newline.


- Michael Park


On Sept. 3, 2015, 1:32 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37826/
> ---
> 
> (Updated Sept. 3, 2015, 1:32 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Michael Park.
> 
> 
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 57d5fdf45273c620655b44b5f5572290cffa4bf6 
> 
> Diff: https://reviews.apache.org/r/37826/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 37929: Changed provisioner Store API and implementation so it works as a read-through cache.

[Chi Zhang]

> On Aug. 31, 2015, 9:42 p.m., Chi Zhang wrote:
> > src/slave/containerizer/provisioners/appc/store.hpp, lines 76-81
> > 
> >
> > Maybe not yet the guarantee on ordering since we don't support 
> > dependency in images.
> > 
> > Mainly I think if we don't have to worry about ordering, it is simpler.
> > 
> > If backends can't get away from that, we should then be more explicit 
> > on what kind of ordering we provie. BFS, first order traversal or sth else?
> 
> Jiang Yan Xu wrote:
> This comment is about what the method SHOULD do. And the TODO below says: 
> OK we are actually not doing it just yet. :)
> 
> It's important to be clear about what we WILL do in this case because 
> this is the contract between the provisioner and the store. We need to 
> determine right now where the logic should go in order to make sure the 
> interface is future proof.
> 
> The backend is told about the ordering because it itself doesn't have 
> enough information to otherwise detect it.
> 
> The ordering is basically "what overwites what in the fact of conflict" 
> so it's not BFS or how we RESOLVE the dependencies.
> 
> Chi Zhang wrote:
> I guess I wasn't clear if C, B, D, A and C, D, B, A are both acceptable 
> in your example, afte reading the comment?
> 
> Jiang Yan Xu wrote:
> Thanks. You are right I could have been more clear about this. In fact B 
> needs to go before D so I added the following comment: 
> (B before D in this example is decided by the order in which A specifies 
> its dependencies)

is it true that the order at which A speicifies dependecies matters?


- Chi


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


On Sept. 3, 2015, 12:07 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37929/
> ---
> 
> (Updated Sept. 3, 2015, 12:07 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - i.e., It fetches images transparently when it's not in the local cache.
> - This way, the store doesn't have the sense of "localness" anymore but 
> rather it's an abstraction that provides access to all discoverable images, 
> no matter where they can be found.
> 
> Some context for motivation of this change can be found at: 
> https://reviews.apache.org/r/37881/
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioners/appc/store.hpp 
> e48d91be06410bfc028a7b2ed88218e13adbffee 
>   src/slave/containerizer/provisioners/appc/store.cpp 
> fbd1c535d398a4d37c30ba23f5408095c7d35b65 
>   src/tests/containerizer/appc_provisioner_tests.cpp 
> 47b66b9c30cefe8f9a8e2c1c1341776c2d235020 
> 
> Diff: https://reviews.apache.org/r/37929/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 37273: [1/2]Add CMake macro VsBuildCommand in libprocess.

[haosdent huang]

> On Aug. 31, 2015, 6:46 p.m., Joseph Wu wrote:
> > For any future CMake reviews, could you also add Artem (hartem) and I 
> > (kaysoky) as reviewers?
> > Thanks in advance :)

Sure? thank you very much.


- haosdent


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


On Aug. 30, 2015, 1:42 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37273/
> ---
> 
> (Updated Aug. 30, 2015, 1:42 p.m.)
> 
> 
> Review request for mesos and Alex Clemmer.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add CMake macro VsBuildCommand in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 997cc0d0e316e316136d4746e50e9e292a82b36b 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> 12506a1369de005285268f895f365aba0c560f78 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
>   3rdparty/libprocess/cmake/macros/VsBuildCommand.bat PRE-CREATION 
>   3rdparty/libprocess/cmake/macros/VsBuildCommand.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37273/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37826: Introduced conversion of JSON arrays to repeated protobufs.

[Michael Park]

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



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 559)


Remove this newline.



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 567)


Remove this newline.



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 591)


The `Parse` above uses `value`, rather than `json`. Can we consolidate 
to one? I think `value` would be more fitting, since we name `JSON::Object` as 
`object`, `JSON::Array` as `array`, etc.



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 627)


`s/json/value/` as suggested above?


- Michael Park


On Sept. 3, 2015, 1:32 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37826/
> ---
> 
> (Updated Sept. 3, 2015, 1:32 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Michael Park.
> 
> 
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 57d5fdf45273c620655b44b5f5572290cffa4bf6 
> 
> Diff: https://reviews.apache.org/r/37826/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 37532: Add QUIESCE call interface to the scheduler

[Guangya Liu]

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

(Updated 九月 4, 2015, 2:20 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This is just part of MESOS-3037, this patch only add the interface
of QUIESCE call.


Diffs (updated)
-

  include/mesos/scheduler.hpp ee198b6955882f4f31466ca05429ca16fbf2f5cd 
  include/mesos/scheduler/scheduler.proto 
89daf8a6b74057ee156b3ad691397e76fcb835b8 
  src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/sched/sched.cpp 012af0508eeceeccd168b29f36fa258d20b28c21 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 37873: Add quiesce logic in allocator

[Guangya Liu]

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

(Updated 九月 4, 2015, 2:19 a.m.)


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


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


Repository: mesos


Description
---

Add quiesce logic in allocator


Diffs (updated)
-

  include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
  src/master/allocator/mesos/allocator.hpp 
aa55755a9c3250579e9366bdbc17a2449e95d659 
  src/master/allocator/mesos/hierarchical.hpp 
cb4020dea897ef198cd9898cabecf61edfade834 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 37669: Ignore overflow docker components in version parsing.

[haosdent huang]

> On Sept. 3, 2015, 3:55 p.m., Jan Schlicht wrote:
> > src/tests/containerizer/docker_tests.cpp, line 229
> > 
> >
> > This is a test of the special case with a "1.7.1.fc22" version string. 
> > How about an additional test of the general case like "1.7.1"?

Sure, already updated. Thank you very much.


- haosdent


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


On Sept. 4, 2015, 2:27 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37669/
> ---
> 
> (Updated Sept. 4, 2015, 2:27 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Timothy Chen.
> 
> 
> Bugs: MESOS-2986
> https://issues.apache.org/jira/browse/MESOS-2986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignore overflow docker components in version parsing.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 2e17cedb3e29548bdd2d10bd87bd0f61bf123be8 
>   src/tests/containerizer/docker_tests.cpp 
> cfad36850b40ade7a41f5b92255320ca1ed1bf93 
> 
> Diff: https://reviews.apache.org/r/37669/diff/
> 
> 
> Testing
> ---
> 
> # Add new test case
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="DockerTest.ROOT_DOCKER_parsing_version" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37826: Introduced conversion of JSON arrays to repeated protobufs.

[Alexander Rukletsov]

> On Sept. 1, 2015, 4:46 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, lines 553-555
> > 
> >
> > Can you explain why this is a struct rather than a function?
> 
> Alexander Rukletsov wrote:
> It's because we cannot partially specialize function templates and 
> overload won't work since we take the same argument. Do you think a comment 
> should be expanded?
> 
> Alexander Rukletsov wrote:
> http://www.gotw.ca/publications/mill17.htm
> 
> Joseph Wu wrote:
> Yes, I think a comment would be good regardless of how it ends up.
> 
> What about using some C++11 type traits?  (But I'm not sure if this will 
> work.)
> Something like:
> ```
> // Specialization for non-repeated fields.
> template 
> Try parse(const JSON::Value& value,
> typename std::enable_if google::protobuf::Message>, int>::type = 0) {...}
> ```
> 
> Note: This file uses some similar Boost type traits further up.
> 
> Alexander Rukletsov wrote:
> I'm not sure how we can use type traits here. IIUC type traits facilitate 
> compile-time checks, and not compile-time dispatch. If we want to keep the 
> same function signature, we should rely on partial specialization. However, 
> we can still add these checks into `Parse<>::operator()` methods.

Which boils down to replacing
```
{ google::protobuf::Message* message = (T*) NULL; (void) message; }
```
with
```
static_assert(std::is_base_of::value,
  "T must be a protobuf message");
```
Does it make sense?


- Alexander


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


On Sept. 1, 2015, 2:20 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37826/
> ---
> 
> (Updated Sept. 1, 2015, 2:20 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Michael Park.
> 
> 
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 57d5fdf45273c620655b44b5f5572290cffa4bf6 
> 
> Diff: https://reviews.apache.org/r/37826/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 37505: Fix broken health check in docker executor.

[haosdent huang]

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

(Updated Sept. 3, 2015, 1:25 p.m.)


Review request for mesos, Adam B and Timothy Chen.


Changes
---

Rebase code and update according @tnachen reviews.


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


Repository: mesos


Description
---

Fix broken health check in docker executor.


Diffs (updated)
-

  src/docker/docker.hpp aaf8884a20901ad3a440d545d4b6316b87fc3403 
  src/docker/docker.cpp 2e17cedb3e29548bdd2d10bd87bd0f61bf123be8 
  src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e 
  src/slave/containerizer/docker.cpp 289d4ec0fba9071dfe0cbf5391b5391d4566dd9c 
  src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea 

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


Testing
---

# Add two new test cases, HealthCheckTest.ROOT_DOCKER_DockerHealthyTask and 
HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange
# Docker health check command is run through "docker exec"
sudo ./bin/mesos-tests.sh 
--gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthyTask" --verbose
sudo ./bin/mesos-tests.sh 
--gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange" --verbose


Thanks,

haosdent huang

Re: Review Request 37826: Introduced conversion of JSON arrays to repeated protobufs.

[Alexander Rukletsov]

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

(Updated Sept. 3, 2015, 1:32 p.m.)


Review request for mesos, Joseph Wu and Michael Park.


Changes
---

Expanded a comment.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
57d5fdf45273c620655b44b5f5572290cffa4bf6 

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


Testing
---

make check (Mac OS 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 37830: Added a test for converting JSON arrays to Resources.

[Alexander Rukletsov]

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

(Updated Sept. 3, 2015, 1:33 p.m.)


Review request for mesos, Joseph Wu and Michael Park.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/resources_tests.cpp 2ae93a9c8235e5e4643539d409df51c39c6d7e56 

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


Testing
---

make check (Mac OS 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 37827: Added a test for converting JSON arrays to repeated protobufs.

[Alexander Rukletsov]

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

(Updated Sept. 3, 2015, 1:33 p.m.)


Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
c56d6a3098293eb3659b3066f10e875927ec3ac3 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 
cfc2803e42284f641879fb24bce1282215c8ea52 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 
a1d4084661345f9367c75f9db61279f032b93e69 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 
bbd36d39e9588eb8eea6d739451ad3bab029ca08 

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


Testing
---

make check (Mac OS 10.10.4)

**NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) 
to clean up protobuf generation.


Thanks,

Alexander Rukletsov

Re: Review Request 37826: Introduced conversion of JSON arrays to repeated protobufs.

[Alexander Rukletsov]

> On Sept. 1, 2015, 4:46 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, lines 553-555
> > 
> >
> > Can you explain why this is a struct rather than a function?
> 
> Alexander Rukletsov wrote:
> It's because we cannot partially specialize function templates and 
> overload won't work since we take the same argument. Do you think a comment 
> should be expanded?
> 
> Alexander Rukletsov wrote:
> http://www.gotw.ca/publications/mill17.htm
> 
> Joseph Wu wrote:
> Yes, I think a comment would be good regardless of how it ends up.
> 
> What about using some C++11 type traits?  (But I'm not sure if this will 
> work.)
> Something like:
> ```
> // Specialization for non-repeated fields.
> template 
> Try parse(const JSON::Value& value,
> typename std::enable_if google::protobuf::Message>, int>::type = 0) {...}
> ```
> 
> Note: This file uses some similar Boost type traits further up.

I'm not sure how we can use type traits here. IIUC type traits facilitate 
compile-time checks, and not compile-time dispatch. If we want to keep the same 
function signature, we should rely on partial specialization. However, we can 
still add these checks into `Parse<>::operator()` methods.


- Alexander


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


On Sept. 1, 2015, 2:20 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37826/
> ---
> 
> (Updated Sept. 1, 2015, 2:20 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Michael Park.
> 
> 
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 57d5fdf45273c620655b44b5f5572290cffa4bf6 
> 
> Diff: https://reviews.apache.org/r/37826/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 37505: Fix broken health check in docker executor.

[haosdent huang]

> On Aug. 31, 2015, 4:32 a.m., Timothy Chen wrote:
> > src/docker/executor.cpp, line 361
> > 
> >
> > You should be able to push the arguments with another vector insert?
> > 
> > e.g:
> > 
> > argv.push_back("--executor");
> > argv.push_back(stringify(self());
> > .

If I use this way, I got error like this:
```
Failed to load non-boolean flag 'executor': Missing value

Usage: lt-mesos-health-check [options]

  --executor=VALUE  Executor UPID to send health check messages to
  --health_check_json=VALUE JSON describing health check to perform
  --[no-]help   Prints this help message (default: false)
  --task_id=VALUE   Task ID that this health check process is 
checking
```


- haosdent


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


On Aug. 30, 2015, 10:23 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37505/
> ---
> 
> (Updated Aug. 30, 2015, 10:23 a.m.)
> 
> 
> Review request for mesos, Adam B and Timothy Chen.
> 
> 
> Bugs: MESOS-3136
> https://issues.apache.org/jira/browse/MESOS-3136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix broken health check in docker executor.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 6086710fff32a25e46197a69ae1063074317221b 
>   src/docker/docker.cpp 12dc0505c9ec4bd380e817d44da2c4e8d1b0d5f5 
>   src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 
>   src/slave/containerizer/docker.cpp a17e4f21e7f5a1dfd47699ec84c7a48fd82294ad 
>   src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea 
> 
> Diff: https://reviews.apache.org/r/37505/diff/
> 
> 
> Testing
> ---
> 
> # Add two new test cases, HealthCheckTest.ROOT_DOCKER_DockerHealthyTask and 
> HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange
> # Docker health check command is run through "docker exec"
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthyTask" --verbose
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange" 
> --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37873: Add quiesce logic in allocator

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37532, 37873]

All tests passed.

- Mesos ReviewBot


On Sept. 4, 2015, 2:19 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37873/
> ---
> 
> (Updated Sept. 4, 2015, 2:19 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add quiesce logic in allocator
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp 
> aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 
> cb4020dea897ef198cd9898cabecf61edfade834 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 
> 
> Diff: https://reviews.apache.org/r/37873/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38050: Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;

[Yong Qiao Wang]

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

(Updated 九月 4, 2015, 1:18 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Master should send PingSlaveMessage instead of "PING"; Slave should accept 
PingSlaveMessage but not "PING" message;


Diffs (updated)
-

  src/master/master.cpp 56bcbcc 
  src/tests/partition_tests.cpp b7030ad 
  src/tests/slave_recovery_tests.cpp 4d137e0 
  src/tests/slave_tests.cpp 2411918 

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


Testing
---

Test2: Start master and slave to debug the PingSlaveMessage. Passed!
Test1: Run make check successfully!


Thanks,

Yong Qiao Wang

Re: Review Request 38050: Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Sept. 4, 2015, 1:18 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38050/
> ---
> 
> (Updated Sept. 4, 2015, 1:18 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-1831
> https://issues.apache.org/jira/browse/MESOS-1831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master should send PingSlaveMessage instead of "PING"; Slave should accept 
> PingSlaveMessage but not "PING" message;
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 56bcbcc 
>   src/tests/partition_tests.cpp b7030ad 
>   src/tests/slave_recovery_tests.cpp 4d137e0 
>   src/tests/slave_tests.cpp 2411918 
> 
> Diff: https://reviews.apache.org/r/38050/diff/
> 
> 
> Testing
> ---
> 
> Test2: Start master and slave to debug the PingSlaveMessage. Passed!
> Test1: Run make check successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38117: Export per container SNMP statistics

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38117]

All tests passed.

- Mesos ReviewBot


On Sept. 4, 2015, 1:17 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38117/
> ---
> 
> (Updated Sept. 4, 2015, 1:17 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-3365
> https://issues.apache.org/jira/browse/MESOS-3365
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These stats are those we get by `netstat -s`, they are important for diagnose 
> networking issues.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> 4bca0b81bf69fb4cd75e05aacd02d3e818e32d09 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 34ba2294b0bd7d57aa9de073692a2ea8ec62681d 
>   src/slave/flags.hpp b8335aa585376d27b71897f8cbaefaa12f4b3a5c 
>   src/slave/flags.cpp 49fb0007356eedf7095d34a1312e99bafa8fdc4d 
> 
> Diff: https://reviews.apache.org/r/38117/diff/
> 
> 
> Testing
> ---
> 
> Manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 37532: Add QUIESCE call interface to the scheduler

[Guangya Liu]

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

(Updated Sept. 4, 2015, 3:35 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

removed circular dep -- @vinodkone


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


Repository: mesos


Description
---

This is just part of MESOS-3037, this patch only add the interface
of QUIESCE call.


Diffs
-

  include/mesos/scheduler.hpp ee198b6955882f4f31466ca05429ca16fbf2f5cd 
  include/mesos/scheduler/scheduler.proto 
89daf8a6b74057ee156b3ad691397e76fcb835b8 
  src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/sched/sched.cpp 012af0508eeceeccd168b29f36fa258d20b28c21 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 37669: Ignore overflow docker components in version parsing.

[haosdent huang]

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

(Updated Sept. 4, 2015, 2:27 a.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Timothy Chen.


Changes
---

update according @Jan's reviews


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


Repository: mesos


Description
---

Ignore overflow docker components in version parsing.


Diffs (updated)
-

  src/docker/docker.cpp 2e17cedb3e29548bdd2d10bd87bd0f61bf123be8 
  src/tests/containerizer/docker_tests.cpp 
cfad36850b40ade7a41f5b92255320ca1ed1bf93 

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


Testing
---

# Add new test case
sudo ./bin/mesos-tests.sh 
--gtest_filter="DockerTest.ROOT_DOCKER_parsing_version" --verbose


Thanks,

haosdent huang