date:20150902

Re: Review Request 37168: MESOS-3063

2015-09-02 Thread Joerg Schad


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


As this is an example framework (which will be used by people to start their 
own framework), would it make sense to add some more comments explaining 
especially the relevant bits here?

- Joerg Schad


On Aug. 28, 2015, 3:31 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated Aug. 28, 2015, 3:31 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3063 (Add an example framework using dynamic reservation)
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7b620ff 
>   src/examples/dynamic_reservation_executor.cpp PRE-CREATION 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 3f56b30 
>   src/tests/flags.hpp 3644956 
>   src/tests/script.cpp bcc1fab 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Review Request 38046: Replace hard-coded reap interval with a constant

2015-09-02 Thread Guangya Liu


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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Replace hard-coded reap interval with a constant


Diffs
-

  src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e 
  src/examples/balloon_executor.cpp 6166f511b8977bc3b6f876de19f1cd91a4a8f03d 
  src/examples/event_call_framework.cpp 
f0058fe0e748e5d91c4b7f0c7569108b00e3eab0 
  src/examples/load_generator_framework.cpp 
725478f7162858e9e3136b30476ed1bfbe417681 
  src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
  src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
  src/slave/constants.cpp cf3ee7bbc252364a1b73731feab6a9da68ee1f55 
  src/slave/monitor.cpp 82aa659b02f98ad8dc4c4f556be7a332dfba6816 
  src/tests/containerizer/cgroups_tests.cpp 
75a3bc0009c037dc18ce319db2eb44630f083e8c 
  src/tests/containerizer/docker_tests.cpp 
cfad36850b40ade7a41f5b92255320ca1ed1bf93 
  src/tests/containerizer/isolator_tests.cpp 
d34c82a7f24da6f60cfb22790f516dc6065b1f6f 
  src/tests/containerizer/launch_tests.cpp 
d211fc0f665988068c67836ef80916828a0df2bd 
  src/tests/fault_tolerance_tests.cpp 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075 
  src/tests/gc_tests.cpp ec27ac7f1fca7f425abcea0eafaac4fae905fb8f 
  src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea 
  src/tests/limiter.hpp 1bdf8ae85eb4369846c8b367d2f9c81fdaacdf07 
  src/tests/log_tests.cpp f2dd47cfbe73fb18c360a637db009b7d391a782e 
  src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 
  src/tests/rate_limiting_tests.cpp f3aeddee00c7bb7905092aa8a760603468063126 
  src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23 
  src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c 
  src/zookeeper/zookeeper.cpp e44403ee91904415471382dfa4e0a6e0adfdb74f 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 37989: Replace hard-coded reap interval with a constant

2015-09-02 Thread Guangya Liu



> On 九月 2, 2015, 12:47 a.m., Ben Mahler wrote:
> > Hey Guangya, the point of MESOS-1935 was to use these constant functions 
> > throughout the code base:
> > 
> > ```
> > ?  mesos git:(master) ? grep -R 'Seconds(1)' src/tests
> > src/tests/containerizer/cgroups_tests.cpp:if (elapsed > Seconds(1)) {
> > src/tests/containerizer/docker_tests.cpp:docker->inspect(containerName, 
> > Seconds(1));
> > src/tests/containerizer/docker_tests.cpp:  inspect = 
> > docker->inspect(containerName, Seconds(1));
> > src/tests/containerizer/isolator_tests.cpp:  } while (waited < Seconds(1));
> > src/tests/containerizer/isolator_tests.cpp:  } while (waited < Seconds(1));
> > src/tests/containerizer/launch_tests.cpp:Clock::advance(Seconds(1));
> > src/tests/fault_tolerance_tests.cpp:  Clock::advance(Seconds(1)); // 
> > TODO(benh): Pull out constant from Slave.
> > src/tests/gc_tests.cpp:  Clock::advance(Seconds(1));
> > src/tests/gc_tests.cpp:Clock::advance(Seconds(1));
> > src/tests/health_check_tests.cpp:  Clock::advance(Seconds(1));
> > src/tests/limiter.hpp:  MockRateLimiter() : process::RateLimiter(1, 
> > Seconds(1)) {}
> > src/tests/log_tests.cpp:  EXPECT_GT(Seconds(1), stopwatch.elapsed());
> > src/tests/log_tests.cpp:  Clock::advance(Seconds(1));
> > src/tests/log_tests.cpp:  Clock::advance(Seconds(1));
> > src/tests/master_tests.cpp:  Clock::advance(Seconds(1));
> > src/tests/rate_limiting_tests.cpp:Clock::advance(Seconds(1));
> > src/tests/rate_limiting_tests.cpp:Clock::advance(Seconds(1));
> > src/tests/rate_limiting_tests.cpp:  Clock::advance(Seconds(1));
> > src/tests/slave_recovery_tests.cpp:Clock::advance(Seconds(1));
> > src/tests/slave_recovery_tests.cpp:Clock::advance(Seconds(1));
> > src/tests/slave_recovery_tests.cpp:Clock::advance(Seconds(1));
> > src/tests/slave_recovery_tests.cpp:Clock::advance(Seconds(1));
> > src/tests/slave_recovery_tests.cpp:Clock::advance(Seconds(1));
> > src/tests/slave_recovery_tests.cpp:Clock::advance(Seconds(1));
> > src/tests/slave_recovery_tests.cpp:Clock::advance(Seconds(1));
> > src/tests/slave_recovery_tests.cpp:Clock::advance(Seconds(1));
> > src/tests/slave_recovery_tests.cpp:Clock::advance(Seconds(1));
> > src/tests/slave_recovery_tests.cpp:Clock::advance(Seconds(1));
> > src/tests/slave_recovery_tests.cpp:Clock::advance(Seconds(1));
> > src/tests/slave_tests.cpp:Clock::advance(Seconds(1));
> > ```
> > 
> > Some of these can be replaced by the MAX_REAP_INTERVAL(), for example, this 
> > one:
> > https://github.com/apache/mesos/blob/0.24.0-rc1/src/tests/slave_recovery_tests.cpp#L1687

Thanks Ben, I will abandon this as the Mesos project do not allow uploading one 
patch with both 3rd party and mesos. A new RR was here 
https://reviews.apache.org/r/38046/


- Guangya


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


On 九月 1, 2015, 5:30 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37989/
> ---
> 
> (Updated 九月 1, 2015, 5:30 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1935
> https://issues.apache.org/jira/browse/MESOS-1935
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replace hard-coded reap interval with a constant
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/reap.hpp 
> ca5acc4c8f5a62a49b7fde83946e283ea40baa65 
>   3rdparty/libprocess/src/reap.cpp 9a994b052c7920a16557c3d880ad499a5efd43cb 
> 
> Diff: https://reviews.apache.org/r/37989/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38046: Replace hard-coded reap interval with a constant

2015-09-02 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38046]

All tests passed.

- Mesos ReviewBot


