Re: Review Request 34976: Added installation instructions for Ubuntu 14.04 and OSX

[Marco Massenzio]

> On June 3, 2015, 5:29 a.m., Adam B wrote:
> > docs/getting-started.md, line 24
> > 
> >
> > Can you merge this in with the Ubuntu 12.04 instructions below? I don't 
> > think they're drastically different, and we can probably assume 14.04 
> > instead of 12.04 now.

They were actually identical - but was unsure as to whether anyone cared enough 
to want to keep them separate.
Please see the new version (a generic `Ubuntu` one).


- Marco


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


On June 3, 2015, 6:44 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34976/
> ---
> 
> (Updated June 3, 2015, 6:44 a.m.)
> 
> 
> Review request for mesos and Dave Lester.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added installation instructions for Ubuntu 14.04 and OSX
> 
> 
> Diffs
> -
> 
>   docs/getting-started.md f0436575ec568e445f897ed28f50bcd823302d75 
> 
> Diff: https://reviews.apache.org/r/34976/diff/
> 
> 
> Testing
> ---
> 
> This are the steps I've followed to build Mesos on my Mac and on my Ubuntu 
> box.
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 34980: Pass callback to the resource estimator to retrieve ResourceUsage from resource monitor on demand.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [34962, 34980]

All tests passed.

- Mesos ReviewBot


