Re: Review Request 38239: Renamed appc.{hpp|cpp} to slave/containerizer/provisioners/appc/provisioner.{hpp|cpp}.

[Jiang Yan Xu]

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

(Updated Sept. 9, 2015, 11:57 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Renamed header guard.


Repository: mesos


Description
---

Renamed appc.{hpp|cpp} to 
slave/containerizer/provisioners/appc/provisioner.{hpp|cpp}.


Diffs (updated)
-

  src/Makefile.am 0a8ef6d8551cf177cb565b2a443c05e8eea5ab1c 
  src/slave/containerizer/provisioner.cpp 
95894c0ed97388cf2830058c13804b6baf5daad4 
  src/slave/containerizer/provisioners/appc.hpp 
68e82e31869f05a0118154e7e601f42125dc2cf6 
  src/slave/containerizer/provisioners/appc.cpp 
d54b6882346df7121ec63803810fab979ac0848a 

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


Testing
---

make check.


Thanks,

Jiang Yan Xu

Re: Review Request 37830: Added a test for converting JSON arrays to Resources.

[Michael Park]

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

Ship it!



src/tests/resources_tests.cpp (lines 153 - 158)


I needed the same thing recently for [reservation 
tests](https://github.com/apache/mesos/blob/master/src/tests/reservation_endpoints_tests.cpp#L70)!

I've filed [MESOS-3405](https://issues.apache.org/jira/browse/MESOS-3405) 
for this :)



src/tests/resources_tests.cpp (line 160)


This seems like an unrelated test.


- Michael Park


On Sept. 9, 2015, 2:46 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37830/
> ---
> 
> (Updated Sept. 9, 2015, 2:46 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Michael Park.
> 
> 
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/resources_tests.cpp 2ae93a9c8235e5e4643539d409df51c39c6d7e56 
> 
> Diff: https://reviews.apache.org/r/37830/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 38137: Added Docker provisioner and local store

[Jiang Yan Xu]

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


Partial review, mostly high level.


src/messages/docker_provisioner.hpp (line 22)


We typically have a comment above:

// ONLY USEFUL AFTER RUNNING PROTOC.



src/slave/containerizer/provisioners/docker.hpp (line 56)


s/Image Name/image name/

s/composes/consists/ or s/composes/is composed/?



src/slave/containerizer/provisioners/docker.hpp (line 59)


re Jie's comment, Appc uses a spec.hpp|cpp to break the circular dependency 
and consolidates definitions and validations of concepts specific to the spec. 
For docker it's something similar: 
https://github.com/docker/docker/blob/master/image/spec/v1.md

Just an example.



src/slave/containerizer/provisioners/docker.hpp (line 66)


Should registry be part of the name?

I know it has significance in discovery but it doesn't appear to be part of 
the name.

I would suggest we model our data structure close to the spec so when 
there's confusion it's easy to refer to something that's definitive.

I have another comment below about the "registry" in "name".



src/slave/containerizer/provisioners/docker.hpp (line 71)


I see that this is for `ImageName::create()` but if we get rid of 
`registry` and use the other constructor while parsing the value we don't need 
this and we can make the fileds const.



src/slave/containerizer/provisioners/docker.hpp (line 122)


Is the order of the list significant? We should document it.



src/slave/containerizer/provisioners/docker.hpp (line 132)


s/mesos::internal::slave:://

It's a parent namespace.



src/slave/containerizer/provisioners/docker.hpp (line 135)


Ditto about `mesos::internal::slave::`.



src/slave/containerizer/provisioners/docker.cpp (line 100)


re my comment above. Something close to a spec.hpp|cpp to put it would be 
nice.



src/slave/containerizer/provisioners/docker.cpp (lines 104 - 107)


I guess you are modeling this after the `docker pull` syntax here: 
`[REGISTRY_HOST[:REGISTRY_PORT]/]NAME[:TAG]`

I found that some docker help pages have conflicting use of repository vs. 
name and we should probably refer to the spec.

I think the following questions need to be answered:

1) If we use image name as the indentifier, it defines "what the image is". 
The registry specifies "where the image comes from". If an identical image is 
hosted on two registries, should they have the same name?

2) ID. `docker pull` supports @sha256 syntax and `repositories` file (which 
our docker metadata file is modeled after) maps repo:tag to an image ID. 
Shouldn't the ID be part of the name? If ubuntu:latest changes when use 
releases come out, I don't think repo:tag uniquely identifies the image.



src/slave/containerizer/provisioners/docker.cpp (line 351)


The necessity of this conversion makes me think that we should probably 
take push the structured definition of the `name` to `Image::Docker`.

It's probably the frameowrk's reponsibility to ensure the name is "valid" 
(i.e. can be parsed into a structured name)

The Store::get() could just be:

```
process::Future get(const Image::Docker& image);
```

If we push towards a common provisioner, we need a common store API and 
this could be a first step. 

But most importantly, I think we should push up the conversion from a 
unstrutured string and use a structured name throughout the provisioner 
components (metadata manger, store, etc.) The structured name can always be 
stringified when we need to encode it into the diretory names and such.

Thoughts?



src/slave/containerizer/provisioners/docker/local_store.cpp (lines 158 - 160)


Can we actually use fetcher here?



src/slave/containerizer/provisioners/docker/local_store.cpp (line 276)


So the schema of the "repositories" file seems to be straightfoward here. 
Worth defining a proto schema so it's clearer what we are parsing and where to 
look for which fields here?

Re: Review Request 38137: Added Docker provisioner and local store

[Timothy Chen]

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

(Updated Sept. 10, 2015, 7:31 a.m.)


Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang Yan 
Xu.


Repository: mesos


Description
---

Joining all the commits around provisioner and local store into one review so 
it's easier to review, as patches
are changing code on top of each other.

All the commits are going to committed together.


Diffs (updated)
-

  src/Makefile.am cea470e 
  src/messages/docker_provisioner.hpp PRE-CREATION 
  src/messages/docker_provisioner.proto PRE-CREATION 
  src/slave/containerizer/provisioner.hpp 9e0e0b8 
  src/slave/containerizer/provisioner.cpp 95894c0 
  src/slave/containerizer/provisioners/docker.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/local_store.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/local_store.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/metadata_manager.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/metadata_manager.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/paths.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/paths.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
  src/slave/flags.hpp b8335aa 
  src/slave/flags.cpp 7539441 
  src/tests/containerizer/docker_provisioner_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 38154: Switched to type traits for checking whether a type is a protobuf message.

[Michael Park]

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

Ship it!


Ship It!

- Michael Park


