Re: Review Request 35510: Introduced General and Markdown Documentation Style Guides.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35510/ --- (Updated June 17, 2015, 6:52 a.m.) Review request for mesos and Bernd Mathiske.

Re: Review Request 30032: Added support for cache control in libprocess when dealing with static files.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30032/#review88198 --- Patch looks great! Reviews applied: [34703, 30032] All tests passe

Re: Review Request 35544: containerizer: statically initialize isolator factories

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

Re: Review Request 35257: Decode network statistics from helper

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35257/#review88190 --- Patch looks great! Reviews applied: [35536, 35229, 35257] All test

Re: Review Request 30032: Added support for cache control in libprocess when dealing with static files.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30032/ --- (Updated June 17, 2015, 4:08 a.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 35510: Introduced General and Markdown Documentation Style Guides.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35510/#review88184 --- Bad patch! Reviews applied: [35364, 35509, 35510] Failed command:

Re: Review Request 35494: Added format for template declaration in the styleguide.

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

Re: Review Request 35433: CHECK that checkpointed resources exist on the slave.

> On June 14, 2015, 10:46 a.m., Benjamin Hindman wrote: > > Just so I understand, does this mean if we happen to get in the unfortunate > > situation where a slave has neglected to get the dynamic reservation > > because it was just starting up and then it gets the task launch it will > > shut

Re: Review Request 35506: Excluding MarkDown files from the style hook

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35506/#review88176 --- Ship it! Ship It! - Isabel Jimenez On June 16, 2015, 11:24 a.m.,

Re: Review Request 35433: CHECK that checkpointed resources exist on the slave.

> On June 14, 2015, 10:46 a.m., Benjamin Hindman wrote: > > Just so I understand, does this mean if we happen to get in the unfortunate > > situation where a slave has neglected to get the dynamic reservation > > because it was just starting up and then it gets the task launch it will > > shut

Review Request 35544: containerizer: statically initialize isolator factories

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35544/ --- Review request for mesos and Timothy Chen. Repository: mesos Description

Re: Review Request 34721: Added QoS kill executor correction test.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34721/ --- (Updated June 16, 2015, 5 p.m.) Review request for mesos, Bartek Plotka, Jie Yu

Re: Review Request 34721: Added QoS kill executor correction test.

> On June 16, 2015, 4:56 p.m., Jie Yu wrote: > > src/tests/oversubscription_tests.cpp, line 762 > > > > > > I cannot tell, does this exceed 80 chars? Good catch - it is too long. Will get that fixed right away. - Ni

Re: Review Request 34721: Added QoS kill executor correction test.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34721/#review88169 --- Ship it! LGTM! src/tests/oversubscription_tests.cpp (line 762)

Re: Review Request 34721: Added QoS kill executor correction test.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34721/ --- (Updated June 16, 2015, 4:52 p.m.) Review request for mesos, Bartek Plotka, Jie

Re: Review Request 35506: Excluding MarkDown files from the style hook

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

Re: Review Request 34721: Added QoS kill executor correction test.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34721/ --- (Updated June 16, 2015, 4:50 p.m.) Review request for mesos, Bartek Plotka, Jie

Re: Review Request 34721: Added QoS kill executor correction test.

> On June 16, 2015, 2:31 p.m., Jie Yu wrote: > > src/tests/mesos.hpp, line 205 > > > > > > Can you instead add a overload for qosController only (similar to what > > we did for resourceEstimator above)? Done > On J

Re: Review Request 34720: Added kill executor correction to slave.

> On June 16, 2015, 2:22 p.m., Jie Yu wrote: > > src/slave/slave.cpp, line 4210 > > > > > > Shouldn't be 'executorId' here? Also, can you put single quotes for > > executorId since it's specifed by the user (i.e., mi

Re: Review Request 34720: Added kill executor correction to slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34720/ --- (Updated June 16, 2015, 4:47 p.m.) Review request for mesos, Bartek Plotka, Jie

Re: Review Request 34719: Added REASON_EXECUTOR_PREEMPTED as status reason.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34719/ --- (Updated June 16, 2015, 4:45 p.m.) Review request for mesos, Bartek Plotka, Jie

Re: Review Request 35536: Replace adhoc JSON conversion functions for ResourceStatistics with a protocol buffer to JSON converter.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35536/ --- (Updated June 16, 2015, 11:44 p.m.) Review request for mesos, Chi Zhang, Ian Do

Re: Review Request 35536: Replace adhoc JSON conversion functions for ResourceStatistics with a protocol buffer to JSON converter.

> On June 16, 2015, 10:33 p.m., Jie Yu wrote: > > src/slave/containerizer/isolators/network/port_mapping.cpp, line 713 > > > > > > I don't think you need to set this. It is a required field. Without it, the conversio

Re: Review Request 35330: Capped number of parallel inspect instances on a docker ps call.

> On June 16, 2015, 10:02 p.m., Ben Mahler wrote: > > High level review, why don't we just have an abstraction for limiting the > > number of concurrent operations? > > > > You could keep this abstraction local to this file for now and consider > > pulling it out into libprocess. > > For examp

Re: Review Request 35538: [MESOS-1988] Remove unused drop(...) overload

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35538/#review88155 --- Ship it! Ship It! - Vinod Kone On June 16, 2015, 9:44 p.m., Anan

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/#review88151 --- Can you split this review into 1) adding support for a custom execut

Re: Review Request 35330: Capped number of parallel inspect instances on a docker ps call.

> On June 16, 2015, 10:02 p.m., Ben Mahler wrote: > > High level review, why don't we just have an abstraction for limiting the > > number of concurrent operations? > > > > You could keep this abstraction local to this file for now and consider > > pulling it out into libprocess. > > For examp

Re: Review Request 35536: Replace adhoc JSON conversion functions for ResourceStatistics with a protocol buffer to JSON converter.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35536/#review88147 --- src/slave/containerizer/isolators/network/port_mapping.cpp (line 70

Re: Review Request 35536: Replace adhoc JSON conversion functions for ResourceStatistics with a protocol buffer to JSON converter.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35536/#review88146 --- src/tests/port_mapping_tests.cpp (line 1587)

Re: Review Request 35536: Replace adhoc JSON conversion functions for ResourceStatistics with a protocol buffer to JSON converter.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35536/#review88145 --- Ship it! Great cleanup! Thanks Paul! src/slave/containerizer/isol

Re: Review Request 35257: Decode network statistics from helper

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35257/ --- (Updated June 16, 2015, 10:22 p.m.) Review request for mesos, Chi Zhang, Ian Do

Re: Review Request 35330: Capped number of parallel inspect instances on a docker ps call.

> On June 16, 2015, 10:02 p.m., Ben Mahler wrote: > > High level review, why don't we just have an abstraction for limiting the > > number of concurrent operations? > > > > You could keep this abstraction local to this file for now and consider > > pulling it out into libprocess. > > For examp

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/#review88140 --- This is pretty hacky. Can you elaborate why this is beneficial for t

Re: Review Request 35330: Capped number of parallel inspect instances on a docker ps call.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35330/#review88134 --- High level review, why don't we just have an abstraction for limitin

Re: Review Request 35229: Report per-container metrics for network bandwidth throttling to the slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35229/ --- (Updated June 16, 2015, 10 p.m.) Review request for mesos, Chi Zhang, Ian Downe

Re: Review Request 35538: [MESOS-1988] Remove unused drop(...) overload

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35538/#review88137 --- Ship it! Ship It! - Isabel Jimenez On June 16, 2015, 9:44 p.m.,

Review Request 35538: [MESOS-1988] Remove unused drop(...) overload

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35538/ --- Review request for mesos and Vinod Kone. Bugs: MESOS-1988 https://issues.ap

Re: Review Request 35508: Improve readability in post-review

> On June 16, 2015, 4:40 p.m., Marco Massenzio wrote: > > Beauty being in the eye of the beholder, I'm not entirely sure that I like > > the new version better (that is: assuming that the "new and improved" > > version is the bottom one?). > > > > Can you please make the description clearer by

Re: Review Request 35330: Capped number of parallel inspect instances on a docker ps call.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35330/#review88133 --- src/docker/docker.cpp (line 887)

Re: Review Request 35330: Capped number of parallel inspect instances on a docker ps call.

> On June 13, 2015, 5:58 a.m., Timothy Chen wrote: > > src/docker/docker.cpp, line 834 > > > > > > this too? doesn't fit on 80 lines > On June 13, 2015, 5:58 a.m., Timothy Chen wrote: > > src/docker/docker.cpp, line

Re: Review Request 34721: Added QoS kill executor correction test.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34721/#review88128 --- src/tests/mesos.hpp (line 205)

Re: Review Request 34720: Added kill executor correction to slave.

> On June 16, 2015, 9:22 p.m., Jie Yu wrote: > > LGTM. Some nits on logging. IN fact, can you consistently use `QoS correction KILL` (instead of QoS KILL correction) in the logging? - Jie --- This is an automatically generated e-mail.

Re: Review Request 34720: Added kill executor correction to slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34720/#review88124 --- Ship it! LGTM. Some nits on logging. src/slave/slave.cpp (line 41

Re: Review Request 34135: Add filesystem/ isolators for persistent volumes.

> On June 2, 2015, 2:45 p.m., Timothy Chen wrote: > > src/slave/containerizer/isolators/filesystem/linux.cpp, line 95 > > > > > > actually I'm wrong, I was reading the old style guide. The newest style > > guide we do

Re: Review Request 34719: Added REASON_EXECUTOR_PREEMPTED as status reason.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34719/#review88123 --- Ship it! include/mesos/mesos.proto (line 902)

Re: Review Request 34721: Added QoS kill executor correction test.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34721/ --- (Updated June 16, 2015, 1:43 p.m.) Review request for mesos, Bartek Plotka, Jie

Re: Review Request 34720: Added kill executor correction to slave.

> On June 12, 2015, 3:49 p.m., Jie Yu wrote: > > src/slave/slave.cpp, lines 4203-4210 > > > > > > Can you add a TODO here saying that we may want to check if > > containerId matches or not due to race (i.e., qos con

Re: Review Request 34720: Added kill executor correction to slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34720/ --- (Updated June 16, 2015, 1:42 p.m.) Review request for mesos, Bartek Plotka, Jie

Re: Review Request 34719: Added REASON_EXECUTOR_PREEMPTED as status reason.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34719/ --- (Updated June 16, 2015, 1:40 p.m.) Review request for mesos, Bartek Plotka, Jie

Re: Review Request 35536: Replace adhoc JSON conversion functions for ResourceStatistics with a protocol buffer to JSON converter.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35536/ --- (Updated June 16, 2015, 8:27 p.m.) Review request for mesos, Chi Zhang, Ian Dow

Review Request 35536: Replace adhoc JSON conversion functions for ResourceStatistics with a protocol buffer to JSON converter.

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

Re: Review Request 31444: Support chrooting in MesosContainerizer launch helper.

> On June 4, 2015, 11:32 a.m., Chi Zhang wrote: > > src/tests/launch_tests.cpp, line 91 > > > > > > for discussion: this requires these directories not existent in rootfs. > > should we specify requirements for the str

Re: Review Request 35509: Doxygen Style Guide Improvements.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35509/ --- (Updated June 16, 2015, 6:11 p.m.) Review request for mesos and Bernd Mathiske.

Re: Review Request 35508: Improve readability in post-review

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35508/#review88101 --- Ship it! It looks like this was probably the original intent (doesn

Re: Review Request 35510: Introduced General and Markdown Documentation Style Guides.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35510/ --- (Updated June 16, 2015, 6:10 p.m.) Review request for mesos and Bernd Mathiske.

Re: Review Request 35510: Introduced General and Markdown Documentation Style Guides.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35510/ --- (Updated June 16, 2015, 5:42 p.m.) Review request for mesos and Bernd Mathiske.

Re: Review Request 35473: Removed a few incorrect CHECKs in DRF sorter.

> On June 16, 2015, 5:09 p.m., Niklas Nielsen wrote: > > src/master/allocator/sorter/drf/sorter.cpp, line 177 > > > > > > This isn't just fixing the check's then but is introducing a fast path? > > Should we capture t

Re: Review Request 35473: Removed a few incorrect CHECKs in DRF sorter.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35473/#review88090 --- LGTM module Ben's comment and removing the (now obsolete) test comme

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30339/#review88088 --- Ship it! Ship It! - Niklas Nielsen On June 15, 2015, 3:26 p.m.,

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

> On June 16, 2015, 4:55 p.m., Niklas Nielsen wrote: > > src/hook/manager.cpp, lines 96-98 > > > > > > Won't some compilers potentially break with reaching end of non-void > > function? > > > > How about: > >

Re: Review Request 35508: Improve readability in post-review

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35508/ --- (Updated June 16, 2015, 6:58 p.m.) Review request for mesos, Ben Mahler and Vin

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30339/#review88085 --- src/hook/manager.cpp (lines 96 - 98)

Re: Review Request 35508: Improve readability in post-review

> On June 16, 2015, 6:40 p.m., Marco Massenzio wrote: > > Beauty being in the eye of the beholder, I'm not entirely sure that I like > > the new version better (that is: assuming that the "new and improved" > > version is the bottom one?). > > > > Can you please make the description clearer by

Re: Review Request 35508: Improve readability in post-review

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35508/#review88082 --- Beauty being in the eye of the beholder, I'm not entirely sure that

Re: Review Request 35510: Introduced General and Markdown Documentation Style Guides.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35510/ --- (Updated June 16, 2015, 2:03 p.m.) Review request for mesos and Bernd Mathiske.

Re: Review Request 35179: MESOS-1733 Variadic Path Join

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35179/#review88059 --- Ship it! Ship It! - Michael Park On June 15, 2015, 11:23 p.m., A

Review Request 35508: Improve readability in post-review

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35508/ --- Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos Descri

Re: Review Request 34703: Added stream manipulators for the Time object.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34703/ --- (Updated June 16, 2015, 2:24 p.m.) Review request for mesos, Bernd Mathiske, Mi

Re: Review Request 34703: Added stream manipulators for the Time object.

> On June 16, 2015, 10:29 a.m., Bernd Mathiske wrote: > > 3rdparty/libprocess/src/time.cpp, line 41 > > > > > > Why can't we use strfmt_l with a locale argument that matches the > > output we want? `strfmt_l` is avail

Re: Review Request 35494: Added format for template declaration in the styleguide.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35494/ --- (Updated June 16, 2015, 1:27 p.m.) Review request for mesos, Bernd Mathiske and

Review Request 35506: Excluding MarkDown files from the style hook

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35506/ --- Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-2873

Re: Review Request 35494: Added format for template declaration in the styleguide.

> On June 16, 2015, 11:21 a.m., Michael Park wrote: > > docs/mesos-c++-style-guide.md, line 64 > > > > > > (1) `s/template/`template`/` > > (2) `s/template /template ` > > (3) `s/template/template` Yikes, the f

Re: Review Request 35494: Added format for template declaration in the styleguide.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35494/#review88054 --- Ship it! docs/mesos-c++-style-guide.md (line 64)

Re: Review Request 35364: Consistent code examples in doxygen style.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35364/#review88045 --- Ship it! Ship It! - Bernd Mathiske On June 11, 2015, 1:39 p.m.,

Re: Review Request 34703: Added stream manipulators for the Time object.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34703/#review88038 --- 3rdparty/libprocess/src/tests/time_tests.cpp (line 42)

Re: Review Request 35494: Added format for template declaration in the styleguide.

> On June 16, 2015, 8:13 a.m., Joerg Schad wrote: > > Actually I would really like to understand why we do it this way. Most > > other code I know omits the space. Still, as this seems to be common > > practice... That question should get raised on the JIRA to make things transparent. - Till

Re: Review Request 35494: Added format for template declaration in the styleguide.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35494/ --- (Updated June 16, 2015, 10:17 a.m.) Review request for mesos, Bernd Mathiske an

Re: Review Request 35494: Added format for template declaration in the styleguide.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35494/#review88039 --- Ship it! Actually I would really like to understand why we do it th

Re: Review Request 30032: Added support for cache control in libprocess when dealing with static files.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30032/ --- (Updated June 16, 2015, 9:53 a.m.) Review request for mesos, Benjamin Hindman,

Review Request 35494: Added format for template declaration in the styleguide.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35494/ --- Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-2705