On June 3, 2015, 5:56 a.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34980/
> ---
> 
> (Updated June 3, 2015, 5:56 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2773
> https://issues.apache.org/jira/browse/MESOS-2773
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Pass callback to the resource estimator to retrieve ResourceUsage from 
> Resource Monitor on demand.
> 
> This is neccessary, since ResourceEstimator needs data (current statistics 
> for each executor) on which it will base its slack predictions.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/resource_estimator.hpp 
> e7f5decdabdfc75587d784a08a98c62ffdfc2909 
>   src/slave/resource_estimators/noop.hpp 
> 2234ed6da17346c74c2e232ff9eae501531515e1 
>   src/slave/resource_estimators/noop.cpp PRE-CREATION 
>   src/slave/slave.cpp 271cb03770cd08406054dfce35d0821475e49b05 
>   src/tests/mesos.hpp aeeed61ea987963345a8b5c6e09d14e51098bb2e 
>   src/tests/oversubscription_tests.cpp 
> f047b90d8f0b4dfe7e6cad6dc58909451e9f894b 
> 
> Diff: https://reviews.apache.org/r/34980/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 34976: Added installation instructions for Ubuntu 14.04 and OSX

[Marco Massenzio]

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

(Updated June 3, 2015, 6:44 a.m.)


Review request for mesos and Dave Lester.


Changes
---

Merged the two Ubuntu sections (they were identical anyway)


Repository: mesos


Description
---

Added installation instructions for Ubuntu 14.04 and OSX


Diffs (updated)
-

  docs/getting-started.md f0436575ec568e445f897ed28f50bcd823302d75 

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


Testing
---

This are the steps I've followed to build Mesos on my Mac and on my Ubuntu box.


Thanks,

Marco Massenzio

Review Request 34984: Added help for files

[Aditi Dixit]

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

Review request for mesos, Niklas Nielsen and Vinod Kone.


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


Repository: mesos


Description
---

Added help for files


Diffs
-

  src/files/files.cpp ce02411c5e579d7551b4325ec141fd89e4ee7255 

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


Testing
---

make check


Thanks,

Aditi Dixit

Re: Review Request 34980: Pass callback to the resource estimator to retrieve ResourceUsage from resource monitor on demand.

[Bartek Plotka]

> On June 3, 2015, 5:58 a.m., Bartek Plotka wrote:
> > Any idea why review requests failed to show diff of noop.hpp?
> > 
> > I guess it something with 
> > https://reviews.apache.org/r/34962/diff/#file976910,  since this patch 
> > depends on that.

reviewboard*


- Bartek


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


On June 3, 2015, 5:56 a.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34980/
> ---
> 
> (Updated June 3, 2015, 5:56 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2773
> https://issues.apache.org/jira/browse/MESOS-2773
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Pass callback to the resource estimator to retrieve ResourceUsage from 
> Resource Monitor on demand.
> 
> This is neccessary, since ResourceEstimator needs data (current statistics 
> for each executor) on which it will base its slack predictions.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/resource_estimator.hpp 
> e7f5decdabdfc75587d784a08a98c62ffdfc2909 
>   src/slave/resource_estimators/noop.hpp 
> 2234ed6da17346c74c2e232ff9eae501531515e1 
>   src/slave/resource_estimators/noop.cpp PRE-CREATION 
>   src/slave/slave.cpp 271cb03770cd08406054dfce35d0821475e49b05 
>   src/tests/mesos.hpp aeeed61ea987963345a8b5c6e09d14e51098bb2e 
>   src/tests/oversubscription_tests.cpp 
> f047b90d8f0b4dfe7e6cad6dc58909451e9f894b 
> 
> Diff: https://reviews.apache.org/r/34980/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 34980: Pass callback to the resource estimator to retrieve ResourceUsage from resource monitor on demand.

[Bartek Plotka]

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


Any idea why review requests failed to show diff of noop.hpp?

I guess it something with https://reviews.apache.org/r/34962/diff/#file976910,  
since this patch depends on that.

- Bartek Plotka


On June 3, 2015, 5:56 a.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34980/
> ---
> 
> (Updated June 3, 2015, 5:56 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2773
> https://issues.apache.org/jira/browse/MESOS-2773
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Pass callback to the resource estimator to retrieve ResourceUsage from 
> Resource Monitor on demand.
> 
> This is neccessary, since ResourceEstimator needs data (current statistics 
> for each executor) on which it will base its slack predictions.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/resource_estimator.hpp 
> e7f5decdabdfc75587d784a08a98c62ffdfc2909 
>   src/slave/resource_estimators/noop.hpp 
> 2234ed6da17346c74c2e232ff9eae501531515e1 
>   src/slave/resource_estimators/noop.cpp PRE-CREATION 
>   src/slave/slave.cpp 271cb03770cd08406054dfce35d0821475e49b05 
>   src/tests/mesos.hpp aeeed61ea987963345a8b5c6e09d14e51098bb2e 
>   src/tests/oversubscription_tests.cpp 
> f047b90d8f0b4dfe7e6cad6dc58909451e9f894b 
> 
> Diff: https://reviews.apache.org/r/34980/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Review Request 34980: Pass callback to the resource estimator to retrieve ResourceUsage from resource monitor on demand.

[Bartek Plotka]

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

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


Summary (updated)
-

Pass callback to the resource estimator to retrieve ResourceUsage from resource 
monitor on demand.


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


Repository: mesos


Description (updated)
---

Pass callback to the resource estimator to retrieve ResourceUsage from Resource 
Monitor on demand.

This is neccessary, since ResourceEstimator needs data (current statistics for 
each executor) on which it will base its slack predictions.


Diffs (updated)
-

  include/mesos/slave/resource_estimator.hpp 
e7f5decdabdfc75587d784a08a98c62ffdfc2909 
  src/slave/resource_estimators/noop.hpp 
2234ed6da17346c74c2e232ff9eae501531515e1 
  src/slave/resource_estimators/noop.cpp PRE-CREATION 
  src/slave/slave.cpp 271cb03770cd08406054dfce35d0821475e49b05 
  src/tests/mesos.hpp aeeed61ea987963345a8b5c6e09d14e51098bb2e 
  src/tests/oversubscription_tests.cpp f047b90d8f0b4dfe7e6cad6dc58909451e9f894b 

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


Testing (updated)
---

make check


Thanks,

Bartek Plotka

Re: Review Request 34976: Added installation instructions for Ubuntu 14.04 and OSX

[Adam B]

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



docs/getting-started.md


Can you merge this in with the Ubuntu 12.04 instructions below? I don't 
think they're drastically different, and we can probably assume 14.04 instead 
of 12.04 now.


- Adam B


On June 2, 2015, 7:25 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34976/
> ---
> 
> (Updated June 2, 2015, 7:25 p.m.)
> 
> 
> Review request for mesos and Dave Lester.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added installation instructions for Ubuntu 14.04 and OSX
> 
> 
> Diffs
> -
> 
>   docs/getting-started.md f0436575ec568e445f897ed28f50bcd823302d75 
> 
> Diff: https://reviews.apache.org/r/34976/diff/
> 
> 
> Testing
> ---
> 
> This are the steps I've followed to build Mesos on my Mac and on my Ubuntu 
> box.
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 34721: Added QoS kill executor correction test.

[Niklas Nielsen]

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

(Updated June 2, 2015, 9:30 p.m.)


Review request for mesos, Bartek Plotka, Jie Yu, Joris Van Remoortere, and 
Vinod Kone.


Changes
---

Rebased


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


Repository: mesos


Description
---

See summary


Diffs (updated)
-

  src/tests/mesos.hpp aeeed61ea987963345a8b5c6e09d14e51098bb2e 
  src/tests/mesos.cpp d3a8bb70478993883f34804c8e2d7559d16c3538 
  src/tests/oversubscription_tests.cpp f047b90d8f0b4dfe7e6cad6dc58909451e9f894b 

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


Testing
---

make check (with new qos test)


Thanks,

Niklas Nielsen

Re: Review Request 34720: Added kill executor correction to slave.

[Niklas Nielsen]

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

(Updated June 2, 2015, 9:29 p.m.)


Review request for mesos, Bartek Plotka, Jie Yu, Joris Van Remoortere, and 
Vinod Kone.


Changes
---

Rebased


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


Repository: mesos


Description
---

See summary


Diffs (updated)
-

  src/slave/slave.hpp 245ea062a56461d96ee3055be1c93ec508d1bec7 
  src/slave/slave.cpp 271cb03770cd08406054dfce35d0821475e49b05 

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


Testing
---

make check


Thanks,

Niklas Nielsen

Re: Review Request 34633: Added QoS Controller test.

[Niklas Nielsen]

> On May 29, 2015, 12:23 p.m., Vinod Kone wrote:
> > src/tests/oversubscription_tests.cpp, line 117
> > 
> >
> > Why not implement the TODO?
> > 
> > s/AWAIT_READY(received)/AWAIT_ASSERT_EQ(expected, received)/ ?

We still don't have the equality operator overload - want me to throw that in a 
dependent review?


- Niklas


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


On June 2, 2015, 9:29 p.m., Niklas Nielsen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34633/
> ---
> 
> (Updated June 2, 2015, 9:29 p.m.)
> 
> 
> Review request for mesos, Bartek Plotka, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new QoS Controller test which use the TestQoSController to fill the 
> correction queue and verify the slave receiving corrections.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp aeeed61ea987963345a8b5c6e09d14e51098bb2e 
>   src/tests/oversubscription_tests.cpp 
> f047b90d8f0b4dfe7e6cad6dc58909451e9f894b 
> 
> Diff: https://reviews.apache.org/r/34633/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>

Re: Review Request 34633: Added QoS Controller test.

[Niklas Nielsen]

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

(Updated June 2, 2015, 9:29 p.m.)


Review request for mesos, Bartek Plotka, Jie Yu, and Vinod Kone.


Repository: mesos


Description
---

Added new QoS Controller test which use the TestQoSController to fill the 
correction queue and verify the slave receiving corrections.


Diffs (updated)
-

  src/tests/mesos.hpp aeeed61ea987963345a8b5c6e09d14e51098bb2e 
  src/tests/oversubscription_tests.cpp f047b90d8f0b4dfe7e6cad6dc58909451e9f894b 

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


Testing
---

make check


Thanks,

Niklas Nielsen

Re: Review Request 34632: Added QoS Controller in slave

[Niklas Nielsen]

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

(Updated June 2, 2015, 9:28 p.m.)


Review request for mesos, Bartek Plotka, Jie Yu, and Vinod Kone.


Repository: mesos


Description
---

Wired up QoS Controller in slave and necessary test code for the new slave 
constructor argument.


Diffs (updated)
-

  src/local/local.cpp 84f73e21e66e7a654b6b09f6a57a70a1ccc34b40 
  src/slave/flags.hpp 15dd8387df1344807b54e911a8094a89f4629cbd 
  src/slave/main.cpp e3a45f4dfa0c8cc5d28d795f42bdc8cd14b7b10d 
  src/slave/slave.hpp 245ea062a56461d96ee3055be1c93ec508d1bec7 
  src/slave/slave.cpp 271cb03770cd08406054dfce35d0821475e49b05 
  src/tests/cluster.hpp 7370c77de68d3a13340a69f6e794d97306d4bbb6 
  src/tests/mesos.hpp aeeed61ea987963345a8b5c6e09d14e51098bb2e 
  src/tests/mesos.cpp d3a8bb70478993883f34804c8e2d7559d16c3538 

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


Testing
---

make check


Thanks,

Niklas Nielsen

Re: Review Request 34631: Added QoS Controller.

[Niklas Nielsen]

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

(Updated June 2, 2015, 9:26 p.m.)


Review request for mesos, Bartek Plotka, Jie Yu, and Vinod Kone.


Repository: mesos


Description
---

Added QoS Controller class and NOOP controller implementation


Diffs (updated)
-

  include/mesos/slave/qos_controller.hpp PRE-CREATION 
  src/Makefile.am f045b89e8590c4775b21980bedff14cc27d31bfb 
  src/slave/qos_controller.hpp PRE-CREATION 
  src/slave/qos_controller.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Niklas Nielsen

Re: Review Request 34976: Added installation instructions for Ubuntu 14.04 and OSX

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [34976]

All tests passed.

- Mesos ReviewBot


On June 3, 2015, 2:25 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34976/
> ---
> 
> (Updated June 3, 2015, 2:25 a.m.)
> 
> 
> Review request for mesos and Dave Lester.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added installation instructions for Ubuntu 14.04 and OSX
> 
> 
> Diffs
> -
> 
>   docs/getting-started.md f0436575ec568e445f897ed28f50bcd823302d75 
> 
> Diff: https://reviews.apache.org/r/34976/diff/
> 
> 
> Testing
> ---
> 
> This are the steps I've followed to build Mesos on my Mac and on my Ubuntu 
> box.
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 34970: Added an example framework for oversubscribing resources.

[Vinod Kone]

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



src/examples/oversubscribing_framework.cpp


Also, note that with my changes in fight, a task cannot use revocable 
resources if it's executor does not. This currently means a command task cannot 
be launched.

The right fix is for the master to ignore this check for command executors 
and update the Slave::getExecutorInfo() set executor's resources as revocable 
if the task uses revocable resources. I will work on it next.


- Vinod Kone


On June 3, 2015, 1:21 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34970/
> ---
> 
> (Updated June 3, 2015, 1:21 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-2655
> https://issues.apache.org/jira/browse/MESOS-2655
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Based on a flag, this framework oversubscribes a particular resource.
> 
> Currently, only `--oversubscribed_resource=cpus` is supported, which launches 
> tasks that consume revocable cpus using a tight while loop in the shell.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a5a7306b1ef65ca2b643653779ab76c26dbb5c90 
>   src/examples/oversubscribing_framework.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34970/diff/
> 
> 
> Testing
> ---
> 
> Since we do not have an non-zero estimator yet, I modified the slave to send 
> revocable resources in order to manually test this.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 34970: Added an example framework for oversubscribing resources.

[Vinod Kone]

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



src/examples/oversubscribing_framework.cpp


reorder



src/examples/oversubscribing_framework.cpp


Is the framework expected to ever terminate under normal circumstances?

As it is currently written, this framework cannot be part of 'make check' 
right? That's a bummer. We  really need to have an end to end test that 
exercises oversubscription flow.

Can we take the task command line as a parameter? that way for make check 
we can pass in a short lived task and for using externally we can pass in a 
never-neding task as you did here.

thoughts?


- Vinod Kone


On June 3, 2015, 1:21 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34970/
> ---
> 
> (Updated June 3, 2015, 1:21 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-2655
> https://issues.apache.org/jira/browse/MESOS-2655
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Based on a flag, this framework oversubscribes a particular resource.
> 
> Currently, only `--oversubscribed_resource=cpus` is supported, which launches 
> tasks that consume revocable cpus using a tight while loop in the shell.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a5a7306b1ef65ca2b643653779ab76c26dbb5c90 
>   src/examples/oversubscribing_framework.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34970/diff/
> 
> 
> Testing
> ---
> 
> Since we do not have an non-zero estimator yet, I modified the slave to send 
> revocable resources in order to manually test this.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 33824: Stub for the new HTTP API in the slave

[Marco Massenzio]

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


Hi Alex - I noticed the new 'code drop' but none of the comments were addressed 
in the new diff: is something missing?

Thanks!

- Marco Massenzio


On June 2, 2015, 2:03 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33824/
> ---
> 
> (Updated June 2, 2015, 2:03 p.m.)
> 
> 
> Review request for mesos, Isabel Jimenez, Marco Massenzio, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds a stub for the dispatcher of the incoming messages for the new HTTP API.
> 
> 
> Diffs
> -
> 
>   include/mesos/executor.hpp f3cd3ccd505ed4b308a1a42b238fc21fe45cc3b3 
>   src/slave/slave.hpp 245ea062a56461d96ee3055be1c93ec508d1bec7 
>   src/slave/slave.cpp 271cb03770cd08406054dfce35d0821475e49b05 
> 
> Diff: https://reviews.apache.org/r/33824/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Review Request 34976: Added installation instructions for Ubuntu 14.04 and OSX

[Marco Massenzio]

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

Review request for mesos and Dave Lester.


Repository: mesos


Description
---

Added installation instructions for Ubuntu 14.04 and OSX


Diffs
-

  docs/getting-started.md f0436575ec568e445f897ed28f50bcd823302d75 

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


Testing
---

This are the steps I've followed to build Mesos on my Mac and on my Ubuntu box.


Thanks,

Marco Massenzio

Re: Review Request 34886: MESOS-2787: Fixed the exception KeyError: 'mem_rss_bytes' when run command mesos-ps

[weitao zhou]

> On 六月 1, 2015, 6:41 p.m., Ben Mahler wrote:
> > src/cli/mesos-ps, line 138
> > 
> >
> > s/+/ + /

@Ben, can we ship it now? :-)


- weitao


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


On 六月 2, 2015, 2:29 a.m., weitao zhou wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34886/
> ---
> 
> (Updated 六月 2, 2015, 2:29 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-2787
> https://issues.apache.org/jira/browse/MESOS-2787
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The exception KeyError: 'mem_rss_bytes' or 'KeyError: cpus_system_time_secs' 
> raised
> when run command mesos ps --master=<> which is caused by some mesos slave 
> nodes can't
> return the statistics yet now
> 
> 
> Diffs
> -
> 
>   src/cli/mesos-ps 8ca7a64d0ab6f0b8db823ca95d5f7500487b8753 
> 
> Diff: https://reviews.apache.org/r/34886/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> weitao zhou
> 
>

Re: Review Request 33934: Added the client address to http::Request.

[Ben Mahler]

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



3rdparty/libprocess/src/process.cpp


This is incorrect, need to use `getpeername` here!


- Ben Mahler


On May 7, 2015, 5:47 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33934/
> ---
> 
> (Updated May 7, 2015, 5:47 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-2519
> https://issues.apache.org/jira/browse/MESOS-2519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This information is not part of the http request, but it seems that many http 
> libraries include it inside their "Request" abstraction as it is useful 
> information.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 5210ce07c8ea47cf67681455c2f5eb0fda4bb426 
>   3rdparty/libprocess/src/process.cpp 
> 8ab0dbcd3e9b858244f1e67061897143204eada0 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> dfdb233de93552ab7040647ee1958a85d47c9482 
> 
> Diff: https://reviews.apache.org/r/33934/diff/
> 
> 
> Testing
> ---
> 
> Added to the existing tests to ensure it is being set.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 34970: Added an example framework for oversubscribing resources.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [34968, 34969, 34970]

All tests passed.

- Mesos ReviewBot


On June 3, 2015, 1:21 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34970/
> ---
> 
> (Updated June 3, 2015, 1:21 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-2655
> https://issues.apache.org/jira/browse/MESOS-2655
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Based on a flag, this framework oversubscribes a particular resource.
> 
> Currently, only `--oversubscribed_resource=cpus` is supported, which launches 
> tasks that consume revocable cpus using a tight while loop in the shell.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a5a7306b1ef65ca2b643653779ab76c26dbb5c90 
>   src/examples/oversubscribing_framework.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34970/diff/
> 
> 
> Testing
> ---
> 
> Since we do not have an non-zero estimator yet, I modified the slave to send 
> revocable resources in order to manually test this.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 34969: Print executor status consistently in the example frameworks.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On June 3, 2015, 1:21 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34969/
> ---
> 
> (Updated June 3, 2015, 1:21 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This uses WSTRINGIFY from status_utils.hpp instead of manually or incorrectly 
> printing the status.
> 
> 
> Diffs
> -
> 
>   src/examples/low_level_scheduler_libprocess.cpp 
> bd228fd178d639146fe237794c39872dab9085cb 
>   src/examples/low_level_scheduler_pthread.cpp 
> 81388f145ede3803ed5a533797a25d910d467316 
>   src/examples/persistent_volume_framework.cpp 
> 6a0c0cb61a06d1a0e7608fe2447167c18111ea43 
> 
> Diff: https://reviews.apache.org/r/34969/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 34968: Fixed a deadlock in libprocess Metrics initialization.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On June 3, 2015, 1:07 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34968/
> ---
> 
> (Updated June 3, 2015, 1:07 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Discovered this while writing an example framework that uses metrics.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> 38905d3e920288ac8c18ebed2743ebac9f438d2e 
> 
> Diff: https://reviews.apache.org/r/34968/diff/
> 
> 
> Testing
> ---
> 
> This fixed the deadlock here:
> 
> ```
> Thread 1 (Thread 0x7f421282f770 (LWP 9942)):
> #0  0x7f420d625019 in pthread_cond_wait@@GLIBC_2.3.2 () from 
> /lib64/libpthread.so.0
> #1  0x7f42104023e7 in once () from 
> /home/bmahler/git/mesos/build/src/.libs/libmesos-0.23.0.so
> #2  0x7f4210f17fb1 in instance () from 
> /home/bmahler/git/mesos/build/src/.libs/libmesos-0.23.0.so
> #3  0x7f4210f4be51 in initialize () from 
> /home/bmahler/git/mesos/build/src/.libs/libmesos-0.23.0.so
> #4  0x7f4210f58014 in ProcessBase () from 
> /home/bmahler/git/mesos/build/src/.libs/libmesos-0.23.0.so
> #5  0x7f4210f1a289 in MetricsProcess () from 
> /home/bmahler/git/mesos/build/src/.libs/libmesos-0.23.0.so
> #6  0x7f4210f17fcd in instance () from 
> /home/bmahler/git/mesos/build/src/.libs/libmesos-0.23.0.so
> #7  0x0048cc94 in add ()
> #8  0x0048821c in Metrics ()
> #9  0x00486c07 in OversubscribingScheduler ()
> #10 0x0047f06a in main ()
> ```
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Review Request 34970: Added an example framework for oversubscribing resources.

[Ben Mahler]

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

Review request for mesos, Jie Yu and Vinod Kone.


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


Repository: mesos


Description
---

Based on a flag, this framework oversubscribes a particular resource.

Currently, only `--oversubscribed_resource=cpus` is supported, which launches 
tasks that consume revocable cpus using a tight while loop in the shell.


Diffs
-

  src/Makefile.am a5a7306b1ef65ca2b643653779ab76c26dbb5c90 
  src/examples/oversubscribing_framework.cpp PRE-CREATION 

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


Testing
---

Since we do not have an non-zero estimator yet, I modified the slave to send 
revocable resources in order to manually test this.


Thanks,

Ben Mahler

Review Request 34969: Print executor status consistently in the example frameworks.

[Ben Mahler]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

This uses WSTRINGIFY from status_utils.hpp instead of manually or incorrectly 
printing the status.


Diffs
-

  src/examples/low_level_scheduler_libprocess.cpp 
bd228fd178d639146fe237794c39872dab9085cb 
  src/examples/low_level_scheduler_pthread.cpp 
81388f145ede3803ed5a533797a25d910d467316 
  src/examples/persistent_volume_framework.cpp 
6a0c0cb61a06d1a0e7608fe2447167c18111ea43 

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


Testing
---

make check


Thanks,

Ben Mahler

Re: Review Request 34962: Moved NoopResourceEstimator to a separate file.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On June 3, 2015, 12:11 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34962/
> ---
> 
> (Updated June 3, 2015, 12:11 a.m.)
> 
> 
> Review request for mesos, Bartek Plotka, Niklas Nielsen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved NoopResourceEstimator to a separate file.
> 
> As we're going to introduce more resource estimators in the code base, it 
> makes sense to put them into a directory.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am f045b89e8590c4775b21980bedff14cc27d31bfb 
>   src/examples/test_resource_estimator_module.cpp 
> df85978f2beeccd0cccd893f13a8a0eeacd15e23 
>   src/local/local.cpp 84f73e21e66e7a654b6b09f6a57a70a1ccc34b40 
>   src/slave/main.cpp e3a45f4dfa0c8cc5d28d795f42bdc8cd14b7b10d 
>   src/slave/resource_estimator.hpp 717804d65072a55e6c2bead14ab17be7fec57077 
>   src/slave/resource_estimator.cpp a67640c3878c5707b8ef39c0750711c619680a39 
>   src/slave/resource_estimators/noop.cpp PRE-CREATION 
>   src/tests/cluster.hpp 7370c77de68d3a13340a69f6e794d97306d4bbb6 
> 
> Diff: https://reviews.apache.org/r/34962/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 34968: Fixed a deadlock in libprocess Metrics initialization.

[Ben Mahler]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Discovered this while writing an example framework that uses metrics.


Diffs
-

  3rdparty/libprocess/src/metrics/metrics.cpp 
38905d3e920288ac8c18ebed2743ebac9f438d2e 

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


Testing
---

This fixed the deadlock here:

```
Thread 1 (Thread 0x7f421282f770 (LWP 9942)):
#0  0x7f420d625019 in pthread_cond_wait@@GLIBC_2.3.2 () from 
/lib64/libpthread.so.0
#1  0x7f42104023e7 in once () from 
/home/bmahler/git/mesos/build/src/.libs/libmesos-0.23.0.so
#2  0x7f4210f17fb1 in instance () from 
/home/bmahler/git/mesos/build/src/.libs/libmesos-0.23.0.so
#3  0x7f4210f4be51 in initialize () from 
/home/bmahler/git/mesos/build/src/.libs/libmesos-0.23.0.so
#4  0x7f4210f58014 in ProcessBase () from 
/home/bmahler/git/mesos/build/src/.libs/libmesos-0.23.0.so
#5  0x7f4210f1a289 in MetricsProcess () from 
/home/bmahler/git/mesos/build/src/.libs/libmesos-0.23.0.so
#6  0x7f4210f17fcd in instance () from 
/home/bmahler/git/mesos/build/src/.libs/libmesos-0.23.0.so
#7  0x0048cc94 in add ()
#8  0x0048821c in Metrics ()
#9  0x00486c07 in OversubscribingScheduler ()
#10 0x0047f06a in main ()
```


Thanks,

Ben Mahler

Re: Review Request 34956: Refactored the queueing discipline data structure.

[Ben Mahler]

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



src/linux/routing/queueing/discipline.hpp


Any reason for the const getter boilerplate? From what I can tell, it looks 
like you have a `const Discipline` in your code already.

I think we've avoided adding getters like this in general for simple 
structs (which this looks like to me), so it seems a bit inconsistent?


- Ben Mahler


On June 2, 2015, 10:04 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34956/
> ---
> 
> (Updated June 2, 2015, 10:04 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored the queueing discipline data structure.
> 
> This is for us to more easily extend qdisc support.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am f2110a360b6e2a1e99e1d9630d5dfacfd818530a 
>   src/linux/routing/queueing/discipline.hpp PRE-CREATION 
>   src/linux/routing/queueing/fq_codel.hpp 
> 7de1e31d4c43ac9cbffab1e472ea51140719f900 
>   src/linux/routing/queueing/fq_codel.cpp 
> 4dc2a9d2ed52937f0a78a083980db488c06b45a7 
>   src/linux/routing/queueing/ingress.hpp 
> 84506fecd01522471a7998176c28bea8f1367aed 
>   src/linux/routing/queueing/ingress.cpp 
> ae0c38d2215e7fabcc1060e7385484bd455e1699 
>   src/linux/routing/queueing/internal.hpp 
> d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70 
> 
> Diff: https://reviews.apache.org/r/34956/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 34729: Updated slave to send total amount of oversubscribed resources.

[Ben Mahler]

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



src/slave/slave.cpp


Is the lack of a delay() in this case a bug? Doesn't this break the 
`forwardOversubscribedResources` loop? Is there something else going on that 
ensures the loop continues..?


- Ben Mahler


On May 29, 2015, 12:30 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34729/
> ---
> 
> (Updated May 29, 2015, 12:30 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, and Brian Wickman.
> 
> 
> Bugs: MESOS-2770
> https://issues.apache.org/jira/browse/MESOS-2770
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Per the latest design, the slave needs to send the total amount of 
> oversubscribed resources (both allocated and available).
> 
> Also renamed the message to UpdateSlave to make it more generic (e.g., can be 
> used in the future to update slave's total resources too).
> 
> 
> Diffs
> -
> 
>   src/messages/messages.proto 39dac721fbe5b97842dd5d1d68cc135148ae02a2 
>   src/slave/flags.hpp 6ca59dc9fc748ec738259406642ec17c0752590c 
>   src/slave/flags.cpp a8c7c498d674ca832fa052412a373c9ace4b3fc3 
>   src/slave/slave.hpp 0207eafa914e23e4c72b1e5c4fb43aae6c97049c 
>   src/slave/slave.cpp b4d20294330f791e64a597c67b686aed9de84837 
>   src/tests/oversubscription_tests.cpp 
> 75c25b04c1e6a8e0e7e8fd55440743fe1699af88 
> 
> Diff: https://reviews.apache.org/r/34729/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Will add a test in a subsequent review when a framework is able to launch a 
> revocable task.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 34962: Moved NoopResourceEstimator to a separate file.

[Jie Yu]

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

(Updated June 3, 2015, 12:11 a.m.)


Review request for mesos, Bartek Plotka, Niklas Nielsen, and Vinod Kone.


Changes
---

s/resource_estimator/resource_estimators/


Repository: mesos


Description
---

Moved NoopResourceEstimator to a separate file.

As we're going to introduce more resource estimators in the code base, it makes 
sense to put them into a directory.


Diffs (updated)
-

  src/Makefile.am f045b89e8590c4775b21980bedff14cc27d31bfb 
  src/examples/test_resource_estimator_module.cpp 
df85978f2beeccd0cccd893f13a8a0eeacd15e23 
  src/local/local.cpp 84f73e21e66e7a654b6b09f6a57a70a1ccc34b40 
  src/slave/main.cpp e3a45f4dfa0c8cc5d28d795f42bdc8cd14b7b10d 
  src/slave/resource_estimator.hpp 717804d65072a55e6c2bead14ab17be7fec57077 
  src/slave/resource_estimator.cpp a67640c3878c5707b8ef39c0750711c619680a39 
  src/slave/resource_estimators/noop.cpp PRE-CREATION 
  src/tests/cluster.hpp 7370c77de68d3a13340a69f6e794d97306d4bbb6 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 34962: Moved NoopResourceEstimator to a separate file.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [34962]

All tests passed.

- Mesos ReviewBot


On June 2, 2015, 11:15 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34962/
> ---
> 
> (Updated June 2, 2015, 11:15 p.m.)
> 
> 
> Review request for mesos, Bartek Plotka, Niklas Nielsen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved NoopResourceEstimator to a separate file.
> 
> As we're going to introduce more resource estimators in the code base, it 
> makes sense to put them into a directory.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 081185bb28fa58c8fe0b1ffa8db6599def56e85b 
>   src/examples/test_resource_estimator_module.cpp 
> df85978f2beeccd0cccd893f13a8a0eeacd15e23 
>   src/local/local.cpp 84f73e21e66e7a654b6b09f6a57a70a1ccc34b40 
>   src/slave/main.cpp e3a45f4dfa0c8cc5d28d795f42bdc8cd14b7b10d 
>   src/slave/resource_estimator.hpp 717804d65072a55e6c2bead14ab17be7fec57077 
>   src/slave/resource_estimator.cpp a67640c3878c5707b8ef39c0750711c619680a39 
>   src/slave/resource_estimator/noop.cpp PRE-CREATION 
>   src/tests/cluster.hpp 7370c77de68d3a13340a69f6e794d97306d4bbb6 
> 
> Diff: https://reviews.apache.org/r/34962/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 34962: Moved NoopResourceEstimator to a separate file.

[Jie Yu]

> On June 2, 2015, 11:24 p.m., Niklas Nielsen wrote:
> > Should we name the 'resource_estimator' directory 'resource_estimators' 
> > like we name the directory isolators?
> > 
> > I am fine with either; just wanted to get a feel for what pattern we want 
> > to go with (the containerizer directory is in singular)
> > 
> > Otherwise, LGTM
> 
> Jie Yu wrote:
> Hum, seems that we use plural more often than singular (src/tests, 
> src/credentials, src/files, etc.)

OK, I think 'containerizer' is a legacy issue (there was only one containerizer 
at that time)


- Jie


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


On June 2, 2015, 11:15 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34962/
> ---
> 
> (Updated June 2, 2015, 11:15 p.m.)
> 
> 
> Review request for mesos, Bartek Plotka, Niklas Nielsen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved NoopResourceEstimator to a separate file.
> 
> As we're going to introduce more resource estimators in the code base, it 
> makes sense to put them into a directory.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 081185bb28fa58c8fe0b1ffa8db6599def56e85b 
>   src/examples/test_resource_estimator_module.cpp 
> df85978f2beeccd0cccd893f13a8a0eeacd15e23 
>   src/local/local.cpp 84f73e21e66e7a654b6b09f6a57a70a1ccc34b40 
>   src/slave/main.cpp e3a45f4dfa0c8cc5d28d795f42bdc8cd14b7b10d 
>   src/slave/resource_estimator.hpp 717804d65072a55e6c2bead14ab17be7fec57077 
>   src/slave/resource_estimator.cpp a67640c3878c5707b8ef39c0750711c619680a39 
>   src/slave/resource_estimator/noop.cpp PRE-CREATION 
>   src/tests/cluster.hpp 7370c77de68d3a13340a69f6e794d97306d4bbb6 
> 
> Diff: https://reviews.apache.org/r/34962/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 34306: Added capabilities field to FrameworkInfo.

[Ben Mahler]

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


Could we print the framework's capabilities when it registers with the master? 
Would help debugging, as I was just tripped up by forgetting to set it.

Also, might be easy to fold in to printing the 'checkpoint' capability (maybe 
also at some point folding it in to the Capabilities enum).

- Ben Mahler


On May 21, 2015, 12:42 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34306/
> ---
> 
> (Updated May 21, 2015, 12:42 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2654
> https://issues.apache.org/jira/browse/MESOS-2654
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Capability message was added to make this extensible in future. For example, 
> receiving optimistic offers might be another capability.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 9cc5782256156ed59fd4640091413b76480d939f 
> 
> Diff: https://reviews.apache.org/r/34306/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 34957: Fixed a bug in qdisc search function.

[Jie Yu]

> On June 2, 2015, 11:09 p.m., Paul Brett wrote:
> > src/linux/routing/queueing/internal.hpp, line 143
> > 
> >
> > Not your problem, but we really should either explain why we have to 
> > increment the reference counter here but nowhere else or just fix Netlink 
> > to hide this (it does hide most of the cleanup logic)
> 
> Jie Yu wrote:
> Yeah, we should definitely add some comments. Can you sugguest how to fix 
> Netlink to hide this?
> 
> Paul Brett wrote:
> Chi and I were talking about adding a copy constructor to Netlink that 
> would increment the reference counter using helper functions like the 
> destructors, but that needs a separate ticket and some more thought.

OK, I added a NOTE to explain why nl_object_get(o) is needed here.


- Jie


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


On June 2, 2015, 10:05 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34957/
> ---
> 
> (Updated June 2, 2015, 10:05 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a bug in qdisc search function.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/queueing/internal.hpp 
> d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70 
> 
> Diff: https://reviews.apache.org/r/34957/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 34962: Moved NoopResourceEstimator to a separate file.

[Bartek Plotka]

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

Ship it!


Ship It!

- Bartek Plotka


On June 2, 2015, 11:15 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34962/
> ---
> 
> (Updated June 2, 2015, 11:15 p.m.)
> 
> 
> Review request for mesos, Bartek Plotka, Niklas Nielsen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved NoopResourceEstimator to a separate file.
> 
> As we're going to introduce more resource estimators in the code base, it 
> makes sense to put them into a directory.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 081185bb28fa58c8fe0b1ffa8db6599def56e85b 
>   src/examples/test_resource_estimator_module.cpp 
> df85978f2beeccd0cccd893f13a8a0eeacd15e23 
>   src/local/local.cpp 84f73e21e66e7a654b6b09f6a57a70a1ccc34b40 
>   src/slave/main.cpp e3a45f4dfa0c8cc5d28d795f42bdc8cd14b7b10d 
>   src/slave/resource_estimator.hpp 717804d65072a55e6c2bead14ab17be7fec57077 
>   src/slave/resource_estimator.cpp a67640c3878c5707b8ef39c0750711c619680a39 
>   src/slave/resource_estimator/noop.cpp PRE-CREATION 
>   src/tests/cluster.hpp 7370c77de68d3a13340a69f6e794d97306d4bbb6 
> 
> Diff: https://reviews.apache.org/r/34962/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 34956: Refactored the queueing discipline data structure.

[Jie Yu]

> On June 2, 2015, 10:40 p.m., Paul Brett wrote:
> > src/linux/routing/queueing/discipline.hpp, line 39
> > 
> >
> > Should config be optional?
> 
> Jie Yu wrote:
> We still need a Config struct for each type of qdisc so that the template 
> specification could work. It just looks weired to me to do
> ```
> Discipline(
>   ...
>   None());
> ```
> 
> Do you have a strong opinion on that?

Synced with Paul offline. Decided to punt this for now. I'll add a TODO.


- Jie


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


On June 2, 2015, 10:04 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34956/
> ---
> 
> (Updated June 2, 2015, 10:04 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored the queueing discipline data structure.
> 
> This is for us to more easily extend qdisc support.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am f2110a360b6e2a1e99e1d9630d5dfacfd818530a 
>   src/linux/routing/queueing/discipline.hpp PRE-CREATION 
>   src/linux/routing/queueing/fq_codel.hpp 
> 7de1e31d4c43ac9cbffab1e472ea51140719f900 
>   src/linux/routing/queueing/fq_codel.cpp 
> 4dc2a9d2ed52937f0a78a083980db488c06b45a7 
>   src/linux/routing/queueing/ingress.hpp 
> 84506fecd01522471a7998176c28bea8f1367aed 
>   src/linux/routing/queueing/ingress.cpp 
> ae0c38d2215e7fabcc1060e7385484bd455e1699 
>   src/linux/routing/queueing/internal.hpp 
> d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70 
> 
> Diff: https://reviews.apache.org/r/34956/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 34962: Moved NoopResourceEstimator to a separate file.

[Jie Yu]

> On June 2, 2015, 11:24 p.m., Niklas Nielsen wrote:
> > Should we name the 'resource_estimator' directory 'resource_estimators' 
> > like we name the directory isolators?
> > 
> > I am fine with either; just wanted to get a feel for what pattern we want 
> > to go with (the containerizer directory is in singular)
> > 
> > Otherwise, LGTM

Hum, seems that we use plural more often than singular (src/tests, 
src/credentials, src/files, etc.)


- Jie


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


On June 2, 2015, 11:15 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34962/
> ---
> 
> (Updated June 2, 2015, 11:15 p.m.)
> 
> 
> Review request for mesos, Bartek Plotka, Niklas Nielsen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved NoopResourceEstimator to a separate file.
> 
> As we're going to introduce more resource estimators in the code base, it 
> makes sense to put them into a directory.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 081185bb28fa58c8fe0b1ffa8db6599def56e85b 
>   src/examples/test_resource_estimator_module.cpp 
> df85978f2beeccd0cccd893f13a8a0eeacd15e23 
>   src/local/local.cpp 84f73e21e66e7a654b6b09f6a57a70a1ccc34b40 
>   src/slave/main.cpp e3a45f4dfa0c8cc5d28d795f42bdc8cd14b7b10d 
>   src/slave/resource_estimator.hpp 717804d65072a55e6c2bead14ab17be7fec57077 
>   src/slave/resource_estimator.cpp a67640c3878c5707b8ef39c0750711c619680a39 
>   src/slave/resource_estimator/noop.cpp PRE-CREATION 
>   src/tests/cluster.hpp 7370c77de68d3a13340a69f6e794d97306d4bbb6 
> 
> Diff: https://reviews.apache.org/r/34962/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 34962: Moved NoopResourceEstimator to a separate file.

[Niklas Nielsen]

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


Should we name the 'resource_estimator' directory 'resource_estimators' like we 
name the directory isolators?

I am fine with either; just wanted to get a feel for what pattern we want to go 
with (the containerizer directory is in singular)

Otherwise, LGTM

- Niklas Nielsen


On June 2, 2015, 4:15 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34962/
> ---
> 
> (Updated June 2, 2015, 4:15 p.m.)
> 
> 
> Review request for mesos, Bartek Plotka, Niklas Nielsen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved NoopResourceEstimator to a separate file.
> 
> As we're going to introduce more resource estimators in the code base, it 
> makes sense to put them into a directory.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 081185bb28fa58c8fe0b1ffa8db6599def56e85b 
>   src/examples/test_resource_estimator_module.cpp 
> df85978f2beeccd0cccd893f13a8a0eeacd15e23 
>   src/local/local.cpp 84f73e21e66e7a654b6b09f6a57a70a1ccc34b40 
>   src/slave/main.cpp e3a45f4dfa0c8cc5d28d795f42bdc8cd14b7b10d 
>   src/slave/resource_estimator.hpp 717804d65072a55e6c2bead14ab17be7fec57077 
>   src/slave/resource_estimator.cpp a67640c3878c5707b8ef39c0750711c619680a39 
>   src/slave/resource_estimator/noop.cpp PRE-CREATION 
>   src/tests/cluster.hpp 7370c77de68d3a13340a69f6e794d97306d4bbb6 
> 
> Diff: https://reviews.apache.org/r/34962/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 34957: Fixed a bug in qdisc search function.

[Paul Brett]

> On June 2, 2015, 11:09 p.m., Paul Brett wrote:
> > src/linux/routing/queueing/internal.hpp, line 143
> > 
> >
> > Not your problem, but we really should either explain why we have to 
> > increment the reference counter here but nowhere else or just fix Netlink 
> > to hide this (it does hide most of the cleanup logic)
> 
> Jie Yu wrote:
> Yeah, we should definitely add some comments. Can you sugguest how to fix 
> Netlink to hide this?

Chi and I were talking about adding a copy constructor to Netlink that would 
increment the reference counter using helper functions like the destructors, 
but that needs a separate ticket and some more thought.


- Paul


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


On June 2, 2015, 10:05 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34957/
> ---
> 
> (Updated June 2, 2015, 10:05 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a bug in qdisc search function.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/queueing/internal.hpp 
> d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70 
> 
> Diff: https://reviews.apache.org/r/34957/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 34956: Refactored the queueing discipline data structure.

[Paul Brett]

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

Ship it!


Ship It!

- Paul Brett


On June 2, 2015, 10:04 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34956/
> ---
> 
> (Updated June 2, 2015, 10:04 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored the queueing discipline data structure.
> 
> This is for us to more easily extend qdisc support.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am f2110a360b6e2a1e99e1d9630d5dfacfd818530a 
>   src/linux/routing/queueing/discipline.hpp PRE-CREATION 
>   src/linux/routing/queueing/fq_codel.hpp 
> 7de1e31d4c43ac9cbffab1e472ea51140719f900 
>   src/linux/routing/queueing/fq_codel.cpp 
> 4dc2a9d2ed52937f0a78a083980db488c06b45a7 
>   src/linux/routing/queueing/ingress.hpp 
> 84506fecd01522471a7998176c28bea8f1367aed 
>   src/linux/routing/queueing/ingress.cpp 
> ae0c38d2215e7fabcc1060e7385484bd455e1699 
>   src/linux/routing/queueing/internal.hpp 
> d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70 
> 
> Diff: https://reviews.apache.org/r/34956/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 34962: Moved NoopResourceEstimator to a separate file.

[Jie Yu]

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

Review request for mesos, Bartek Plotka, Niklas Nielsen, and Vinod Kone.


Repository: mesos


Description
---

Moved NoopResourceEstimator to a separate file.

As we're going to introduce more resource estimators in the code base, it makes 
sense to put them into a directory.


Diffs
-

  src/Makefile.am 081185bb28fa58c8fe0b1ffa8db6599def56e85b 
  src/examples/test_resource_estimator_module.cpp 
df85978f2beeccd0cccd893f13a8a0eeacd15e23 
  src/local/local.cpp 84f73e21e66e7a654b6b09f6a57a70a1ccc34b40 
  src/slave/main.cpp e3a45f4dfa0c8cc5d28d795f42bdc8cd14b7b10d 
  src/slave/resource_estimator.hpp 717804d65072a55e6c2bead14ab17be7fec57077 
  src/slave/resource_estimator.cpp a67640c3878c5707b8ef39c0750711c619680a39 
  src/slave/resource_estimator/noop.cpp PRE-CREATION 
  src/tests/cluster.hpp 7370c77de68d3a13340a69f6e794d97306d4bbb6 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 34956: Refactored the queueing discipline data structure.

[Jie Yu]

> On June 2, 2015, 10:40 p.m., Paul Brett wrote:
> > src/linux/routing/queueing/discipline.hpp, line 39
> > 
> >
> > Should config be optional?

We still need a Config struct for each type of qdisc so that the template 
specification could work. It just looks weired to me to do
```
Discipline(
  ...
  None());
```

Do you have a strong opinion on that?


- Jie


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


On June 2, 2015, 10:04 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34956/
> ---
> 
> (Updated June 2, 2015, 10:04 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored the queueing discipline data structure.
> 
> This is for us to more easily extend qdisc support.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am f2110a360b6e2a1e99e1d9630d5dfacfd818530a 
>   src/linux/routing/queueing/discipline.hpp PRE-CREATION 
>   src/linux/routing/queueing/fq_codel.hpp 
> 7de1e31d4c43ac9cbffab1e472ea51140719f900 
>   src/linux/routing/queueing/fq_codel.cpp 
> 4dc2a9d2ed52937f0a78a083980db488c06b45a7 
>   src/linux/routing/queueing/ingress.hpp 
> 84506fecd01522471a7998176c28bea8f1367aed 
>   src/linux/routing/queueing/ingress.cpp 
> ae0c38d2215e7fabcc1060e7385484bd455e1699 
>   src/linux/routing/queueing/internal.hpp 
> d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70 
> 
> Diff: https://reviews.apache.org/r/34956/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 34958: Moved ingress root handle to a proper location.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [34956, 34957, 34958]

All tests passed.

- Mesos ReviewBot


On June 2, 2015, 10:06 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34958/
> ---
> 
> (Updated June 2, 2015, 10:06 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved ingress root handle to a proper location.
> 
> This is because ingress root is not qdisc specific.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/handle.hpp aed810ed97f29fb79d13feb6fa72ce4360e8623b 
>   src/linux/routing/queueing/ingress.hpp 
> 84506fecd01522471a7998176c28bea8f1367aed 
>   src/linux/routing/queueing/ingress.cpp 
> ae0c38d2215e7fabcc1060e7385484bd455e1699 
> 
> Diff: https://reviews.apache.org/r/34958/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 34958: Moved ingress root handle to a proper location.

[Paul Brett]

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

Ship it!


Ship It!

- Paul Brett


On June 2, 2015, 10:06 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34958/
> ---
> 
> (Updated June 2, 2015, 10:06 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved ingress root handle to a proper location.
> 
> This is because ingress root is not qdisc specific.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/handle.hpp aed810ed97f29fb79d13feb6fa72ce4360e8623b 
>   src/linux/routing/queueing/ingress.hpp 
> 84506fecd01522471a7998176c28bea8f1367aed 
>   src/linux/routing/queueing/ingress.cpp 
> ae0c38d2215e7fabcc1060e7385484bd455e1699 
> 
> Diff: https://reviews.apache.org/r/34958/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 34957: Fixed a bug in qdisc search function.

[Jie Yu]

> On June 2, 2015, 11:09 p.m., Paul Brett wrote:
> > src/linux/routing/queueing/internal.hpp, line 143
> > 
> >
> > Not your problem, but we really should either explain why we have to 
> > increment the reference counter here but nowhere else or just fix Netlink 
> > to hide this (it does hide most of the cleanup logic)

Yeah, we should definitely add some comments. Can you sugguest how to fix 
Netlink to hide this?


- Jie


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


On June 2, 2015, 10:05 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34957/
> ---
> 
> (Updated June 2, 2015, 10:05 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a bug in qdisc search function.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/queueing/internal.hpp 
> d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70 
> 
> Diff: https://reviews.apache.org/r/34957/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 34957: Fixed a bug in qdisc search function.

[Paul Brett]

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

Ship it!


Ship It!

- Paul Brett


On June 2, 2015, 10:05 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34957/
> ---
> 
> (Updated June 2, 2015, 10:05 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a bug in qdisc search function.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/queueing/internal.hpp 
> d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70 
> 
> Diff: https://reviews.apache.org/r/34957/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 34957: Fixed a bug in qdisc search function.

[Paul Brett]

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



src/linux/routing/queueing/internal.hpp


Not your problem, but we really should either explain why we have to 
increment the reference counter here but nowhere else or just fix Netlink to 
hide this (it does hide most of the cleanup logic)


- Paul Brett


On June 2, 2015, 10:05 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34957/
> ---
> 
> (Updated June 2, 2015, 10:05 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a bug in qdisc search function.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/queueing/internal.hpp 
> d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70 
> 
> Diff: https://reviews.apache.org/r/34957/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 34724: Doxygen enhancements

[Niklas Nielsen]

> On May 27, 2015, 4:24 p.m., Marco Massenzio wrote:
> > Doxyfile, line 833
> > 
> >
> > Apache Mesos Docs, maybe?

Great point! Will change that before committing.


- Niklas


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


On May 27, 2015, 11:38 a.m., Niklas Nielsen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34724/
> ---
> 
> (Updated May 27, 2015, 11:38 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Joerg Schad, and Marco Massenzio.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removes latex generation, enables class index and docset's (for use in, for 
> example Dash)
> 
> 
> Diffs
> -
> 
>   Doxyfile 8bba46152f59478bbd5a4573eab85ec9628316bf 
> 
> Diff: https://reviews.apache.org/r/34724/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> class index
>   
> https://reviews.apache.org/media/uploaded/files/2015/05/27/97de9c4f-dba9-4fc0-bec2-7c2a918a341b__Screen_Shot_2015-05-27_at_11.38.12_AM.png
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>

Re: Review Request 34957: Fixed a bug in qdisc search function.

[Cong Wang]

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

Ship it!


This bug was found by Paul, just FYI.

- Cong Wang


On June 2, 2015, 10:05 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34957/
> ---
> 
> (Updated June 2, 2015, 10:05 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a bug in qdisc search function.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/queueing/internal.hpp 
> d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70 
> 
> Diff: https://reviews.apache.org/r/34957/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 34956: Refactored the queueing discipline data structure.

[Paul Brett]

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



src/linux/routing/queueing/discipline.hpp


Should config be optional?


- Paul Brett


On June 2, 2015, 10:04 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34956/
> ---
> 
> (Updated June 2, 2015, 10:04 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored the queueing discipline data structure.
> 
> This is for us to more easily extend qdisc support.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am f2110a360b6e2a1e99e1d9630d5dfacfd818530a 
>   src/linux/routing/queueing/discipline.hpp PRE-CREATION 
>   src/linux/routing/queueing/fq_codel.hpp 
> 7de1e31d4c43ac9cbffab1e472ea51140719f900 
>   src/linux/routing/queueing/fq_codel.cpp 
> 4dc2a9d2ed52937f0a78a083980db488c06b45a7 
>   src/linux/routing/queueing/ingress.hpp 
> 84506fecd01522471a7998176c28bea8f1367aed 
>   src/linux/routing/queueing/ingress.cpp 
> ae0c38d2215e7fabcc1060e7385484bd455e1699 
>   src/linux/routing/queueing/internal.hpp 
> d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70 
> 
> Diff: https://reviews.apache.org/r/34956/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 34687: (De)Serializing MasterInfo PB to JSON

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [34687]

All tests passed.

- Mesos ReviewBot


On June 2, 2015, 9:52 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34687/
> ---
> 
> (Updated June 2, 2015, 9:52 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2340
> https://issues.apache.org/jira/browse/MESOS-2340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-2340
> 
> This is a preliminary step to enabling JSON API
> for Master Discovery via Zookeeper.
> 
> We currently save the MasterInfo PB to ZK
> serializing directly the binary data, so that
> for clients to retrieve that information, they
> need to either link up with libmesos or
> obtain the PB definition (in mesos/mesos.proto).
> 
> This change only provides the (de)serialization
> utility methods and associated tests.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am f2110a360b6e2a1e99e1d9630d5dfacfd818530a 
>   src/common/http.hpp afce7fea334c7bfa57efc48c413bf906a2ebde32 
>   src/common/http.cpp 2ac7fba7a3aac913540f1b09768777393b79284a 
>   src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 
>   src/tests/common/http_tests.cpp f087b2313a13c3199b70b3d7feb728e1449a52e7 
>   src/tests/common/parse_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34687/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On June 2, 2015, 10:06 p.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34748/
> ---
> 
> (Updated June 2, 2015, 10:06 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, Niklas 
> Nielsen, Szymon Konefal, and Vinod Kone.
> 
> 
> Bugs: MESOS-2772
> https://issues.apache.org/jira/browse/MESOS-2772
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed ResourceMonitor to use ResourceUsage instead of 
> ResourceMonitor::Usage.
> Reused old ResoureUsage in message mesos.proto
> 
> NOTE: That is required for modules which need to fetch ResourceUsage e.g 
> ResourceEstimator and QoSController.
> Discussed that message ResourceUsage in mesos.proto is free to reuse and 
> modify, since it's not used anywhere else.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5cf81e20bc77288eb4c183722c608827b5000cb4 
>   src/slave/monitor.hpp 48fe59faacf53f1015665b0867aefd08d5b29225 
>   src/slave/monitor.cpp a5a52b116ec38535dcc23e720e8a3a1fbc73762e 
> 
> Diff: https://reviews.apache.org/r/34748/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage

[Niklas Nielsen]

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

Ship it!


Ship It!

- Niklas Nielsen


On June 2, 2015, 3:06 p.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34748/
> ---
> 
> (Updated June 2, 2015, 3:06 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, Niklas 
> Nielsen, Szymon Konefal, and Vinod Kone.
> 
> 
> Bugs: MESOS-2772
> https://issues.apache.org/jira/browse/MESOS-2772
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed ResourceMonitor to use ResourceUsage instead of 
> ResourceMonitor::Usage.
> Reused old ResoureUsage in message mesos.proto
> 
> NOTE: That is required for modules which need to fetch ResourceUsage e.g 
> ResourceEstimator and QoSController.
> Discussed that message ResourceUsage in mesos.proto is free to reuse and 
> modify, since it's not used anywhere else.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5cf81e20bc77288eb4c183722c608827b5000cb4 
>   src/slave/monitor.hpp 48fe59faacf53f1015665b0867aefd08d5b29225 
>   src/slave/monitor.cpp a5a52b116ec38535dcc23e720e8a3a1fbc73762e 
> 
> Diff: https://reviews.apache.org/r/34748/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Review Request 34958: Moved ingress root handle to a proper location.

[Jie Yu]

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

Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.


Repository: mesos


Description
---

Moved ingress root handle to a proper location.

This is because ingress root is not qdisc specific.


Diffs
-

  src/linux/routing/handle.hpp aed810ed97f29fb79d13feb6fa72ce4360e8623b 
  src/linux/routing/queueing/ingress.hpp 
84506fecd01522471a7998176c28bea8f1367aed 
  src/linux/routing/queueing/ingress.cpp 
ae0c38d2215e7fabcc1060e7385484bd455e1699 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage

[Bartek Plotka]

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

(Updated June 2, 2015, 10:06 p.m.)


Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, Niklas 
Nielsen, Szymon Konefal, and Vinod Kone.


Summary (updated)
-

Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage


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


Repository: mesos


Description
---

Changed ResourceMonitor to use ResourceUsage instead of ResourceMonitor::Usage.
Reused old ResoureUsage in message mesos.proto

NOTE: That is required for modules which need to fetch ResourceUsage e.g 
ResourceEstimator and QoSController.
Discussed that message ResourceUsage in mesos.proto is free to reuse and 
modify, since it's not used anywhere else.


Diffs (updated)
-

  include/mesos/mesos.proto 5cf81e20bc77288eb4c183722c608827b5000cb4 
  src/slave/monitor.hpp 48fe59faacf53f1015665b0867aefd08d5b29225 
  src/slave/monitor.cpp a5a52b116ec38535dcc23e720e8a3a1fbc73762e 

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


Testing
---

make check


Thanks,

Bartek Plotka

Review Request 34957: Fixed a bug in qdisc search function.

[Jie Yu]

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

Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.


Repository: mesos


Description
---

Fixed a bug in qdisc search function.


Diffs
-

  src/linux/routing/queueing/internal.hpp 
d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Review Request 34956: Refactored the queueing discipline data structure.

[Jie Yu]

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

Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.


Repository: mesos


Description
---

Refactored the queueing discipline data structure.

This is for us to more easily extend qdisc support.


Diffs
-

  src/Makefile.am f2110a360b6e2a1e99e1d9630d5dfacfd818530a 
  src/linux/routing/queueing/discipline.hpp PRE-CREATION 
  src/linux/routing/queueing/fq_codel.hpp 
7de1e31d4c43ac9cbffab1e472ea51140719f900 
  src/linux/routing/queueing/fq_codel.cpp 
4dc2a9d2ed52937f0a78a083980db488c06b45a7 
  src/linux/routing/queueing/ingress.hpp 
84506fecd01522471a7998176c28bea8f1367aed 
  src/linux/routing/queueing/ingress.cpp 
ae0c38d2215e7fabcc1060e7385484bd455e1699 
  src/linux/routing/queueing/internal.hpp 
d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 34687: (De)Serializing MasterInfo PB to JSON

[Marco Massenzio]

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

(Updated June 2, 2015, 9:52 p.m.)


Review request for mesos, haosdent huang and Niklas Nielsen.


Changes
---

BenH's comments


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


Repository: mesos


Description
---

Jira: MESOS-2340

This is a preliminary step to enabling JSON API
for Master Discovery via Zookeeper.

We currently save the MasterInfo PB to ZK
serializing directly the binary data, so that
for clients to retrieve that information, they
need to either link up with libmesos or
obtain the PB definition (in mesos/mesos.proto).

This change only provides the (de)serialization
utility methods and associated tests.


Diffs (updated)
-

  src/Makefile.am f2110a360b6e2a1e99e1d9630d5dfacfd818530a 
  src/common/http.hpp afce7fea334c7bfa57efc48c413bf906a2ebde32 
  src/common/http.cpp 2ac7fba7a3aac913540f1b09768777393b79284a 
  src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 
  src/tests/common/http_tests.cpp f087b2313a13c3199b70b3d7feb728e1449a52e7 
  src/tests/common/parse_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Marco Massenzio

Re: Review Request 34908: Rename --docker_sandbox_directory flag for general use.

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On June 1, 2015, 9:48 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34908/
> ---
> 
> (Updated June 1, 2015, 9:48 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rename --docker_sandbox_directory flag for general use.
> 
> This will require users to update configuration scripts etc if they specify 
> the old flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 4e20913cf7a088997235f399733c89db3441fb7c 
>   src/slave/containerizer/docker.cpp 41e0b98c387e57b692df4c56ae21ce70f67f9a19 
>   src/slave/flags.hpp 84dbb8a2aa963bb38cf0f610ab442b626179b173 
>   src/slave/flags.cpp ab8709861eb037e75dd2e91a097048ea6cd20c9c 
>   src/tests/docker_containerizer_tests.cpp 
> 3a983c6813dab6fa03ccb2c87e1ea71866766d6e 
> 
> Diff: https://reviews.apache.org/r/34908/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34135: Add filesystem/ isolators for persistent volumes.

[Timothy Chen]

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



src/slave/containerizer/isolators/filesystem/linux.cpp


actually I'm wrong, I was reading the old style guide. The newest style 
guide we do put a space, ignore my earler comment.


- Timothy Chen


On May 13, 2015, 12:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34135/
> ---
> 
> (Updated May 13, 2015, 12:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved code from Mesos Containerizer to filesystem isolators
>  - filesystem/posix (symlinks, doesn't support container rootfs)
>  - filesystem/linux (bind mounts, does support container rootfs)
> 
> The filesystem/posix isolator will be automatically included if no 
> filesystem/ isolator is specified.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 
>   src/slave/containerizer/isolators/filesystem/linux.hpp PRE-CREATION 
>   src/slave/containerizer/isolators/filesystem/linux.cpp PRE-CREATION 
>   src/slave/containerizer/isolators/filesystem/posix.hpp PRE-CREATION 
>   src/slave/containerizer/isolators/filesystem/posix.cpp PRE-CREATION 
>   src/slave/containerizer/linux_launcher.cpp 
> b9e22e3c70bed0c29e2ca8632411789d33f779a8 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b644b9c74bc23cf78c0a53284544be6cdaef2f8a 
> 
> Diff: https://reviews.apache.org/r/34135/diff/
> 
> 
> Testing
> ---
> 
> existing persistent volumes tests.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34687: (De)Serializing MasterInfo PB to JSON

[Marco Massenzio]

> On June 2, 2015, 9:10 a.m., Benjamin Hindman wrote:
> > src/common/http.cpp, lines 212-213
> > 
> >
> > Please see formatting for braces: 
> > https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Braced_Initializer_List_Format
> > 
> > tl;dr; Use braced-initializer list construction like a constructor.
> > 
> > JSON::Protobuf protobuf{masterInfo};
> > 
> > Please fix throughout this review.
> > 
> > But this brings me to a second question, why use braced-initializer 
> > list here at all when you're just calling a normal constructor (i.e., not a 
> > constructor that takes an std::initializer_list or an implicit constructor 
> > that just sets the fields of the object).
> > 
> > Finally, this can be simplified:
> > 
> > JSON::Object json(JSON::Protobuf(masterInfo));
> 
> Marco Massenzio wrote:
> sorry - this was a remnant from some trial code I'd run when playing 
> around with JSON::Protobuf (just to see what came out).
> Fixed.
> 
> I'm hoping I can bring cpplint to detect all this stuff.

sorry to break your heart, mate - but your code causes the dreaded "most vexing 
parse"... other variants fail too.
I'm sure we can come with a clever one-liner constructor, but in the spirit of 
the C++ Committee 
(guidelines](https://isocpp.org/std/library-design-guidelines) of "don't be 
clever", would you object to a (readable):
```
JSON::Protobuf protobuf(masterInfo);
JSON::Object json(protobuf);
```
not too "clever", but not terribly "stupid" either :)


- Marco


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


On May 31, 2015, 2:58 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34687/
> ---
> 
> (Updated May 31, 2015, 2:58 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2340
> https://issues.apache.org/jira/browse/MESOS-2340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-2340
> 
> This is a preliminary step to enabling JSON API
> for Master Discovery via Zookeeper.
> 
> We currently save the MasterInfo PB to ZK
> serializing directly the binary data, so that
> for clients to retrieve that information, they
> need to either link up with libmesos or
> obtain the PB definition (in mesos/mesos.proto).
> 
> This change only provides the (de)serialization
> utility methods and associated tests.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e7281ac6667562a27b0427b0ba7f41de98702928 
>   src/common/http.hpp afce7fea334c7bfa57efc48c413bf906a2ebde32 
>   src/common/http.cpp 2ac7fba7a3aac913540f1b09768777393b79284a 
>   src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 
>   src/tests/common/http_tests.cpp f087b2313a13c3199b70b3d7feb728e1449a52e7 
>   src/tests/common/parse_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34687/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 34816: Changed TestResourceEstimator to be mocked.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On June 2, 2015, 9:19 p.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34816/
> ---
> 
> (Updated June 2, 2015, 9:19 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> TestResourceEstimator changed to be a  MockResourceEstimator with mocked 
> methods - to inject resources and test plumbing between RE and Slave.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp ac986a0687a576a0bd5693c82b3c7d543aaa956b 
>   src/tests/oversubscription_tests.cpp 
> ea5857cb579aa904fd05530684bdde51a0b3f27f 
> 
> Diff: https://reviews.apache.org/r/34816/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 34687: (De)Serializing MasterInfo PB to JSON

[Marco Massenzio]

> On June 2, 2015, 9:10 a.m., Benjamin Hindman wrote:
> > I haven't done a full review here but took interest in this because the 
> > required 'ip' field of the MasterInfo protobuf (as well as other protobufs) 
> > is deprecated because it doesn't support IPv6. We need to make 'ip' an 
> > optional field and replace it by an 'address' field which includes both the 
> > 'hostname' or 'ip' and the port that can be parsed by our Address. Once we 
> > do that, then we shoudn't need any special JSON conversion or parsing.

Well, according to Protobuf Law, you can't: `required` is like a diamond: it's 
for life! ;)

But I'm all for it - if you think this is safe to do over a couple of releases?

I had already created https://issues.apache.org/jira/browse/MESOS-2736 - please 
add your suggestions/comments there.

Once we finalize that, I think patching this code to make it work with the new 
design should be trivial.


> On June 2, 2015, 9:10 a.m., Benjamin Hindman wrote:
> > src/common/http.hpp, line 52
> > 
> >
> > Please end sentences with a period.

Just so you know, in my spare time I'm submitting patches to cpplint.py :-)
(thanks for catching it!)


> On June 2, 2015, 9:10 a.m., Benjamin Hindman wrote:
> > src/common/http.cpp, lines 212-213
> > 
> >
> > Please see formatting for braces: 
> > https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Braced_Initializer_List_Format
> > 
> > tl;dr; Use braced-initializer list construction like a constructor.
> > 
> > JSON::Protobuf protobuf{masterInfo};
> > 
> > Please fix throughout this review.
> > 
> > But this brings me to a second question, why use braced-initializer 
> > list here at all when you're just calling a normal constructor (i.e., not a 
> > constructor that takes an std::initializer_list or an implicit constructor 
> > that just sets the fields of the object).
> > 
> > Finally, this can be simplified:
> > 
> > JSON::Object json(JSON::Protobuf(masterInfo));

sorry - this was a remnant from some trial code I'd run when playing around 
with JSON::Protobuf (just to see what came out).
Fixed.

I'm hoping I can bring cpplint to detect all this stuff.


> On June 2, 2015, 9:10 a.m., Benjamin Hindman wrote:
> > src/common/http.cpp, line 219
> > 
> >
> > This seems rather aggressive, while I understand that 'ip' is required, 
> > that doesn't mean that someone might not pass an incomplete protobuf where 
> > this could crash hard instead of just returning an Error.

Well, I'd asked around whether there was a method to check both !error && !none 
and CHECK_SOME() was what had come up.
My bad.


> On June 2, 2015, 9:10 a.m., Benjamin Hindman wrote:
> > src/common/http.cpp, lines 220-225
> > 
> >
> > Why the extra temporary? Doing the compilers job Marco? ;-)
> > 
> > net::IP ip(ntohl(ipAsInt.get().value));

old habit - when I see gnarly code, my brain immediately projects an image of 
me doing debugging :)
hey, compared to a manager's job, a compiler's is a piece of cake!


- Marco


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


On May 31, 2015, 2:58 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34687/
> ---
> 
> (Updated May 31, 2015, 2:58 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2340
> https://issues.apache.org/jira/browse/MESOS-2340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-2340
> 
> This is a preliminary step to enabling JSON API
> for Master Discovery via Zookeeper.
> 
> We currently save the MasterInfo PB to ZK
> serializing directly the binary data, so that
> for clients to retrieve that information, they
> need to either link up with libmesos or
> obtain the PB definition (in mesos/mesos.proto).
> 
> This change only provides the (de)serialization
> utility methods and associated tests.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e7281ac6667562a27b0427b0ba7f41de98702928 
>   src/common/http.hpp afce7fea334c7bfa57efc48c413bf906a2ebde32 
>   src/common/http.cpp 2ac7fba7a3aac913540f1b09768777393b79284a 
>   src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 
>   src/tests/common/http_tests.cpp f087b2313a13c3199b70b3d7feb728e1449a52e7 
>   src/tests/common/parse_tests.cpp PRE-CREATION 
> 
> Diff: https://revie

Re: Review Request 34894: Add new message for Traffic Control statistics

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [34899, 34830, 34832, 34426]

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

Error:
 2015-06-02 21:24:30 URL:https://reviews.apache.org/r/34426/diff/raw/ 
[9373/9373] -> "34426.patch" [1]
error: patch failed: src/linux/routing/queueing/fq_codel.cpp:97
error: src/linux/routing/queueing/fq_codel.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On June 2, 2015, 8:52 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34894/
> ---
> 
> (Updated June 2, 2015, 8:52 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add new message for Traffic Control statistics
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5cf81e20bc77288eb4c183722c608827b5000cb4 
> 
> Diff: https://reviews.apache.org/r/34894/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 34816: Changed TestResourceEstimator to be mocked.

[Bartek Plotka]

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

(Updated June 2, 2015, 9:19 p.m.)


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


Changes
---

Dropped times(1) since it's implicitly done.


Repository: mesos


Description
---

TestResourceEstimator changed to be a  MockResourceEstimator with mocked 
methods - to inject resources and test plumbing between RE and Slave.


Diffs (updated)
-

  src/tests/mesos.hpp ac986a0687a576a0bd5693c82b3c7d543aaa956b 
  src/tests/oversubscription_tests.cpp ea5857cb579aa904fd05530684bdde51a0b3f27f 

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


Testing
---

make check


Thanks,

Bartek Plotka

Re: Review Request 34816: Changed TestResourceEstimator to be mocked.

[Bartek Plotka]

> On June 2, 2015, 9:08 p.m., Niklas Nielsen wrote:
> > src/tests/oversubscription_tests.cpp, line 91
> > 
> >
> > This is implicit and you can remove it :)
> > 
> > Here and below

oh, sure (:


- Bartek


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


On June 2, 2015, 8:36 p.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34816/
> ---
> 
> (Updated June 2, 2015, 8:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> TestResourceEstimator changed to be a  MockResourceEstimator with mocked 
> methods - to inject resources and test plumbing between RE and Slave.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp ac986a0687a576a0bd5693c82b3c7d543aaa956b 
>   src/tests/oversubscription_tests.cpp 
> ea5857cb579aa904fd05530684bdde51a0b3f27f 
> 
> Diff: https://reviews.apache.org/r/34816/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 34816: Changed TestResourceEstimator to be mocked.

[Niklas Nielsen]

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

Ship it!



src/tests/mesos.hpp


Any reason for WillByDefault(..)?

EXPECT_CALL(...)
  .WillRepeatedly(Return(...));

Should work, right?



src/tests/oversubscription_tests.cpp


This is implicit and you can remove it :)

Here and below


- Niklas Nielsen


On June 2, 2015, 1:36 p.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34816/
> ---
> 
> (Updated June 2, 2015, 1:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> TestResourceEstimator changed to be a  MockResourceEstimator with mocked 
> methods - to inject resources and test plumbing between RE and Slave.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp ac986a0687a576a0bd5693c82b3c7d543aaa956b 
>   src/tests/oversubscription_tests.cpp 
> ea5857cb579aa904fd05530684bdde51a0b3f27f 
> 
> Diff: https://reviews.apache.org/r/34816/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 34863: Add tests for new qdisc statistics functions.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [34899, 34830, 34832, 34426]

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

Error:
 2015-06-02 21:05:08 URL:https://reviews.apache.org/r/34426/diff/raw/ 
[9373/9373] -> "34426.patch" [1]
error: patch failed: src/linux/routing/queueing/fq_codel.cpp:97
error: src/linux/routing/queueing/fq_codel.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On June 2, 2015, 5:31 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34863/
> ---
> 
> (Updated June 2, 2015, 5:31 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Bugs: MESOS-2750
> https://issues.apache.org/jira/browse/MESOS-2750
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add tests for new qdisc statistics functions.
> 
> 
> Diffs
> -
> 
>   src/tests/routing_tests.cpp 2066a56571f43a012992b4294fbf5b6e07c2b159 
> 
> Diff: https://reviews.apache.org/r/34863/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

[Bartek Plotka]

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

(Updated June 2, 2015, 8:58 p.m.)


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


Changes
---

Jie Yu issues addressed.


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


Repository: mesos


Description
---

Added *ResourceEstimator* (RE) module interface. 
Added *TestResourceEstimator* example module.


Diffs (updated)
-

  include/mesos/module/resource_estimator.hpp PRE-CREATION 
  src/Makefile.am 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241 
  src/examples/test_resource_estimator_module.cpp PRE-CREATION 
  src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be 
  src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8 
  src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29 

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


Testing
---

make check


Thanks,

Bartek Plotka

Re: Review Request 34894: Add new message for Traffic Control statistics

[Paul Brett]

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

(Updated June 2, 2015, 8:52 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.


Changes
---

Switch to returning statistics as an array


Repository: mesos


Description
---

Add new message for Traffic Control statistics


Diffs (updated)
-

  include/mesos/mesos.proto 5cf81e20bc77288eb4c183722c608827b5000cb4 

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


Testing
---

make check


Thanks,

Paul Brett

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

[Bartek Plotka]

> On June 2, 2015, 5:07 p.m., Jie Yu wrote:
> > src/tests/module.hpp, lines 76-79
> > 
> >
> > Do we need this?

Nope, we deleted RE moduled test so in fact we don't - sorry.


- Bartek


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


On June 2, 2015, 12:40 a.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> ---
> 
> (Updated June 2, 2015, 12:40 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added *ResourceEstimator* (RE) module interface. 
> Added *TestResourceEstimator* example module.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/resource_estimator.hpp PRE-CREATION 
>   src/Makefile.am 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241 
>   src/examples/test_resource_estimator_module.cpp PRE-CREATION 
>   src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be 
>   src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8 
>   src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29 
> 
> Diff: https://reviews.apache.org/r/34662/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 34894: Add new message for Traffic Control statistics

[Paul Brett]

> On June 1, 2015, 6:32 p.m., Chi Zhang wrote:
> > include/mesos/mesos.proto, lines 543-545
> > 
> >
> > Maybe some comments for the three names? (kinda the reason you are not 
> > using 'repeated' of TrafficControlStatistics in ResourceStatistics?)

Having talk this over, we will convert this to a repeated structure with id 
fields which are defined in the port mapping isolator (disk volume reporting 
will need to use a similar structure).


- Paul


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


On June 1, 2015, 4:40 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34894/
> ---
> 
> (Updated June 1, 2015, 4:40 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add new message for Traffic Control statistics
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5cf81e20bc77288eb4c183722c608827b5000cb4 
> 
> Diff: https://reviews.apache.org/r/34894/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 34816: Changed TestResourceEstimator to be mocked.

[Bartek Plotka]

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

(Updated June 2, 2015, 8:36 p.m.)


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


Changes
---

issue addressed.


Repository: mesos


Description
---

TestResourceEstimator changed to be a  MockResourceEstimator with mocked 
methods - to inject resources and test plumbing between RE and Slave.


Diffs (updated)
-

  src/tests/mesos.hpp ac986a0687a576a0bd5693c82b3c7d543aaa956b 
  src/tests/oversubscription_tests.cpp ea5857cb579aa904fd05530684bdde51a0b3f27f 

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


Testing
---

make check


Thanks,

Bartek Plotka

Re: Review Request 34816: Changed TestResourceEstimator to be mocked.

[Bartek Plotka]

> On June 2, 2015, 8:05 p.m., Vinod Kone wrote:
> > src/tests/mesos.hpp, lines 716-723
> > 
> >
> > You want to use both ON_CALL and EXPECT_CALL. See TestAllocator in 
> > tests/mesos.hpp for the rationale.

+1

However, i can see quite a mess here in tests/mesos.hpp, because every other 
Test/Mock class (except TestAllocator) uses EXPECT_CALL and WillRepeatedly ):


> On June 2, 2015, 8:05 p.m., Vinod Kone wrote:
> > src/tests/oversubscription_tests.cpp, lines 90-91
> > 
> >
> > You don't need to explicitly do this you setup both ON_CALL and 
> > EXPECT_CALL above.

I think there is a need - i want to call initialize only ONCE.

Without this line there will be *WillRepeatedly* action when i setup ON_CALL 
and EXPECT_CALL above without any restriction to call count.
However - i can change that to:
```
 EXPECT_CALL(resourceEstimator, initialize())
.Times(1);
```
...to be more consistent. Is that more preferable?


- Bartek


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


On June 2, 2015, 7:37 p.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34816/
> ---
> 
> (Updated June 2, 2015, 7:37 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> TestResourceEstimator changed to be a  MockResourceEstimator with mocked 
> methods - to inject resources and test plumbing between RE and Slave.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp ac986a0687a576a0bd5693c82b3c7d543aaa956b 
>   src/tests/oversubscription_tests.cpp 
> ea5857cb579aa904fd05530684bdde51a0b3f27f 
> 
> Diff: https://reviews.apache.org/r/34816/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage. Pass ResourceUsage to ResourceEstimator

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [34816, 34662, 34748]

All tests passed.

- Mesos ReviewBot


On June 2, 2015, 8:16 p.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34748/
> ---
> 
> (Updated June 2, 2015, 8:16 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, Niklas 
> Nielsen, Szymon Konefal, and Vinod Kone.
> 
> 
> Bugs: MESOS-2772
> https://issues.apache.org/jira/browse/MESOS-2772
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed ResourceMonitor to use ResourceUsage instead of 
> ResourceMonitor::Usage.
> Reused old ResoureUsage in message mesos.proto
> 
> NOTE: That is required for modules which need to fetch ResourceUsage e.g 
> ResourceEstimator and QoSController.
> Discussed that message ResourceUsage in mesos.proto is free to reuse and 
> modify, since it's not used anywhere else.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto a66888916dc90e476c8bb20e67e3f6b08c47fb99 
>   src/slave/monitor.hpp 48fe59faacf53f1015665b0867aefd08d5b29225 
>   src/slave/monitor.cpp a5a52b116ec38535dcc23e720e8a3a1fbc73762e 
> 
> Diff: https://reviews.apache.org/r/34748/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage. Pass ResourceUsage to ResourceEstimator

[Bartek Plotka]

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

(Updated June 2, 2015, 8:16 p.m.)


Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, Niklas 
Nielsen, Szymon Konefal, and Vinod Kone.


Changes
---

In previous request - lost 2 files.


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


Repository: mesos


Description
---

Changed ResourceMonitor to use ResourceUsage instead of ResourceMonitor::Usage.
Reused old ResoureUsage in message mesos.proto

NOTE: That is required for modules which need to fetch ResourceUsage e.g 
ResourceEstimator and QoSController.
Discussed that message ResourceUsage in mesos.proto is free to reuse and 
modify, since it's not used anywhere else.


Diffs (updated)
-

  include/mesos/mesos.proto a66888916dc90e476c8bb20e67e3f6b08c47fb99 
  src/slave/monitor.hpp 48fe59faacf53f1015665b0867aefd08d5b29225 
  src/slave/monitor.cpp a5a52b116ec38535dcc23e720e8a3a1fbc73762e 

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


Testing
---

make check


Thanks,

Bartek Plotka

Re: Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage. Pass ResourceUsage to ResourceEstimator

[Bartek Plotka]

> On June 1, 2015, 9:40 p.m., Ben Mahler wrote:
> > include/mesos/mesos.proto, lines 577-580
> > 
> >
> > We've lost the important information (what does it mean for these to be 
> > unset?). Could you revert the comment changes here?
> 
> Bartek Plotka wrote:
> Do you mean the "Resource usage is for an executor" sentence or sth else?

Oh, i think you ment:
```
required SlaveID slave_id = 1;
required FrameworkID framework_id = 2;
```
So the SlaveID is not needed, because (in current implementation) ResourceUsage 
is for internal usage only. 
And FrameworkID is included in ExecutorInfo.

Does that make sense?


- Bartek


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


On June 2, 2015, 7:40 p.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34748/
> ---
> 
> (Updated June 2, 2015, 7:40 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, Niklas 
> Nielsen, Szymon Konefal, and Vinod Kone.
> 
> 
> Bugs: MESOS-2772
> https://issues.apache.org/jira/browse/MESOS-2772
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed ResourceMonitor to use ResourceUsage instead of 
> ResourceMonitor::Usage.
> Reused old ResoureUsage in message mesos.proto
> 
> NOTE: That is required for modules which need to fetch ResourceUsage e.g 
> ResourceEstimator and QoSController.
> Discussed that message ResourceUsage in mesos.proto is free to reuse and 
> modify, since it's not used anywhere else.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto a66888916dc90e476c8bb20e67e3f6b08c47fb99 
> 
> Diff: https://reviews.apache.org/r/34748/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage. Pass ResourceUsage to ResourceEstimator

[Bartek Plotka]

> On June 1, 2015, 9:40 p.m., Ben Mahler wrote:
> > include/mesos/mesos.proto, lines 577-580
> > 
> >
> > We've lost the important information (what does it mean for these to be 
> > unset?). Could you revert the comment changes here?

Do you mean the "Resource usage is for an executor" sentence or sth else?


- Bartek


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


On June 2, 2015, 7:40 p.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34748/
> ---
> 
> (Updated June 2, 2015, 7:40 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, Niklas 
> Nielsen, Szymon Konefal, and Vinod Kone.
> 
> 
> Bugs: MESOS-2772
> https://issues.apache.org/jira/browse/MESOS-2772
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed ResourceMonitor to use ResourceUsage instead of 
> ResourceMonitor::Usage.
> Reused old ResoureUsage in message mesos.proto
> 
> NOTE: That is required for modules which need to fetch ResourceUsage e.g 
> ResourceEstimator and QoSController.
> Discussed that message ResourceUsage in mesos.proto is free to reuse and 
> modify, since it's not used anywhere else.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto a66888916dc90e476c8bb20e67e3f6b08c47fb99 
> 
> Diff: https://reviews.apache.org/r/34748/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 34816: Changed TestResourceEstimator to be mocked.

[Vinod Kone]

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



src/tests/mesos.hpp


You want to use both ON_CALL and EXPECT_CALL. See TestAllocator in 
tests/mesos.hpp for the rationale.



src/tests/oversubscription_tests.cpp


You don't need to explicitly do this you setup both ON_CALL and EXPECT_CALL 
above.



src/tests/oversubscription_tests.cpp


Why is this *action* pulled up? What if the schedule receives an offer 
before #194 gets executed?

As a general rule, an action should come after an expectation is set and 
before the expectation is evaluated.


- Vinod Kone


On June 2, 2015, 7:37 p.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34816/
> ---
> 
> (Updated June 2, 2015, 7:37 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> TestResourceEstimator changed to be a  MockResourceEstimator with mocked 
> methods - to inject resources and test plumbing between RE and Slave.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp ac986a0687a576a0bd5693c82b3c7d543aaa956b 
>   src/tests/oversubscription_tests.cpp 
> ea5857cb579aa904fd05530684bdde51a0b3f27f 
> 
> Diff: https://reviews.apache.org/r/34816/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage. Pass ResourceUsage to ResourceEstimator

[Bartek Plotka]

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

(Updated June 2, 2015, 7:40 p.m.)


Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, Niklas 
Nielsen, Szymon Konefal, and Vinod Kone.


Changes
---

First part of issues addressed.


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


Repository: mesos


Description
---

Changed ResourceMonitor to use ResourceUsage instead of ResourceMonitor::Usage.
Reused old ResoureUsage in message mesos.proto

NOTE: That is required for modules which need to fetch ResourceUsage e.g 
ResourceEstimator and QoSController.
Discussed that message ResourceUsage in mesos.proto is free to reuse and 
modify, since it's not used anywhere else.


Diffs (updated)
-

  include/mesos/mesos.proto a66888916dc90e476c8bb20e67e3f6b08c47fb99 

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


Testing
---

make check


Thanks,

Bartek Plotka

Re: Review Request 34816: Changed TestResourceEstimator to be mocked.

[Bartek Plotka]

> On June 2, 2015, 5:05 p.m., Jie Yu wrote:
> > src/tests/oversubscription_tests.cpp, lines 77-80
> > 
> >
> > Any reason change the comments and the test name here?

Sorry - mistake during fixing git merge conflicts


- Bartek


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


On June 2, 2015, 7:37 p.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34816/
> ---
> 
> (Updated June 2, 2015, 7:37 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> TestResourceEstimator changed to be a  MockResourceEstimator with mocked 
> methods - to inject resources and test plumbing between RE and Slave.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp ac986a0687a576a0bd5693c82b3c7d543aaa956b 
>   src/tests/oversubscription_tests.cpp 
> ea5857cb579aa904fd05530684bdde51a0b3f27f 
> 
> Diff: https://reviews.apache.org/r/34816/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 34816: Changed TestResourceEstimator to be mocked.

[Bartek Plotka]

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

(Updated June 2, 2015, 7:37 p.m.)


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


Changes
---

All issues addressed.


Repository: mesos


Description
---

TestResourceEstimator changed to be a  MockResourceEstimator with mocked 
methods - to inject resources and test plumbing between RE and Slave.


Diffs (updated)
-

  src/tests/mesos.hpp ac986a0687a576a0bd5693c82b3c7d543aaa956b 
  src/tests/oversubscription_tests.cpp ea5857cb579aa904fd05530684bdde51a0b3f27f 

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


Testing
---

make check


Thanks,

Bartek Plotka

Re: Review Request 34816: Changed TestResourceEstimator to be mocked.

[Bartek Plotka]

> On June 2, 2015, 4:24 p.m., Niklas Nielsen wrote:
> > src/tests/mesos.hpp, line 797
> > 
> >
> > How will this work, when we can't access the mocked resource estimator 
> > from the test body?

You were right - default MockResourceEstimator behaviour needed. Fixed that.


> On June 2, 2015, 4:24 p.m., Niklas Nielsen wrote:
> > src/tests/oversubscription_tests.cpp, line 93
> > 
> >
> > Shouldn't we have 'using process::Queue;' somewhere above?

Actually - we have.


> On June 2, 2015, 4:24 p.m., Niklas Nielsen wrote:
> > src/tests/oversubscription_tests.cpp, line 267
> > 
> >
> > Why did you change the cpu counts?

I thought - it would more intersting test case - to shrink slack resources. I 
reverted that (:


- Bartek


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


On June 2, 2015, 7:37 p.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34816/
> ---
> 
> (Updated June 2, 2015, 7:37 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> TestResourceEstimator changed to be a  MockResourceEstimator with mocked 
> methods - to inject resources and test plumbing between RE and Slave.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp ac986a0687a576a0bd5693c82b3c7d543aaa956b 
>   src/tests/oversubscription_tests.cpp 
> ea5857cb579aa904fd05530684bdde51a0b3f27f 
> 
> Diff: https://reviews.apache.org/r/34816/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

[Niklas Nielsen]

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

Ship it!


Modulo Jie's comments

- Niklas Nielsen


On June 1, 2015, 5:40 p.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> ---
> 
> (Updated June 1, 2015, 5:40 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added *ResourceEstimator* (RE) module interface. 
> Added *TestResourceEstimator* example module.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/resource_estimator.hpp PRE-CREATION 
>   src/Makefile.am 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241 
>   src/examples/test_resource_estimator_module.cpp PRE-CREATION 
>   src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be 
>   src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8 
>   src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29 
> 
> Diff: https://reviews.apache.org/r/34662/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 34195: Refactoring to use FlagsBase common functionality

[Marco Massenzio]

> On June 2, 2015, 5:52 p.m., Ben Mahler wrote:
> > I'm seeing the following pattern:
> > 
> > ```
> > if (flags.master.isNone()) {
> >   cerr << flags.usage("Missing required option --master") << endl;
> >   return EXIT_FAILURE;
> > }
> > ```
> > 
> > Why not leverage EXIT here?
> > 
> > ```
> > if (flags.master.isNone()) {
> >   EXIT(EXIT_FAILURE) << flags.usage("Missing required option --master");
> > }
> > ```
> > 
> > I'm assuming there's probably also a ticket or TODO somewhere to be able to 
> > declare when an argument is required to avoid these manual checks entirely. 
> > :)
> 
> Ben Mahler wrote:
> Specifically, it seems like EXIT_FAILURE/EXIT_SUCCESS are documented as 
> arguments for exit(), so returning these from main() seems a little less 
> intuitive than passing them into EXIT to me.

Yes, please see: [MESOS-2766](https://issues.apache.org/jira/browse/MESOS-2766)
Regarding the different behavior, I tried to upset the least possible existing 
code.

I completely agree with you, we should (possibly in the style guide? 
elsewhere?) formalize:

1. the use of `EXIT_{FAILURE,SUCCESS}` as return codes from `main()`
2. the use of `EXIT(code)` (or `return code`) as the expected behavior


- Marco


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


On May 29, 2015, 1:10 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34195/
> ---
> 
> (Updated May 29, 2015, 1:10 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2711
> https://issues.apache.org/jira/browse/MESOS-2711
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-2711
> 
> All the main() methods have been refactored to use the
> definition of FlagsBase::help flag and FlagsBase::usage().
> 
> This CL also tries to bring some uniformity to the
> use of exit codes: if this is deemed to be worth
> making it uniform, we can come up with common
> rules and extend the changes here to be compliant.
> 
> This touches a lot of files, but keep scrolling, and you will see a pattern 
> emerge.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp dbd19e67f56a150f54180ad13e6402842eb68e17 
>   src/cli/mesos.cpp 171a707cd2ba2348898e7fbe8fe9f0634edd6d86 
>   src/cli/resolve.cpp a99b6094dffc9f7aa44bcf63ad40121e1abb120b 
>   src/docker/executor.cpp 075c6b5b787532bb1a85018b966aa7e2a1add4f5 
>   src/examples/load_generator_framework.cpp 
> be1a3bf5f16bd811cb4039c8f15478183712a426 
>   src/examples/low_level_scheduler_libprocess.cpp 
> bee2e7ef8432cc42733260c668f1100c68f73b8d 
>   src/examples/low_level_scheduler_pthread.cpp 
> fb8cd66c2e94270971184c1e3dcc92eccab8b223 
>   src/examples/persistent_volume_framework.cpp 
> 8a893fcfa3d5d988a88fdeaf0bfc08e0a49b7a65 
>   src/health-check/main.cpp a4ce742ab8deff1ebd99359112670493fdaeeac3 
>   src/launcher/executor.cpp de6f1b104a765a8e53934154e78872b03695b24c 
>   src/local/main.cpp a641b9e83862743890597a2981a9419517e7c589 
>   src/log/tool/benchmark.hpp e0109e20d354a6545ab10750e5d2c3e800f7a6a8 
>   src/log/tool/benchmark.cpp 01e55115f35d155efbea190b5308b294ba76e7cb 
>   src/log/tool/initialize.hpp 10ac269627052d89e1936e891127486d88c74253 
>   src/log/tool/initialize.cpp ccda7fb1c0f7113f865ec61adee76b2ea6180442 
>   src/log/tool/read.hpp 9a6971bca7efc1ddd45efaa44733c1a474804da0 
>   src/log/tool/read.cpp d14138502f5bc9a725deb83da505765865da017f 
>   src/log/tool/replica.cpp 3985fc7df4f7153ae623589fbdd769ccbae57125 
>   src/master/main.cpp d5666bc8ee8d7a0f0b8685f76d65dd1f9ac2a280 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> 6579286b779882bec493a8e0f10486adc316dc6e 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 843e52d6f9923d0ee0a0297cd5c464b8b72f5de3 
>   src/slave/main.cpp f762f5b06be74c391cbc336b2da28f8358952ba4 
>   src/tests/main.cpp e3fff5d60c0468c0d258f2bb301efc1309c071b0 
>   src/usage/main.cpp 97f55e938dc7678f8331970d8953d09218f70902 
> 
> Diff: https://reviews.apache.org/r/34195/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> **NOTE** this fixes completely the chained changes from 34193 and makes all 
> the tests pass.
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 34195: Refactoring to use FlagsBase common functionality

[Ben Mahler]

> On June 2, 2015, 5:52 p.m., Ben Mahler wrote:
> > I'm seeing the following pattern:
> > 
> > ```
> > if (flags.master.isNone()) {
> >   cerr << flags.usage("Missing required option --master") << endl;
> >   return EXIT_FAILURE;
> > }
> > ```
> > 
> > Why not leverage EXIT here?
> > 
> > ```
> > if (flags.master.isNone()) {
> >   EXIT(EXIT_FAILURE) << flags.usage("Missing required option --master");
> > }
> > ```
> > 
> > I'm assuming there's probably also a ticket or TODO somewhere to be able to 
> > declare when an argument is required to avoid these manual checks entirely. 
> > :)

Specifically, it seems like EXIT_FAILURE/EXIT_SUCCESS are documented as 
arguments for exit(), so returning these from main() seems a little less 
intuitive than passing them into EXIT to me.


- Ben


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


On May 29, 2015, 1:10 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34195/
> ---
> 
> (Updated May 29, 2015, 1:10 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2711
> https://issues.apache.org/jira/browse/MESOS-2711
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-2711
> 
> All the main() methods have been refactored to use the
> definition of FlagsBase::help flag and FlagsBase::usage().
> 
> This CL also tries to bring some uniformity to the
> use of exit codes: if this is deemed to be worth
> making it uniform, we can come up with common
> rules and extend the changes here to be compliant.
> 
> This touches a lot of files, but keep scrolling, and you will see a pattern 
> emerge.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp dbd19e67f56a150f54180ad13e6402842eb68e17 
>   src/cli/mesos.cpp 171a707cd2ba2348898e7fbe8fe9f0634edd6d86 
>   src/cli/resolve.cpp a99b6094dffc9f7aa44bcf63ad40121e1abb120b 
>   src/docker/executor.cpp 075c6b5b787532bb1a85018b966aa7e2a1add4f5 
>   src/examples/load_generator_framework.cpp 
> be1a3bf5f16bd811cb4039c8f15478183712a426 
>   src/examples/low_level_scheduler_libprocess.cpp 
> bee2e7ef8432cc42733260c668f1100c68f73b8d 
>   src/examples/low_level_scheduler_pthread.cpp 
> fb8cd66c2e94270971184c1e3dcc92eccab8b223 
>   src/examples/persistent_volume_framework.cpp 
> 8a893fcfa3d5d988a88fdeaf0bfc08e0a49b7a65 
>   src/health-check/main.cpp a4ce742ab8deff1ebd99359112670493fdaeeac3 
>   src/launcher/executor.cpp de6f1b104a765a8e53934154e78872b03695b24c 
>   src/local/main.cpp a641b9e83862743890597a2981a9419517e7c589 
>   src/log/tool/benchmark.hpp e0109e20d354a6545ab10750e5d2c3e800f7a6a8 
>   src/log/tool/benchmark.cpp 01e55115f35d155efbea190b5308b294ba76e7cb 
>   src/log/tool/initialize.hpp 10ac269627052d89e1936e891127486d88c74253 
>   src/log/tool/initialize.cpp ccda7fb1c0f7113f865ec61adee76b2ea6180442 
>   src/log/tool/read.hpp 9a6971bca7efc1ddd45efaa44733c1a474804da0 
>   src/log/tool/read.cpp d14138502f5bc9a725deb83da505765865da017f 
>   src/log/tool/replica.cpp 3985fc7df4f7153ae623589fbdd769ccbae57125 
>   src/master/main.cpp d5666bc8ee8d7a0f0b8685f76d65dd1f9ac2a280 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> 6579286b779882bec493a8e0f10486adc316dc6e 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 843e52d6f9923d0ee0a0297cd5c464b8b72f5de3 
>   src/slave/main.cpp f762f5b06be74c391cbc336b2da28f8358952ba4 
>   src/tests/main.cpp e3fff5d60c0468c0d258f2bb301efc1309c071b0 
>   src/usage/main.cpp 97f55e938dc7678f8331970d8953d09218f70902 
> 
> Diff: https://reviews.apache.org/r/34195/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> **NOTE** this fixes completely the chained changes from 34193 and makes all 
> the tests pass.
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 34195: Refactoring to use FlagsBase common functionality

[Ben Mahler]

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


I'm seeing the following pattern:

```
if (flags.master.isNone()) {
  cerr << flags.usage("Missing required option --master") << endl;
  return EXIT_FAILURE;
}
```

Why not leverage EXIT here?

```
if (flags.master.isNone()) {
  EXIT(EXIT_FAILURE) << flags.usage("Missing required option --master");
}
```

I'm assuming there's probably also a ticket or TODO somewhere to be able to 
declare when an argument is required to avoid these manual checks entirely. :)

- Ben Mahler


On May 29, 2015, 1:10 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34195/
> ---
> 
> (Updated May 29, 2015, 1:10 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2711
> https://issues.apache.org/jira/browse/MESOS-2711
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-2711
> 
> All the main() methods have been refactored to use the
> definition of FlagsBase::help flag and FlagsBase::usage().
> 
> This CL also tries to bring some uniformity to the
> use of exit codes: if this is deemed to be worth
> making it uniform, we can come up with common
> rules and extend the changes here to be compliant.
> 
> This touches a lot of files, but keep scrolling, and you will see a pattern 
> emerge.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp dbd19e67f56a150f54180ad13e6402842eb68e17 
>   src/cli/mesos.cpp 171a707cd2ba2348898e7fbe8fe9f0634edd6d86 
>   src/cli/resolve.cpp a99b6094dffc9f7aa44bcf63ad40121e1abb120b 
>   src/docker/executor.cpp 075c6b5b787532bb1a85018b966aa7e2a1add4f5 
>   src/examples/load_generator_framework.cpp 
> be1a3bf5f16bd811cb4039c8f15478183712a426 
>   src/examples/low_level_scheduler_libprocess.cpp 
> bee2e7ef8432cc42733260c668f1100c68f73b8d 
>   src/examples/low_level_scheduler_pthread.cpp 
> fb8cd66c2e94270971184c1e3dcc92eccab8b223 
>   src/examples/persistent_volume_framework.cpp 
> 8a893fcfa3d5d988a88fdeaf0bfc08e0a49b7a65 
>   src/health-check/main.cpp a4ce742ab8deff1ebd99359112670493fdaeeac3 
>   src/launcher/executor.cpp de6f1b104a765a8e53934154e78872b03695b24c 
>   src/local/main.cpp a641b9e83862743890597a2981a9419517e7c589 
>   src/log/tool/benchmark.hpp e0109e20d354a6545ab10750e5d2c3e800f7a6a8 
>   src/log/tool/benchmark.cpp 01e55115f35d155efbea190b5308b294ba76e7cb 
>   src/log/tool/initialize.hpp 10ac269627052d89e1936e891127486d88c74253 
>   src/log/tool/initialize.cpp ccda7fb1c0f7113f865ec61adee76b2ea6180442 
>   src/log/tool/read.hpp 9a6971bca7efc1ddd45efaa44733c1a474804da0 
>   src/log/tool/read.cpp d14138502f5bc9a725deb83da505765865da017f 
>   src/log/tool/replica.cpp 3985fc7df4f7153ae623589fbdd769ccbae57125 
>   src/master/main.cpp d5666bc8ee8d7a0f0b8685f76d65dd1f9ac2a280 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> 6579286b779882bec493a8e0f10486adc316dc6e 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 843e52d6f9923d0ee0a0297cd5c464b8b72f5de3 
>   src/slave/main.cpp f762f5b06be74c391cbc336b2da28f8358952ba4 
>   src/tests/main.cpp e3fff5d60c0468c0d258f2bb301efc1309c071b0 
>   src/usage/main.cpp 97f55e938dc7678f8331970d8953d09218f70902 
> 
> Diff: https://reviews.apache.org/r/34195/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> **NOTE** this fixes completely the chained changes from 34193 and makes all 
> the tests pass.
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 34863: Add tests for new qdisc statistics functions.

[Paul Brett]

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

(Updated June 2, 2015, 5:31 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.


Changes
---

Incorporate review comments.


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


Repository: mesos


Description
---

Add tests for new qdisc statistics functions.


Diffs (updated)
-

  src/tests/routing_tests.cpp 2066a56571f43a012992b4294fbf5b6e07c2b159 

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


Testing
---

make check


Thanks,

Paul Brett

Re: Review Request 31505: Add flow classifiers for fq_codel on egress

[Cong Wang]

> On June 2, 2015, 12:57 a.m., Chi Zhang wrote:
> > Cong, is this the patch the requires code from 3.2.26 for the first time? 
> > If so, should we add a compile-time flag to config.ac? That way Paul can 
> > safely back off the code for the "stat string conversion" from the other 
> > patch you commented on.
> 
> Cong Wang wrote:
> No, https://reviews.apache.org/r/31503/ this one should be the first 
> commit requires 3.2.26.
> 
> Chi Zhang wrote:
> hmm looks like both are committed? could you submit a fix for that please?

Done: https://issues.apache.org/jira/browse/MESOS-2803 . Feel free to take it 
if you have time.


- Cong


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


On June 1, 2015, 10:45 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31505/
> ---
> 
> (Updated June 1, 2015, 10:45 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
> 
> 
> Bugs: MESOS-2422
> https://issues.apache.org/jira/browse/MESOS-2422
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we do nothing on the host egress side. By default, kernel uses its 
> own hash function to classify the packets to different TX queues (if the 
> hardware interface supports multiqueue). So packets coming out of different 
> containers could end up queueing in the same TX queue, in this case we saw 
> buffer bloat on some TX queue caused packet drops.
> 
> We need to isolation the egress traffic so that containers will not have 
> interference with each other. The number of hardware TX queues is limited by 
> hardware interface, usually not enough to map our container in 1:1 way, 
> therefore we need some software solution. We choose fq_codel and use tc 
> filters to classify packets coming out of different containers to different 
> fq_codel flows, and the codel algorithm on each flow could also help us to 
> reduce the buffer bloat. Note when the packets leave fq_codel, they still 
> share the physical TX queue(s), this is however (almost) beyond what we can 
> control, we have to rely on the kernel behavior.
> 
> TODO: get some performance numbers
> 
> 
> Diffs
> -
> 
>   src/linux/routing/filter/basic.cpp 4ce8acb7040f2ed8ec3834dd7702a20f5316c20f 
>   src/linux/routing/filter/filter.hpp 
> aaca57fbe80e3ffa3dd2c2bbed93849013b7a382 
>   src/linux/routing/filter/icmp.cpp 76877fb94a1c1e79133b8975419d2ea7d0300650 
>   src/linux/routing/filter/ip.hpp 9645f9488938c55fec253b36d9fa30eae72a8ca2 
>   src/linux/routing/filter/ip.cpp 0f3b856bd04f6a881e35452a24399715cd8a174f 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> 6579286b779882bec493a8e0f10486adc316dc6e 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 843e52d6f9923d0ee0a0297cd5c464b8b72f5de3 
> 
> Diff: https://reviews.apache.org/r/31505/diff/
> 
> 
> Testing
> ---
> 
> Manually start two mesos containers with netperf running side.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

[Jie Yu]

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

Ship it!



src/examples/test_resource_estimator_module.cpp


Try result = NoopResourceEstimator::create(None());



src/tests/module.hpp


Do we need this?


- Jie Yu


On June 2, 2015, 12:40 a.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34662/
> ---
> 
> (Updated June 2, 2015, 12:40 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-2650
> https://issues.apache.org/jira/browse/MESOS-2650
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added *ResourceEstimator* (RE) module interface. 
> Added *TestResourceEstimator* example module.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/resource_estimator.hpp PRE-CREATION 
>   src/Makefile.am 9d1f0d5de2e7647a6677d86f5032ae1b79f1b241 
>   src/examples/test_resource_estimator_module.cpp PRE-CREATION 
>   src/module/manager.cpp 5fed1aa82f72aa55c8e4e946f5f458ee357162be 
>   src/tests/module.hpp c379f01ae4e185960b8710b6a10626a3b8784fc8 
>   src/tests/module.cpp c4f811998aef14ab247380318fab79f18672ab29 
> 
> Diff: https://reviews.apache.org/r/34662/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 34816: Changed TestResourceEstimator to be mocked.

[Jie Yu]

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



src/tests/oversubscription_tests.cpp


Any reason change the comments and the test name here?



src/tests/oversubscription_tests.cpp


No snake_case please


- Jie Yu


On June 2, 2015, 12:38 a.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34816/
> ---
> 
> (Updated June 2, 2015, 12:38 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> TestResourceEstimator changed to be a  MockResourceEstimator with mocked 
> methods - to inject resources and test plumbing between RE and Slave.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp ac986a0687a576a0bd5693c82b3c7d543aaa956b 
>   src/tests/oversubscription_tests.cpp 
> ea5857cb579aa904fd05530684bdde51a0b3f27f 
> 
> Diff: https://reviews.apache.org/r/34816/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 34939: Started Doxygen Style-Guide.

[Joerg Schad]

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

(Updated June 2, 2015, 5:02 p.m.)


Review request for mesos, Benjamin Hindman and Bernd Mathiske.


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


Repository: mesos


Description
---

Started Doxygen Style-Guide.


Diffs (updated)
-

  docs/mesos-doxygen-style-guide.md PRE-CREATION 

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


Testing
---


Thanks,

Joerg Schad

Re: Review Request 34926: Added help for state.json

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On June 2, 2015, 6:34 a.m., Aditi Dixit wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34926/
> ---
> 
> (Updated June 2, 2015, 6:34 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-2277
> https://issues.apache.org/jira/browse/MESOS-2277
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added help for roles.json, state-summary and edited help for state.json
> 
> 
> Fixed issues
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 01dbdad9720a9cd30505683ae1117970cda918a0 
>   src/master/master.hpp c0cc2933a2cc094401f633df12356bda3d294564 
>   src/master/master.cpp 710b8149c9d855d0f47cb2952366be10bc78c74d 
> 
> Diff: https://reviews.apache.org/r/34926/diff/
> 
> 
> Testing
> ---
> 
> Make check
> 
> 
> Thanks,
> 
> Aditi Dixit
> 
>

Re: Review Request 34426: Report per-container metrics for network bandwidth throttling

[Paul Brett]

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

(Updated June 2, 2015, 4:31 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.


Changes
---

Fix rebase


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


Repository: mesos


Description
---

Report the network statistics from libnl


Diffs (updated)
-

  src/Makefile.am a5a7306b1ef65ca2b643653779ab76c26dbb5c90 
  src/linux/routing/queueing/fq_codel.hpp 
7de1e31d4c43ac9cbffab1e472ea51140719f900 
  src/linux/routing/queueing/fq_codel.cpp 
4dc2a9d2ed52937f0a78a083980db488c06b45a7 
  src/linux/routing/queueing/ingress.hpp 
84506fecd01522471a7998176c28bea8f1367aed 
  src/linux/routing/queueing/ingress.cpp 
ae0c38d2215e7fabcc1060e7385484bd455e1699 
  src/linux/routing/queueing/internal.hpp 
d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70 

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


Testing
---

make check


Thanks,

Paul Brett

Re: Review Request 34816: Changed TestResourceEstimator to be mocked.

[Niklas Nielsen]

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


Looks good! Final questions, tiny nits and let's get this in!


src/tests/mesos.hpp


How will this work, when we can't access the mocked resource estimator from 
the test body?



src/tests/oversubscription_tests.cpp


One newline



src/tests/oversubscription_tests.cpp


Shouldn't we have 'using process::Queue;' somewhere above?



src/tests/oversubscription_tests.cpp


Mind moving the comment above line 92 (before the chain?) Here and below



src/tests/oversubscription_tests.cpp


fits on one line



src/tests/oversubscription_tests.cpp


Why did you change the cpu counts?


- Niklas Nielsen


On June 1, 2015, 5:38 p.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34816/
> ---
> 
> (Updated June 1, 2015, 5:38 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod 
> Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> TestResourceEstimator changed to be a  MockResourceEstimator with mocked 
> methods - to inject resources and test plumbing between RE and Slave.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp ac986a0687a576a0bd5693c82b3c7d543aaa956b 
>   src/tests/oversubscription_tests.cpp 
> ea5857cb579aa904fd05530684bdde51a0b3f27f 
> 
> Diff: https://reviews.apache.org/r/34816/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 30612: Added /master/frameworks/{framework}/tasks/{task} endpoint.

[Adam B]

> On May 28, 2015, 2:53 p.m., Marco Massenzio wrote:
> > Folks - this thing has been out for almost four months now, we should 
> > really get it moving.
> > 
> > @alexdr - can you please address Nik's comments (Nik - you got late to the 
> > party :) but that's ok)
> > 
> > @till - I think Alex addressed your comments, can you please give it a 
> > quick once-over and, unless you see major trouble, can we please commit?
> > 
> > In general, high-level API/protocol isssues should be hashed out during the 
> > design state (here's to hoping we had a Design Doc for this API back in the 
> > day: this patch pre-dates me by more than two months!) so that when we come 
> > to the implementation stage, there are no longer discussions about URI 
> > structure, payload, etc.
> > 
> > Thanks everyone for helping moving this forward!
> 
> Adam B wrote:
> I would love to see a document detailing (with examples) what 
> does/doesn't go into each nested endpoint. The FRAMEWORKS_HELP is rather 
> vague.
> And I am concerned about the payloads of these different endpoints. 
> /state.json is already very bloated, and I'm not convinced a /frameworks that 
> includes all info on all tasks is going to be any better.
> 
> Alexander Rojas wrote:
> @adam-mesos: So we were talking jesterday with ben about the rationale 
> behind this patch (after all I just continued made an old patch work). After 
> all we came to the following conclusions:
> 
> 1. It is true that this can be bloated, though no as bloated as 
> `state.json` since `/frameworks` is a subset of the former.
> 2. The reason behind this (and `/slaves`) endpoint is to unravel 
> `/state.json` into multiple more specific endpoints.
> 3. I will write now a document providing a broad roadmap for this 
> unraveling. With more specifics into the `/frameworks` and `/slaves` 
> endpoints.

Thanks! Please also consider where executorInfos and completed tasks/frameworks 
will show up in these (or other) endpoints.


- Adam


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


On May 29, 2015, 7:47 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30612/
> ---
> 
> (Updated May 29, 2015, 7:47 a.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, Marco Massenzio, Niklas 
> Nielsen, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2157
> https://issues.apache.org/jira/browse/MESOS-2157
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds endpoints for paths /master/frameworks/{framework}/tasks/{task}.
> 
> Adds tests for the endpoints.
> 
> Works with [29883](https://reviews.apache.org/r/29883).
> 
> Builds on the abandoned patch 14286.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 01dbdad9720a9cd30505683ae1117970cda918a0 
>   src/master/master.hpp c0cc2933a2cc094401f633df12356bda3d294564 
>   src/master/master.cpp 710b8149c9d855d0f47cb2952366be10bc78c74d 
>   src/tests/master_tests.cpp ba3858fb3026f2cc7af0523bd1a0c541eeafc34b 
> 
> Diff: https://reviews.apache.org/r/30612/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 34832: Add new qdisc tests

[Paul Brett]

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

(Updated June 2, 2015, 4:13 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.


Changes
---

Change tests to match feedback on function.


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


Repository: mesos


Description
---

Add new qdisc tests for fq_codel and ingress queueing discipline.


Diffs (updated)
-

  src/tests/routing_tests.cpp 2066a56571f43a012992b4294fbf5b6e07c2b159 

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


Testing (updated)
---

make check


Thanks,

Paul Brett

Re: Review Request 33823: Protocol file with the HTTP API messages between executor and slave

[Isabel Jimenez]

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

Ship it!


Ship It!

- Isabel Jimenez


On June 2, 2015, 12:12 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33823/
> ---
> 
> (Updated June 2, 2015, 12:12 p.m.)
> 
> 
> Review request for mesos, Isabel Jimenez, Marco Massenzio, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/executor/executor.hpp PRE-CREATION 
>   include/mesos/executor/executor.proto PRE-CREATION 
>   src/Makefile.am a5a7306b1ef65ca2b643653779ab76c26dbb5c90 
> 
> Diff: https://reviews.apache.org/r/33823/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 34944: Used flags validation to handle --help.

[Marco Massenzio]

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

Ship it!


Sweet!

Only comment is that this does not give the client of FlagsBase an option as to 
the behavior of --help (ie, it prints and exit: like it or lump it :).

Which is perfectly fine, IMO, but was wondering whether this was too 
restrictive?
(I haven't seen any other pattern in the 10+ refactorings I've done - so this 
should be just fine)

- Marco Massenzio


On June 2, 2015, 2:46 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34944/
> ---
> 
> (Updated June 2, 2015, 2:46 p.m.)
> 
> 
> Review request for mesos and Marco Massenzio.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is one possible way of handling processing --help. Also, this is missing 
> a test.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
> 61a405f225d14acbc38a80d35570426cb05a3d0a 
> 
> Diff: https://reviews.apache.org/r/34944/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>