Re: Review Request 33490: MESOS-2633 Avoid adding duplicate tasks

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33490/ --- (Updated May 7, 2015, 1:26 a.m.) Review request for mesos, Adam B and Joris Van

Re: Review Request 33490: MESOS-2633 Avoid adding duplicate tasks

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33490/#review82809 --- Thanks for attacking one of my TODOs. I've got a couple of questions

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/ --- (Updated May 7, 2015, 9:17 a.m.) Review request for mesos, Benjamin Hindman, Ji

Re: Review Request 30643: Optionally specify executor for "mesos execute".

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30643/#review82830 --- Are you still planning to merge this? - Timothy Chen On Feb. 4, 2

Re: Review Request 33918: Added resources estimator abstraction for oversubscription.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33918/#review82829 --- Ship it! Looks good! I agree with Vinod about the naming, but apart

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/#review82833 --- Patch looks great! Reviews applied: [33249] All tests passed. - M

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/#review82832 --- Do we have any tests to check for the change in mesos containerizer

Re: Review Request 33933: Slowed the webui polling for larger clusters.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33933/#review82836 --- Ship it! Ship It! - Jie Yu On May 7, 2015, 5:47 a.m., Ben Mahler

Re: Review Request 33934: Added the client address to http::Request.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33934/#review82840 --- Ship it! Ship It! - Jie Yu On May 7, 2015, 5:47 a.m., Ben Mahler

Re: Review Request 33935: Improved HTTP request logging for master/slave endpoints.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33935/#review82844 --- src/master/master.cpp

Re: Review Request 30609: Added a function that reports file size, not following links.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30609/#review82848 --- 3rdparty/libprocess/3rdparty/stout/include/stout/os/stat.hpp

Re: Review Request 29748: Added tests for dynamic reservation.

> On April 23, 2015, 4:15 p.m., Alexander Rukletsov wrote: > > src/tests/reservation_tests.cpp, line 108 > > > > > > How about we use a single resource string for clarity? Here we start a > > slave with `"cpus:1;mem:

Re: Review Request 33919: Integrated resources estimator with the slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33919/#review82858 --- src/messages/messages.proto

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

> On May 4, 2015, 8:52 p.m., Bernd Mathiske wrote: > > src/tests/docker_containerizer_tests.cpp, line 1260 > > > > > > Same magical 30 as above. global constant? We have a lot of these in the test code base, I think

Re: Review Request 33733: Fixed style error with template definitions withing common/parse.hpp

> On May 5, 2015, 11:41 p.m., Till Toenshoff wrote: > > Could you fill us in on the reasoning and/or update the styleguide for > > this? Thanks a bunch :) It was a request from benh and mcypark in this review: https://reviews.apache.org/r/33296/ - Alexander

Re: Review Request 32163: Added a function which checks if a json object is contained within another.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32163/#review82073 --- 3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp

Re: Review Request 33490: MESOS-2633 Avoid adding duplicate tasks

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33490/#review82867 --- Hm, I looked at MESOS-2633, that's only about moving implementation

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

> On May 7, 2015, 10:02 a.m., Timothy Chen wrote: > > src/examples/docker_no_executor_framework.cpp, line 109 > > > > > > is this something you like to commit? Whops! That snuck in. I'll fix. > On May 7, 2015, 10:

Re: Review Request 33935: Improved HTTP request logging for master/slave endpoints.

> On May 7, 2015, 5:40 p.m., Jie Yu wrote: > > src/slave/http.cpp, lines 239-241 > > > > > > I think we usually put `?` and `:` in the end (similar to what we put > > `=` in the end): > > > > ``` > > << (

Re: Review Request 33272: Fix capture by reference of temporary strings in Stout.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33272/#review82872 --- Ship it! - Joerg Schad On April 22, 2015, 6:11 p.m., Joris Van Re

Re: Review Request 33276: Fix capture by reference of temporaries in Libprocess.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33276/#review82873 --- Ship it! Maybe we could use benchmark to ensure the overall change

Re: Review Request 32163: Added a function which checks if a json object is contained within another.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32163/#review82870 --- Ship it! 3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/ --- (Updated May 7, 2015, 11:57 a.m.) Review request for mesos, Benjamin Hindman, J

Re: Review Request 33275: Fix capture by reference of temporaries in Stout.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33275/#review82874 --- Ship it! Short bechmark (for the combined effort of removing tempor

Re: Review Request 33274: Fix capture by reference of temporary strings in Libprocess.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33274/#review82875 --- Ship it! Agree with Adam's benchmark comment, would be great. - Jo

Re: Review Request 32198: Added a not equal operator for json objects.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32198/ --- (Updated May 7, 2015, 9:09 p.m.) Review request for mesos, Benjamin Hindman, Be

Re: Review Request 32163: Added a function which checks if a json object is contained within another.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32163/ --- (Updated May 7, 2015, 9:10 p.m.) Review request for mesos, Benjamin Hindman, Be

Re: Review Request 33733: Fixed style error with template definitions withing common/parse.hpp

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33733/#review82876 --- Ship it! Ship It! - Joerg Schad On April 30, 2015, 10:45 p.m., A

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/#review82883 --- Patch looks great! Reviews applied: [33249] All tests passed. - M

Re: Review Request 32198: Added a not equal operator for json objects.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32198/#review82887 --- configure.ac

Re: Review Request 32198: Added a not equal operator for json objects.

> On May 7, 2015, 8:07 p.m., Joerg Schad wrote: > > configure.ac, line 566 > > > > > > How is this related to your change? Looking at the review history could it be that this was added by mistake? - Joerg

Re: Review Request 33935: Improved HTTP request logging for master/slave endpoints.

> On May 7, 2015, 5:40 p.m., Jie Yu wrote: > > src/master/master.cpp, lines 732-735 > > > > > > Why do you need to capture `this`? Http::log is a static function, I > > don't think you need to capture `this`. > >

Re: Review Request 32198: Added a not equal operator for json objects.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32198/ --- (Updated May 7, 2015, 10:37 p.m.) Review request for mesos, Benjamin Hindman, B

Re: Review Request 33358: Moved implementation of StatusUpdateStream to a compilation unit.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33358/#review82897 --- Ship it! Ship It! - Joerg Schad On April 22, 2015, 9:09 a.m., Al

Re: Review Request 33718: Extended documentation on Mesos hooks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33718/ --- (Updated May 7, 2015, 8:40 p.m.) Review request for mesos, Adam B, Kapil Arya,

Re: Review Request 33918: Added resources estimator abstraction for oversubscription.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33918/#review82905 --- src/slave/resources_estimator.hpp

Re: Review Request 33718: Extended documentation on Mesos hooks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33718/#review82908 --- Ship it! Have you rendered this in a markdown viewer? As far as I k

Re: Review Request 33919: Integrated resources estimator with the slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33919/#review82912 --- src/slave/slave.cpp

Re: Review Request 33718: Extended documentation on Mesos hooks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33718/#review82913 --- Patch looks great! Reviews applied: [33718] All tests passed. - M

Re: Review Request 33935: Improved HTTP request logging for master/slave endpoints.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33935/ --- (Updated May 7, 2015, 9:53 p.m.) Review request for mesos, Jie Yu and Vinod Kon

Re: Review Request 33935: Improved HTTP request logging for master/slave endpoints.

> On May 7, 2015, 5:40 p.m., Jie Yu wrote: > > src/master/master.cpp, lines 732-735 > > > > > > Why do you need to capture `this`? Http::log is a static function, I > > don't think you need to capture `this`. > >

Re: Review Request 33935: Improved HTTP request logging for master/slave endpoints.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33935/#review82920 --- Ship it! src/master/master.cpp

Re: Review Request 33643: Add EMPTY to stout hashset

> On May 6, 2015, 12:52 a.m., Cody Maloney wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/hashset.hpp, line 75 > > > > > > Any reason we can't do this as a function with a static in it? Solves > > the dest

Re: Review Request 33935: Improved HTTP request logging for master/slave endpoints.

> On May 7, 2015, 9:56 p.m., Jie Yu wrote: > > src/master/master.cpp, lines 740-742 > > > > > > Forgot this one? Thanks for catching that! - Ben --- This is

Re: Review Request 33935: Improved HTTP request logging for master/slave endpoints.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33935/ --- (Updated May 7, 2015, 10 p.m.) Review request for mesos, Jie Yu and Vinod Kone.

Re: Review Request 33505: Add state-summary endpoint to master.

> On May 5, 2015, 10:36 p.m., Ben Mahler wrote: > > Is there a ticket for this? Would love to get a higher level view of the > > motivation for the new endpoint and its format. A bit tough to tell what a > > sample response looks like without reading through this diff carefully. :) There shoul

