Re: Review Request 34353: Added right ammount of spacing between structs
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34353/#review84157 --- Patch looks great! Reviews applied: [34353] All tests passed. - Mesos ReviewBot On May 18, 2015, 2:14 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34353/ --- (Updated May 18, 2015, 2:14 p.m.) Review request for mesos and Till Toenshoff. Repository: mesos Description --- Added right ammount of spacing between structs Diffs - 3rdparty/libprocess/include/process/http.hpp 058fa02eeecdf31023db731734257a924d770079 Diff: https://reviews.apache.org/r/34353/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 30612: Added /master/frameworks/{framework}/tasks/{task} endpoint.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30612/
---
(Updated May 18, 2015, 2:56 p.m.)
Review request for mesos, Adam B, Joerg Schad, Marco Massenzio, Niklas Nielsen,
and Till Toenshoff.
Changes
---
Changes according to Adam's review (most of them)
Bugs: MESOS-2157
https://issues.apache.org/jira/browse/MESOS-2157
Repository: mesos
Description
---
Adds endpoints for paths /master/frameworks/{framework}/tasks/{task}.
Adds tests for the endpoints.
Works with [29883](https://reviews.apache.org/r/29883).
Builds on the abandoned patch 14286.
Diffs (updated)
-
src/master/http.cpp 7fa47a9fa2f5667631169876964dccf7fdcc172e
src/master/master.hpp da0a83510784f4f7dbd933e666ac12c04c413a62
src/master/master.cpp eaea79df2c693d15087d70b3c9b988e57c894f8e
src/tests/master_tests.cpp ba3858fb3026f2cc7af0523bd1a0c541eeafc34b
Diff: https://reviews.apache.org/r/30612/diff/
Testing
---
make check
Thanks,
Alexander Rojas
Re: Review Request 30612: Added /master/frameworks/{framework}/tasks/{task} endpoint.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30612/#review82935
---
This one may be entirely outdated - sry for that in advance
src/master/http.cpp
https://reviews.apache.org/r/30612/#comment133790
`const Framework* framework` please.
src/master/http.cpp
https://reviews.apache.org/r/30612/#comment133791
`const Task* task` please.
src/master/http.cpp
https://reviews.apache.org/r/30612/#comment133801
`const`?
src/master/http.cpp
https://reviews.apache.org/r/30612/#comment133802
s/there's/there is/
src/tests/master_tests.cpp
https://reviews.apache.org/r/30612/#comment133793
`const std::string`?
src/tests/master_tests.cpp
https://reviews.apache.org/r/30612/#comment133795
`const std::string`?
src/tests/master_tests.cpp
https://reviews.apache.org/r/30612/#comment133796
`const std::string`?
src/tests/master_tests.cpp
https://reviews.apache.org/r/30612/#comment133798
`const std::string`?
src/tests/master_tests.cpp
https://reviews.apache.org/r/30612/#comment133799
`const std::string`?
- Till Toenshoff
On May 18, 2015, 12:56 p.m., Alexander Rojas wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30612/
---
(Updated May 18, 2015, 12:56 p.m.)
Review request for mesos, Adam B, Joerg Schad, Marco Massenzio, Niklas
Nielsen, and Till Toenshoff.
Bugs: MESOS-2157
https://issues.apache.org/jira/browse/MESOS-2157
Repository: mesos
Description
---
Adds endpoints for paths /master/frameworks/{framework}/tasks/{task}.
Adds tests for the endpoints.
Works with [29883](https://reviews.apache.org/r/29883).
Builds on the abandoned patch 14286.
Diffs
-
src/master/http.cpp 7fa47a9fa2f5667631169876964dccf7fdcc172e
src/master/master.hpp da0a83510784f4f7dbd933e666ac12c04c413a62
src/master/master.cpp eaea79df2c693d15087d70b3c9b988e57c894f8e
src/tests/master_tests.cpp ba3858fb3026f2cc7af0523bd1a0c541eeafc34b
Diff: https://reviews.apache.org/r/30612/diff/
Testing
---
make check
Thanks,
Alexander Rojas
Re: Review Request 30612: Added /master/frameworks/{framework}/tasks/{task} endpoint.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30612/#review84146
---
Patch looks great!
Reviews applied: [32198, 32163, 30612]
All tests passed.
- Mesos ReviewBot
On May 18, 2015, 12:56 p.m., Alexander Rojas wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30612/
---
(Updated May 18, 2015, 12:56 p.m.)
Review request for mesos, Adam B, Joerg Schad, Marco Massenzio, Niklas
Nielsen, and Till Toenshoff.
Bugs: MESOS-2157
https://issues.apache.org/jira/browse/MESOS-2157
Repository: mesos
Description
---
Adds endpoints for paths /master/frameworks/{framework}/tasks/{task}.
Adds tests for the endpoints.
Works with [29883](https://reviews.apache.org/r/29883).
Builds on the abandoned patch 14286.
Diffs
-
src/master/http.cpp 7fa47a9fa2f5667631169876964dccf7fdcc172e
src/master/master.hpp da0a83510784f4f7dbd933e666ac12c04c413a62
src/master/master.cpp eaea79df2c693d15087d70b3c9b988e57c894f8e
src/tests/master_tests.cpp ba3858fb3026f2cc7af0523bd1a0c541eeafc34b
Diff: https://reviews.apache.org/r/30612/diff/
Testing
---
make check
Thanks,
Alexander Rojas
Re: Review Request 34299: Changed to use a push model for resource estimator.
On May 16, 2015, 1:03 a.m., Niklas Nielsen wrote: src/slave/slave.cpp, line 4080 https://reviews.apache.org/r/34299/diff/1/?file=961836#file961836line4080 This is being executed in the context of the estimator thread? Is this safe? This will be executed in the slave's thread because we registered a `defer(self(), Slave::receiveOversubscribedResources, lambda::_1)` with the resource estimator. - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34299/#review84011 --- On May 15, 2015, 11:38 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34299/ --- (Updated May 15, 2015, 11:38 p.m.) Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2735 https://issues.apache.org/jira/browse/MESOS-2735 Repository: mesos Description --- Changed to use a push model for resource estimator. See ticket for details. This patch also makes the interval configurable through slave flags. Diffs - include/mesos/slave/resource_estimator.hpp 363961541c9b49763e8cbafe982421d45516db0d src/slave/constants.hpp df02043fcd1b5bffc13aa0683ad9b64372188300 src/slave/constants.cpp 07f699a9e5c1b56d6e0435077823ad0f863ad63c src/slave/flags.hpp ca7cc131f70d58982262fd29986d46877d563035 src/slave/flags.cpp f35c76a342d03700710bb91bf4c523cc99769228 src/slave/resource_estimator.hpp bdf62bab9b632aa16801e47978962a7ab24b53b4 src/slave/resource_estimator.cpp 13d706c30d36c9f304acf288f02d6c9558bac781 src/slave/slave.hpp b62ed7b207fa337027baffa1c75139641a41dad6 src/slave/slave.cpp 132f83e05dd4113c8f6d20a6a64b12fb8f05dd36 src/tests/mesos.hpp df8cd200a6970209588b63a1d58709b1f913a305 src/tests/oversubscription_tests.cpp 64c2ede1f50546f6de1ecb2972036b51fe17e358 Diff: https://reviews.apache.org/r/34299/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 34193: Refactored common functionality into BaseFlags
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34193/ --- (Updated May 18, 2015, 5:26 p.m.) Review request for mesos, Benjamin Hindman and Joris Van Remoortere. Repository: mesos Description --- Jira: MESOS-2711 Every program that uses stout's `BaseFlags` ends up re-implementing the `printUsage()` function, and adding a `bool help` (and associated --help flag); this functionality has now been refactored in the base class and is available everywhere. This change attempts to be backward-compatible, so it does not alter the behavior of the program when --help is invoked (by, eg, automatically printing usage and exiting) but leaves up to the caller to check for `flags.help` and then decide what action to take. There is now a default behavior for the leader (Usage: prog name [options]) but the client API allows to modify that too. Note - anywhere I found the use of the `--help` flag the behavior was the same: print usage and exit (see also https://reviews.apache.org/r/34195). Diffs (updated) - 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp fb383b463a99924483634eebf22bf34de318f920 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 00281195b53d2597bdb46e3fe6cd9d46a5e9b1f1 Diff: https://reviews.apache.org/r/34193/diff/ Testing --- make check **NOTE** this change by itself breaks the build, because the --help is redefined everywhere (16 places, as of last count): CL 34195 fixes that and makes all build/tests pass. Thanks, Marco Massenzio
Re: Review Request 34362: Include ExecutorInfos in master/state.json
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34362/ --- (Updated May 18, 2015, 5:41 p.m.) Review request for mesos and Adam B. Bugs: MESOS-2743 https://issues.apache.org/jira/browse/MESOS-2743 Repository: mesos Description --- Include ExecutorInfos in master/state.json Diffs (updated) - src/common/http.hpp afce7fea334c7bfa57efc48c413bf906a2ebde32 src/common/http.cpp 2ac7fba7a3aac913540f1b09768777393b79284a src/master/http.cpp 7fa47a9fa2f5667631169876964dccf7fdcc172e src/slave/http.cpp b5e77b09435e50db5231a2b32faf76dab91dae54 Diff: https://reviews.apache.org/r/34362/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 34299: Changed to use a push model for resource estimator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34299/ --- (Updated May 18, 2015, 5:58 p.m.) Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone. Changes --- Vinod's comments. Bugs: MESOS-2735 https://issues.apache.org/jira/browse/MESOS-2735 Repository: mesos Description --- Changed to use a push model for resource estimator. See ticket for details. This patch also makes the interval configurable through slave flags. Diffs (updated) - include/mesos/slave/resource_estimator.hpp 363961541c9b49763e8cbafe982421d45516db0d src/messages/messages.proto 19e244453577dc7147352ee94d99107a8a0a1d68 src/slave/constants.hpp df02043fcd1b5bffc13aa0683ad9b64372188300 src/slave/constants.cpp 07f699a9e5c1b56d6e0435077823ad0f863ad63c src/slave/flags.hpp ca7cc131f70d58982262fd29986d46877d563035 src/slave/flags.cpp da30973ce31377532dc6ee2f4bd66e4325f9d381 src/slave/resource_estimator.hpp bdf62bab9b632aa16801e47978962a7ab24b53b4 src/slave/resource_estimator.cpp 13d706c30d36c9f304acf288f02d6c9558bac781 src/slave/slave.hpp b62ed7b207fa337027baffa1c75139641a41dad6 src/slave/slave.cpp 132f83e05dd4113c8f6d20a6a64b12fb8f05dd36 src/tests/mesos.hpp df8cd200a6970209588b63a1d58709b1f913a305 src/tests/oversubscription_tests.cpp 64c2ede1f50546f6de1ecb2972036b51fe17e358 Diff: https://reviews.apache.org/r/34299/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 34299: Changed to use a push model for resource estimator.
On May 16, 2015, 12:13 a.m., Vinod Kone wrote: include/mesos/slave/resource_estimator.hpp, line 59 https://reviews.apache.org/r/34299/diff/1/?file=961828#file961828line59 s/oversubscribed/oversubscribable/ ? or s/oversubscribed/oversubscribe/ because these are not resources that are already oversubscribed? Used oversubscribe. - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34299/#review84003 --- On May 15, 2015, 11:38 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34299/ --- (Updated May 15, 2015, 11:38 p.m.) Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2735 https://issues.apache.org/jira/browse/MESOS-2735 Repository: mesos Description --- Changed to use a push model for resource estimator. See ticket for details. This patch also makes the interval configurable through slave flags. Diffs - include/mesos/slave/resource_estimator.hpp 363961541c9b49763e8cbafe982421d45516db0d src/slave/constants.hpp df02043fcd1b5bffc13aa0683ad9b64372188300 src/slave/constants.cpp 07f699a9e5c1b56d6e0435077823ad0f863ad63c src/slave/flags.hpp ca7cc131f70d58982262fd29986d46877d563035 src/slave/flags.cpp f35c76a342d03700710bb91bf4c523cc99769228 src/slave/resource_estimator.hpp bdf62bab9b632aa16801e47978962a7ab24b53b4 src/slave/resource_estimator.cpp 13d706c30d36c9f304acf288f02d6c9558bac781 src/slave/slave.hpp b62ed7b207fa337027baffa1c75139641a41dad6 src/slave/slave.cpp 132f83e05dd4113c8f6d20a6a64b12fb8f05dd36 src/tests/mesos.hpp df8cd200a6970209588b63a1d58709b1f913a305 src/tests/oversubscription_tests.cpp 64c2ede1f50546f6de1ecb2972036b51fe17e358 Diff: https://reviews.apache.org/r/34299/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 34310: Use IDLE scheduling for revocable CPU in cgroups isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34310/ --- (Updated May 18, 2015, 10:33 a.m.) Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2652 https://issues.apache.org/jira/browse/MESOS-2652 Repository: mesos Description --- Use IDLE scheduling for revocable CPU in cgroups isolator. Diffs - src/slave/containerizer/isolators/cgroups/cpushare.hpp ff4a9dbdb1b655e71bf87dcee8fe62433d396f52 src/slave/containerizer/isolators/cgroups/cpushare.cpp 6a5b2b5c6e2844fe1a10815956569194b6f56681 Diff: https://reviews.apache.org/r/34310/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 34195: Refactoring to use BaseFlags common functionality
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34195/
---
(Updated May 18, 2015, 5:28 p.m.)
Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
Bugs: MESOS-2711
https://issues.apache.org/jira/browse/MESOS-2711
Repository: mesos
Description
---
Jira: MESOS-2711
All the main() methods have been refactored to use the
definition of BaseFlags::help flag and BaseFlags::printUsage().
This CL also tries to bring some uniformity to the
use of exit codes: if this is deemed to be worth
making it uniform, we can come up with common
rules and extend the changes here to be compliant.
This touches a lot of files, but keep scrolling, and you will see a pattern
emerge ;-)
TODO(marco): we should also abstract away the error checking
Diffs (updated)
-
src/cli/execute.cpp dbd19e67f56a150f54180ad13e6402842eb68e17
src/cli/resolve.cpp a99b6094dffc9f7aa44bcf63ad40121e1abb120b
src/examples/load_generator_framework.cpp
be1a3bf5f16bd811cb4039c8f15478183712a426
src/examples/persistent_volume_framework.cpp
8a893fcfa3d5d988a88fdeaf0bfc08e0a49b7a65
src/health-check/main.cpp a4ce742ab8deff1ebd99359112670493fdaeeac3
src/launcher/executor.cpp de6f1b104a765a8e53934154e78872b03695b24c
src/local/main.cpp a641b9e83862743890597a2981a9419517e7c589
src/log/tool/benchmark.cpp 01e55115f35d155efbea190b5308b294ba76e7cb
src/log/tool/initialize.cpp ccda7fb1c0f7113f865ec61adee76b2ea6180442
src/log/tool/read.cpp d14138502f5bc9a725deb83da505765865da017f
src/log/tool/replica.cpp 3985fc7df4f7153ae623589fbdd769ccbae57125
src/master/main.cpp d5666bc8ee8d7a0f0b8685f76d65dd1f9ac2a280
src/slave/containerizer/isolators/network/port_mapping.hpp
c72fb47f60f40cda8d84a10497b9133f83cf018e
src/slave/containerizer/isolators/network/port_mapping.cpp
a4abaff30bb4646b1b1edfdbbc243c9e3f6851df
src/slave/main.cpp f762f5b06be74c391cbc336b2da28f8358952ba4
src/tests/main.cpp e3fff5d60c0468c0d258f2bb301efc1309c071b0
src/usage/main.cpp 97f55e938dc7678f8331970d8953d09218f70902
Diff: https://reviews.apache.org/r/34195/diff/
Testing
---
make check
**NOTE** this fixes completely the chained changes from 3419{3,4} and makes all
the tests pass.
Thanks,
Marco Massenzio
Re: Review Request 34309: Support manipulating scheduler policy on Linux.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34309/ --- (Updated May 18, 2015, 10:33 a.m.) Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2652 https://issues.apache.org/jira/browse/MESOS-2652 Repository: mesos Description --- Support manipulating scheduler policy on Linux. Diffs - src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 src/linux/sched.hpp PRE-CREATION src/tests/sched_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/34309/diff/ Testing --- Added test. Thanks, Ian Downes
Re: Review Request 30032: Added support for cache control in libprocess when dealing with static files.
On March 26, 2015, 5:59 p.m., Benjamin Hindman wrote: 3rdparty/libprocess/src/process.cpp, line 2854 https://reviews.apache.org/r/30032/diff/4/?file=834184#file834184line2854 ResultTime time = Path(response.path).mtime(); This cannot be implemented in terms on `Time`, since `Path` is part of stout and `Time` belongs to libprocess. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30032/#review69827 --- On April 20, 2015, 1:58 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30032/ --- (Updated April 20, 2015, 1:58 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Joerg Schad, Michael Park, and Till Toenshoff. Bugs: mesos-708 https://issues.apache.org/jira/browse/mesos-708 Repository: mesos Description --- When serving a static file, libprocess returns the header `Last-Modified` which is used by browsers to control Cache. When a http request arrives containing the header `If-Modified-Since`, a response `304 Not Modified` is returned if the date in the request and the modification time (as returned by doing `stat` in the file) coincide. Unit tests added. Diffs - 3rdparty/libprocess/include/process/http.hpp 07825b2b7195fe7fe752e8fda65b7f0a8b8b1f38 3rdparty/libprocess/src/process.cpp 97ac09fd10b767bc46387abc3657d005a00830c8 3rdparty/libprocess/src/tests/process_tests.cpp eb38edce2c483ba7f963a826893a15a075238618 Diff: https://reviews.apache.org/r/30032/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 34306: Added 'revocable_offers' field to FrameworkInfo.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34306/#review84161 --- include/mesos/mesos.proto https://reviews.apache.org/r/34306/#comment135282 Should we make this a general 'Capability' message, such that we can announce that we can deal with revocable offers, optimistic offers, etc? - Niklas Nielsen On May 15, 2015, 5:24 p.m., Vinod Kone wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34306/ --- (Updated May 15, 2015, 5:24 p.m.) Review request for mesos, Jie Yu and Niklas Nielsen. Bugs: MESOS-2654 https://issues.apache.org/jira/browse/MESOS-2654 Repository: mesos Description --- Currently setting this field is a no-op. Just wanted to send it out for feedback, esp whether a 'bool' type is enough. Diffs - include/mesos/mesos.proto 9cc5782256156ed59fd4640091413b76480d939f Diff: https://reviews.apache.org/r/34306/diff/ Testing --- Thanks, Vinod Kone
Review Request 34361: converted hard-coded strings to consts
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34361/ --- Review request for mesos. Bugs: MESOS-2637 https://issues.apache.org/jira/browse/MESOS-2637 Repository: mesos Description --- converted hard-coded strings to consts Diffs - src/examples/test_hook_module.cpp b25830ab6475f997422cfd2f60cc9a79e1acadfe src/tests/hook_tests.cpp a65c0ab7c41ec3b7964f9d572381fa3e61746dc3 src/tests/master_tests.cpp ba3858fb3026f2cc7af0523bd1a0c541eeafc34b src/tests/slave_tests.cpp acae49731ff17103b529cdf828a63d9d55668549 Diff: https://reviews.apache.org/r/34361/diff/ Testing --- Thanks, Colin Williams
Review Request 34362: Include ExecutorInfos in master/state.json
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34362/ --- Review request for mesos and Adam B. Bugs: MESOS-2743 https://issues.apache.org/jira/browse/MESOS-2743 Repository: mesos Description --- Include ExecutorInfos in master/state.json Diffs - src/master/http.cpp 7fa47a9fa2f5667631169876964dccf7fdcc172e Diff: https://reviews.apache.org/r/34362/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 34299: Changed to use a push model for resource estimator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34299/#review84187 --- There are still places where it says oversubscribed. I pointed to some here. Please do a sweep to catch everything. src/slave/flags.hpp https://reviews.apache.org/r/34299/#comment135313 s/oversubscribed/oversubscribe/ ? src/slave/flags.cpp https://reviews.apache.org/r/34299/#comment135314 s/oversubscribed/oversubscribe/ ? src/slave/slave.cpp https://reviews.apache.org/r/34299/#comment135319 Looking at how we did status update forwarding, maybe s/send/forward/ ? sorry, didn't catch this earlier. src/slave/slave.cpp https://reviews.apache.org/r/34299/#comment135320 s/Sending/Forwarding/ s/oversubscribed/oversubscribable/ src/tests/oversubscription_tests.cpp https://reviews.apache.org/r/34299/#comment135321 s/oversubscribed/oversubscribable/ src/messages/messages.proto https://reviews.apache.org/r/34299/#comment135312 s/Oversubscribed/Oversubscribe/ ? - Vinod Kone On May 18, 2015, 5:58 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34299/ --- (Updated May 18, 2015, 5:58 p.m.) Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2735 https://issues.apache.org/jira/browse/MESOS-2735 Repository: mesos Description --- Changed to use a push model for resource estimator. See ticket for details. This patch also makes the interval configurable through slave flags. Diffs - include/mesos/slave/resource_estimator.hpp 363961541c9b49763e8cbafe982421d45516db0d src/messages/messages.proto 19e244453577dc7147352ee94d99107a8a0a1d68 src/slave/constants.hpp df02043fcd1b5bffc13aa0683ad9b64372188300 src/slave/constants.cpp 07f699a9e5c1b56d6e0435077823ad0f863ad63c src/slave/flags.hpp ca7cc131f70d58982262fd29986d46877d563035 src/slave/flags.cpp da30973ce31377532dc6ee2f4bd66e4325f9d381 src/slave/resource_estimator.hpp bdf62bab9b632aa16801e47978962a7ab24b53b4 src/slave/resource_estimator.cpp 13d706c30d36c9f304acf288f02d6c9558bac781 src/slave/slave.hpp b62ed7b207fa337027baffa1c75139641a41dad6 src/slave/slave.cpp 132f83e05dd4113c8f6d20a6a64b12fb8f05dd36 src/tests/mesos.hpp df8cd200a6970209588b63a1d58709b1f913a305 src/tests/oversubscription_tests.cpp 64c2ede1f50546f6de1ecb2972036b51fe17e358 Diff: https://reviews.apache.org/r/34299/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 34309: Support manipulating scheduler policy on Linux.
On May 16, 2015, 12:31 p.m., Joris Van Remoortere wrote: src/tests/sched_tests.cpp, line 40 https://reviews.apache.org/r/34309/diff/1/?file=961961#file961961line40 Could you elaborate on this comment and explain what you are testing in the child? I don't think it's clear. Should we also test the inheritance? (i.e. fork while in the IDLE policy, and then verify in the child?) It's testing that we can set the policy for a child/different process, not that a child process can set its own policy. I amended the comment to make that clearer. Regarding inheritance, this is specified for sched_setscheduler() and I'm not sure I want to get into the business of testing every system call ;-) ? Child processes inherit the scheduling policy and parameters across a fork(2). The scheduling policy and parameters are preserved across execve(2). On May 16, 2015, 12:31 p.m., Joris Van Remoortere wrote: src/tests/sched_tests.cpp, line 30 https://reviews.apache.org/r/34309/diff/1/?file=961961#file961961line30 Do you want to add a TODO for some tests that: 1) Actually test that the priority / interruption behavior is as expected 2) Shows people how to use this / explains how it works I think 1) and 2) can be done in the same test with nice comments :-) Added a TODO. Testing priority is easy. Do you have suggestions on how to test preemption behavior? On May 16, 2015, 12:31 p.m., Joris Van Remoortere wrote: src/linux/sched.hpp, line 53 https://reviews.apache.org/r/34309/diff/1/?file=961960#file961960line53 Should we add the pid for which this failed to the error message? I think the caller should be responsible for this? On May 16, 2015, 12:31 p.m., Joris Van Remoortere wrote: src/linux/sched.hpp, line 73 https://reviews.apache.org/r/34309/diff/1/?file=961960#file961960line73 Should we add the pid for which this failed to the error message? I think the caller should be responsible for this? - Ian --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34309/#review84038 --- On May 18, 2015, 10:33 a.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34309/ --- (Updated May 18, 2015, 10:33 a.m.) Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2652 https://issues.apache.org/jira/browse/MESOS-2652 Repository: mesos Description --- Support manipulating scheduler policy on Linux. Diffs - src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 src/linux/sched.hpp PRE-CREATION src/tests/sched_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/34309/diff/ Testing --- Added test. Thanks, Ian Downes
Re: Review Request 34310: Use IDLE scheduling for revocable CPU in cgroups isolator.
On May 16, 2015, 1:03 p.m., Joris Van Remoortere wrote: Hi Ian, I'm wondering about the `Note` regarding only setting the scheduling policy to IDLE if the initial resources are revocable. I think this exposes many scenarios where the isolator will seem `enabled` to the operator, but won't actually be isolating correctly. I don't think it is uncommon to launch an executor with non-revocable resources, and then later launch BE tasks with revocable resources. This pattern would always lead to the isolator being in-effective right? I am not super familiar with the rules around the isolators, but why can we not adjust the policy within the update() call as opposed to in the isolate() function? This is not a limitation of the isolator interface but a complexity of setting the scheduling policy because it's task rather than cgroup based. We'd need to ensure we set it on all tasks and forking children. One approach could be to briefly freeze the cgroup and set the scheduling policy but I'm concerned that if the slave dies during the freeze/set/unfreeze operation then we've frozen a running container. Otherwise, we need some hackery as used in os:killtree() to ensure we get every task. We need to do it in isolate() regardless because that's the initial forked child which will later exec the executor and we may never get a subsequent update(). I'll look at adding it also to the update() call to capture updates to the resources after the executor has launched. I presume we also want to support the reverse update from having some revocable to having no revocable CPU, i.e., SCHED_IDLE - SCHED_OTHER? - Ian --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34310/#review84039 --- On May 18, 2015, 10:33 a.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34310/ --- (Updated May 18, 2015, 10:33 a.m.) Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2652 https://issues.apache.org/jira/browse/MESOS-2652 Repository: mesos Description --- Use IDLE scheduling for revocable CPU in cgroups isolator. Diffs - src/slave/containerizer/isolators/cgroups/cpushare.hpp ff4a9dbdb1b655e71bf87dcee8fe62433d396f52 src/slave/containerizer/isolators/cgroups/cpushare.cpp 6a5b2b5c6e2844fe1a10815956569194b6f56681 Diff: https://reviews.apache.org/r/34310/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 34310: Use IDLE scheduling for revocable CPU in cgroups isolator.
On May 15, 2015, 9:46 p.m., Timothy Chen wrote: src/slave/containerizer/isolators/cgroups/cpushare.cpp, line 345 https://reviews.apache.org/r/34310/diff/1/?file=961963#file961963line345 What if the same set of resources contains both revocable and non-revocable resources? Hmm, I presumed that any amount of revocable CPU means the container is treated as revocable. Thoughts? - Ian --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34310/#review84024 --- On May 18, 2015, 10:33 a.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34310/ --- (Updated May 18, 2015, 10:33 a.m.) Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2652 https://issues.apache.org/jira/browse/MESOS-2652 Repository: mesos Description --- Use IDLE scheduling for revocable CPU in cgroups isolator. Diffs - src/slave/containerizer/isolators/cgroups/cpushare.hpp ff4a9dbdb1b655e71bf87dcee8fe62433d396f52 src/slave/containerizer/isolators/cgroups/cpushare.cpp 6a5b2b5c6e2844fe1a10815956569194b6f56681 Diff: https://reviews.apache.org/r/34310/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 33754: Update pthread autoconf macros for libprocess.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33754/#review84184 --- 3rdparty/libprocess/configure.ac https://reviews.apache.org/r/33754/#comment135310 Maybe we could search the whole project and replace acx_pthread.m4 - ax_pthread.m4 - haosdent huang On May 1, 2015, 4:02 p.m., James Peach wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33754/ --- (Updated May 1, 2015, 4:02 p.m.) Review request for mesos. Bugs: MESOS-2659 https://issues.apache.org/jira/browse/MESOS-2659 Repository: mesos Description --- Replace acx_pthread with ax_pthread. Update configure.ac to use the new autoconf macros. Diffs - 3rdparty/libprocess/configure.ac c5106cd09901781ca77d8c02c73919553a085876 3rdparty/libprocess/m4/acx_pthread.m4 2cf20de144a11be2aa603b04ea511244191037b7 3rdparty/libprocess/m4/ax_pthread.m4 PRE-CREATION Diff: https://reviews.apache.org/r/33754/diff/ Testing --- Bootstrap and verify there are no autotools warnings. Tested on CentOS 7 and Mac OS X 10.10.3. Thanks, James Peach
Re: Review Request 34195: Refactoring to use BaseFlags common functionality
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34195/#review84186
---
Bad patch!
Reviews applied: [34193, 34193]
Failed command: ./support/apply-review.sh -n -r 34193
Error:
2015-05-18 18:35:17 URL:https://reviews.apache.org/r/34193/diff/raw/
[5544/5544] - 34193.patch [1]
error: patch failed:
3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp:42
error: 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp: patch
does not apply
error: patch failed:
3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp:483
error: 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp: patch does not
apply
Failed to apply patch
- Mesos ReviewBot
On May 18, 2015, 5:28 p.m., Marco Massenzio wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34195/
---
(Updated May 18, 2015, 5:28 p.m.)
Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
Bugs: MESOS-2711
https://issues.apache.org/jira/browse/MESOS-2711
Repository: mesos
Description
---
Jira: MESOS-2711
All the main() methods have been refactored to use the
definition of BaseFlags::help flag and BaseFlags::printUsage().
This CL also tries to bring some uniformity to the
use of exit codes: if this is deemed to be worth
making it uniform, we can come up with common
rules and extend the changes here to be compliant.
This touches a lot of files, but keep scrolling, and you will see a pattern
emerge ;-)
TODO(marco): we should also abstract away the error checking
Diffs
-
src/cli/execute.cpp dbd19e67f56a150f54180ad13e6402842eb68e17
src/cli/resolve.cpp a99b6094dffc9f7aa44bcf63ad40121e1abb120b
src/examples/load_generator_framework.cpp
be1a3bf5f16bd811cb4039c8f15478183712a426
src/examples/persistent_volume_framework.cpp
8a893fcfa3d5d988a88fdeaf0bfc08e0a49b7a65
src/health-check/main.cpp a4ce742ab8deff1ebd99359112670493fdaeeac3
src/launcher/executor.cpp de6f1b104a765a8e53934154e78872b03695b24c
src/local/main.cpp a641b9e83862743890597a2981a9419517e7c589
src/log/tool/benchmark.cpp 01e55115f35d155efbea190b5308b294ba76e7cb
src/log/tool/initialize.cpp ccda7fb1c0f7113f865ec61adee76b2ea6180442
src/log/tool/read.cpp d14138502f5bc9a725deb83da505765865da017f
src/log/tool/replica.cpp 3985fc7df4f7153ae623589fbdd769ccbae57125
src/master/main.cpp d5666bc8ee8d7a0f0b8685f76d65dd1f9ac2a280
src/slave/containerizer/isolators/network/port_mapping.hpp
c72fb47f60f40cda8d84a10497b9133f83cf018e
src/slave/containerizer/isolators/network/port_mapping.cpp
a4abaff30bb4646b1b1edfdbbc243c9e3f6851df
src/slave/main.cpp f762f5b06be74c391cbc336b2da28f8358952ba4
src/tests/main.cpp e3fff5d60c0468c0d258f2bb301efc1309c071b0
src/usage/main.cpp 97f55e938dc7678f8331970d8953d09218f70902
Diff: https://reviews.apache.org/r/34195/diff/
Testing
---
make check
**NOTE** this fixes completely the chained changes from 3419{3,4} and makes
all the tests pass.
Thanks,
Marco Massenzio
Re: Review Request 34195: Refactoring to use BaseFlags common functionality
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34195/ --- (Updated May 18, 2015, 7:01 p.m.) Review request for Benjamin Hindman and Joris Van Remoortere. Bugs: MESOS-2711 https://issues.apache.org/jira/browse/MESOS-2711 Repository: mesos Description --- Jira: MESOS-2711 All the main() methods have been refactored to use the definition of BaseFlags::help flag and BaseFlags::printUsage(). This CL also tries to bring some uniformity to the use of exit codes: if this is deemed to be worth making it uniform, we can come up with common rules and extend the changes here to be compliant. This touches a lot of files, but keep scrolling, and you will see a pattern emerge ;-) TODO(marco): we should also abstract away the error checking Diffs - src/cli/execute.cpp dbd19e67f56a150f54180ad13e6402842eb68e17 src/cli/resolve.cpp a99b6094dffc9f7aa44bcf63ad40121e1abb120b src/examples/load_generator_framework.cpp be1a3bf5f16bd811cb4039c8f15478183712a426 src/examples/persistent_volume_framework.cpp 8a893fcfa3d5d988a88fdeaf0bfc08e0a49b7a65 src/health-check/main.cpp a4ce742ab8deff1ebd99359112670493fdaeeac3 src/launcher/executor.cpp de6f1b104a765a8e53934154e78872b03695b24c src/local/main.cpp a641b9e83862743890597a2981a9419517e7c589 src/log/tool/benchmark.cpp 01e55115f35d155efbea190b5308b294ba76e7cb src/log/tool/initialize.cpp ccda7fb1c0f7113f865ec61adee76b2ea6180442 src/log/tool/read.cpp d14138502f5bc9a725deb83da505765865da017f src/log/tool/replica.cpp 3985fc7df4f7153ae623589fbdd769ccbae57125 src/master/main.cpp d5666bc8ee8d7a0f0b8685f76d65dd1f9ac2a280 src/slave/containerizer/isolators/network/port_mapping.hpp c72fb47f60f40cda8d84a10497b9133f83cf018e src/slave/containerizer/isolators/network/port_mapping.cpp a4abaff30bb4646b1b1edfdbbc243c9e3f6851df src/slave/main.cpp f762f5b06be74c391cbc336b2da28f8358952ba4 src/tests/main.cpp e3fff5d60c0468c0d258f2bb301efc1309c071b0 src/usage/main.cpp 97f55e938dc7678f8331970d8953d09218f70902 Diff: https://reviews.apache.org/r/34195/diff/ Testing (updated) --- make check **NOTE** this fixes completely the chained changes from 34193 and makes all the tests pass. Thanks, Marco Massenzio
Re: Review Request 34362: Include ExecutorInfos in master/state.json
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34362/#review84198 --- Patch looks great! Reviews applied: [34362] All tests passed. - Mesos ReviewBot On May 18, 2015, 5:41 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34362/ --- (Updated May 18, 2015, 5:41 p.m.) Review request for mesos and Adam B. Bugs: MESOS-2743 https://issues.apache.org/jira/browse/MESOS-2743 Repository: mesos Description --- Include ExecutorInfos in master/state.json Diffs - src/common/http.hpp afce7fea334c7bfa57efc48c413bf906a2ebde32 src/common/http.cpp 2ac7fba7a3aac913540f1b09768777393b79284a src/master/http.cpp 7fa47a9fa2f5667631169876964dccf7fdcc172e src/slave/http.cpp b5e77b09435e50db5231a2b32faf76dab91dae54 Diff: https://reviews.apache.org/r/34362/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 34299: Changed to use a push model for resource estimator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34299/ --- (Updated May 18, 2015, 7:19 p.m.) Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone. Changes --- Vinod's comments. Bugs: MESOS-2735 https://issues.apache.org/jira/browse/MESOS-2735 Repository: mesos Description --- Changed to use a push model for resource estimator. See ticket for details. This patch also makes the interval configurable through slave flags. Diffs (updated) - include/mesos/slave/resource_estimator.hpp 363961541c9b49763e8cbafe982421d45516db0d src/messages/messages.proto 19e244453577dc7147352ee94d99107a8a0a1d68 src/slave/constants.hpp df02043fcd1b5bffc13aa0683ad9b64372188300 src/slave/constants.cpp 07f699a9e5c1b56d6e0435077823ad0f863ad63c src/slave/flags.hpp ca7cc131f70d58982262fd29986d46877d563035 src/slave/flags.cpp da30973ce31377532dc6ee2f4bd66e4325f9d381 src/slave/resource_estimator.hpp bdf62bab9b632aa16801e47978962a7ab24b53b4 src/slave/resource_estimator.cpp 13d706c30d36c9f304acf288f02d6c9558bac781 src/slave/slave.hpp b62ed7b207fa337027baffa1c75139641a41dad6 src/slave/slave.cpp 132f83e05dd4113c8f6d20a6a64b12fb8f05dd36 src/tests/mesos.hpp df8cd200a6970209588b63a1d58709b1f913a305 src/tests/oversubscription_tests.cpp 64c2ede1f50546f6de1ecb2972036b51fe17e358 Diff: https://reviews.apache.org/r/34299/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 30032: Added support for cache control in libprocess when dealing with static files.
On March 26, 2015, 4:59 p.m., Benjamin Hindman wrote: 3rdparty/libprocess/src/process.cpp, line 2854 https://reviews.apache.org/r/30032/diff/4/?file=834184#file834184line2854 ResultTime time = Path(response.path).mtime(); Alexander Rojas wrote: This cannot be implemented in terms on `Time`, since `Path` is part of stout and `Time` belongs to libprocess. Seems `Time` actually should be a stout class. - Till --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30032/#review69827 --- On April 20, 2015, 11:58 a.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30032/ --- (Updated April 20, 2015, 11:58 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Joerg Schad, Michael Park, and Till Toenshoff. Bugs: mesos-708 https://issues.apache.org/jira/browse/mesos-708 Repository: mesos Description --- When serving a static file, libprocess returns the header `Last-Modified` which is used by browsers to control Cache. When a http request arrives containing the header `If-Modified-Since`, a response `304 Not Modified` is returned if the date in the request and the modification time (as returned by doing `stat` in the file) coincide. Unit tests added. Diffs - 3rdparty/libprocess/include/process/http.hpp 07825b2b7195fe7fe752e8fda65b7f0a8b8b1f38 3rdparty/libprocess/src/process.cpp 97ac09fd10b767bc46387abc3657d005a00830c8 3rdparty/libprocess/src/tests/process_tests.cpp eb38edce2c483ba7f963a826893a15a075238618 Diff: https://reviews.apache.org/r/30032/diff/ Testing --- make check Thanks, Alexander Rojas
Review Request 34371: Add framework's pid to json summary of framework.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34371/ --- Review request for mesos and Benjamin Hindman. Bugs: MESOS-2746 https://issues.apache.org/jira/browse/MESOS-2746 Repository: mesos Description --- Following pattern of slave exposing its pid. Diffs - src/master/http.cpp 7fa47a9fa2f5667631169876964dccf7fdcc172e Diff: https://reviews.apache.org/r/34371/diff/ Testing --- make check. Thanks, Joris Van Remoortere
Review Request 34321: Merge class Handle which is duplicated between filter/handle and queueing/handle.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34321/ --- Review request for mesos. Bugs: mesos-2665 https://issues.apache.org/jira/browse/mesos-2665 Repository: mesos Description --- Merge class Handle which is duplicated between filter/handle and queueing/handle. Diffs - src/linux/routing/filter/basic.hpp 99b0b058633403e334e7230dfa7d705c99b3cb10 src/linux/routing/filter/basic.cpp 755be7de5c43c2f3b336af6cb47af9c97a21218c src/linux/routing/filter/filter.hpp 024582cf8274da2e5b4ebd00fcf83d6930e2 src/linux/routing/filter/handle.hpp 190177441bc95cf77690dbdeca189a816c6e2324 src/linux/routing/filter/icmp.hpp d558f1e31036e75780c52690e812de265ec6 src/linux/routing/filter/icmp.cpp 60703e727ecab9557911ab0562773771db51db90 src/linux/routing/filter/internal.hpp c74098dab97807084e6630998da354265680c763 src/linux/routing/filter/ip.hpp 62bb5f89d35fc9622fca303aa01a789fcfbf2f76 src/linux/routing/filter/ip.cpp 0d25e2d4f0ac4c3f151ad40e796dbdbc54dba9fa src/linux/routing/handle.hpp PRE-CREATION src/linux/routing/queueing/handle.hpp 5f0cb7775f9190caba6b85cabf9019a97b2a7de2 src/tests/routing_tests.cpp 6bf5e63aecf2dee36fb8cf3575c0bd2625d29dfa Diff: https://reviews.apache.org/r/34321/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 34129: Add 2 optional args public_ip and public_port for libprocess to advertise.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34129/ --- (Updated May 18, 2015, 10:08 p.m.) Review request for mesos and Cosmin Lehene. Bugs: MESOS-809 https://issues.apache.org/jira/browse/MESOS-809 Repository: mesos Description --- If set, these IP/Port shall be advertised by libprocess (although bind is not done on this IP/Port). If not set, libprocess advertises the IP/Port on which bind was done. Command line arguments added: public_ip: Public IP address to reach mesos. Defaults to the command line argument `ip`. public_port: Public port to reach mesos. Defaults to the command line argument `port`. Diffs (updated) - docs/configuration.md 54c4e31ed6dfed3c23d492c19a301ce119a0519b docs/operational-guide.md 23b76ff129ca396a4b14a6826b4d842fc8527a8a src/master/http.cpp 01dbdad9720a9cd30505683ae1117970cda918a0 src/master/main.cpp d5666bc8ee8d7a0f0b8685f76d65dd1f9ac2a280 Diff: https://reviews.apache.org/r/34129/diff/ Testing --- Testing: make test Thanks, Anindya Sinha
Re: Review Request 34310: Use IDLE scheduling for revocable CPU in cgroups isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34310/#review84213 --- Bad patch! Reviews applied: [34308, 34309, 34308] Failed command: ./support/apply-review.sh -n -r 34308 Error: 2015-05-18 20:30:01 URL:https://reviews.apache.org/r/34308/diff/raw/ [1838/1838] - 34308.patch [1] error: patch failed: include/mesos/resources.hpp:105 error: include/mesos/resources.hpp: patch does not apply Failed to apply patch - Mesos ReviewBot On May 18, 2015, 5:33 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34310/ --- (Updated May 18, 2015, 5:33 p.m.) Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2652 https://issues.apache.org/jira/browse/MESOS-2652 Repository: mesos Description --- Use IDLE scheduling for revocable CPU in cgroups isolator. Diffs - src/slave/containerizer/isolators/cgroups/cpushare.hpp ff4a9dbdb1b655e71bf87dcee8fe62433d396f52 src/slave/containerizer/isolators/cgroups/cpushare.cpp 6a5b2b5c6e2844fe1a10815956569194b6f56681 Diff: https://reviews.apache.org/r/34310/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 34310: Use IDLE scheduling for revocable CPU in cgroups isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34310/#review84218 --- Bad patch! Reviews applied: [34308, 34308] Failed command: ./support/apply-review.sh -n -r 34308 Error: 2015-05-18 21:03:45 URL:https://reviews.apache.org/r/34308/diff/raw/ [1126/1126] - 34308.patch [1] error: patch failed: include/mesos/resources.hpp:189 error: include/mesos/resources.hpp: patch does not apply Failed to apply patch - Mesos ReviewBot On May 18, 2015, 8:49 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34310/ --- (Updated May 18, 2015, 8:49 p.m.) Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2652 https://issues.apache.org/jira/browse/MESOS-2652 Repository: mesos Description --- Use IDLE scheduling for revocable CPU in cgroups isolator. Diffs - src/slave/containerizer/isolators/cgroups/cpushare.hpp ff4a9dbdb1b655e71bf87dcee8fe62433d396f52 src/slave/containerizer/isolators/cgroups/cpushare.cpp 6a5b2b5c6e2844fe1a10815956569194b6f56681 Diff: https://reviews.apache.org/r/34310/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 34142: AppC provisioner.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34142/#review84199
---
src/slave/containerizer/mesos/containerizer.cpp
https://reviews.apache.org/r/34142/#comment135361
can tweak the constructors so that the containerizer doesn't need to be
exposed to ProvisionerProcesses, which is implementation detail?
src/slave/containerizer/provisioners/appc.hpp
https://reviews.apache.org/r/34142/#comment135339
nit: return type could be const?
src/slave/containerizer/provisioners/appc.hpp
https://reviews.apache.org/r/34142/#comment135375
If we say the dependency itself is an AppcImage that has n
AppcImage(dependencies). We can kill the dependency type?
AppcImage could then recursively resolve the dependencies on its own, using
the composite pattern described above, i.e., AppcProvisionerProcess simply
needs to call rootImage.resolve(), who loops over dependent images and calls
image.resolve().
We can then have fetch, fetchDependencies (not needed anymore), fetchURIs
and match togerther go into AppcImage::resolve. AppcImage will now need
functionality provided by discover and store; AppcPP only uses backend and does
mounts.
src/slave/containerizer/provisioners/appc.hpp
https://reviews.apache.org/r/34142/#comment135340
const-able?
src/slave/containerizer/provisioners/appc.hpp
https://reviews.apache.org/r/34142/#comment135341
nit: newline after ','
src/slave/containerizer/provisioners/appc.hpp
https://reviews.apache.org/r/34142/#comment135342
nit: craft a list and then return -.join(list)?
src/slave/containerizer/provisioners/appc.hpp
https://reviews.apache.org/r/34142/#comment135343
some of these should be const?
src/slave/containerizer/provisioners/appc.hpp
https://reviews.apache.org/r/34142/#comment135358
nit: const-able?
src/slave/containerizer/provisioners/appc.cpp
https://reviews.apache.org/r/34142/#comment135360
Is it possible to tweak the store{process}'s constructors to hide the
implemention detail of there existing a StoreProcess?
- Chi Zhang
On May 13, 2015, 12:48 a.m., Ian Downes wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34142/
---
(Updated May 13, 2015, 12:48 a.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
-
src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693
src/slave/containerizer/mesos/containerizer.cpp
b644b9c74bc23cf78c0a53284544be6cdaef2f8a
src/slave/containerizer/provisioners/appc.hpp PRE-CREATION
src/slave/containerizer/provisioners/appc.cpp PRE-CREATION
src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032
src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932
Diff: https://reviews.apache.org/r/34142/diff/
Testing
---
Thanks,
Ian Downes
Re: Review Request 34371: Add framework's pid to json summary of framework.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34371/#review84220 --- Ship it! Ship It! - Benjamin Hindman On May 18, 2015, 8:57 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34371/ --- (Updated May 18, 2015, 8:57 p.m.) Review request for mesos and Benjamin Hindman. Bugs: MESOS-2746 https://issues.apache.org/jira/browse/MESOS-2746 Repository: mesos Description --- Following pattern of slave exposing its pid. Diffs - src/master/http.cpp 7fa47a9fa2f5667631169876964dccf7fdcc172e Diff: https://reviews.apache.org/r/34371/diff/ Testing --- make check. Thanks, Joris Van Remoortere
Re: Review Request 34018: Update existing lambdas to meet style guide
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34018/#review84222 --- Ship it! Ship It! - Benjamin Hindman On May 17, 2015, 10:11 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34018/ --- (Updated May 17, 2015, 10:11 a.m.) Review request for mesos, Benjamin Hindman and Joris Van Remoortere. Bugs: MESOS-2670 https://issues.apache.org/jira/browse/MESOS-2670 Repository: mesos Description --- According Joris advice, replace the 'lambda::bind' expressions match these rules: 1. Binds to a static function without any side-effects. 2. Is self contained (i.e. does not rely on contextual parameters) 3. Does not bind in any arguments. (i.e. only uses lambda::_N for arguments) 4. Is only called in 1 place OR is so small that it is ok to repeat the code. Diffs - src/linux/cgroups.cpp df3211a0c25d7a16f36814886d14f81caaef2b9c src/log/network.hpp 7c74a55cc2a71fa2acd207605f972e7fbd203be4 src/slave/containerizer/isolators/cgroups/cpushare.cpp 6a5b2b5c6e2844fe1a10815956569194b6f56681 src/slave/containerizer/isolators/cgroups/mem.cpp 2c218b2b83cf42f54dbc7ec4c2ba8960b6e194de src/slave/containerizer/isolators/network/port_mapping.cpp a4abaff30bb4646b1b1edfdbbc243c9e3f6851df src/slave/containerizer/mesos/containerizer.cpp b644b9c74bc23cf78c0a53284544be6cdaef2f8a Diff: https://reviews.apache.org/r/34018/diff/ Testing --- make check Thanks, haosdent huang
Review Request 34375: Removed use of namespace aliases.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34375/ --- Review request for mesos. Repository: mesos Description --- See summary. Diffs - src/master/master.cpp eaea79df2c693d15087d70b3c9b988e57c894f8e src/slave/slave.cpp 8e88482f41f37ce7f2559fe793565b66ac46fb35 src/tests/master_tests.cpp ba3858fb3026f2cc7af0523bd1a0c541eeafc34b src/tests/slave_tests.cpp acae49731ff17103b529cdf828a63d9d55668549 Diff: https://reviews.apache.org/r/34375/diff/ Testing --- make check Thanks, Michael Park
Re: Review Request 34017: Update existing lambdas to meet style guide
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34017/#review84221 --- Ship it! Ship It! - Benjamin Hindman On May 17, 2015, 10:11 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34017/ --- (Updated May 17, 2015, 10:11 a.m.) Review request for mesos, Benjamin Hindman and Joris Van Remoortere. Bugs: MESOS-2670 https://issues.apache.org/jira/browse/MESOS-2670 Repository: mesos Description --- According Joris advice, replace the 'lambda::bind' expressions match these rules: 1. Binds to a static function without any side-effects. 2. Is self contained (i.e. does not rely on contextual parameters) 3. Does not bind in any arguments. (i.e. only uses lambda::_N for arguments) 4. Is only called in 1 place OR is so small that it is ok to repeat the code. Diffs - 3rdparty/libprocess/src/process.cpp 588bd3e40a0b350ceca15bb8f3f78290ba41d173 3rdparty/libprocess/src/tests/benchmarks.cpp 0d6714807f7027e6ab199eb0d9ff57bc8a3a2d8a Diff: https://reviews.apache.org/r/34017/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 34375: Removed use of namespace aliases.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34375/#review84238 --- Ship it! Ship It! - Till Toenshoff On May 18, 2015, 11:13 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34375/ --- (Updated May 18, 2015, 11:13 p.m.) Review request for mesos and Till Toenshoff. Repository: mesos Description --- See summary. Diffs - src/master/master.cpp eaea79df2c693d15087d70b3c9b988e57c894f8e src/slave/slave.cpp 8e88482f41f37ce7f2559fe793565b66ac46fb35 src/tests/master_tests.cpp ba3858fb3026f2cc7af0523bd1a0c541eeafc34b src/tests/slave_tests.cpp acae49731ff17103b529cdf828a63d9d55668549 Diff: https://reviews.apache.org/r/34375/diff/ Testing --- make check Thanks, Michael Park
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 May 19, 2015, 6:20 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Joerg Schad, Michael Park, and Till Toenshoff. Changes --- Solves issues from reviewers. Bugs: mesos-708 https://issues.apache.org/jira/browse/mesos-708 Repository: mesos Description --- When serving a static file, libprocess returns the header `Last-Modified` which is used by browsers to control Cache. When a http request arrives containing the header `If-Modified-Since`, a response `304 Not Modified` is returned if the date in the request and the modification time (as returned by doing `stat` in the file) coincide. Unit tests added. Diffs (updated) - 3rdparty/libprocess/include/process/http.hpp bba62b393dc863e724cb602b1504eb6517ae9730 3rdparty/libprocess/src/process.cpp e3de3cd6b536aaaf59784360aed546512dd04dc9 3rdparty/libprocess/src/tests/process_tests.cpp 67e582cc250a9767a389e2bd0cc68985477f3ffb Diff: https://reviews.apache.org/r/30032/diff/ Testing --- make check Thanks, Alexander Rojas
Review Request 34392: Added a method to Path which returns the modification time of the represented path.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34392/ --- Review request for mesos, Benjamin Hindman, Bernd Mathiske, Joerg Schad, Michael Park, and Till Toenshoff. Bugs: mesos-708 https://issues.apache.org/jira/browse/mesos-708 Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp d4df6502d1297ea3ad8e2a1e3bb16ea9d7c7913c Diff: https://reviews.apache.org/r/34392/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 34378: Fixed the dependency between 'summarize' and 'model'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34378/#review84241 --- Ship it! Ship It! - Till Toenshoff On May 18, 2015, 11:23 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34378/ --- (Updated May 18, 2015, 11:23 p.m.) Review request for mesos and Till Toenshoff. Repository: mesos Description --- Since `summarize` captures the important fields of an object, `model` will always be a superset of it. We can see that `model(const Framework)` calls `summarize(const Framework)` and augments additional fields. This patch removes the unnecessary forward declaration and makes `model(const Slave)` depend on `summarize(const Slave)` instead. Diffs - src/master/http.cpp 7fa47a9fa2f5667631169876964dccf7fdcc172e Diff: https://reviews.apache.org/r/34378/diff/ Testing --- make check Thanks, Michael Park
Re: Review Request 34268: stout library - adding support for Solaris
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34268/#review84202 --- Thanks a lot for this, Stan - much appreciated! There are a couple of style nits here and there and one basic question on the need of the `read`-variant for Solaris. For submitting an updated patch, please consult the patch submission guidelines http://mesos.apache.org/documentation/latest/submitting-a-patch/ specifically after Submit your patch - we need a patch that can be processed using our tooling and for that to work, an easy way is to follow that guide. File Attachment: adding missing new file: stout/os/sunos.hpp - sunos.hpp https://reviews.apache.org//r/34268/#fcomment67 s/Linux/SunOS/ File Attachment: adding missing new file: stout/os/sunos.hpp - sunos.hpp https://reviews.apache.org//r/34268/#fcomment68 Not used?! File Attachment: adding missing new file: stout/os/sunos.hpp - sunos.hpp https://reviews.apache.org//r/34268/#fcomment69 Not used?! File Attachment: adding missing new file: stout/os/sunos.hpp - sunos.hpp https://reviews.apache.org//r/34268/#fcomment70 Please wrap to get below 80 chars per line. ``` TryDuration utime = Seconds(pstatus.pr_utime.tv_sec) + Nanoseconds(pstatus.pr_utime.tv_nsec); TryDuration stime = Seconds(pstatus.pr_stime.tv_sec) + Nanoseconds(pstatus.pr_stime.tv_nsec); ``` File Attachment: adding missing new file: stout/os/sunos.hpp - sunos.hpp https://reviews.apache.org//r/34268/#fcomment71 Please wrap to stay below 80 chars per line. Also when looking at this patch with an editor, I noticed that your intendtion is partially off here - we use soft-tabs, 2 spaces for all C++ source files. ``` return Process(pstatus.pr_pid, pstatus.pr_ppid, pstatus.pr_ppid, pstatus.pr_sid, None(), utime.isSome() ? utime.get() : OptionDuration::none(), stime.isSome() ? stime.get() : OptionDuration::none(), psinfo.pr_fname, (psinfo.pr_nzomb == 0) (psinfo.pr_nlwp == 0) (psinfo.pr_lwp.pr_lwpid == 0)); ``` 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp https://reviews.apache.org/r/34268/#comment135382 We commonly comment on the `#endif` from `#ifdef` `#endif` combinations quoting the clause. ``` #endif // NAME_MAX ``` 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp https://reviews.apache.org/r/34268/#comment135381 ``` #endif // __sun ``` 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp https://reviews.apache.org/r/34268/#comment135383 ``` #endif // __sun ``` 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp https://reviews.apache.org/r/34268/#comment135378 Complete sentence with punctuation, please. ``` // FTS is not available on Solaris. ``` 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp https://reviews.apache.org/r/34268/#comment135384 ``` #endif // __sun ``` 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp https://reviews.apache.org/r/34268/#comment135371 Add a new line please. 3rdparty/libprocess/3rdparty/stout/include/stout/os/open.hpp https://reviews.apache.org/r/34268/#comment135372 Use complete sentences with punctuation please: ``` // Not defined on Solaris, taking a spare flag. ``` 3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp https://reviews.apache.org/r/34268/#comment135377 See below on `read`. 3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp https://reviews.apache.org/r/34268/#comment135373 Could you please explain why the standard implementation of this function would not work for Solaris? 3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp https://reviews.apache.org/r/34268/#comment135376 Insert new line, please. 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp https://reviews.apache.org/r/34268/#comment135374 We do not commonly comment the `#endif` of a `#if define()` ``` #endif ``` Looking forward to give your updated patch another review, thanks again. - Till Toenshoff On May 15, 2015, 2:25 p.m., Stan Teresen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34268/ --- (Updated May 15, 2015, 2:25 p.m.) Review request for mesos. Repository: mesos-incubating Description --- stout library - adding support for Solaris Diffs -
Re: Review Request 34140: Appc image store
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34140/#review84233 --- src/slave/containerizer/provisioners/appc/store.hpp https://reviews.apache.org/r/34140/#comment135392 Looks like this is a global store for all images. Would it make sense to make sure at most one StoreProcess can be instantiated? - Chi Zhang On May 13, 2015, 12:48 a.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34140/ --- (Updated May 13, 2015, 12:48 a.m.) Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone. Repository: mesos Description --- Images are fetched into the store (after discovery). Stored images are currently kept indefinitely. Diffs - src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 src/slave/containerizer/provisioners/appc/store.hpp PRE-CREATION src/slave/containerizer/provisioners/appc/store.cpp PRE-CREATION src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 Diff: https://reviews.apache.org/r/34140/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 34141: AppC provsioning backend.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34141/#review84237 --- src/slave/containerizer/provisioners/appc/backend.hpp https://reviews.apache.org/r/34141/#comment135398 Not a big deal, but would it be more flexible for Bankend to take a rootImage as an argument instead of a vector, which is already flattened? Resolving an dependency tree could yield a list of all tree nodes for the copy backend. src/slave/containerizer/provisioners/appc/backend.cpp https://reviews.apache.org/r/34141/#comment135399 put a nothing into the list before the for loop? - Chi Zhang On May 13, 2015, 12:48 a.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34141/ --- (Updated May 13, 2015, 12:48 a.m.) Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone. Repository: mesos Description --- Simple copy backend that forms the rootfs by copying each layer. Diffs - src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 src/slave/containerizer/provisioners/appc/backend.hpp PRE-CREATION src/slave/containerizer/provisioners/appc/backend.cpp PRE-CREATION src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 Diff: https://reviews.apache.org/r/34141/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 34139: AppC image discovery.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34139/#review84196 --- src/slave/containerizer/provisioners/appc/discovery.hpp https://reviews.apache.org/r/34139/#comment135337 AppcImage is introduced in r/34142 src/slave/containerizer/provisioners/appc/discovery.cpp https://reviews.apache.org/r/34139/#comment135389 Please see my comment in r/34142. I would like to suggest have AppcImage drive discover instead of AppcPP. src/slave/containerizer/provisioners/appc/discovery.cpp https://reviews.apache.org/r/34139/#comment135390 nit: this call is duplicated? maybe have discover just understand a raw canonicalized string? src/slave/flags.cpp https://reviews.apache.org/r/34139/#comment135335 should introduce all avaiable options to users here? Ditto to other flags introduced? - Chi Zhang On May 13, 2015, 12:47 a.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34139/ --- (Updated May 13, 2015, 12:47 a.m.) Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone. Repository: mesos Description --- Local and simple discovery only. Diffs - src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 src/slave/containerizer/provisioners/appc/discovery.hpp PRE-CREATION src/slave/containerizer/provisioners/appc/discovery.cpp PRE-CREATION src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 Diff: https://reviews.apache.org/r/34139/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 34138: AppC hash computation.
On May 18, 2015, 4:37 p.m., Chi Zhang wrote: push the implementation down to stout? is it possible to swap to use devel packages for hashing in the future? Not to stout because it's asynchronous but perhaps to libprocess. - Ian --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34138/#review84195 --- On May 12, 2015, 5:47 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34138/ --- (Updated May 12, 2015, 5:47 p.m.) Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone. Repository: mesos Description --- AppC hash computation. Diffs - src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 src/slave/containerizer/provisioners/appc/hash.hpp PRE-CREATION Diff: https://reviews.apache.org/r/34138/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 34375: Removed use of namespace aliases.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34375/#review84247 --- Patch looks great! Reviews applied: [34375] All tests passed. - Mesos ReviewBot On May 18, 2015, 11:13 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34375/ --- (Updated May 18, 2015, 11:13 p.m.) Review request for mesos and Till Toenshoff. Repository: mesos Description --- See summary. Diffs - src/master/master.cpp eaea79df2c693d15087d70b3c9b988e57c894f8e src/slave/slave.cpp 8e88482f41f37ce7f2559fe793565b66ac46fb35 src/tests/master_tests.cpp ba3858fb3026f2cc7af0523bd1a0c541eeafc34b src/tests/slave_tests.cpp acae49731ff17103b529cdf828a63d9d55668549 Diff: https://reviews.apache.org/r/34375/diff/ Testing --- make check Thanks, Michael Park
Re: Review Request 34359: Support multiple reasons in status update message.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34359/#review84205 --- Patch looks great! Reviews applied: [34359] All tests passed. - Mesos ReviewBot On May 18, 2015, 6:03 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34359/ --- (Updated May 18, 2015, 6:03 p.m.) Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone. Bugs: MESOS-2657 https://issues.apache.org/jira/browse/MESOS-2657 Repository: mesos Description --- Support multiple reasons in status update message. Diffs - include/mesos/mesos.proto 9cc5782256156ed59fd4640091413b76480d939f include/mesos/type_utils.hpp 837be6f1844d5fa01c0fd84a585e7ff2cc0c987b src/common/protobuf_utils.cpp bd6996159e73bf63bb7c2fa3a28def6a2be92b1b src/common/type_utils.cpp e92f6f36de0955784619029a016667b46bbe221b src/examples/balloon_framework.cpp c2337ba7ae00e5c59dfb3734d2f314c89687c4ad src/examples/java/TestFramework.java 9e95369b7a53c49328794ba9f8bd777e69f33889 src/master/master.cpp eaea79df2c693d15087d70b3c9b988e57c894f8e src/sched/sched.cpp 8c366ec3e3cf55dacf49483e1ceeef61ab0187b3 src/tests/master_authorization_tests.cpp 5633c823c116cdb24fe2620c746a982e69ab91ca src/tests/master_tests.cpp ba3858fb3026f2cc7af0523bd1a0c541eeafc34b src/tests/master_validation_tests.cpp dc9e91e120c2af9e72013557730f6a2fbb5b00fe src/tests/resource_offers_tests.cpp 882a9ff4d09aace486182828bf43b643b0d0c519 src/tests/scheduler_tests.cpp cbe6c91a1b4f864ceb11cf062da0ada6c9666f9f src/tests/slave_tests.cpp acae49731ff17103b529cdf828a63d9d55668549 Diff: https://reviews.apache.org/r/34359/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 34309: Support manipulating scheduler policy on Linux.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34309/ --- (Updated May 18, 2015, 1:48 p.m.) Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone. Changes --- Addressed comments. Bugs: MESOS-2652 https://issues.apache.org/jira/browse/MESOS-2652 Repository: mesos Description --- Support manipulating scheduler policy on Linux. Diffs (updated) - src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 src/linux/sched.hpp PRE-CREATION src/tests/sched_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/34309/diff/ Testing --- Added test. Thanks, Ian Downes
Re: Review Request 34310: Use IDLE scheduling for revocable CPU in cgroups isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34310/ --- (Updated May 18, 2015, 1:49 p.m.) Review request for mesos, Joris Van Remoortere, Niklas Nielsen, and Vinod Kone. Changes --- Addressed comments. Bugs: MESOS-2652 https://issues.apache.org/jira/browse/MESOS-2652 Repository: mesos Description --- Use IDLE scheduling for revocable CPU in cgroups isolator. Diffs (updated) - src/slave/containerizer/isolators/cgroups/cpushare.hpp ff4a9dbdb1b655e71bf87dcee8fe62433d396f52 src/slave/containerizer/isolators/cgroups/cpushare.cpp 6a5b2b5c6e2844fe1a10815956569194b6f56681 Diff: https://reviews.apache.org/r/34310/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 34308: Filter revocable resources.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34308/ --- (Updated May 18, 2015, 1:49 p.m.) Review request for mesos and Vinod Kone. Changes --- Removed duplicate definitions that crept in. Repository: mesos Description --- Filter revocable resources. Diffs (updated) - include/mesos/resources.hpp 1e98c13fe8075b14454f7899b98006fdaf88f484 src/common/resources.cpp 92b9e7f60323e0f7cf69c42e712468b631f3 Diff: https://reviews.apache.org/r/34308/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 30032: Added support for cache control in libprocess when dealing with static files.
On March 26, 2015, 5:59 p.m., Benjamin Hindman wrote: 3rdparty/libprocess/src/process.cpp, line 2854 https://reviews.apache.org/r/30032/diff/4/?file=834184#file834184line2854 ResultTime time = Path(response.path).mtime(); Alexander Rojas wrote: This cannot be implemented in terms on `Time`, since `Path` is part of stout and `Time` belongs to libprocess. Till Toenshoff wrote: Seems `Time` actually should be a stout class. From benh: Why is Time in libprocess? Because we have the ability to stop time via the Clock, which means we don't want anyone to _get_ time from anyplace else. If we put Time in stout then it's possible that someone could construct a Time that wasn't controlled by the libprocess Clock which wouldn't let us stop or advance time (per process) effectively. Make sense? - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30032/#review69827 --- On April 20, 2015, 1:58 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30032/ --- (Updated April 20, 2015, 1:58 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Joerg Schad, Michael Park, and Till Toenshoff. Bugs: mesos-708 https://issues.apache.org/jira/browse/mesos-708 Repository: mesos Description --- When serving a static file, libprocess returns the header `Last-Modified` which is used by browsers to control Cache. When a http request arrives containing the header `If-Modified-Since`, a response `304 Not Modified` is returned if the date in the request and the modification time (as returned by doing `stat` in the file) coincide. Unit tests added. Diffs - 3rdparty/libprocess/include/process/http.hpp 07825b2b7195fe7fe752e8fda65b7f0a8b8b1f38 3rdparty/libprocess/src/process.cpp 97ac09fd10b767bc46387abc3657d005a00830c8 3rdparty/libprocess/src/tests/process_tests.cpp eb38edce2c483ba7f963a826893a15a075238618 Diff: https://reviews.apache.org/r/30032/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 34138: AppC hash computation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34138/#review84195 --- push the implementation down to stout? is it possible to swap to use devel packages for hashing in the future? - Chi Zhang On May 13, 2015, 12:47 a.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34138/ --- (Updated May 13, 2015, 12:47 a.m.) Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone. Repository: mesos Description --- AppC hash computation. Diffs - src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 src/slave/containerizer/provisioners/appc/hash.hpp PRE-CREATION Diff: https://reviews.apache.org/r/34138/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 34129: Add 2 optional args public_ip and public_port for libprocess to advertise.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34129/#review84243 --- Patch looks great! Reviews applied: [34129] All tests passed. - Mesos ReviewBot On May 18, 2015, 10:08 p.m., Anindya Sinha wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34129/ --- (Updated May 18, 2015, 10:08 p.m.) Review request for mesos and Cosmin Lehene. Bugs: MESOS-809 https://issues.apache.org/jira/browse/MESOS-809 Repository: mesos Description --- If set, these IP/Port shall be advertised by libprocess (although bind is not done on this IP/Port). If not set, libprocess advertises the IP/Port on which bind was done. Command line arguments added: public_ip: Public IP address to reach mesos. Defaults to the command line argument `ip`. public_port: Public port to reach mesos. Defaults to the command line argument `port`. Diffs - docs/configuration.md 54c4e31ed6dfed3c23d492c19a301ce119a0519b docs/operational-guide.md 23b76ff129ca396a4b14a6826b4d842fc8527a8a src/master/http.cpp 01dbdad9720a9cd30505683ae1117970cda918a0 src/master/main.cpp d5666bc8ee8d7a0f0b8685f76d65dd1f9ac2a280 Diff: https://reviews.apache.org/r/34129/diff/ Testing --- Testing: make test Thanks, Anindya Sinha
Re: Review Request 34375: Removed use of namespace aliases.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34375/#review84251 --- This might be a valid namespace alias use case that we hadn't considered, because there is no way to be able to write just `http::Response` otherwise, is there? Seems quite verbose to write process::http everywhere, and on the otherhand just having `Request` or `Response` seems to miss the context of it being http, thoughts? - Ben Mahler On May 18, 2015, 11:13 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34375/ --- (Updated May 18, 2015, 11:13 p.m.) Review request for mesos and Till Toenshoff. Repository: mesos Description --- See summary. Diffs - src/master/master.cpp eaea79df2c693d15087d70b3c9b988e57c894f8e src/slave/slave.cpp 8e88482f41f37ce7f2559fe793565b66ac46fb35 src/tests/master_tests.cpp ba3858fb3026f2cc7af0523bd1a0c541eeafc34b src/tests/slave_tests.cpp acae49731ff17103b529cdf828a63d9d55668549 Diff: https://reviews.apache.org/r/34375/diff/ Testing --- make check Thanks, Michael Park
Re: Review Request 34378: Fixed the dependency between 'summarize' and 'model'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34378/#review84252 --- Patch looks great! Reviews applied: [34378] All tests passed. - Mesos ReviewBot On May 18, 2015, 11:23 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34378/ --- (Updated May 18, 2015, 11:23 p.m.) Review request for mesos and Till Toenshoff. Repository: mesos Description --- Since `summarize` captures the important fields of an object, `model` will always be a superset of it. We can see that `model(const Framework)` calls `summarize(const Framework)` and augments additional fields. This patch removes the unnecessary forward declaration and makes `model(const Slave)` depend on `summarize(const Slave)` instead. Diffs - src/master/http.cpp 7fa47a9fa2f5667631169876964dccf7fdcc172e Diff: https://reviews.apache.org/r/34378/diff/ Testing --- make check Thanks, Michael Park
Re: Review Request 34387: Moved up Slave and Framework structs in master.hpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34387/#review84254 --- Patch looks great! Reviews applied: [34387] All tests passed. - Mesos ReviewBot On May 19, 2015, 1:56 a.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34387/ --- (Updated May 19, 2015, 1:56 a.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-2507 https://issues.apache.org/jira/browse/MESOS-2507 Repository: mesos Description --- This is a pure code movement to pull up the Slave and Framework structs, needed for: https://reviews.apache.org/r/34388/ Unfortunately the diff renders terribly, not sure why, but it is a pure code movement. Diffs - src/master/master.hpp da0a83510784f4f7dbd933e666ac12c04c413a62 Diff: https://reviews.apache.org/r/34387/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 34361: converted hard-coded strings to consts
On May 18, 2015, 5:48 p.m., Marco Massenzio wrote: src/examples/test_hook_module.cpp, lines 36-38 https://reviews.apache.org/r/34361/diff/1/?file=962951#file962951line36 Thanks for doing this! I'm wondering whether, as these are constants, shouldn't they be in `SCREAMING_SNAKE_CASE` as per the [Style Guide](http://mesos.apache.org/documentation/latest/mesos-c++-style-guide/)? I completely realize that the existing code (lines above) does not follow this style, so I'm really wondering whether that was by design or accident? And, if the latter, would this be also a good time to fix those too? No matter what, though, this is A Good Thing. I'd be fine converting the other constants too, I guess just waiting on somebody saying if it was intentional or not. - Colin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34361/#review84172 --- On May 18, 2015, 5:01 p.m., Colin Williams wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34361/ --- (Updated May 18, 2015, 5:01 p.m.) Review request for mesos. Bugs: MESOS-2637 https://issues.apache.org/jira/browse/MESOS-2637 Repository: mesos Description --- converted hard-coded strings to consts Diffs - src/examples/test_hook_module.cpp b25830ab6475f997422cfd2f60cc9a79e1acadfe src/tests/hook_tests.cpp a65c0ab7c41ec3b7964f9d572381fa3e61746dc3 src/tests/master_tests.cpp ba3858fb3026f2cc7af0523bd1a0c541eeafc34b src/tests/slave_tests.cpp acae49731ff17103b529cdf828a63d9d55668549 Diff: https://reviews.apache.org/r/34361/diff/ Testing --- Thanks, Colin Williams
Review Request 34387: Moved up Slave and Framework structs in master.hpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34387/ --- Review request for mesos and Vinod Kone. Bugs: MESOS-2507 https://issues.apache.org/jira/browse/MESOS-2507 Repository: mesos Description --- This is a pure code movement to pull up the Slave and Framework structs, needed for: https://reviews.apache.org/r/34388/ Unfortunately the diff renders terribly, not sure why, but it is a pure code movement. Diffs - src/master/master.hpp da0a83510784f4f7dbd933e666ac12c04c413a62 Diff: https://reviews.apache.org/r/34387/diff/ Testing --- make check Thanks, Ben Mahler
Review Request 34389: Removed Master::getSlave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34389/ --- Review request for mesos and Vinod Kone. Bugs: MESOS-2507 https://issues.apache.org/jira/browse/MESOS-2507 Repository: mesos Description --- `getSlave` and `slaves.registered.get` were used interchangeably throughout the code. This removeds `getSlave` in favor of just using the `slaves.registered.get()`. Diffs - src/master/master.hpp da0a83510784f4f7dbd933e666ac12c04c413a62 src/master/master.cpp eaea79df2c693d15087d70b3c9b988e57c894f8e src/master/validation.cpp 20a6ac833ec5dc437f80159a9234c7b94d86ba29 Diff: https://reviews.apache.org/r/34389/diff/ Testing --- make check Thanks, Ben Mahler
