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/#review87550
---
3rdparty/libprocess/include/process/time.hpp
https://reviews.apache.org/r/34703/#comment139934
`s/constexpr/static/`
3rdparty/libprocess/include/process/time.hpp
https://reviews.apache.org/r/34703/#comment139936
Formatting:
```static const char* WEEK_DAYS[] = {
Sun, Mon, Tue, Wed, Thu, Fri, Sat
};
```
3rdparty/libprocess/include/process/time.hpp
https://reviews.apache.org/r/34703/#comment139935
`s/constexpr/static/`
3rdparty/libprocess/include/process/time.hpp
https://reviews.apache.org/r/34703/#comment139937
Formatting:
```
static const char* MONTHS[] = {
Jan,
Feb,
Mar,
Apr,
May,
Jun,
Jul,
Aug,
Sep,
Oct,
Nov,
Dec
};
```
- Michael Park
On June 8, 2015, 3:14 p.m., Alexander Rojas wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34703/
---
(Updated June 8, 2015, 3:14 p.m.)
Review request for mesos, Bernd Mathiske, Michael Park, Nikita Vetoshkin, and
Till Toenshoff.
Bugs: mesos-708
https://issues.apache.org/jira/browse/mesos-708
Repository: mesos
Description
---
Adds some manipulator classes which allows formatting Time objects to
ostreams.
Diffs
-
3rdparty/libprocess/include/process/time.hpp
c5ab2a3cfa83590eb6612152ae365dd67f51cea9
3rdparty/libprocess/src/tests/time_tests.cpp
be314182c65c05d439b81aa5248a71d93f6f0a0b
Diff: https://reviews.apache.org/r/34703/diff/
Testing
---
make check
Thanks,
Alexander Rojas
Review Request 35350: Fixes markdown of the Doxygen Styleguide to provide better rendering.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35350/ --- Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Joerg Schad. Repository: mesos Description --- See summary. Diffs - docs/mesos-doxygen-style-guide.md 93decf9984a8ebf5c713b968da53fc38caabeab0 Diff: https://reviews.apache.org/r/35350/diff/ Testing --- Thanks, Alexander Rojas
Re: Review Request 34703: Added stream manipulators for the Time object.
On June 11, 2015, 11:10 a.m., Michael Park wrote:
3rdparty/libprocess/include/process/time.hpp, lines 111-114
https://reviews.apache.org/r/34703/diff/4/?file=980765#file980765line111
Formatting:
```static const char* WEEK_DAYS[] = {
Sun, Mon, Tue, Wed, Thu, Fri, Sat
};
```
Sorry, not sure what happened there.
I meant
```cpp
static const char* WEEK_DAYS[] = {
Sun, Mon, Tue, Wed, Thu, Fri, Sat
};
```
- Michael
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34703/#review87550
---
On June 8, 2015, 3:14 p.m., Alexander Rojas wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34703/
---
(Updated June 8, 2015, 3:14 p.m.)
Review request for mesos, Bernd Mathiske, Michael Park, Nikita Vetoshkin, and
Till Toenshoff.
Bugs: mesos-708
https://issues.apache.org/jira/browse/mesos-708
Repository: mesos
Description
---
Adds some manipulator classes which allows formatting Time objects to
ostreams.
Diffs
-
3rdparty/libprocess/include/process/time.hpp
c5ab2a3cfa83590eb6612152ae365dd67f51cea9
3rdparty/libprocess/src/tests/time_tests.cpp
be314182c65c05d439b81aa5248a71d93f6f0a0b
Diff: https://reviews.apache.org/r/34703/diff/
Testing
---
make check
Thanks,
Alexander Rojas
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/#review87543
---
Can we also introduce a corresponding `.cpp` for the implementation?
3rdparty/libprocess/include/process/time.hpp
https://reviews.apache.org/r/34703/#comment139914
The pattern in Mesos for output streamers is such that we define the
behavior in `operator` and `friend` it from the class.
3rdparty/libprocess/include/process/time.hpp
https://reviews.apache.org/r/34703/#comment139916
(1) `s/struct//`
(2) `= {};`
3rdparty/libprocess/include/process/time.hpp
https://reviews.apache.org/r/34703/#comment139922
(1) If we want to zero-initialize the array, we should do `= {};`
(2) Can we choose a power of 2 for the size?
(3) In `RFC3339`, we name this `date`. Let's call them both `buffer` or
both `date`.
(4) Let's move this down past `HTTP_DATE`, `WEEK_DAYS` and `MONTHS`, just
above the `snprintf` call.
3rdparty/libprocess/include/process/time.hpp
https://reviews.apache.org/r/34703/#comment139933
Can we just inline this? We inline the format string in `RFC3339`
3rdparty/libprocess/include/process/time.hpp
https://reviews.apache.org/r/34703/#comment139923
We should indent 4 spaces from the beginning of the function call:
```
if (snprintf(
buffer,
sizeof(buffer),
HTTP_DATE,
WEEK_DAYS[timeInfo.tm_wday],
timeInfo.tm_mday,
MONTHS[timeInfo.tm_mon],
timeInfo.tm_year + 1900,
timeInfo.tm_hour,
timeInfo.tm_min,
timeInfo.tm_sec) 0) {
```
3rdparty/libprocess/include/process/time.hpp
https://reviews.apache.org/r/34703/#comment139927
No need to construct a `std::string` here.
3rdparty/libprocess/include/process/time.hpp
https://reviews.apache.org/r/34703/#comment139920
`s/tm_/timeInfo/`
`= {};`
3rdparty/libprocess/include/process/time.hpp
https://reviews.apache.org/r/34703/#comment139921
(1) If we want to zero-initialize the array, we should do `= {};`
(2) Can we choose a power of 2 for the size?
3rdparty/libprocess/include/process/time.hpp
https://reviews.apache.org/r/34703/#comment139932
Should be 2-space indented.
3rdparty/libprocess/src/tests/time_tests.cpp
https://reviews.apache.org/r/34703/#comment139931
We don't need to use `stringstream` here to use the `operator` since
`stringify` uses it.
We can simplify to:
```
EXPECT_EQ(Thu, 02 Mar 1989 00:00:00 UTC,
stringify(RFC1123(Time::epoch() + Weeks(1000;
```
- Michael Park
On June 8, 2015, 3:14 p.m., Alexander Rojas wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34703/
---
(Updated June 8, 2015, 3:14 p.m.)
Review request for mesos, Bernd Mathiske, Michael Park, Nikita Vetoshkin, and
Till Toenshoff.
Bugs: mesos-708
https://issues.apache.org/jira/browse/mesos-708
Repository: mesos
Description
---
Adds some manipulator classes which allows formatting Time objects to
ostreams.
Diffs
-
3rdparty/libprocess/include/process/time.hpp
c5ab2a3cfa83590eb6612152ae365dd67f51cea9
3rdparty/libprocess/src/tests/time_tests.cpp
be314182c65c05d439b81aa5248a71d93f6f0a0b
Diff: https://reviews.apache.org/r/34703/diff/
Testing
---
make check
Thanks,
Alexander Rojas
Re: Review Request 34976: Added installation instructions for Ubuntu 14.04 and OSX
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34976/#review87539 --- Sorry it took me so long to get back to this. I just had a few minor comments/questions, but you ought to get a Mac user to verify the OSX instructions. docs/getting-started.md https://reviews.apache.org/r/34976/#comment139910 This was a general comment that came up during our last review meeting, and I wanted to make a note so it wasn't lost. Apologies if this was confusing to you. Let me explain the intention in more detail. _If_ there was content for 12.04 being dropped in favor of the more modern 14.04 instructions, then it would be better to archive the old instructions than lose them completely. Seems like no content was dropped, so this is a non-issue here. Also, we agreed that it would be better to keep a single set of instructions per distro, only referencing the latest (e.g. Ubuntu LTS) release, rather than having instructions for multiple versions of the same distro. This echoes my original comment. It seems like you've acted accordingly, and there is no content lost from 12.04, so no need to archive. Feel free to drop the issue. docs/getting-started.md https://reviews.apache.org/r/34976/#comment139911 Why upgrade? Doesn't seem necessary. docs/getting-started.md https://reviews.apache.org/r/34976/#comment139912 `build-essential` is no longer needed? Or pre-installed in 14.04? docs/getting-started.md https://reviews.apache.org/r/34976/#comment139913 What about these python packages? Probably still necessary if you don't `configure --disable-python`. docs/getting-started.md https://reviews.apache.org/r/34976/#comment139915 0.21 and newer? (sasl is 0.14+, maven is 0.18+) Otherwise why call out the version at all, if you're just going to recommend they install the latest? docs/getting-started.md https://reviews.apache.org/r/34976/#comment139917 Can you get a Mac user to review/approve these instructions? Maybe @davelester or @nnielsen docs/getting-started.md https://reviews.apache.org/r/34976/#comment139918 Both of these links go to `https://developer.apple.com/xcode/` Is this intentional? Also on your Mac is probably redundant. docs/getting-started.md https://reviews.apache.org/r/34976/#comment139919 s/deps/dependencies/ docs/getting-started.md https://reviews.apache.org/r/34976/#comment139894 Probably better to recommend j=number of cores or j=#GB RAM, since j=8 could be overkill on a dual-core or smaller. - Adam B On June 2, 2015, 11:44 p.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34976/ --- (Updated June 2, 2015, 11:44 p.m.) Review request for mesos and Dave Lester. Repository: mesos Description --- Added installation instructions for Ubuntu 14.04 and OSX Diffs - docs/getting-started.md f0436575ec568e445f897ed28f50bcd823302d75 Diff: https://reviews.apache.org/r/34976/diff/ Testing --- This are the steps I've followed to build Mesos on my Mac and on my Ubuntu box. Thanks, Marco Massenzio
Re: Review Request 34976: Added installation instructions for Ubuntu 14.04 and OSX
On June 11, 2015, 3:02 a.m., Adam B wrote: Sorry it took me so long to get back to this. I just had a few minor comments/questions, but you ought to get a Mac user to verify the OSX instructions. Also, for doc changes like this, it's helpful if you can post a link to a personal fork/branch with the rendered markdown. Something like https://github.com/massenz/mesos/blob/add_docs_ubuntu/docs/getting-started.md - Adam --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34976/#review87539 --- On June 2, 2015, 11:44 p.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34976/ --- (Updated June 2, 2015, 11:44 p.m.) Review request for mesos and Dave Lester. Repository: mesos Description --- Added installation instructions for Ubuntu 14.04 and OSX Diffs - docs/getting-started.md f0436575ec568e445f897ed28f50bcd823302d75 Diff: https://reviews.apache.org/r/34976/diff/ Testing --- This are the steps I've followed to build Mesos on my Mac and on my Ubuntu box. Thanks, Marco Massenzio
Re: Review Request 28763: Add configure flag to enable SSL.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28763/ --- (Updated June 11, 2015, 9:26 a.m.) Review request for mesos, Benjamin Hindman and Michael Park. Changes --- unwind pthread configuration change Bugs: MESOS-2108 https://issues.apache.org/jira/browse/MESOS-2108 Repository: mesos Description --- See summary. Diffs (updated) - 3rdparty/libprocess/configure.ac 710490b2a7c71f35434494e87e2d132f78ef370a Diff: https://reviews.apache.org/r/28763/diff/ Testing --- configure with '--enable-libevent --enable--ssl' and without. Thanks, Joris Van Remoortere
Re: Review Request 29406: Introduce libevent ssl socket.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29406/ --- (Updated June 11, 2015, 2:57 p.m.) Review request for Benjamin Hindman and Michael Park. Changes --- Unwind accidental inclusion of upgrade path. Turn more static functions into lambdas. Clean up some style issues. Use EXIT(EXIT_FAILURE) for some user based error messages. Bugs: MESOS-1913 https://issues.apache.org/jira/browse/MESOS-1913 Repository: mesos Description --- Requires: configure --enable-libevent --enable-libevent-socket --enable-ssl New environment variables: ``` SSL_CERT_FILE=(path to certificate) SSL_KEY_FILE=(path to key) SSL_VERIFY_CERT=(false|0,true|1) SSL_REQUIRE_CERT=(false|0,true|1) SSL_VERIFY_DEPTH=(4) SSL_CA_DIR=(path to CA directory) SSL_CA_FILE=(path to CA file) SSL_CIPHERS=(accepted ciphers separated by ':') SSL_ENABLE_SSL_V2=(false|0,true|1) SSL_ENABLE_SSL_V3=(false|0,true|1) SSL_ENABLE_TLS_V1_0=(false|0,true|1) SSL_ENABLE_TLS_V1_1=(false|0,true|1) SSL_ENABLE_TLS_V1_2=(false|0,true|1) ``` Only TLSV1.2 is enabled by default. Use the `ENABLE_SSL_V*` and `ENABLE_TLS_V*` environment variables to open up more protocols. Use the `SSL_CIPHERS` environment variable to restrict or open up the supported ciphers. Diffs (updated) - 3rdparty/libprocess/Makefile.am 489ce359f383d819299335cbaa8c95724b0c6ac2 3rdparty/libprocess/include/process/socket.hpp b8c2274de535ac473e49a09165b601c96d3ebe8b 3rdparty/libprocess/src/libevent.hpp f6cc72178613a30446629532a773afccfd404212 3rdparty/libprocess/src/libevent.cpp fb038597358135a06c1927d079cb7cb09fea7452 3rdparty/libprocess/src/libevent_ssl_socket.hpp PRE-CREATION 3rdparty/libprocess/src/libevent_ssl_socket.cpp PRE-CREATION 3rdparty/libprocess/src/openssl.hpp PRE-CREATION 3rdparty/libprocess/src/openssl.cpp PRE-CREATION 3rdparty/libprocess/src/process.cpp aadd7bb0ae12b93336900c76d8d5aaa4421ea198 3rdparty/libprocess/src/socket.cpp 0e1cebb19e21c706b152d35a0b8722924c971a35 Diff: https://reviews.apache.org/r/29406/diff/ Testing --- make check (uses non-ssl socket) benchmarks using ssl sockets master, slave, framework, webui launch with ssl sockets Thanks, Joris Van Remoortere
Re: Review Request 35361: Added doc for standalone to HA
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35361/#review87570 --- Patch looks great! Reviews applied: [35361] All tests passed. - Mesos ReviewBot On June 11, 2015, 3:22 p.m., Michael Schenck wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35361/ --- (Updated June 11, 2015, 3:22 p.m.) Review request for mesos. Bugs: MESOS-2855 https://issues.apache.org/jira/browse/MESOS-2855 Repository: mesos Description --- Added doc for standalone to HA. This methodology has been discuessed in the mailing list and I've tested this several times with a dev clsuter. Most recently, I've used this procedure to migrate a production mesos cluster (with over 100 services and crons, and 6 mesos slaves) from _standalone_ to `--quorum=2` with no issues. It is probably worth noting that the only framework in use on the cluster is Aurora. Diffs - docs/operational-guide.md 23b76ff129ca396a4b14a6826b4d842fc8527a8a Diff: https://reviews.apache.org/r/35361/diff/ Testing --- Thanks, Michael Schenck
Re: Review Request 35260: Refactored the ResourceMonitor to get statistics from the Slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35260/#review87571 --- include/mesos/mesos.proto https://reviews.apache.org/r/35260/#comment139959 I just thought of one thing - this represents the entire container and not the executor alone. Does it make sense to call it 'Container' instead? - Niklas Nielsen On June 10, 2015, 4:13 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35260/ --- (Updated June 10, 2015, 4:13 p.m.) Review request for mesos, Ben Mahler, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2818 https://issues.apache.org/jira/browse/MESOS-2818 Repository: mesos Description --- Refactored the ResourceMonitor to get statistics from the Slave. 1) Modified ResourceUsage to include allocation information (see ticket for reaons). 2) Simplied the ResourceMonitor by taking a lambda from the slave to get statistics. 3) Adjusted (and simplified) the MonitorTest accordingly. Diffs - include/mesos/mesos.proto 7457ff11f6a55099ccb95beb2f0ccb9a2f7ccd87 include/mesos/slave/resource_estimator.hpp 7f78fd86760397d5b885a2c00b41081d0b546cdd src/slave/monitor.hpp bee91ba143c26059fc8badf56beccb68c6556cb7 src/slave/monitor.cpp 8f7ff63b4daf0286d4cf912e2f3d2d1b68caab1c src/slave/resource_estimators/fixed.cpp 3efa18d7e3c6ac62e67f75ea45a832f3f6349036 src/slave/resource_estimators/noop.hpp 7a44e6deefc9c1985c14d076a427aa5c654aa1bb src/slave/resource_estimators/noop.cpp 5f135ff37e84c248ede29bbe4a7d1b8319417e20 src/slave/slave.hpp 98c64f62f60c286e42379b2151321b190c4410b8 src/slave/slave.cpp b9fb929684ae582b9133126fefa5a06d4181d836 src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 src/tests/monitor_tests.cpp 6de8b1f65843fd7b852dfa69627a1c435b482fe0 Diff: https://reviews.apache.org/r/35260/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 34943: Added validation to flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34943/#review87573 --- 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp https://reviews.apache.org/r/34943/#comment139961 Style comment: Default captures are considered stylistically bad. Captures should be explicit(Meyers, Effective Modern C++, Item 31). - Jojy Varghese On June 11, 2015, 1:52 p.m., Benjamin Hindman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34943/ --- (Updated June 11, 2015, 1:52 p.m.) Review request for mesos. Repository: mesos Description --- Also refactored existing 'lambda::bind' arguments to use C++11 lambdas, enabling us to get rid of our loader and stringifier functors. Diffs - 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 6ac2f04a6a1d8db1725972a3b4b46a0dd734566f 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp 87606d860dea3235ec70d127d3379065d42dc90b 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp ee855da6128c2d671eb2fc7eaa2c0aab2341aebb 3rdparty/libprocess/3rdparty/stout/include/stout/flags/loader.hpp 51d3ab020bd2bae1f12b66cac0da5383c813d5d7 3rdparty/libprocess/3rdparty/stout/include/stout/flags/stringifier.hpp fda5ae1328a23a4371475652f921341dce7448d5 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 95b4b33b70c37640d97dff5d5724550d42048b76 3rdparty/libprocess/3rdparty/stout/include/stout/os/open.hpp 134453e91d39fd086baa9396ab42a002fed8b73e 3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp 59cde89b5e035807a510b331b85a8cd48da36ae3 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 4485e41feb8320d2ec7251b8b894ce437f61addd 3rdparty/libprocess/3rdparty/stout/include/stout/os/sunos.hpp cfe6d742560f50079fb1ed7346526d432615613c 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 80450185f60c5b273face490e0bb9e695b0cb984 3rdparty/libprocess/include/Makefile.am d01880c66ca544d62d3ada1ea79c8045884858da 3rdparty/libprocess/include/process/firewall.hpp 16ed852d07344e53c6f9d30d4cd7d99e07c6606d 3rdparty/libprocess/include/process/process.hpp e70dd388601bd26fcc82f66e0cb95924da8a8c2f 3rdparty/libprocess/src/process.cpp aadd7bb0ae12b93336900c76d8d5aaa4421ea198 3rdparty/libprocess/src/tests/process_tests.cpp 660af45e7fd45bdf5d43ad9aa54477fd94f87058 Doxyfile 1b0a2beda1d9223eafd9d14d335c651eb4961a15 docs/configuration.md aaf65bfa2848318c8d07c772ba2c521aa72c2677 docs/engineering-principles-and-practices.md 6babb929ead758ee5187d5fc5760d084876c3298 docs/home.md ee3d3e9a417d75af776dad3ea9627d3c6d780dca docs/mesos-c++-style-guide.md 94107ed21c6f09349ce691f9f4d36b43bbbe809e docs/powered-by-mesos.md 0ee763ffe32e6667e4ba708dd275d9f1b5116815 include/mesos/containerizer/containerizer.proto f16ccc89f83da28c413ccfa0687a06b7515a605c include/mesos/master/allocator.hpp 92de1af414321281b00eaa6f129e5e3e2c849448 include/mesos/mesos.proto 7457ff11f6a55099ccb95beb2f0ccb9a2f7ccd87 src/Makefile.am 10b190288425d735b116ed2dd3f040e871c5cb18 src/cli/mesos-ps ae48c8cde31e90024381ed3e6e22ec3bc96560b6 src/examples/no_executor_framework.cpp 6c5c7d3121e3a2ea78f66dffdeaecb72cca6293c src/files/files.cpp 3cdd38a3c7122bd5e13c8928279d85ab1373a63e src/linux/routing/handle.hpp 052c7cc1a967797d245a275d08cc774f627398a5 src/linux/routing/queueing/fq_codel.hpp 18c53b416718c27045986d939bb85f9fec731ca5 src/linux/routing/queueing/fq_codel.cpp 446863391d1c6dd2f91f890596927a20ee59a35f src/main.dox d5b29cf96b18ba84ac39f1747c4a174a5bfbc2e1 src/master/allocator/mesos/allocator.hpp 6cfa04650d91a80211cfbc0809236f9438926c78 src/master/allocator/mesos/hierarchical.hpp c220ea673320d4d06403afa76f0eb9f3b7a4e5eb src/master/flags.hpp 55ed3a9ef84a39841305cca2ba6c5df45c1990e1 src/master/flags.cpp 4377715029878cfee36f067e9c53c42b522b79d8 src/master/main.cpp 1c33e3bf7baae45b1671f9566b4993f39d9f1294 src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 src/master/master.cpp e91e8815471b49d3c649470197d6da3f06de62fb src/master/metrics.hpp 3d389e68f22e3d1e00bde0db4e25f897c79a2316 src/master/metrics.cpp d2489c8897fb2f2f21f021ac5e7a2ada7997ea06 src/messages/flags.hpp 41be419ba5593a600aa0c6c411210fa4faa829a8 src/messages/flags.proto 5400c92297f252734789982d21d7117ef4c57a57 src/slave/containerizer/isolators/network/port_mapping.cpp d2da1a4e96baeac7d1af9a5468f90c2e4c1cb50f src/slave/flags.hpp 6c24e56d15881b0e3aeec3d4824842cf57121fc6 src/slave/flags.cpp 99142fb1cf9d1978df86fdbf612e656b8fe852a6
Re: Review Request 35341: Added Uber to the Powered-by-Mesos page
On June 10, 2015, 7:48 p.m., Niklas Nielsen wrote: docs/powered-by-mesos.md, line 91 https://reviews.apache.org/r/35341/diff/1/?file=982409#file982409line91 35341.patch:17: new blank line at EOF. + warning: 1 line adds whitespace errors. Mind fixing this, Marco? I fixed it before committing. Thanks Marco! - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35341/#review87514 --- On June 10, 2015, 7:41 p.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35341/ --- (Updated June 10, 2015, 7:41 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos Description --- Added Uber to the Powered-by-Mesos page Diffs - docs/powered-by-mesos.md 0ee763ffe32e6667e4ba708dd275d9f1b5116815 Diff: https://reviews.apache.org/r/35341/diff/ Testing --- Thanks, Marco Massenzio
Re: Review Request 35260: Refactored the ResourceMonitor to get statistics from the Slave.
On June 11, 2015, 4:18 p.m., Niklas Nielsen wrote: include/mesos/mesos.proto, line 626 https://reviews.apache.org/r/35260/diff/3/?file=982288#file982288line626 I just thought of one thing - this represents the entire container and not the executor alone. Does it make sense to call it 'Container' instead? +1 - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35260/#review87571 --- On June 10, 2015, 11:13 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35260/ --- (Updated June 10, 2015, 11:13 p.m.) Review request for mesos, Ben Mahler, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2818 https://issues.apache.org/jira/browse/MESOS-2818 Repository: mesos Description --- Refactored the ResourceMonitor to get statistics from the Slave. 1) Modified ResourceUsage to include allocation information (see ticket for reaons). 2) Simplied the ResourceMonitor by taking a lambda from the slave to get statistics. 3) Adjusted (and simplified) the MonitorTest accordingly. Diffs - include/mesos/mesos.proto 7457ff11f6a55099ccb95beb2f0ccb9a2f7ccd87 include/mesos/slave/resource_estimator.hpp 7f78fd86760397d5b885a2c00b41081d0b546cdd src/slave/monitor.hpp bee91ba143c26059fc8badf56beccb68c6556cb7 src/slave/monitor.cpp 8f7ff63b4daf0286d4cf912e2f3d2d1b68caab1c src/slave/resource_estimators/fixed.cpp 3efa18d7e3c6ac62e67f75ea45a832f3f6349036 src/slave/resource_estimators/noop.hpp 7a44e6deefc9c1985c14d076a427aa5c654aa1bb src/slave/resource_estimators/noop.cpp 5f135ff37e84c248ede29bbe4a7d1b8319417e20 src/slave/slave.hpp 98c64f62f60c286e42379b2151321b190c4410b8 src/slave/slave.cpp b9fb929684ae582b9133126fefa5a06d4181d836 src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 src/tests/monitor_tests.cpp 6de8b1f65843fd7b852dfa69627a1c435b482fe0 Diff: https://reviews.apache.org/r/35260/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 34687: (De)Serializing MasterInfo PB to JSON
On June 10, 2015, 8:37 p.m., Ben Mahler wrote:
How about adding an 'Address' message, which can contain 'ip' and 'port'
for now?
```
message Address {
required string ip;
required uint32 port;
// Later we can add 'hostname' or 'public_hostname', etc!
}
```
In the future, we can further use this in other protobufs to have a
conistent representation (e.g. SlaveInfo only has 'hostname' and 'port'
currently, no 'ip'!). That way, you can add an address to MasterInfo:
```
message MasterInfo {
required string id = 1;
required uint32 ip = 2;
required uint32 port = 3 [default = 5050];
optional string pid = 4;
optional string hostname = 5;
optional Address address = 6;
}
```
This way, you don't need all the custom logic introduced here, and
consequently compatibility is easier to test (i.e. we have already tested
our JSON - Protobuf conversion facilities).
Have you guys considered this approach?
@BenH, Vinod and BenM: We have too many chefs her. Let's please assign one to
drive this. It sounds like we have to take this back to the design (and JIRA
discussions).
Here is my concern; I was assigned as shepherd on this and got blocked in the
11th hour.
There is very little space to iterate on this and the patch would have worked
fine. It was a part of a bigger patch set (zookeeper detection for HTTP, which
Vinod drove) and I am amazed that a conversion between proto to json and
vice-versa takes more than 2 weeks back and forth, to then (most likely)
getting dropped. This is one example where both committers and contributors
(us) have a bad experience. I would like to know how to get better and avoid
this in the future, without giving up 'shepherd'ship.
@Marco; I am sorry that we wasted your (and our) time. Let's try to fix the
process. It hurts me as well as I thought we had this in a good shape but it
sounds to me like we need to drop this patch.
- Niklas
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/#review87520
---
On June 5, 2015, 1:18 p.m., Marco Massenzio wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/
---
(Updated June 5, 2015, 1:18 p.m.)
Review request for mesos, haosdent huang and Niklas Nielsen.
Bugs: MESOS-2807
https://issues.apache.org/jira/browse/MESOS-2807
Repository: mesos
Description
---
Jira: MESOS-2340
This is a preliminary step to enabling JSON API
for Master Discovery via Zookeeper.
We currently save the MasterInfo PB to ZK
serializing directly the binary data, so that
for clients to retrieve that information, they
need to either link up with libmesos or
obtain the PB definition (in mesos/mesos.proto).
This change only provides the (de)serialization
utility methods and associated tests.
Diffs
-
src/Makefile.am f2110a360b6e2a1e99e1d9630d5dfacfd818530a
src/common/http.hpp afce7fea334c7bfa57efc48c413bf906a2ebde32
src/common/http.cpp 2ac7fba7a3aac913540f1b09768777393b79284a
src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098
src/tests/common/http_tests.cpp f087b2313a13c3199b70b3d7feb728e1449a52e7
src/tests/common/parse_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/34687/diff/
Testing
---
make check
Thanks,
Marco Massenzio
Re: Review Request 35260: Refactored the ResourceMonitor to get statistics from the Slave.
On June 11, 2015, 4:18 p.m., Niklas Nielsen wrote: include/mesos/mesos.proto, line 626 https://reviews.apache.org/r/35260/diff/3/?file=982288#file982288line626 I just thought of one thing - this represents the entire container and not the executor alone. Does it make sense to call it 'Container' instead? Vinod Kone wrote: +1 OK, This is not consistent in the code base. We use Executor in master and slave to represent the entire executor (executor itself + its tasks). For example, in master, we have executor-addTask. In slave, we have executor-resources which contains resources not only for itself, but also its tasks. I prefer the name 'Executor' for consistency (e.g., executorLaunched, executorTerminated, etc.). @bmahler - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35260/#review87571 --- On June 10, 2015, 11:13 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35260/ --- (Updated June 10, 2015, 11:13 p.m.) Review request for mesos, Ben Mahler, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2818 https://issues.apache.org/jira/browse/MESOS-2818 Repository: mesos Description --- Refactored the ResourceMonitor to get statistics from the Slave. 1) Modified ResourceUsage to include allocation information (see ticket for reaons). 2) Simplied the ResourceMonitor by taking a lambda from the slave to get statistics. 3) Adjusted (and simplified) the MonitorTest accordingly. Diffs - include/mesos/mesos.proto 7457ff11f6a55099ccb95beb2f0ccb9a2f7ccd87 include/mesos/slave/resource_estimator.hpp 7f78fd86760397d5b885a2c00b41081d0b546cdd src/slave/monitor.hpp bee91ba143c26059fc8badf56beccb68c6556cb7 src/slave/monitor.cpp 8f7ff63b4daf0286d4cf912e2f3d2d1b68caab1c src/slave/resource_estimators/fixed.cpp 3efa18d7e3c6ac62e67f75ea45a832f3f6349036 src/slave/resource_estimators/noop.hpp 7a44e6deefc9c1985c14d076a427aa5c654aa1bb src/slave/resource_estimators/noop.cpp 5f135ff37e84c248ede29bbe4a7d1b8319417e20 src/slave/slave.hpp 98c64f62f60c286e42379b2151321b190c4410b8 src/slave/slave.cpp b9fb929684ae582b9133126fefa5a06d4181d836 src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 src/tests/monitor_tests.cpp 6de8b1f65843fd7b852dfa69627a1c435b482fe0 Diff: https://reviews.apache.org/r/35260/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 35260: Refactored the ResourceMonitor to get statistics from the Slave.
On June 11, 2015, 4:18 p.m., Niklas Nielsen wrote: include/mesos/mesos.proto, line 626 https://reviews.apache.org/r/35260/diff/3/?file=982288#file982288line626 I just thought of one thing - this represents the entire container and not the executor alone. Does it make sense to call it 'Container' instead? Vinod Kone wrote: +1 Jie Yu wrote: OK, This is not consistent in the code base. We use Executor in master and slave to represent the entire executor (executor itself + its tasks). For example, in master, we have executor-addTask. In slave, we have executor-resources which contains resources not only for itself, but also its tasks. I prefer the name 'Executor' for consistency (e.g., executorLaunched, executorTerminated, etc.). @bmahler kk. looks like we call just the executor as ExecutorInfo in our protobufs. sgtm. - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35260/#review87571 --- On June 10, 2015, 11:13 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35260/ --- (Updated June 10, 2015, 11:13 p.m.) Review request for mesos, Ben Mahler, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2818 https://issues.apache.org/jira/browse/MESOS-2818 Repository: mesos Description --- Refactored the ResourceMonitor to get statistics from the Slave. 1) Modified ResourceUsage to include allocation information (see ticket for reaons). 2) Simplied the ResourceMonitor by taking a lambda from the slave to get statistics. 3) Adjusted (and simplified) the MonitorTest accordingly. Diffs - include/mesos/mesos.proto 7457ff11f6a55099ccb95beb2f0ccb9a2f7ccd87 include/mesos/slave/resource_estimator.hpp 7f78fd86760397d5b885a2c00b41081d0b546cdd src/slave/monitor.hpp bee91ba143c26059fc8badf56beccb68c6556cb7 src/slave/monitor.cpp 8f7ff63b4daf0286d4cf912e2f3d2d1b68caab1c src/slave/resource_estimators/fixed.cpp 3efa18d7e3c6ac62e67f75ea45a832f3f6349036 src/slave/resource_estimators/noop.hpp 7a44e6deefc9c1985c14d076a427aa5c654aa1bb src/slave/resource_estimators/noop.cpp 5f135ff37e84c248ede29bbe4a7d1b8319417e20 src/slave/slave.hpp 98c64f62f60c286e42379b2151321b190c4410b8 src/slave/slave.cpp b9fb929684ae582b9133126fefa5a06d4181d836 src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 src/tests/monitor_tests.cpp 6de8b1f65843fd7b852dfa69627a1c435b482fe0 Diff: https://reviews.apache.org/r/35260/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 35157: Added unit tests for fetching ResourceUsage in both QoS Controller and Resource Estimator .
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35157/#review87587
---
src/common/type_utils.cpp
https://reviews.apache.org/r/35157/#comment139978
This doesn't match all fields in include/mesos/mesos.proto. Mind adding
those?
src/tests/oversubscription_tests.cpp
https://reviews.apache.org/r/35157/#comment139981
s/that /that the/
src/tests/oversubscription_tests.cpp
https://reviews.apache.org/r/35157/#comment139982
s/from /from the/
src/tests/oversubscription_tests.cpp
https://reviews.apache.org/r/35157/#comment139983
Can you do this with an initializer list?
```
vectorTaskInfo tasks = {task};
```
src/tests/oversubscription_tests.cpp
https://reviews.apache.org/r/35157/#comment139984
Don't you want to ensure that the status update is actually TASK_RUNNING? :)
src/tests/oversubscription_tests.cpp
https://reviews.apache.org/r/35157/#comment139980
Let's not do these fly by changes :)
src/tests/oversubscription_tests.cpp
https://reviews.apache.org/r/35157/#comment139985
s/that /that the/
src/tests/oversubscription_tests.cpp
https://reviews.apache.org/r/35157/#comment139986
s/from /from the /
src/tests/oversubscription_tests.cpp
https://reviews.apache.org/r/35157/#comment139987
We need to update this to match the test above
- Niklas Nielsen
On June 8, 2015, 5:17 p.m., Bartek Plotka wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35157/
---
(Updated June 8, 2015, 5:17 p.m.)
Review request for mesos, Ben Mahler, Jie Yu, Niklas Nielsen, Szymon Konefal,
and Vinod Kone.
Repository: mesos
Description
---
Added tests for QoS Controller and ResourceEstimator fetching ResoruceUsage
from ResourceMonitor.
This is the unit test for https://reviews.apache.org/r/34980/ and
https://reviews.apache.org/r/35164/
Diffs
-
include/mesos/type_utils.hpp 52380c2461841026ee492797b4d8081f944f7b7b
src/common/type_utils.cpp e92f6f36de0955784619029a016667b46bbe221b
src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417
src/tests/mesos.cpp dff45b0d3bf9ef53f19575ab3d90a0b223755d6a
src/tests/oversubscription_tests.cpp
afd7ff4f2b50cb20cc2c8865b655ad1f8eb0c8b7
Diff: https://reviews.apache.org/r/35157/diff/
Testing
---
make check
Thanks,
Bartek Plotka
Re: Review Request 35260: Refactored the ResourceMonitor to get statistics from the Slave.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35260/#review87592 --- Ship it! include/mesos/mesos.proto https://reviews.apache.org/r/35260/#comment139991 Does this represent the total resources allocated for the container (executor + tasks) or only tasks? If the first; then let's update this comment src/slave/monitor.cpp https://reviews.apache.org/r/35260/#comment139993 C-style headers should be before C++ - take a look at zookeeper.cpp src/slave/slave.cpp https://reviews.apache.org/r/35260/#comment139994 I am not sure about lambda wrappings, but shouldn't it as least be 4 indent taken a argument wrapping? Can we prevent that wrapping by wrapping a bit earlier? src/tests/monitor_tests.cpp https://reviews.apache.org/r/35260/#comment139995 What do you need to copy in this lambda scope? I just saw a comment from BenH about preferring being explicit on bringing the environment in. The style guide suggests that you can do either; I will let that be up to you. - Niklas Nielsen On June 10, 2015, 4:13 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35260/ --- (Updated June 10, 2015, 4:13 p.m.) Review request for mesos, Ben Mahler, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2818 https://issues.apache.org/jira/browse/MESOS-2818 Repository: mesos Description --- Refactored the ResourceMonitor to get statistics from the Slave. 1) Modified ResourceUsage to include allocation information (see ticket for reaons). 2) Simplied the ResourceMonitor by taking a lambda from the slave to get statistics. 3) Adjusted (and simplified) the MonitorTest accordingly. Diffs - include/mesos/mesos.proto 7457ff11f6a55099ccb95beb2f0ccb9a2f7ccd87 include/mesos/slave/resource_estimator.hpp 7f78fd86760397d5b885a2c00b41081d0b546cdd src/slave/monitor.hpp bee91ba143c26059fc8badf56beccb68c6556cb7 src/slave/monitor.cpp 8f7ff63b4daf0286d4cf912e2f3d2d1b68caab1c src/slave/resource_estimators/fixed.cpp 3efa18d7e3c6ac62e67f75ea45a832f3f6349036 src/slave/resource_estimators/noop.hpp 7a44e6deefc9c1985c14d076a427aa5c654aa1bb src/slave/resource_estimators/noop.cpp 5f135ff37e84c248ede29bbe4a7d1b8319417e20 src/slave/slave.hpp 98c64f62f60c286e42379b2151321b190c4410b8 src/slave/slave.cpp b9fb929684ae582b9133126fefa5a06d4181d836 src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 src/tests/monitor_tests.cpp 6de8b1f65843fd7b852dfa69627a1c435b482fe0 Diff: https://reviews.apache.org/r/35260/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 35260: Refactored the ResourceMonitor to get statistics from the Slave.
On June 11, 2015, 6:12 p.m., Niklas Nielsen wrote: src/slave/monitor.cpp, line 23 https://reviews.apache.org/r/35260/diff/3/?file=982291#file982291line23 C-style headers should be before C++ - take a look at zookeeper.cpp I think glog/logging.h is a C++ header (google never uses hpp I remember) :) I'll fix the zookeeper.cpp On June 11, 2015, 6:12 p.m., Niklas Nielsen wrote: src/slave/slave.cpp, lines 4218-4219 https://reviews.apache.org/r/35260/diff/3/?file=982296#file982296line4218 I am not sure about lambda wrappings, but shouldn't it as least be 4 indent taken a argument wrapping? Can we prevent that wrapping by wrapping a bit earlier? Restructured. - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35260/#review87592 --- On June 10, 2015, 11:13 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35260/ --- (Updated June 10, 2015, 11:13 p.m.) Review request for mesos, Ben Mahler, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2818 https://issues.apache.org/jira/browse/MESOS-2818 Repository: mesos Description --- Refactored the ResourceMonitor to get statistics from the Slave. 1) Modified ResourceUsage to include allocation information (see ticket for reaons). 2) Simplied the ResourceMonitor by taking a lambda from the slave to get statistics. 3) Adjusted (and simplified) the MonitorTest accordingly. Diffs - include/mesos/mesos.proto 7457ff11f6a55099ccb95beb2f0ccb9a2f7ccd87 include/mesos/slave/resource_estimator.hpp 7f78fd86760397d5b885a2c00b41081d0b546cdd src/slave/monitor.hpp bee91ba143c26059fc8badf56beccb68c6556cb7 src/slave/monitor.cpp 8f7ff63b4daf0286d4cf912e2f3d2d1b68caab1c src/slave/resource_estimators/fixed.cpp 3efa18d7e3c6ac62e67f75ea45a832f3f6349036 src/slave/resource_estimators/noop.hpp 7a44e6deefc9c1985c14d076a427aa5c654aa1bb src/slave/resource_estimators/noop.cpp 5f135ff37e84c248ede29bbe4a7d1b8319417e20 src/slave/slave.hpp 98c64f62f60c286e42379b2151321b190c4410b8 src/slave/slave.cpp b9fb929684ae582b9133126fefa5a06d4181d836 src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 src/tests/monitor_tests.cpp 6de8b1f65843fd7b852dfa69627a1c435b482fe0 Diff: https://reviews.apache.org/r/35260/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 35264: Added a slave integration test in MonitorTest.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35264/#review87599 --- Ship it! src/tests/monitor_tests.cpp https://reviews.apache.org/r/35264/#comment139998 C-style include before C++ includes :) src/tests/monitor_tests.cpp https://reviews.apache.org/r/35264/#comment13 'Executor' or 'Task' ? - Niklas Nielsen On June 10, 2015, 4:13 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35264/ --- (Updated June 10, 2015, 4:13 p.m.) Review request for mesos, Ben Mahler, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2818 https://issues.apache.org/jira/browse/MESOS-2818 Repository: mesos Description --- Added a slave integration test in MonitorTest. This test verifies the wiring between resource monitor and the slave. Diffs - src/tests/monitor_tests.cpp 6de8b1f65843fd7b852dfa69627a1c435b482fe0 Diff: https://reviews.apache.org/r/35264/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 35280: Added Test QoS Controller module
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35280/ --- (Updated June 11, 2015, 11:55 a.m.) Review request for mesos, Ben Mahler, Bartek Plotka, and Jie Yu. Bugs: MESOS-2703 https://issues.apache.org/jira/browse/MESOS-2703 Repository: mesos Description --- See summary. Diffs (updated) - src/Makefile.am 9c6b52a9991a6bebb759e6a307c0593180507477 src/examples/test_qos_controller_module.cpp PRE-CREATION Diff: https://reviews.apache.org/r/35280/diff/ Testing --- make check. Thanks, Niklas Nielsen
Re: Review Request 35279: Added QoS Controller module
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35279/ --- (Updated June 11, 2015, 11:55 a.m.) Review request for mesos, Ben Mahler, Bartek Plotka, and Jie Yu. Bugs: MESOS-2703 https://issues.apache.org/jira/browse/MESOS-2703 Repository: mesos Description --- See summary. Diffs (updated) - include/mesos/module/qos_controller.hpp PRE-CREATION src/Makefile.am 9c6b52a9991a6bebb759e6a307c0593180507477 src/module/manager.cpp c6de654ec6188665f20bf75f079bc95f378b6a3a Diff: https://reviews.apache.org/r/35279/diff/ Testing --- make check Thanks, Niklas Nielsen
Re: Review Request 35278: Added missing flag initialization for qos controller.
On June 10, 2015, 6:32 a.m., Bartek Plotka wrote: src/slave/flags.cpp, line 519 https://reviews.apache.org/r/35278/diff/1/?file=981788#file981788line519 Could you change s/resource estimator/Resource Estimator/ to be compliant with QoS Controller name? If you are Upper casing names, you can do it for RE as well.. (: Thanks Bartek! I would prefer to go this in a subsequent review (if we agree on it). Let's create a ticket for now, to track it and unify the style. - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35278/#review87401 --- On June 9, 2015, 6:09 p.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35278/ --- (Updated June 9, 2015, 6:09 p.m.) Review request for mesos, Ben Mahler, Bartek Plotka, and Jie Yu. Bugs: MESOS-2703 https://issues.apache.org/jira/browse/MESOS-2703 Repository: mesos Description --- Missed this initialization during a rebase during the qos controller patch set. Diffs - src/slave/flags.cpp 99142fb1cf9d1978df86fdbf612e656b8fe852a6 Diff: https://reviews.apache.org/r/35278/diff/ Testing --- make check Ran ./bin/mesos-slave.sh --help and confirmed flag appearing. Thanks, Niklas Nielsen
Re: Review Request 35281: Added QoS module loader to ::create() factory.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35281/#review87610 --- Patch looks great! Reviews applied: [35278, 35279, 35280, 35281] All tests passed. - Mesos ReviewBot On June 11, 2015, 6:55 p.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35281/ --- (Updated June 11, 2015, 6:55 p.m.) Review request for mesos, Ben Mahler, Bartek Plotka, and Jie Yu. Bugs: MESOS-2703 https://issues.apache.org/jira/browse/MESOS-2703 Repository: mesos Description --- See summary. Diffs - src/slave/qos_controller.cpp 81c4b3e658902be0438f42d9e86911e424828a73 Diff: https://reviews.apache.org/r/35281/diff/ Testing --- make check Tested new example module manually, since we don't have a good type param'ed test for resource estimator and qos controller yet. Thanks, Niklas Nielsen
Re: Review Request 35157: Added unit tests for fetching ResourceUsage in both QoS Controller and Resource Estimator .
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35157/#review87609 --- src/common/type_utils.cpp https://reviews.apache.org/r/35157/#comment140005 Wow. This will become very hard to maintian (i.e., keep up-to-date) as we introduce more and more statistics (e.g., https://reviews.apache.org/r/34894). So could you please serialize the protobuf to string and test string equality since you want the strict equality here. src/tests/oversubscription_tests.cpp https://reviews.apache.org/r/35157/#comment140006 Can you align FutureSatisfy and Return? Here and everywhere else. - Jie Yu On June 11, 2015, 6:55 p.m., Bartek Plotka wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35157/ --- (Updated June 11, 2015, 6:55 p.m.) Review request for mesos, Ben Mahler, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone. Repository: mesos Description --- Added tests for QoS Controller and ResourceEstimator fetching ResoruceUsage from ResourceMonitor. This is the unit test for https://reviews.apache.org/r/34980/ and https://reviews.apache.org/r/35164/ Diffs - include/mesos/type_utils.hpp 52380c2461841026ee492797b4d8081f944f7b7b src/common/type_utils.cpp e92f6f36de0955784619029a016667b46bbe221b src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 src/tests/mesos.cpp dff45b0d3bf9ef53f19575ab3d90a0b223755d6a src/tests/oversubscription_tests.cpp afd7ff4f2b50cb20cc2c8865b655ad1f8eb0c8b7 Diff: https://reviews.apache.org/r/35157/diff/ Testing --- make check Thanks, Bartek Plotka
Re: Review Request 35157: Added unit tests for fetching ResourceUsage in both QoS Controller and Resource Estimator .
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35157/#review87613 --- Bad patch! Reviews applied: [35164, 35157] Failed command: ./support/apply-review.sh -n -r 35157 Error: 2015-06-11 19:41:24 URL:https://reviews.apache.org/r/35157/diff/raw/ [18101/18101] - 35157.patch [1] error: patch failed: src/tests/oversubscription_tests.cpp:63 error: src/tests/oversubscription_tests.cpp: patch does not apply Failed to apply patch - Mesos ReviewBot On June 11, 2015, 6:55 p.m., Bartek Plotka wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35157/ --- (Updated June 11, 2015, 6:55 p.m.) Review request for mesos, Ben Mahler, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod Kone. Repository: mesos Description --- Added tests for QoS Controller and ResourceEstimator fetching ResoruceUsage from ResourceMonitor. This is the unit test for https://reviews.apache.org/r/34980/ and https://reviews.apache.org/r/35164/ Diffs - include/mesos/type_utils.hpp 52380c2461841026ee492797b4d8081f944f7b7b src/common/type_utils.cpp e92f6f36de0955784619029a016667b46bbe221b src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 src/tests/mesos.cpp dff45b0d3bf9ef53f19575ab3d90a0b223755d6a src/tests/oversubscription_tests.cpp afd7ff4f2b50cb20cc2c8865b655ad1f8eb0c8b7 Diff: https://reviews.apache.org/r/35157/diff/ Testing --- make check Thanks, Bartek Plotka
Re: Review Request 35247: Fixed race between EXPECT_CALL(resourceOffers, _) and driver.start() in fetcher_cache_tests.cpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35247/#review87616 --- I'll commit this for now with the reordering fix. Please follow up with the proper fix as discussed. - Vinod Kone On June 9, 2015, 9:32 a.m., Bernd Mathiske wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35247/ --- (Updated June 9, 2015, 9:32 a.m.) Review request for mesos, Benjamin Hindman, Till Toenshoff, and Vinod Kone. Bugs: MESOS-2815, MESOS-2829 and MESOS-2831 https://issues.apache.org/jira/browse/MESOS-2815 https://issues.apache.org/jira/browse/MESOS-2829 https://issues.apache.org/jira/browse/MESOS-2831 Repository: mesos Description --- Fixed race between EXPECT_CALL(resourceOffers, _) and driver.start() in fetcher_cache_tests.cpp. Installed a default response that provides teporary cover for this mocked method until we install more interesting callbacks later. This prevents gmock from complaining about an uninteresting gmock call, which led to a variety of tests failing due to offers not getting through subsequently. All fetcher cache tests have been affected by this race and should be fixed in this regard now. (Also fixed some typos.) Diffs - src/tests/fetcher_cache_tests.cpp cbd44b98d19953d174fac977f509d4900a37481f Diff: https://reviews.apache.org/r/35247/diff/ Testing --- make check Thanks, Bernd Mathiske
Re: Review Request 34943: Added validation to flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34943/ --- (Updated June 11, 2015, 1:52 p.m.) Review request for mesos. Repository: mesos Description --- Also refactored existing 'lambda::bind' arguments to use C++11 lambdas, enabling us to get rid of our loader and stringifier functors. Diffs (updated) - 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 6ac2f04a6a1d8db1725972a3b4b46a0dd734566f 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp 87606d860dea3235ec70d127d3379065d42dc90b 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp ee855da6128c2d671eb2fc7eaa2c0aab2341aebb 3rdparty/libprocess/3rdparty/stout/include/stout/flags/loader.hpp 51d3ab020bd2bae1f12b66cac0da5383c813d5d7 3rdparty/libprocess/3rdparty/stout/include/stout/flags/stringifier.hpp fda5ae1328a23a4371475652f921341dce7448d5 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 95b4b33b70c37640d97dff5d5724550d42048b76 3rdparty/libprocess/3rdparty/stout/include/stout/os/open.hpp 134453e91d39fd086baa9396ab42a002fed8b73e 3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp 59cde89b5e035807a510b331b85a8cd48da36ae3 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 4485e41feb8320d2ec7251b8b894ce437f61addd 3rdparty/libprocess/3rdparty/stout/include/stout/os/sunos.hpp cfe6d742560f50079fb1ed7346526d432615613c 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 80450185f60c5b273face490e0bb9e695b0cb984 3rdparty/libprocess/include/Makefile.am d01880c66ca544d62d3ada1ea79c8045884858da 3rdparty/libprocess/include/process/firewall.hpp 16ed852d07344e53c6f9d30d4cd7d99e07c6606d 3rdparty/libprocess/include/process/process.hpp e70dd388601bd26fcc82f66e0cb95924da8a8c2f 3rdparty/libprocess/src/process.cpp aadd7bb0ae12b93336900c76d8d5aaa4421ea198 3rdparty/libprocess/src/tests/process_tests.cpp 660af45e7fd45bdf5d43ad9aa54477fd94f87058 Doxyfile 1b0a2beda1d9223eafd9d14d335c651eb4961a15 docs/configuration.md aaf65bfa2848318c8d07c772ba2c521aa72c2677 docs/engineering-principles-and-practices.md 6babb929ead758ee5187d5fc5760d084876c3298 docs/home.md ee3d3e9a417d75af776dad3ea9627d3c6d780dca docs/mesos-c++-style-guide.md 94107ed21c6f09349ce691f9f4d36b43bbbe809e docs/powered-by-mesos.md 0ee763ffe32e6667e4ba708dd275d9f1b5116815 include/mesos/containerizer/containerizer.proto f16ccc89f83da28c413ccfa0687a06b7515a605c include/mesos/master/allocator.hpp 92de1af414321281b00eaa6f129e5e3e2c849448 include/mesos/mesos.proto 7457ff11f6a55099ccb95beb2f0ccb9a2f7ccd87 src/Makefile.am 10b190288425d735b116ed2dd3f040e871c5cb18 src/cli/mesos-ps ae48c8cde31e90024381ed3e6e22ec3bc96560b6 src/examples/no_executor_framework.cpp 6c5c7d3121e3a2ea78f66dffdeaecb72cca6293c src/files/files.cpp 3cdd38a3c7122bd5e13c8928279d85ab1373a63e src/linux/routing/handle.hpp 052c7cc1a967797d245a275d08cc774f627398a5 src/linux/routing/queueing/fq_codel.hpp 18c53b416718c27045986d939bb85f9fec731ca5 src/linux/routing/queueing/fq_codel.cpp 446863391d1c6dd2f91f890596927a20ee59a35f src/main.dox d5b29cf96b18ba84ac39f1747c4a174a5bfbc2e1 src/master/allocator/mesos/allocator.hpp 6cfa04650d91a80211cfbc0809236f9438926c78 src/master/allocator/mesos/hierarchical.hpp c220ea673320d4d06403afa76f0eb9f3b7a4e5eb src/master/flags.hpp 55ed3a9ef84a39841305cca2ba6c5df45c1990e1 src/master/flags.cpp 4377715029878cfee36f067e9c53c42b522b79d8 src/master/main.cpp 1c33e3bf7baae45b1671f9566b4993f39d9f1294 src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 src/master/master.cpp e91e8815471b49d3c649470197d6da3f06de62fb src/master/metrics.hpp 3d389e68f22e3d1e00bde0db4e25f897c79a2316 src/master/metrics.cpp d2489c8897fb2f2f21f021ac5e7a2ada7997ea06 src/messages/flags.hpp 41be419ba5593a600aa0c6c411210fa4faa829a8 src/messages/flags.proto 5400c92297f252734789982d21d7117ef4c57a57 src/slave/containerizer/isolators/network/port_mapping.cpp d2da1a4e96baeac7d1af9a5468f90c2e4c1cb50f src/slave/flags.hpp 6c24e56d15881b0e3aeec3d4824842cf57121fc6 src/slave/flags.cpp 99142fb1cf9d1978df86fdbf612e656b8fe852a6 src/slave/main.cpp c379243e01919a5ab30bb9dea1b738665ba4e746 src/slave/slave.cpp 98036b2d5f2c765aef4a416c3cbc082df77ab3ac src/tests/master_contender_detector_tests.cpp af6f15a8a85f2a292f5a64286aaa986f7dc29eff src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 src/tests/no_executor_framework_test.sh b5bb111ceb99d4dc836537516de57c1ba0582371 src/tests/oversubscription_tests.cpp 59cf07ef93460537ce1343793fd4a5d11d2ae242 src/tests/routing_tests.cpp 4be9967d38177d9f7def5c0da98d4c4266e7f0b5 src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 Diff: https://reviews.apache.org/r/34943/diff/ Testing --- make check
Re: Review Request 35350: Fixes markdown of the Doxygen Styleguide to provide better rendering.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35350/#review87556 --- Patch looks great! Reviews applied: [35350] All tests passed. - Mesos ReviewBot On June 11, 2015, 11:59 a.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35350/ --- (Updated June 11, 2015, 11:59 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Joerg Schad. Repository: mesos Description --- See summary. Diffs - docs/mesos-doxygen-style-guide.md 93decf9984a8ebf5c713b968da53fc38caabeab0 Diff: https://reviews.apache.org/r/35350/diff/ Testing --- Thanks, Alexander Rojas
Re: Review Request 33208: Delete detector in MesosSchedulerDriver::stop
On April 19, 2015, 10:55 p.m., Adam B wrote: LGTM, barring a question about ordering/synchronization. I'll let another committer take a look before we commit it. Adam B wrote: Would also like to see a successful ReviewBot pass. That MasterFailover segfault seems like it could be related to your detector changes. Niklas Nielsen wrote: Hey Robert, do you need some help on this patch? Niklas Nielsen wrote: Ping :) Let's get some cadence behind this, or drop it (and reopen when we can work on this again). Alright - I think I understand the problem. In the test cases, the owned pointer will get double deleted. Let's iterate on how to address this. - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33208/#review80655 --- On April 14, 2015, 10:23 p.m., Robert Lacroix wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33208/ --- (Updated April 14, 2015, 10:23 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-1634 https://issues.apache.org/jira/browse/MESOS-1634 Repository: mesos Description --- When the Mesos Scheduler Driver stops, it does not delete the detector process before the object is garbage collected, which leaves ZooKeeper connections hanging around unnecessarily. This deletes the process on stop as well, not only on destruction. Diffs - src/sched/sched.cpp 66fd2b3146568900356cc48e0f17c6720665ae80 Diff: https://reviews.apache.org/r/33208/diff/ Testing --- make check, manual testing Thanks, Robert Lacroix
Re: Review Request 35350: Fixes markdown of the Doxygen Styleguide to provide better rendering.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35350/#review87623 --- This should be covered by https://reviews.apache.org/r/35364/. - Joerg Schad On June 11, 2015, 11:59 a.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35350/ --- (Updated June 11, 2015, 11:59 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Joerg Schad. Repository: mesos Description --- See summary. Diffs - docs/mesos-doxygen-style-guide.md 93decf9984a8ebf5c713b968da53fc38caabeab0 Diff: https://reviews.apache.org/r/35350/diff/ Testing --- Thanks, Alexander Rojas
Re: Review Request 35367: Changed Resourcs JSON model() to combine non-revocable resources and ignore revocable resources.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35367/#review87624 --- Patch looks great! Reviews applied: [35367] All tests passed. - Mesos ReviewBot On June 11, 2015, 7:56 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35367/ --- (Updated June 11, 2015, 7:56 p.m.) Review request for mesos, Niklas Nielsen and Vinod Kone. Bugs: MESOS-2838 https://issues.apache.org/jira/browse/MESOS-2838 Repository: mesos Description --- Combine means taking the sum of scalars or coalescing ranges and sets of all resources of the same name regardless of their other attributes. Diffs - include/mesos/resources.hpp 7da6e6e8bc1fa1d62fc8be2b5169b68e10392b73 src/common/http.cpp 2ac7fba7a3aac913540f1b09768777393b79284a src/common/resources.cpp 05969199889c6bc1421c8e5be10c74a50660719f src/tests/common/http_tests.cpp f087b2313a13c3199b70b3d7feb728e1449a52e7 Diff: https://reviews.apache.org/r/35367/diff/ Testing --- make check. Added a test `HTTP.ModelResources` for this. Thanks, Jiang Yan Xu
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/ --- (Updated June 11, 2015, 9:39 p.m.) Review request for mesos and Timothy Chen. Bugs: MESOS-2797 https://issues.apache.org/jira/browse/MESOS-2797 Repository: mesos Description --- Capped number of parallel inspect instances on a docker ps call. Diffs (updated) - src/docker/docker.hpp 7790d0ff9a6b085025f595533c5f46b702447927 src/docker/docker.cpp 71383294c6234d08b2156565b170ada329b8e30f src/slave/constants.hpp 84927e589499e989249c217db571bbeb84a88af1 Diff: https://reviews.apache.org/r/35330/diff/ Testing (updated) --- sudo ./mesos-tests.sh --gtest_filter=*DOCKER* --gtest_break_on_failure --gtest_shuffle --gtest_repeat=30 Thanks, Lily Chen
Re: Review Request 34361: converted hard-coded strings to consts
On June 9, 2015, 6:25 p.m., Ben Mahler wrote: src/tests/master_tests.cpp, lines 3031-3034 https://reviews.apache.org/r/34361/diff/3/?file=971359#file971359line3031 Why bother with all this? Why not just have `key1`, `value1`, `key2`, `value2` inlined appropriately throughout these tests. Very straightforward to read. Colin Williams wrote: I think the issue with the changes remaining is that the test depends on the same value occurring in several places. By consolidating to a variable it's no longer possible for these values to get out of sync. Niklas Nielsen wrote: Colin: exactly Ben: We have label tests three places; in the master, slave and in the modules and they use the same foo, bar, baz, qux key value pairs. The idea was to centralize them, so they don't get out of date and to avoid code duplication. Does that make sense? Ben Mahler wrote: Well, then let's just keep it simple and get rid of these special strings by just making the tests use key1, value1, key2, value2, etc. I'm not following your code duplication point, this introduces more code (now there are additional global constants, which we like to avoid if unnecessary). Also, each test is ideally independent, why does the master's test for labels affect the slave's test for labels? Let's say I need a test with 5 labels, you want to have 5x2=10 global constants to address this? Niklas Nielsen wrote: Try to see how testLabelKey and testLabelValue was used previously and foo, bar, qux was used on others and I created this ticket to unify the way we do this, along with seeing these key value pair being created in the slave and master tests. Dispite the current patch, I do think we can simply and unify the test label creation. Maybe create a CREATE_LABEL_PAIR(id) to make it more obvious which key pairs are being tested. The comments in the test code need to carry a bunch of context, to make sense out of the label transformations (especially across the library border for the hooks tests). This is all in spirit of reducing the tech debt we introduced. We are on the same page on not introducing more lines/key pairs than before. Let us iterate on this and get back to you. Colin Williams wrote: Ben: I'm more in agreement with your sentiment, I'm not sure I see the point of unifying label names into a centralized variable that aren't at all related. Niklas: I was a little confused by the original task description, so I thought a sample patch would be a good discussion point. I don't entirely understand the tech debt you're trying to solve. For example, it seems like you're asking to have some constants defined somewhere that are used by both master_test and slave_test, but the the similarities between these two only seem incidental. I'm concerned that creating something like CREATE_LABEL_PAIR(id) would instead obfuscate the intent of the code. Okay - thinking about this; I am on board with key1, value1 etc. Colin: the tech debt is that we have some inlined constants (like foo, bar, etc) and some which are constants (which have to be kept in sync between hooks modules and test body). The idea was to unify the way we name the key value pairs. Do you want to update this ticket to reflect this, or come with a new patch set which tackles streaming the two? - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34361/#review87331 --- On June 8, 2015, 12:05 p.m., Colin Williams wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34361/ --- (Updated June 8, 2015, 12:05 p.m.) Review request for mesos and Niklas Nielsen. Bugs: MESOS-2637 https://issues.apache.org/jira/browse/MESOS-2637 Repository: mesos Description --- converted hard-coded strings to consts Diffs - src/tests/hook_tests.cpp 3ffde6d src/tests/master_tests.cpp ba3858f src/tests/slave_tests.cpp acae497 Diff: https://reviews.apache.org/r/34361/diff/ Testing --- Thanks, Colin Williams
Re: Review Request 35367: Changed Resourcs JSON model() to combine non-revocable resources and ignore revocable resources.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35367/#review87649 --- src/common/http.cpp https://reviews.apache.org/r/35367/#comment140058 s/nonrevocable/nonRevocable/ src/common/resources.cpp https://reviews.apache.org/r/35367/#comment140053 add a test for this in resources_tests.cpp ? src/tests/common/http_tests.cpp https://reviews.apache.org/r/35367/#comment140057 s/nonrevocable/nonRevocable/ src/tests/common/http_tests.cpp https://reviews.apache.org/r/35367/#comment140054 camelcase? - Vinod Kone On June 11, 2015, 7:56 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35367/ --- (Updated June 11, 2015, 7:56 p.m.) Review request for mesos, Niklas Nielsen and Vinod Kone. Bugs: MESOS-2838 https://issues.apache.org/jira/browse/MESOS-2838 Repository: mesos Description --- Combine means taking the sum of scalars or coalescing ranges and sets of all resources of the same name regardless of their other attributes. Diffs - include/mesos/resources.hpp 7da6e6e8bc1fa1d62fc8be2b5169b68e10392b73 src/common/http.cpp 2ac7fba7a3aac913540f1b09768777393b79284a src/common/resources.cpp 05969199889c6bc1421c8e5be10c74a50660719f src/tests/common/http_tests.cpp f087b2313a13c3199b70b3d7feb728e1449a52e7 Diff: https://reviews.apache.org/r/35367/diff/ Testing --- make check. Added a test `HTTP.ModelResources` for this. Thanks, Jiang Yan Xu
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 11, 2015, 3:58 p.m.) Review request for mesos, Bartek Plotka, Jie Yu, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2653 https://issues.apache.org/jira/browse/MESOS-2653 Repository: mesos Description --- See summary Diffs (updated) - src/tests/mesos.hpp e19ef984f9e4696bd405027d6f19756cf23d0df2 src/tests/mesos.cpp 5e574c5aa79d548e5730db021b0e8b77d27f220b src/tests/oversubscription_tests.cpp e8ae053dd9cd712e49bd2830e414b7a3d99c20ca Diff: https://reviews.apache.org/r/34721/diff/ Testing --- make check (with new qos test) Thanks, Niklas Nielsen
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 11, 2015, 3:58 p.m.) Review request for mesos, Bartek Plotka, Jie Yu, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2653 https://issues.apache.org/jira/browse/MESOS-2653 Repository: mesos Description --- See summary Diffs (updated) - src/slave/slave.hpp 0df1b55791963fb4159b7ea5318d09dde4f7d8c7 src/slave/slave.cpp b523c2fce50e56f4f94d55a9488f49c53452e4d4 Diff: https://reviews.apache.org/r/34720/diff/ Testing --- make check Thanks, Niklas Nielsen
Re: Review Request 34719: Added QOS_KILLED as status reason
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34719/ --- (Updated June 11, 2015, 3:58 p.m.) Review request for mesos, Bartek Plotka, Jie Yu, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2653 https://issues.apache.org/jira/browse/MESOS-2653 Repository: mesos Description --- See summary Diffs (updated) - include/mesos/mesos.proto 20340099dc87b3ea7f375c50fa3848ab5b0d5134 Diff: https://reviews.apache.org/r/34719/diff/ Testing --- make check Thanks, Niklas Nielsen
Re: Review Request 35363: Improvements on libprocess/README.md.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35363/#review87641 --- Patch looks great! Reviews applied: [35363] All tests passed. - Mesos ReviewBot On June 11, 2015, 8:40 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35363/ --- (Updated June 11, 2015, 8:40 p.m.) Review request for mesos and Bernd Mathiske. Bugs: MESOS-2545 https://issues.apache.org/jira/browse/MESOS-2545 Repository: mesos Description --- Improvements on libprocess/README.md. Diffs - 3rdparty/libprocess/README.md 4dcd15042dd0b7105c903115cbe8875c3159365e Diff: https://reviews.apache.org/r/35363/diff/ Testing --- Rendered version: https://gist.github.com/joerg84/a6c5fd399bef8cb75b0d Thanks, Joerg Schad
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/#review87645 --- Patch looks great! Reviews applied: [35330] All tests passed. - Mesos ReviewBot On June 11, 2015, 9:39 p.m., Lily Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35330/ --- (Updated June 11, 2015, 9:39 p.m.) Review request for mesos and Timothy Chen. Bugs: MESOS-2797 https://issues.apache.org/jira/browse/MESOS-2797 Repository: mesos Description --- Capped number of parallel inspect instances on a docker ps call. Diffs - src/docker/docker.hpp 7790d0ff9a6b085025f595533c5f46b702447927 src/docker/docker.cpp 71383294c6234d08b2156565b170ada329b8e30f src/slave/constants.hpp 84927e589499e989249c217db571bbeb84a88af1 Diff: https://reviews.apache.org/r/35330/diff/ Testing --- sudo ./mesos-tests.sh --gtest_filter=*DOCKER* --gtest_break_on_failure --gtest_shuffle --gtest_repeat=30 Thanks, Lily Chen
Re: Review Request 31915: MemIsolator: Improved some statistics naming. (MESOS-2104)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31915/ --- (Updated June 11, 2015, 10:51 p.m.) Review request for mesos, Ian Downes, Jie Yu, and Paul Brett. Bugs: mesos-2104 https://issues.apache.org/jira/browse/mesos-2104 Repository: mesos Description --- MemIsolator: Improved some statistics naming. (MESOS-2104) Diffs (updated) - include/mesos/mesos.proto 20340099dc87b3ea7f375c50fa3848ab5b0d5134 src/slave/containerizer/isolators/cgroups/mem.cpp 9647e79fd27ed08f1d86d13ea1e2ab98de3367c7 Diff: https://reviews.apache.org/r/31915/diff/ Testing --- Thanks, Chi Zhang
Re: Review Request 31915: MemIsolator: Improved some statistics naming. (MESOS-2104)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31915/#review87655 --- Patch looks great! Reviews applied: [31914, 31915] All tests passed. - Mesos ReviewBot On June 11, 2015, 10:51 p.m., Chi Zhang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31915/ --- (Updated June 11, 2015, 10:51 p.m.) Review request for mesos, Ian Downes, Jie Yu, and Paul Brett. Bugs: mesos-2104 https://issues.apache.org/jira/browse/mesos-2104 Repository: mesos Description --- MemIsolator: Improved some statistics naming. (MESOS-2104) Diffs - include/mesos/mesos.proto 20340099dc87b3ea7f375c50fa3848ab5b0d5134 src/slave/containerizer/isolators/cgroups/mem.cpp 9647e79fd27ed08f1d86d13ea1e2ab98de3367c7 Diff: https://reviews.apache.org/r/31915/diff/ Testing --- Thanks, Chi Zhang
Re: Review Request 35164: Added callback to the QoS Controller to retrieve usages from the monitor.
On June 9, 2015, 12:34 a.m., Jie Yu wrote:
I think the interface of getting resource usage is going to change per
discussion here:
https://issues.apache.org/jira/browse/MESOS-2818
Niklas Nielsen wrote:
The new proposal doesn't mention changing the callback, does it?
Jie Yu wrote:
The new protobuf message is what needed by the resource estimator:
```
message ResourceUsage {
message Executor {
optional ExecutorInfo executor_info = 1;
repeated Resource allocated = 2;
repeated ResourceStatistics statistics = 3;
}
repeated Resource total = 1; // Slave's total resources.
repeated Executor executors = 2; // Per-executor allocated/usage
information.
}
```
So we'll need to change the interface accordingly.
Niklas Nielsen wrote:
To rephrase, it is because we no longer have a list of ResourceUsages?
Can you advise how to change accordingly?
Please make these kind of blocking comments actionable. We have
traditionally spent way to much time getting blocked in the 11th hour (either
having to redo completely or drop). We have to redo this for the resource
estimator anyway; so that can be done in a single pass (moving from the
previous pattern to the new one).
Any recommendations?
Jie Yu wrote:
To rephrase, it is because we no longer have a list of ResourceUsages?
Yes.
I'll probably wait until the new interfaces are out and rebase since this
is a relatively small patch. I'll get the changes out today.
Although, I am ok if you want to commit this one (i'll need to rebase
mine).
Bartek Plotka wrote:
Niklas +1
The only change will be s/list// for this patch and resource Estimator is
todo anyway. Can we ship it and change when MESOS-2818 patch is ready?
Rebased JieYu patches, so this patch should be now ready to be shiped! (:
- Bartek
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35164/#review87106
---
On June 12, 2015, 12:20 a.m., Bartek Plotka wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35164/
---
(Updated June 12, 2015, 12:20 a.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod
Kone.
Bugs: MESOS-2823
https://issues.apache.org/jira/browse/MESOS-2823
Repository: mesos
Description
---
Passed callback to the QoS Controller to retrieve ResourceUsage from Resource
Monitor on demand.
This is neccessary, since QoS Controller needs data (current statistics for
each executor) on which it will base its potential corrections.
Diffs
-
include/mesos/slave/qos_controller.hpp
1d89acfd9c742b044674e0a0815f9f01eccb69b3
src/slave/qos_controller.hpp b37798303561eb79aee202b9c110794517eeed06
src/slave/qos_controller.cpp febb2365f51ca226df699badb8626cbdfc1430b9
src/slave/slave.cpp b523c2fce50e56f4f94d55a9488f49c53452e4d4
src/tests/mesos.hpp e19ef984f9e4696bd405027d6f19756cf23d0df2
Diff: https://reviews.apache.org/r/35164/diff/
Testing
---
make check
Thanks,
Bartek Plotka
Re: Review Request 35367: Changed Resourcs JSON model() to combine non-revocable resources and ignore revocable resources.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35367/#review87662 --- Ship it! Ship It! - Vinod Kone On June 12, 2015, 12:32 a.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35367/ --- (Updated June 12, 2015, 12:32 a.m.) Review request for mesos, Niklas Nielsen and Vinod Kone. Bugs: MESOS-2838 https://issues.apache.org/jira/browse/MESOS-2838 Repository: mesos Description --- Combine means taking the sum of scalars or coalescing ranges and sets of all resources of the same name regardless of their other attributes. Diffs - include/mesos/resources.hpp 7da6e6e8bc1fa1d62fc8be2b5169b68e10392b73 src/common/http.cpp 2ac7fba7a3aac913540f1b09768777393b79284a src/common/resources.cpp 05969199889c6bc1421c8e5be10c74a50660719f src/tests/common/http_tests.cpp f087b2313a13c3199b70b3d7feb728e1449a52e7 src/tests/resources_tests.cpp ad12648a08ebc90da6bc9b9aa196fcef5ebe9f11 Diff: https://reviews.apache.org/r/35367/diff/ Testing --- make check. Added a test `HTTP.ModelResources` and `ResourcesTest.Types` for this. Thanks, Jiang Yan Xu
Re: Review Request 13620: Fix the Allocator to recover resources when a slave/framework is removed
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13620/#review87671 --- Ben; is this still relevant? - Niklas Nielsen On Aug. 19, 2013, 1:39 p.m., Thomas Marshall wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13620/ --- (Updated Aug. 19, 2013, 1:39 p.m.) Review request for mesos, Benjamin Hindman and Vinod Kone. Bugs: MESOS-621 https://issues.apache.org/jira/browse/MESOS-621 Repository: mesos Description --- Previously, when a slave or framework was removed the allocator didn't recover the associated resources, instead relying on the master calling Allocator::resourcesRecovered for all resources allocated. This was difficult to reason about and meant that the allocator's state was sometimes inconsistent with the reality of the cluster (for example, a framework could have resources allocated to it on a slave that had been removed), so this patch fixes this. This also solves a problem with the upcoming implementation of revocation where resources were recovered from a removed framework and the allocator didn't know what that framework's role is because it had been removed. Diffs - src/master/hierarchical_allocator_process.hpp 183b205 src/master/master.cpp d53b8bb Diff: https://reviews.apache.org/r/13620/diff/ Testing --- make check Thanks, Thomas Marshall
Re: Review Request 35367: Changed Resourcs JSON model() to combine non-revocable resources and ignore revocable resources.
On June 11, 2015, 4:36 p.m., Vinod Kone wrote: src/tests/common/http_tests.cpp, line 127 https://reviews.apache.org/r/35367/diff/1/?file=983104#file983104line127 camelcase? Wrote too much Python lately. - Jiang Yan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35367/#review87649 --- On June 11, 2015, 5:32 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35367/ --- (Updated June 11, 2015, 5:32 p.m.) Review request for mesos, Niklas Nielsen and Vinod Kone. Bugs: MESOS-2838 https://issues.apache.org/jira/browse/MESOS-2838 Repository: mesos Description --- Combine means taking the sum of scalars or coalescing ranges and sets of all resources of the same name regardless of their other attributes. Diffs - include/mesos/resources.hpp 7da6e6e8bc1fa1d62fc8be2b5169b68e10392b73 src/common/http.cpp 2ac7fba7a3aac913540f1b09768777393b79284a src/common/resources.cpp 05969199889c6bc1421c8e5be10c74a50660719f src/tests/common/http_tests.cpp f087b2313a13c3199b70b3d7feb728e1449a52e7 src/tests/resources_tests.cpp ad12648a08ebc90da6bc9b9aa196fcef5ebe9f11 Diff: https://reviews.apache.org/r/35367/diff/ Testing --- make check. Added a test `HTTP.ModelResources` and `ResourcesTest.Types` for this. Thanks, Jiang Yan Xu
Re: Review Request 34976: Added installation instructions for Ubuntu 14.04 and OSX
On June 11, 2015, 10:02 a.m., Adam B wrote: docs/getting-started.md, line 39 https://reviews.apache.org/r/34976/diff/2/?file=977278#file977278line39 What about these python packages? Probably still necessary if you don't `configure --disable-python`. they are there - a few lines below On June 11, 2015, 10:02 a.m., Adam B wrote: docs/getting-started.md, line 41 https://reviews.apache.org/r/34976/diff/2/?file=977278#file977278line41 0.21 and newer? (sasl is 0.14+, maven is 0.18+) Otherwise why call out the version at all, if you're just going to recommend they install the latest? because this is the version I tested them with - I have no idea if this works with anything older also, this is what it was used before On June 11, 2015, 10:02 a.m., Adam B wrote: docs/getting-started.md, line 51 https://reviews.apache.org/r/34976/diff/2/?file=977278#file977278line51 Can you get a Mac user to review/approve these instructions? Maybe @davelester or @nnielsen I tested it on a Mac - and they worked I would argue that this is better than nothing at all? On June 11, 2015, 10:02 a.m., Adam B wrote: docs/getting-started.md, line 109 https://reviews.apache.org/r/34976/diff/2/?file=977278#file977278line109 Probably better to recommend j=number of cores or j=#GB RAM, since j=8 could be overkill on a dual-core or smaller. Good suggestion! will do - Marco --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34976/#review87539 --- On June 3, 2015, 6:44 a.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34976/ --- (Updated June 3, 2015, 6:44 a.m.) Review request for mesos and Dave Lester. Repository: mesos Description --- Added installation instructions for Ubuntu 14.04 and OSX Diffs - docs/getting-started.md f0436575ec568e445f897ed28f50bcd823302d75 Diff: https://reviews.apache.org/r/34976/diff/ Testing --- This are the steps I've followed to build Mesos on my Mac and on my Ubuntu box. Thanks, Marco Massenzio
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/#review87663 --- Patch looks great! Reviews applied: [35079, 34631, 34632, 34633, 34719, 34720, 34721] All tests passed. - Mesos ReviewBot On June 11, 2015, 10:58 p.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34721/ --- (Updated June 11, 2015, 10:58 p.m.) Review request for mesos, Bartek Plotka, Jie Yu, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2653 https://issues.apache.org/jira/browse/MESOS-2653 Repository: mesos Description --- See summary Diffs - src/tests/mesos.hpp e19ef984f9e4696bd405027d6f19756cf23d0df2 src/tests/mesos.cpp 5e574c5aa79d548e5730db021b0e8b77d27f220b src/tests/oversubscription_tests.cpp e8ae053dd9cd712e49bd2830e414b7a3d99c20ca Diff: https://reviews.apache.org/r/34721/diff/ Testing --- make check (with new qos test) Thanks, Niklas Nielsen
Re: Review Request 13620: Fix the Allocator to recover resources when a slave/framework is removed
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13620/#review87673 --- Bad patch! Reviews applied: [13620] Failed command: ./support/apply-review.sh -n -r 13620 Error: 2015-06-12 02:04:37 URL:https://reviews.apache.org/r/13620/diff/raw/ [8945/8945] - 13620.patch [1] error: src/master/hierarchical_allocator_process.hpp: does not exist in index error: patch failed: src/master/master.cpp:564 error: src/master/master.cpp: patch does not apply Failed to apply patch - Mesos ReviewBot On Aug. 19, 2013, 8:39 p.m., Thomas Marshall wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13620/ --- (Updated Aug. 19, 2013, 8:39 p.m.) Review request for mesos, Benjamin Hindman and Vinod Kone. Bugs: MESOS-621 https://issues.apache.org/jira/browse/MESOS-621 Repository: mesos Description --- Previously, when a slave or framework was removed the allocator didn't recover the associated resources, instead relying on the master calling Allocator::resourcesRecovered for all resources allocated. This was difficult to reason about and meant that the allocator's state was sometimes inconsistent with the reality of the cluster (for example, a framework could have resources allocated to it on a slave that had been removed), so this patch fixes this. This also solves a problem with the upcoming implementation of revocation where resources were recovered from a removed framework and the allocator didn't know what that framework's role is because it had been removed. Diffs - src/master/hierarchical_allocator_process.hpp 183b205 src/master/master.cpp d53b8bb Diff: https://reviews.apache.org/r/13620/diff/ Testing --- make check Thanks, Thomas Marshall
Re: Review Request 34703: Added stream manipulators for the Time object.
On June 8, 2015, 2:59 p.m., Till Toenshoff wrote: 3rdparty/libprocess/include/process/time.hpp, line 150 https://reviews.apache.org/r/34703/diff/3/?file=974298#file974298line150 Lets think about a good replacement for that class name to make it intuitive - first thing that came to mind is ```UnixTimestamp```. Lets also add an example right here in this comment. Apparently BenH is OK with leaving the RFC as part of the name. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34703/#review87013 --- On June 8, 2015, 5:14 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34703/ --- (Updated June 8, 2015, 5:14 p.m.) Review request for mesos, Bernd Mathiske, Michael Park, Nikita Vetoshkin, and Till Toenshoff. Bugs: mesos-708 https://issues.apache.org/jira/browse/mesos-708 Repository: mesos Description --- Adds some manipulator classes which allows formatting Time objects to ostreams. Diffs - 3rdparty/libprocess/include/process/time.hpp c5ab2a3cfa83590eb6612152ae365dd67f51cea9 3rdparty/libprocess/src/tests/time_tests.cpp be314182c65c05d439b81aa5248a71d93f6f0a0b Diff: https://reviews.apache.org/r/34703/diff/ Testing --- make check Thanks, Alexander Rojas
