Re: Review Request 36318: [MESOS-2294] Add support to master for streaming subscribed events

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36318]

All tests passed.

- Mesos ReviewBot


On July 15, 2015, 4:56 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36318/
> ---
> 
> (Updated July 15, 2015, 4:56 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Isabel Jimenez, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-2294
> https://issues.apache.org/jira/browse/MESOS-2294
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change lays the ground-work for the master's ability to stream events 
> back to the client. This review turned out a bit too large for my own liking 
> but in a nutshell, it just takes a subscribe request and puts a subscribed 
> event back on the stream.
> 
> Explanation of changes:
> - Made a generic FrameworkDriver interface that the master now uses to 
> communicate with the frameworks instead of just invoking 
> send(framework->pid,...)
> - FrameworkDriver can be of 2 types http, libprocess. An Optional member 
> variable is used to distinguiush between them.
> - This still uses hard-coded http related constants. They can go away when 
> Isabel submits her validation change (36217)
> - This change prefers use of using trailing under-scores as member variables 
> from the style guide.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp afe5a85d3f58eaabb16807253c5fcc07cabcf8e8 
>   src/common/protobuf_utils.cpp 9ac81c38efd70f92c64a5865fa79fe516e84dd92 
>   src/master/http.cpp 23a6d4bd2f60cb4a4ad463aea7cc032941578bdc 
>   src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec 
>   src/master/master.cpp b877676afa6f3833eb7d2fb06beeaa288bd8bd5d 
>   src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac 
> 
> Diff: https://reviews.apache.org/r/36318/diff/
> 
> 
> Testing
> ---
> 
> make check + a simple test for subscribe call received a subscribed event 
> back on the stream.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 36318: [MESOS-2294] Add support to master for streaming subscribed events

[Anand Mazumdar]

> On July 9, 2015, 10:24 p.m., Marco Massenzio wrote:
> > src/master/http.cpp, lines 311-316
> > 
> >
> > unless you know for a fact that none of this will be `None()` you 
> > *must* check, or this will crash Mesos: hence
> > ```
> > if (contentType.isNone()) {
> >   return BadRequest("Content-Type header MUST be specified");
> > } else if (contentType.get() == Constants.APPLICATION_JSON) {
> >   return MediaNotSupported("We do not support JSON request/response 
> > content yet");
> > } else {
> >   ...
> > }
> > ```
> 
> Isabel Jimenez wrote:
> Hi @marco I commented on previous review that this valdiations will be 
> handle in the split of the /call patch. That's why it's missing here. It'll 
> be added with the rebase.
> 
> Marco Massenzio wrote:
> I'm not entirely sure I understand (but I don't really need to): please 
> then add either a TODO to check the checks (so to speak) or an inline comment 
> stating the invariant - but, if so, I would like to see a CHECK_SOME() to 
> make it explicit.
> Otherwise, someone else (yourself in 3 months!) may not know/remember 
> about this and may add redundant checks and/or or remove the others (and 
> leave the condition unchecked).
> 
> Anand Mazumdar wrote:
> My bad, I should have added a better TODO instead of the vague one that 
> says "Fix logic when we start supporting application/json". I will add a 
> better TODO. I was under the impression that all this would be taken care as 
> part of the validations patch.

Added a TODO for validation.


> On July 9, 2015, 10:24 p.m., Marco Massenzio wrote:
> > src/master/http.cpp, lines 1246-1249
> > 
> >
> > can you please avoid the leading underscore? they seem largely 
> > unnecessary now that we have the trailing ones for the private members
> 
> Anand Mazumdar wrote:
> From the style guide :
> 
> - "We prepend constructor and function arguments with a leading 
> underscore to avoid ambiguity and / or shadowing:"
> 
> - "Prefer trailing underscores for use as member fields (but not 
> required). Some trailing underscores are used to distinguish between similar 
> variables in the same scope (think prime symbols), but this should be avoided 
> as much as possible, including removing existing instances in the code base."
> 
> Seems like I am missing something here, I don't find any reason to follow 
> one and discard the other i.e. not use both at once. If not, these need to be 
> better explained in the style guide. What do you think ?

Removed leading "_"


