Re: Review Request 48588: Devolved v1 operator protos to unversioned operator protos in Agent.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48583, 48584, 48585, 48587, 48588]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On June 12, 2016, 4:31 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48588/
> ---
> 
> (Updated June 12, 2016, 4:31 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Devolved v1 operator protos to unversioned operator protos in Agent.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
>   src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
>   src/slave/validation.hpp 59b12f0109f6fa4c570f02d90834502a8edebe45 
>   src/slave/validation.cpp 189d10caf4517a1aff62b523d8eb99d9d1ca4b6a 
> 
> Diff: https://reviews.apache.org/r/48588/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48585: Devolved v1 operator protos to unversioned operator protos in Master.

[haosdent huang]

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

(Updated June 12, 2016, 5:55 a.m.)


Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


Changes
---

Address @anandmazumdar's comments.


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


Repository: mesos


Description
---

Devolved v1 operator protos to unversioned operator protos in Master.


Diffs (updated)
-

  src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
  src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
  src/master/validation.hpp e1271bbaebaac52a078eedfdc2299c7c6f4474f8 
  src/master/validation.cpp 7b9c2281b2ab1295211f0dd385cd77947fbd63be 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48585: Devolved v1 operator protos to unversioned operator protos in Master.

[Anand Mazumdar]

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


Fix it, then Ship it!




LGTM.. Just a few more nits to clean up...


src/internal/devolve.hpp (line 30)


Why do we need this include for this patch?



src/internal/devolve.hpp (line 32)


Bring this above the `mesos.hpp` include.



src/master/http.cpp (line 578)


Micro Nit: remove `mesos::`, just `master::Call` should suffice.



src/master/http.cpp (lines 1298 - 1299)


Let's just do this inline as per my earlier comment:

```cpp
return OK(serialize(responseContentType,
evolve(_flags())),
  stringify(responseContentType));
```

The reasoning being that we should not have `v1::` response variables in 
our internal functions and leave that business logic to the `evolve` functions. 
Sound good?



src/master/master.hpp (line 47)


Can we get rid of this now that we don't use the `v1` headers in our 
internal code.


- Anand Mazumdar


On June 12, 2016, 4:15 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48585/
> ---
> 
> (Updated June 12, 2016, 4:15 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Devolved v1 operator protos to unversioned operator protos in Master.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/master/validation.hpp e1271bbaebaac52a078eedfdc2299c7c6f4474f8 
>   src/master/validation.cpp 7b9c2281b2ab1295211f0dd385cd77947fbd63be 
> 
> Diff: https://reviews.apache.org/r/48585/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 48594: Added bundled zookeeper to the module-dependencies.

[Avinash sridharan]

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

Review request for mesos, Jie Yu and Kapil Arya.


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


Repository: mesos


Description
---

Modules that want to use the `State` API for storing information in
the replicated log require zookeeper client headers. With this change
the bundled zookeeper client headers will be installed along with the
Mesos libraries and headers under the Mesos installation path.


Diffs
-

  3rdparty/Makefile.am fb4a37d50e751703b4ccddb0e004b58560707067 

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


Testing
---

make and make install

After running make install made sure that the bundled client libraries are 
installed under the $MESOS_INSTALL path. Here is an example output:

vagrant@mesos-dev:~/mesos_install$ find ./ -name "zookeeper*"
./lib/mesos/3rdparty/home/vagrant/mesos_install/include/zookeeper
./lib/mesos/3rdparty/home/vagrant/mesos_install/include/zookeeper/zookeeper.h
./lib/mesos/3rdparty/home/vagrant/mesos_install/include/zookeeper/zookeeper_version.h
./lib/mesos/3rdparty/home/vagrant/mesos_install/include/zookeeper/zookeeper_log.h
./lib/mesos/3rdparty/home/vagrant/mesos_install/include/zookeeper/zookeeper.jute.h
./include/mesos/zookeeper
./include/mesos/zookeeper/zookeeper.hpp
./include/mesos/state/zookeeper.hpp


Thanks,

Avinash sridharan

Re: Review Request 48585: Devolved v1 operator protos to unversioned operator protos in Master.

[haosdent huang]

> On June 11, 2016, 8:33 p.m., Anand Mazumdar wrote:
> > src/internal/evolve.hpp, lines 120-121
> > 
> >
> > It's fine to evolve `JSON::Object` directly into `v1` to avoid making 
> > an extra copy. We adopted a similar approach with the Scheduler/Executor 
> > API's i.e. we had an `evolve` from old style messages to `v1::Call`.

Thanks a lot for your review! It solve all my concerns about this patch. I just 
updated, may you help review this again, thank you in advance.


- haosdent


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


On June 12, 2016, 4:15 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48585/
> ---
> 
> (Updated June 12, 2016, 4:15 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Devolved v1 operator protos to unversioned operator protos in Master.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/master/validation.hpp e1271bbaebaac52a078eedfdc2299c7c6f4474f8 
>   src/master/validation.cpp 7b9c2281b2ab1295211f0dd385cd77947fbd63be 
> 
> Diff: https://reviews.apache.org/r/48585/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48588: Devolved v1 operator protos to unversioned operator protos in Agent.

[haosdent huang]

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

(Updated June 12, 2016, 4:31 a.m.)


Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


Changes
---

Address @anandmazumdar's comments.


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


Repository: mesos


Description
---

Devolved v1 operator protos to unversioned operator protos in Agent.


Diffs (updated)
-

  src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
  src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
  src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
  src/slave/validation.hpp 59b12f0109f6fa4c570f02d90834502a8edebe45 
  src/slave/validation.cpp 189d10caf4517a1aff62b523d8eb99d9d1ca4b6a 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48585: Devolved v1 operator protos to unversioned operator protos in Master.

[haosdent huang]

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

(Updated June 12, 2016, 4:15 a.m.)


Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


Changes
---

Address @anandmazumdar's comments.


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


Repository: mesos


Description
---

Devolved v1 operator protos to unversioned operator protos in Master.


Diffs (updated)
-

  src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
  src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
  src/master/validation.hpp e1271bbaebaac52a078eedfdc2299c7c6f4474f8 
  src/master/validation.cpp 7b9c2281b2ab1295211f0dd385cd77947fbd63be 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48593: Refactor Ranges Subtraction.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On June 12, 2016, 3:28 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 3:28 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

[haosdent huang]

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




src/common/values.cpp (line 282)


Usually we perfer to return `Value::Ranges` directly.
```
Value::Ranges IntervalSet2Ranges(const IntervalSet& set)
```

In additional, `convertToRanges` maybe a better name



src/common/values.cpp (line 285)


I think we could use foreach here?


- haosdent huang


On June 12, 2016, 3:28 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 3:28 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Review Request 48593: Refactor Ranges Subtraction.

[Yanyan Hu]

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

Review request for mesos.


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


Repository: mesos


Description
---

This patch reimplement Ranges subtraction using
IntervalSet data type: Ranges values will be
converted to IntervalSet values for subtraction
and the result will be converted back to Ranges
after subtraction is done. This change is for
fixing jira MESOS-5425.


Diffs
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 

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


Testing
---

make check


Thanks,

Yanyan Hu

Re: Review Request 45472: Add `NetClsSubsystem` for cgroups unified isolator.

[Qian Zhang]

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



I think in `NetClsSubsystem`, you still need a stub for the `update()` method, 
please check the following code:
https://github.com/apache/mesos/blob/0.28.2/src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp#L461:L465

If we do not have the stub for the `update()` method, then the TODO in the 
above code will be lost.

- Qian Zhang


On April 16, 2016, 6:22 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45472/
> ---
> 
> (Updated April 16, 2016, 6:22 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, Guangya Liu, Ian 
> Downes, Jie Yu, and Kevin Klues.
> 
> 
> Bugs: MESOS-5046
> https://issues.apache.org/jira/browse/MESOS-5046
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add `NetClsSubsystem` for cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45472/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48588: Devolved v1 operator protos to unversioned operator protos in Agent.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48583, 48584, 48585, 48587, 48588]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On June 11, 2016, 6:57 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48588/
> ---
> 
> (Updated June 11, 2016, 6:57 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Devolved v1 operator protos to unversioned operator protos in Agent.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
>   src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
>   src/slave/validation.hpp 59b12f0109f6fa4c570f02d90834502a8edebe45 
>   src/slave/validation.cpp 189d10caf4517a1aff62b523d8eb99d9d1ca4b6a 
> 
> Diff: https://reviews.apache.org/r/48588/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48584: Added unversioned protos for master API.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On June 11, 2016, 5:19 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48584/
> ---
> 
> (Updated June 11, 2016, 5:19 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unversioned protos for master API.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.hpp PRE-CREATION 
>   include/mesos/master/master.proto PRE-CREATION 
>   src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 
> 
> Diff: https://reviews.apache.org/r/48584/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48585: Devolved v1 operator protos to unversioned operator protos in Master.

[Anand Mazumdar]

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



LGTM overall. Minor comments around evolving `Response` to `v1::Response` 
before returning back to the client.


src/internal/evolve.hpp (lines 120 - 121)


It's fine to evolve `JSON::Object` directly into `v1` to avoid making an 
extra copy. We adopted a similar approach with the Scheduler/Executor API's 
i.e. we had an `evolve` from old style messages to `v1::Call`.



src/internal/evolve.hpp (line 123)


You would need another overload for evolving `master::Response` -> 
`master::v1::Response`.



src/internal/evolve.cpp (line 468)


`evolve(response)`;



src/internal/evolve.cpp (line 514)


`evolve(response)`



src/master/http.cpp (lines 1293 - 1299)


hmm .. We need to return a Response of type `v1::master::Response` but this 
is of type of `master::Response`.  :-)

Read my earlier comment. It should be fine to `evolve` to a 
`v1::master::Response` directly. Something like:

```cpp
return OK(serialize(responseContentType,
evolve(_flags())),
  stringify(responseContentType));
```



src/master/http.cpp (line 1347)


It's fine to evolve to v1 directly from `JSON::Object` like `GET_FLAGS` in 
my earlier comment.



src/master/http.cpp (line 1363)


`evolve(response)`



src/master/master.hpp (line 1285)


Not yours: Can we make the first character in caps?


- Anand Mazumdar


On June 11, 2016, 6:42 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48585/
> ---
> 
> (Updated June 11, 2016, 6:42 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Devolved v1 operator protos to unversioned operator protos in Master.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/master/validation.hpp e1271bbaebaac52a078eedfdc2299c7c6f4474f8 
>   src/master/validation.cpp 7b9c2281b2ab1295211f0dd385cd77947fbd63be 
> 
> Diff: https://reviews.apache.org/r/48585/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46594: Added test for isolator cleanup before prepare.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46576, 46577, 46593, 46594]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On June 11, 2016, 6:06 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46594/
> ---
> 
> (Updated June 11, 2016, 6:06 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, Neil Conway, and Timothy 
> Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for isolator cleanup before prepare.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 64b331d389f0b7757f50c81619114e18db2dd564 
> 
> Diff: https://reviews.apache.org/r/46594/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Verified that the test fails if we reverted the fix.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48583: Added `VersionInfo`, `Flag`, `Metrics` to unversioned mesos protos.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On June 11, 2016, 5:19 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48583/
> ---
> 
> (Updated June 11, 2016, 5:19 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We add `VersionInfo`, `Flag`, `Metrics` to v1 mesos protos in 549a0340,
> but didn't add them to unversioned mesos protos as well. This change
> attempt to fix this.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dbffb16f4317df786ee38999a49e4e89733ff9ff 
> 
> Diff: https://reviews.apache.org/r/48583/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 48588: Devolved v1 operator protos to unversioned operator protos in Agent.

[haosdent huang]

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

Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


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


Repository: mesos


Description
---

Devolved v1 operator protos to unversioned operator protos in Agent.


Diffs
-

  src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
  src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
  src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
  src/slave/validation.hpp 59b12f0109f6fa4c570f02d90834502a8edebe45 
  src/slave/validation.cpp 189d10caf4517a1aff62b523d8eb99d9d1ca4b6a 

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


Testing
---


Thanks,

haosdent huang

Review Request 48587: Added unversioned protos for agent API.

[haosdent huang]

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

Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


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


Repository: mesos


Description
---

Added unversioned protos for agent API.


Diffs
-

  include/mesos/slave/slave.hpp PRE-CREATION 
  include/mesos/slave/slave.proto PRE-CREATION 
  src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48585: Devolved v1 operator protos to unversioned operator protos in Master.

[haosdent huang]

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

(Updated June 11, 2016, 6:42 p.m.)


Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Devolved v1 operator protos to unversioned operator protos in Master.


Diffs (updated)
-

  src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
  src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
  src/master/validation.hpp e1271bbaebaac52a078eedfdc2299c7c6f4474f8 
  src/master/validation.cpp 7b9c2281b2ab1295211f0dd385cd77947fbd63be 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 46594: Added test for isolator cleanup before prepare.

[Gilbert Song]

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

(Updated June 11, 2016, 11:06 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, Neil Conway, and Timothy 
Chen.


Repository: mesos


Description
---

Added test for isolator cleanup before prepare.


Diffs (updated)
-

  src/tests/containerizer/mesos_containerizer_tests.cpp 
64b331d389f0b7757f50c81619114e18db2dd564 

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


Testing
---

make check

Verified that the test fails if we reverted the fix.


Thanks,

Gilbert Song

Re: Review Request 48373: Integrated the `NvidiaGpuAllocator` into the `NvidiaGpuIsolator`.

[Kevin Klues]

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

(Updated June 11, 2016, 5:37 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Updated to properly use `defer()` on lambdas in `.then()` statements from after 
operations through the `NvidiaGPUAllocator`


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


Repository: mesos


Description
---

All logic to do GPU allocation is now conducted asynchronously through
the `NvidiaGpuAllocator` component.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/nvidia.hpp 
181a2aad97da9ee0f6ffa42cdba9c93dc0077ff7 
  src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp 
d7557a0c338e8c0e51461b2326600c03f89c2e8b 

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


Testing
---

GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 48585: Devolved v1 operator protos to unversioned operator protos in Master.

[haosdent huang]

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




src/internal/evolve.cpp (line 445)


Hi, @anandmazumdar, @dongdong, @guoger, @vinodkone

I have a question here, should I move this out of `evolve.cpp` and create 
some file to put `Try parse(const JSON::Object& 
json);`?



src/master/http.cpp (line 1298)


@anandmazumdar, @dongdong, @guoger, @vinodkone
Another question is should I evolve the response to v1 before call 
`serialize`?


- haosdent huang


On June 11, 2016, 5:20 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48585/
> ---
> 
> (Updated June 11, 2016, 5:20 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Devolved v1 operator protos to unversioned operator protos in Master.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/master/validation.hpp e1271bbaebaac52a078eedfdc2299c7c6f4474f8 
>   src/master/validation.cpp 7b9c2281b2ab1295211f0dd385cd77947fbd63be 
> 
> Diff: https://reviews.apache.org/r/48585/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 48585: Devolved v1 operator protos to unversioned operator protos in Master.

[haosdent huang]

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

Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


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


Repository: mesos


Description
---

Devolved v1 operator protos to unversioned operator protos in Master.


Diffs
-

  src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
  src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
  src/master/validation.hpp e1271bbaebaac52a078eedfdc2299c7c6f4474f8 
  src/master/validation.cpp 7b9c2281b2ab1295211f0dd385cd77947fbd63be 

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


Testing
---


Thanks,

haosdent huang

Review Request 48583: Added `VersionInfo`, `Flag`, `Metrics` to unversioned mesos protos.

[haosdent huang]

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

Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


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


Repository: mesos


Description
---

We add `VersionInfo`, `Flag`, `Metrics` to v1 mesos protos in 549a0340,
but didn't add them to unversioned mesos protos as well. This change
attempt to fix this.


Diffs
-

  include/mesos/mesos.proto dbffb16f4317df786ee38999a49e4e89733ff9ff 

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


Testing
---


Thanks,

haosdent huang

Review Request 48584: Added unversioned protos for master API.

[haosdent huang]

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

Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


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


Repository: mesos


Description
---

Added unversioned protos for master API.


Diffs
-

  include/mesos/master/master.hpp PRE-CREATION 
  include/mesos/master/master.proto PRE-CREATION 
  src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48566: Fixed continuation logic for sandbox authorization.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48563, 48566]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On June 11, 2016, 3:01 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48566/
> ---
> 
> (Updated June 11, 2016, 3:01 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-5587
> https://issues.apache.org/jira/browse/MESOS-5587
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the continuation followed via `.then([=]`,
> which potentially executes the continuation on a differen
> process. This patch fixes this behavior (it should
> run on the same process) and avoids potential race 
> conditions if the file logic is handled by different 
> processes.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp 55c310af149cb03abfa1ffbb6477dc1ac5dc14be 
> 
> Diff: https://reviews.apache.org/r/48566/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

> On June 8, 2016, 5:32 a.m., Benjamin Mahler wrote:
> > src/tests/values_tests.cpp, lines 39-72
> > 
> >
> > It would be great to split apart this test into the respective value 
> > types, e.g.
> > 
> > ```
> > ValuesTest.ParseScalar
> > ValuesTest.ParseRanges
> > ValuesTest.ParseSet
> > ```
> > 
> > With more test cases that show what is currently accepted and what is 
> > not. There are a lot of other formats currently accepted from what I can 
> > tell (see my comment below).

Logged MESOS-5603 to trace those test refactor; I'll continue to work on this.

https://issues.apache.org/jira/browse/MESOS-5603


- Klaus


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


On June 11, 2016, 10:29 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated June 11, 2016, 10:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43561]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On June 11, 2016, 2:29 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated June 11, 2016, 2:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

> On June 8, 2016, 5:32 a.m., Benjamin Mahler wrote:
> > src/tests/values_tests.cpp, line 210
> > 
> >
> > If I understand correctly, the following are currently accepted way to 
> > specify 1-4:
> > 
> > `[[1-4]]`
> > `[1-2]\n[3-4]`
> > `[1-2],[3-4]`
> > 
> > I'm ok with rejecting these in favor of the following canonical formats:
> > 
> > `[1-2,3-4]`
> > 
> > Although it doesn't have to be coalesced. However, we should notify the 
> > user mailing list that we're moving towards more strict parsing of ranges 
> > if you're going to change what we accept.
> > 
> > Ideally, we could pull in a regex or parsing expression grammar library 
> > to more formally define our formats. The current parsing code is a mess.

Logged MESOS-5602 to trace "expression grammar library", 
https://issues.apache.org/jira/browse/MESOS-5602

And i'll send an email to dev@ and user@ list for this notification.


- Klaus


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


On June 11, 2016, 10:29 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated June 11, 2016, 10:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 48566: Fixed continuation logic for sandbox authorization.

[Joerg Schad]

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

(Updated June 11, 2016, 3:01 p.m.)


Review request for mesos, Adam B and Alexander Rojas.


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


Repository: mesos


Description (updated)
---

Previously the continuation followed via `.then([=]`,
which potentially executes the continuation on a differen
process. This patch fixes this behavior (it should
run on the same process) and avoids potential race 
conditions if the file logic is handled by different 
processes.


Diffs
-

  src/files/files.cpp 55c310af149cb03abfa1ffbb6477dc1ac5dc14be 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 48566: Fixed continuation logic for sandbox authorization.

[Joerg Schad]

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

(Updated June 11, 2016, 3:01 p.m.)


Review request for mesos, Adam B and Alexander Rojas.


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


Repository: mesos


Description (updated)
---

Previously the continuation followed via `.then([=]`,
which potentially executes the continuation on a differen
process. This patch fixes this behavior (it should
run on the same process).


Diffs (updated)
-

  src/files/files.cpp 55c310af149cb03abfa1ffbb6477dc1ac5dc14be 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 48563: Fixed continuation logic in endpoint authorization.

[Joerg Schad]

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

(Updated June 11, 2016, 3 p.m.)


Review request for mesos and Adam B.


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


Repository: mesos


Description (updated)
---

Previously the continuation followed via `.then([=]`,
which potentially executes the continuation on a different
process. This patch fixes this behavior and avoids race 
conditions between the master process changing the 
related data-structures and another process reading them.


Diffs (updated)
-

  src/master/http.cpp 4b2d1386e1ecb447b597a432f9df9adaa5c3aa37 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 46158: Completed implementation of the cgroups unified isolator.

[Qian Zhang]

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




src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (line 131)


I think there is a special case that we need to handle: In the OS using 
systemd (e.g., RHEL 7.1), `cpu` and `cpuacct` subsystems are co-mounted, like 
this:
```
ls -la /sys/fs/cgroup/
...
lrwxrwxrwx.  1 root root  11 Dec 28 15:12 cpu -> cpu,cpuacct
lrwxrwxrwx.  1 root root  11 Dec 28 15:12 cpuacct -> cpu,cpuacct
drwxr-xr-x.  4 root root   0 Jan  2 17:01 cpu,cpuacct
...
```
That means in this `hierarchies` hashmap, the values of the two keys `cpu` 
and `cpuacct` are same (both of them are `/sys/fs/cgroup/cpu,cpuacct`), this 
will cause an issue in `CgroupsIsolatorProcess::prepare()`: In this method, we 
will call `prepareHierarchy()` which will check if the cgroup for the container 
to be creatd exists or not, if not, create the cgroup, if yes, return an Error. 
For the OS using systemd, I think we will always get the Error since for both 
`cpu` and `cpuacct` subsystems, we will try to create cgroup in the same 
location, i.e., `/sys/fs/cgroup/cpu,cpuacct/mesos/`.

You can take a look at the following code in the existing 
`CgroupsCpushareIsolatorProcess` class for how we handle this case. 
https://github.com/apache/mesos/blob/0.28.2/src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp#L94:L144



src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (line 141)


I would suggest to rename this method to `prepareHierarchy()`,  the word 
`get` usually means this is a read-only method, but I think this is not a 
read-only method, we may do some write operations, e.g., mount subsystem, 
create root cgroup.

And I know there is already a method in this class named 
`prepareHierarchy()`, I would suggest to rename it to `createCgroup()` because 
the intent of that method is to create cgroup for a specific container, right?



src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (line 153)


I think it is a bit strange to name it as `cgroupName`, actually it is 
subsystem, right?



src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (line 288)


I would suggest to rename this method to `createCgroup()` since its intent 
is to create cgroup for a specific container.


- Qian Zhang


On April 16, 2016, 6:14 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46158/
> ---
> 
> (Updated April 16, 2016, 6:14 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Completed implementation of the cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46158/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

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

(Updated June 11, 2016, 10:29 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joris Van 
Remoortere.


Changes
---

rebase


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


Repository: mesos


Description
---

Improve Ranges parsing to handle single values.


Diffs (updated)
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

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


Testing
---

make
make check GTEST_FILTER=~"*"
./src/mesos-test


Thanks,

Klaus Ma

Re: Review Request 45574: Add `PerfEventSubsystem` for cgroups unified isolator.

[Qian Zhang]

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




src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (lines 37 - 38)


So even after we introduce the unified cgroups isolator, we still need to 
keep the code of the original `cgroups/xxx` isolators? But I think it does not 
make sense since we have consolidated their code into the unified cgroups 
isolator, so ideally under `src/slave/containerizer/mesos/isolators/cgroups`, 
we will only have `cgroups.hpp/cpp` and `subsystem.hpp/cpp`, right?



src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (lines 57 - 75)


In this method, I think you missed the part that we need to ensure no other 
subsystem is attached to the hierarchy, please check the the following code as 
a reference, and actually  `CgroupsCpushareIsolatorProcess::create()`, 
`CgroupsNetClsIsolatorProcess::create()` also have the similar logic.

https://github.com/apache/mesos/blob/0.28.2/src/slave/containerizer/mesos/isolators/cgroups/mem.cpp#L96:L105


- Qian Zhang


On April 16, 2016, 6:28 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45574/
> ---
> 
> (Updated April 16, 2016, 6:28 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5047
> https://issues.apache.org/jira/browse/MESOS-5047
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add `PerfEventSubsystem` for cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp 674d5f886ea41b939a8e48832ee6595a78b2f6ce 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45574/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 45363: Add `MemorySubsystem` for cgroups unified isolator.

[Qian Zhang]

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




src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (lines 650 - 656)


I see this method is different from the original one: 
https://github.com/apache/mesos/blob/0.28.2/src/slave/containerizer/mesos/isolators/cgroups/mem.cpp#L66:L69

Can you please let me know why you want to do this change?


- Qian Zhang


On April 16, 2016, 6:22 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45363/
> ---
> 
> (Updated April 16, 2016, 6:22 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5045
> https://issues.apache.org/jira/browse/MESOS-5045
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add `MemorySubsystem` for cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45363/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48046: Implement v1::master::Call::GET_TASKS.

[Jay Guo]

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



Ping. I don't think it is necessary to devolve protobuf in this API. `_tasks()` 
only takes primitives: int32 and string.

- Jay Guo


On June 9, 2016, 11:17 p.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48046/
> ---
> 
> (Updated June 9, 2016, 11:17 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5493
> https://issues.apache.org/jira/browse/MESOS-5493
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement v1::master::Call::GET_TASKS.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48046/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu 14.04
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 48581: Removed duplicated code in 'strings::tokenize()'.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46425, 48581]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On June 11, 2016, 11 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48581/
> ---
> 
> (Updated June 11, 2016, 11 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed duplicated code in 'strings::tokenize()'.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/strings.hpp 
> 162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 
> 
> Diff: https://reviews.apache.org/r/48581/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 48581: Removed duplicated code in 'strings::tokenize()'.

[Klaus Ma]

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

(Updated June 11, 2016, 7 p.m.)


Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Removed duplicated code in 'strings::tokenize()'.


Diffs
-

  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 

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


Testing (updated)
---

make && make check


Thanks,

Klaus Ma

Review Request 48581: Removed duplicated code in 'strings::tokenize()'.

[Klaus Ma]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Removed duplicated code in 'strings::tokenize()'.


Diffs
-

  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 48259: Added test case `MasterAPITest.UpdateAndGetMaintenanceSchedule`.

[haosdent huang]

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

(Updated June 11, 2016, 9:25 a.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added test case `MasterAPITest.UpdateAndGetMaintenanceSchedule`.


Diffs (updated)
-

  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48285: Implemented START_MAINTENANCE Call in v1 master API.

[haosdent huang]

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

(Updated June 11, 2016, 9:25 a.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented START_MAINTENANCE Call in v1 master API.


Diffs (updated)
-

  src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
  src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48286: Implemented STOP_MAINTENANCE Call in v1 master API.

[haosdent huang]

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

(Updated June 11, 2016, 9:25 a.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented STOP_MAINTENANCE Call in v1 master API.


Diffs (updated)
-

  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48260: Added test case `MasterAPITest.GetMaintenanceStatus`.

[haosdent huang]

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

(Updated June 11, 2016, 9:25 a.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added test case `MasterAPITest.GetMaintenanceStatus`.


Diffs (updated)
-

  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---

Test in CentOS 7.2

```
./bin/mesos-tests.sh --gtest_filter="*MasterAPITest*"
```


Thanks,

haosdent huang

Re: Review Request 48115: Added maintenance V1 header.

[haosdent huang]

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

(Updated June 11, 2016, 9:24 a.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added maintenance V1 header.


Diffs (updated)
-

  include/mesos/v1/maintenance/maintenance.hpp PRE-CREATION 
  src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48257: Implemented GET_MAINTENANCE_SCHEDULE Call in v1 master API.

[haosdent huang]

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

(Updated June 11, 2016, 9:24 a.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented GET_MAINTENANCE_SCHEDULE Call in v1 master API.


Diffs (updated)
-

  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48084: Implemented v1::master::Call::GET_MAINTENANCE_STATUS.

[haosdent huang]

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

(Updated June 11, 2016, 9:24 a.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented v1::master::Call::GET_MAINTENANCE_STATUS.


Diffs (updated)
-

  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48116: Implemented UPDATE_MAINTENANCE_SCHEDULE Call in v1 master API.

[haosdent huang]

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

(Updated June 11, 2016, 9:24 a.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented UPDATE_MAINTENANCE_SCHEDULE Call in v1 master API.


Diffs (updated)
-

  src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
  src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 46425: Add helper function to simplify tokenize handling.

[Klaus Ma]

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

(Updated June 11, 2016, 5:21 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

rebase


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


Repository: mesos


Description
---

Add helper function to simplify tokenize handling.


Diffs (updated)
-

  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 
  3rdparty/stout/tests/strings_tests.cpp 
b54a9dbf162403310b8bba687442e184a473f5a6 

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


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 48566: Fixed continuation logic for sandbox authorization.

[Adam B]

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


Fix it, then Ship it!




LGTM, barring my nits about lambda indentation.


src/files/files.cpp (lines 347 - 350)


I prefer wrapping before the `[]`
```
  return authorize(requestedPath, principal)
.then(defer(self(),
[this, path, jsonp](bool authorized) -> Future {
  if (authorized) {
```



src/files/files.cpp (lines 488 - 491)


Wrap before `[]()` (inputs) and `->` (outputs):
```
  return authorize(requestedPath, principal)
.then(defer(self(),
[this, offset, length, path, jsonp](bool authorized)
  -> Future {
  if (authorized) {
```



src/files/files.cpp (lines 635 - 638)


```
  return authorize(requestedPath, principal)
.then(defer(self(),
[this, path](bool authorized) -> Future {
  if (authorized) {
```


- Adam B


On June 10, 2016, 9:19 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48566/
> ---
> 
> (Updated June 10, 2016, 9:19 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-5587
> https://issues.apache.org/jira/browse/MESOS-5587
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the continuation followed via `.then([=]`,
> which potentially executes the continuation on a different
> process. This patch fixes this behavior (it should
> run on the same process) and avoids potential race 
> conditions if the file logic is handled by different 
> processes.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp 55c310af149cb03abfa1ffbb6477dc1ac5dc14be 
> 
> Diff: https://reviews.apache.org/r/48566/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 48563: Fixed continuation logic in endpoint authorization.

[Adam B]

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


Fix it, then Ship it!




Logically looks fine to me, although now I'm worried that we have another 100 
`.then()`'s that should be deferring.
Please consider my suggestions for wrapping/indentation.


src/master/http.cpp (lines 1869 - 1872)


I don't really like this indentation style, since it makes the tuple fields 
seem like they're at a higher level than they are. Looking around, it seems we 
have a stronger preference to wrap the `[=]` onto the next line (indented 4), 
and sometimes even wrap the `->` onto the next line. How about:
```
  return collect(frameworksApprover, tasksApprover, executorsApprover)
.then(defer(master->self(),
[=](const tuple& approvers) -> Response {
  auto state = [this, ](JSON::ObjectWriter* writer) {
```



src/master/http.cpp (lines 2217 - )


Similarly, I think this looks better if we wrap before the `[]` instead of 
after the `(`:
```
  return frameworksApprover
.then(defer(master->self(),
[=](const Owned& frameworksApprover) -> Response {
  auto stateSummary =
  [this, ](JSON::ObjectWriter* writer) {
Owned frameworksApprover_ = frameworksApprover;
```
See how it keeps the lambda function definition on one line?



src/master/http.cpp (lines 2662 - 2668)


Ditto:
```
  return collect(frameworksApprover, tasksApprover, executorsApprover)
.then(defer(master->self(),
[=](const tuple& approvers) -> Response {
  // Get approver from tuple.
  Owned frameworksApprover;
```


- Adam B


On June 10, 2016, 9:28 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48563/
> ---
> 
> (Updated June 10, 2016, 9:28 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-5587
> https://issues.apache.org/jira/browse/MESOS-5587
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the continuation followed via `.then([=]`,
> which potentially executes the continuation on a different
> process. This patch fixes this behavior and avoids race 
> conditions between the master process changing the 
> related data-structures and another process reading them.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 4b2d1386e1ecb447b597a432f9df9adaa5c3aa37 
> 
> Diff: https://reviews.apache.org/r/48563/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 48579: Reset "dirty" to false in sort().

[Guangya Liu]

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

(Updated 六月 11, 2016, 8:30 a.m.)


Review request for mesos, Benjamin Mahler and Klaus Ma.


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


Repository: mesos


Description
---

The "dirty" was set to "true" when the total resources in cluster was
updated. But in sorter, the "dirty" was never set back as "false" after
re-calculate share for each clients.

This patch reset the "dirty" to "false" in sort(), this can make sure
only one client share was updated when there are allocation changes but
not all clients in the cluster.

This can improve the performance of sorter.


Diffs (updated)
-

  src/master/allocator/sorter/drf/sorter.cpp 
65d473a5da0d846214c930c14d333040b2085b13 
  src/tests/sorter_tests.cpp eb207a36c18198588cd8a98b3860a66c2ff7a641 

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


Testing
---

[==] Running 9 tests from 1 test case.
[--] Global test environment set-up.
[--] 9 tests from SorterTest
[ RUN  ] SorterTest.DRFSorter
[   OK ] SorterTest.DRFSorter (1 ms)
[ RUN  ] SorterTest.WDRFSorter
[   OK ] SorterTest.WDRFSorter (1 ms)
[ RUN  ] SorterTest.SplitResourceShares
[   OK ] SorterTest.SplitResourceShares (0 ms)
[ RUN  ] SorterTest.UpdateAllocation
[   OK ] SorterTest.UpdateAllocation (0 ms)
[ RUN  ] SorterTest.MultipleSlaves
[   OK ] SorterTest.MultipleSlaves (0 ms)
[ RUN  ] SorterTest.MultipleSlavesUpdateAllocation
[   OK ] SorterTest.MultipleSlavesUpdateAllocation (1 ms)
[ RUN  ] SorterTest.UpdateTotal
[   OK ] SorterTest.UpdateTotal (0 ms)
[ RUN  ] SorterTest.MultipleSlavesUpdateTotal
[   OK ] SorterTest.MultipleSlavesUpdateTotal (0 ms)
[ RUN  ] SorterTest.RevocableResources
[   OK ] SorterTest.RevocableResources (0 ms)
[--] 9 tests from SorterTest (37 ms total)

[--] Global test environment tear-down
[==] 9 tests from 1 test case ran. (48 ms total)
[  PASSED  ] 9 tests.


Thanks,

Guangya Liu

Re: Review Request 48455: Some cleanup for sorter.cpp.

[Guangya Liu]

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

(Updated 六月 11, 2016, 8:27 a.m.)


Review request for mesos and Benjamin Mahler.


Summary (updated)
-

Some cleanup for sorter.cpp.


Repository: mesos


Description (updated)
---

Some cleanup for sorter.cpp.


Diffs (updated)
-

  src/master/allocator/sorter/drf/sorter.cpp 
65d473a5da0d846214c930c14d333040b2085b13 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 48259: Added test case `MasterAPITest.UpdateAndGetMaintenanceSchedule`.

[haosdent huang]

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

(Updated June 11, 2016, 8:12 a.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Address @kaysoky and @vinodkone's comments.


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


Repository: mesos


Description
---

Added test case `MasterAPITest.UpdateAndGetMaintenanceSchedule`.


Diffs (updated)
-

  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48260: Added test case `MasterAPITest.GetMaintenanceStatus`.

[haosdent huang]

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

(Updated June 11, 2016, 8:12 a.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Address @vinodkone's comments.


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


Repository: mesos


Description
---

Added test case `MasterAPITest.GetMaintenanceStatus`.


Diffs (updated)
-

  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---

Test in CentOS 7.2

```
./bin/mesos-tests.sh --gtest_filter="*MasterAPITest*"
```


Thanks,

haosdent huang

Re: Review Request 47374: Separated mesos test helpers into a separate library.

[Adam B]

> On May 23, 2016, 5:09 p.m., Adam B wrote:
> > src/Makefile.am, lines 1937-1938
> > 
> >
> > Why does libmesos_tests_la_SOURCES need to include qos_controllers code?
> 
> Joseph Wu wrote:
> I don't remember the exact reason, but one of my previous iterations (not 
> published) broke on this module.  It's unnecessary for the current 
> implementation.

I looked it up. LoadQoSController isn't used by Mesos (by default), since only 
the NoopQoSController is referenced in non-test code (as a default). Both are 
included in oversubscription_tests.cpp


- Adam


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


On June 10, 2016, 5:03 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47374/
> ---
> 
> (Updated June 10, 2016, 5:03 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, Kapil Arya, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives external projects easier access to the test helpers used in
> mesos tests.  
> 
> For example, a module writer may want to write a test like 
> `src/tests/oversubscription_tests.cpp`.  To build and link against 
> this library, the module writer would mimic the build flags for tests:
> ```
> # Main test file is taken directly from Mesos.
> my_module_tests_SOURCES = \
>   $(MESOS)/src/tests/main.cpp
> 
> my_module_tests_CPPFLAGS = \
>   -I$(GMOCK)/include   \
>   -I$(GTEST)/include   \
>   -I$(MESOS)/include/mesos \
>   -I$(ZOOKEEPER)/include   \
>   -I$(ZOOKEEPER)/generated \
>   $(AM_CPPFLAGS)
>   
> my_module_tests_LDADD = \
>   $(MESOS)/3rdparty/.libs/libgmock.la \
>   $(MESOS)/src/.libs/libmesos.la  \
>   $(MESOS)/src/.libs/libmesos_tests.la
> ```
> 
> 
> Diffs
> -
> 
>   src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
> 
> Diff: https://reviews.apache.org/r/47374/diff/
> 
> 
> Testing
> ---
> 
> make check on OSX, CentOS 7, Ubuntu 14
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>