Re: Review Request 36389: [WIP] Enable remote execution of arbitrary command.

[Marco Massenzio]

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

(Updated July 14, 2015, 5:16 a.m.)


Review request for mesos, Benjamin Hindman and Cody Maloney.


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


Repository: mesos


Description
---

Jira: MESOS-2830

Under certain (maintenance) circumnstance, it may be necessary
(or desirable) to execute arbitrary operator's commands on the
slave (the entire fleet or a subset thereof) bypassing the Mesos
Task execution mechanism; this may typically be necessary for
maintenance and/or emergency actions.

This patch adds an HTTP endpoint (/execute) which accepts a
JSON-encoded CommandInfo structure and executes the given
command (with optional arguments).

The output of the command (along with potentially any stderr
messages) is returned JSON-encoded in the Response.

For more details, see the design doc at:
https://goo.gl/4npTMU


Diffs (updated)
-

  src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
  src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
  src/slave/main.cpp 8008430e98773d8be9ba5ac6385cffb2e351932a 
  src/slave/slave.hpp dec4ca8323e151a6d0f9139214ff0ef6e3e3375a 
  src/slave/slave.cpp 2119b5176aa7cfb7b0b551d4d4f65ee12818b9e4 

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


Testing
---

make check

lots of manual testing (using Postman, REST client)


Thanks,

Marco Massenzio

Re: Review Request 36389: [WIP] Enable remote execution of arbitrary command.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36389]

All tests passed.

- Mesos ReviewBot


On July 14, 2015, 3:54 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36389/
> ---
> 
> (Updated July 14, 2015, 3:54 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Cody Maloney.
> 
> 
> Bugs: MESOS-2830
> https://issues.apache.org/jira/browse/MESOS-2830
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-2830
> 
> Under certain (maintenance) circumnstance, it may be necessary
> (or desirable) to execute arbitrary operator's commands on the
> slave (the entire fleet or a subset thereof) bypassing the Mesos
> Task execution mechanism; this may typically be necessary for
> maintenance and/or emergency actions.
> 
> This patch adds an HTTP endpoint (/execute) which accepts a
> JSON-encoded CommandInfo structure and executes the given
> command (with optional arguments).
> 
> The output of the command (along with potentially any stderr
> messages) is returned JSON-encoded in the Response.
> 
> For more details, see the design doc at:
> https://goo.gl/4npTMU
> 
> 
> Diffs
> -
> 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/main.cpp 8008430e98773d8be9ba5ac6385cffb2e351932a 
>   src/slave/slave.hpp dec4ca8323e151a6d0f9139214ff0ef6e3e3375a 
>   src/slave/slave.cpp 2119b5176aa7cfb7b0b551d4d4f65ee12818b9e4 
> 
> Diff: https://reviews.apache.org/r/36389/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> lots of manual testing (using Postman, REST client)
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 36424: Created a command executor helper method.

[Marco Massenzio]

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

(Updated July 14, 2015, 4:21 a.m.)


Review request for mesos, Benjamin Hindman and Cody Maloney.


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


Repository: mesos


Description
---

Jira: MESOS-2902

While researching how to execute an arbitrary script
to detect the Master IP address, it emerged clearly that
a helper method to execute an arbitrary command/script on
a node and obtain either stdout or stderr would have been
useful and avoided a lot of code repetition.

This could not be ultimately used for the purpose at hand,
but I believe it to be useful enough (particularly, to avoid
people doing "coding by copy&paste" and/or waste time
researching the same functionality).

This would also be beneficial in MESOS-2830, factoring out the remote command 
execution logic.


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess.hpp 
310cb4f8e4e2faa5545dffd196d7490c868bc5d6 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
f6acb204582a9e696c3b09d4e4c543bb052e97d4 

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


Testing
---

make check


Thanks,

Marco Massenzio

Re: Review Request 36326: containerizer: added cgroups based statistics.

[Jojy Varghese]

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

(Updated July 14, 2015, 4:09 a.m.)


Review request for mesos and Timothy Chen.


Changes
---

build fix.


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


Repository: mesos


Description
---

Adding cgroups cpustat and memory statistics as preferred way to get usage
statistics for containerizers.

Jira: MESOS-2951


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 9a7a951a72390b38f5dda3a3d50c4b35a6c784fd 
  src/slave/containerizer/docker.cpp cfb60177fe48ec0eeab12ff392c6c9f89634b92f 
  src/tests/docker_containerizer_tests.cpp 
a3da786c1b733e9dd4abf1d02d5dae051393d921 

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


Testing
---

make check


Thanks,

Jojy Varghese

Re: Review Request 36389: [WIP] Enable remote execution of arbitrary command.

[Marco Massenzio]

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

(Updated July 14, 2015, 3:54 a.m.)


Review request for mesos, Benjamin Hindman and Cody Maloney.


Changes
---

Added flag --enable_remote_execution to slave (as well as 
--remote_execution_timeout).
Refactored process invocation to (a) *not* block waiting for process execution 
and (b) timing out after a (configurable) timeout.


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


Repository: mesos


Description
---

Jira: MESOS-2830

Under certain (maintenance) circumnstance, it may be necessary
(or desirable) to execute arbitrary operator's commands on the
slave (the entire fleet or a subset thereof) bypassing the Mesos
Task execution mechanism; this may typically be necessary for
maintenance and/or emergency actions.

This patch adds an HTTP endpoint (/execute) which accepts a
JSON-encoded CommandInfo structure and executes the given
command (with optional arguments).

The output of the command (along with potentially any stderr
messages) is returned JSON-encoded in the Response.

For more details, see the design doc at:
https://goo.gl/4npTMU


Diffs (updated)
-

  src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
  src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
  src/slave/main.cpp 8008430e98773d8be9ba5ac6385cffb2e351932a 
  src/slave/slave.hpp dec4ca8323e151a6d0f9139214ff0ef6e3e3375a 
  src/slave/slave.cpp 2119b5176aa7cfb7b0b551d4d4f65ee12818b9e4 

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


Testing
---

make check

lots of manual testing (using Postman, REST client)


Thanks,

Marco Massenzio

Re: Review Request 36472: Update submitting-a-patch.md

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36472]

All tests passed.

- Mesos ReviewBot


On July 14, 2015, 3 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36472/
> ---
> 
> (Updated July 14, 2015, 3 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When developer is using git to clone the mesos source code and
> want to build & submit a patch, s/he needs to run ./bootstrap
> first to generate some files such as configure, .gitignore etc.
> But the document do not including such instructions.
> 
> This patch is adding bootstrap to the guidelines.
> 
> 
> Diffs
> -
> 
>   docs/submitting-a-patch.md 0221fe36f3f061b2f4ffe0fc4684420608f853b2 
> 
> Diff: https://reviews.apache.org/r/36472/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 36472: Update submitting-a-patch.md

[Guangya Liu]

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

(Updated July 14, 2015, 3 a.m.)


Review request for mesos.


Repository: mesos


Description
---

When developer is using git to clone the mesos source code and
want to build & submit a patch, s/he needs to run ./bootstrap
first to generate some files such as configure, .gitignore etc.
But the document do not including such instructions.

This patch is adding bootstrap to the guidelines.


Diffs (updated)
-

  docs/submitting-a-patch.md 0221fe36f3f061b2f4ffe0fc4684420608f853b2 

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


Testing
---


Thanks,

Guangya Liu

Review Request 36472: Update submitting-a-patch.md

[Guangya Liu]

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

Review request for mesos.


Repository: mesos


Description
---

When developer is using git to clone the mesos source code and
want to build & submit a patch, s/he needs to run ./bootstrap
first to generate some files such as configure, .gitignore etc.
But the document do not including such instructions.

This patch is adding bootstrap to the guidelines.


Diffs
-

  docs/submitting-a-patch.md 0221fe36f3f061b2f4ffe0fc4684420608f853b2 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 36106: cgroups: added cpuacct subsystem

[Jojy Varghese]

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

(Updated July 14, 2015, 2:53 a.m.)


Review request for mesos, Ian Downes, Jie Yu, Joris Van Remoortere, and Timothy 
Chen.


Changes
---

fixed rebase issue


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


Repository: mesos


Description
---

cgroups implementation does not have a cpuacct subsystem implementation as of
today. Adding the implementation for stat function.

Changes:
  - added Stats class to encapsulate cpuacct.stat data
  - added implementation for cpuacct::stats
  - added unit tests

Jira: MESOS-2961


Diffs (updated)
-

  src/linux/cgroups.hpp 73b98317880eea3d6a2ba37ac56d1f7e3600ba94 
  src/linux/cgroups.cpp 4c006d0c7382b940a83359d636c0d48952cdbb00 
  src/tests/cgroups_tests.cpp 475f48a474eea708f98d8c0300862351a2d4379a 

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


Testing
---

make check


Thanks,

Jojy Varghese

Re: Review Request 36470: Updated scheduler driver to send TEARDOWN call.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36461, 36462, 36463, 36464, 36465, 36466, 36467, 36469, 36470]

All tests passed.

- Mesos ReviewBot