> On July 9, 2015, 10:24 p.m., Marco Massenzio wrote:
> > src/master/http.cpp, line 1261
> > 
> >
> > no leading underscore
> > please add doxy for method
> 
> Anand Mazumdar wrote:
> The _ was added as it was needed for resolving ambiguity with the already 
> declared event variable used in the function.
> 
> Also it already has a explanation of what the method does in the .hpp 
> file base-class. What would we gain by adding documentation again here ?

This method was no longer needed in the recent iteration. Dropping the issue.


- Anand


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


On July 15, 2015, 4:56 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36318/
> ---
> 
> (Updated July 15, 2015, 4:56 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Isabel Jimenez, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-2294
> https://issues.apache.org/jira/browse/MESOS-2294
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change lays the ground-work for the master's ability to stream events 
> back to the client. This review turned out a bit too large for my own liking 
> but in a nutshell, it just takes a subscribe request and puts a subscribed 
> event back on the stream.
> 
> Explanation of changes:
> - Made a generic FrameworkDriver interface that the master now uses to 
> communicate with the frameworks instead of just invoking 
> send(framework->pid,...)
> - FrameworkDriver can be of 2 types http, libprocess. An Optional member 
> variable is used to distinguiush between them.
> - This still uses hard-coded http related constants. They can go away when 
> Isabel submits her validation change (36217)
> - This change prefers use of using trailing under-scores as member variables 
> from the style guide.
> 

Re: Review Request 35084: Provided consistent behavior for bundled packages.

[James Peach]

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

(Updated July 15, 2015, 4:59 a.m.)


Review request for mesos, Benjamin Hindman, Cody Maloney, and Timothy St. Clair.


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


Repository: mesos


Description
---

Add the MESOS_USE_BUNDLED_PACKAGE() macro to make it easy to provide
consistent behavior for bundled packages selected by either
--enable-bundled-$PACKAGE or --with-$PACKAGE.

The default policy is set by --enable-bundled and overridden when
the user specifies an --enable-bundled-$PACKAGE or --with-$PACKAGE
option. If --with-$PACKAGE is specified as "bundled", the bundled
version is selected.


Diffs (updated)
-

  configure.ac cad7f0e92eacc86d37b3f578382946db8b466531 

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


Testing
---

Configure and build on CentOS 7 and Mac OS X 10.10.3. Verify various (not 
exhaustive!) combinations of enabling and disableing bundled packages.

For example, on CentOS, this alost works:
  $ onfigure.developer  --disable-bundled --with-zookeeper=bundled 
--with-gmock=bundled

To work completely, this change needs to be propagated to libprocess, which I 
can do once reviewers agree that it's the right behavior.


Thanks,

James Peach

Re: Review Request 36318: [MESOS-2294] Add support to master for streaming subscribed events

[Anand Mazumdar]

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

(Updated July 15, 2015, 4:56 a.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Isabel Jimenez, Marco 
Massenzio, and Vinod Kone.


Changes
---

Simplified the design as per benh's comments, now the FrameworkDriver class 
just has an optional field "pipe" for http frameworks.


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


Repository: mesos


Description (updated)
---

This change lays the ground-work for the master's ability to stream events back 
to the client. This review turned out a bit too large for my own liking but in 
a nutshell, it just takes a subscribe request and puts a subscribed event back 
on the stream.

Explanation of changes:
- Made a generic FrameworkDriver interface that the master now uses to 
communicate with the frameworks instead of just invoking 
send(framework->pid,...)
- FrameworkDriver can be of 2 types http, libprocess. An Optional member 
variable is used to distinguiush between them.
- This still uses hard-coded http related constants. They can go away when 
Isabel submits her validation change (36217)
- This change prefers use of using trailing under-scores as member variables 
from the style guide.


Diffs (updated)
-

  src/common/protobuf_utils.hpp afe5a85d3f58eaabb16807253c5fcc07cabcf8e8 
  src/common/protobuf_utils.cpp 9ac81c38efd70f92c64a5865fa79fe516e84dd92 
  src/master/http.cpp 23a6d4bd2f60cb4a4ad463aea7cc032941578bdc 
  src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec 
  src/master/master.cpp b877676afa6f3833eb7d2fb06beeaa288bd8bd5d 
  src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac 

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


Testing
---

make check + a simple test for subscribe call received a subscribed event back 
on the stream.


Thanks,

Anand Mazumdar

Re: Review Request 36501: MESOS-3023

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36501]

All tests passed.

- Mesos ReviewBot


On July 15, 2015, 2:59 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36501/
> ---
> 
> (Updated July 15, 2015, 2:59 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3023
> https://issues.apache.org/jira/browse/MESOS-3023
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix for MESOS-3023 (Factoring out the pattern for URL generation)
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp ae10c420f7dddb8650377c91b5343591e8560392 
>   src/tests/utils.hpp f2eed2e6fbc2cc8772c642bba976b25b426784e8 
> 
> Diff: https://reviews.apache.org/r/36501/diff/
> 
> 
> Testing
> ---
> 
> 1. Build successfully in Linux
> 2. Test passed by src/mesos-tests --gtest_filter=FetcherTest.OSNetUriTest
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 36501: MESOS-3023

[haosdent huang]

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



src/tests/utils.hpp (line 26)


why not use methods in ?



src/tests/utils.hpp (line 54)


I suggest move the implement to cpp file.



src/tests/utils.hpp (line 55)


The code styple need change to follow 
https://github.com/apache/mesos/blob/master/docs/mesos-c%2B%2B-style-guide.md



src/tests/utils.hpp (line 56)





- haosdent huang


On July 15, 2015, 2:59 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36501/
> ---
> 
> (Updated July 15, 2015, 2:59 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3023
> https://issues.apache.org/jira/browse/MESOS-3023
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix for MESOS-3023 (Factoring out the pattern for URL generation)
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp ae10c420f7dddb8650377c91b5343591e8560392 
>   src/tests/utils.hpp f2eed2e6fbc2cc8772c642bba976b25b426784e8 
> 
> Diff: https://reviews.apache.org/r/36501/diff/
> 
> 
> Testing
> ---
> 
> 1. Build successfully in Linux
> 2. Test passed by src/mesos-tests --gtest_filter=FetcherTest.OSNetUriTest
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 36360: Adding common constants for HTTP API

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36360]

All tests passed.

- Mesos ReviewBot


On July 15, 2015, 2:50 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36360/
> ---
> 
> (Updated July 15, 2015, 2:50 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, Ben Mahler, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-2860
> https://issues.apache.org/jira/browse/MESOS-2860
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Adding constants used commonly through the different HTTP endpoints
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp bbd063d 
>   src/common/http.cpp 73a4de1 
> 
> Diff: https://reviews.apache.org/r/36360/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 36501: MESOS-3023

[Klaus Ma]

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

(Updated July 15, 2015, 2:59 a.m.)


Review request for mesos.


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


Repository: mesos


Description
---

Fix for MESOS-3023 (Factoring out the pattern for URL generation)


Diffs
-

  src/tests/fetcher_tests.cpp ae10c420f7dddb8650377c91b5343591e8560392 
  src/tests/utils.hpp f2eed2e6fbc2cc8772c642bba976b25b426784e8 

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


Testing
---

1. Build successfully in Linux
2. Test passed by src/mesos-tests --gtest_filter=FetcherTest.OSNetUriTest


Thanks,

Klaus Ma

Review Request 36501: MESOS-3023

[Klaus Ma]

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

Review request for mesos.


Repository: mesos


Description
---

Fix for MESOS-3023 (Factoring out the pattern for URL generation)


Diffs
-

  src/tests/fetcher_tests.cpp ae10c420f7dddb8650377c91b5343591e8560392 
  src/tests/utils.hpp f2eed2e6fbc2cc8772c642bba976b25b426784e8 

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


Testing
---

1. Build successfully in Linux
2. Test passed by src/mesos-tests --gtest_filter=FetcherTest.OSNetUriTest


Thanks,

Klaus Ma

Re: Review Request 36360: Adding common constants for HTTP API

[Isabel Jimenez]

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

(Updated July 15, 2015, 2:50 a.m.)


Review request for mesos, Anand Mazumdar, Benjamin Hindman, Ben Mahler, Marco 
Massenzio, and Vinod Kone.


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


Repository: mesos-incubating


Description
---

Adding constants used commonly through the different HTTP endpoints


Diffs (updated)
-

  src/common/http.hpp bbd063d 
  src/common/http.cpp 73a4de1 

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


Testing
---


Thanks,

Isabel Jimenez

Re: Review Request 36499: Implemented the UPDATE Event handler in the scheduler driver.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36450, 36493, 35752, 36494, 36495, 36496, 36497, 36498, 36499]

All tests passed.

- Mesos ReviewBot


On July 15, 2015, 1:47 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36499/
> ---
> 
> (Updated July 15, 2015, 1:47 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2910
> https://issues.apache.org/jira/browse/MESOS-2910
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See above.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp e7bfe8ca60af945e76b5d85ab37cc97b17ff1b4a 
>   src/common/type_utils.cpp 19f79b47539ab51a5dff97f381a44c679cf5ecaf 
>   src/sched/sched.cpp e372a15db035f74d525561839b873ed659e2c33f 
>   src/tests/scheduler_event_call_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36499/diff/
> 
> 
> Testing
> ---
> 
> Added a test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 36450: Introduced Address and URL protobufs.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36450]

All tests passed.

- Mesos ReviewBot


On July 15, 2015, 1:01 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36450/
> ---
> 
> (Updated July 15, 2015, 1:01 a.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 way to express a 
> network address.
> Also would like a way to express an HTTP address (a URL), which may include a 
> path prefix.
> 
> This also enables the message passing optimization in the scheduler driver 
> when receiving events, per MESOS-3012.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
>   include/mesos/type_utils.hpp e7bfe8ca60af945e76b5d85ab37cc97b17ff1b4a 
>   src/common/type_utils.cpp 19f79b47539ab51a5dff97f381a44c679cf5ecaf 
>   src/master/master.cpp b877676afa6f3833eb7d2fb06beeaa288bd8bd5d 
>   src/tests/master_tests.cpp 57721b788d0c70f4c6f5cc44d87465f52a70b6c2 
> 
> Diff: https://reviews.apache.org/r/36450/diff/
> 
> 
> Testing
> ---
> 
> Modified the simplest test I could find for offers.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Review Request 36498: Implemented the OFFERS Event handler in the scheduler driver.

[Ben Mahler]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This relies on 'Offer.url' to compute the pids needed for the message passing 
optimization (see 
[MESOS-3012](https://issues.apache.org/jira/browse/MESOS-3012)).


Diffs
-

  src/sched/sched.cpp e372a15db035f74d525561839b873ed659e2c33f 
  src/tests/scheduler_event_call_tests.cpp PRE-CREATION 

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


Testing
---

Added a test.


Thanks,

Ben Mahler

Review Request 36499: Implemented the UPDATE Event handler in the scheduler driver.

[Ben Mahler]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

See above.


Diffs
-

  include/mesos/type_utils.hpp e7bfe8ca60af945e76b5d85ab37cc97b17ff1b4a 
  src/common/type_utils.cpp 19f79b47539ab51a5dff97f381a44c679cf5ecaf 
  src/sched/sched.cpp e372a15db035f74d525561839b873ed659e2c33f 
  src/tests/scheduler_event_call_tests.cpp PRE-CREATION 

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


Testing
---

Added a test.


Thanks,

Ben Mahler

Review Request 36497: Implemented the SUBSCRIBE Event handler in the scheduler driver.

[Ben Mahler]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This one is non-trivial, note that we follow MESOS-786 with the exception of 
the 3rd case, since it is not possible and schedulers could not have possibly 
relied on getting registered instead of re-registered in that case.

We now need to store the MasterInfo coming from the detector, as it is not 
provided in Event.


Diffs
-

  src/sched/sched.cpp e372a15db035f74d525561839b873ed659e2c33f 
  src/tests/scheduler_event_call_tests.cpp PRE-CREATION 

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


Testing
---

Added a test.


Thanks,

Ben Mahler

Re: Review Request 35752: Implemented the ERROR Event handler in the scheduler driver.

[Ben Mahler]

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

(Updated July 15, 2015, 1:47 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebased, pulled out the stub handler change into:
https://reviews.apache.org/r/36493/


Summary (updated)
-

Implemented the ERROR Event handler in the scheduler driver.


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


Repository: mesos


Description (updated)
---

See above.


Diffs (updated)
-

  src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
  src/sched/sched.cpp e372a15db035f74d525561839b873ed659e2c33f 
  src/tests/scheduler_event_call_tests.cpp PRE-CREATION 

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


Testing
---

For now, testing needs to be done via manual injection of events. I've added a 
new test file for this, which will include the driver and library tests for 
event / call handling. We should be able to remove these tests as all of the 
existing tests will later test the event/call path.


Thanks,

Ben Mahler

Review Request 36495: Implemented the RESCIND Event handler in the scheduler driver.

[Ben Mahler]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

See above.


Diffs
-

  src/sched/sched.cpp e372a15db035f74d525561839b873ed659e2c33f 
  src/tests/scheduler_event_call_tests.cpp PRE-CREATION 

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


Testing
---

Added a test.


Thanks,

Ben Mahler

Review Request 36496: Implemented the FAILURE Event handler in the scheduler driver.

[Ben Mahler]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

See above.


Diffs
-

  src/sched/sched.cpp e372a15db035f74d525561839b873ed659e2c33f 
  src/tests/scheduler_event_call_tests.cpp PRE-CREATION 

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


Testing
---

Added a test.


Thanks,

Ben Mahler

Review Request 36493: Added a stub Event message handler in the scheduler driver.

[Ben Mahler]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

See MESOS-2910.


Diffs
-

  src/sched/sched.cpp e372a15db035f74d525561839b873ed659e2c33f 

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


Testing
---

Each event is tested in the follow up patches.


Thanks,

Ben Mahler

Review Request 36494: Implemented the MESSAGE Event handler in the scheduler driver.

[Ben Mahler]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

See above.


Diffs
-

  src/sched/sched.cpp e372a15db035f74d525561839b873ed659e2c33f 
  src/tests/scheduler_event_call_tests.cpp PRE-CREATION 

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


Testing
---

Added a test.


Thanks,

Ben Mahler

Re: Review Request 36450: Introduced Address and URL protobufs.

[Ben Mahler]

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

(Updated July 15, 2015, 1:01 a.m.)


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


Summary (updated)
-

Introduced Address and URL protobufs.


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


Repository: mesos


Description (updated)
---

To make the API more consistent, we'd like to have a single way to express a 
network address.
Also would like a way to express an HTTP address (a URL), which may include a 
path prefix.

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


Diffs (updated)
-

  include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
  include/mesos/type_utils.hpp e7bfe8ca60af945e76b5d85ab37cc97b17ff1b4a 
  src/common/type_utils.cpp 19f79b47539ab51a5dff97f381a44c679cf5ecaf 
  src/master/master.cpp b877676afa6f3833eb7d2fb06beeaa288bd8bd5d 
  src/tests/master_tests.cpp 57721b788d0c70f4c6f5cc44d87465f52a70b6c2 

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


Testing (updated)
---

Modified the simplest test I could find for offers.


Thanks,

Ben Mahler

Re: Review Request 36488: Added oversubscription user doc.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [36488]

Failed command: ./support/apply-review.sh -n -r 36488

Error:
 2015-07-15 00:29:33 URL:https://reviews.apache.org/r/36488/diff/raw/ 
[8751/8751] -> "36488.patch" [1]
error: missing binary patch data for 'docs/images/oversubscription-overview.jpg'
error: binary patch does not apply to 
'docs/images/oversubscription-overview.jpg'
error: docs/images/oversubscription-overview.jpg: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On July 15, 2015, 12:08 a.m., Niklas Nielsen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36488/
> ---
> 
> (Updated July 15, 2015, 12:08 a.m.)
> 
> 
> Review request for mesos, Adam B and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added first draft of oversubscription user doc
> 
> 
> Diffs
> -
> 
>   docs/images/oversubscription-overview.jpg PRE-CREATION 
>   docs/oversubscription.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36488/diff/
> 
> 
> Testing
> ---
> 
> Rendered at: 
> https://github.com/nqn/mesos/blob/niklas/oversubscription-user-doc/docs/oversubscription.md
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>

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

[Jie Yu]

> On July 14, 2015, 11:21 p.m., Jie Yu wrote:
> > Discussed with Cody on the design doc. Does it make sense to implement that 
> > as an anonymous module?
> 
> Marco Massenzio wrote:
> Can you please elaborate on the pros/cons of doing so?
> thanks!

Please refer to the design doc for the discussion. To summerize, I am not 
convinced that this functionality should belong to Mesos. It could be a 
standalone service (possibly launched by Mesos) that runs on each slave.

ALso, what if the slave is down? Do you still need some way to access the box?


- Jie


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


On July 14, 2015, 11:20 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36389/
> ---
> 
> (Updated July 14, 2015, 11:20 p.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 36488: Added oversubscription user doc.

[Niklas Nielsen]

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

(Updated July 14, 2015, 5:08 p.m.)


Review request for mesos, Adam B and Jie Yu.


Summary (updated)
-

Added oversubscription user doc.


Repository: mesos


Description
---

Added first draft of oversubscription user doc


Diffs
-

  docs/images/oversubscription-overview.jpg PRE-CREATION 
  docs/oversubscription.md PRE-CREATION 

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


Testing
---

Rendered at: 
https://github.com/nqn/mesos/blob/niklas/oversubscription-user-doc/docs/oversubscription.md


Thanks,

Niklas Nielsen

Re: Review Request 36488: Added first draft of oversubscription user doc

[Niklas Nielsen]

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

(Updated July 14, 2015, 5:08 p.m.)


Review request for mesos, Adam B and Jie Yu.


Changes
---

Added flags section.


Repository: mesos


Description
---

Added first draft of oversubscription user doc


Diffs (updated)
-

  docs/images/oversubscription-overview.jpg PRE-CREATION 
  docs/oversubscription.md PRE-CREATION 

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


Testing
---

Rendered at: 
https://github.com/nqn/mesos/blob/niklas/oversubscription-user-doc/docs/oversubscription.md


Thanks,

Niklas Nielsen

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36389]

All tests passed.

- Mesos ReviewBot


On July 14, 2015, 11:20 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36389/
> ---
> 
> (Updated July 14, 2015, 11:20 p.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 36389: Enable remote execution of arbitrary command.

[Marco Massenzio]

> On July 14, 2015, 11:21 p.m., Jie Yu wrote:
> > Discussed with Cody on the design doc. Does it make sense to implement that 
> > as an anonymous module?

Can you please elaborate on the pros/cons of doing so?
thanks!


- Marco


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


On July 14, 2015, 11:20 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36389/
> ---
> 
> (Updated July 14, 2015, 11:20 p.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 36488: Added first draft of oversubscription user doc

[Niklas Nielsen]

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

(Updated July 14, 2015, 5 p.m.)


Review request for mesos, Adam B and Jie Yu.


Changes
---

Added link to rendered doc.


Repository: mesos


Description
---

Added first draft of oversubscription user doc


Diffs
-

  docs/images/oversubscription-overview.jpg PRE-CREATION 
  docs/oversubscription.md PRE-CREATION 

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


Testing (updated)
---

Rendered at: 
https://github.com/nqn/mesos/blob/niklas/oversubscription-user-doc/docs/oversubscription.md


Thanks,

Niklas Nielsen

Review Request 36488: Added first draft of oversubscription user doc

[Niklas Nielsen]

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

Review request for mesos and Adam B.


Repository: mesos


Description
---

Added first draft of oversubscription user doc


Diffs
-

  docs/images/oversubscription-overview.jpg PRE-CREATION 
  docs/oversubscription.md PRE-CREATION 

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


Testing
---


Thanks,

Niklas Nielsen

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

[Jie Yu]

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


Discussed with Cody on the design doc. Does it make sense to implement that as 
an anonymous module?

- Jie Yu


On July 14, 2015, 11:20 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36389/
> ---
> 
> (Updated July 14, 2015, 11:20 p.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 36389: 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, 11:20 p.m.)


Review request for mesos, Benjamin Hindman and Cody Maloney.


Summary (updated)
-

Enable remote execution of arbitrary command.


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 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, 11:20 p.m.)


Review request for mesos, Benjamin Hindman and Cody Maloney.


Changes
---

Fixed build break


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 34136: Add ContainerImage protobuf.

[Jiang Yan Xu]

> On June 26, 2015, 2:57 p.m., Jiang Yan Xu wrote:
> > include/mesos/mesos.proto, lines 1212-1214
> > 
> >
> > Is it the intention that Image type is **defined** outside MesosInfo 
> > because DockerInfo can later reference it?
> > 
> > Otherwise it feels more natual to define Image within MesosInfo.
> 
> Vinod Kone wrote:
> +1 to put it inside MesosInfo.
> 
> Ian Downes wrote:
> I wanted it to be outside MesosInfo so it could be used by other 
> container types, i.e., I don't see it as specific to MesosInfo, e.g., we 
> could also support an AppcInfo which contained an Image::APPC.

OK SGTM.


- Jiang Yan


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


On July 11, 2015, 9:47 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34136/
> ---
> 
> (Updated July 11, 2015, 9:47 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add ContainerImage protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
> 
> Diff: https://reviews.apache.org/r/34136/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34136: Add ContainerImage protobuf.

[Jiang Yan Xu]

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


Sorry I didn't notice it in my original review but I traced the issue back from 
future reviews.


include/mesos/mesos.proto (lines 1211 - 1213)


So I found the use of the field `id` inconsistent in the code.

Sometimes `id` has the `sha512-` prefix and sometimes not.

I think we should consistently refer to `id` using the definition in the 
[spec](https://github.com/appc/spec/blob/806b17c86ba5e5d595fca3f7ed339c8a22fb46c3/spec/aci.md#image-id),
 i.e., with the prefix.

The fact that the ID is computed by the image creator using sha512 and that 
the provisioner validates it using sha512 is merely an implementation detail 
that is not a conern of higher level abstractions / APIs.

So here I think in the comments we should not call it "Image hash" but 
rather refer to the spec for its full definition. We can of course call out the 
fact that it should have the "sha512-" perfix.

What do you think?


- Jiang Yan Xu


On July 11, 2015, 9:47 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34136/
> ---
> 
> (Updated July 11, 2015, 9:47 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add ContainerImage protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
> 
> Diff: https://reviews.apache.org/r/34136/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Vinod Kone]

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


can you fix the "depends on" please?


src/slave/containerizer/provisioner.cpp (lines 41 - 44)


Why do you need foreach loop here if you were going to return error anyway?


- Vinod Kone


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 12, 2015, 4:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
>   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 
>   src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Vinod Kone]

> On July 8, 2015, 11:34 p.m., Vinod Kone wrote:
> > can you please set the dependency for this review correctly? it's hard to 
> > understand which reviews introduced the code that this review depends on.

none of the dropped issues have comments. did you forget to hit publish on your 
comments?


- Vinod


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


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 12, 2015, 4:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
>   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 
>   src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36106, 36326]

All tests passed.

- Mesos ReviewBot


On July 14, 2015, 5:04 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36326/
> ---
> 
> (Updated July 14, 2015, 5:04 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> 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
> -
> 
>   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 29437: Bug fix: Start the executor registration timer, only when the container has launched successfully

[Jiang Yan Xu]

> On July 14, 2015, 10:21 a.m., Timothy Chen wrote:
> > Nishant are you still around to help rebase this? Sorry I think we dropped 
> > this review somehow.

He responded on https://issues.apache.org/jira/browse/MESOS-999. I did some 
work on it but ended up not pushing it. Would you like to comment on the ticket?


- Jiang Yan


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


On Jan. 7, 2015, 11:54 p.m., Nishant Suneja wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29437/
> ---
> 
> (Updated Jan. 7, 2015, 11:54 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-999
> https://issues.apache.org/jira/browse/MESOS-999
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As part of this bug fix, I have trigerred the executor registration timeout 
> timer after the container's future object is set, instead of starting the 
> timer when the container launch is still pending.
> 
> Also, a new executor launch timer has been added. This timer gates the time 
> in which a successful executor container launch should happen. The executor 
> registration timer starts after the successful container launch.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp fd1c1aba0aa62372ab399bee5709ce81b8e92cec 
>   src/slave/constants.cpp 2a99b1105af0e2d62b956fa96988f477937a46bd 
>   src/slave/flags.hpp 670997dc3a702cd5edf33f2e5824c5e4dfe4ecef 
>   src/slave/slave.hpp 70bd8c1fde4ea09fa54c76aa93424a1adb0309f6 
>   src/slave/slave.cpp 50b57819b55bdcdb9f49f20648199badc4d3f37b 
>   src/tests/composing_containerizer_tests.cpp 
> 5ab5a36cadb7f8622bad0c5814e9a5fb338753ad 
>   src/tests/containerizer.hpp 24b014f44d9eec56840e18cf39fbf9100f2c0711 
>   src/tests/slave_tests.cpp c50cbc799d4793243f184f9fe628b69a020adc66 
> 
> Diff: https://reviews.apache.org/r/29437/diff/
> 
> 
> Testing
> ---
> 
> Added the unit test : SlaveTest::ExecutorRegistrationTimeoutTrigger
> Added the unit test : SlaveTest:: ExecutorLaunchTimeoutTrigger
> Added the unit test : SlaveTest:: ExecutorNoLaunchTimeoutTrigger
> make check succeeds.
> 
> 
> Thanks,
> 
> Nishant Suneja
> 
>

Re: Review Request 29437: Bug fix: Start the executor registration timer, only when the container has launched successfully

[Timothy Chen]

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


Nishant are you still around to help rebase this? Sorry I think we dropped this 
review somehow.

- Timothy Chen


On Jan. 8, 2015, 7:54 a.m., Nishant Suneja wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29437/
> ---
> 
> (Updated Jan. 8, 2015, 7:54 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-999
> https://issues.apache.org/jira/browse/MESOS-999
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As part of this bug fix, I have trigerred the executor registration timeout 
> timer after the container's future object is set, instead of starting the 
> timer when the container launch is still pending.
> 
> Also, a new executor launch timer has been added. This timer gates the time 
> in which a successful executor container launch should happen. The executor 
> registration timer starts after the successful container launch.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp fd1c1aba0aa62372ab399bee5709ce81b8e92cec 
>   src/slave/constants.cpp 2a99b1105af0e2d62b956fa96988f477937a46bd 
>   src/slave/flags.hpp 670997dc3a702cd5edf33f2e5824c5e4dfe4ecef 
>   src/slave/slave.hpp 70bd8c1fde4ea09fa54c76aa93424a1adb0309f6 
>   src/slave/slave.cpp 50b57819b55bdcdb9f49f20648199badc4d3f37b 
>   src/tests/composing_containerizer_tests.cpp 
> 5ab5a36cadb7f8622bad0c5814e9a5fb338753ad 
>   src/tests/containerizer.hpp 24b014f44d9eec56840e18cf39fbf9100f2c0711 
>   src/tests/slave_tests.cpp c50cbc799d4793243f184f9fe628b69a020adc66 
> 
> Diff: https://reviews.apache.org/r/29437/diff/
> 
> 
> Testing
> ---
> 
> Added the unit test : SlaveTest::ExecutorRegistrationTimeoutTrigger
> Added the unit test : SlaveTest:: ExecutorLaunchTimeoutTrigger
> Added the unit test : SlaveTest:: ExecutorNoLaunchTimeoutTrigger
> make check succeeds.
> 
> 
> Thanks,
> 
> Nishant Suneja
> 
>

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, 5:05 p.m.)


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


Changes
---

reveretd back to using std namespace


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 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, 5:04 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

reverted back to using std namespace


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 35998: Added doxygen styled comments to Path::basename and Path::dirname.

[Bernd Mathiske]

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



3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp (line 45)


"/" -> "|" ?



3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp (line 99)


"/" -> "|" ?


- Bernd Mathiske


On July 9, 2015, 9:02 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35998/
> ---
> 
> (Updated July 9, 2015, 9:02 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp 86c12a5 
> 
> Diff: https://reviews.apache.org/r/35998/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 36314: Added test for passing total slave's resources in ResourceUsage.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36204, 36314]

All tests passed.

- Mesos ReviewBot


On July 14, 2015, 11:09 a.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36314/
> ---
> 
> (Updated July 14, 2015, 11:09 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Follow up tests for https://reviews.apache.org/r/36204/
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp ea0ddf38444915a1cda71cce6a8897803fa49198 
> 
> Diff: https://reviews.apache.org/r/36314/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 36314: Added test for passing total slave's resources in ResourceUsage.

[Bartek Plotka]

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

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


Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
Kone.


Changes
---

Issues addressed.


Repository: mesos


Description
---

Follow up tests for https://reviews.apache.org/r/36204/


Diffs (updated)
-

  src/tests/slave_tests.cpp ea0ddf38444915a1cda71cce6a8897803fa49198 

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


Testing
---

make check.


Thanks,

Bartek Plotka

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`?
> 
> 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.
> 
> Joris Van Remoortere wrote:
> Cody had some patches for enabling sub namespaces in endpoints (as in 
> enabling slashes). Might be worth pulling those in.

Yep, it's https://issues.apache.org/jira/browse/MESOS-2130, I plan to bring up 
the discussion today at the community sync.


- 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
> 
>