Review Request 37955: Remove hashmap::existsValue since it is never called

[Jian Qiu]

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

Review request for mesos.


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


Repository: mesos


Description
---

Remove hashmap::existsValue since it is never called


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp 
f09bea125035aa3621402b83608b233e42877559 

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


Testing
---

make check


Thanks,

Jian Qiu

Re: Review Request 37827: Added a test for converting JSON arrays to repeated protobufs.

[haosdent huang]

> On Aug. 27, 2015, 2:22 a.m., haosdent huang wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h, line 38
> > 
> >
> > Need adjust order here?
> 
> Alexander Rukletsov wrote:
> Not sure I follow, what exactly do you mean?

Oh, sorry for my english. I mean do we need ajust the order to
```
class Message;
class Nested;
class SimpleMessage;
```


- haosdent


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


On Aug. 27, 2015, 3:38 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37827/
> ---
> 
> (Updated Aug. 27, 2015, 3:38 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
> c56d6a3098293eb3659b3066f10e875927ec3ac3 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 
> cfc2803e42284f641879fb24bce1282215c8ea52 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 
> a1d4084661345f9367c75f9db61279f032b93e69 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 
> bbd36d39e9588eb8eea6d739451ad3bab029ca08 
> 
> Diff: https://reviews.apache.org/r/37827/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> **NOTE**: Filed 
> [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up 
> protobuf generation.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 37955: Remove hashmap::existsValue since it is never called

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37955]

All tests passed.

- Mesos ReviewBot


On Aug. 31, 2015, 3:42 p.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37955/
> ---
> 
> (Updated Aug. 31, 2015, 3:42 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3329
> https://issues.apache.org/jira/browse/MESOS-3329
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Remove hashmap::existsValue since it is never called
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp 
> f09bea125035aa3621402b83608b233e42877559 
> 
> Diff: https://reviews.apache.org/r/37955/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 37958: Add an option to only perform batch allocations.

[James Peach]

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

(Updated Aug. 31, 2015, 4:49 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Normally, the mesos-master performs allocation cycles in response
to protocol messages and cluster events. This ties the cost of
allocation to the amount of churn in the environment, which can led
to unpredictable performance. Add an option to disable this, meaning
that allocations are only every performed by the batch interval
task. This makes resource allocation a fixed cost, independent of
the amount of environmental churn.


Diffs
-

  docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
  include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
  src/master/allocator/mesos/allocator.hpp 
aa55755a9c3250579e9366bdbc17a2449e95d659 
  src/master/allocator/mesos/hierarchical.hpp 
38f8fd2c84314bb3731684d0e9795cb4f50a227e 
  src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
  src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
  src/tests/hierarchical_allocator_tests.cpp 
9748ca0b3fee25dcec51c64d8ba84dbd4aaf 
  src/tests/mesos.hpp b2160f50db3e0b0d04b0625e7589004016c0f746 

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


Testing
---

"make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple 
weeks.


Thanks,

James Peach

Review Request 37958: Add an option to only perform batch allocations.

[James Peach]

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

Review request for mesos.


Repository: mesos


Description
---

Normally, the mesos-master performs allocation cycles in response
to protocol messages and cluster events. This ties the cost of
allocation to the amount of churn in the environment, which can led
to unpredictable performance. Add an option to disable this, meaning
that allocations are only every performed by the batch interval
task. This makes resource allocation a fixed cost, independent of
the amount of environmental churn.


Diffs
-

  docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
  include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
  src/master/allocator/mesos/allocator.hpp 
aa55755a9c3250579e9366bdbc17a2449e95d659 
  src/master/allocator/mesos/hierarchical.hpp 
38f8fd2c84314bb3731684d0e9795cb4f50a227e 
  src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
  src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
  src/tests/hierarchical_allocator_tests.cpp 
9748ca0b3fee25dcec51c64d8ba84dbd4aaf 
  src/tests/mesos.hpp b2160f50db3e0b0d04b0625e7589004016c0f746 

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


Testing
---

"make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple 
weeks.


Thanks,

James Peach

Re: Review Request 37945: change const pass-by-value to const reference in stout

[Neil Conway]

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



3rdparty/libprocess/3rdparty/stout/include/stout/format.hpp (line 82)


This comment seems to contradict the changes made by this patch -- the 
comment seems correct (http://stackoverflow.com/a/222314).


- Neil Conway


On Aug. 31, 2015, 10:46 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37945/
> ---
> 
> (Updated Aug. 31, 2015, 10:46 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-1805
> https://issues.apache.org/jira/browse/MESOS-1805
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> change const pass-by-value to const reference in stout
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/format.hpp 
> 1cf6dd18aa163688d6c8f3a6d33eacad3918015d 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp 
> 68fc1fd179ee51fc5de0452c0f2ea3d354e0567f 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp 
> 01e59de466496dec9367ad6f48538327f53a7e18 
> 
> Diff: https://reviews.apache.org/r/37945/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 37958: Add an option to only perform batch allocations.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37958]

All tests passed.

- Mesos ReviewBot


On Aug. 31, 2015, 4:49 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37958/
> ---
> 
> (Updated Aug. 31, 2015, 4:49 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3157
> https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Normally, the mesos-master performs allocation cycles in response
> to protocol messages and cluster events. This ties the cost of
> allocation to the amount of churn in the environment, which can led
> to unpredictable performance. Add an option to disable this, meaning
> that allocations are only every performed by the batch interval
> task. This makes resource allocation a fixed cost, independent of
> the amount of environmental churn.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp 
> aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 
> 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9748ca0b3fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/mesos.hpp b2160f50db3e0b0d04b0625e7589004016c0f746 
> 
> Diff: https://reviews.apache.org/r/37958/diff/
> 
> 
> Testing
> ---
> 
> "make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple 
> weeks.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 37894: Renamed cgroups::kill to cgroups::signal.

[Timothy Chen]

> On Aug. 28, 2015, 4:45 p.m., Cong Wang wrote:
> > Why? Everyone knows kill(2) sends a signal while signal(2) installs a 
> > signal handler...
> 
> Jie Yu wrote:
> IMO, that naming is confusing, and should be 'signal' and 'install'.
> 
> Joerg Schad wrote:
> This is actually answering a discussion here 
> https://reviews.apache.org/r/36620/#comment152376. And I agree that the 
> original names are weird, but you are right in that we should have an open 
> discussion about this. I will send a mail to dev later on...
> 
> Cong Wang wrote:
> Like it or not, it is already a part of POSIX (if not just UNIX), 
> pthread_kill() has the same "problem". It's just too late to change, as it 
> simply breaks every programmer's expectation.
> 
> Jie Yu wrote:
> This is cgroups::signal and cgroups::kill (has nothing to do with Posix 
> kill and signal).
> 
> It reads more natural: signal a cgroup, kill a cgroup. I don't think 
> it'll confuse the reader.
> 
> Are you suggesting that any thing suffixed with 'kill' should be linked 
> to kill(2) in posix? I have to disagree with that.

I think I personally don't think we need to tie with POSIX, just need some 
comments to clarify what the intent is and I personally like to see this rename 
as well. Don't want this to sit and get stale again, Jie/Cong is there more to 
add or can we agree on this change?


- Timothy


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


On Aug. 28, 2015, 4:39 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37894/
> ---
> 
> (Updated Aug. 28, 2015, 4:39 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Till Toenshoff, and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As this function is actually just sending the signal we renamed it. 
> Additionally we want to introduce an actual cgroups::kill() with MESOS-3086.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 204c53038a1ccfa693f4f2293488cff8cdd60835 
>   src/linux/cgroups.cpp 6ef42ed1bc719f334d1ac6e90919a1bc1840d31f 
>   src/tests/containerizer/cgroups_tests.cpp 
> 0b171eeb53037f26b7e952830e88e59f1278e7c6 
> 
> Diff: https://reviews.apache.org/r/37894/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 37722: Added definitions of container rootfs directories.

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On Aug. 25, 2015, 6:41 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37722/
> ---
> 
> (Updated Aug. 25, 2015, 6:41 p.m.)
> 
> 
> Review request for mesos, Lily Chen, Chi Zhang, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-3308
> https://issues.apache.org/jira/browse/MESOS-3308
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> More context in /r/37382/ and MESOS-3308.
> 
> I will add the appc backend + provisioner implementation shortly but would 
> like to seek feedback about this design early.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp dafe1df0cb5d0b83ca0579068916fe7fda848f02 
>   src/slave/containerizer/provisioners/appc/paths.hpp 
> e35805179e67770c6eff7406668caecabefe4fea 
>   src/slave/containerizer/provisioners/appc/paths.cpp 
> a244d9a40e7143134b7bf883514bfcd04d6a6af5 
>   src/slave/paths.hpp dbb1f68d8a789303b595d9e455e2f6f1f0de5c5b 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/tests/paths_tests.cpp 9b7179691bb8c146e5d5cca7437ec64a456a38ad 
> 
> Diff: https://reviews.apache.org/r/37722/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 37873: Add quiesce logic in allocator

[James Peach]

> On Aug. 30, 2015, 5:10 p.m., Robert Lacroix wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 909
> > 
> >
> > We need to store the information that a framework is quiescing offers 
> > in a more memory efficient way. This doesn't work for clusters with a lot 
> > of frameworks and a lot of slaves.
> > 
> > Can't we just use a boolean to indicate the framework has quiesced?
> 
> Guangya Liu wrote:
> The reason that I did not use a boolean value is because here 
> quiesceOffers is a complement to the current reviveOffers, and reviveOffers 
> can just clear the filters to enable the framework can get resource again, so 
> here I was setting filters to this framework to disable it getting offers. 
> For performance issue, the current settting are all memory operations and 
> only for framework, I think that the Mesos cluster may not have thousands of 
> frameworks. Comments? Thanks.
> 
> Robert Lacroix wrote:
> reviveOffers could just flip the boolean in addition to removing the 
> filters, so that shouldn't make the logic much more complicated.
> 
> The number of filters is indeed a problem because they (and all of their 
> resources - which in case the ports are fragmented can be a lot) need to be 
> traversed on every allocation.
> 
> Guangya Liu wrote:
> Thanks Robert for the update. The design for quiesce offer also want a 
> timeout for quiesce, if the timeout reached, then the framework can get 
> resource offers automatically and no need for cluster admin to get involved 
> to revive offer. Do you have any comments for adding a filter to quiesce 
> offer? Is there are any other methods to add a filter which is more 
> efficient? Hope can get some comments from you.

You can add the quiesce timeout using the mechanism that refuse timeouts use. 
It might get a little more involved since if you receive a second quiesce, you 
would need to update the timeout. You can't implement quiesce using a filter 
because filters are not ordered. You have to be able to check whether a 
framework is quiesced without traversing the whole filter set.


- James


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


On Aug. 31, 2015, 5:49 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37873/
> ---
> 
> (Updated Aug. 31, 2015, 5:49 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add quiesce logic in allocator
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp 
> aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 
> 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/mesos.hpp b2160f50db3e0b0d04b0625e7589004016c0f746 
> 
> Diff: https://reviews.apache.org/r/37873/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 37699: Removed remnants of LIBPROCESS_STATISTICS_WINDOW.

[Greg Mann]

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

(Updated Aug. 31, 2015, 6:03 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Removed remnants of LIBPROCESS_STATISTICS_WINDOW.


Diffs
-

  3rdparty/libprocess/src/process.cpp 755187c8761137cb2bf2f7295b29a63f63c68bc6 

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


Testing (updated)
---

make check


Thanks,

Greg Mann

Re: Review Request 37585: Maintenance primitives: Add a user doc.

[Joseph Wu]

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

(Updated Aug. 31, 2015, 11:11 a.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, 
Joris Van Remoortere, and Vinod Kone.


Changes
---

Addressed comments.  Added image file to review for reviewer convenience.


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


Repository: mesos


Description
---

Heavily based on the design doc 
(https://docs.google.com/document/d/16k0lVwpSGVOyxPSyXKmGC-gbNmRlisNEe4p-fAUSojk/).

Includes a diagram of the maintenance mode transitions.

This documents the current working prototype (MVP), which starts here:
https://reviews.apache.org/r/36321/

One TODO remaining: Update with the release version that maintenance primitives 
will be released in.


Diffs (updated)
-

  docs/images/maintenance-primitives-modes.png PRE-CREATION 
  docs/maintenance.md PRE-CREATION 

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


Testing
---

Copied to: https://gist.github.com/kaysoky/b9789c88ee204e3b49a2
Checked for markdown correctness.


File Attachments (updated)


Same as the image in the binary diff.  (Uploaded for reviewer convenience.)
  
https://reviews.apache.org/media/uploaded/files/2015/08/31/ce897571-0d3a-4ecb-8d36-a63eff855647__maintenance-primitives-modes.png


Thanks,

Joseph Wu

Re: Review Request 37826: Introduced conversion of JSON arrays to repeated protobufs.

[Michael Park]

> On Aug. 31, 2015, 6:20 p.m., Michael Park wrote:
> > What do you think of the following API?
> > 
> > ```
> > JSON::Value value = ...;
> > auto message = protobuf::parse(value);  // message has type Try.
> > auto repeated = 
> > protobuf::parse(value);  // repeated 
> > has type google::protobuf::RepeatedPtrField.
> > ```
> > 
> > This makes it so that `parse` always returns `Try`, which I think 
> > enables the use of `auto` for the result under our style guide.
> > I think this is a simpler pattern to remember than `parse` -> `Try` 
> > and `parseRepeated` -> `Try`.
> > 
> > It would take `JSON::Value` rather than `JSON::Array` but check that it is 
> > indeed an instance of `JSON::Array`,
> > the same way the existing version takes `JSON::Value` and checks that it's 
> > an instance of `JSON::Object`.
> > 
> > It also makes the API symmetric with `read()`. We simply do `read` or 
> > `read` and it does the right thing.

__NOTE:__ `parseRepeated` is actually @joseph's suggestion in the subsequent 
patch.

In this patch, `parse(value)` returns `Try` and `parse(array)` returns 
`Try`,
which I think is just as hard if not harder to intuitively deduce.

What do you think?


- Michael


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


On Aug. 27, 2015, 3:38 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37826/
> ---
> 
> (Updated Aug. 27, 2015, 3:38 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 57d5fdf45273c620655b44b5f5572290cffa4bf6 
> 
> Diff: https://reviews.apache.org/r/37826/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 37826: Introduced conversion of JSON arrays to repeated protobufs.

[Michael Park]

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


What do you think of the following API?

```
JSON::Value value = ...;
auto message = protobuf::parse(value);  // message has type Try.
auto repeated = protobuf::parse(value);  
// repeated has type google::protobuf::RepeatedPtrField.
```

This makes it so that `parse` always returns `Try`, which I think enables 
the use of `auto` for the result under our style guide.
I think this is a simpler pattern to remember than `parse` -> `Try` and 
`parseRepeated` -> `Try`.

It would take `JSON::Value` rather than `JSON::Array` but check that it is 
indeed an instance of `JSON::Array`,
the same way the existing version takes `JSON::Value` and checks that it's an 
instance of `JSON::Object`.

It also makes the API symmetric with `read()`. We simply do `read` or 
`read` and it does the right thing.

- Michael Park


On Aug. 27, 2015, 3:38 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37826/
> ---
> 
> (Updated Aug. 27, 2015, 3:38 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 57d5fdf45273c620655b44b5f5572290cffa4bf6 
> 
> Diff: https://reviews.apache.org/r/37826/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 37908: Silence oversubscription logging

[Niklas Nielsen]

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

(Updated Aug. 31, 2015, 11:28 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Reduced patch to move LOG(INFO) for oversubscription into VLOG(1)


Repository: mesos


Description (updated)
---

Resource estimates are continuously written to the logs even though they are 
empty (as with the majority of Mesos clusters at the moment). To reduce the 
noise, this patch moves log lines for empty resources to VLOG(1).


Diffs (updated)
-

  src/slave/slave.cpp 8cfad7b86675b10c98524174975828bbcf47aed2 

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


Testing
---

make check - with and without the patch, verifying the log statements.


Thanks,

Niklas Nielsen

Re: Review Request 37699: Removed remnants of LIBPROCESS_STATISTICS_WINDOW.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37699]

All tests passed.

- Mesos ReviewBot


On Aug. 31, 2015, 6:03 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37699/
> ---
> 
> (Updated Aug. 31, 2015, 6:03 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3304
> https://issues.apache.org/jira/browse/MESOS-3304
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed remnants of LIBPROCESS_STATISTICS_WINDOW.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 755187c8761137cb2bf2f7295b29a63f63c68bc6 
> 
> Diff: https://reviews.apache.org/r/37699/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37273: [1/2]Add CMake macro VsBuildCommand in libprocess.

[Joseph Wu]

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


For any future CMake reviews, could you also add Artem (hartem) and I (kaysoky) 
as reviewers?
Thanks in advance :)


3rdparty/libprocess/3rdparty/CMakeLists.txt (lines 68 - 71)


This comment needs some rewording.  Something like:

# NOTE: The no-ops are important in the following commands.  Building 
\`glog\` on WIN32 must be done in Visual Studio.  In that case, these commands 
must be no-ops.  If the no-ops are removed, then CMake will treat \`glog\` as 
an empty build command and will attemp to build \`glog\` as a CMake project.



3rdparty/libprocess/3rdparty/CMakeLists.txt (lines 72 - 84)


(This is just speculation, since I haven't tried this on a Windows box.)  
`TRUE` won't quite work on Windows, because it'll get run as a binary, and 
therefore won't be found.

Perhaps you should define a macro, like:

```
if (WIN32)
  set(CMAKE_NOOP ${CMAKE_COMMAND} -E echo)
else (WIN32)
  set(CMAKE_NOOP TRUE)
endif (WIN32)
```

The `-E echo` is fairly close to a no-op.

See: http://public.kitware.com/pipermail/cmake/2014-June/057885.html



3rdparty/libprocess/3rdparty/CMakeLists.txt (line 175)


For clarity, I think you should comment that this function overwrites the 
`GMOCK_BUILD_CMD`.  Same for the other usages.



3rdparty/libprocess/3rdparty/CMakeLists.txt (line 189)


Comment, same reason as above.



3rdparty/libprocess/cmake/macros/VsBuildCommand.bat (line 19)


I think the mesos style should still apply here.  So you should add a 
period at the end of all comments.



3rdparty/libprocess/cmake/macros/VsBuildCommand.bat (line 24)


Delimited



3rdparty/libprocess/cmake/macros/VsBuildCommand.bat (lines 30 - 40)


Is there any cleaner way of finding the MSVC install directory?

Try reading the directory from the registry:
 * 
http://stackoverflow.com/questions/445167/how-can-i-get-the-value-of-a-registry-key-from-within-a-batch-script
 * http://pascoal.net/2011/04/getting-visual-studio-installation-directory/



3rdparty/libprocess/cmake/macros/VsBuildCommand.bat (line 35)


Period at the end.



3rdparty/libprocess/cmake/macros/VsBuildCommand.bat (lines 39 - 43)


Instead of the `goto`, use `ELSE`:

```
) ELSE ( 
  exit
)
```


- Joseph Wu


On Aug. 30, 2015, 6:42 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37273/
> ---
> 
> (Updated Aug. 30, 2015, 6:42 a.m.)
> 
> 
> Review request for mesos and Alex Clemmer.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add CMake macro VsBuildCommand in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 997cc0d0e316e316136d4746e50e9e292a82b36b 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> 12506a1369de005285268f895f365aba0c560f78 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
>   3rdparty/libprocess/cmake/macros/VsBuildCommand.bat PRE-CREATION 
>   3rdparty/libprocess/cmake/macros/VsBuildCommand.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37273/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37275: [2/2]Generate make batch file to build project in windows.

[Joseph Wu]

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



cmake/MesosConfigure.cmake (line 76)


This comment doesn't need to change.  If anything, you could add an extra 
line, like:
```
Your version of MSVC is ${MSVC_VERSION}.
```
Or:
```
Mesos does not support compiling on MSVC version ${MSVC_VERSION}, which is 
earlier than version 1900.
```



cmake/MesosConfigure.cmake (line 125)


Why do you need this?  (If you do need it, comment why.)

It doesn't look like the inputs have any semicolons.


- Joseph Wu


On Aug. 30, 2015, 6:42 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37275/
> ---
> 
> (Updated Aug. 30, 2015, 6:42 a.m.)
> 
> 
> Review request for mesos and Alex Clemmer.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Generate make batch file to build project in windows.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
> 
> Diff: https://reviews.apache.org/r/37275/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37826: Introduced conversion of JSON arrays to repeated protobufs.

[Joseph Wu]

> On Aug. 31, 2015, 11:20 a.m., Michael Park wrote:
> > What do you think of the following API?
> > 
> > ```
> > JSON::Value value = ...;
> > auto message = protobuf::parse(value);  // message has type Try.
> > auto repeated = 
> > protobuf::parse(value);  // repeated 
> > has type google::protobuf::RepeatedPtrField.
> > ```
> > 
> > This makes it so that `parse` always returns `Try`, which I think 
> > enables the use of `auto` for the result under our style guide.
> > I think this is a simpler pattern to remember than `parse` -> `Try` 
> > and `parseRepeated` -> `Try`.
> > 
> > It would take `JSON::Value` rather than `JSON::Array` but check that it is 
> > indeed an instance of `JSON::Array`,
> > the same way the existing version takes `JSON::Value` and checks that it's 
> > an instance of `JSON::Object`.
> > 
> > It also makes the API symmetric with `read()`. We simply do `read` or 
> > `read` and it does the right thing.
> 
> Michael Park wrote:
> __NOTE:__ `parseRepeated` is actually @joseph's suggestion in the 
> subsequent patch.
> 
> In this patch, `parse(value)` returns `Try` and `parse(array)` 
> returns `Try`,
> which I think is just as hard if not harder to intuitively deduce.
> 
> What do you think?

I think that would be good.  It seems more verbose, but also a lot more 
explicit.

(But I can imagine that it might not fit in 80 characters in some places...)


- Joseph


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


On Aug. 26, 2015, 8:38 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37826/
> ---
> 
> (Updated Aug. 26, 2015, 8:38 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 57d5fdf45273c620655b44b5f5572290cffa4bf6 
> 
> Diff: https://reviews.apache.org/r/37826/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 37585: Maintenance primitives: Add a user doc.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37585]

All tests passed.

- Mesos ReviewBot


On Aug. 31, 2015, 6:11 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37585/
> ---
> 
> (Updated Aug. 31, 2015, 6:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, 
> Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2083
> https://issues.apache.org/jira/browse/MESOS-2083
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Heavily based on the design doc 
> (https://docs.google.com/document/d/16k0lVwpSGVOyxPSyXKmGC-gbNmRlisNEe4p-fAUSojk/).
> 
> Includes a diagram of the maintenance mode transitions.
> 
> This documents the current working prototype (MVP), which starts here:
> https://reviews.apache.org/r/36321/
> 
> One TODO remaining: Update with the release version that maintenance 
> primitives will be released in.
> 
> 
> Diffs
> -
> 
>   docs/images/maintenance-primitives-modes.png PRE-CREATION 
>   docs/maintenance.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37585/diff/
> 
> 
> Testing
> ---
> 
> Copied to: https://gist.github.com/kaysoky/b9789c88ee204e3b49a2
> Checked for markdown correctness.
> 
> 
> File Attachments
> 
> 
> Same as the image in the binary diff.  (Uploaded for reviewer convenience.)
>   
> https://reviews.apache.org/media/uploaded/files/2015/08/31/ce897571-0d3a-4ecb-8d36-a63eff855647__maintenance-primitives-modes.png
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 37416: Perf supported() should be based on the version of perf, not the version of the kernel.

[Paul Brett]

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

(Updated Aug. 31, 2015, 7:41 p.m.)


Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Perf supported() should be based on the version of perf, not the version of the 
kernel.


Diffs (updated)
-

  src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 37894: Renamed cgroups::kill to cgroups::signal.

[Ben Mahler]

> On Aug. 28, 2015, 4:45 p.m., Cong Wang wrote:
> > Why? Everyone knows kill(2) sends a signal while signal(2) installs a 
> > signal handler...
> 
> Jie Yu wrote:
> IMO, that naming is confusing, and should be 'signal' and 'install'.
> 
> Joerg Schad wrote:
> This is actually answering a discussion here 
> https://reviews.apache.org/r/36620/#comment152376. And I agree that the 
> original names are weird, but you are right in that we should have an open 
> discussion about this. I will send a mail to dev later on...
> 
> Cong Wang wrote:
> Like it or not, it is already a part of POSIX (if not just UNIX), 
> pthread_kill() has the same "problem". It's just too late to change, as it 
> simply breaks every programmer's expectation.
> 
> Jie Yu wrote:
> This is cgroups::signal and cgroups::kill (has nothing to do with Posix 
> kill and signal).
> 
> It reads more natural: signal a cgroup, kill a cgroup. I don't think 
> it'll confuse the reader.
> 
> Are you suggesting that any thing suffixed with 'kill' should be linked 
> to kill(2) in posix? I have to disagree with that.
> 
> Timothy Chen wrote:
> I think I personally don't think we need to tie with POSIX, just need 
> some comments to clarify what the intent is and I personally like to see this 
> rename as well. Don't want this to sit and get stale again, Jie/Cong is there 
> more to add or can we agree on this change?

What's the plan for other utilities, like `os::killtree` and `os::kill`? Jie, 
what does it mean to "kill" a cgroup, send SIGKILL? Or destroy it completely? I 
thought that's why we have `cgroups::destroy`..?


- Ben


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


On Aug. 28, 2015, 4:39 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37894/
> ---
> 
> (Updated Aug. 28, 2015, 4:39 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Till Toenshoff, and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As this function is actually just sending the signal we renamed it. 
> Additionally we want to introduce an actual cgroups::kill() with MESOS-3086.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 204c53038a1ccfa693f4f2293488cff8cdd60835 
>   src/linux/cgroups.cpp 6ef42ed1bc719f334d1ac6e90919a1bc1840d31f 
>   src/tests/containerizer/cgroups_tests.cpp 
> 0b171eeb53037f26b7e952830e88e59f1278e7c6 
> 
> Diff: https://reviews.apache.org/r/37894/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36321: Maintenance primitives: Add Unavailability and InverseOffer protobufs.

[Ben Mahler]

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



include/mesos/mesos.proto (lines 140 - 154)


Should this have been:

```
message Interval {
  required TimeInfo start;
  optional DurationInfo duration;
}

message Unavailability {
  required Interval interval;
  
  // Add additional fields related to unavailability.
}
```

Curious if we're going to have to duplicate Interval elsewhere.



include/mesos/mesos.proto (lines 949 - 963)


Does this need to be required? For example, if we'd like to do ASAP 
de-allocation in the future, what would that look like?

Would we be omitting an unavailability? Or would we be setting the start 
time to 'now'? Also, would we need additional information in the inverse offer 
(e.g. hard vs soft?). Food for thought.


- Ben Mahler


On Aug. 28, 2015, 5:11 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36321/
> ---
> 
> (Updated Aug. 28, 2015, 5:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, 
> Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2061 and MESOS-2066
> https://issues.apache.org/jira/browse/MESOS-2061
> https://issues.apache.org/jira/browse/MESOS-2066
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-2061: Add Unavailability and InverseOffer protobufs declarations.
> MESOS-2066: Add the Unavailability field to Offers.
> 
> Also copied to v1 API.
> 
> No integration with other components (that part is tracked in separate JIRAs, 
> see MESOS-1474).
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto c40a09b2bb2b9444e9b90e86eefe6fc8f98b613d 
>   include/mesos/v1/mesos.proto ee15b9ae70aeb8cf803ea7e4b06f4443bd7bc9e2 
> 
> Diff: https://reviews.apache.org/r/36321/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 37197: Docker image store.

[Timothy Chen]

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



src/slave/containerizer/provisioners/docker/store.cpp (line 131)


For local store I don't think this comment is valid.
Should just comment that only local images with file:// is supported.



src/slave/containerizer/provisioners/docker/store.cpp (line 150)


does path::join return a Try? I was curious about this of what kind of 
error can it come back with, and I think it actually just returns a String.



src/slave/containerizer/provisioners/docker/store.cpp (line 192)


I think path::join just returns a string



src/slave/containerizer/provisioners/docker/store.cpp (line 247)


If tar is terminated in the middle, will this operation succeed again? We 
only check the directory exist to signal that we untarred the layer, but it 
might not necessarily really finished.
I Think we will have to do a mv here as well


- Timothy Chen


On Aug. 27, 2015, 11:39 p.m., Lily Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37197/
> ---
> 
> (Updated Aug. 27, 2015, 11:39 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Till Toenshoff, 
> and Timothy Chen.
> 
> 
> Bugs: MESOS-2849
> https://issues.apache.org/jira/browse/MESOS-2849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stored images currently kept indefinitely.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION 
>   src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 
>   src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 
> 
> Diff: https://reviews.apache.org/r/37197/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Lily Chen
> 
>

Re: Review Request 37787: Reworked Jenkins build script. Added test images for ubuntu-14.04-clang-3.6, ubuntu-12.04-gcc-4.8, centos-6.6-gcc-4.8.

[Timothy Chen]

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

Ship it!


Ship It!


support/jenkins_build_docker.sh (line 28)


Btw why not remove the image on the end? This means we'll pile up images 
every run right?


- Timothy Chen


On Aug. 28, 2015, 7:51 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37787/
> ---
> 
> (Updated Aug. 28, 2015, 7:51 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Timothy Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-3313
> https://issues.apache.org/jira/browse/MESOS-3313
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reworked Jenkins build script. Added test images for ubuntu-14.04-clang-3.6, 
> ubuntu-12.04-gcc-4.8, centos-6.6-gcc-4.8.
> 
> 
> Diffs
> -
> 
>   support/docker/centos-7.1-gcc-4.8/Dockerfile PRE-CREATION 
>   support/docker/ubuntu-12.04-gcc-4.8/Dockerfile PRE-CREATION 
>   support/docker/ubuntu-14.04-clang-3.6/Dockerfile PRE-CREATION 
>   support/docker/ubuntu-14.04-gcc-4.8/Dockerfile PRE-CREATION 
>   support/jenkins_build_docker.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37787/diff/
> 
> 
> Testing
> ---
> 
> `jenkins-build-docker.sh` is a reworked version of the original 
> `jenkins-build.sh` that is ran by Jenkins buildbot for building and testing 
> Mesos distributions. 
> 
> Features:
>  * Runs libevent, SSL and ROOT tests.
>  * Easily add OS/compiler Docker images for testing Mesos.
>  * Exclude tests on per-image basis.
>  * Easily reproduce the test image locally.
>  * Three new test images (ubuntu-14.04-clang-3.6, ubuntu-12.04-gcc-4.8, 
> centos-6.6-gcc-4.8).
> 
> How to run
> 
> The following environment variables have to be set for the script to run:
>  * OS - OS name/version. Currently images are available for ubuntu-14.04, 
> ubuntu-12.04, centos-7.1.
>  * CONFIGURATION - ./configure flags (e.g. '--enable-libevent').
>  * COMPILER - Compiler name/version. Currently available images include 
> gcc-4.8 (default value) on all platforms, clang-3.6 on ubuntu-14.04.
> 
> Examples:
> `OS=ubuntu-14.04 CONFIGURATION='--enable-ssl --enable-libevent' 
> COMPILER=clang-3.6 ./jenkins_build_docker.sh`
> `OS=centos-7.1 CONFIGURATION='--enable-ssl --enable-libevent' 
> ./jenkins_build_docker.sh`
> 
> NOTE: Mesos Python module has a known issue on centos-6.6 ( 
> https://issues.apache.org/jira/browse/MESOS-3314 ), so **for now centos-6.6 
> should not be enabled in Jenkins**. Docker image is removed from the final 
> review (it will be added later once the issue is resolved).
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 37908: Silence oversubscription logging

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37908]

All tests passed.

- Mesos ReviewBot


On Aug. 31, 2015, 6:28 p.m., Niklas Nielsen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37908/
> ---
> 
> (Updated Aug. 31, 2015, 6:28 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Resource estimates are continuously written to the logs even though they are 
> empty (as with the majority of Mesos clusters at the moment). To reduce the 
> noise, this patch moves log lines for empty resources to VLOG(1).
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 8cfad7b86675b10c98524174975828bbcf47aed2 
> 
> Diff: https://reviews.apache.org/r/37908/diff/
> 
> 
> Testing
> ---
> 
> make check - with and without the patch, verifying the log statements.
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>

Re: Review Request 37826: Introduced conversion of JSON arrays to repeated protobufs.

[Michael Park]

> On Aug. 31, 2015, 6:20 p.m., Michael Park wrote:
> > What do you think of the following API?
> > 
> > ```
> > JSON::Value value = ...;
> > auto message = protobuf::parse(value);  // message has type Try.
> > auto repeated = 
> > protobuf::parse(value);  // repeated 
> > has type google::protobuf::RepeatedPtrField.
> > ```
> > 
> > This makes it so that `parse` always returns `Try`, which I think 
> > enables the use of `auto` for the result under our style guide.
> > I think this is a simpler pattern to remember than `parse` -> `Try` 
> > and `parseRepeated` -> `Try`.
> > 
> > It would take `JSON::Value` rather than `JSON::Array` but check that it is 
> > indeed an instance of `JSON::Array`,
> > the same way the existing version takes `JSON::Value` and checks that it's 
> > an instance of `JSON::Object`.
> > 
> > It also makes the API symmetric with `read()`. We simply do `read` or 
> > `read` and it does the right thing.
> 
> Michael Park wrote:
> __NOTE:__ `parseRepeated` is actually @joseph's suggestion in the 
> subsequent patch.
> 
> In this patch, `parse(value)` returns `Try` and `parse(array)` 
> returns `Try`,
> which I think is just as hard if not harder to intuitively deduce.
> 
> What do you think?
> 
> Joseph Wu wrote:
> I think that would be good.  It seems more verbose, but also a lot more 
> explicit.
> 
> (But I can imagine that it might not fit in 80 characters in some 
> places...)

> (But I can imagine that it might not fit in 80 characters in some places...)

Yeah, but I think that's fine. It's the case with most uses of 
`google::protobuf::RepeatedPtrField` across the codebase currently.
We can also do `using google::protobuf::RepeatedPtrField;` in places where 
possible.


- Michael


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


On Aug. 27, 2015, 3:38 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37826/
> ---
> 
> (Updated Aug. 27, 2015, 3:38 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 57d5fdf45273c620655b44b5f5572290cffa4bf6 
> 
> Diff: https://reviews.apache.org/r/37826/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 36321: Maintenance primitives: Add Unavailability and InverseOffer protobufs.

[Joseph Wu]

> On Aug. 31, 2015, 1:06 p.m., Ben Mahler wrote:
> > include/mesos/mesos.proto, lines 140-154
> > 
> >
> > Should this have been:
> > 
> > ```
> > message Interval {
> >   required TimeInfo start;
> >   optional DurationInfo duration;
> > }
> > 
> > message Unavailability {
> >   required Interval interval;
> >   
> >   // Add additional fields related to unavailability.
> > }
> > ```
> > 
> > Curious if we're going to have to duplicate Interval elsewhere.

It was difficult to decide which combination of required/optional was 
appropriate for a generic `Interval` message.

Would it be a strict definition (required start and duration), the existing one 
(required start, optional duration, no duration == infinite), or a lax one 
(optional start and duration)?  Each definition has slightly different 
connotations and I didn't want to lock `Interval` into any one of them, just 
because we use it for this feature.


> On Aug. 31, 2015, 1:06 p.m., Ben Mahler wrote:
> > include/mesos/mesos.proto, lines 949-963
> > 
> >
> > Does this need to be required? For example, if we'd like to do ASAP 
> > de-allocation in the future, what would that look like?
> > 
> > Would we be omitting an unavailability? Or would we be setting the 
> > start time to 'now'? Also, would we need additional information in the 
> > inverse offer (e.g. hard vs soft?). Food for thought.

Note: In the MVP, the "emergency" maintenance workflow is still to go from UP 
-> DRAINING -> DOWN (Normal -> Draining -> Deactivated, if we use the design 
doc's terminology), with an Unavailability set to "now".  

The unavailability is required because:
* We want as much info as possible.  And it's not painful to supply.
* The master uses (the presence of) Unavailability for some logic.  If we 
didn't have it required, we might end up with an awkward 
`Option`.
* Presumably, additional information (hard/soft, etc) would go in the 
Unavailability and thereby end up in the Offer and InverseOffer.


- Joseph


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


On Aug. 28, 2015, 10:11 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36321/
> ---
> 
> (Updated Aug. 28, 2015, 10:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, 
> Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2061 and MESOS-2066
> https://issues.apache.org/jira/browse/MESOS-2061
> https://issues.apache.org/jira/browse/MESOS-2066
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-2061: Add Unavailability and InverseOffer protobufs declarations.
> MESOS-2066: Add the Unavailability field to Offers.
> 
> Also copied to v1 API.
> 
> No integration with other components (that part is tracked in separate JIRAs, 
> see MESOS-1474).
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto c40a09b2bb2b9444e9b90e86eefe6fc8f98b613d 
>   include/mesos/v1/mesos.proto ee15b9ae70aeb8cf803ea7e4b06f4443bd7bc9e2 
> 
> Diff: https://reviews.apache.org/r/36321/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

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

[Ben Mahler]

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


This seems too tedious to fix throughout the code base in all of our logging 
messages, I'd suggest discarding.

- Ben Mahler


On Aug. 31, 2015, 11:04 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36441/
> ---
> 
> (Updated Aug. 31, 2015, 11:04 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> 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 37787: Reworked Jenkins build script. Added test images for ubuntu-14.04-gcc-4.8, ubuntu-14.04-clang-3.6, ubuntu-12.04-gcc-4.8, centos-7.1-gcc-4.8.

[Artem Harutyunyan]

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

(Updated Aug. 31, 2015, 2:15 p.m.)


Review request for mesos, Benjamin Hindman, Timothy Chen, and Vinod Kone.


Changes
---

Addressed comments; fixed commit message.


Summary (updated)
-

Reworked Jenkins build script. Added test images for ubuntu-14.04-gcc-4.8, 
ubuntu-14.04-clang-3.6, ubuntu-12.04-gcc-4.8, centos-7.1-gcc-4.8.


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


Repository: mesos


Description (updated)
---

See summary.


Diffs (updated)
-

  support/docker/centos-7.1-gcc-4.8/Dockerfile PRE-CREATION 
  support/docker/ubuntu-12.04-gcc-4.8/Dockerfile PRE-CREATION 
  support/docker/ubuntu-14.04-clang-3.6/Dockerfile PRE-CREATION 
  support/docker/ubuntu-14.04-gcc-4.8/Dockerfile PRE-CREATION 
  support/jenkins_build_docker.sh PRE-CREATION 

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


Testing (updated)
---

`jenkins-build-docker.sh` is a reworked version of the original 
`jenkins-build.sh` that is ran by Jenkins buildbot for building and testing 
Mesos distributions. 

Features:
 * Runs libevent, SSL and ROOT tests.
 * Easily add OS/compiler Docker images for testing Mesos.
 * Exclude tests on per-image basis.
 * Easily reproduce the test image locally.
 * Three new test images (ubuntu-14.04-gcc-4.8, ubuntu-14.04-clang-3.6, 
ubuntu-12.04-gcc-4.8, centos-7.1-gcc-4.8).

How to run

The following environment variables have to be set for the script to run:
 * OS - OS name/version. Currently images are available for ubuntu-14.04, 
ubuntu-12.04, centos-7.1.
 * CONFIGURATION - ./configure flags (e.g. '--enable-libevent').
 * COMPILER - Compiler name/version. Currently available images include gcc-4.8 
(default value) on all platforms, clang-3.6 on ubuntu-14.04.

Examples:
`OS=ubuntu-14.04 CONFIGURATION='--enable-ssl --enable-libevent' 
COMPILER=clang-3.6 ./jenkins_build_docker.sh`
`OS=centos-7.1 CONFIGURATION='--enable-ssl --enable-libevent' 
./jenkins_build_docker.sh`


Thanks,

Artem Harutyunyan

Re: Review Request 37945: change const pass-by-value to const reference in stout

[Michael Park]

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


@Guangya: I pointed out the undefined behavior as specified in the C++ Standard 
in this review: [r25735](https://reviews.apache.org/r/25735/). Also, refer to  
[MESOS-3246](https://issues.apache.org/jira/browse/MESOS-3246) which I filed in 
addition to adding the comment.

- Michael Park


On Aug. 31, 2015, 8:55 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37945/
> ---
> 
> (Updated Aug. 31, 2015, 8:55 p.m.)
> 
> 
> Review request for mesos, switched to 'mcypark' and Vinod Kone.
> 
> 
> Bugs: MESOS-1805
> https://issues.apache.org/jira/browse/MESOS-1805
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> change const pass-by-value to const reference in stout
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/format.hpp 
> 1cf6dd18aa163688d6c8f3a6d33eacad3918015d 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp 
> 68fc1fd179ee51fc5de0452c0f2ea3d354e0567f 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp 
> 01e59de466496dec9367ad6f48538327f53a7e18 
> 
> Diff: https://reviews.apache.org/r/37945/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 37968: Add more comments for why not using const reference for some functions

[Guangya Liu]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Add more comments for why not using const reference for some functions


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/format.hpp 
1cf6dd18aa163688d6c8f3a6d33eacad3918015d 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp 
68fc1fd179ee51fc5de0452c0f2ea3d354e0567f 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp 
01e59de466496dec9367ad6f48538327f53a7e18 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 37945: change const pass-by-value to const reference in stout

[Michael Park]

> On Aug. 31, 2015, 9:25 p.m., Michael Park wrote:
> > @Guangya: I pointed out the undefined behavior as specified in the C++ 
> > Standard in this review: [r25735](https://reviews.apache.org/r/25735/). 
> > Also, refer to  
> > [MESOS-3246](https://issues.apache.org/jira/browse/MESOS-3246) which I 
> > filed in addition to adding the comment.
> 
> Michael Park wrote:
> P.S. I would be happy to shepherd 
> [MESOS-3246](https://issues.apache.org/jira/browse/MESOS-3246) if you would 
> like to tackle it.

P.P.S. I'm `mcypark` on ReviewBoard.


- Michael


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


On Aug. 31, 2015, 8:55 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37945/
> ---
> 
> (Updated Aug. 31, 2015, 8:55 p.m.)
> 
> 
> Review request for mesos, switched to 'mcypark' and Vinod Kone.
> 
> 
> Bugs: MESOS-1805
> https://issues.apache.org/jira/browse/MESOS-1805
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> change const pass-by-value to const reference in stout
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/format.hpp 
> 1cf6dd18aa163688d6c8f3a6d33eacad3918015d 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp 
> 68fc1fd179ee51fc5de0452c0f2ea3d354e0567f 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp 
> 01e59de466496dec9367ad6f48538327f53a7e18 
> 
> Diff: https://reviews.apache.org/r/37945/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 37955: Remove hashmap::existsValue since it is never called

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 八月 31, 2015, 3:42 p.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37955/
> ---
> 
> (Updated 八月 31, 2015, 3:42 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3329
> https://issues.apache.org/jira/browse/MESOS-3329
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Remove hashmap::existsValue since it is never called
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp 
> f09bea125035aa3621402b83608b233e42877559 
> 
> Diff: https://reviews.apache.org/r/37955/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 37945: change const pass-by-value to const reference in stout

[Guangya Liu]

> On 八月 31, 2015, 9:25 p.m., Michael Park wrote:
> > @Guangya: I pointed out the undefined behavior as specified in the C++ 
> > Standard in this review: [r25735](https://reviews.apache.org/r/25735/). 
> > Also, refer to  
> > [MESOS-3246](https://issues.apache.org/jira/browse/MESOS-3246) which I 
> > filed in addition to adding the comment.
> 
> Michael Park wrote:
> P.S. I would be happy to shepherd 
> [MESOS-3246](https://issues.apache.org/jira/browse/MESOS-3246) if you would 
> like to tackle it.
> 
> Michael Park wrote:
> P.P.S. I'm `mcypark` on ReviewBoard.

Thanks @mcypark, I have filed a new issue 
https://issues.apache.org/jira/browse/MESOS-3344 and want to add more comments 
there for why we are not using const reference for those APIs, and I have set 
you as shepherd, hope you can take a look. I will check MESOS-3246 when have 
time slot, Thanks


- Guangya


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


On 八月 31, 2015, 8:55 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37945/
> ---
> 
> (Updated 八月 31, 2015, 8:55 p.m.)
> 
> 
> Review request for mesos, switched to 'mcypark' and Vinod Kone.
> 
> 
> Bugs: MESOS-1805
> https://issues.apache.org/jira/browse/MESOS-1805
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> change const pass-by-value to const reference in stout
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/format.hpp 
> 1cf6dd18aa163688d6c8f3a6d33eacad3918015d 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp 
> 68fc1fd179ee51fc5de0452c0f2ea3d354e0567f 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp 
> 01e59de466496dec9367ad6f48538327f53a7e18 
> 
> Diff: https://reviews.apache.org/r/37945/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 37921: Add Copy backend for provisioners.

[Timothy Chen]

> On Aug. 31, 2015, 5:50 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/provisioners/backends/copy.cpp, line 19
> > 
> >
> > Move this below.
> > 
> > ```
> > #include "common/status_utils.hpp"
> > 
> > #include "slave/containerizer/provisioners/backends/copy.hpp"
> > ```

I think our newest style guide points that we include the cpp's main header 
file on the top, but I'm fine moving this down.


> On Aug. 31, 2015, 5:50 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/provisioners/backends/copy.cpp, lines 127-132
> > 
> >
> > Using shell expansion here has others issues:
> > 
> > 1) Spaces (or other $IFS) in the paths
> > 2) Not sure if filename expansions works the same way across shells.
> > 
> > GNU `cp` has a `--no-target-directory` flag that's handy in this 
> > situation but Mac `cp` doesn't have it.
> > 
> > So I think we should probably just rely on a precondition check: return 
> > Failure if os::exists(rootfs) already exists and remove the mkdir in 
> > `provision()`. This allows us to just use the *argv* form of `subprocess` 
> > with `{"cp", "-a", layer, rootfs}`.
> > 
> > AppcProvisioner already expects the Backend to create the `rootfs` and 
> > DockerProvisioner can do the same. 
> > 
> > We can emphaize this by adding a note to `Backend::provision()` 
> > documentation.
> > 
> > Thoughts?

Ok as long we can make sure provisioners output the same layout I definitely 
prefer just doing a straight copy!
I'll make sure the basename of all layers are the same.


- Timothy


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


On Aug. 29, 2015, 7:59 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37921/
> ---
> 
> (Updated Aug. 29, 2015, 7:59 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add Copy backend for provisioners.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/slave/containerizer/provisioners/backend.cpp 
> 2f7c335f62fdeb27526ab9a38a07c097422ae92b 
>   src/slave/containerizer/provisioners/backends/copy.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/backends/copy.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> d321850613223a2357ca1646a9d988d05171772c 
> 
> Diff: https://reviews.apache.org/r/37921/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 37773: Docker: Adding registry client.

[Jojy Varghese]

> On Aug. 31, 2015, 8:42 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioners/docker/registry_client.cpp, line 456
> > 
> >
> > is maxSize being used at all? If not we should remove all references of 
> > it.

I added it as part of the external facing interface so that it doesnt change 
when we add validation of max size.


- Jojy


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


On Aug. 30, 2015, 3:12 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37773/
> ---
> 
> (Updated Aug. 30, 2015, 3:12 p.m.)
> 
> 
> Review request for mesos, Lily Chen and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added implementation for docker registry's Get Manifest and Get Blob APIs.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/slave/containerizer/provisioners/docker/registry_client.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/registry_client.cpp 
> PRE-CREATION 
>   src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37773/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 37416: Perf supported() should be based on the version of perf, not the version of the kernel.

[Paul Brett]

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

(Updated Aug. 31, 2015, 10:14 p.m.)


Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Perf supported() should be based on the version of perf, not the version of the 
kernel.


Diffs (updated)
-

  src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 37416: Perf supported() should be based on the version of perf, not the version of the kernel.

[Paul Brett]

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

(Updated Aug. 31, 2015, 10:27 p.m.)


Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Perf supported() should be based on the version of perf, not the version of the 
kernel.


Diffs
-

  src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 37416: Perf supported() should be based on the version of perf, not the version of the kernel.

[Paul Brett]

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

(Updated Aug. 31, 2015, 10:27 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Rebased


Repository: mesos


Description
---

Perf supported() should be based on the version of perf, not the version of the 
kernel.


Diffs (updated)
-

  src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 37903: stout: Fix bug in IPNetwork::create() with zero prefix.

[Cong Wang]

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

Ship it!


Or we can simply make 'prefix' unsigned.

- Cong Wang


On Aug. 28, 2015, 8:02 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37903/
> ---
> 
> (Updated Aug. 28, 2015, 8:02 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3328
> https://issues.apache.org/jira/browse/MESOS-3328
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous coding would try to shift a uint32_t value 32 bits to the left; 
> per
> C++ spec, this yields undefined behavior. On my machine, this resulted in
> treating a prefix of 0 as equivalent to a prefix of 32, which is obviously
> wrong.
> 
> Spotted via ubsan: see MESOS-3328.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
> 1ad119d54820e97497b1773518875be25ddbf98a 
>   3rdparty/libprocess/3rdparty/stout/tests/ip_tests.cpp 
> b0cbcb38cfcb923ec7c185bacf139ceb0a28924f 
> 
> Diff: https://reviews.apache.org/r/37903/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 37814: Added documentation for libprocess environment variables

[Greg Mann]

> On Aug. 31, 2015, 9:45 p.m., Michael Park wrote:
> > docs/configuration.md, line 1505
> > 
> >
> > Below in the `Some influential environment variables for configure 
> > script:` section we list environment variables as `Flags`. Do we maybe want 
> > to keep it consistent?

Thanks Michael! Comments addressed.


- Greg


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


On Aug. 28, 2015, 10:25 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37814/
> ---
> 
> (Updated Aug. 28, 2015, 10:25 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and haosdent huang.
> 
> 
> Bugs: MESOS-2466
> https://issues.apache.org/jira/browse/MESOS-2466
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for libprocess environment variables
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
> 
> Diff: https://reviews.apache.org/r/37814/diff/
> 
> 
> Testing
> ---
> 
> Built the updated website using the docker site builder 
> (https://github.com/mesosphere/mesos-website-container).
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37442: Factor out the token extraction rules in prepartion for extending them to cope with multiple versions.

[Paul Brett]

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

(Updated Aug. 31, 2015, 10:33 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Rebased


Repository: mesos


Description
---

Factor out the token extraction rules in prepartion for extending them to cope 
with multiple versions.


Diffs (updated)
-

  src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 37427: Docker registry: adding TokenManager.

[Jojy Varghese]

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

(Updated Aug. 31, 2015, 10:39 p.m.)


Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen.


Changes
---

review comments addressed.


Repository: mesos


Description
---

Changes:
  - Added Token implementation (RFC 7519).
  - Added TokenManager implementation. This component keeps a cache of tokens
  requested for any future requests.


Diffs (updated)
-

  src/Makefile.am 7b4d9f65506e7fa8425966009401aae73cdb79a5 
  src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION 
  src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 37024: Exposes mesos version information in components.

[Ben Mahler]

> On Aug. 3, 2015, 7:37 p.m., Ben Mahler wrote:
> > src/master/http.cpp, lines 526-542
> > 
> >
> > Could you include these fields as well? They capture "version" / 
> > "build" information. Once you do, perhaps it is worth adding a 
> > '`JSON::Object version()`' helper in common/http.hpp.
> 
> Marco Massenzio wrote:
> great point.
> also wondering if, at this point, it would make sense to have a Protobuf 
> `VersionInfo` (that we could also use to replace the `version` string in 
> `MasterInfo`) which can then be used everywhere (and gets auto-converted to 
> JSON)?
> 
> haosdent huang wrote:
> Thank you @marco, it is OK for me. What's @bmahler opinion? :-P

Sounds good Marco! I think we'll punt on that in this review since it sounds 
like a change we could make independently of the /version endpoint.


- Ben


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


On Aug. 16, 2015, 5:38 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37024/
> ---
> 
> (Updated Aug. 16, 2015, 5:38 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1841
> https://issues.apache.org/jira/browse/MESOS-1841
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add an endpoint that exposes component version.
> 
> 
> Diffs
> -
> 
>   include/mesos/executor.hpp 72eca97dd84fb1300b37764a3ef3a57fb5e676c2 
>   include/mesos/scheduler.hpp ee198b6955882f4f31466ca05429ca16fbf2f5cd 
>   src/Makefile.am 457ad26ee55bd7a2aedf27f45db58a9a4a6a5dc5 
>   src/exec/exec.cpp 31e0c2f17a9092d18285828111d27628fb07bc02 
>   src/local/local.cpp 4d98bf23705027f3ba0cbb571289f21b288fe7db 
>   src/master/main.cpp bafc605d6c20bd264b932e44ee80373a3f692734 
>   src/sched/sched.cpp 012af0508eeceeccd168b29f36fa258d20b28c21 
>   src/slave/main.cpp 364dc7fc7ab2e3cef01aea7267dafa014b60e2b9 
>   src/version/version_info.hpp PRE-CREATION 
>   src/version/version_info.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37024/diff/
> 
> 
> Testing
> ---
> 
> Manual test result:
> 
> ```
> $ curl http://localhost:5050/version 2>/dev/null|jq .
> 
> {
>   "version": "0.24.0",
>   "build_user": "haosdent",
>   "build_time": 1439702338,
>   "build_date": "2015-08-16 13:18:58"
> }
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37929: Changed provisioner Store API and implementation so it works as a read-through cache.

[Jiang Yan Xu]

> On Aug. 31, 2015, 2:42 p.m., Chi Zhang wrote:
> > src/slave/containerizer/provisioners/appc/store.hpp, lines 76-81
> > 
> >
> > Maybe not yet the guarantee on ordering since we don't support 
> > dependency in images.
> > 
> > Mainly I think if we don't have to worry about ordering, it is simpler.
> > 
> > If backends can't get away from that, we should then be more explicit 
> > on what kind of ordering we provie. BFS, first order traversal or sth else?

This comment is about what the method SHOULD do. And the TODO below says: OK we 
are actually not doing it just yet. :)

It's important to be clear about what we WILL do in this case because this is 
the contract between the provisioner and the store. We need to determine right 
now where the logic should go in order to make sure the interface is future 
proof.

The backend is told about the ordering because it itself doesn't have enough 
information to otherwise detect it.

The ordering is basically "what overwites what in the fact of conflict" so it's 
not BFS or how we RESOLVE the dependencies.


- Jiang Yan


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


On Aug. 31, 2015, 10:45 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37929/
> ---
> 
> (Updated Aug. 31, 2015, 10:45 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - i.e., It fetches images transparently when it's not in the local cache.
> - This way, the store doesn't have the sense of "localness" anymore but 
> rather it's an abstraction that provides access to all discoverable images, 
> no matter where they can be found.
> 
> Some context for motivation of this change can be found at: 
> https://reviews.apache.org/r/37881/
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioners/appc/store.hpp 
> e48d91be06410bfc028a7b2ed88218e13adbffee 
>   src/slave/containerizer/provisioners/appc/store.cpp 
> fbd1c535d398a4d37c30ba23f5408095c7d35b65 
>   src/tests/containerizer/appc_provisioner_tests.cpp 
> 47b66b9c30cefe8f9a8e2c1c1341776c2d235020 
> 
> Diff: https://reviews.apache.org/r/37929/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 37968: Add more comments for why not using const reference for some functions

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37968]

All tests passed.

- Mesos ReviewBot


On Aug. 31, 2015, 9:25 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37968/
> ---
> 
> (Updated Aug. 31, 2015, 9:25 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3344
> https://issues.apache.org/jira/browse/MESOS-3344
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add more comments for why not using const reference for some functions
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/format.hpp 
> 1cf6dd18aa163688d6c8f3a6d33eacad3918015d 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp 
> 68fc1fd179ee51fc5de0452c0f2ea3d354e0567f 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp 
> 01e59de466496dec9367ad6f48538327f53a7e18 
> 
> Diff: https://reviews.apache.org/r/37968/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 37903: stout: Fix bug in IPNetwork::create() with zero prefix.

[Neil Conway]

> On Aug. 31, 2015, 10:28 p.m., Cong Wang wrote:
> > Or we can simply make 'prefix' unsigned.

Thanks for the review!

As far as I know, making "prefix" unsigned would not help. The code in question 
is: "0x << (32 - prefix)". 0x is an unsigned integer literal -- 
left-shifting a 32-bit integer (0x) by 32 bits is undefined, regardless 
of sign.


- Neil


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


On Aug. 28, 2015, 8:02 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37903/
> ---
> 
> (Updated Aug. 28, 2015, 8:02 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3328
> https://issues.apache.org/jira/browse/MESOS-3328
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous coding would try to shift a uint32_t value 32 bits to the left; 
> per
> C++ spec, this yields undefined behavior. On my machine, this resulted in
> treating a prefix of 0 as equivalent to a prefix of 32, which is obviously
> wrong.
> 
> Spotted via ubsan: see MESOS-3328.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
> 1ad119d54820e97497b1773518875be25ddbf98a 
>   3rdparty/libprocess/3rdparty/stout/tests/ip_tests.cpp 
> b0cbcb38cfcb923ec7c185bacf139ceb0a28924f 
> 
> Diff: https://reviews.apache.org/r/37903/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 37700: Document libsasl2-modules as a required dependency when building on ubuntu.

[Andrew Smith]

> On Aug. 31, 2015, 10:47 p.m., Vinod Kone wrote:
> > Our ASF Docker build that tests unbuntu 14.04 doesn't have this dependency 
> > explicitly specified! 
> > https://github.com/apache/mesos/blob/master/support/jenkins_build.sh
> > 
> > Are you sure the CRAM-MD5 module is not automatically installed when you 
> > install "libsasl2-dev"? cc @till

I wasn't able to proceed with the build until I added the aforementioned module 
in this issue. From the docs for the package (at 
http://packages.ubuntu.com/trusty/amd64/libsasl2-modules/filelist), it lists 
/usr/lib/x86_64-linux-gnu/sasl2/libcrammd5.so as a provided file.


- Andrew


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


On Aug. 22, 2015, 3:53 a.m., Andrew Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37700/
> ---
> 
> (Updated Aug. 22, 2015, 3:53 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3305
> https://issues.apache.org/jira/browse/MESOS-3305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Document libsasl2-modules as a required dependency when building on ubuntu.
> 
> 
> Diffs
> -
> 
>   docs/getting-started.md 1203ab88fde1c4a72c1bef6e85293f4d9abb2e24 
> 
> Diff: https://reviews.apache.org/r/37700/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Smith
> 
>

Re: Review Request 37921: Add Copy backend for provisioners.

[Timothy Chen]

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

(Updated Sept. 1, 2015, 12:56 a.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


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


Repository: mesos


Description
---

Add Copy backend for provisioners.


Diffs (updated)
-

  src/Makefile.am 7b4d9f65506e7fa8425966009401aae73cdb79a5 
  src/slave/containerizer/provisioners/backend.cpp 
2f7c335f62fdeb27526ab9a38a07c097422ae92b 
  src/slave/containerizer/provisioners/backends/copy.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/backends/copy.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_backend_tests.cpp 
d321850613223a2357ca1646a9d988d05171772c 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 37921: Add Copy backend for provisioners.

[Timothy Chen]

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

(Updated Sept. 1, 2015, 12:58 a.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


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


Repository: mesos


Description
---

Add Copy backend for provisioners.


Diffs (updated)
-

  src/Makefile.am 7b4d9f65506e7fa8425966009401aae73cdb79a5 
  src/slave/containerizer/provisioners/backend.cpp 
2f7c335f62fdeb27526ab9a38a07c097422ae92b 
  src/slave/containerizer/provisioners/backends/copy.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/backends/copy.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_backend_tests.cpp 
d321850613223a2357ca1646a9d988d05171772c 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 37700: Document libsasl2-modules as a required dependency when building on ubuntu.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37700]

All tests passed.

- Mesos ReviewBot


On Aug. 22, 2015, 3:53 a.m., Andrew Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37700/
> ---
> 
> (Updated Aug. 22, 2015, 3:53 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3305
> https://issues.apache.org/jira/browse/MESOS-3305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Document libsasl2-modules as a required dependency when building on ubuntu.
> 
> 
> Diffs
> -
> 
>   docs/getting-started.md 1203ab88fde1c4a72c1bef6e85293f4d9abb2e24 
> 
> Diff: https://reviews.apache.org/r/37700/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Smith
> 
>

Re: Review Request 37814: Added documentation for libprocess environment variables

[Alexander Rojas]

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

Ship it!


Ship It!

- Alexander Rojas


On Aug. 29, 2015, 12:25 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37814/
> ---
> 
> (Updated Aug. 29, 2015, 12:25 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas and haosdent huang.
> 
> 
> Bugs: MESOS-2466
> https://issues.apache.org/jira/browse/MESOS-2466
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for libprocess environment variables
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
> 
> Diff: https://reviews.apache.org/r/37814/diff/
> 
> 
> Testing
> ---
> 
> Built the updated website using the docker site builder 
> (https://github.com/mesosphere/mesos-website-container).
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 37945: change const pass-by-value to const reference in stout

[Guangya Liu]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

change const pass-by-value to const reference in stout


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/format.hpp 
1cf6dd18aa163688d6c8f3a6d33eacad3918015d 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp 
68fc1fd179ee51fc5de0452c0f2ea3d354e0567f 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp 
01e59de466496dec9367ad6f48538327f53a7e18 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

[Jian Qiu]

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



src/master/master.cpp (line 4904)


If the allocator calls inverse callback followed by an offer callback, will 
the two messages be handled in two separate threads of the framework? I think 
it will be framework's responsibility to ensure data synchronization between 
the two threads?


- Jian Qiu


On 八月 26, 2015, 2:13 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37180/
> ---
> 
> (Updated 八月 26, 2015, 2:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
> 
> Diff: https://reviews.apache.org/r/37180/diff/
> 
> 
> Testing
> ---
> 
> The tests break as expected.
> With the scheduler API change there are CHECKs that fail.
> Once we update the API these will be resolved.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 37945: change const pass-by-value to const reference in stout

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37945]

All tests passed.

- Mesos ReviewBot


On Aug. 31, 2015, 10:46 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37945/
> ---
> 
> (Updated Aug. 31, 2015, 10:46 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-1805
> https://issues.apache.org/jira/browse/MESOS-1805
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> change const pass-by-value to const reference in stout
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/format.hpp 
> 1cf6dd18aa163688d6c8f3a6d33eacad3918015d 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp 
> 68fc1fd179ee51fc5de0452c0f2ea3d354e0567f 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp 
> 01e59de466496dec9367ad6f48538327f53a7e18 
> 
> Diff: https://reviews.apache.org/r/37945/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 37958: Add an option to only perform batch allocations.

[Guangya Liu]

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



docs/configuration.md (line 288)


is it only a flag or also have value? If only flag, can we use the 
following:
--[no-]allocate_on_events



docs/configuration.md (line 292)


The following format may be better

  If 'true' xxx (default: true)
  
  If 'false' xxx 



- Guangya Liu


On 八月 31, 2015, 4:49 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37958/
> ---
> 
> (Updated 八月 31, 2015, 4:49 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3157
> https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Normally, the mesos-master performs allocation cycles in response
> to protocol messages and cluster events. This ties the cost of
> allocation to the amount of churn in the environment, which can led
> to unpredictable performance. Add an option to disable this, meaning
> that allocations are only every performed by the batch interval
> task. This makes resource allocation a fixed cost, independent of
> the amount of environmental churn.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 2b23d48c8841e43e6e2776dfe6bfa7c022a941a7 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp 
> aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 
> 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 230c1dcedfbbb410b89defd90159d964c001a615 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9748ca0b3fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/mesos.hpp b2160f50db3e0b0d04b0625e7589004016c0f746 
> 
> Diff: https://reviews.apache.org/r/37958/diff/
> 
> 
> Testing
> ---
> 
> "make check" on CentOS 6 w/ devtoolset-3. Running in production for multiple 
> weeks.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 37177: Maintenance Primitives: Added inverse offers.

[Qian Zhang]

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



src/master/allocator/mesos/hierarchical.hpp (line 391)


Indent:
Line 392: Need to delete a space.
Line 393: Need to add 7 more spaces to align with line 392.



src/master/allocator/mesos/hierarchical.hpp (line 1118)


s/send inverse offer/deallocate resources/



src/master/allocator/mesos/hierarchical.hpp (line 1136)


So here we only use framework sorters, then why we check  role sorter in 
the beginning of this method?



src/master/allocator/mesos/hierarchical.hpp (line 1154)


Remove "we"?



src/master/master.cpp (line 623)


In future, we may need to add more callback functions, so here can we 
change to use a general structure to keep the callback functions that we want 
to pass into allocator? So in future, if we want to add more callback  
functions, just need to change that structure rather than the signature of 
allocator->initialize()


- Qian Zhang


On Aug. 26, 2015, 2:12 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37177/
> ---
> 
> (Updated Aug. 26, 2015, 2:12 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp 
> aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 
> 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9748ca0b3fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/master_allocator_tests.cpp 
> 89331965553505f6b7eebf39ad27d943df816a24 
>   src/tests/mesos.hpp 637636ac69dde02da6b7200d7c666cac89b051cb 
>   src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b 
>   src/tests/resource_offers_tests.cpp 
> 882a9ff4d09aace486182828bf43b643b0d0c519 
>   src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc 
> 
> Diff: https://reviews.apache.org/r/37177/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 37871: SSL tests refactoring

[Jojy Varghese]

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

(Updated Aug. 31, 2015, 2:32 p.m.)


Review request for mesos, Joris Van Remoortere and Timothy Chen.


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


Repository: mesos


Description
---

Refactored libprocess SSL tests to be available for reuse by other projects.


Diffs
-

  3rdparty/libprocess/Makefile.am 7ef515848508c2e84ab7607595f635f67e24b19b 
  3rdparty/libprocess/include/Makefile.am 
3b6108dd37d23bd5104162e9e8f4a3aa0518cdcd 
  3rdparty/libprocess/include/process/ssl/gtest.hpp PRE-CREATION 
  3rdparty/libprocess/src/openssl_util.hpp  
  3rdparty/libprocess/src/openssl_util.cpp 
42b2860beccff929563ed05905da226b781918b7 
  3rdparty/libprocess/src/tests/ssl_tests.cpp 
7a316bc10575325ffe732fcc87d72d15a4fc5eaf 

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


Testing
---

make check


Thanks,

Jojy Varghese