On July 14, 2015, 12:30 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36470/
> ---
> 
> (Updated July 14, 2015, 12:30 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2913
> https://issues.apache.org/jira/browse/MESOS-2913
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp a748686dfc6bff39d81fd7adbd5cce88ddaaa73d 
>   src/tests/exception_tests.cpp 9af16748ae67671bd327a1ea7c946a7a38c5f7ff 
>   src/tests/master_allocator_tests.cpp 
> 534b2486a6a02ef32b5a7235d3ad30e82a78d6a5 
>   src/tests/master_tests.cpp 57721b788d0c70f4c6f5cc44d87465f52a70b6c2 
>   src/tests/rate_limiting_tests.cpp 49d907b0be45ccfed8af5c8fda89ad560e012c1e 
>   src/tests/slave_recovery_tests.cpp 2f882cf7b4235583b0ec8397cfcbbc02930fbc88 
> 
> Diff: https://reviews.apache.org/r/36470/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Review Request 36461: Added FutureMessageType, DropMessagesType and ExpectNoFutureMessagesType.

[Vinod Kone]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Needed these abstractions for capturing specific CALLs explicitly in subsequent 
reviews.


Diffs
-

  3rdparty/libprocess/include/process/gmock.hpp 
0fd3f8cf06c9efd357fa7c933cc28d527855ac9a 

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


Testing
---

Tested in subsequent reviews.


Thanks,

Vinod Kone

Re: Review Request 36106: cgroups: added cpuacct subsystem

[Jojy Varghese]

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

(Updated July 14, 2015, 12:38 a.m.)


Review request for mesos, Ian Downes, Jie Yu, Joris Van Remoortere, and Timothy 
Chen.


Changes
---

review comments @tim


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


Repository: mesos


Description
---

cgroups implementation does not have a cpuacct subsystem implementation as of
today. Adding the implementation for stat function.

Changes:
  - added Stats class to encapsulate cpuacct.stat data
  - added implementation for cpuacct::stats
  - added unit tests

Jira: MESOS-2961


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/Makefile.am 
856c2b289451fd404b97285b825e72913feb2f04 
  3rdparty/libprocess/3rdparty/stout/Makefile.am 
89e7b1854bd7f449f4f0027d76c6430d259a24de 
  3rdparty/libprocess/3rdparty/stout/configure.ac 
a1f86d0f943bbff91a8b021046eb66d624df7896 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
2394b95462182273464f0847f416ad83c3b64485 
  3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 
81f6e50baca95ad7a3a0ac1434c8db1c4de6adf2 
  3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp 
22d30eb80b4eca8be9cd4d48288f63fd52040ddd 
  3rdparty/libprocess/3rdparty/stout/tests/bits_tests.cpp 
672820ab775648e459f34b0c0a20e4482cebfdac 
  3rdparty/libprocess/3rdparty/stout/tests/bytes_tests.cpp 
5239805704cce8a545e11fcd83aa80484617f582 
  3rdparty/libprocess/3rdparty/stout/tests/cache_tests.cpp 
6e42c7d56bd6ef47a968bee2a01ac47fb8a7ed26 
  3rdparty/libprocess/3rdparty/stout/tests/duration_tests.cpp 
ef2f87781e7abcf1d25bc7a12cb6e18f695b 
  3rdparty/libprocess/3rdparty/stout/tests/error_tests.cpp 
9e7605c53e6636e7fea32e4f69fbaff9100a979f 
  3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
ebf8cd656625b7fd414cacaa87f156c95df29438 
  3rdparty/libprocess/3rdparty/stout/tests/gzip_tests.cpp 
01425d7205032280987021adcc97eb862a80c42c 
  3rdparty/libprocess/3rdparty/stout/tests/hashmap_tests.cpp 
984c02c982dc4dd8eb528d79e715847ef7a693b8 
  3rdparty/libprocess/3rdparty/stout/tests/hashset_tests.cpp 
2826742187411e5be8da0517859ec0d558fc2921 
  3rdparty/libprocess/3rdparty/stout/tests/interval_tests.cpp 
134ea2ee675aba13ddb7a28545b25b4f3701d5d9 
  3rdparty/libprocess/3rdparty/stout/tests/ip_tests.cpp 
532d064dcc5e31d2824df750b7d3e6bddabddbb9 
  3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
dd692094e164e33ba0f13e2b994c14cbfd827cbf 
  3rdparty/libprocess/3rdparty/stout/tests/linkedhashmap_tests.cpp 
5e385f4e7c69dc252bdee2436c6a57f1a0f17d32 
  3rdparty/libprocess/3rdparty/stout/tests/mac_tests.cpp 
0d398069ecd76002d5ee61765f0dd77f3b40bbb4 
  3rdparty/libprocess/3rdparty/stout/tests/main.cpp 
1410cf18132fbaddea1498736ac539f77232de38 
  3rdparty/libprocess/3rdparty/stout/tests/multimap_tests.cpp 
b625ffaeb3672f58fbd9558a868f87404e659c53 
  3rdparty/libprocess/3rdparty/stout/tests/none_tests.cpp 
55e75207a8c5d2ac17cbee848855f4c66a2d2eb4 
  3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp 
0c3f89bafe1afb15d1a2d775ed598cdf1a5ea147 
  3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp 
e740d5bc0f0cc5cf8e99b2064c1e39c08282da67 
  3rdparty/libprocess/3rdparty/stout/tests/os/signals_tests.cpp 
de86232cd4a646c63cdb41d18cd4375615cb42e4 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
2556bd428cc8990659e30e804b9c96c1659ef4a1 
  3rdparty/libprocess/3rdparty/stout/tests/proc_tests.cpp 
5d24f21f63433b8525370736dd630880d324ebeb 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
018ff51d10f4ba076609704d6e3b2c704c82b016 
  3rdparty/libprocess/3rdparty/stout/tests/set_tests.cpp 
6f6b0babf896216815f447b2548b4a8036751d22 
  3rdparty/libprocess/3rdparty/stout/tests/some_tests.cpp 
efb94ef3523e85d08498b5a3d5744d387ac5 
  3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 
ffe5f1bcf764f24cec1d191dcde289976b6281c4 
  3rdparty/libprocess/3rdparty/stout/tests/subcommand_tests.cpp 
f0a69d49c7c97b2d437c751fbee0a9c0f0ada0a9 
  3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp 
319fcdf517b24f5bb9c85dad4093b09ec87e915e 
  3rdparty/libprocess/3rdparty/stout/tests/uuid_tests.cpp 
b931151ed62d2a356c9c96188ae967089d92168c 
  3rdparty/libprocess/Makefile.am e9c42d7ecd44eebeadcc3d1d5011e01674b3415a 
  3rdparty/libprocess/configure.ac 7d1221bd5ddfc4fa816b0bbea0be5c6b2cbb 
  3rdparty/libprocess/examples/example.cpp 
22ae083d8b5bf59cf52e18405e005cfe94edb81d 
  3rdparty/libprocess/include/Makefile.am 
3b6108dd37d23bd5104162e9e8f4a3aa0518cdcd 
  3rdparty/libprocess/include/process/address.hpp 
be216db823160f5db1dfb4502bf832246fb3df6d 
  3rdparty/libprocess/include/process/async.hpp 
d0c560aa48ef1c88407a6b1c42223fce3170245c 
  3rdparty/libprocess/include/process/clock.hpp 
0f73f894c753711db4fdefa9df40d5674aacc6f7 
  3rdparty/libprocess/include/process/

Review Request 36462: Added FUTURE_CALL, DROP_CALL, DROP_CALLS and EXPECT_NO_FUTURE_CALLS.

[Vinod Kone]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Needed these abstractions for capturing specific CALLs explicitly in subsequent 
reviews.


Diffs
-

  src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 

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


Testing
---

Tested in subsequent reviews.


Thanks,

Vinod Kone

Review Request 36470: Updated scheduler driver to send TEARDOWN call.

[Vinod Kone]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/sched/sched.cpp a748686dfc6bff39d81fd7adbd5cce88ddaaa73d 
  src/tests/exception_tests.cpp 9af16748ae67671bd327a1ea7c946a7a38c5f7ff 
  src/tests/master_allocator_tests.cpp 534b2486a6a02ef32b5a7235d3ad30e82a78d6a5 
  src/tests/master_tests.cpp 57721b788d0c70f4c6f5cc44d87465f52a70b6c2 
  src/tests/rate_limiting_tests.cpp 49d907b0be45ccfed8af5c8fda89ad560e012c1e 
  src/tests/slave_recovery_tests.cpp 2f882cf7b4235583b0ec8397cfcbbc02930fbc88 

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


Testing
---

make check


Thanks,

Vinod Kone

Review Request 36469: Updated scheduler driver to send MESSAGE call.

[Vinod Kone]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/sched/sched.cpp a748686dfc6bff39d81fd7adbd5cce88ddaaa73d 
  src/tests/fault_tolerance_tests.cpp 1070ccf17f98f6b3800684a5edd6517d90631c3e 

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


Testing
---

make check


Thanks,

Vinod Kone

Review Request 36466: Updated scheduler driver to send RECONCILE call.

[Vinod Kone]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/sched/sched.cpp a748686dfc6bff39d81fd7adbd5cce88ddaaa73d 
  src/tests/reconciliation_tests.cpp 6042d8c02d86f486e0c4d82d5a70666d7ac9019b 

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


Testing
---

make check


Thanks,

Vinod Kone

Review Request 36463: Updated scheduler driver to send Kill Call.

[Vinod Kone]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/sched/sched.cpp a748686dfc6bff39d81fd7adbd5cce88ddaaa73d 
  src/tests/fault_tolerance_tests.cpp 1070ccf17f98f6b3800684a5edd6517d90631c3e 
  src/tests/master_tests.cpp 57721b788d0c70f4c6f5cc44d87465f52a70b6c2 

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


Testing
---

make check


Thanks,

Vinod Kone

Review Request 36467: Updated scheduler driver to send ACKNOWLEDGE call.

[Vinod Kone]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/sched/sched.cpp a748686dfc6bff39d81fd7adbd5cce88ddaaa73d 
  src/tests/fault_tolerance_tests.cpp 1070ccf17f98f6b3800684a5edd6517d90631c3e 
  src/tests/reconciliation_tests.cpp 6042d8c02d86f486e0c4d82d5a70666d7ac9019b 
  src/tests/scheduler_tests.cpp 946fa8245d8ab35e04bad642d69114caf0ccf6a9 
  src/tests/slave_recovery_tests.cpp 2f882cf7b4235583b0ec8397cfcbbc02930fbc88 
  src/tests/slave_tests.cpp 4ddc608ab9636fcc0166e8c80a252dcf67b45ad3 
  src/tests/status_update_manager_tests.cpp 
440b07475e28dc491ab640acaca8ee20db8411f8 

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


Testing
---

make check


Thanks,

Vinod Kone

Review Request 36464: Updated scheduler driver to send ACCEPT call.

[Vinod Kone]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/sched/sched.cpp a748686dfc6bff39d81fd7adbd5cce88ddaaa73d 
  src/tests/master_tests.cpp 57721b788d0c70f4c6f5cc44d87465f52a70b6c2 

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


Testing
---

make check


Thanks,

Vinod Kone

Review Request 36465: Updated scheduler driver to send REVIVE call.

[Vinod Kone]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/sched/sched.cpp a748686dfc6bff39d81fd7adbd5cce88ddaaa73d 

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


Testing
---

make check

(NOTE: There is already an existing test that tests the revive workflow, but it 
didn't need any update)


Thanks,

Vinod Kone

Re: Review Request 36466: Updated scheduler driver to send RECONCILE call.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36461, 36462, 36463, 36464, 36465, 36466]

All tests passed.

- Mesos ReviewBot


On July 14, 2015, 12:01 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36466/
> ---
> 
> (Updated July 14, 2015, 12:01 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2913
> https://issues.apache.org/jira/browse/MESOS-2913
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp a748686dfc6bff39d81fd7adbd5cce88ddaaa73d 
>   src/tests/reconciliation_tests.cpp 6042d8c02d86f486e0c4d82d5a70666d7ac9019b 
> 
> Diff: https://reviews.apache.org/r/36466/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 36425: Enabling IP Discovery script

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36424, 36425]

All tests passed.

- Mesos ReviewBot


On July 13, 2015, 9:35 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36425/
> ---
> 
> (Updated July 13, 2015, 9:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Cody Maloney.
> 
> 
> Bugs: MESOS-2902
> https://issues.apache.org/jira/browse/MESOS-2902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-2902
> 
> It is sometimes useful to enable an external script to
> configure the IP address the Mesos Master will bind to
> on the server, where it's not desirable to set the
> --ip flag and/or a "wrapper" script is not a viable option.
> 
> This patch adds a --ip_discovery_script to point to a local
> script that will emit as its only output the IP address that
> the Master will bind to: only spaces and newlines are allowed;
> further, as we cannot use the `libprocess` sub-processing
> facilities, we cannot timeout the script, should this block
> for long times (or even forever).
> 
> This will override the --ip flag, which, even if set, will be
> ignored.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md feee5594c88112f77ce382cb3dd8628653f92d01 
>   src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 
> 
> Diff: https://reviews.apache.org/r/36425/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 36450: Added an Address protobuf message for consistent address information.

[Ben Mahler]

> On July 13, 2015, 8:18 p.m., Benjamin Hindman wrote:
> > include/mesos/mesos.proto, lines 108-110
> > 
> >
> > Why not also add an Endpoint message to capture this part? Or a URL/I?
> > 
> > message Endpoint {
> >   required Address address = 1;
> >   optional string path = 2;
> > }
> > 
> > 
> > OR:
> > 
> > 
> > message URL/URI {
> >   optional string scheme = 1;
> >   required Address address = 2;
> >   optional string path = 3;
> >   repeated Parameter query = 4;
> >   optional string fragment = 5;
> > }
> 
> Marco Massenzio wrote:
> +1

After chatting with vinod, the simplest thing seems to be just adding an 
'optional pid' to the Offer message (much like we already have 'pid' in 
MasterInfo, etc), and commenting that this field is for internal use only. 
Sound good?

Hoping to just focus on an MVP for the HTTP API that does not include the 
message passing optimization for now, while preserving the optimization in the 
driver for folks that aren't ready to migrate. If there turns out to be enough 
migration pain from users relying on the optimization, we can work it in to the 
HTTP API in a proper way.


- Ben


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


On July 13, 2015, 5:55 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36450/
> ---
> 
> (Updated July 13, 2015, 5:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3012
> https://issues.apache.org/jira/browse/MESOS-3012
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To make the API more consistent, we'd like to have a single message that 
> expresses a network address.
> 
> Also, this enables the message passing optimization in the scheduler driver 
> when receiving events, per MESOS-3012.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
>   src/master/master.cpp b877676afa6f3833eb7d2fb06beeaa288bd8bd5d 
> 
> Diff: https://reviews.apache.org/r/36450/diff/
> 
> 
> Testing
> ---
> 
> Will add a follow up test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 36450: Added an Address protobuf message for consistent address information.

[Marco Massenzio]

> On July 13, 2015, 8:18 p.m., Benjamin Hindman wrote:
> > include/mesos/mesos.proto, lines 108-110
> > 
> >
> > Why not also add an Endpoint message to capture this part? Or a URL/I?
> > 
> > message Endpoint {
> >   required Address address = 1;
> >   optional string path = 2;
> > }
> > 
> > 
> > OR:
> > 
> > 
> > message URL/URI {
> >   optional string scheme = 1;
> >   required Address address = 2;
> >   optional string path = 3;
> >   repeated Parameter query = 4;
> >   optional string fragment = 5;
> > }

+1


- Marco


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


On July 13, 2015, 5:55 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36450/
> ---
> 
> (Updated July 13, 2015, 5:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3012
> https://issues.apache.org/jira/browse/MESOS-3012
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To make the API more consistent, we'd like to have a single message that 
> expresses a network address.
> 
> Also, this enables the message passing optimization in the scheduler driver 
> when receiving events, per MESOS-3012.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
>   src/master/master.cpp b877676afa6f3833eb7d2fb06beeaa288bd8bd5d 
> 
> Diff: https://reviews.apache.org/r/36450/diff/
> 
> 
> Testing
> ---
> 
> Will add a follow up test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 36450: Added an Address protobuf message for consistent address information.

[Marco Massenzio]

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


Thanks for doing this!
Please ping me when this gets committed (in case I miss it) and I'll update 
MasterInfo.
(we also need to see how it gets translated into the ZK JSON for 0.24 and 
update our API/Release Notes/docs etc.)


include/mesos/mesos.proto (line 99)


can you please take the TODO out of the javadoc (it's an implementation 
detail) and maybe add a bit more detail there about usage, assumptions, etc.



include/mesos/mesos.proto (line 102)


what happens if the hostname and the ip are both defined but don't match?

hostname == "www.google.com"
ip = "10.10.1.0"

which one "wins"?

Also, slightly confused by the "asymmetry" - `port` is required, but 
neither ip/hostname are: one could "legally" end up with an Address which is 
"8080".

I would much prefer to have `ip` as `required` (and, possibly, filled in 
with an empty string, or a "0.0.0.0" string).



include/mesos/mesos.proto (line 110)


personally, I don't think this belongs in an `Address` class, but I accept 
I don't really know enough of how libprocess works.



include/mesos/mesos.proto (line 793)


nit: I think you need an extra space between ; and /


- Marco Massenzio


On July 13, 2015, 5:55 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36450/
> ---
> 
> (Updated July 13, 2015, 5:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3012
> https://issues.apache.org/jira/browse/MESOS-3012
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To make the API more consistent, we'd like to have a single message that 
> expresses a network address.
> 
> Also, this enables the message passing optimization in the scheduler driver 
> when receiving events, per MESOS-3012.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
>   src/master/master.cpp b877676afa6f3833eb7d2fb06beeaa288bd8bd5d 
> 
> Diff: https://reviews.apache.org/r/36450/diff/
> 
> 
> Testing
> ---
> 
> Will add a follow up test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 36425: Enabling IP Discovery script

[Cody Maloney]

> On July 13, 2015, 9:12 p.m., Cody Maloney wrote:
> > Generally looks reasonable to me. I haven't integration tested yet inside 
> > DCOS, will work on that before too long.
> 
> Marco Massenzio wrote:
> > reasonable
> 
> not exactly a ringing endorsement :) but I guess I'll take it...
> care to give me a Ship it?

Ship It comes post integration test. I still dislike the fopen() api a lot, but 
that one isn't your fault.


- Cody


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


On July 13, 2015, 9:35 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36425/
> ---
> 
> (Updated July 13, 2015, 9:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Cody Maloney.
> 
> 
> Bugs: MESOS-2902
> https://issues.apache.org/jira/browse/MESOS-2902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-2902
> 
> It is sometimes useful to enable an external script to
> configure the IP address the Mesos Master will bind to
> on the server, where it's not desirable to set the
> --ip flag and/or a "wrapper" script is not a viable option.
> 
> This patch adds a --ip_discovery_script to point to a local
> script that will emit as its only output the IP address that
> the Master will bind to: only spaces and newlines are allowed;
> further, as we cannot use the `libprocess` sub-processing
> facilities, we cannot timeout the script, should this block
> for long times (or even forever).
> 
> This will override the --ip flag, which, even if set, will be
> ignored.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md feee5594c88112f77ce382cb3dd8628653f92d01 
>   src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 
> 
> Diff: https://reviews.apache.org/r/36425/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 36380: Test cases for performance monitor support of multiple output versions depending on kernel version.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36424, 36378, 36380]

All tests passed.

- Mesos ReviewBot


On July 13, 2015, 9:05 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36380/
> ---
> 
> (Updated July 13, 2015, 9:05 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Cong Wang.
> 
> 
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Test cases for performance monitor support of multiple output versions 
> depending on kernel version.
> 
> 
> Diffs
> -
> 
>   src/tests/perf_tests.cpp 281eed0094faead67dc7f84df6407686aae88b01 
> 
> Diff: https://reviews.apache.org/r/36380/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 36425: Enabling IP Discovery script

[Marco Massenzio]

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

(Updated July 13, 2015, 9:35 p.m.)


Review request for mesos, Benjamin Hindman and Cody Maloney.


Changes
---

Updated help string for the flag.


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


Repository: mesos


Description
---

Jira: MESOS-2902

It is sometimes useful to enable an external script to
configure the IP address the Mesos Master will bind to
on the server, where it's not desirable to set the
--ip flag and/or a "wrapper" script is not a viable option.

This patch adds a --ip_discovery_script to point to a local
script that will emit as its only output the IP address that
the Master will bind to: only spaces and newlines are allowed;
further, as we cannot use the `libprocess` sub-processing
facilities, we cannot timeout the script, should this block
for long times (or even forever).

This will override the --ip flag, which, even if set, will be
ignored.


Diffs (updated)
-

  docs/configuration.md feee5594c88112f77ce382cb3dd8628653f92d01 
  src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 

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


Testing
---

make check


Thanks,

Marco Massenzio

Re: Review Request 36425: Enabling IP Discovery script

[Marco Massenzio]

> On July 13, 2015, 9:12 p.m., Cody Maloney wrote:
> > Generally looks reasonable to me. I haven't integration tested yet inside 
> > DCOS, will work on that before too long.

> reasonable

not exactly a ringing endorsement :) but I guess I'll take it...
care to give me a Ship it?


> On July 13, 2015, 9:12 p.m., Cody Maloney wrote:
> > docs/configuration.md, line 101
> > 
> >
> > nit: I don't like string here

You're right.
Changed to (both in the .md doc and the flag help string):
```
  "Optional IP discovery binary: if set, it is expected to emit\n"
  "the IP address which Master will try to bind to.\n"
  "Overrides the setting in --ip."
```


- Marco


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


On July 13, 2015, 7:25 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36425/
> ---
> 
> (Updated July 13, 2015, 7:25 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Cody Maloney.
> 
> 
> Bugs: MESOS-2902
> https://issues.apache.org/jira/browse/MESOS-2902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-2902
> 
> It is sometimes useful to enable an external script to
> configure the IP address the Mesos Master will bind to
> on the server, where it's not desirable to set the
> --ip flag and/or a "wrapper" script is not a viable option.
> 
> This patch adds a --ip_discovery_script to point to a local
> script that will emit as its only output the IP address that
> the Master will bind to: only spaces and newlines are allowed;
> further, as we cannot use the `libprocess` sub-processing
> facilities, we cannot timeout the script, should this block
> for long times (or even forever).
> 
> This will override the --ip flag, which, even if set, will be
> ignored.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md feee5594c88112f77ce382cb3dd8628653f92d01 
>   src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 
> 
> Diff: https://reviews.apache.org/r/36425/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 36425: Enabling IP Discovery script

[Cody Maloney]

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


Generally looks reasonable to me. I haven't integration tested yet inside DCOS, 
will work on that before too long.


docs/configuration.md (line 101)


nit: I don't like string here


- Cody Maloney


On July 13, 2015, 7:25 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36425/
> ---
> 
> (Updated July 13, 2015, 7:25 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Cody Maloney.
> 
> 
> Bugs: MESOS-2902
> https://issues.apache.org/jira/browse/MESOS-2902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-2902
> 
> It is sometimes useful to enable an external script to
> configure the IP address the Mesos Master will bind to
> on the server, where it's not desirable to set the
> --ip flag and/or a "wrapper" script is not a viable option.
> 
> This patch adds a --ip_discovery_script to point to a local
> script that will emit as its only output the IP address that
> the Master will bind to: only spaces and newlines are allowed;
> further, as we cannot use the `libprocess` sub-processing
> facilities, we cannot timeout the script, should this block
> for long times (or even forever).
> 
> This will override the --ip flag, which, even if set, will be
> ignored.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md feee5594c88112f77ce382cb3dd8628653f92d01 
>   src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 
> 
> Diff: https://reviews.apache.org/r/36425/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 36106: cgroups: added cpuacct subsystem

[Timothy Chen]

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

Ship it!


Ship It!


src/linux/cgroups.cpp (line 2022)


Extra space in the message.



src/tests/cgroups_tests.cpp (line 1193)


We should use string instead of std::string here but I can fix that



src/tests/cgroups_tests.cpp (line 1193)


We should use string instead of std::string here but I can fix that


- Timothy Chen


On July 10, 2015, 10:46 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36106/
> ---
> 
> (Updated July 10, 2015, 10:46 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, Joris Van Remoortere, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-2961
> https://issues.apache.org/jira/browse/MESOS-2961
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> cgroups implementation does not have a cpuacct subsystem implementation as of
> today. Adding the implementation for stat function.
> 
> Changes:
>   - added Stats class to encapsulate cpuacct.stat data
>   - added implementation for cpuacct::stats
>   - added unit tests
> 
> Jira: MESOS-2961
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 73b98317880eea3d6a2ba37ac56d1f7e3600ba94 
>   src/linux/cgroups.cpp 4c006d0c7382b940a83359d636c0d48952cdbb00 
>   src/tests/cgroups_tests.cpp 475f48a474eea708f98d8c0300862351a2d4379a 
> 
> Diff: https://reviews.apache.org/r/36106/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 36380: Test cases for performance monitor support of multiple output versions depending on kernel version.

[Paul Brett]

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

(Updated July 13, 2015, 9:05 p.m.)


Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Cong Wang.


Changes
---

Incorporate review comments.


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


Repository: mesos


Description
---

Test cases for performance monitor support of multiple output versions 
depending on kernel version.


Diffs (updated)
-

  src/tests/perf_tests.cpp 281eed0094faead67dc7f84df6407686aae88b01 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 36378: Refactor Linux Performance monitor to handle changing 'perf stat' output versions depending on kernel version.

[Paul Brett]

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

(Updated July 13, 2015, 9:04 p.m.)


Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Cong Wang.


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


Repository: mesos


Description
---

Refactor Linux Performance monitor to handle changing 'perf stat' output 
versions depending on kernel version.


Diffs
-

  src/linux/perf.cpp 697b75e846a43d4f106ad8f39a18882836d7dc02 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 36378: Refactor Linux Performance monitor to handle changing 'perf stat' output versions depending on kernel version.

[Paul Brett]

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

(Updated July 13, 2015, 9:02 p.m.)


Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Cong Wang.


Changes
---

Get version from 'perf --stat' not os::release.


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


Repository: mesos


Description
---

Refactor Linux Performance monitor to handle changing 'perf stat' output 
versions depending on kernel version.


Diffs (updated)
-

  src/linux/perf.cpp 697b75e846a43d4f106ad8f39a18882836d7dc02 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 36453: Ignored subsequent calls to 'resourceOffers' in 'ReservationTest.CompatibleCheckpointedResources' test.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36453]

All tests passed.

- Mesos ReviewBot


On July 13, 2015, 8:18 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36453/
> ---
> 
> (Updated July 13, 2015, 8:18 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-3034
> https://issues.apache.org/jira/browse/MESOS-3034
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignored subsequent calls to 'resourceOffers' in 
> 'ReservationTest.CompatibleCheckpointedResources' test.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 99bf393ed4e806dee13518cfbd2d44d740285c2c 
> 
> Diff: https://reviews.apache.org/r/36453/diff/
> 
> 
> Testing
> ---
> 
> I was able to reproduce more reliably after changing 
> `master.allocation_interval` to `Milliseconds(1)` and running 
> `GTEST_FILTER="ReservationTest.CompatibleCheckpointedResources" 
> ./bin/mesos-tests.sh --gtest_repeat=1000`.
> 
> Ran the above command with the fix and observed that it passes.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 36450: Added an Address protobuf message for consistent address information.

[Benjamin Hindman]

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



include/mesos/mesos.proto (lines 108 - 110)


Why not also add an Endpoint message to capture this part? Or a URL/I?

message Endpoint {
  required Address address = 1;
  optional string path = 2;
}

OR:

message URL/URI {
  optional string scheme = 1;
  required Address address = 2;
  optional string path = 3;
  repeated Parameter query = 4;
  optional string fragment = 5;
}


- Benjamin Hindman


On July 13, 2015, 5:55 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36450/
> ---
> 
> (Updated July 13, 2015, 5:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3012
> https://issues.apache.org/jira/browse/MESOS-3012
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To make the API more consistent, we'd like to have a single message that 
> expresses a network address.
> 
> Also, this enables the message passing optimization in the scheduler driver 
> when receiving events, per MESOS-3012.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
>   src/master/master.cpp b877676afa6f3833eb7d2fb06beeaa288bd8bd5d 
> 
> Diff: https://reviews.apache.org/r/36450/diff/
> 
> 
> Testing
> ---
> 
> Will add a follow up test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 36453: Ignored subsequent calls to 'resourceOffers' in 'ReservationTest.CompatibleCheckpointedResources' test.

[Michael Park]

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

(Updated July 13, 2015, 8:18 p.m.)


Review request for mesos, Ben Mahler and Jie Yu.


Changes
---

Addressed Jie's comment.


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


Repository: mesos


Description
---

Ignored subsequent calls to 'resourceOffers' in 
'ReservationTest.CompatibleCheckpointedResources' test.


Diffs (updated)
-

  src/tests/reservation_tests.cpp 99bf393ed4e806dee13518cfbd2d44d740285c2c 

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


Testing
---

I was able to reproduce more reliably after changing 
`master.allocation_interval` to `Milliseconds(1)` and running 
`GTEST_FILTER="ReservationTest.CompatibleCheckpointedResources" 
./bin/mesos-tests.sh --gtest_repeat=1000`.

Ran the above command with the fix and observed that it passes.


Thanks,

Michael Park

Re: Review Request 36425: Enabling IP Discovery script

[Cody Maloney]

> On July 13, 2015, 6:14 p.m., Anand Mazumdar wrote:
> > src/master/main.cpp, line 211
> > 
> >
> > Can we remove the reference here ? 
> > 
> > strings::trim(...) returns a temporary that would be destroyed at the 
> > end of the line leaving you with a dangling reference.

Not actually a dangling reference since the C++ spec actually specifies the 
value lives as long as  the reference in this case. But it is against the style 
guide. See http://mesos.apache.org/documentation/latest/mesos-c++-style-guide/ 
section "Capture by Reference"


- Cody


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


On July 13, 2015, 7:25 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36425/
> ---
> 
> (Updated July 13, 2015, 7:25 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Cody Maloney.
> 
> 
> Bugs: MESOS-2902
> https://issues.apache.org/jira/browse/MESOS-2902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-2902
> 
> It is sometimes useful to enable an external script to
> configure the IP address the Mesos Master will bind to
> on the server, where it's not desirable to set the
> --ip flag and/or a "wrapper" script is not a viable option.
> 
> This patch adds a --ip_discovery_script to point to a local
> script that will emit as its only output the IP address that
> the Master will bind to: only spaces and newlines are allowed;
> further, as we cannot use the `libprocess` sub-processing
> facilities, we cannot timeout the script, should this block
> for long times (or even forever).
> 
> This will override the --ip flag, which, even if set, will be
> ignored.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md feee5594c88112f77ce382cb3dd8628653f92d01 
>   src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 
> 
> Diff: https://reviews.apache.org/r/36425/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 36380: Test cases for performance monitor support of multiple output versions depending on kernel version.

[Paul Brett]

> On July 10, 2015, 9:43 p.m., Ian Downes wrote:
> > src/tests/perf_tests.cpp, line 159
> > 
> >
> > Do you need to keep all the logging of the context  on failed 
> > expectations after the test has been correctly written? It clutters the 
> > code...

If you remove it, you cant tell which test failed so it becomes necessary to 
re-add it if you ever get a failure.


- Paul


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


On July 10, 2015, 8:52 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36380/
> ---
> 
> (Updated July 10, 2015, 8:52 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Cong Wang.
> 
> 
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Test cases for performance monitor support of multiple output versions 
> depending on kernel version.
> 
> 
> Diffs
> -
> 
>   src/tests/perf_tests.cpp 281eed0094faead67dc7f84df6407686aae88b01 
> 
> Diff: https://reviews.apache.org/r/36380/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 36425: Enabling IP Discovery script

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36424, 36425]

All tests passed.

- Mesos ReviewBot


On July 13, 2015, 7:25 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36425/
> ---
> 
> (Updated July 13, 2015, 7:25 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Cody Maloney.
> 
> 
> Bugs: MESOS-2902
> https://issues.apache.org/jira/browse/MESOS-2902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-2902
> 
> It is sometimes useful to enable an external script to
> configure the IP address the Mesos Master will bind to
> on the server, where it's not desirable to set the
> --ip flag and/or a "wrapper" script is not a viable option.
> 
> This patch adds a --ip_discovery_script to point to a local
> script that will emit as its only output the IP address that
> the Master will bind to: only spaces and newlines are allowed;
> further, as we cannot use the `libprocess` sub-processing
> facilities, we cannot timeout the script, should this block
> for long times (or even forever).
> 
> This will override the --ip flag, which, even if set, will be
> ignored.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md feee5594c88112f77ce382cb3dd8628653f92d01 
>   src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 
> 
> Diff: https://reviews.apache.org/r/36425/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 36453: Ignored subsequent calls to 'resourceOffers' in 'ReservationTest.CompatibleCheckpointedResources' test.

[Jie Yu]

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

Ship it!



src/tests/reservation_tests.cpp (line 928)


Please add a comment since it's not quite obvious.

.WillRepeatedly(Return());// Ignore subsequent offers.


- Jie Yu


On July 13, 2015, 7:29 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36453/
> ---
> 
> (Updated July 13, 2015, 7:29 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-3034
> https://issues.apache.org/jira/browse/MESOS-3034
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignored subsequent calls to 'resourceOffers' in 
> 'ReservationTest.CompatibleCheckpointedResources' test.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 99bf393ed4e806dee13518cfbd2d44d740285c2c 
> 
> Diff: https://reviews.apache.org/r/36453/diff/
> 
> 
> Testing
> ---
> 
> I was able to reproduce more reliably after changing 
> `master.allocation_interval` to `Milliseconds(1)` and running 
> `GTEST_FILTER="ReservationTest.CompatibleCheckpointedResources" 
> ./bin/mesos-tests.sh --gtest_repeat=1000`.
> 
> Ran the above command with the fix and observed that it passes.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Review Request 36453: Ignored subsequent calls to 'resourceOffers' in 'ReservationTest.CompatibleCheckpointedResources' test.

[Michael Park]

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

Review request for mesos, Ben Mahler and Jie Yu.


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


Repository: mesos


Description
---

Ignored subsequent calls to 'resourceOffers' in 
'ReservationTest.CompatibleCheckpointedResources' test.


Diffs
-

  src/tests/reservation_tests.cpp 99bf393ed4e806dee13518cfbd2d44d740285c2c 

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


Testing
---

I was able to reproduce more reliably after changing 
`master.allocation_interval` to `Milliseconds(1)` and running 
`GTEST_FILTER="ReservationTest.CompatibleCheckpointedResources" 
./bin/mesos-tests.sh --gtest_repeat=1000`.

Ran the above command with the fix and observed that it passes.


Thanks,

Michael Park

Re: Review Request 36425: Enabling IP Discovery script

[Marco Massenzio]

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

(Updated July 13, 2015, 7:25 p.m.)


Review request for mesos, Benjamin Hindman and Cody Maloney.


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


Repository: mesos


Description
---

Jira: MESOS-2902

It is sometimes useful to enable an external script to
configure the IP address the Mesos Master will bind to
on the server, where it's not desirable to set the
--ip flag and/or a "wrapper" script is not a viable option.

This patch adds a --ip_discovery_script to point to a local
script that will emit as its only output the IP address that
the Master will bind to: only spaces and newlines are allowed;
further, as we cannot use the `libprocess` sub-processing
facilities, we cannot timeout the script, should this block
for long times (or even forever).

This will override the --ip flag, which, even if set, will be
ignored.


Diffs (updated)
-

  docs/configuration.md feee5594c88112f77ce382cb3dd8628653f92d01 
  src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 

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


Testing
---

make check


Thanks,

Marco Massenzio

Re: Review Request 36425: Enabling IP Discovery script

[Marco Massenzio]

> On July 13, 2015, 6:14 p.m., Anand Mazumdar wrote:
> > a few fly-by general comments.

thanks!


> On July 13, 2015, 6:14 p.m., Anand Mazumdar wrote:
> > src/master/main.cpp, line 205
> > 
> >
> > Can we align this similar to the one you have already done a few lines 
> > before ?

because of the way this gets indented (which was, in fact, my initial approach) 
it looks really horrible (also, it does the `jagged` look we so dread :) )
This way is much more pleasing on the eye, whilst not impacting at all on 
readability (IMO).


> On July 13, 2015, 6:14 p.m., Anand Mazumdar wrote:
> > src/master/main.cpp, line 209
> > 
> >
> > Can we use our already existing abstraction stringify(...) here or did 
> > it not work here ?

mah - this is the "standard" way of doing it (I think our `stringify` was 
necessary prior to C++11 and `std::to_string()`, but I may be wrong here).
anyways - changed to stringify().


- Marco


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


On July 13, 2015, 4:35 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36425/
> ---
> 
> (Updated July 13, 2015, 4:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Cody Maloney.
> 
> 
> Bugs: MESOS-2902
> https://issues.apache.org/jira/browse/MESOS-2902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-2902
> 
> It is sometimes useful to enable an external script to
> configure the IP address the Mesos Master will bind to
> on the server, where it's not desirable to set the
> --ip flag and/or a "wrapper" script is not a viable option.
> 
> This patch adds a --ip_discovery_script to point to a local
> script that will emit as its only output the IP address that
> the Master will bind to: only spaces and newlines are allowed;
> further, as we cannot use the `libprocess` sub-processing
> facilities, we cannot timeout the script, should this block
> for long times (or even forever).
> 
> This will override the --ip flag, which, even if set, will be
> ignored.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md feee5594c88112f77ce382cb3dd8628653f92d01 
>   src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 
> 
> Diff: https://reviews.apache.org/r/36425/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 35687: Added capabilities to state.json

[Vinod Kone]

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


LGTM modulo Alex's reviews. Fix them and I'll commit this for you.

- Vinod Kone


On July 12, 2015, 6:25 p.m., Aditi Dixit wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35687/
> ---
> 
> (Updated July 12, 2015, 6:25 p.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Vinod Kone.
> 
> 
> Bugs: MESOS-2900
> https://issues.apache.org/jira/browse/MESOS-2900
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added capabilities to state.json and test for the same
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
>   src/tests/master_tests.cpp 962455cc368c6e5405599d6565660d4c3fd0fc22 
> 
> Diff: https://reviews.apache.org/r/35687/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Aditi Dixit
> 
>

Re: Review Request 36425: Enabling IP Discovery script

[Marco Massenzio]

> On July 13, 2015, 5:46 p.m., Cody Maloney wrote:
> > src/master/main.cpp, line 35
> > 
> >
> > unused include

how did you find out?
(I really miss IntelliJ highlighting that for me in Java...)


> On July 13, 2015, 5:46 p.m., Cody Maloney wrote:
> > src/master/main.cpp, line 148
> > 
> >
> > Supersedes -> Overrides like you did in the docs?

yes - fixed.


> On July 13, 2015, 5:46 p.m., Cody Maloney wrote:
> > src/master/main.cpp, line 211
> > 
> >
> > Could you add a basic "Can we change this discoveredIp -> an actual IP 
> > address" check here?  The error message when LIBPROCESS_IP ends up 
> > incorrect inside libprocess is going to not say very cleanly where that 
> > value came from, and I suspect when doing iteration it will be fairly 
> > common for people to write IP scripts which give incorrect / invalid output.

This is what happens when you give a non-parseable IP:
```
$ ./bin/mesos-master.sh --ip_discovery_script=~/discovery-ip.py --work_dir=/tmp
WARNING: Logging before InitGoogleLogging() is written to STDERR
I0713 12:12:52.721210 1955922688 main.cpp:185] Discovering Master server IP 
using ~/discovery-ip.py
I0713 12:12:52.741296 1955922688 main.cpp:211] Setting IP for Mesos Master to: 
[10.0.77.243/24]
F0713 12:12:52.741581 1955922688 process.cpp:861] Parsing 
LIBPROCESS_IP=10.0.77.243/24 failed: Failed to parse the IP
*** Check failure stack trace: ***
Abort trap: 6
```
and, if things go catastophically off the rails:
```
$ ./bin/mesos-master.sh --ip_discovery_script=~/discovery-ip.py --work_dir=/tmp
WARNING: Logging before InitGoogleLogging() is written to STDERR
I0713 12:15:07.231408 1955922688 main.cpp:185] Discovering Master server IP 
using ~/discovery-ip.py
I0713 12:15:07.251881 1955922688 main.cpp:211] Setting IP for Mesos Master to: 
[mesos-master.rack-101.az12.comcast.wp.com]
F0713 12:15:07.252142 1955922688 process.cpp:861] Parsing 
LIBPROCESS_IP=mesos-master.rack-101.az12.comcast.wp.com failed: Failed to parse 
the IP
*** Check failure stack trace: ***
Abort trap: 6
```
Looks pretty clear to me what went on?


- Marco


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


On July 13, 2015, 4:35 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36425/
> ---
> 
> (Updated July 13, 2015, 4:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Cody Maloney.
> 
> 
> Bugs: MESOS-2902
> https://issues.apache.org/jira/browse/MESOS-2902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-2902
> 
> It is sometimes useful to enable an external script to
> configure the IP address the Mesos Master will bind to
> on the server, where it's not desirable to set the
> --ip flag and/or a "wrapper" script is not a viable option.
> 
> This patch adds a --ip_discovery_script to point to a local
> script that will emit as its only output the IP address that
> the Master will bind to: only spaces and newlines are allowed;
> further, as we cannot use the `libprocess` sub-processing
> facilities, we cannot timeout the script, should this block
> for long times (or even forever).
> 
> This will override the --ip flag, which, even if set, will be
> ignored.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md feee5594c88112f77ce382cb3dd8628653f92d01 
>   src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 
> 
> Diff: https://reviews.apache.org/r/36425/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 36450: Added an Address protobuf message for consistent address information.

[Vinod Kone]

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


LGTM. Will wait for the test before giving shipit.

- Vinod Kone


On July 13, 2015, 5:55 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36450/
> ---
> 
> (Updated July 13, 2015, 5:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3012
> https://issues.apache.org/jira/browse/MESOS-3012
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To make the API more consistent, we'd like to have a single message that 
> expresses a network address.
> 
> Also, this enables the message passing optimization in the scheduler driver 
> when receiving events, per MESOS-3012.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
>   src/master/master.cpp b877676afa6f3833eb7d2fb06beeaa288bd8bd5d 
> 
> Diff: https://reviews.apache.org/r/36450/diff/
> 
> 
> Testing
> ---
> 
> Will add a follow up test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 36326: containerizer: added cgroups based statistics.

[Jojy Varghese]

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

(Updated July 13, 2015, 6:41 p.m.)


Review request for Timothy Chen.


Changes
---

added check for rss in usage test


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


Repository: mesos


Description
---

Adding cgroups cpustat and memory statistics as preferred way to get usage
statistics for containerizers.

Jira: MESOS-2951


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 646a277be21deaded47324bea5474bd68699f25f 
  src/slave/containerizer/docker.cpp 6eb1c84b1a6ef17c3e2ea2028e2f2d75110176ba 
  src/tests/docker_containerizer_tests.cpp 
a3da786c1b733e9dd4abf1d02d5dae051393d921 

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


Testing
---

make check


Thanks,

Jojy Varghese

Re: Review Request 34141: AppC provsioning backend.

[Jiang Yan Xu]

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



src/slave/containerizer/provisioners/appc/backend.hpp (lines 25 - 28)


Alphabetic order.



src/slave/containerizer/provisioners/appc/backend.hpp (line 49)


s/image/images.



src/slave/containerizer/provisioners/appc/backend.hpp (lines 98 - 99)


Indentation.



src/slave/containerizer/provisioners/appc/backend.cpp (line 1)


Kill this line.



src/slave/containerizer/provisioners/appc/backend.cpp (lines 20 - 28)


Reorder includes.



src/slave/containerizer/provisioners/appc/backend.cpp (line 109)


Indentation.



src/slave/containerizer/provisioners/appc/backend.cpp (lines 119 - 120)


Indentation.



src/slave/containerizer/provisioners/appc/backend.cpp (line 130)


OSX [doesn't take this 
flag](https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man1/cp.1.html).

How about `-a`?



src/slave/containerizer/provisioners/appc/backend.cpp (lines 147 - 156)


The style guilde requires:

```
// 2: OK (when no chaining, compare to 1).
instance.method([]() {
  ...;
});
```

i.e. 2 space indentation for the body and have the closing brace aligned 
with the dot in ".then".



src/slave/containerizer/provisioners/appc/backend.cpp (lines 179 - 188)


Ditto.


- Jiang Yan Xu


On July 7, 2015, 12:43 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34141/
> ---
> 
> (Updated July 7, 2015, 12:43 p.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 e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/slave/containerizer/provisioners/appc/backend.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/appc/backend.cpp PRE-CREATION 
>   src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
>   src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
> 
> Diff: https://reviews.apache.org/r/34141/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 36450: Added an Address protobuf message for consistent address information.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36450]

All tests passed.

- Mesos ReviewBot


On July 13, 2015, 5:55 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36450/
> ---
> 
> (Updated July 13, 2015, 5:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3012
> https://issues.apache.org/jira/browse/MESOS-3012
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To make the API more consistent, we'd like to have a single message that 
> expresses a network address.
> 
> Also, this enables the message passing optimization in the scheduler driver 
> when receiving events, per MESOS-3012.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
>   src/master/master.cpp b877676afa6f3833eb7d2fb06beeaa288bd8bd5d 
> 
> Diff: https://reviews.apache.org/r/36450/diff/
> 
> 
> Testing
> ---
> 
> Will add a follow up test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 36425: Enabling IP Discovery script

[Anand Mazumdar]

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


a few fly-by general comments.


src/master/main.cpp (line 93)


We can get rid of this , I don' think this is being used ?



src/master/main.cpp (line 191)


s/fopen/popen



src/master/main.cpp (line 205)


Can we align this similar to the one you have already done a few lines 
before ?



src/master/main.cpp (line 209)


Can we use our already existing abstraction stringify(...) here or did it 
not work here ?



src/master/main.cpp (line 211)


Can we remove the reference here ? 

strings::trim(...) returns a temporary that would be destroyed at the end 
of the line leaving you with a dangling reference.


- Anand Mazumdar


On July 13, 2015, 4:35 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36425/
> ---
> 
> (Updated July 13, 2015, 4:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Cody Maloney.
> 
> 
> Bugs: MESOS-2902
> https://issues.apache.org/jira/browse/MESOS-2902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-2902
> 
> It is sometimes useful to enable an external script to
> configure the IP address the Mesos Master will bind to
> on the server, where it's not desirable to set the
> --ip flag and/or a "wrapper" script is not a viable option.
> 
> This patch adds a --ip_discovery_script to point to a local
> script that will emit as its only output the IP address that
> the Master will bind to: only spaces and newlines are allowed;
> further, as we cannot use the `libprocess` sub-processing
> facilities, we cannot timeout the script, should this block
> for long times (or even forever).
> 
> This will override the --ip flag, which, even if set, will be
> ignored.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md feee5594c88112f77ce382cb3dd8628653f92d01 
>   src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 
> 
> Diff: https://reviews.apache.org/r/36425/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Review Request 36450: Added an Address protobuf message for consistent address information.

[Ben Mahler]

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

Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

To make the API more consistent, we'd like to have a single message that 
expresses a network address.

Also, this enables the message passing optimization in the scheduler driver 
when receiving events, per MESOS-3012.


Diffs
-

  include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
  src/master/master.cpp b877676afa6f3833eb7d2fb06beeaa288bd8bd5d 

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


Testing
---

Will add a follow up test.


Thanks,

Ben Mahler

Re: Review Request 36440: Update comments for mesos master cpp file

[Marco Massenzio]

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

Ship it!


Ship It!

- Marco Massenzio


On July 13, 2015, 5:42 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36440/
> ---
> 
> (Updated July 13, 2015, 5:42 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In the function of initialize() in mesos master cpp, the comment
> of "Install handler functions for certain messages" should be moved
> to cover all install<>() functions.
> 
> This patch is making the comment also cover scheduler::call.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp b877676afa6f3833eb7d2fb06beeaa288bd8bd5d 
> 
> Diff: https://reviews.apache.org/r/36440/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 36425: Enabling IP Discovery script

[Cody Maloney]

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



docs/configuration.md (line 92)


Default isn't quite correct here. More accurate is `Mesos will do a lookup 
of the machine's hostname to try and figure out what IP address to use`



src/master/main.cpp (line 22)


Please use  or use the  variants consistently



src/master/main.cpp (line 35)


unused include



src/master/main.cpp (line 39)


unused include



src/master/main.cpp (line 148)


Supersedes -> Overrides like you did in the docs?



src/master/main.cpp (line 211)


Could you add a basic "Can we change this discoveredIp -> an actual IP 
address" check here?  The error message when LIBPROCESS_IP ends up incorrect 
inside libprocess is going to not say very cleanly where that value came from, 
and I suspect when doing iteration it will be fairly common for people to write 
IP scripts which give incorrect / invalid output.


- Cody Maloney


On July 13, 2015, 4:35 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36425/
> ---
> 
> (Updated July 13, 2015, 4:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Cody Maloney.
> 
> 
> Bugs: MESOS-2902
> https://issues.apache.org/jira/browse/MESOS-2902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-2902
> 
> It is sometimes useful to enable an external script to
> configure the IP address the Mesos Master will bind to
> on the server, where it's not desirable to set the
> --ip flag and/or a "wrapper" script is not a viable option.
> 
> This patch adds a --ip_discovery_script to point to a local
> script that will emit as its only output the IP address that
> the Master will bind to: only spaces and newlines are allowed;
> further, as we cannot use the `libprocess` sub-processing
> facilities, we cannot timeout the script, should this block
> for long times (or even forever).
> 
> This will override the --ip flag, which, even if set, will be
> ignored.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md feee5594c88112f77ce382cb3dd8628653f92d01 
>   src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 
> 
> Diff: https://reviews.apache.org/r/36425/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

[Joris Van Remoortere]

> On June 22, 2015, 1:32 p.m., Alexander Rukletsov wrote:
> > src/master/master.cpp, line 749
> > 
> >
> > I think reserve is too abstract and may collide with future actions 
> > (think quota). How about `/dynamic/reserve`?
> 
> Alexander Rukletsov wrote:
> Though we currently do not support slashes in endpoints, I think we 
> should fix that first before introducing a `/reserve` endpoint, given these 
> endpoint are not targeted for 0.23.

Cody had some patches for enabling sub namespaces in endpoints (as in enabling 
slashes). Might be worth pulling those in.


- Joris


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


On June 28, 2015, 8:36 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35702/
> ---
> 
> (Updated June 28, 2015, 8:36 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris 
> Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
> https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This involved a lot more challenges than I anticipated, I've captured the 
> various approaches and limitations and deal-breakers of those approaches 
> here: [Master Endpoint Implementation 
> Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
> 
> Key points:
> 
> * This is a stop-gap solution until we shift the offer creation/management 
> logic from the master to the allocator.
> * `updateAvailable` and `updateSlave` are kept separate because
>   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
>   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
>   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
> and must not, whereas `updateSlave` does, and can.
> * The algorithm:
> * Initially, the master pessimistically assume that what seems like 
> "available" resources will be gone.
>   This is due to the race between the allocator scheduling an `allocate` 
> call to itself vs master's `allocator->updateAvailable` invocation.
>   As such, we first try to satisfy the request only with the offered 
> resources.
> * We greedily rescind one offer at a time until we've rescinded 
> sufficiently many offers.
>   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
> `recoverResources(..., None())` so that we can pretty much always win the 
> race against `allocate`.
>  In the case that we lose, no disaster occurs. We simply fail 
> to satisfy the request.
> * If we still don't have enough resources after resciding all offers, be 
> optimistic and forward the request to the allocator since there may be 
> available resources to satisfy the request.
> * If the allocator returns a failure, report the error to the user with 
> `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
> maybe as well. We'll pick one eventually.
> 
> This approach is clearly not ideal, since we would prefer to rescind as 
> little offers as possible.
> The challenges of implementing the ideal solution in the current state is 
> described in the document above.
> 
> TODO(mpark): Add more comments and test cases.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
>   src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
>   src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 
> 
> Diff: https://reviews.apache.org/r/35702/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 36425: Enabling IP Discovery script

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36424, 36425]

All tests passed.

- Mesos ReviewBot


On July 13, 2015, 4:35 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36425/
> ---
> 
> (Updated July 13, 2015, 4:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Cody Maloney.
> 
> 
> Bugs: MESOS-2902
> https://issues.apache.org/jira/browse/MESOS-2902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-2902
> 
> It is sometimes useful to enable an external script to
> configure the IP address the Mesos Master will bind to
> on the server, where it's not desirable to set the
> --ip flag and/or a "wrapper" script is not a viable option.
> 
> This patch adds a --ip_discovery_script to point to a local
> script that will emit as its only output the IP address that
> the Master will bind to: only spaces and newlines are allowed;
> further, as we cannot use the `libprocess` sub-processing
> facilities, we cannot timeout the script, should this block
> for long times (or even forever).
> 
> This will override the --ip flag, which, even if set, will be
> ignored.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md feee5594c88112f77ce382cb3dd8628653f92d01 
>   src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 
> 
> Diff: https://reviews.apache.org/r/36425/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

[Alexander Rukletsov]

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


A high level question: do you think rescinding offers is a big deal for now?


src/master/http.cpp (line 507)


The code until this line is basically request validation and authorization. 
Though it's not how we do it now, do you think it makes sense to split the 
function into smaller logical parts?

How about something like this:

```
Future Master::Http::reserve(const Request& request) const
{
  return Master::Http::reserveValidate();
}

Future Master::Http::reserveValidate(const Request& request) const
{
  <...>
  return Master::Http::reserveAuthorize();
}

<...>
```



src/master/http.cpp (lines 515 - 516)


It looks like we actually have the role, but it's buried in resources. Do 
you envision having resources collection with various roles in one request? 
Maybe it makes sense to add a validation step which ensures there is just one 
role per request and use it here, also avoiding changes in the 
`validate()`function.



src/master/http.cpp (lines 523 - 524)


Let's leave a comment here, that we do want to defer the decision 
completely to an allocator, but can do it currently because offers are issued 
and handled by the master.



src/master/http.cpp (lines 541 - 545)


We basically defer the decision whether request can be granted or not to an 
allocator (up to rescinding). Let's capture it in a comment!



src/master/master.hpp (line 962)


I see that you extracted this function for code reusal, but let's document 
it. You may want to add a comment for `applyResourceOperation()`and update the 
comment for `applyOfferOperation()` as well.


- Alexander Rukletsov


On June 28, 2015, 8:36 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35702/
> ---
> 
> (Updated June 28, 2015, 8:36 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris 
> Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
> https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This involved a lot more challenges than I anticipated, I've captured the 
> various approaches and limitations and deal-breakers of those approaches 
> here: [Master Endpoint Implementation 
> Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
> 
> Key points:
> 
> * This is a stop-gap solution until we shift the offer creation/management 
> logic from the master to the allocator.
> * `updateAvailable` and `updateSlave` are kept separate because
>   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
>   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
>   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
> and must not, whereas `updateSlave` does, and can.
> * The algorithm:
> * Initially, the master pessimistically assume that what seems like 
> "available" resources will be gone.
>   This is due to the race between the allocator scheduling an `allocate` 
> call to itself vs master's `allocator->updateAvailable` invocation.
>   As such, we first try to satisfy the request only with the offered 
> resources.
> * We greedily rescind one offer at a time until we've rescinded 
> sufficiently many offers.
>   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
> `recoverResources(..., None())` so that we can pretty much always win the 
> race against `allocate`.
>  In the case that we lose, no disaster occurs. We simply fail 
> to satisfy the request.
> * If we still don't have enough resources after resciding all offers, be 
> optimistic and forward the request to the allocator since there may be 
> available resources to satisfy the request.
> * If the allocator returns a failure, report the error to the user with 
> `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
> maybe as well. We'll pick one eventually.
> 
> This approach is clearly not ideal, since we would prefer to rescind as 
> little offers as possible.
> The challenges of implementing the ideal solution in the current state is 
> described in the document above.
> 
> TODO(mpark): Add more comments and test cases.
> 
> 
> Diffs
> -
> 
> 

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

[Alexander Rukletsov]

> On June 22, 2015, 1:32 p.m., Alexander Rukletsov wrote:
> > src/master/master.cpp, line 749
> > 
> >
> > I think reserve is too abstract and may collide with future actions 
> > (think quota). How about `/dynamic/reserve`?

Though we currently do not support slashes in endpoints, I think we should fix 
that first before introducing a `/reserve` endpoint, given these endpoint are 
not targeted for 0.23.


- Alexander


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


On June 28, 2015, 8:36 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35702/
> ---
> 
> (Updated June 28, 2015, 8:36 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris 
> Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
> https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This involved a lot more challenges than I anticipated, I've captured the 
> various approaches and limitations and deal-breakers of those approaches 
> here: [Master Endpoint Implementation 
> Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
> 
> Key points:
> 
> * This is a stop-gap solution until we shift the offer creation/management 
> logic from the master to the allocator.
> * `updateAvailable` and `updateSlave` are kept separate because
>   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
>   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
>   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
> and must not, whereas `updateSlave` does, and can.
> * The algorithm:
> * Initially, the master pessimistically assume that what seems like 
> "available" resources will be gone.
>   This is due to the race between the allocator scheduling an `allocate` 
> call to itself vs master's `allocator->updateAvailable` invocation.
>   As such, we first try to satisfy the request only with the offered 
> resources.
> * We greedily rescind one offer at a time until we've rescinded 
> sufficiently many offers.
>   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
> `recoverResources(..., None())` so that we can pretty much always win the 
> race against `allocate`.
>  In the case that we lose, no disaster occurs. We simply fail 
> to satisfy the request.
> * If we still don't have enough resources after resciding all offers, be 
> optimistic and forward the request to the allocator since there may be 
> available resources to satisfy the request.
> * If the allocator returns a failure, report the error to the user with 
> `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
> maybe as well. We'll pick one eventually.
> 
> This approach is clearly not ideal, since we would prefer to rescind as 
> little offers as possible.
> The challenges of implementing the ideal solution in the current state is 
> described in the document above.
> 
> TODO(mpark): Add more comments and test cases.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
>   src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
>   src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 
> 
> Diff: https://reviews.apache.org/r/35702/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 36425: Enabling IP Discovery script

[Marco Massenzio]

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

(Updated July 13, 2015, 4:35 p.m.)


Review request for mesos, Benjamin Hindman and Cody Maloney.


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


Repository: mesos


Description
---

Jira: MESOS-2902

It is sometimes useful to enable an external script to
configure the IP address the Mesos Master will bind to
on the server, where it's not desirable to set the
--ip flag and/or a "wrapper" script is not a viable option.

This patch adds a --ip_discovery_script to point to a local
script that will emit as its only output the IP address that
the Master will bind to: only spaces and newlines are allowed;
further, as we cannot use the `libprocess` sub-processing
facilities, we cannot timeout the script, should this block
for long times (or even forever).

This will override the --ip flag, which, even if set, will be
ignored.


Diffs (updated)
-

  docs/configuration.md feee5594c88112f77ce382cb3dd8628653f92d01 
  src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 

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


Testing
---

make check


Thanks,

Marco Massenzio

Re: Review Request 36441: Update message when master is sending offer(s)

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36441]

All tests passed.

- Mesos ReviewBot


On July 13, 2015, 6:33 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36441/
> ---
> 
> (Updated July 13, 2015, 6:33 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the mesos master send only 1 offer, it should use offer but not
> offers.
> 
> This patch is updating the offers to offer(s) to be more generic.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp b877676afa6f3833eb7d2fb06beeaa288bd8bd5d 
> 
> Diff: https://reviews.apache.org/r/36441/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 35687: Added capabilities to state.json

[Alexander Rukletsov]

> On June 20, 2015, 10:50 a.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, line 124
> > 
> >
> > Let's make it `foreachvalue` for consistency.
> 
> Aditi Dixit wrote:
> Hi,
> I'm sorry, but I'm unable to understand how foreachvalue works.

No worries, I was referring to `foreach` anyway : ).


- Alexander


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


On July 12, 2015, 6:25 p.m., Aditi Dixit wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35687/
> ---
> 
> (Updated July 12, 2015, 6:25 p.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Vinod Kone.
> 
> 
> Bugs: MESOS-2900
> https://issues.apache.org/jira/browse/MESOS-2900
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added capabilities to state.json and test for the same
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
>   src/tests/master_tests.cpp 962455cc368c6e5405599d6565660d4c3fd0fc22 
> 
> Diff: https://reviews.apache.org/r/35687/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Aditi Dixit
> 
>

Re: Review Request 35687: Added capabilities to state.json

[Alexander Rukletsov]

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



src/master/http.cpp (line 124)


Please add a space between `foreach` and a bracket for consistency: 
`foreach (...`



src/tests/master_tests.cpp (line 2949)


We tend to limit comments to 70 chars, this line looks longer. Mind 
wrapping it?



src/tests/master_tests.cpp (line 2997)


We use `lowerCamelCase` for variable names, please rename.



src/tests/master_tests.cpp (line 3001)


Instead of the hardcoded constant, it makes sense to use 
`FrameworkInfo::Capability::Type_Name()` IMO.


- Alexander Rukletsov


On July 12, 2015, 6:25 p.m., Aditi Dixit wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35687/
> ---
> 
> (Updated July 12, 2015, 6:25 p.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Vinod Kone.
> 
> 
> Bugs: MESOS-2900
> https://issues.apache.org/jira/browse/MESOS-2900
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added capabilities to state.json and test for the same
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
>   src/tests/master_tests.cpp 962455cc368c6e5405599d6565660d4c3fd0fc22 
> 
> Diff: https://reviews.apache.org/r/35687/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Aditi Dixit
> 
>