On Sept. 2, 2015, 8:16 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38046/
> ---
> 
> (Updated Sept. 2, 2015, 8:16 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1935
> https://issues.apache.org/jira/browse/MESOS-1935
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replace hard-coded reap interval with a constant
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e 
>   src/examples/balloon_executor.cpp 6166f511b8977bc3b6f876de19f1cd91a4a8f03d 
>   src/examples/event_call_framework.cpp 
> f0058fe0e748e5d91c4b7f0c7569108b00e3eab0 
>   src/examples/load_generator_framework.cpp 
> 725478f7162858e9e3136b30476ed1bfbe417681 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
>   src/slave/constants.cpp cf3ee7bbc252364a1b73731feab6a9da68ee1f55 
>   src/slave/monitor.cpp 82aa659b02f98ad8dc4c4f556be7a332dfba6816 
>   src/tests/containerizer/cgroups_tests.cpp 
> 75a3bc0009c037dc18ce319db2eb44630f083e8c 
>   src/tests/containerizer/docker_tests.cpp 
> cfad36850b40ade7a41f5b92255320ca1ed1bf93 
>   src/tests/containerizer/isolator_tests.cpp 
> d34c82a7f24da6f60cfb22790f516dc6065b1f6f 
>   src/tests/containerizer/launch_tests.cpp 
> d211fc0f665988068c67836ef80916828a0df2bd 
>   src/tests/fault_tolerance_tests.cpp 
> 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075 
>   src/tests/gc_tests.cpp ec27ac7f1fca7f425abcea0eafaac4fae905fb8f 
>   src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea 
>   src/tests/limiter.hpp 1bdf8ae85eb4369846c8b367d2f9c81fdaacdf07 
>   src/tests/log_tests.cpp f2dd47cfbe73fb18c360a637db009b7d391a782e 
>   src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 
>   src/tests/rate_limiting_tests.cpp f3aeddee00c7bb7905092aa8a760603468063126 
>   src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23 
>   src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c 
>   src/zookeeper/zookeeper.cpp e44403ee91904415471382dfa4e0a6e0adfdb74f 
> 
> Diff: https://reviews.apache.org/r/38046/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 37280: Maintenance Primitives: Added updateInverseOffer to Allocator.

2015-09-02 Thread Guangya Liu


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



include/mesos/master/allocator.hpp (line 147)


I was adding comments for all allocator interface in 
https://issues.apache.org/jira/browse/MESOS-2224 , and it would be great that 
all new added interface have comments.



src/master/allocator/mesos/hierarchical.hpp (line 885)


"We shouldn't update an inverse offer if we didn't send one out." may be 
better?


- Guangya Liu


On 八月 26, 2015, 2:13 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37280/
> ---
> 
> (Updated 八月 26, 2015, 2:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp 
> aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 
> 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/tests/mesos.hpp 637636ac69dde02da6b7200d7c666cac89b051cb 
> 
> Diff: https://reviews.apache.org/r/37280/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

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

2015-09-02 Thread Yong Qiao Wang


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

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
-

  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39 
  src/slave/slave.cpp 5e5522e1254a5ed6084de36782753f9aad2894c4 

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


Testing
---


Thanks,

Yong Qiao Wang

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

2015-09-02 Thread 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?

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


---
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 38050: Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;

2015-09-02 Thread Klaus Ma


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



src/master/master.cpp 


The indent seems incorrect, here's the suggestion:

install(
&SlaveObserver::pong);



src/slave/slave.cpp 


In slave's UT case, here're at least two cases about PING/PONG package: 
PingTimeoutSomePings & PingTimeoutNoPings; are those UT cases impacted? If so 
please help to update accordingly; and please also help to check the other UT 
cases about PING/PONG message, I'd not go through all slave UT cases.


- Klaus Ma


On Sept. 2, 2015, 12:11 p.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38050/
> ---
> 
> (Updated Sept. 2, 2015, 12:11 p.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
> -
> 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39 
>   src/slave/slave.cpp 5e5522e1254a5ed6084de36782753f9aad2894c4 
> 
> Diff: https://reviews.apache.org/r/38050/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

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

2015-09-02 Thread 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?

http://www.gotw.ca/publications/mill17.htm


- 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 37168: MESOS-3063

2015-09-02 Thread Klaus Ma



> On Sept. 2, 2015, 7:30 a.m., Joerg Schad wrote:
> > As this is an example framework (which will be used by people to start 
> > their own framework), would it make sense to add some more comments 
> > explaining especially the relevant bits here?

Agree, let me add more comments for the detail :); that'll be helpful to our 
uers.


- Klaus


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


On Aug. 28, 2015, 3:31 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated Aug. 28, 2015, 3:31 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3063 (Add an example framework using dynamic reservation)
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7b620ff 
>   src/examples/dynamic_reservation_executor.cpp PRE-CREATION 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 3f56b30 
>   src/tests/flags.hpp 3644956 
>   src/tests/script.cpp bcc1fab 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Review Request 38051: Only update the task status when its old status is not terminal.

2015-09-02 Thread Yong Qiao Wang


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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Only update the task status when its old status is not terminal.


Diffs
-

  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 

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


Testing
---


Thanks,

Yong Qiao Wang

Re: Review Request 36908: Added QuotaInfo Protobuf.

2015-09-02 Thread Joerg Schad



> On Aug. 25, 2015, 5:51 a.m., Guangya Liu wrote:
> > include/mesos/master/quota.proto, line 19
> > 
> >
> > Yes, does v1 API will be supportted for quota?
> 
> Alexander Rukletsov wrote:
> Adding new protobufs should be backwards-compatible, so I suppose the 
> answer is "yes".

Agree.


- Joerg


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


On Aug. 5, 2015, 2:03 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36908/
> ---
> 
> (Updated Aug. 5, 2015, 2:03 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3164
> https://issues.apache.org/jira/browse/MESOS-3164
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added QuotaInfo Protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/quota.hpp PRE-CREATION 
>   include/mesos/master/quota.proto PRE-CREATION 
>   src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 
> 
> Diff: https://reviews.apache.org/r/36908/diff/
> 
> 
> Testing
> ---
> 
> make distcheck
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36908: Added QuotaInfo Protobuf.

2015-09-02 Thread Joerg Schad



> On Aug. 24, 2015, 9:12 p.m., Alexander Rukletsov wrote:
> > include/mesos/master/quota.proto, line 19
> > 
> >
> > I think we should version this protobuf.

As so far only mesos and scheduler are versioned, we don't do this right now 
(as discussed).


- Joerg


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


On Aug. 5, 2015, 2:03 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36908/
> ---
> 
> (Updated Aug. 5, 2015, 2:03 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3164
> https://issues.apache.org/jira/browse/MESOS-3164
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added QuotaInfo Protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/quota.hpp PRE-CREATION 
>   include/mesos/master/quota.proto PRE-CREATION 
>   src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 
> 
> Diff: https://reviews.apache.org/r/36908/diff/
> 
> 
> Testing
> ---
> 
> make distcheck
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36908: Added QuotaInfo Protobuf.

2015-09-02 Thread Joerg Schad


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

(Updated Sept. 2, 2015, 1:38 p.m.)


Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Rebased and minor rename.


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


Repository: mesos


Description
---

Added QuotaInfo Protobuf.


Diffs (updated)
-

  include/mesos/master/quota.hpp PRE-CREATION 
  include/mesos/master/quota.proto PRE-CREATION 
  src/Makefile.am 7b4d9f65506e7fa8425966009401aae73cdb79a5 

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


Testing
---

make distcheck


Thanks,

Joerg Schad

Re: Review Request 38051: Only update the task status when its old status is not terminal.

2015-09-02 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38051]

All tests passed.

- Mesos ReviewBot


On Sept. 2, 2015, 1:13 p.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 2, 2015, 1:13 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

2015-09-02 Thread Joerg Schad


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

(Updated Sept. 2, 2015, 1:46 p.m.)


Review request for mesos, Alexander Rukletsov and Bernd Mathiske.


Changes
---

Rebased


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


Repository: mesos


Description
---

Added /quota HTTP Endpoint for Quota handling.


Diffs (updated)
-

  src/Makefile.am 7b4d9f65506e7fa8425966009401aae73cdb79a5 
  src/master/http.cpp 94e97a2898106579434e8cdec04b7b0e130a810e 
  src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/master/quota_handler.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Joerg Schad

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

2015-09-02 Thread Yong Qiao Wang


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

(Updated 九月 2, 2015, 2:16 p.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/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39 
  src/slave/slave.cpp 5e5522e1254a5ed6084de36782753f9aad2894c4 
  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
---


Thanks,

Yong Qiao Wang

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

2015-09-02 Thread Yong Qiao Wang



> On 九月 2, 2015, 12:55 p.m., Klaus Ma wrote:
> >

Thanks @klaus Ma for your comments. I have addressed all of them.


- Yong Qiao


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


On 九月 2, 2015, 2:16 p.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38050/
> ---
> 
> (Updated 九月 2, 2015, 2:16 p.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
> -
> 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39 
>   src/slave/slave.cpp 5e5522e1254a5ed6084de36782753f9aad2894c4 
>   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
> ---
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

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

2015-09-02 Thread Yong Qiao Wang


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

Ship it!


Ship It!

- Yong Qiao Wang


On 八月 30, 2015, 8:45 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37669/
> ---
> 
> (Updated 八月 30, 2015, 8:45 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 12dc0505c9ec4bd380e817d44da2c4e8d1b0d5f5 
> 
> Diff: https://reviews.apache.org/r/37669/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 38056: Fix webui task informations not update bug.

2015-09-02 Thread haosdent huang


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

Review request for mesos, Adam B and Ross Allen.


Repository: mesos


Description
---

Fix webui task informations not update bug.


Diffs
-

  src/webui/master/static/js/controllers.js 
3445028138fd621d0e3a5427668c5eca7db5224f 

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


Testing
---

Manual test, test scenarios:
* Normal scenarios updated after the new task is finished
* After mesos-master down, could updated after reconnected


Thanks,

haosdent huang

Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

2015-09-02 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [36908, 36913]

All tests passed.

- Mesos ReviewBot


On Sept. 2, 2015, 1:46 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36913/
> ---
> 
> (Updated Sept. 2, 2015, 1:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
> 
> 
> Bugs: MESOS-3073
> https://issues.apache.org/jira/browse/MESOS-3073
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added /quota HTTP Endpoint for Quota handling.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7b4d9f65506e7fa8425966009401aae73cdb79a5 
>   src/master/http.cpp 94e97a2898106579434e8cdec04b7b0e130a810e 
>   src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36913/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 37500: Update the FrameworkInfo.user on scheduler failover

2015-09-02 Thread Guangya Liu


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



src/slave/slave.cpp (line 2133)


This message needs to be updated as u are not only updating pid but also 
user name


- Guangya Liu


On 八月 25, 2015, 8:25 p.m., Aditi Dixit wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37500/
> ---
> 
> (Updated 八月 25, 2015, 8:25 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3240
> https://issues.apache.org/jira/browse/MESOS-3240
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added user to master's state, slave's state (not exposed) and updated user in 
> all slaves that have the registered framework. Added the test too.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 
>   src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c 
>   src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39 
>   src/slave/slave.cpp 2a99abc00c525a93508d38e74d351d2f36572d86 
>   src/tests/fault_tolerance_tests.cpp 
> 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075 
>   src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 
>   src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc 
>   src/tests/slave_tests.cpp d55e9dd4f4eb84a8fda85439e31a38e70890b377 
> 
> Diff: https://reviews.apache.org/r/37500/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Aditi Dixit
> 
>

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

2015-09-02 Thread Bernd Mathiske

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

src/docker/docker.cpp (line 199)

s/strVersion/versionString

(Let's not use arbitrary abbreviations where we can avoid it.)

src/docker/docker.cpp (line 201)

The "[0]" access looks dicy, but after reading the implementation of 
strings::split(), I am fairly convinced that we will never crash here. Such 
tricky code needs to be commented, though. I'd break out the inner split and 
comment on what we are doing there. This seems to erase trailing version string 
components starting with a "-". But how does this relate to what is written in 
line 195? Do we need to worry about splitting at "-" at all?

- Bernd Mathiske

On Aug. 30, 2015, 1:45 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37669/
> ---
> 
> (Updated Aug. 30, 2015, 1:45 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 12dc0505c9ec4bd380e817d44da2c4e8d1b0d5f5 
> 
> Diff: https://reviews.apache.org/r/37669/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

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

2015-09-02 Thread haosdent huang



> On Sept. 2, 2015, 2:54 p.m., Bernd Mathiske wrote:
> > src/docker/docker.cpp, line 201
> > 
> >
> > The "[0]" access looks dicy, but after reading the implementation of 
> > strings::split(), I am fairly convinced that we will never crash here. Such 
> > tricky code needs to be commented, though. I'd break out the inner split 
> > and comment on what we are doing there. This seems to erase trailing 
> > version string components starting with a "-". But how does this relate to 
> > what is written in line 195? Do we need to worry about splitting at "-" at 
> > all?

LoL, thank you very much. I think my code still have bug here. Let me update 
again.


- haosdent


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


On Aug. 30, 2015, 8:45 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37669/
> ---
> 
> (Updated Aug. 30, 2015, 8:45 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 12dc0505c9ec4bd380e817d44da2c4e8d1b0d5f5 
> 
> Diff: https://reviews.apache.org/r/37669/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

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

2015-09-02 Thread haosdent huang



> On Sept. 2, 2015, 2:54 p.m., Bernd Mathiske wrote:
> > src/docker/docker.cpp, line 201
> > 
> >
> > The "[0]" access looks dicy, but after reading the implementation of 
> > strings::split(), I am fairly convinced that we will never crash here. Such 
> > tricky code needs to be commented, though. I'd break out the inner split 
> > and comment on what we are doing there. This seems to erase trailing 
> > version string components starting with a "-". But how does this relate to 
> > what is written in line 195? Do we need to worry about splitting at "-" at 
> > all?
> 
> haosdent huang wrote:
> LoL, thank you very much. I think my code still have bug here. Let me 
> update again.

I would also append with a mock test case in this patch. Need sometime here.


- haosdent


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


On Aug. 30, 2015, 8:45 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37669/
> ---
> 
> (Updated Aug. 30, 2015, 8:45 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 12dc0505c9ec4bd380e817d44da2c4e8d1b0d5f5 
> 
> Diff: https://reviews.apache.org/r/37669/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37873: Add quiesce logic in allocator

2015-09-02 Thread James Peach


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


Using a ```RefusedFilter``` for this is not the right solution. The point of 
the suppress call is to have a cheap way to disable offers at a framework 
granularity. Adding a ```RefusedFilter``` for the total is not cheap. Filters 
are not ordered and a framework might have already accumulated a lot of filters 
by declining offers. All that is needed is a boolean flag on the framework.

- James Peach


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 38046: Replace hard-coded reap interval with a constant

2015-09-02 Thread Alexander Rukletsov


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


I've started to review this patch and then I realized that it was very likely 
generated by automatic substitution of `Seconds(1)` by `MAX_REAP_INTERVAL()`. 
But not every 1s interval is related to reaping. A good example is allocation 
interval. Can we exclude those from the patch? Thanks!

- Alexander Rukletsov


On Sept. 2, 2015, 8:16 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38046/
> ---
> 
> (Updated Sept. 2, 2015, 8:16 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1935
> https://issues.apache.org/jira/browse/MESOS-1935
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replace hard-coded reap interval with a constant
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e 
>   src/examples/balloon_executor.cpp 6166f511b8977bc3b6f876de19f1cd91a4a8f03d 
>   src/examples/event_call_framework.cpp 
> f0058fe0e748e5d91c4b7f0c7569108b00e3eab0 
>   src/examples/load_generator_framework.cpp 
> 725478f7162858e9e3136b30476ed1bfbe417681 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
>   src/slave/constants.cpp cf3ee7bbc252364a1b73731feab6a9da68ee1f55 
>   src/slave/monitor.cpp 82aa659b02f98ad8dc4c4f556be7a332dfba6816 
>   src/tests/containerizer/cgroups_tests.cpp 
> 75a3bc0009c037dc18ce319db2eb44630f083e8c 
>   src/tests/containerizer/docker_tests.cpp 
> cfad36850b40ade7a41f5b92255320ca1ed1bf93 
>   src/tests/containerizer/isolator_tests.cpp 
> d34c82a7f24da6f60cfb22790f516dc6065b1f6f 
>   src/tests/containerizer/launch_tests.cpp 
> d211fc0f665988068c67836ef80916828a0df2bd 
>   src/tests/fault_tolerance_tests.cpp 
> 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075 
>   src/tests/gc_tests.cpp ec27ac7f1fca7f425abcea0eafaac4fae905fb8f 
>   src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea 
>   src/tests/limiter.hpp 1bdf8ae85eb4369846c8b367d2f9c81fdaacdf07 
>   src/tests/log_tests.cpp f2dd47cfbe73fb18c360a637db009b7d391a782e 
>   src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 
>   src/tests/rate_limiting_tests.cpp f3aeddee00c7bb7905092aa8a760603468063126 
>   src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23 
>   src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c 
>   src/zookeeper/zookeeper.cpp e44403ee91904415471382dfa4e0a6e0adfdb74f 
> 
> Diff: https://reviews.apache.org/r/38046/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

2015-09-02 Thread Joerg Schad


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

(Updated Sept. 2, 2015, 4:16 p.m.)


Review request for mesos, Alexander Rukletsov and Bernd Mathiske.


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


Repository: mesos


Description
---

Added /quota HTTP Endpoint for Quota handling.


Diffs (updated)
-

  src/Makefile.am 7b4d9f65506e7fa8425966009401aae73cdb79a5 
  src/master/http.cpp 94e97a2898106579434e8cdec04b7b0e130a810e 
  src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/master/quota_handler.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 37873: Add quiesce logic in allocator

2015-09-02 Thread Guangya Liu



> On 九月 2, 2015, 4:07 p.m., James Peach wrote:
> > Using a ```RefusedFilter``` for this is not the right solution. The point 
> > of the suppress call is to have a cheap way to disable offers at a 
> > framework granularity. Adding a ```RefusedFilter``` for the total is not 
> > cheap. Filters are not ordered and a framework might have already 
> > accumulated a lot of filters by declining offers. All that is needed is a 
> > boolean flag on the framework.

Thanks James, but the problem is that we also want to add some logic of 
disabling quiesce offer after a specified time out so there is no need to 
involve operator to disable the quiesce offer. It is good to add a boolean 
value to framework to quiesce offer, but the problem is how to disable quiesce 
offer automatically? Adding a filter can enable disable a quiesce offer 
automatically, do you have any other solutions for this?


- Guangya


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


On 八月 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 八月 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 38056: Fix webui task informations not update bug.

2015-09-02 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38056]

All tests passed.

- Mesos ReviewBot


On Sept. 2, 2015, 2:30 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38056/
> ---
> 
> (Updated Sept. 2, 2015, 2:30 p.m.)
> 
> 
> Review request for mesos, Adam B and Ross Allen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix webui task informations not update bug.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/js/controllers.js 
> 3445028138fd621d0e3a5427668c5eca7db5224f 
> 
> Diff: https://reviews.apache.org/r/38056/diff/
> 
> 
> Testing
> ---
> 
> Manual test, test scenarios:
> * Normal scenarios updated after the new task is finished
> * After mesos-master down, could updated after reconnected
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 38061: Corrected include order in master.hpp

2015-09-02 Thread Alexander Rukletsov


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

Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 

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


Testing
---

None.


Thanks,

Alexander Rukletsov

Re: Review Request 38046: Replace hard-coded reap interval with a constant

2015-09-02 Thread Guangya Liu


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

(Updated 九月 2, 2015, 4:37 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Replace hard-coded reap interval with a constant


Diffs (updated)
-

  src/tests/containerizer/launch_tests.cpp 
d211fc0f665988068c67836ef80916828a0df2bd 
  src/tests/fault_tolerance_tests.cpp 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075 
  src/tests/gc_tests.cpp ec27ac7f1fca7f425abcea0eafaac4fae905fb8f 
  src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea 
  src/tests/log_tests.cpp f2dd47cfbe73fb18c360a637db009b7d391a782e 
  src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 
  src/tests/rate_limiting_tests.cpp f3aeddee00c7bb7905092aa8a760603468063126 
  src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23 
  src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 38061: Corrected include order in master.hpp

2015-09-02 Thread Guangya Liu


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



src/master/master.hpp 


Alex, can you please show more detail for why we need to move #include 
? Does the correct order is sub grouping 
interfaces in a same folder with alpha order? Thanks


- Guangya Liu


On 九月 2, 2015, 4:34 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38061/
> ---
> 
> (Updated 九月 2, 2015, 4:34 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
> 
> Diff: https://reviews.apache.org/r/38061/diff/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 37921: Add Copy backend for provisioners.

2015-09-02 Thread Timothy Chen


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

(Updated Sept. 2, 2015, 4:54 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


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


Repository: mesos


Description
---

Add Copy backend for provisioners.


Diffs (updated)
-

  src/Makefile.am 7b4d9f65506e7fa8425966009401aae73cdb79a5 
  src/slave/containerizer/provisioners/backend.cpp 
2f7c335f62fdeb27526ab9a38a07c097422ae92b 
  src/slave/containerizer/provisioners/backends/copy.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/backends/copy.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_backend_tests.cpp 
d321850613223a2357ca1646a9d988d05171772c 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 38061: Corrected include order in master.hpp

2015-09-02 Thread Alexander Rukletsov



> On Sept. 2, 2015, 4:45 p.m., Guangya Liu wrote:
> > src/master/master.hpp, line 34
> > 
> >
> > Alex, can you please show more detail for why we need to move #include 
> > ? Does the correct order is sub grouping 
> > interfaces in a same folder with alpha order? Thanks

Exactly right: we sort lexicographically inside the folder, nested folders 
after files with newlines between groups.


- Alexander


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


On Sept. 2, 2015, 4:34 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38061/
> ---
> 
> (Updated Sept. 2, 2015, 4:34 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
> 
> Diff: https://reviews.apache.org/r/38061/diff/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 37873: Add quiesce logic in allocator

2015-09-02 Thread James Peach



> On Sept. 2, 2015, 4:07 p.m., James Peach wrote:
> > Using a ```RefusedFilter``` for this is not the right solution. The point 
> > of the suppress call is to have a cheap way to disable offers at a 
> > framework granularity. Adding a ```RefusedFilter``` for the total is not 
> > cheap. Filters are not ordered and a framework might have already 
> > accumulated a lot of filters by declining offers. All that is needed is a 
> > boolean flag on the framework.
> 
> Guangya Liu wrote:
> Thanks James, but the problem is that we also want to add some logic of 
> disabling quiesce offer after a specified time out so there is no need to 
> involve operator to disable the quiesce offer. It is good to add a boolean 
> value to framework to quiesce offer, but the problem is how to disable 
> quiesce offer automatically? Adding a filter can enable disable a quiesce 
> offer automatically, do you have any other solutions for this?

I think you can solve this by using a pointer instead of a boolean flag. Add a 
new ```SuppressedFilter``` class that just represents a suppression request. 
When you receive a suppression, stash the pointer in the framework. Now you can 
tell that the framework is suppressed because the pointer is non-null. On 
suppression timeout, you can clear the framework pointer if it is the same as 
the filter you are clearing. Since you never dereference the framework's copy 
of the pointer, the whole scheme is safe.


- James


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


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 37773: Docker: Adding registry client.

2015-09-02 Thread Jojy Varghese



- Jojy


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


On Sept. 1, 2015, 9:36 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37773/
> ---
> 
> (Updated Sept. 1, 2015, 9:36 p.m.)
> 
> 
> Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added implementation for docker registry's Get Manifest and Get Blob APIs.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7b4d9f65506e7fa8425966009401aae73cdb79a5 
>   src/slave/containerizer/provisioners/docker/registry_client.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/registry_client.cpp 
> PRE-CREATION 
>   src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37773/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

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

2015-09-02 Thread 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

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, 
int>::type = 0) {...}
```

Note: This file uses some similar Boost type traits further up.


- Joseph


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


On Sept. 1, 2015, 7:20 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37826/
> ---
> 
> (Updated Sept. 1, 2015, 7:20 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 38061: Corrected include order in master.hpp

2015-09-02 Thread Joseph Wu


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

Ship it!


Ship It!

- Joseph Wu


On Sept. 2, 2015, 9:34 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38061/
> ---
> 
> (Updated Sept. 2, 2015, 9:34 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
> 
> Diff: https://reviews.apache.org/r/38061/diff/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 37773: Docker: Adding registry client.

2015-09-02 Thread Jojy Varghese


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

(Updated Sept. 2, 2015, 5:06 p.m.)


Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen.


Changes
---

review comments addressed.


Repository: mesos


Description
---

Added implementation for docker registry's Get Manifest and Get Blob APIs.


Diffs (updated)
-

  src/Makefile.am 7b4d9f65506e7fa8425966009401aae73cdb79a5 
  src/slave/containerizer/provisioners/docker/registry_client.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/registry_client.cpp PRE-CREATION 
  src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Jojy Varghese

Re: Review Request 37427: Docker registry: adding TokenManager.

2015-09-02 Thread Jojy Varghese


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

(Updated Sept. 2, 2015, 5:07 p.m.)


Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen.


Changes
---

review comments addressed.


Repository: mesos


Description
---

Changes:
  - Added Token implementation (RFC 7519).
  - Added TokenManager implementation. This component keeps a cache of tokens
  requested for any future requests.


Diffs (updated)
-

  src/Makefile.am 7b4d9f65506e7fa8425966009401aae73cdb79a5 
  src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION 
  src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38046: Replace hard-coded reap interval with a constant

2015-09-02 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38046]

All tests passed.

- Mesos ReviewBot


On Sept. 2, 2015, 4:37 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38046/
> ---
> 
> (Updated Sept. 2, 2015, 4:37 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1935
> https://issues.apache.org/jira/browse/MESOS-1935
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replace hard-coded reap interval with a constant
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/launch_tests.cpp 
> d211fc0f665988068c67836ef80916828a0df2bd 
>   src/tests/fault_tolerance_tests.cpp 
> 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075 
>   src/tests/gc_tests.cpp ec27ac7f1fca7f425abcea0eafaac4fae905fb8f 
>   src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea 
>   src/tests/log_tests.cpp f2dd47cfbe73fb18c360a637db009b7d391a782e 
>   src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 
>   src/tests/rate_limiting_tests.cpp f3aeddee00c7bb7905092aa8a760603468063126 
>   src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23 
>   src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c 
> 
> Diff: https://reviews.apache.org/r/38046/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 34142: AppC provisioner.

2015-09-02 Thread Timothy Chen


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


This is no longer valid right? Can we discard all ian's patches?

- Timothy Chen


On July 7, 2015, 7:43 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34142/
> ---
> 
> (Updated July 7, 2015, 7:43 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Discovers image(s), fetches to the image store, then provisions using
> a backend.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70 
>   src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8c102fb7d1f79ee768cb06de3a976ea12f958712 
>   src/slave/containerizer/provisioners/appc.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/appc.cpp PRE-CREATION 
>   src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
>   src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
> 
> Diff: https://reviews.apache.org/r/34142/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 37993: Add explanatory comments for Allocator interface

2015-09-02 Thread Joseph Wu

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

First of all, thanks for adding comments!

include/mesos/master/allocator.hpp (line 67)

s/start/starts/

include/mesos/master/allocator.hpp (line 68)

The second part of this sentence seem unnecessary.

You could say:

Initializes the allocator and allocator process.  This is called when the 
master starts up.

include/mesos/master/allocator.hpp (line 70)

Suggestion:
Generally determines how often the allocator should update the allocation.  
Note that other calls to the allocator, like Allocator::reviveOffers, may also 
trigger an update.

include/mesos/master/allocator.hpp (lines 71 - 72)

Suggestion:
A callback the allocator should use to send offers to frameworks.  See 
Master::offer for more details.

---
You could also document the Master::offer function (currently undocumented).

include/mesos/master/allocator.hpp (lines 91 - 92)

These two @param's don't add much, so you could exclude them.

I added comments like this in the past, which led to this email thread:
http://www.mail-archive.com/dev%40mesos.apache.org/msg32792.html

include/mesos/master/allocator.hpp (line 94)

s/re-registered/re-registers/

include/mesos/master/allocator.hpp (lines 105 - 108)

Try not to document the implementation of the HierarchicalDRF.  You can, of 
course, offer possible behaviors.

Suggestion:
Removes a framework from the Mesos cluster.  The allocator can decide what 
to do with framework's resources.  
For example, the removed framework's resources may be released and added 
back to the shared pool of resources.

include/mesos/master/allocator.hpp (line 110)

Again, this doesn't add much.  It's up to you if you want to keep it or not.

Same for all the similar @param's below (not marked).

include/mesos/master/allocator.hpp (line 118)

s/resoure/resource/

include/mesos/master/allocator.hpp (line 129)

s/resoure/resource/

include/mesos/master/allocator.hpp (lines 140 - 144)

Run-on sentence.  You could easily split this into 3-4 sentences.

include/mesos/master/allocator.hpp (line 142)

Again, try not to document the HierarchicalDRF ("the built-in allocator").

s/revocable/revocable resources/

include/mesos/master/allocator.hpp (line 160)

Unnecessary @param.  If you do keep it, s/add/added/

include/mesos/master/allocator.hpp (lines 216 - 217)

Suggestion:
An agent may be deactivated if it is disconnnected from the master.

include/mesos/master/allocator.hpp (line 227)

Missing a period.

include/mesos/master/allocator.hpp (lines 229 - 231)

Run-on.  Split this into 2-3 sentences.

include/mesos/master/allocator.hpp (lines 241 - 244)

Run-on.

Suggestion:
A framework requests resources via this API.  The allocator can choose how 
to respond.  For example, requests may be ignored or the allocator may change 
its priorities accordingly.

- Joseph Wu

On Sept. 1, 2015, 8:06 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> ---
> 
> (Updated Sept. 1, 2015, 8:06 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
> https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 37921: Add Copy backend for provisioners.

2015-09-02 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [37921]

All tests passed.

- Mesos ReviewBot


On Sept. 2, 2015, 4:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37921/
> ---
> 
> (Updated Sept. 2, 2015, 4:54 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-2968
> https://issues.apache.org/jira/browse/MESOS-2968
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add Copy backend for provisioners.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7b4d9f65506e7fa8425966009401aae73cdb79a5 
>   src/slave/containerizer/provisioners/backend.cpp 
> 2f7c335f62fdeb27526ab9a38a07c097422ae92b 
>   src/slave/containerizer/provisioners/backends/copy.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/backends/copy.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> d321850613223a2357ca1646a9d988d05171772c 
> 
> Diff: https://reviews.apache.org/r/37921/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 37462: Add support for version detection and parsing.

2015-09-02 Thread Paul Brett


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

(Updated Sept. 2, 2015, 6:52 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Add support for version detection and parsing.


Diffs (updated)
-

  src/linux/perf.hpp c5630118f4858b1a805f25a2db7a24ca0989 
  src/linux/perf.cpp bb8b5918e4c8f4379784d4fa18c3b4709696e1ea 
  src/tests/containerizer/perf_tests.cpp 
8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 

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


Testing
---

sudo make check

Perf tests may fail on many machines because the tests are currently using the 
version of perf installed on the machine to choose decode.  The next patch will 
extend the perf tests to supply test cases for each of the supported perf 
versions.


Thanks,

Paul Brett

Re: Review Request 37462: Add support for version detection and parsing.

2015-09-02 Thread Paul Brett



> On Sept. 1, 2015, 11:43 p.m., Ben Mahler wrote:
> > src/linux/perf.hpp, line 56
> > 
> >
> > Whoops, doesn't this comment belong on the parse function?

Actually, it applies to both of them.  I'll make that clear.


- Paul


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


On Sept. 2, 2015, 6:52 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37462/
> ---
> 
> (Updated Sept. 2, 2015, 6:52 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for version detection and parsing.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp c5630118f4858b1a805f25a2db7a24ca0989 
>   src/linux/perf.cpp bb8b5918e4c8f4379784d4fa18c3b4709696e1ea 
>   src/tests/containerizer/perf_tests.cpp 
> 8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 
> 
> Diff: https://reviews.apache.org/r/37462/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> Perf tests may fail on many machines because the tests are currently using 
> the version of perf installed on the machine to choose decode.  The next 
> patch will extend the perf tests to supply test cases for each of the 
> supported perf versions.
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 37170: Maintenance Primitives: Added `Machine` to Slave struct in Master.

2015-09-02 Thread Joris Van Remoortere


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

(Updated Sept. 2, 2015, 7:31 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
  src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 37176: Maintenance Primitives: Added a new allocation overload to sorter.

2015-09-02 Thread Joris Van Remoortere


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

(Updated Sept. 2, 2015, 7:31 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/allocator/sorter/drf/sorter.hpp 
217c7c434874b3870668c69799d6b59ce1d83973 
  src/master/allocator/sorter/drf/sorter.cpp 
bfc273493419fe46a4d907f4f7fa282cff71b800 
  src/master/allocator/sorter/sorter.hpp 
536a7ad9a2d661bc8aa352d2e0ae41115b1e8a04 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 37773: Docker: Adding registry client.

2015-09-02 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [37871, 37427, 37773]

All tests passed.

- Mesos ReviewBot


On Sept. 2, 2015, 5:06 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37773/
> ---
> 
> (Updated Sept. 2, 2015, 5:06 p.m.)
> 
> 
> Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added implementation for docker registry's Get Manifest and Get Blob APIs.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7b4d9f65506e7fa8425966009401aae73cdb79a5 
>   src/slave/containerizer/provisioners/docker/registry_client.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/registry_client.cpp 
> PRE-CREATION 
>   src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37773/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 37173: Maintenance Primitives: Added unavailability to Allocator's Slave struct.

2015-09-02 Thread Joris Van Remoortere


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

(Updated Sept. 2, 2015, 7:31 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
  include/mesos/master/allocator.proto 10fd9a2d5fcbc18a9ca2d6c9c0ec1c605f21872b 
  src/master/allocator/mesos/allocator.hpp 
aa55755a9c3250579e9366bdbc17a2449e95d659 
  src/master/allocator/mesos/hierarchical.hpp 
38f8fd2c84314bb3731684d0e9795cb4f50a227e 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/tests/hierarchical_allocator_tests.cpp 
9748ca0b3fee25dcec51c64d8ba84dbd4aaf 
  src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24 
  src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 
  src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b 
  src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

2015-09-02 Thread Joris Van Remoortere


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

(Updated Sept. 2, 2015, 7:32 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 

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


Testing
---

The tests break as expected.
With the scheduler API change there are CHECKs that fail.
Once we update the API these will be resolved.


Thanks,

Joris Van Remoortere

Re: Review Request 37282: Maintenance Primitives: Added InverseOffer to V1 API.

2015-09-02 Thread Joris Van Remoortere


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

(Updated Sept. 2, 2015, 7:32 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/internal/devolve.hpp 6e4306df78b9b8d2054e6550209341fd7b0972d6 
  src/internal/devolve.cpp 0a069e51053b572a8d5dc95380732119504dd0c9 
  src/internal/evolve.hpp 13e9f52da98567038ec717f394f79e526a1521e9 
  src/internal/evolve.cpp 11ce9e77490e93f781ceebc33063d13953a11765 
  src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 37283: Maintenance Primitives: Refactored Master maintenance test to use V1 API.

2015-09-02 Thread Joris Van Remoortere


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

(Updated Sept. 2, 2015, 7:32 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/master_maintenance_tests.cpp 
fb8dca3757a9565d5eb5a69eed10aa34602bb15c 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 37175: Maintenance Primitives: Added updateUnavailability to master.

2015-09-02 Thread Joris Van Remoortere


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

(Updated Sept. 2, 2015, 7:31 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
  src/master/allocator/mesos/allocator.hpp 
aa55755a9c3250579e9366bdbc17a2449e95d659 
  src/master/allocator/mesos/hierarchical.hpp 
38f8fd2c84314bb3731684d0e9795cb4f50a227e 
  src/master/http.cpp 94e97a2898106579434e8cdec04b7b0e130a810e 
  src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/tests/master_maintenance_tests.cpp 
fb8dca3757a9565d5eb5a69eed10aa34602bb15c 
  src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 37178: Maintenance Primitives: Added InverseOffers to Scheduler Event Offers.

2015-09-02 Thread Joris Van Remoortere


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

(Updated Sept. 2, 2015, 7:32 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/scheduler/scheduler.proto 
89daf8a6b74057ee156b3ad691397e76fcb835b8 
  include/mesos/v1/scheduler/scheduler.proto 
bd5e82a614b1163b29f9b20e562208efa1ba4b55 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 37177: Maintenance Primitives: Added inverse offers.

2015-09-02 Thread Joris Van Remoortere


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

(Updated Sept. 2, 2015, 7:32 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/maintenance/maintenance.hpp 
7fec3ffe063e766dcec4952001fa5c6e20d9eec8 
  include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
  src/master/allocator/mesos/allocator.hpp 
aa55755a9c3250579e9366bdbc17a2449e95d659 
  src/master/allocator/mesos/hierarchical.hpp 
38f8fd2c84314bb3731684d0e9795cb4f50a227e 
  src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/tests/hierarchical_allocator_tests.cpp 
9748ca0b3fee25dcec51c64d8ba84dbd4aaf 
  src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24 
  src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 
  src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b 
  src/tests/resource_offers_tests.cpp 882a9ff4d09aace486182828bf43b643b0d0c519 
  src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 37172: Maintenance Primitives: Set offer `unavailability` if slave is scheduled for maintenance.

2015-09-02 Thread Joris Van Remoortere


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

(Updated Sept. 2, 2015, 7:31 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/type_utils.hpp 4fb0037a224cab7ebeb6644b5274227fedb172c8 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/tests/master_maintenance_tests.cpp 
fb8dca3757a9565d5eb5a69eed10aa34602bb15c 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 37621: Maintenance Primitives: Gracefully handle inverse offers in pre-V1 scheduler.

2015-09-02 Thread Joris Van Remoortere


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

(Updated Sept. 2, 2015, 7:33 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/sched/sched.cpp 012af0508eeceeccd168b29f36fa258d20b28c21 
  src/tests/master_maintenance_tests.cpp 
fb8dca3757a9565d5eb5a69eed10aa34602bb15c 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 37234: Maintenance Primitives: Added URL field to InverseOffer proto.

2015-09-02 Thread Joris Van Remoortere


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

(Updated Sept. 2, 2015, 7:32 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 37622: Maintenance Primitives: Shutdown & remove slave when maintenance is started.

2015-09-02 Thread Joris Van Remoortere


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

(Updated Sept. 2, 2015, 7:33 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/http.cpp 94e97a2898106579434e8cdec04b7b0e130a810e 
  src/tests/master_maintenance_tests.cpp 
fb8dca3757a9565d5eb5a69eed10aa34602bb15c 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 37280: Maintenance Primitives: Added updateInverseOffer to Allocator.

2015-09-02 Thread Joris Van Remoortere


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

(Updated Sept. 2, 2015, 7:32 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
  src/master/allocator/mesos/allocator.hpp 
aa55755a9c3250579e9366bdbc17a2449e95d659 
  src/master/allocator/mesos/hierarchical.hpp 
38f8fd2c84314bb3731684d0e9795cb4f50a227e 
  src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 37284: Maintenance Primitives: Added support for Accept / Decline of InverseOffers in master.

2015-09-02 Thread Joris Van Remoortere


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

(Updated Sept. 2, 2015, 7:33 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 37623: Maintenance Primitives: Prevent Slaves from registering if the machine is under maintenance.

2015-09-02 Thread Joris Van Remoortere


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

(Updated Sept. 2, 2015, 7:33 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/type_utils.hpp 4fb0037a224cab7ebeb6644b5274227fedb172c8 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/tests/master_maintenance_tests.cpp 
fb8dca3757a9565d5eb5a69eed10aa34602bb15c 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 37427: Docker registry: adding TokenManager.

2015-09-02 Thread Anand Mazumdar


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


Nothing major, Just some fly-by style comments. Looking at the implementation 
in greater detail now.

I wonder if we should split this review into smaller chunks. It currently 
stands at ~900 lines. It's very hard to review this otherwise. A possible 
trivial split can be to separate the implementation/tests. Thoughts ?


src/Makefile.am 


What's the rationale behind moving these ? They had a pattern of the 
.cpp/.hpp files being together and now only *some* of them follow this style.



src/Makefile.am (line 756)


Extra space ?



src/Makefile.am (line 758)


Ditto. Same for the other 3 occurences



src/slave/containerizer/provisioners/docker/token_manager.hpp (line 19)


Can we change this to be __MESOS_DOCKER_TOKEN_MANAGER_HPP__ 

to be in sync with 
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#The__define_Guard

The one's in appc/* seem to be using __MESOS_APPC_STORE__ which seems to be 
correct.



src/slave/containerizer/provisioners/docker/token_manager.hpp (line 56)


Nit: Should this be named just rawToken or just raw as is the case with the 
other occurences ?



src/slave/containerizer/provisioners/docker/token_manager.hpp (line 76)






src/slave/containerizer/provisioners/docker/token_manager.hpp (line 79)


Remove this TODO ? You already have it in the cpp file ?



src/slave/containerizer/provisioners/docker/token_manager.hpp (line 82)


Token(
const std::string& raw,
const JSON::Object& headerJson,
const JSON::Object& claimsJson,
const Option& expireTime,
const Option& notBeforeTime);



src/slave/containerizer/provisioners/docker/token_manager.hpp (line 88)


static Result getTimeValue(
const JSON::Object& object,
const std::string& key);



src/slave/containerizer/provisioners/docker/token_manager.hpp (line 104)


Space after TODO



src/slave/containerizer/provisioners/docker/token_manager.hpp (line 116)


Should we indent all @param across multi-lines similar to what we do for 
@returns ?



src/slave/containerizer/provisioners/docker/token_manager.cpp (line 24)


This is usually frowned upon. Whatever, occurences we have left of this in 
our code-base, we should strive towards removing them. Can you selectively 
include what you need ?



src/slave/containerizer/provisioners/docker/token_manager.cpp (line 25)


Same as above ?



src/slave/containerizer/provisioners/docker/token_manager.cpp (line 48)


Nit: s/std::string/string . There might be more occurences too.



src/slave/containerizer/provisioners/docker/token_manager.cpp (line 120)


Nit: Do you even need this ?

Why not just do segment.length() % 4 on the next line ?



src/slave/containerizer/provisioners/docker/token_manager.cpp (line 167)


Time notBefore; ?



src/slave/containerizer/provisioners/docker/token_manager.cpp (line 175)


Indent



src/slave/containerizer/provisioners/docker/token_manager.cpp (line 205)


How about just :

return expiration.isSome() && Clock::now() >= expiration.get() ?



src/slave/containerizer/provisioners/docker/token_manager.cpp (line 278)


Nit: s/tokenString/token ?



src/slave/containerizer/provisioners/docker/token_manager.cpp (line 286)


Nit: Why the blank-line? We only insert an assingment > 80 chars on the 
previous line



src/slave/containerizer/provisioners/docker/token_manager.cpp (line 300)


Nit: Why not just key ?



src/slave/containerizer/provisioners/docker/token_manager.cpp (line 308)


Remove the "." . We don't use periods to end a LOG line.

Re: Review Request 37466: Update perf tests to including testing the supported perf output formats.

2015-09-02 Thread Paul Brett


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

(Updated Sept. 2, 2015, 8:06 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Addressed reviewer comments.  Reversed tuple order to improve readability.  
Remove ROOT flag from event validation test.


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


Repository: mesos


Description
---

Update perf tests to including testing the supported perf output formats.


Diffs (updated)
-

  src/tests/containerizer/perf_tests.cpp 
8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 38061: Corrected include order in master.hpp

2015-09-02 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [38061]

All tests passed.

- Mesos ReviewBot


On Sept. 2, 2015, 4:34 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38061/
> ---
> 
> (Updated Sept. 2, 2015, 4:34 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
> 
> Diff: https://reviews.apache.org/r/38061/diff/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 37623: Maintenance Primitives: Prevent Slaves from registering if the machine is under maintenance.

2015-09-02 Thread Mesos ReviewBot


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


Bad patch!

Reviews applied: [37655, 36321, 36571, 37314, 37900, 37325, 37358, 37362, 
37364, 37170, 37172, 37173, 37175, 37176, 37177]

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

Error:
 2015-09-02 20:39:49 URL:https://reviews.apache.org/r/37177/diff/raw/ 
[36178/36178] -> "37177.patch" [1]
error: patch failed: src/master/allocator/mesos/hierarchical.hpp:175
error: src/master/allocator/mesos/hierarchical.hpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Sept. 2, 2015, 7:33 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37623/
> ---
> 
> (Updated Sept. 2, 2015, 7:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp 4fb0037a224cab7ebeb6644b5274227fedb172c8 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/master_maintenance_tests.cpp 
> fb8dca3757a9565d5eb5a69eed10aa34602bb15c 
> 
> Diff: https://reviews.apache.org/r/37623/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 37466: Update perf tests to including testing the supported perf output formats.

2015-09-02 Thread Mesos ReviewBot


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


Bad patch!

Reviews applied: [37416, 37442, 37462, 37466]

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

Error:
 2015-09-02 20:58:54 URL:https://reviews.apache.org/r/37466/diff/raw/ 
[8757/8757] -> "37466.patch" [1]
error: patch failed: src/tests/containerizer/perf_tests.cpp:56
error: src/tests/containerizer/perf_tests.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Sept. 2, 2015, 8:06 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37466/
> ---
> 
> (Updated Sept. 2, 2015, 8:06 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update perf tests to including testing the supported perf output formats.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/perf_tests.cpp 
> 8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 
> 
> Diff: https://reviews.apache.org/r/37466/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 37427: Docker registry: adding TokenManager.

2015-09-02 Thread Jojy Varghese



> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > Nothing major, Just some fly-by style comments. Looking at the 
> > implementation in greater detail now.
> > 
> > I wonder if we should split this review into smaller chunks. It currently 
> > stands at ~900 lines. It's very hard to review this otherwise. A possible 
> > trivial split can be to separate the implementation/tests. Thoughts ?

The review is already a sub-unit of the larger registry client patch.


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/Makefile.am, line 484
> > 
> >
> > What's the rationale behind moving these ? They had a pattern of the 
> > .cpp/.hpp files being together and now only *some* of them follow this 
> > style.

I thought an early reviewer asked me to do it. I might have misunderstood.


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/Makefile.am, line 761
> > 
> >
> > Extra space ?

looks to be aligned with the rest


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 19
> > 
> >
> > Can we change this to be __MESOS_DOCKER_TOKEN_MANAGER_HPP__ 
> > 
> > to be in sync with 
> > https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#The__define_Guard
> > 
> > The one's in appc/* seem to be using __MESOS_APPC_STORE__ which seems 
> > to be correct.

Most of the source code in mesos follow this style (my reference was http.hpp).


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 79
> > 
> >
> > Remove this TODO ? You already have it in the cpp file ?

I doubt that cpp file has the same TODO.


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 82
> > 
> >
> > Token(
> > const std::string& raw,
> > const JSON::Object& headerJson,
> > const JSON::Object& claimsJson,
> > const Option& expireTime,
> > const Option& notBeforeTime);

I believe the pattern in code is an acceptable one according to the "Function 
Definition/Invocation" of the style guide.


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 88
> > 
> >
> > static Result getTimeValue(
> > const JSON::Object& object,
> > const std::string& key);

same as above.


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 104
> > 
> >
> > Space after TODO

TODO(jojy): is the correct patter, unless i understand your comment wrong.


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 116
> > 
> >
> > Should we indent all @param across multi-lines similar to what we do 
> > for @returns ?

No. If you look at the doxygen guide for mesos, params are separated by single 
space (not aligned)


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 24
> > 
> >
> > This is usually frowned upon. Whatever, occurences we have left of this 
> > in our code-base, we should strive towards removing them. Can you 
> > selectively include what you need ?

hrm.. i need process namespace.


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 25
> > 
> >
> > Same as above ?

i need process::http namespace.


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 167
> > 
> >
> > Time notBefore; ?

No i meant option.


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 205
> > 
> >
> > How about just :
> > 
> > return expiration.isSome() && Clock::now() >= expiration.get() ?

for readability.


> On Sept. 2, 2

Review Request 38069: Added a tuple overload for process::collect.

2015-09-02 Thread Ben Mahler


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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Adds an overload similar to what we did for process::await.

This was needed for https://reviews.apache.org/r/37462.


Diffs
-

  3rdparty/libprocess/include/process/collect.hpp 
a1e20515536c002290a4832d94e132a4d42056d8 
  3rdparty/libprocess/src/tests/process_tests.cpp 
debc7317f3ae7c9a9a00244de7ea8e125fb927d6 

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


Testing
---

Added tests.


Thanks,

Ben Mahler

Re: Review Request 37170: Maintenance Primitives: Added `Machine` to Slave struct in Master.

2015-09-02 Thread Joseph Wu


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

Ship it!


LGTM!


src/master/master.cpp (lines 1367 - 1369)


Do we want to modify the registry operations to include this information?  
(On the add/remove slave operations.)


- Joseph Wu


On Sept. 2, 2015, 12:31 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37170/
> ---
> 
> (Updated Sept. 2, 2015, 12:31 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
> 
> Diff: https://reviews.apache.org/r/37170/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 37466: Update perf tests to including testing the supported perf output formats.

2015-09-02 Thread Paul Brett


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

(Updated Sept. 2, 2015, 9:13 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Rebase


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


Repository: mesos


Description
---

Update perf tests to including testing the supported perf output formats.


Diffs (updated)
-

  src/tests/containerizer/perf_tests.cpp 
8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 38069: Added a tuple overload for process::collect.

2015-09-02 Thread Vinod Kone


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

Ship it!


Ship It!

- Vinod Kone


On Sept. 2, 2015, 9:09 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38069/
> ---
> 
> (Updated Sept. 2, 2015, 9:09 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds an overload similar to what we did for process::await.
> 
> This was needed for https://reviews.apache.org/r/37462.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/collect.hpp 
> a1e20515536c002290a4832d94e132a4d42056d8 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> debc7317f3ae7c9a9a00244de7ea8e125fb927d6 
> 
> Diff: https://reviews.apache.org/r/38069/diff/
> 
> 
> Testing
> ---
> 
> Added tests.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 37427: Docker registry: adding TokenManager.

2015-09-02 Thread Anand Mazumdar



> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 19
> > 
> >
> > Can we change this to be __MESOS_DOCKER_TOKEN_MANAGER_HPP__ 
> > 
> > to be in sync with 
> > https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#The__define_Guard
> > 
> > The one's in appc/* seem to be using __MESOS_APPC_STORE__ which seems 
> > to be correct.
> 
> Jojy Varghese wrote:
> Most of the source code in mesos follow this style (my reference was 
> http.hpp).

Which http.hpp are we talking about here ? If you are referring to 
include/process/http.hpp , it already does the right thing i.e. 
PROCESS_HTTP_HPP ?


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/Makefile.am, line 761
> > 
> >
> > Extra space ?
> 
> Jojy Varghese wrote:
> looks to be aligned with the rest

The '\''s at the end does not seem to be aligned.


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 79
> > 
> >
> > Remove this TODO ? You already have it in the cpp file ?
> 
> Jojy Varghese wrote:
> I doubt that cpp file has the same TODO.

My bad, the other's were for validation. Dropped.


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 82
> > 
> >
> > Token(
> > const std::string& raw,
> > const JSON::Object& headerJson,
> > const JSON::Object& claimsJson,
> > const Option& expireTime,
> > const Option& notBeforeTime);
> 
> Jojy Varghese wrote:
> I believe the pattern in code is an acceptable one according to the 
> "Function Definition/Invocation" of the style guide.

Then follow one style and not mix and match. There are other occurences in 
review that seem to follow this pattern. I wonder what's the harm in sticking 
to one ?


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 116
> > 
> >
> > Should we indent all @param across multi-lines similar to what we do 
> > for @returns ?
> 
> Jojy Varghese wrote:
> No. If you look at the doxygen guide for mesos, params are separated by 
> single space (not aligned)

sgtm.


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 104
> > 
> >
> > Space after TODO
> 
> Jojy Varghese wrote:
> TODO(jojy): is the correct patter, unless i understand your comment wrong.

Dropped.


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 24
> > 
> >
> > This is usually frowned upon. Whatever, occurences we have left of this 
> > in our code-base, we should strive towards removing them. Can you 
> > selectively include what you need ?
> 
> Jojy Varghese wrote:
> hrm.. i need process namespace.

Why do you need the entire namespace here ? You should be able to selectively 
use whatever you want from the 'process' namespace or am I missing something ?


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 167
> > 
> >
> > Time notBefore; ?
> 
> Jojy Varghese wrote:
> No i meant option.

Ahh, I see. Dropped.


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 300
> > 
> >
> > Nit: Why not just key ?
> 
> Jojy Varghese wrote:
> to be explicit.

Looks pretty redundant here. "key" can refer to only one thing inside this 
function and that being a "tokenKey"


- Anand


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


On Sept. 2, 2015, 5:07 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37427/
> ---
> 
> (Updated Sept. 2, 2015, 5:07 p.m.)
> 
> 
> Review re

Re: Review Request 37921: Add Copy backend for provisioners.

2015-09-02 Thread Jiang Yan Xu


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

Ship it!



src/slave/containerizer/provisioners/backends/copy.hpp (lines 41 - 42)


This requirement is no longer true.



src/slave/containerizer/provisioners/backends/copy.cpp (line 135)


Are we sure if the layer previous didn't have the trailing slash. 

AppcProvisioner does make sure but within this file perhaps it won't hurt 
to check. i.e., trim the tail first and then add a trailing list.



src/slave/containerizer/provisioners/backends/copy.cpp (line 139)


One blank line above.


- Jiang Yan Xu


On Sept. 2, 2015, 9:54 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37921/
> ---
> 
> (Updated Sept. 2, 2015, 9:54 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-2968
> https://issues.apache.org/jira/browse/MESOS-2968
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add Copy backend for provisioners.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7b4d9f65506e7fa8425966009401aae73cdb79a5 
>   src/slave/containerizer/provisioners/backend.cpp 
> 2f7c335f62fdeb27526ab9a38a07c097422ae92b 
>   src/slave/containerizer/provisioners/backends/copy.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/backends/copy.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> d321850613223a2357ca1646a9d988d05171772c 
> 
> Diff: https://reviews.apache.org/r/37921/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 37466: Update perf tests to including testing the supported perf output formats.

2015-09-02 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [37416, 37442, 37462, 37466]

All tests passed.

- Mesos ReviewBot


On Sept. 2, 2015, 9:13 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37466/
> ---
> 
> (Updated Sept. 2, 2015, 9:13 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update perf tests to including testing the supported perf output formats.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/perf_tests.cpp 
> 8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 
> 
> Diff: https://reviews.apache.org/r/37466/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 37427: Docker registry: adding TokenManager.

2015-09-02 Thread Jojy Varghese



> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 19
> > 
> >
> > Can we change this to be __MESOS_DOCKER_TOKEN_MANAGER_HPP__ 
> > 
> > to be in sync with 
> > https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#The__define_Guard
> > 
> > The one's in appc/* seem to be using __MESOS_APPC_STORE__ which seems 
> > to be correct.
> 
> Jojy Varghese wrote:
> Most of the source code in mesos follow this style (my reference was 
> http.hpp).
> 
> Anand Mazumdar wrote:
> Which http.hpp are we talking about here ? If you are referring to 
> include/process/http.hpp , it already does the right thing i.e. 
> PROCESS_HTTP_HPP ?

I must be misunderstanding what you are trying to say. google style guide asks 
to have the header guard to follow directory path. Here the dierctory path is 
too deep. So the relative path is slave/containerizer. Then the rest of the 
path is provisioner/docker. We dont follow that in any provisioner headers now. 
The uniqueness about this header is that it is a "provisioner" of type "docker" 
and belongs to the "registry" namespace. I have the extra "registry" in the 
guard to reflect that.


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 300
> > 
> >
> > Nit: Why not just key ?
> 
> Jojy Varghese wrote:
> to be explicit.
> 
> Anand Mazumdar wrote:
> Looks pretty redundant here. "key" can refer to only one thing inside 
> this function and that being a "tokenKey"

:). Its a personal choice.


> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 24
> > 
> >
> > This is usually frowned upon. Whatever, occurences we have left of this 
> > in our code-base, we should strive towards removing them. Can you 
> > selectively include what you need ?
> 
> Jojy Varghese wrote:
> hrm.. i need process namespace.
> 
> Anand Mazumdar wrote:
> Why do you need the entire namespace here ? You should be able to 
> selectively use whatever you want from the 'process' namespace or am I 
> missing something ?

I understand namespace pollution issue but its a common pattern i saw in most 
of the cpp files.


- Jojy


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


On Sept. 2, 2015, 5:07 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37427/
> ---
> 
> (Updated Sept. 2, 2015, 5:07 p.m.)
> 
> 
> Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes:
>   - Added Token implementation (RFC 7519).
>   - Added TokenManager implementation. This component keeps a cache of tokens
>   requested for any future requests.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7b4d9f65506e7fa8425966009401aae73cdb79a5 
>   src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION 
>   src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37427/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 37172: Maintenance Primitives: Set offer `unavailability` if slave is scheduled for maintenance.

2015-09-02 Thread Joseph Wu


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

Ship it!


LGTM.  
(And I think BenH's issue is no longer present either.)

- Joseph Wu


On Sept. 2, 2015, 12:31 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37172/
> ---
> 
> (Updated Sept. 2, 2015, 12:31 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp 4fb0037a224cab7ebeb6644b5274227fedb172c8 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/master_maintenance_tests.cpp 
> fb8dca3757a9565d5eb5a69eed10aa34602bb15c 
> 
> Diff: https://reviews.apache.org/r/37172/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 37462: Add support for version detection and parsing.

2015-09-02 Thread Ben Mahler

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

Ship it!

I've made adjustments based on the feedback below before committing, please 
take a look over the comments to make sure they all make sense to you.

src/linux/perf.cpp (line 271)

Missed this earlier, s/=//

src/linux/perf.cpp (line 287)

Looks like this comment belongs in the overload now?

src/linux/perf.cpp (lines 348 - 351)

If you use process::collect instead of process::await, this gets cleaned up 
substantially, you can deal directly with the values rather than the 
transitioned Futures.

src/linux/perf.cpp (line 352)

I'd suggest we just unpack the tuple at the top, otherwise it looks pretty 
verbose. Note that this would be similar to adding an `unpacked` function 
wrapper as michael suggested here:

https://issues.apache.org/jira/browse/MESOS-3188

src/linux/perf.cpp (lines 358 - 359)

The reason should come after the colon, so this reads a bit strangely:

Perf version unsupported: 2.6.38

How about:

Perf 2.6.38 is not supported

src/linux/perf.cpp (lines 383 - 384)

Much like we don't wait for version and output forever in perf::supported, 
let's add a TODO here to not wait forever:

```
// TODO(pbrett): Don't wait for these forever.
```

src/linux/perf.cpp (line 384)

Generally we try to put the .then on the next line, as if it was a 
statement:

```
  return process::collect(perf::version(), output)
.then(parse);
```

src/linux/perf.cpp 

Seems minor, but makes life easier for reviewers when code movement is 
pulled out in separate patches as the diff is easier to read. :)

src/linux/perf.cpp (line 417)

We already say this in the comment for this function, and it's shown in the 
call to split, do we need to say it again?

src/linux/perf.cpp (lines 419 - 421)

How about putting the format on a separate line as you did before?

```
  // Optional running time and ratio were introduced in Linux v4.0,
  // which make the format either:
  //   value,unit,event,cgroup
  //   value,unit,event,cgroup,running,ratio
```

src/linux/perf.cpp (lines 468 - 469)

It's a little bit easier to avoid missing close quotes when it's on the 
same line:

```
return Error("Unexpected event '" + sample->event + "'"
 " in perf output at line: " + line);
```

- Ben Mahler

On Sept. 2, 2015, 6:52 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37462/
> ---
> 
> (Updated Sept. 2, 2015, 6:52 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for version detection and parsing.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp c5630118f4858b1a805f25a2db7a24ca0989 
>   src/linux/perf.cpp bb8b5918e4c8f4379784d4fa18c3b4709696e1ea 
>   src/tests/containerizer/perf_tests.cpp 
> 8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 
> 
> Diff: https://reviews.apache.org/r/37462/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> Perf tests may fail on many machines because the tests are currently using 
> the version of perf installed on the machine to choose decode.  The next 
> patch will extend the perf tests to supply test cases for each of the 
> supported perf versions.
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 37175: Maintenance Primitives: Added updateUnavailability to master.

2015-09-02 Thread Joseph Wu


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

Ship it!



include/mesos/master/allocator.hpp (lines 138 - 141)


Consider Doxygen-ification.



src/master/http.cpp (lines 1448 - 1450)


You'll probably want to delete this line, since `updateUnavailability` does 
that for us.



src/master/http.cpp (line 1466)


You'll probably want to delete this line, since `updateUnavailability` does 
that for us.



src/tests/master_maintenance_tests.cpp (lines 360 - 361)


Rebase mistake?


- Joseph Wu


On Sept. 2, 2015, 12:31 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37175/
> ---
> 
> (Updated Sept. 2, 2015, 12:31 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp 
> aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 
> 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/http.cpp 94e97a2898106579434e8cdec04b7b0e130a810e 
>   src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/master_maintenance_tests.cpp 
> fb8dca3757a9565d5eb5a69eed10aa34602bb15c 
>   src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 
> 
> Diff: https://reviews.apache.org/r/37175/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 37177: Maintenance Primitives: Added inverse offers.

2015-09-02 Thread Joseph Wu


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

Ship it!


LGTM!


src/master/allocator/mesos/hierarchical.hpp (line 1119)


Nano-nit: You used `NOTE:` on all the other "we implement maintenance here" 
comments.


- Joseph Wu


On Sept. 2, 2015, 12:32 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37177/
> ---
> 
> (Updated Sept. 2, 2015, 12:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/maintenance/maintenance.hpp 
> 7fec3ffe063e766dcec4952001fa5c6e20d9eec8 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp 
> aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 
> 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9748ca0b3fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/master_allocator_tests.cpp 
> 89331965553505f6b7eebf39ad27d943df816a24 
>   src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 
>   src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b 
>   src/tests/resource_offers_tests.cpp 
> 882a9ff4d09aace486182828bf43b643b0d0c519 
>   src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23 
> 
> Diff: https://reviews.apache.org/r/37177/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 37280: Maintenance Primitives: Added updateInverseOffer to Allocator.

2015-09-02 Thread Joseph Wu


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

Ship it!



include/mesos/master/allocator.hpp (lines 152 - 155)


This comment seems copied from: https://reviews.apache.org/r/37175/diff/6#0

I think you meant to say something like:
/**
 * Stores a frameworks response to an inverse offer.
 * If the inverse offer times out, `response` is not set.
 */


- Joseph Wu


On Sept. 2, 2015, 12:32 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37280/
> ---
> 
> (Updated Sept. 2, 2015, 12:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp 
> aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 
> 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 
> 
> Diff: https://reviews.apache.org/r/37280/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 37540: Add PerfEvent API

2015-09-02 Thread Cong Wang


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

(Updated Sept. 2, 2015, 10:13 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.


Changes
---

Rebase and cleanup


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


Repository: mesos


Description
---

Abstract Linux kernel perf event API and provide API to collect schedule events.


Diffs (updated)
-

  src/linux/perf.hpp c5630118f4858b1a805f25a2db7a24ca0989 
  src/linux/perf.cpp bb8b5918e4c8f4379784d4fa18c3b4709696e1ea 
  src/tests/containerizer/perf_tests.cpp 
8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 37541: Add TraceEvent API

2015-09-02 Thread Cong Wang


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

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


Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.


Changes
---

Rebase and cleanup


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


Repository: mesos


Description
---

Based on the PerfEvent API's, add API for Linux kernel trace events, especially 
the schedule trace events.


Diffs (updated)
-

  src/linux/perf.hpp c5630118f4858b1a805f25a2db7a24ca0989 
  src/linux/perf.cpp bb8b5918e4c8f4379784d4fa18c3b4709696e1ea 
  src/tests/containerizer/cgroups_tests.cpp 
75a3bc0009c037dc18ce319db2eb44630f083e8c 
  src/tests/containerizer/perf_tests.cpp 
8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 37427: Docker registry: adding TokenManager.

2015-09-02 Thread Anand Mazumdar



> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 19
> > 
> >
> > Can we change this to be __MESOS_DOCKER_TOKEN_MANAGER_HPP__ 
> > 
> > to be in sync with 
> > https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#The__define_Guard
> > 
> > The one's in appc/* seem to be using __MESOS_APPC_STORE__ which seems 
> > to be correct.
> 
> Jojy Varghese wrote:
> Most of the source code in mesos follow this style (my reference was 
> http.hpp).
> 
> Anand Mazumdar wrote:
> Which http.hpp are we talking about here ? If you are referring to 
> include/process/http.hpp , it already does the right thing i.e. 
> PROCESS_HTTP_HPP ?
> 
> Jojy Varghese wrote:
> I must be misunderstanding what you are trying to say. google style guide 
> asks to have the header guard to follow directory path. Here the dierctory 
> path is too deep. So the relative path is slave/containerizer. Then the rest 
> of the path is provisioner/docker. We dont follow that in any provisioner 
> headers now. The uniqueness about this header is that it is a "provisioner" 
> of type "docker" and belongs to the "registry" namespace. I have the extra 
> "registry" in the guard to reflect that.

I was simply referring to us not having "namespace" names in the header guard 
includes and just using "shorter" relative paths as is being done in other 
parts of the code. Hence, I asked you to remove the "registry" in the guard.

Had no intentions of dragging on this trivial thing !


- Anand


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


On Sept. 2, 2015, 5:07 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37427/
> ---
> 
> (Updated Sept. 2, 2015, 5:07 p.m.)
> 
> 
> Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes:
>   - Added Token implementation (RFC 7519).
>   - Added TokenManager implementation. This component keeps a cache of tokens
>   requested for any future requests.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7b4d9f65506e7fa8425966009401aae73cdb79a5 
>   src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION 
>   src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37427/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 37541: Add trace event API

2015-09-02 Thread Cong Wang


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

(Updated Sept. 2, 2015, 10:17 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.


Summary (updated)
-

Add trace event API


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


Repository: mesos


Description
---

Based on the PerfEvent API's, add API for Linux kernel trace events, especially 
the schedule trace events.


Diffs
-

  src/linux/perf.hpp c5630118f4858b1a805f25a2db7a24ca0989 
  src/linux/perf.cpp bb8b5918e4c8f4379784d4fa18c3b4709696e1ea 
  src/tests/containerizer/cgroups_tests.cpp 
75a3bc0009c037dc18ce319db2eb44630f083e8c 
  src/tests/containerizer/perf_tests.cpp 
8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 37540: Add perf event API

2015-09-02 Thread Cong Wang


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

(Updated Sept. 2, 2015, 10:16 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.


Summary (updated)
-

Add perf event API


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


Repository: mesos


Description
---

Abstract Linux kernel perf event API and provide API to collect schedule events.


Diffs
-

  src/linux/perf.hpp c5630118f4858b1a805f25a2db7a24ca0989 
  src/linux/perf.cpp bb8b5918e4c8f4379784d4fa18c3b4709696e1ea 
  src/tests/containerizer/perf_tests.cpp 
8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 

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


Testing
---

make check


Thanks,

Cong Wang

Review Request 38073: Avoid parsing perf events twice

2015-09-02 Thread Cong Wang


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

Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

We already parse perf events in create(), don't need to parse it again.


Diffs
-

  src/slave/containerizer/isolators/cgroups/perf_event.hpp 
c1578b11ea1afd30929b4ea6f2c8272fb65454ce 
  src/slave/containerizer/isolators/cgroups/perf_event.cpp 
8c3018de8e77a2a00c90559a995eae2b3678e42f 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 38073: Avoid parsing perf events twice

2015-09-02 Thread Ben Mahler


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

Ship it!


Ship It!

- Ben Mahler


On Sept. 2, 2015, 10:18 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38073/
> ---
> 
> (Updated Sept. 2, 2015, 10:18 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2769
> https://issues.apache.org/jira/browse/MESOS-2769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We already parse perf events in create(), don't need to parse it again.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/cgroups/perf_event.hpp 
> c1578b11ea1afd30929b4ea6f2c8272fb65454ce 
>   src/slave/containerizer/isolators/cgroups/perf_event.cpp 
> 8c3018de8e77a2a00c90559a995eae2b3678e42f 
> 
> Diff: https://reviews.apache.org/r/38073/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Review Request 38074: Calculate schedule latency with trace events

2015-09-02 Thread Cong Wang


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

Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

Finally, calculate schedule latency with the sched trace events, and add it to 
our statistics


Diffs
-

  include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
  src/slave/containerizer/isolators/cgroups/perf_event.hpp 
c1578b11ea1afd30929b4ea6f2c8272fb65454ce 
  src/slave/containerizer/isolators/cgroups/perf_event.cpp 
8c3018de8e77a2a00c90559a995eae2b3678e42f 
  src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 
  src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 

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


Testing
---

manual tests


Thanks,

Cong Wang

Re: Review Request 38074: Calculate schedule latency with trace events

2015-09-02 Thread Mesos ReviewBot


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


Bad patch!

Reviews applied: [37540]

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

Error:
 2015-09-02 22:32:04 URL:https://reviews.apache.org/r/37540/diff/raw/ 
[12808/12808] -> "37540.patch" [1]
error: patch failed: src/linux/perf.hpp:57
error: src/linux/perf.hpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Sept. 2, 2015, 10:22 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38074/
> ---
> 
> (Updated Sept. 2, 2015, 10:22 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2769
> https://issues.apache.org/jira/browse/MESOS-2769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Finally, calculate schedule latency with the sched trace events, and add it 
> to our statistics
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   src/slave/containerizer/isolators/cgroups/perf_event.hpp 
> c1578b11ea1afd30929b4ea6f2c8272fb65454ce 
>   src/slave/containerizer/isolators/cgroups/perf_event.cpp 
> 8c3018de8e77a2a00c90559a995eae2b3678e42f 
>   src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 
>   src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 
> 
> Diff: https://reviews.apache.org/r/38074/diff/
> 
> 
> Testing
> ---
> 
> manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 37881: Implemented AppcProvisioner.

2015-09-02 Thread Jiang Yan Xu


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

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


Review request for mesos, Chi Zhang, Jie Yu, and Timothy Chen.


Changes
---

Comments and some refactoring in paths.cpp.


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


Repository: mesos


Description
---

Implemented AppcProvisioner.


Diffs (updated)
-

  src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
  src/slave/containerizer/provisioner.hpp 
541dd4e0b2f0c92a45c00cab6132a2be69654838 
  src/slave/containerizer/provisioner.cpp 
efc7e6996ff6663bebaf61989a7e040bd2ad7a5e 
  src/slave/containerizer/provisioners/appc.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/appc.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/appc/paths.hpp 
41e3bf79da0854406c488855f953111e67353829 
  src/slave/containerizer/provisioners/appc/paths.cpp 
3113c84b9526dd9e9e89fb9aa4ec75ed66a996c7 
  src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 
  src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 
  src/tests/containerizer/appc_provisioner_tests.cpp 
47b66b9c30cefe8f9a8e2c1c1341776c2d235020 

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


Testing
---

sudo make check.

More test cases coming.


Thanks,

Jiang Yan Xu

Re: Review Request 37466: Update perf tests to including testing the supported perf output formats.

2015-09-02 Thread Ben Mahler

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

Thinking about this further, the test parameteratization here seems to be a bit 
of a mess, could we instead just create lists of valid / invalid inputs within 
a test and loop over them? I don't think the test parameterization here is 
worth the complexity.

src/tests/containerizer/perf_tests.cpp (line 51)

Why not just Events? This is testing both valid and invalid events.

Also, can you pull this out into a separate patch?

src/tests/containerizer/perf_tests.cpp (line 67)

Why is this called ParseTypes and the one below called ParseCgroups? They 
both seem to parse cgroup-based perf output, so I'm a bit confused.

src/tests/containerizer/perf_tests.cpp (lines 85 - 100)

Please instantiate right below the class as it makes it clearer when there 
are many tests under the test fixture (it's also what we do in all the other 
tests).

Ditto for the rest of these.

src/tests/containerizer/perf_tests.cpp (lines 223 - 233)

What's with the inconsistent newlines here?

src/tests/containerizer/perf_tests.cpp (line 253)

I think you meant to split the string literal on the newline like you did 
in most cases, can you do a sweep?

- Ben Mahler

On Sept. 2, 2015, 9:13 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37466/
> ---
> 
> (Updated Sept. 2, 2015, 9:13 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update perf tests to including testing the supported perf output formats.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/perf_tests.cpp 
> 8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 
> 
> Diff: https://reviews.apache.org/r/37466/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 37427: Docker registry: adding TokenManager.

2015-09-02 Thread Jojy Varghese


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

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


Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen.


Changes
---

review comments.


Repository: mesos


Description
---

Changes:
  - Added Token implementation (RFC 7519).
  - Added TokenManager implementation. This component keeps a cache of tokens
  requested for any future requests.


Diffs (updated)
-

  src/Makefile.am 7b4d9f65506e7fa8425966009401aae73cdb79a5 
  src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION 
  src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 37773: Docker: Adding registry client.

2015-09-02 Thread Jojy Varghese


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

(Updated Sept. 2, 2015, 10:52 p.m.)


Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen.


Changes
---

rebased with dependent patch.


Repository: mesos


Description
---

Added implementation for docker registry's Get Manifest and Get Blob APIs.


Diffs (updated)
-

  src/Makefile.am 7b4d9f65506e7fa8425966009401aae73cdb79a5 
  src/slave/containerizer/provisioners/docker/registry_client.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/registry_client.cpp PRE-CREATION 
  src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Jojy Varghese

Re: Review Request 37284: Maintenance Primitives: Added support for Accept / Decline of InverseOffers in master.

2015-09-02 Thread Joseph Wu


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

Ship it!



src/master/master.cpp (line 3243)


Missing a space here.


- Joseph Wu


On Sept. 2, 2015, 12:33 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37284/
> ---
> 
> (Updated Sept. 2, 2015, 12:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
> 
> Diff: https://reviews.apache.org/r/37284/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38074: Calculate schedule latency with trace events

2015-09-02 Thread Cong Wang


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

(Updated Sept. 2, 2015, 11:18 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.


Changes
---

Rebase


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


Repository: mesos


Description
---

Finally, calculate schedule latency with the sched trace events, and add it to 
our statistics


Diffs (updated)
-

  include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
  src/slave/containerizer/isolators/cgroups/perf_event.hpp 
1f722ef3ef7ab7fce5542d4affae961d6cec2406 
  src/slave/containerizer/isolators/cgroups/perf_event.cpp 
7dc8b7a99074b74ade019ef4df296780650a2e4e 
  src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 
  src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 

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


Testing
---

manual tests


Thanks,

Cong Wang

Re: Review Request 38074: Calculate schedule latency with trace events

2015-09-02 Thread Cong Wang


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

(Updated Sept. 2, 2015, 11:19 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

Finally, calculate schedule latency with the sched trace events, and add it to 
our statistics


Diffs
-

  include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
  src/slave/containerizer/isolators/cgroups/perf_event.hpp 
1f722ef3ef7ab7fce5542d4affae961d6cec2406 
  src/slave/containerizer/isolators/cgroups/perf_event.cpp 
7dc8b7a99074b74ade019ef4df296780650a2e4e 
  src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 
  src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 

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


Testing
---

manual tests


Thanks,

Cong Wang

Re: Review Request 37622: Maintenance Primitives: Shutdown & remove slave when maintenance is started.

2015-09-02 Thread Joseph Wu


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

Ship it!



src/master/http.cpp (line 1567)


Now that the endpoint is called `/machine/down`, should the message be set 
to something like:
"Operator initiated"?  Or "Machine down"?



src/tests/master_maintenance_tests.cpp (lines 631 - 632)


The first `{` could be moved up a line.


- Joseph Wu


On Sept. 2, 2015, 12:33 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37622/
> ---
> 
> (Updated Sept. 2, 2015, 12:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 94e97a2898106579434e8cdec04b7b0e130a810e 
>   src/tests/master_maintenance_tests.cpp 
> fb8dca3757a9565d5eb5a69eed10aa34602bb15c 
> 
> Diff: https://reviews.apache.org/r/37622/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

1 2 >

1 - 100 of 124 matches

Mail list logo