On Sept. 9, 2015, 2:45 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38154/
> ---
> 
> (Updated Sept. 9, 2015, 2:45 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/38154/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 38251: FrameworkInfo should only be updated if the re-registration is valid

[Guangya Liu]

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

(Updated Sept. 10, 2015, 10:12 a.m.)


Review request for mesos, Joris Van Remoortere and Vinod Kone.


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


Repository: mesos


Description
---

FrameworkInfo should only be updated if the re-registration is valid


Diffs
-

  src/master/master.cpp 4b60e637b19e749e27c4a8eb9b0a95c7e9305c5f 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 38075: Added an Appc provisioner recovery test.

[Jiang Yan Xu]

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

(Updated Sept. 9, 2015, 11:59 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


Repository: mesos


Description
---

Added an Appc provisioner recovery test.


Diffs (updated)
-

  src/tests/containerizer/appc_provisioner_tests.cpp 
f30e1a1dee9a68be01133117339d91771f15988c 

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


Testing
---

make check.

This also utilizes the copy backend.


Thanks,

Jiang Yan Xu

Re: Review Request 38240: Renamed appc_backend flag to appc_provisioner_backend.

[Jiang Yan Xu]

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

(Updated Sept. 10, 2015, midnight)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


Repository: mesos


Description
---

Renamed appc_backend flag to appc_provisioner_backend.


Diffs (updated)
-

  src/slave/containerizer/provisioners/appc/provisioner.cpp 
d54b6882346df7121ec63803810fab979ac0848a 
  src/slave/flags.hpp b8335aa585376d27b71897f8cbaefaa12f4b3a5c 
  src/slave/flags.cpp 7539441c685828027db07173e62a4e5fc1e8b54d 
  src/tests/containerizer/appc_provisioner_tests.cpp 
f30e1a1dee9a68be01133117339d91771f15988c 

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


Testing
---

make check.


Thanks,

Jiang Yan Xu

Re: Review Request 37821: Join threads in libprocess when shutting down.

[Michael Park]

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


I've made a few nit comments below but I have some higher-level questions.

  (1) In this patch, when the destructor of `ProcessManager` is invoked we 
immediately start to ignore messages. It's not obvious to me that this is 
necessary or desired. Could we just enqueue the `TerminateEvent` by calling 
`process::terminate(process, false)`, let it race against other messages that 
may be coming in, and ignore anything that arrives after `TerminateEvent`?
  (2) What are the implications of shutting down the event loop via 
`EventLoop::stop()`? This would be useful to know to learn about the required 
order between process termination, event loop shutdown, and thread pool joins.


3rdparty/libprocess/src/libev.cpp (line 30)


Is there a reason why this was moved up? Seems like `async_shutdown` 
could've been added below this, at its original location?



3rdparty/libprocess/src/process.cpp (line 382)


The `const` here has no effect.



3rdparty/libprocess/src/process.cpp (lines 442 - 443)


This fits in 1 line.



3rdparty/libprocess/src/process.cpp (lines 2119 - 2121)


Could you explain why we want to start ignoring messages here?

Below, we do `process::terminate(process, false);` which purposely does not 
inject the `TerminateEvent` in order to allow the actor to process its queue.

This is great, but wouldn't we be dropping messages that were sent between  
`draining_queue.store(true);` and when the `TerminateEvent` actually gets 
enqueued at the back? Is the decision that this doesn't matter?



3rdparty/libprocess/src/process.cpp (line 2171)


`s/int/long/`?


- Michael Park


On Sept. 8, 2015, 5:54 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37821/
> ---
> 
> (Updated Sept. 8, 2015, 5:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3158
> https://issues.apache.org/jira/browse/MESOS-3158
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Join threads in libprocess when shutting down.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/event_loop.hpp 
> 36a4cd2b1ff59f6922173ad17115bf80cc3c8f30 
>   3rdparty/libprocess/src/libev.cpp 97a2694f9b10bc61841443b21f4f96055493e840 
>   3rdparty/libprocess/src/libevent.cpp 
> ee7906470069b0391dde7cd685b1d4eb3a158c03 
>   3rdparty/libprocess/src/process.cpp 
> 0e5394acff16376809918d583d7aee582cc6da54 
> 
> Diff: https://reviews.apache.org/r/37821/diff/
> 
> 
> Testing
> ---
> 
> After configuring with both "../configure" and "../configure 
> --enable-libevent --enable-ssl":
> 
> make check
> 
> 
> Also, to check for race conditions related to the initialization/shutdown of 
> libprocess, try something like:
> 
> for n in {1..1000}; do echo $n; 3rdparty/libprocess/tests 
> --gtest_filter=ProcessTest.Spawn; done
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 38252: Remove namespaces method in LinuxFilesystemIsolatorProcess.

[haosdent huang]

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

(Updated Sept. 10, 2015, 9:53 a.m.)


Review request for mesos, Ben Mahler and Jie Yu.


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


Repository: mesos


Description
---

Remove namespaces method in LinuxFilesystemIsolatorProcess.


Diffs (updated)
-

  src/slave/containerizer/isolators/filesystem/linux.hpp 
ab60f0ccffa06cb6f1913a6fd4fce8a2ccf6cd94 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
0970b3d48b13d5e9d2e0160df5cf14a3dcd0acc9 

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


Testing
---

sudo GLOG_v=1 ./bin/mesos-tests.sh 
--gtest_filter="PersistentVolumeTest.AccessPersistentVolume" --verbose


Thanks,

haosdent huang

Re: Review Request 38239: Renamed appc.{hpp|cpp} to slave/containerizer/provisioners/appc/provisioner.{hpp|cpp}.

[Jiang Yan Xu]

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

(Updated Sept. 10, 2015, 12:04 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


Repository: mesos


Description
---

Renamed appc.{hpp|cpp} to 
slave/containerizer/provisioners/appc/provisioner.{hpp|cpp}.


Diffs (updated)
-

  src/Makefile.am cea470e14ed93a46fead9da8015df676e818e4bc 
  src/slave/containerizer/provisioner.cpp 
95894c0ed97388cf2830058c13804b6baf5daad4 
  src/slave/containerizer/provisioners/appc.hpp 
68e82e31869f05a0118154e7e601f42125dc2cf6 
  src/slave/containerizer/provisioners/appc.cpp 
d54b6882346df7121ec63803810fab979ac0848a 

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


Testing
---

make check.


Thanks,

Jiang Yan Xu

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37871, 37427, 37773]

All tests passed.

- Mesos ReviewBot


On Sept. 10, 2015, 4:53 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37773/
> ---
> 
> (Updated Sept. 10, 2015, 4:53 a.m.)
> 
> 
> Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added implementation for docker registry's Get Manifest and Get Blob APIs.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am cea470e14ed93a46fead9da8015df676e818e4bc 
>   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 
> ff29d562c7f1bd5f0579e97cdbd60c2b2f36329e 
> 
> Diff: https://reviews.apache.org/r/37773/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

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

[Michael Park]

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



3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (line 37)


We don't need `inline` here as this is a `cpp` file.



3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (lines 44 - 50)


How about we use `std::equal` here?

```cpp
  if (left.id() != right.id() ||
  left.numbers().size() != right.numbers().size()) {
return false;
  }

  return std::equal(
  left.numbers().begin(), left.numbers().end(), 
right.numbers().begin());
```



3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (line 166)


`s/message1/message/`?



3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (line 172)


`s/object1/object/`?



3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (lines 191 - 192)


The above test already performs the roundtrip of Protobuf -> JSON -> 
Protobuf. Is it beneficial to add an additional conversion to JSON here? What 
does it further test?



3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto (line 23)


`s/JSON-> conversion/JSON conversion/`


- Michael Park


On Sept. 9, 2015, 2:46 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37827/
> ---
> 
> (Updated Sept. 9, 2015, 2:46 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Joseph Wu, 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 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/#review98370
---

Ship it!


Looks good!


3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 602)


Would `_value` be better than `v` here? Maybe `elem`...?



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (lines 622 - 623)


> overload will not work because ...

As I mentioned in my comment in reply to Joseph's question, the overloaded 
functions approach with the use of `enable_if` would work but not as clean.


- Michael Park


On Sept. 9, 2015, 2:46 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37826/
> ---
> 
> (Updated Sept. 9, 2015, 2:46 p.m.)
> 
> 
> Review request for mesos, Joseph Wu 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 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/#review98363
---


Patch looks great!

Reviews applied: [37968]

All tests passed.

- Mesos ReviewBot


On Sept. 10, 2015, 2:38 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37968/
> ---
> 
> (Updated Sept. 10, 2015, 2:38 a.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 37505: Fix broken health check in docker executor.

[Timothy Chen]

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



src/tests/health_check_tests.cpp (line 258)


We should do ASSERT here, refer to your Docker exec patch and I commented 
the usual way we've done this.



src/tests/health_check_tests.cpp (line 501)


s/executor/execuotr/g.



src/tests/health_check_tests.cpp (line 507)


ASSERT style here too.


- Timothy Chen


On Sept. 10, 2015, 2:20 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37505/
> ---
> 
> (Updated Sept. 10, 2015, 2:20 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3136
> https://issues.apache.org/jira/browse/MESOS-3136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix broken health check in docker executor.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp aaf8884a20901ad3a440d545d4b6316b87fc3403 
>   src/docker/docker.cpp 553e831029454d6d423842915b8bbfcaf19fa7f9 
>   src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e 
>   src/slave/containerizer/docker.cpp 289d4ec0fba9071dfe0cbf5391b5391d4566dd9c 
>   src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea 
> 
> Diff: https://reviews.apache.org/r/37505/diff/
> 
> 
> Testing
> ---
> 
> # Add two new test cases, HealthCheckTest.ROOT_DOCKER_DockerHealthyTask and 
> HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange
> # Docker health check command is run through "docker exec"
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthyTask" --verbose
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange" 
> --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

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

[Michael Park]

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

Ship it!


I've committed this for you with the following minor changes!


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


`s/for detail/for further details/`



3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp (line 57)


Removed this newline.


- Michael Park


On Sept. 10, 2015, 2:38 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37968/
> ---
> 
> (Updated Sept. 10, 2015, 2:38 a.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 38075: Added an Appc provisioner recovery test.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38239, 38240, 38241, 38075]

All tests passed.

- Mesos ReviewBot


On Sept. 10, 2015, 6:59 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38075/
> ---
> 
> (Updated Sept. 10, 2015, 6:59 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an Appc provisioner recovery test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/appc_provisioner_tests.cpp 
> f30e1a1dee9a68be01133117339d91771f15988c 
> 
> Diff: https://reviews.apache.org/r/38075/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> This also utilizes the copy backend.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 38241: Fixed an issue that caused provisioned filesystems specified in --default_container_info to be not recovered.

[Jiang Yan Xu]

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

(Updated Sept. 9, 2015, 11:59 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


Repository: mesos


Description
---

Fixed an issue that caused provisioned filesystems specified in 
--default_container_info to be not recovered.


Diffs (updated)
-

  src/slave/containerizer/provisioners/appc/provisioner.cpp 
d54b6882346df7121ec63803810fab979ac0848a 

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


Testing
---

Tested in https://reviews.apache.org/r/38075/


Thanks,

Jiang Yan Xu

Re: Review Request 37589: Remove unnecessary usage information.

[Michael Park]

> On Aug. 30, 2015, 10:31 p.m., Michael Park wrote:
> > This is great! Many of these were slightly incorrect. They're all fixed now 
> > :)
> > e.g. `/statistics.json` -> `/monitor/statistics.json`

I've committed this patch for you with a trivial rebase.


- Michael


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


On Aug. 18, 2015, 6:36 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37589/
> ---
> 
> (Updated Aug. 18, 2015, 6:36 p.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-3239
> https://issues.apache.org/jira/browse/MESOS-3239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Remove unnecessary usage information.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp a94a5eec0d4cf095f84255a66117947a308fdb2f 
>   src/master/http.cpp d016b3c055a06e7aa0649a36fd4b095c2938ba56 
>   src/master/registrar.cpp ca8efb4000a688ea42c84123db2a0d3fbe1227c3 
>   src/slave/http.cpp b0fe5f520dfca156548ba8c436d42fc432223f3d 
>   src/slave/monitor.cpp 82aa659b02f98ad8dc4c4f556be7a332dfba6816 
> 
> Diff: https://reviews.apache.org/r/37589/diff/
> 
> 
> Testing
> ---
> 
> manual test
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 38252: Remove namespaces method in LinuxFilesystemIsolatorProcess.

[haosdent huang]

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

Review request for mesos, Ben Mahler and Jie Yu.


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


Repository: mesos


Description
---

Remove namespaces method in LinuxFilesystemIsolatorProcess.


Diffs
-

  src/slave/containerizer/isolators/filesystem/linux.hpp 
ab60f0ccffa06cb6f1913a6fd4fce8a2ccf6cd94 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
0970b3d48b13d5e9d2e0160df5cf14a3dcd0acc9 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 37505: Fix broken health check in docker executor.

[haosdent huang]

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

(Updated Sept. 10, 2015, 7:16 a.m.)


Review request for mesos and Timothy Chen.


Changes
---

Update according to @tnachen's reviews


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


Repository: mesos


Description
---

Fix broken health check in docker executor.


Diffs (updated)
-

  src/docker/docker.hpp aaf8884a20901ad3a440d545d4b6316b87fc3403 
  src/docker/docker.cpp 553e831029454d6d423842915b8bbfcaf19fa7f9 
  src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e 
  src/slave/containerizer/docker.cpp 289d4ec0fba9071dfe0cbf5391b5391d4566dd9c 
  src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea 

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


Testing
---

# Add two new test cases, HealthCheckTest.ROOT_DOCKER_DockerHealthyTask and 
HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange
# Docker health check command is run through "docker exec"
sudo ./bin/mesos-tests.sh 
--gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthyTask" --verbose
sudo ./bin/mesos-tests.sh 
--gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange" --verbose


Thanks,

haosdent huang

Re: Review Request 38253: [MESOS-2875] Add containerId to ResourceUsage to enable QoS controller to target a container

[Guangya Liu]

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



src/slave/slave.cpp (line 4373)


I think that you are still killing executor, what about update as following:

Kill executor [id] with containerid [id] for framework [id] as QoS 
correction?


- Guangya Liu


On Sept. 10, 2015, 10:32 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38253/
> ---
> 
> (Updated Sept. 10, 2015, 10:32 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-2875
> https://issues.apache.org/jira/browse/MESOS-2875
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We should ensure that we are addressing the container which the QoS 
> controller intended to kill. Without this check, we may run into a scenario 
> where the executor has terminated and one with the same id has started in the 
> interim i.e. running in a different container than the one the QoS controller 
> targeted.
> 
> This most likely requires us to add containerId to the ResourceUsage message 
> and encode the containerID in the QoS Correction message.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   include/mesos/slave/oversubscription.proto 
> fa69a95689c8c75765f0800692655e8dde7dde33 
>   src/slave/slave.cpp 5e5522e1254a5ed6084de36782753f9aad2894c4 
>   src/tests/oversubscription_tests.cpp 
> 0c5edafc139d9bfb6806d007a0af85e80893bb1a 
> 
> Diff: https://reviews.apache.org/r/38253/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38251: FrameworkInfo should only be updated if the re-registration is valid

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38251]

All tests passed.

- Mesos ReviewBot


On Sept. 10, 2015, 10:12 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38251/
> ---
> 
> (Updated Sept. 10, 2015, 10:12 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Bugs: MESOS-3184
> https://issues.apache.org/jira/browse/MESOS-3184
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> FrameworkInfo should only be updated if the re-registration is valid
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 4b60e637b19e749e27c4a8eb9b0a95c7e9305c5f 
> 
> Diff: https://reviews.apache.org/r/38251/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38251: FrameworkInfo should only be updated if the re-registration is valid

[Yong Qiao Wang]

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



src/master/master.cpp 


If the frameworkInfo.id()and framework->pid do not changed when reregister, 
then it does not need to call slave to update framework pid. This "return" can 
gurad this, so this return does not be removed.


- Yong Qiao Wang


On Sept. 10, 2015, 10:12 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38251/
> ---
> 
> (Updated Sept. 10, 2015, 10:12 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Bugs: MESOS-3184
> https://issues.apache.org/jira/browse/MESOS-3184
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> FrameworkInfo should only be updated if the re-registration is valid
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 4b60e637b19e749e27c4a8eb9b0a95c7e9305c5f 
> 
> Diff: https://reviews.apache.org/r/38251/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 37505: Fix broken health check in docker executor.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37505]

All tests passed.

- Mesos ReviewBot


On Sept. 10, 2015, 7:16 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37505/
> ---
> 
> (Updated Sept. 10, 2015, 7:16 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3136
> https://issues.apache.org/jira/browse/MESOS-3136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix broken health check in docker executor.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp aaf8884a20901ad3a440d545d4b6316b87fc3403 
>   src/docker/docker.cpp 553e831029454d6d423842915b8bbfcaf19fa7f9 
>   src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e 
>   src/slave/containerizer/docker.cpp 289d4ec0fba9071dfe0cbf5391b5391d4566dd9c 
>   src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea 
> 
> Diff: https://reviews.apache.org/r/37505/diff/
> 
> 
> Testing
> ---
> 
> # Add two new test cases, HealthCheckTest.ROOT_DOCKER_DockerHealthyTask and 
> HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange
> # Docker health check command is run through "docker exec"
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthyTask" --verbose
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange" 
> --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38252: Remove namespaces method in LinuxFilesystemIsolatorProcess.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38252]

All tests passed.

- Mesos ReviewBot


On Sept. 10, 2015, 9:53 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38252/
> ---
> 
> (Updated Sept. 10, 2015, 9:53 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-3349
> https://issues.apache.org/jira/browse/MESOS-3349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Remove namespaces method in LinuxFilesystemIsolatorProcess.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> ab60f0ccffa06cb6f1913a6fd4fce8a2ccf6cd94 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 0970b3d48b13d5e9d2e0160df5cf14a3dcd0acc9 
> 
> Diff: https://reviews.apache.org/r/38252/diff/
> 
> 
> Testing
> ---
> 
> sudo GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="PersistentVolumeTest.AccessPersistentVolume" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38240: Renamed appc_backend flag to appc_provisioner_backend.

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On Sept. 10, 2015, 7 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38240/
> ---
> 
> (Updated Sept. 10, 2015, 7 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed appc_backend flag to appc_provisioner_backend.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioners/appc/provisioner.cpp 
> d54b6882346df7121ec63803810fab979ac0848a 
>   src/slave/flags.hpp b8335aa585376d27b71897f8cbaefaa12f4b3a5c 
>   src/slave/flags.cpp 7539441c685828027db07173e62a4e5fc1e8b54d 
>   src/tests/containerizer/appc_provisioner_tests.cpp 
> f30e1a1dee9a68be01133117339d91771f15988c 
> 
> Diff: https://reviews.apache.org/r/38240/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 38137: Added Docker provisioner and local store

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38137]

All tests passed.

- Mesos ReviewBot


On Sept. 10, 2015, 7:31 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38137/
> ---
> 
> (Updated Sept. 10, 2015, 7:31 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang 
> Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Joining all the commits around provisioner and local store into one review so 
> it's easier to review, as patches
> are changing code on top of each other.
> 
> All the commits are going to committed together.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am cea470e 
>   src/messages/docker_provisioner.hpp PRE-CREATION 
>   src/messages/docker_provisioner.proto PRE-CREATION 
>   src/slave/containerizer/provisioner.hpp 9e0e0b8 
>   src/slave/containerizer/provisioner.cpp 95894c0 
>   src/slave/containerizer/provisioners/docker.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker.cpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/local_store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/local_store.cpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/metadata_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/metadata_manager.cpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/paths.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/paths.cpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
>   src/slave/flags.hpp b8335aa 
>   src/slave/flags.cpp 7539441 
>   src/tests/containerizer/docker_provisioner_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38137/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Review Request 38259: [MESOS-3340] Command-line flags should take precedence over OS Env variables

[Klaus Ma]

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

Review request for mesos and Bernd Mathiske.


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


Repository: mesos


Description
---

Currently, it appears that re-defining a flag on the command-line that was 
already defined via a OS Env var (MESOS_*) causes the Master to fail with a not 
very helpful message.

For example, if one has MESOS_QUORUM defined, this happens:

$ ./mesos-master --zk=zk://192.168.1.4/mesos --quorum=1 
--hostname=192.168.1.4 --ip=192.168.1.4
Duplicate flag 'quorum' on command line

which is not very helpful.

Current solution is to throw error if any duplication; over-write may make user 
confused about the result.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
9da213f802aec6a7768ce6f5aea7b437d980356a 
  3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
ebf8cd656625b7fd414cacaa87f156c95df29438 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Review Request 38256: Update framework-rate-limiting.md

[Guangya Liu]

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

Review request for mesos and Jiang Yan Xu.


Repository: mesos


Description
---

The configuration parameter for rate limits should be --rate_limts
but not --rate-limits.


Diffs
-

  docs/framework-rate-limiting.md cb0bbb707a48657e70ed20f1daf469e28d8d531f 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 38253: [MESOS-2875] Add containerId to ResourceUsage to enable QoS controller to target a container

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38253]

All tests passed.

- Mesos ReviewBot


On Sept. 10, 2015, 10:32 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38253/
> ---
> 
> (Updated Sept. 10, 2015, 10:32 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-2875
> https://issues.apache.org/jira/browse/MESOS-2875
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We should ensure that we are addressing the container which the QoS 
> controller intended to kill. Without this check, we may run into a scenario 
> where the executor has terminated and one with the same id has started in the 
> interim i.e. running in a different container than the one the QoS controller 
> targeted.
> 
> This most likely requires us to add containerId to the ResourceUsage message 
> and encode the containerID in the QoS Correction message.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   include/mesos/slave/oversubscription.proto 
> fa69a95689c8c75765f0800692655e8dde7dde33 
>   src/slave/slave.cpp 5e5522e1254a5ed6084de36782753f9aad2894c4 
>   src/tests/oversubscription_tests.cpp 
> 0c5edafc139d9bfb6806d007a0af85e80893bb1a 
> 
> Diff: https://reviews.apache.org/r/38253/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38256: Update framework-rate-limiting.md

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38256]

All tests passed.

- Mesos ReviewBot


On Sept. 10, 2015, 3:01 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38256/
> ---
> 
> (Updated Sept. 10, 2015, 3:01 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The configuration parameter for rate limits should be --rate_limts
> but not --rate-limits.
> 
> 
> Diffs
> -
> 
>   docs/framework-rate-limiting.md cb0bbb707a48657e70ed20f1daf469e28d8d531f 
> 
> Diff: https://reviews.apache.org/r/38256/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38056: Fix webui task informations not update bug.

[haosdent huang]

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

(Updated Sept. 10, 2015, 4:51 p.m.)


Review request for mesos, Adam B and Michael Park.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Fix webui task informations not update bug.


Diffs (updated)
-

  src/webui/master/static/js/controllers.js 
fbf86965efdb16f11787c24fb50d20af0837c62d 

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


Testing (updated)
---

Manual test, test scenarios:

* Normal scenarios updated after the new task is finished
* After mesos-master down, could updated after reconnected

![task_infomartions.png](https://issues.apache.org/jira/secure/attachment/12753783/task_infomartions.png)


Thanks,

haosdent huang

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

[Alexander Rukletsov]

> On Sept. 10, 2015, 9:54 a.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp, line 37
> > 
> >
> > We don't need `inline` here as this is a `cpp` file.

Right, good catch!


> On Sept. 10, 2015, 9:54 a.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp, lines 44-50
> > 
> >
> > How about we use `std::equal` here?
> > 
> > ```cpp
> >   if (left.id() != right.id() ||
> >   left.numbers().size() != right.numbers().size()) {
> > return false;
> >   }
> > 
> >   return std::equal(
> >   left.numbers().begin(), left.numbers().end(), 
> > right.numbers().begin());
> > ```

Looks like it will work fine with `RepeatedPtrField`. Good suggestion!


- Alexander


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


On Sept. 9, 2015, 2:46 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37827/
> ---
> 
> (Updated Sept. 9, 2015, 2:46 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Joseph Wu, 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 38056: Fix webui task informations not update bug.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38056]

All tests passed.

- Mesos ReviewBot


On Sept. 10, 2015, 4:51 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38056/
> ---
> 
> (Updated Sept. 10, 2015, 4:51 p.m.)
> 
> 
> Review request for mesos, Adam B and Michael Park.
> 
> 
> Bugs: MESOS-3282
> https://issues.apache.org/jira/browse/MESOS-3282
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix webui task informations not update bug.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/js/controllers.js 
> fbf86965efdb16f11787c24fb50d20af0837c62d 
> 
> Diff: https://reviews.apache.org/r/38056/diff/
> 
> 
> Testing
> ---
> 
> Manual test, test scenarios:
> 
> * Normal scenarios updated after the new task is finished
> * After mesos-master down, could updated after reconnected
> 
> ![task_infomartions.png](https://issues.apache.org/jira/secure/attachment/12753783/task_infomartions.png)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38259: [MESOS-3340] Command-line flags should take precedence over OS Env variables

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38259]

All tests passed.

- Mesos ReviewBot


On Sept. 10, 2015, 3:47 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38259/
> ---
> 
> (Updated Sept. 10, 2015, 3:47 p.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Bugs: MESOS-3340
> https://issues.apache.org/jira/browse/MESOS-3340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, it appears that re-defining a flag on the command-line that was 
> already defined via a OS Env var (MESOS_*) causes the Master to fail with a 
> not very helpful message.
> 
> For example, if one has MESOS_QUORUM defined, this happens:
> 
> $ ./mesos-master --zk=zk://192.168.1.4/mesos --quorum=1 
> --hostname=192.168.1.4 --ip=192.168.1.4
> Duplicate flag 'quorum' on command line
> 
> which is not very helpful.
> 
> Current solution is to throw error if any duplication; over-write may make 
> user confused about the result.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
> 9da213f802aec6a7768ce6f5aea7b437d980356a 
>   3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
> ebf8cd656625b7fd414cacaa87f156c95df29438 
> 
> Diff: https://reviews.apache.org/r/38259/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38259: [MESOS-3340] Command-line flags should take precedence over OS Env variables

[haosdent huang]

> On Sept. 10, 2015, 4:40 p.m., haosdent huang wrote:
> > LGTM. But I not sure if this match @marco expect. Could you add he as 
> > reviewers? So currently your approach is show error message when 
> > `duplicates` is true. Use command line parameters when command line 
> > variables conflict with environment variables. I also suggest add comment 
> > and test cases about the last condition.

```
Use command line parameters when command line variables conflict with 
environment variables.
```
->
```
When duplicates is false, use command line parameters while ignore environment 
variables.
```


- haosdent


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


On Sept. 10, 2015, 3:47 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38259/
> ---
> 
> (Updated Sept. 10, 2015, 3:47 p.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Bugs: MESOS-3340
> https://issues.apache.org/jira/browse/MESOS-3340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, it appears that re-defining a flag on the command-line that was 
> already defined via a OS Env var (MESOS_*) causes the Master to fail with a 
> not very helpful message.
> 
> For example, if one has MESOS_QUORUM defined, this happens:
> 
> $ ./mesos-master --zk=zk://192.168.1.4/mesos --quorum=1 
> --hostname=192.168.1.4 --ip=192.168.1.4
> Duplicate flag 'quorum' on command line
> 
> which is not very helpful.
> 
> Current solution is to throw error if any duplication; over-write may make 
> user confused about the result.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
> 9da213f802aec6a7768ce6f5aea7b437d980356a 
>   3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
> ebf8cd656625b7fd414cacaa87f156c95df29438 
> 
> Diff: https://reviews.apache.org/r/38259/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38259: [MESOS-3340] Command-line flags should take precedence over OS Env variables

[haosdent huang]

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


LGTM. But I not sure if this match @marco expect. Could you add he as 
reviewers? So currently your approach is show error message when `duplicates` 
is true. Use command line parameters when command line variables conflict with 
environment variables. I also suggest add comment and test cases about the last 
condition.


3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp (line 569)


Is it possible to change to foreach here?



3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp (line 600)


The style is not correct here. Please change to
```
return Error(
"Duplicate flag '" + name "' in command line with environment");
```



3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp (line 675)


Ditto


- haosdent huang


On Sept. 10, 2015, 3:47 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38259/
> ---
> 
> (Updated Sept. 10, 2015, 3:47 p.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Bugs: MESOS-3340
> https://issues.apache.org/jira/browse/MESOS-3340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, it appears that re-defining a flag on the command-line that was 
> already defined via a OS Env var (MESOS_*) causes the Master to fail with a 
> not very helpful message.
> 
> For example, if one has MESOS_QUORUM defined, this happens:
> 
> $ ./mesos-master --zk=zk://192.168.1.4/mesos --quorum=1 
> --hostname=192.168.1.4 --ip=192.168.1.4
> Duplicate flag 'quorum' on command line
> 
> which is not very helpful.
> 
> Current solution is to throw error if any duplication; over-write may make 
> user confused about the result.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
> 9da213f802aec6a7768ce6f5aea7b437d980356a 
>   3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
> ebf8cd656625b7fd414cacaa87f156c95df29438 
> 
> Diff: https://reviews.apache.org/r/38259/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

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

[Alexander Rukletsov]

> On Sept. 10, 2015, 8:44 a.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 606
> > 
> >
> > Would `_value` be better than `v` here? Maybe `elem`...?

I don't like `_*` variables and prefer to avoid them where possible (not 
dictated by our style guide). `elem` sounds good.


- Alexander


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


On Sept. 9, 2015, 2:46 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37826/
> ---
> 
> (Updated Sept. 9, 2015, 2:46 p.m.)
> 
> 
> Review request for mesos, Joseph Wu 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 36125: Removing '.json' extension in master endpoints url

[Isabel Jimenez]

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

(Updated Sept. 10, 2015, 6:04 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, Marco Massenzio, 
and Vinod Kone.


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


Repository: mesos-incubating


Description
---

Removing json extension for HTTP endpoints in master


Diffs (updated)
-

  src/cli/mesos-cat 73dc63e 
  src/cli/mesos-ps ee14d51 
  src/cli/mesos-scp 77b8557 
  src/cli/mesos-tail 256a804 
  src/master/constants.hpp c3fe140 
  src/master/http.cpp a052e55 
  src/master/master.hpp 1dfc947 
  src/master/master.cpp 4b60e63 
  src/tests/fault_tolerance_tests.cpp 89cb18b 
  src/tests/master_tests.cpp 8a6b98b 
  src/webui/master/static/js/controllers.js 3445028 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 36126: Removing '.json' extension in slave endpoints url

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [36126]

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

Error:
 2015-09-10 18:04:29 URL:https://reviews.apache.org/r/36126/diff/raw/ 
[14060/14060] -> "36126.patch" [1]
error: patch failed: src/slave/http.cpp:202
error: src/slave/http.cpp: patch does not apply
error: patch failed: src/slave/monitor.cpp:48
error: src/slave/monitor.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Sept. 10, 2015, 5:43 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36126/
> ---
> 
> (Updated Sept. 10, 2015, 5:43 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-2983
> https://issues.apache.org/jira/browse/MESOS-2983
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Removing json extension for HTTP endpoints in slave
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 315dc53 
>   docs/network-monitoring.md acd70c5 
>   src/cli/mesos-cat 73dc63e 
>   src/cli/mesos-ps ee14d51 
>   src/cli/mesos-tail 256a804 
>   src/master/flags.cpp 230c1dc 
>   src/slave/flags.cpp 7539441 
>   src/slave/http.cpp b0fe5f5 
>   src/slave/monitor.cpp 82aa659 
>   src/slave/slave.hpp 09172f7 
>   src/slave/slave.cpp 5e5522e 
>   src/tests/fault_tolerance_tests.cpp 89cb18b 
>   src/tests/monitor_tests.cpp 53fb53e 
>   src/tests/slave_tests.cpp 5c1a3d3 
>   src/webui/master/static/js/controllers.js 3445028 
>   src/webui/master/static/js/services.js 2cd9d7d 
> 
> Diff: https://reviews.apache.org/r/36126/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

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

[Alexander Rukletsov]

> On Sept. 10, 2015, 8:44 a.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, lines 626-627
> > 
> >
> > > overload will not work because ...
> > 
> > As I mentioned in my comment in reply to Joseph's question, the 
> > overloaded functions approach with the use of `enable_if` would work but 
> > not as clean.

I didn't mean to `enable_if` here, but rather a common function overload. I'll 
update the comment for clarity.


- Alexander


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


On Sept. 9, 2015, 2:46 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37826/
> ---
> 
> (Updated Sept. 9, 2015, 2:46 p.m.)
> 
> 
> Review request for mesos, Joseph Wu 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 37827: Added a test for converting JSON arrays to repeated protobufs.

[Alexander Rukletsov]

> On Sept. 10, 2015, 9:54 a.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp, lines 191-192
> > 
> >
> > The above test already performs the roundtrip of Protobuf -> JSON -> 
> > Protobuf. Is it beneficial to add an additional conversion to JSON here? 
> > What does it further test?

The rationale here is that equality check for protobufs is not well defined (we 
do it overselves), hence an additional check via converted `JSON` objects 
looked to me like a reasonable addition.


- Alexander


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


On Sept. 10, 2015, 6:36 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37827/
> ---
> 
> (Updated Sept. 10, 2015, 6:36 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Joseph Wu, 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 37876: stout: Replace GCC intrinsics with std::atomic.

[Neil Conway]

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

(Updated Sept. 10, 2015, 6:53 p.m.)


Review request for mesos, Joris Van Remoortere and switched to 'mcypark'.


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


Repository: mesos


Description
---

MESOS-3326.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 
d43433aeab5a1a68ff76eb75416672fae456c70d 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 36125: Removing '.json' extension in master endpoints url

[Isabel Jimenez]

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

(Updated Sept. 10, 2015, 5:33 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, Marco Massenzio, 
and Vinod Kone.


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


Repository: mesos-incubating


Description
---

Removing json extension for HTTP endpoints in master


Diffs (updated)
-

  src/cli/mesos-cat 73dc63e 
  src/cli/mesos-ps ee14d51 
  src/cli/mesos-scp 77b8557 
  src/cli/mesos-tail 256a804 
  src/master/constants.hpp c3fe140 
  src/master/http.cpp a052e55 
  src/master/master.hpp 1dfc947 
  src/master/master.cpp 4b60e63 
  src/tests/fault_tolerance_tests.cpp 89cb18b 
  src/tests/master_tests.cpp 8a6b98b 
  src/webui/master/static/js/controllers.js 3445028 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 38239: Renamed appc.{hpp|cpp} to slave/containerizer/provisioners/appc/provisioner.{hpp|cpp}.

[Jiang Yan Xu]

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

(Updated Sept. 10, 2015, 10:42 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Comments.


Repository: mesos


Description
---

Renamed appc.{hpp|cpp} to 
slave/containerizer/provisioners/appc/provisioner.{hpp|cpp}.


Diffs (updated)
-

  src/Makefile.am cea470e14ed93a46fead9da8015df676e818e4bc 
  src/slave/containerizer/provisioner.cpp 
95894c0ed97388cf2830058c13804b6baf5daad4 
  src/slave/containerizer/provisioners/appc.hpp 
68e82e31869f05a0118154e7e601f42125dc2cf6 
  src/slave/containerizer/provisioners/appc.cpp 
d54b6882346df7121ec63803810fab979ac0848a 

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


Testing
---

make check.


Thanks,

Jiang Yan Xu

Re: Review Request 36126: Removing '.json' extension in slave endpoints url

[Isabel Jimenez]

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

(Updated Sept. 10, 2015, 5:43 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, Marco Massenzio, 
and Vinod Kone.


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


Repository: mesos-incubating


Description
---

Removing json extension for HTTP endpoints in slave


Diffs (updated)
-

  docs/configuration.md 315dc53 
  docs/network-monitoring.md acd70c5 
  src/cli/mesos-cat 73dc63e 
  src/cli/mesos-ps ee14d51 
  src/cli/mesos-tail 256a804 
  src/master/flags.cpp 230c1dc 
  src/slave/flags.cpp 7539441 
  src/slave/http.cpp b0fe5f5 
  src/slave/monitor.cpp 82aa659 
  src/slave/slave.hpp 09172f7 
  src/slave/slave.cpp 5e5522e 
  src/tests/fault_tolerance_tests.cpp 89cb18b 
  src/tests/monitor_tests.cpp 53fb53e 
  src/tests/slave_tests.cpp 5c1a3d3 
  src/webui/master/static/js/controllers.js 3445028 
  src/webui/master/static/js/services.js 2cd9d7d 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 38075: Added an Appc provisioner recovery test.

[Jie Yu]

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

Ship it!



src/tests/containerizer/appc_provisioner_tests.cpp (line 353)


Can you use a random containerId? I.e.,

UUID::random().toString();


- Jie Yu


On Sept. 10, 2015, 6:59 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38075/
> ---
> 
> (Updated Sept. 10, 2015, 6:59 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an Appc provisioner recovery test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/appc_provisioner_tests.cpp 
> f30e1a1dee9a68be01133117339d91771f15988c 
> 
> Diff: https://reviews.apache.org/r/38075/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> This also utilizes the copy backend.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 38154: Switched to type traits for checking whether a type is a protobuf message.

[Alexander Rukletsov]

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

(Updated Sept. 10, 2015, 6:45 p.m.)


Review request for mesos and Michael Park.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
57d5fdf45273c620655b44b5f5572290cffa4bf6 

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


Testing
---

make check (Mac OS 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 37830: Added a test for converting JSON arrays to Resources.

[Alexander Rukletsov]

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

(Updated Sept. 10, 2015, 6:46 p.m.)


Review request for mesos, Joseph Wu and Michael Park.


Changes
---

MPark's comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/resources_tests.cpp 2ae93a9c8235e5e4643539d409df51c39c6d7e56 

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


Testing
---

make check (Mac OS 10.10.4)


Thanks,

Alexander Rukletsov

Review Request 38265: mesos: Update style guide for usage of std::atomic.

[Neil Conway]

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

Review request for mesos.


Repository: mesos


Description
---

mesos: Update style guide for usage of std::atomic.


Diffs
-

  docs/mesos-c++-style-guide.md 5e4d13e0456e577f05631b77349e4b1e6f0945c7 

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 38158: Refactored Value::Ranges coalesce().

[Alexander Rukletsov]

> On Sept. 7, 2015, 9:24 p.m., Alexander Rukletsov wrote:
> > src/common/values.cpp, lines 442-444
> > 
> >
> > Let's let compiler do it's job: how about passing `left` by value and 
> > not creating a copy manually?
> 
> Joerg Schad wrote:
> Wanted to keep the (internal) interface consistent with the other 
> operators. Do you have a strong preference here?

I leave it to you and your shepherd. You may drop the issue.


- Alexander


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


On Sept. 10, 2015, 6:15 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38158/
> ---
> 
> (Updated Sept. 10, 2015, 6:15 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Till Toenshoff.
> 
> 
> Bugs: MESOS-3051
> https://issues.apache.org/jira/browse/MESOS-3051
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The goal of this refactoring was to reuse the Ranges objects as much as 
> possible, as prior there was substantial time spend in allocation/destruction 
> (MESOS-3051).
> 
> 
> Diffs
> -
> 
>   include/mesos/values.hpp e300580431f7fd6cff06e9617c0227b51c4cb8e2 
>   src/common/values.cpp 750264e603b4cde2011f07f4434a4b34fe3e512f 
>   src/tests/resources_tests.cpp 2ae93a9c8235e5e4643539d409df51c39c6d7e56 
>   src/tests/values_tests.cpp fc35d97894a2de6207b9337180e2160e6f2cb1f5 
> 
> Diff: https://reviews.apache.org/r/38158/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 38158: Refactored Value::Ranges coalesce().

[Alexander Rukletsov]

> On Sept. 9, 2015, 3:46 p.m., Alexander Rukletsov wrote:
> > src/common/values.cpp, lines 250-271
> > 
> >
> > I think we can conflate this two cases, how about this:
> > ```
> > if (range->end() + 1 >= current.begin()) {
> >   range->set_end(std::max(range.end(), current.end()));
> >   deleteSet.insert(y);
> >   ++i;
> > } else {
> >   break;
> > }
> > ```
> 
> Joerg Schad wrote:
> Yes we could do this. I personally find the 2 distinct cases (especially 
> with the range comments) more readable.

I think we can also add range comments here to make it more readable : ). 
Usually less branches and less code means faster execution. However, you (and 
your shepherd) decide, means you can drop the issue.


- Alexander


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


On Sept. 10, 2015, 6:15 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38158/
> ---
> 
> (Updated Sept. 10, 2015, 6:15 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Till Toenshoff.
> 
> 
> Bugs: MESOS-3051
> https://issues.apache.org/jira/browse/MESOS-3051
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The goal of this refactoring was to reuse the Ranges objects as much as 
> possible, as prior there was substantial time spend in allocation/destruction 
> (MESOS-3051).
> 
> 
> Diffs
> -
> 
>   include/mesos/values.hpp e300580431f7fd6cff06e9617c0227b51c4cb8e2 
>   src/common/values.cpp 750264e603b4cde2011f07f4434a4b34fe3e512f 
>   src/tests/resources_tests.cpp 2ae93a9c8235e5e4643539d409df51c39c6d7e56 
>   src/tests/values_tests.cpp fc35d97894a2de6207b9337180e2160e6f2cb1f5 
> 
> Diff: https://reviews.apache.org/r/38158/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36125: Removing '.json' extension in master endpoints url

[Isabel Jimenez]

> On Sept. 9, 2015, 10:46 p.m., Adam B wrote:
> > Looks great! I'm not sure if we needed to change all the internal 
> > references to remove .json, since we're keeping that endpoint for 
> > compatibility, but it's good that we're testing the new endpoint.
> > 
> > Did you test to verify that the `.json` endpoints still work?

Yes I tried the .json endpoints


- Isabel


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


On Sept. 10, 2015, 5:33 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36125/
> ---
> 
> (Updated Sept. 10, 2015, 5:33 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-2719
> https://issues.apache.org/jira/browse/MESOS-2719
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Removing json extension for HTTP endpoints in master
> 
> 
> Diffs
> -
> 
>   src/cli/mesos-cat 73dc63e 
>   src/cli/mesos-ps ee14d51 
>   src/cli/mesos-scp 77b8557 
>   src/cli/mesos-tail 256a804 
>   src/master/constants.hpp c3fe140 
>   src/master/http.cpp a052e55 
>   src/master/master.hpp 1dfc947 
>   src/master/master.cpp 4b60e63 
>   src/tests/fault_tolerance_tests.cpp 89cb18b 
>   src/tests/master_tests.cpp 8a6b98b 
>   src/webui/master/static/js/controllers.js 3445028 
> 
> Diff: https://reviews.apache.org/r/36125/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 36127: Removing '.json' extension in files endpoints url

[Isabel Jimenez]

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

(Updated Sept. 10, 2015, 5:58 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, Marco Massenzio, 
and Vinod Kone.


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


Repository: mesos-incubating


Description
---

Removing json extension for HTTP endpoints in files


Diffs (updated)
-

  docs/configuration.md 315dc53 
  src/cli/mesos-cat 73dc63e 
  src/cli/mesos-tail 256a804 
  src/files/files.cpp b2134aa 
  src/master/flags.cpp 230c1dc 
  src/slave/flags.cpp 7539441 
  src/tests/files_tests.cpp 53771cd 
  src/tests/gc_tests.cpp ec27ac7 
  src/webui/master/static/browse.html 0904c87 
  src/webui/master/static/js/controllers.js 3445028 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 36127: Removing '.json' extension in files endpoints url

[Isabel Jimenez]

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

(Updated Sept. 10, 2015, 5:59 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, Marco Massenzio, 
and Vinod Kone.


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


Repository: mesos-incubating


Description
---

Removing json extension for HTTP endpoints in files


Diffs
-

  docs/configuration.md 315dc53 
  src/cli/mesos-cat 73dc63e 
  src/cli/mesos-tail 256a804 
  src/files/files.cpp b2134aa 
  src/master/flags.cpp 230c1dc 
  src/slave/flags.cpp 7539441 
  src/tests/files_tests.cpp 53771cd 
  src/tests/gc_tests.cpp ec27ac7 
  src/webui/master/static/browse.html 0904c87 
  src/webui/master/static/js/controllers.js 3445028 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 38239: Renamed appc.{hpp|cpp} to slave/containerizer/provisioners/appc/provisioner.{hpp|cpp}.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Sept. 10, 2015, 5:42 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38239/
> ---
> 
> (Updated Sept. 10, 2015, 5:42 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed appc.{hpp|cpp} to 
> slave/containerizer/provisioners/appc/provisioner.{hpp|cpp}.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am cea470e14ed93a46fead9da8015df676e818e4bc 
>   src/slave/containerizer/provisioner.cpp 
> 95894c0ed97388cf2830058c13804b6baf5daad4 
>   src/slave/containerizer/provisioners/appc.hpp 
> 68e82e31869f05a0118154e7e601f42125dc2cf6 
>   src/slave/containerizer/provisioners/appc.cpp 
> d54b6882346df7121ec63803810fab979ac0848a 
> 
> Diff: https://reviews.apache.org/r/38239/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 38031: [3/5] Integer Precision for JSON <-> Protobuf conversions.

[Joseph Wu]

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

(Updated Sept. 10, 2015, 11:25 a.m.)


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


Changes
---

Address Joris's comments.  Made the JSON::Number's storage fields private.


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


Repository: mesos


Description
---

* Changes JSON::Number to keep track of whether it is floating, signed 
integral, or unsigned integral.
* Changes how JSON::Number is stringified, to ensure that stringified doubles 
are parsed as JSON doubles.
* Added one test for integer precision between String <-> JSON <-> Protobuf 
conversions.
* Added one test for JSON::Number comparisons.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 
f28138c4682c41e94ab6c7641a78d66b2f9daa5f 
  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
57d5fdf45273c620655b44b5f5572290cffa4bf6 
  3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
850650c269e9be24c0f1ae81b8aa8725f8a0c151 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
c56d6a3098293eb3659b3066f10e875927ec3ac3 

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


Testing
---

No testing done until the last patch in the chain.

However, this patch does breaks some libprocess and mesos tests (because 
JSON::Number is stored differently), which are fixed in the following two 
reviews.


Thanks,

Joseph Wu

Re: Review Request 38241: Fixed an issue that caused provisioned filesystems specified in --default_container_info to be not recovered.

[Jie Yu]

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

Ship it!



src/slave/containerizer/provisioners/appc/provisioner.cpp (line 272)


quote is not needed for containerId as it's generated by us (shouldn't 
contain spaces).


- Jie Yu


On Sept. 10, 2015, 6:59 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38241/
> ---
> 
> (Updated Sept. 10, 2015, 6:59 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed an issue that caused provisioned filesystems specified in 
> --default_container_info to be not recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioners/appc/provisioner.cpp 
> d54b6882346df7121ec63803810fab979ac0848a 
> 
> Diff: https://reviews.apache.org/r/38241/diff/
> 
> 
> Testing
> ---
> 
> Tested in https://reviews.apache.org/r/38075/
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

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

[Alexander Rukletsov]

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

(Updated Sept. 10, 2015, 6:36 p.m.)


Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.


Changes
---

MPark's comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  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 36127: Removing '.json' extension in files endpoints url

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [36126]

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

Error:
 2015-09-10 19:01:59 URL:https://reviews.apache.org/r/36126/diff/raw/ 
[14060/14060] -> "36126.patch" [1]
error: patch failed: src/slave/http.cpp:202
error: src/slave/http.cpp: patch does not apply
error: patch failed: src/slave/monitor.cpp:48
error: src/slave/monitor.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Sept. 10, 2015, 5:59 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36127/
> ---
> 
> (Updated Sept. 10, 2015, 5:59 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-2984
> https://issues.apache.org/jira/browse/MESOS-2984
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Removing json extension for HTTP endpoints in files
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 315dc53 
>   src/cli/mesos-cat 73dc63e 
>   src/cli/mesos-tail 256a804 
>   src/files/files.cpp b2134aa 
>   src/master/flags.cpp 230c1dc 
>   src/slave/flags.cpp 7539441 
>   src/tests/files_tests.cpp 53771cd 
>   src/tests/gc_tests.cpp ec27ac7 
>   src/webui/master/static/browse.html 0904c87 
>   src/webui/master/static/js/controllers.js 3445028 
> 
> Diff: https://reviews.apache.org/r/36127/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 36126: Removing '.json' extension in slave endpoints url

[Isabel Jimenez]

> On Sept. 9, 2015, 10:51 p.m., Adam B wrote:
> > Minor comments, but it looks shippable to me after those are resolved.
> > 
> > Did you test to verify that the old endpoints still work as expected?

Yes I tested the old endpoints :)


- Isabel


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


On Sept. 10, 2015, 5:43 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36126/
> ---
> 
> (Updated Sept. 10, 2015, 5:43 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-2983
> https://issues.apache.org/jira/browse/MESOS-2983
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Removing json extension for HTTP endpoints in slave
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 315dc53 
>   docs/network-monitoring.md acd70c5 
>   src/cli/mesos-cat 73dc63e 
>   src/cli/mesos-ps ee14d51 
>   src/cli/mesos-tail 256a804 
>   src/master/flags.cpp 230c1dc 
>   src/slave/flags.cpp 7539441 
>   src/slave/http.cpp b0fe5f5 
>   src/slave/monitor.cpp 82aa659 
>   src/slave/slave.hpp 09172f7 
>   src/slave/slave.cpp 5e5522e 
>   src/tests/fault_tolerance_tests.cpp 89cb18b 
>   src/tests/monitor_tests.cpp 53fb53e 
>   src/tests/slave_tests.cpp 5c1a3d3 
>   src/webui/master/static/js/controllers.js 3445028 
>   src/webui/master/static/js/services.js 2cd9d7d 
> 
> Diff: https://reviews.apache.org/r/36126/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 38233: os: add swap information to memory().

[Chi Zhang]

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

(Updated Sept. 10, 2015, 5:57 p.m.)


Review request for mesos and Jie Yu.


Bugs: mesos-2918
https://issues.apache.org/jira/browse/mesos-2918


Repository: mesos


Description
---

os: add swap information to memory().


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/os.hpp 
82ccd766bd22393f48be4554c7da46338dd92d1f 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
5d2f39d9a9d963225bf463572cb8fe99dd9aa6f5 

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


Testing
---


Thanks,

Chi Zhang

Re: Review Request 37878: mesos: Replace volatile with std::atomic.

[Neil Conway]

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

(Updated Sept. 10, 2015, 6:55 p.m.)


Review request for mesos, Joris Van Remoortere and switched to 'mcypark'.


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


Repository: mesos


Description
---

MESOS-3326.


Diffs (updated)
-

  src/exec/exec.cpp 31e0c2f17a9092d18285828111d27628fb07bc02 
  src/sched/sched.cpp 012af0508eeceeccd168b29f36fa258d20b28c21 

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 37877: libprocess: Replace GCC instrinsics and volatile with std::atomic.

[Neil Conway]

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

(Updated Sept. 10, 2015, 6:54 p.m.)


Review request for mesos, Joris Van Remoortere and switched to 'mcypark'.


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


Repository: mesos


Description
---

MESOS-3326.


Diffs (updated)
-

  3rdparty/libprocess/include/process/latch.hpp 
a1a2227a9edcc31fd82c6410262aa4565fd66cb2 
  3rdparty/libprocess/include/process/metrics/counter.hpp 
e51a8beb80b15dd64aa2e481036ae8ba37125640 
  3rdparty/libprocess/include/process/process.hpp 
cc8317f1bc25bfa1be207a3e212b8cfc75248404 
  3rdparty/libprocess/src/clock.cpp 09c60e5a5d9bd9fc5511e57f3209fad7dbf834d6 
  3rdparty/libprocess/src/latch.cpp f7d94d92e85c58878d98e13757b6fc37837ca977 
  3rdparty/libprocess/src/process.cpp 0e5394acff16376809918d583d7aee582cc6da54 
  3rdparty/libprocess/src/process_reference.hpp 
f8df4a6dcf01bb7af750c1ed9e85c64cea2042c5 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 38143: Adding executor HTTP API protobuf to V1

[Isabel Jimenez]

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

(Updated Sept. 10, 2015, 11:38 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
ought to be changed as the executor HTTP API design evolves.


Diffs (updated)
-

  include/mesos/v1/executor/executor.hpp PRE-CREATION 
  include/mesos/v1/executor/executor.proto PRE-CREATION 
  src/Makefile.am 8963cea 

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


Testing
---

make && make check


Thanks,

Isabel Jimenez

Re: Review Request 37032: Extend permissions.hpp to work on both Windows and POSIX.

[Artem Harutyunyan]

> On Sept. 8, 2015, 3:51 p.m., Artem Harutyunyan wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/permissions.hpp, line 57
> > 
> >
> > A comment describing why do we need this function would be great.
> 
> Alex Clemmer wrote:
> Just to make sure we're on the same page, this function is moving here -- 
> I didn't write it. It looks like it's used only in the fetcher tests and the 
> `credentials.hpp`, so my guess is it's not really _needed_. But either way, 
> I'd like to suggest that I'm not the best person to provide this 
> justification.

OK. Thanks for clarifying.


- Artem


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


On Sept. 10, 2015, 12:36 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37032/
> ---
> 
> (Updated Sept. 10, 2015, 12:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extend permissions.hpp to work on both Windows and POSIX.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 8853f92fcfcff81d0a3197bade02110685fa0325 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/permissions.hpp 
> 196c3f5fac7c3526924f2bea03c06d1fbce25c61 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/permissions.hpp 
> 98f0b3c8e55190df87d6a581667e21b31ac044bc 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/permissions.hpp 
> daed4b4e9c76d6e7c043a1fa3a46ab1f3db95f48 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 7ab75ece44ab4b0cc42f992daf1101d0faf80b1f 
> 
> Diff: https://reviews.apache.org/r/37032/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 37032: Extend permissions.hpp to work on both Windows and POSIX.

[Joris Van Remoortere]

> On Sept. 8, 2015, 10:51 p.m., Artem Harutyunyan wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp, lines 152-159
> > 
> >
> > Do we really need to leave this up to the user to decide? What's the 
> > benefit of one versus the other? Can't we just stick to one option and just 
> > implement that one? 
> > 
> > If not, is there a build time configuration flag that can be used to 
> > change this?
> 
> Alex Clemmer wrote:
> I provided this (1) because I could see use cases for both permission 
> semantics (both for us and for downstream clients _e.g._ DCOS clients who are 
> concerned about security or something), (2) because making this configurable 
> at runtime would likely mean augmenting or refactoring the existing API for 
> things like `chown`, and (3) because a "final" solution is probably out of 
> scope for the MVP.
> 
> Happy to change this, but for now I think we should probably ship it and 
> re-evaluate later.
> 
> Also, to answer your question about specifying this at compile time, yes, 
> you can just pass a flag like `-DSTRICT_OTHER_PERMISSIONS` and it should 
> "just work." This isn't wired up in CMake but that would be trivial to do.
> 
> Joseph Wu wrote:
> I suggest for now to get rid of the flags and opt for strict-ness (no 
> fallback) by default.  Being strict means places in the codebase that try to 
> use "group/other" permissions will be more likely to error out (i.e. access 
> denied).  We can then catch those cases individually and deal with them.
> 
> We should also create a JIRA to track this, in-case anything breaks as a 
> result (on Windows).

Dropping the compiler flag for now and only using the strict versions for now.


- Joris


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


On Sept. 10, 2015, 7:36 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37032/
> ---
> 
> (Updated Sept. 10, 2015, 7:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extend permissions.hpp to work on both Windows and POSIX.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 8853f92fcfcff81d0a3197bade02110685fa0325 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/permissions.hpp 
> 196c3f5fac7c3526924f2bea03c06d1fbce25c61 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/permissions.hpp 
> 98f0b3c8e55190df87d6a581667e21b31ac044bc 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/permissions.hpp 
> daed4b4e9c76d6e7c043a1fa3a46ab1f3db95f48 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 7ab75ece44ab4b0cc42f992daf1101d0faf80b1f 
> 
> Diff: https://reviews.apache.org/r/37032/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38077: [5/5] Integer Precision for JSON <-> Protobuf conversions.

[Joseph Wu]

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


The above reviewbot failure is related to the recently committed `/reserve` 
`/unreserve` endpoints.  (See https://reviews.apache.org/r/35984/)

@mcypark will have a fix shortly 
(https://issues.apache.org/jira/browse/MESOS-3411).

- Joseph Wu


On Sept. 10, 2015, 11:26 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38077/
> ---
> 
> (Updated Sept. 10, 2015, 11:26 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Timothy Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-3345
> https://issues.apache.org/jira/browse/MESOS-3345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> * Add TODO's for refactoring some JSON parsing in the docker code (See 
> MESOS-3409).  Update how the JSON::Number is used.
> * Tweak some tests to match changes to JSON::Number.
> * Address a TODO on one test, which used a workaround for double-precision 
> comparison.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 553e831029454d6d423842915b8bbfcaf19fa7f9 
>   src/slave/containerizer/provisioners/docker/token_manager.cpp 
> aec915f25f6aada0a1d8f22d63a093bdbac97b25 
>   src/tests/fault_tolerance_tests.cpp 
> 89cb18be96cd60fb77fbcc4acd08cebdcf1ba075 
>   src/tests/master_tests.cpp 8a6b98b9f59ead20f537eb60b5084feed069a5b1 
>   src/tests/monitor_tests.cpp 53fb53eeea7d444ee043eb4569a52fead7ad0da8 
>   src/tests/rate_limiting_tests.cpp f3aeddee00c7bb7905092aa8a760603468063126 
>   src/tests/slave_tests.cpp 5c1a3d36a5f67629aef275eeae12956c524e8102 
> 
> Diff: https://reviews.apache.org/r/38077/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38282: Fixed flaky ReservationEndpointsTest.AvailableResources test.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Sept. 11, 2015, 12:50 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38282/
> ---
> 
> (Updated Sept. 11, 2015, 12:50 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Bugs: MESOS-3411
> https://issues.apache.org/jira/browse/MESOS-3411
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_endpoints_tests.cpp 
> 795f1cf4c88abf3fdb96013b386236bf1a4a312a 
> 
> Diff: https://reviews.apache.org/r/38282/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 38143: Adding executor HTTP API protobuf to V1

[Isabel Jimenez]

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

(Updated Sept. 10, 2015, 10:32 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
ought to be changed as the executor HTTP API design evolves.


Diffs (updated)
-

  include/mesos/v1/executor/executor.hpp PRE-CREATION 
  include/mesos/v1/executor/executor.proto PRE-CREATION 
  src/Makefile.am 8963cea 

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


Testing
---

make && make check


Thanks,

Isabel Jimenez

Re: Review Request 38284: Layers from Appc store should be the rootfs dir, not the top level dir.

[Chi Zhang]

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

Ship it!


Ship It!

- Chi Zhang


On Sept. 11, 2015, 1:12 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38284/
> ---
> 
> (Updated Sept. 11, 2015, 1:12 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Layers from Appc store should be the rootfs dir, not the top level dir.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioners/appc/store.cpp 
> 04ab2add7613b28959ad2031c0cc60f188e97ba2 
>   src/tests/containerizer/appc_provisioner_tests.cpp 
> b1be2d81cbd08d2736dddbc22c163635758c2f59 
> 
> Diff: https://reviews.apache.org/r/38284/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 38284: Layers from Appc store should be the rootfs dir, not the top level dir.

[Jie Yu]

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

Review request for mesos, Chi Zhang, Vinod Kone, and Jiang Yan Xu.


Repository: mesos


Description
---

Layers from Appc store should be the rootfs dir, not the top level dir.


Diffs
-

  src/slave/containerizer/provisioners/appc/store.cpp 
04ab2add7613b28959ad2031c0cc60f188e97ba2 
  src/tests/containerizer/appc_provisioner_tests.cpp 
b1be2d81cbd08d2736dddbc22c163635758c2f59 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 37370: Fix CMake build compile error; don't compile GMock with cxx11 flag.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Sept. 10, 2015, 8:53 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37370/
> ---
> 
> (Updated Sept. 10, 2015, 8:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, haosdent huang, Artem 
> Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Right now if you try to compile GMock a non-Windows platform using the
> CMake build system, it will error out. The problem seems to be the CXX11
> flag.
> 
> This commit will solve this problem by removing that flag.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 997cc0d0e316e316136d4746e50e9e292a82b36b 
> 
> Diff: https://reviews.apache.org/r/37370/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 36127: Removing '.json' extension in files endpoints url

[Isabel Jimenez]

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

(Updated Sept. 10, 2015, 9:46 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, Marco Massenzio, 
and Vinod Kone.


Changes
---

rebase


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


Repository: mesos-incubating


Description
---

Removing json extension for HTTP endpoints in files


Diffs (updated)
-

  docs/configuration.md 315dc53 
  src/cli/mesos-cat 73dc63e 
  src/cli/mesos-tail 256a804 
  src/files/files.cpp a8807b6 
  src/master/flags.cpp 230c1dc 
  src/slave/flags.cpp b676bac 
  src/tests/files_tests.cpp 53771cd 
  src/tests/gc_tests.cpp ec27ac7 
  src/webui/master/static/browse.html 0904c87 
  src/webui/master/static/js/controllers.js fbf8696 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 37877: libprocess: Replace GCC instrinsics and volatile with std::atomic.

[Joris Van Remoortere]

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



3rdparty/libprocess/src/latch.cpp (line 41)


renaming these to `expected`



3rdparty/libprocess/src/process.cpp (line 951)


substituting the triple backticks to single backticks. sorry for the 
confusion.


- Joris Van Remoortere


On Sept. 10, 2015, 6:54 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37877/
> ---
> 
> (Updated Sept. 10, 2015, 6:54 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and switched to 'mcypark'.
> 
> 
> Bugs: MESOS-3326
> https://issues.apache.org/jira/browse/MESOS-3326
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3326.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/latch.hpp 
> a1a2227a9edcc31fd82c6410262aa4565fd66cb2 
>   3rdparty/libprocess/include/process/metrics/counter.hpp 
> e51a8beb80b15dd64aa2e481036ae8ba37125640 
>   3rdparty/libprocess/include/process/process.hpp 
> cc8317f1bc25bfa1be207a3e212b8cfc75248404 
>   3rdparty/libprocess/src/clock.cpp 09c60e5a5d9bd9fc5511e57f3209fad7dbf834d6 
>   3rdparty/libprocess/src/latch.cpp f7d94d92e85c58878d98e13757b6fc37837ca977 
>   3rdparty/libprocess/src/process.cpp 
> 0e5394acff16376809918d583d7aee582cc6da54 
>   3rdparty/libprocess/src/process_reference.hpp 
> f8df4a6dcf01bb7af750c1ed9e85c64cea2042c5 
> 
> Diff: https://reviews.apache.org/r/37877/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38282: Fixed flaky ReservationEndpointsTest.AvailableResources test.

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 九月 11, 2015, 12:50 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38282/
> ---
> 
> (Updated 九月 11, 2015, 12:50 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Bugs: MESOS-3411
> https://issues.apache.org/jira/browse/MESOS-3411
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_endpoints_tests.cpp 
> 795f1cf4c88abf3fdb96013b386236bf1a4a312a 
> 
> Diff: https://reviews.apache.org/r/38282/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 38143: Adding executor HTTP API protobuf to V1

[Isabel Jimenez]

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

(Updated Sept. 10, 2015, 11:33 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
ought to be changed as the executor HTTP API design evolves.


Diffs
-

  include/mesos/v1/executor/executor.hpp PRE-CREATION 
  include/mesos/v1/executor/executor.proto PRE-CREATION 
  src/Makefile.am 8963cea 

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


Testing
---

make && make check


Thanks,

Isabel Jimenez

Review Request 38278: Updating unversioned Executor protobuf

[Isabel Jimenez]

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

Review request for mesos, Adam B, Anand Mazumdar, and Vinod Kone.


Repository: mesos


Description
---

Adding last changes of the executor HTTP API design to the unversioned protobuf


Diffs
-

  include/mesos/executor/executor.proto 52c84b3 

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


Testing
---

make


Thanks,

Isabel Jimenez

Review Request 38282: Fixed flaky ReservationEndpointsTest.AvailableResources test.

[Michael Park]

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

Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/reservation_endpoints_tests.cpp 
795f1cf4c88abf3fdb96013b386236bf1a4a312a 

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


Testing
---


Thanks,

Michael Park

Re: Review Request 37032: Extend permissions.hpp to work on both Windows and POSIX.

[Artem Harutyunyan]

> On Sept. 8, 2015, 3:51 p.m., Artem Harutyunyan wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp, lines 152-159
> > 
> >
> > Do we really need to leave this up to the user to decide? What's the 
> > benefit of one versus the other? Can't we just stick to one option and just 
> > implement that one? 
> > 
> > If not, is there a build time configuration flag that can be used to 
> > change this?
> 
> Alex Clemmer wrote:
> I provided this (1) because I could see use cases for both permission 
> semantics (both for us and for downstream clients _e.g._ DCOS clients who are 
> concerned about security or something), (2) because making this configurable 
> at runtime would likely mean augmenting or refactoring the existing API for 
> things like `chown`, and (3) because a "final" solution is probably out of 
> scope for the MVP.
> 
> Happy to change this, but for now I think we should probably ship it and 
> re-evaluate later.
> 
> Also, to answer your question about specifying this at compile time, yes, 
> you can just pass a flag like `-DSTRICT_OTHER_PERMISSIONS` and it should 
> "just work." This isn't wired up in CMake but that would be trivial to do.
> 
> Joseph Wu wrote:
> I suggest for now to get rid of the flags and opt for strict-ness (no 
> fallback) by default.  Being strict means places in the codebase that try to 
> use "group/other" permissions will be more likely to error out (i.e. access 
> denied).  We can then catch those cases individually and deal with them.
> 
> We should also create a JIRA to track this, in-case anything breaks as a 
> result (on Windows).
> 
> Joris Van Remoortere wrote:
> Dropping the compiler flag for now and only using the strict versions for 
> now.

I agree. Alex, could you please create a JIRA to track?


- Artem


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


On Sept. 10, 2015, 12:36 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37032/
> ---
> 
> (Updated Sept. 10, 2015, 12:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extend permissions.hpp to work on both Windows and POSIX.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 8853f92fcfcff81d0a3197bade02110685fa0325 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/permissions.hpp 
> 196c3f5fac7c3526924f2bea03c06d1fbce25c61 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/permissions.hpp 
> 98f0b3c8e55190df87d6a581667e21b31ac044bc 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/permissions.hpp 
> daed4b4e9c76d6e7c043a1fa3a46ab1f3db95f48 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 7ab75ece44ab4b0cc42f992daf1101d0faf80b1f 
> 
> Diff: https://reviews.apache.org/r/37032/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38284: Layers from Appc store should be the rootfs dir, not the top level dir.

[Jiang Yan Xu]

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

Ship it!


Ship It!

- Jiang Yan Xu


On Sept. 10, 2015, 6:12 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38284/
> ---
> 
> (Updated Sept. 10, 2015, 6:12 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Layers from Appc store should be the rootfs dir, not the top level dir.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioners/appc/store.cpp 
> 04ab2add7613b28959ad2031c0cc60f188e97ba2 
>   src/tests/containerizer/appc_provisioner_tests.cpp 
> b1be2d81cbd08d2736dddbc22c163635758c2f59 
> 
> Diff: https://reviews.apache.org/r/38284/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38143: Adding executor HTTP API protobuf to V1

[Isabel Jimenez]

> On Sept. 10, 2015, 1:17 a.m., Vinod Kone wrote:
> > include/mesos/v1/executor/executor.proto, lines 89-90
> > 
> >
> > Much like with the scheduler, invalid calls result in BadRequest. We 
> > should really update this wording in executor and scheduler protos.

Will open a new patch with both updates.


- Isabel


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


On Sept. 10, 2015, 10:32 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38143/
> ---
> 
> (Updated Sept. 10, 2015, 10:32 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3375
> https://issues.apache.org/jira/browse/MESOS-3375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor protobufswere introduced in Mesos for the HTTP API, they need to be 
> added to /v1 so they reflect changes made on v1/mesos.proto. This protobuf 
> ought to be changed as the executor HTTP API design evolves.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor/executor.hpp PRE-CREATION 
>   include/mesos/v1/executor/executor.proto PRE-CREATION 
>   src/Makefile.am 8963cea 
> 
> Diff: https://reviews.apache.org/r/38143/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 38265: mesos: Update style guide for usage of std::atomic.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Sept. 10, 2015, 6:54 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38265/
> ---
> 
> (Updated Sept. 10, 2015, 6:54 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> mesos: Update style guide for usage of std::atomic.
> 
> 
> Diffs
> -
> 
>   docs/mesos-c++-style-guide.md 5e4d13e0456e577f05631b77349e4b1e6f0945c7 
> 
> Diff: https://reviews.apache.org/r/38265/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38282: Fixed flaky ReservationEndpointsTest.AvailableResources test.

[Joseph Wu]

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

Ship it!


Ship It!

- Joseph Wu


On Sept. 10, 2015, 5:50 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38282/
> ---
> 
> (Updated Sept. 10, 2015, 5:50 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Bugs: MESOS-3411
> https://issues.apache.org/jira/browse/MESOS-3411
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_endpoints_tests.cpp 
> 795f1cf4c88abf3fdb96013b386236bf1a4a312a 
> 
> Diff: https://reviews.apache.org/r/38282/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

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

[Michael Park]

> On Sept. 10, 2015, 9:54 a.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp, lines 191-192
> > 
> >
> > The above test already performs the roundtrip of Protobuf -> JSON -> 
> > Protobuf. Is it beneficial to add an additional conversion to JSON here? 
> > What does it further test?
> 
> Alexander Rukletsov wrote:
> The rationale here is that equality check for protobufs is not well 
> defined (we do it overselves), hence an additional check via converted `JSON` 
> objects looked to me like a reasonable addition.

Hm, I see. So I think you're saying that if the protobuf happened to parse 
incorrectly but the test passed due to a bug in `operator==`, converting it 
back to `JSON` for the final test could expose that bug. Is that correct?

If it is, it doesn't seem like this is the right place to test for that. It 
would make more sense to me to add a separate test for `operator==` for 
`SimpleMessage`.
In general, I think everything being used in a test aside from the thing being 
tested should have already been tested for correctness.
Since otherwise we would have to test the dependent functions being used in any 
given test in addition to the thing that's actually being tested.

What do you think?


- Michael


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


On Sept. 10, 2015, 6:36 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37827/
> ---
> 
> (Updated Sept. 10, 2015, 6:36 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Joseph Wu, 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 37280: Maintenance Primitives: Added updateInverseOffer to Allocator.

[Benjamin Hindman]

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

Ship it!



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


Indentation.


- Benjamin Hindman


On Sept. 2, 2015, 7:32 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37280/
> ---
> 
> (Updated Sept. 2, 2015, 7:32 p.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/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp 
> aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 
> 38f8fd2c84314bb3731684d0e9795cb4f50a227e 
>   src/tests/mesos.hpp 906948d459b5a88a4ad7952801eb8c540b58c569 
> 
> Diff: https://reviews.apache.org/r/37280/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38289: Handle bad request in Docker registry client.

[Timothy Chen]

> On Sept. 11, 2015, 4:57 a.m., Jojy Varghese wrote:
> > src/slave/containerizer/provisioners/docker/registry_client.cpp, line 572
> > 
> >
> > You will have to set O_NONBLOCK on the file descriptor.

This is done in io::write already.


- Timothy


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


On Sept. 11, 2015, 3:58 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38289/
> ---
> 
> (Updated Sept. 11, 2015, 3:58 a.m.)
> 
> 
> Review request for mesos and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Handle bad request in Docker registry client.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8963cea9fd7e3bee450a1059e6383e5ab868a17b 
>   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 
> ff29d562c7f1bd5f0579e97cdbd60c2b2f36329e 
> 
> Diff: https://reviews.apache.org/r/38289/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38289: Handle bad request in Docker registry client.

[Timothy Chen]

> On Sept. 11, 2015, 4:57 a.m., Jojy Varghese wrote:
> > src/slave/containerizer/provisioners/docker/registry_client.cpp, line 386
> > 
> >
> > This block should be refactored out so that it can be handled in line 
> > 278 also.

Not sure what you mean, this is 278:

  if (!resend) {
return Failure("Bad response: " + httpResponse.status);
  }
  
A Bad request shouldn't trigger a resend right?


- Timothy


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


On Sept. 11, 2015, 3:58 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38289/
> ---
> 
> (Updated Sept. 11, 2015, 3:58 a.m.)
> 
> 
> Review request for mesos and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Handle bad request in Docker registry client.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8963cea9fd7e3bee450a1059e6383e5ab868a17b 
>   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 
> ff29d562c7f1bd5f0579e97cdbd60c2b2f36329e 
> 
> Diff: https://reviews.apache.org/r/38289/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38259: [MESOS-3340] Command-line flags should take precedence over OS Env variables

[Klaus Ma]

> On Sept. 10, 2015, 4:40 p.m., haosdent huang wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp, line 569
> > 
> >
> > Is it possible to change to foreach here?
> 
> Klaus Ma wrote:
> OK, let me address it.

Just checked the code, we can not do that because 1) it's an array, 2) no 
requirement that argv is ended with NULL.


- Klaus


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


On Sept. 11, 2015, 2:43 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38259/
> ---
> 
> (Updated Sept. 11, 2015, 2:43 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Marco Massenzio.
> 
> 
> Bugs: MESOS-3340
> https://issues.apache.org/jira/browse/MESOS-3340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, it appears that re-defining a flag on the command-line that was 
> already defined via a OS Env var (MESOS_*) causes the Master to fail with a 
> not very helpful message.
> 
> For example, if one has MESOS_QUORUM defined, this happens:
> 
> $ ./mesos-master --zk=zk://192.168.1.4/mesos --quorum=1 
> --hostname=192.168.1.4 --ip=192.168.1.4
> Duplicate flag 'quorum' on command line
> 
> which is not very helpful.
> 
> Current solution is to throw error if any duplication; over-write may make 
> user confused about the result.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
> 9da213f802aec6a7768ce6f5aea7b437d980356a 
>   3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
> ebf8cd656625b7fd414cacaa87f156c95df29438 
> 
> Diff: https://reviews.apache.org/r/38259/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37284: Maintenance Primitives: Added support for Accept / Decline of InverseOffers in master.

[Benjamin Hindman]

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

Ship it!


Still a Ship it, just another comment.


src/master/master.cpp (lines 2757 - 2758)


Can we make these comments more verbose please?


- Benjamin Hindman


On Sept. 2, 2015, 7:33 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37284/
> ---
> 
> (Updated Sept. 2, 2015, 7:33 p.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.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
> 
> Diff: https://reviews.apache.org/r/37284/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38265: mesos: Update style guide for usage of std::atomic.

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 九月 10, 2015, 6:54 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38265/
> ---
> 
> (Updated 九月 10, 2015, 6:54 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> mesos: Update style guide for usage of std::atomic.
> 
> 
> Diffs
> -
> 
>   docs/mesos-c++-style-guide.md 5e4d13e0456e577f05631b77349e4b1e6f0945c7 
> 
> Diff: https://reviews.apache.org/r/38265/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38158: Refactored Value::Ranges coalesce().

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38158]

All tests passed.

- Mesos ReviewBot


On Sept. 10, 2015, 6:15 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38158/
> ---
> 
> (Updated Sept. 10, 2015, 6:15 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Till Toenshoff.
> 
> 
> Bugs: MESOS-3051
> https://issues.apache.org/jira/browse/MESOS-3051
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The goal of this refactoring was to reuse the Ranges objects as much as 
> possible, as prior there was substantial time spend in allocation/destruction 
> (MESOS-3051).
> 
> 
> Diffs
> -
> 
>   include/mesos/values.hpp e300580431f7fd6cff06e9617c0227b51c4cb8e2 
>   src/common/values.cpp 750264e603b4cde2011f07f4434a4b34fe3e512f 
>   src/tests/resources_tests.cpp 2ae93a9c8235e5e4643539d409df51c39c6d7e56 
>   src/tests/values_tests.cpp fc35d97894a2de6207b9337180e2160e6f2cb1f5 
> 
> Diff: https://reviews.apache.org/r/38158/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 38259: [MESOS-3340] Command-line flags should take precedence over OS Env variables

[Klaus Ma]

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

(Updated Sept. 11, 2015, 5:24 a.m.)


Review request for mesos, Bernd Mathiske and Marco Massenzio.


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


Repository: mesos


Description
---

Currently, it appears that re-defining a flag on the command-line that was 
already defined via a OS Env var (MESOS_*) causes the Master to fail with a not 
very helpful message.

For example, if one has MESOS_QUORUM defined, this happens:

$ ./mesos-master --zk=zk://192.168.1.4/mesos --quorum=1 
--hostname=192.168.1.4 --ip=192.168.1.4
Duplicate flag 'quorum' on command line

which is not very helpful.

Current solution is to throw error if any duplication; over-write may make user 
confused about the result.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 9da213f 
  3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp ebf8cd6 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 38154: Switched to type traits for checking whether a type is a protobuf message.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37826, 37827, 37830, 38154]

All tests passed.

- Mesos ReviewBot


On Sept. 10, 2015, 6:45 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38154/
> ---
> 
> (Updated Sept. 10, 2015, 6:45 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/38154/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 38289: Handle bad request in Docker registry client.

[Jojy Varghese]

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



src/slave/containerizer/provisioners/docker/registry_client.cpp (line 386)


This block should be refactored out so that it can be handled in line 278 
also.



src/slave/containerizer/provisioners/docker/registry_client.cpp (line 563)


You will have to set O_NONBLOCK on the file descriptor.


- Jojy Varghese


On Sept. 11, 2015, 3:58 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38289/
> ---
> 
> (Updated Sept. 11, 2015, 3:58 a.m.)
> 
> 
> Review request for mesos and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Handle bad request in Docker registry client.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8963cea9fd7e3bee450a1059e6383e5ab868a17b 
>   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 
> ff29d562c7f1bd5f0579e97cdbd60c2b2f36329e 
> 
> Diff: https://reviews.apache.org/r/38289/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Review Request 38287: Check if the futrue is failed before dispatch in freeze()

[Jian Qiu]

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

Review request for mesos, Ian Downes, Jie Yu, and Paul Brett.


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


Repository: mesos


Description
---

This check in cgroups::freezer::freeze() is necessary
since the freezer process can be terminated in its
initialize method


Diffs
-

  src/linux/cgroups.cpp 6ef42ed1bc719f334d1ac6e90919a1bc1840d31f 

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


Testing
---

./mesos-tests.sh 
--gtest_filter="CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_FreezeNonFreezer"
 --verbose


Thanks,

Jian Qiu

Review Request 38289: Handle bad request in Docker registry client.

[Timothy Chen]

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

Review request for mesos and Jojy Varghese.


Repository: mesos


Description
---

Handle bad request in Docker registry client.


Diffs
-

  src/Makefile.am 8963cea9fd7e3bee450a1059e6383e5ab868a17b 
  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 
ff29d562c7f1bd5f0579e97cdbd60c2b2f36329e 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 38251: FrameworkInfo should only be updated if the re-registration is valid

[Guangya Liu]

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

(Updated 九月 11, 2015, 5:06 a.m.)


Review request for mesos, Joris Van Remoortere and Vinod Kone.


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


Repository: mesos


Description
---

FrameworkInfo should only be updated if the re-registration is valid


Diffs (updated)
-

  src/master/master.cpp 4b60e637b19e749e27c4a8eb9b0a95c7e9305c5f 

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


Testing
---


Thanks,

Guangya Liu