Re: Review Request 31504: Add a basic filter to match all packets

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31504/#review82918 --- src/linux/routing/filter/basic.hpp

Re: Review Request 31504: Add a basic filter to match all packets

> On May 7, 2015, 10:03 p.m., Jie Yu wrote: > > src/linux/routing/filter/basic.cpp, lines 95-98 > > > > > > Should you pass a `protocol` as well to the `exists` function? > > > > ``` > > Try exists(link, pa

Re: Review Request 33935: Improved HTTP request logging for master/slave endpoints.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33935/#review82930 --- Patch looks great! Reviews applied: [33933, 33934, 33935] All test

Re: Review Request 31504: Add a basic filter to match all packets

> On May 7, 2015, 10:03 p.m., Jie Yu wrote: > > src/linux/routing/filter/basic.cpp, lines 95-98 > > > > > > Should you pass a `protocol` as well to the `exists` function? > > > > ``` > > Try exists(link, pa

Re: Review Request 31504: Add a basic filter to match all packets

> On May 7, 2015, 10:03 p.m., Jie Yu wrote: > > src/linux/routing/filter/basic.cpp, lines 95-98 > > > > > > Should you pass a `protocol` as well to the `exists` function? > > > > ``` > > Try exists(link, pa

Re: Review Request 31504: Add a basic filter to match all packets

> On May 7, 2015, 10:03 p.m., Jie Yu wrote: > > src/linux/routing/filter/basic.cpp, lines 95-98 > > > > > > Should you pass a `protocol` as well to the `exists` function? > > > > ``` > > Try exists(link, pa

Re: Review Request 31504: Add a basic filter to match all packets

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31504/ --- (Updated May 7, 2015, 11:20 p.m.) Review request for mesos, Chi Zhang, Ian Down

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/#review82940 --- Overall, LGTM! The only reason I don't give a ship it here is becaus

Review Request 33963: Move ARP filter implementation onto basic filter

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33963/ --- Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu. Bugs: MESOS-2422

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

> On May 7, 2015, 4:53 p.m., Jie Yu wrote: > > Overall, LGTM! The only reason I don't give a ship it here is because the > > tests. Could you please add an integration test in > > docker_containerizer_tests.cpp? I'll add the tests tomorrow and update the diff. > On May 7, 2015, 4:53 p.m., Ji

Re: Review Request 33963: Move ARP filter implementation onto basic filter

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33963/#review82957 --- Patch looks great! Reviews applied: [31502, 31503, 31504, 33963] A