You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Neil Conway <ne...@gmail.com> on 2016/02/25 23:59:29 UTC

Review Request 44047: Added full reserved resource info to `/slaves` master endpoint.

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

Review request for mesos, Michael Park and Vinod Kone.


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


Repository: mesos


Description
-------

This allows operators to list all the dynamic reservations and persistent
volumes in a cluster. This is important in itself; it also makes it easier to
use the `/unreserve` and `/destroy-volumes` endpoints.


Diffs
-----

  docs/persistent-volume.md 2a794a572ff930aa1f95706b89fef9243be627de 
  docs/reservation.md b98ebe6df0739b48c5fa58e087fd64b1c6c5d456 
  src/master/http.cpp 950206baf7f3a1cccdc49d810126473966d8d021 
  src/tests/persistent_volume_endpoints_tests.cpp 6069ca1e9ed278459c5182e438417e95955b1924 

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


Testing
-------

make check


Thanks,

Neil Conway


Re: Review Request 44047: Added full reserved resource info to `/slaves` master endpoint.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44047/#review121111
-----------------------------------------------------------



Patch looks great!

Reviews applied: [43937, 43938, 43939, 43816, 43822, 43823, 43817, 43910, 43911, 44047]

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

- Mesos ReviewBot


On Feb. 28, 2016, 12:17 a.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44047/
> -----------------------------------------------------------
> 
> (Updated Feb. 28, 2016, 12:17 a.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-4667
>     https://issues.apache.org/jira/browse/MESOS-4667
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This allows operators to list all the dynamic reservations and persistent
> volumes in a cluster. This is important in itself; it also makes it easier to
> use the `/unreserve` and `/destroy-volumes` endpoints.
> 
> 
> Diffs
> -----
> 
>   docs/persistent-volume.md 47ada98413f1670e9fc4ebd9d1ead6af9b120184 
>   docs/reservation.md 450f4eec49d957b096df1380c3e79d5f743cc829 
>   src/master/http.cpp f3ce1aa22f5f753fcb254e9ecaa8ba571e3d2829 
>   src/tests/persistent_volume_endpoints_tests.cpp 08b9102318b826bab9d2c1d389fb80b86949218c 
> 
> Diff: https://reviews.apache.org/r/44047/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


Re: Review Request 44047: Added full reserved resource info to `/slaves` master endpoint.

Posted by Neil Conway <ne...@gmail.com>.

> On Feb. 28, 2016, 3:06 p.m., Klaus Ma wrote:
> > docs/reservation.md, line 363
> > <https://reviews.apache.org/r/44047/diff/3/?file=1273028#file1273028line363>
> >
> >     s/slave/agent/

The rest of this file uses "slave", so I think this is better as-is for now (until we do the rename).


- Neil


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


On Feb. 29, 2016, 7:13 a.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44047/
> -----------------------------------------------------------
> 
> (Updated Feb. 29, 2016, 7:13 a.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-4667
>     https://issues.apache.org/jira/browse/MESOS-4667
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This allows operators to list all the dynamic reservations and persistent
> volumes in a cluster. This is important in itself; it also makes it easier to
> use the `/unreserve` and `/destroy-volumes` endpoints.
> 
> 
> Diffs
> -----
> 
>   docs/persistent-volume.md 47ada98413f1670e9fc4ebd9d1ead6af9b120184 
>   docs/reservation.md 450f4eec49d957b096df1380c3e79d5f743cc829 
>   src/master/http.cpp d6e1f22620dfc4271244a2983195cffc36da6e8e 
>   src/tests/persistent_volume_endpoints_tests.cpp 08b9102318b826bab9d2c1d389fb80b86949218c 
> 
> Diff: https://reviews.apache.org/r/44047/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


Re: Review Request 44047: Added full reserved resource info to `/slaves` master endpoint.

Posted by Klaus Ma <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44047/#review121145
-----------------------------------------------------------




docs/reservation.md (line 363)
<https://reviews.apache.org/r/44047/#comment182798>

    s/slave/agent/



src/master/http.cpp (line 1047)
<https://reviews.apache.org/r/44047/#comment182799>

    `` for /unreserve and /destroy-volumes.


- Klaus Ma


On Feb. 28, 2016, 8:17 a.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44047/
> -----------------------------------------------------------
> 
> (Updated Feb. 28, 2016, 8:17 a.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-4667
>     https://issues.apache.org/jira/browse/MESOS-4667
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This allows operators to list all the dynamic reservations and persistent
> volumes in a cluster. This is important in itself; it also makes it easier to
> use the `/unreserve` and `/destroy-volumes` endpoints.
> 
> 
> Diffs
> -----
> 
>   docs/persistent-volume.md 47ada98413f1670e9fc4ebd9d1ead6af9b120184 
>   docs/reservation.md 450f4eec49d957b096df1380c3e79d5f743cc829 
>   src/master/http.cpp f3ce1aa22f5f753fcb254e9ecaa8ba571e3d2829 
>   src/tests/persistent_volume_endpoints_tests.cpp 08b9102318b826bab9d2c1d389fb80b86949218c 
> 
> Diff: https://reviews.apache.org/r/44047/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


Re: Review Request 44047: Added full reserved resource info to `/slaves` master endpoint.

Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44047/
-----------------------------------------------------------

(Updated Feb. 29, 2016, 7:20 a.m.)


Review request for mesos, Michael Park and Vinod Kone.


Changes
-------

Tweaked comment per Klaus.


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


Repository: mesos


Description
-------

This allows operators to list all the dynamic reservations and persistent
volumes in a cluster. This is important in itself; it also makes it easier to
use the `/unreserve` and `/destroy-volumes` endpoints.


Diffs (updated)
-----

  docs/persistent-volume.md 47ada98413f1670e9fc4ebd9d1ead6af9b120184 
  docs/reservation.md 450f4eec49d957b096df1380c3e79d5f743cc829 
  src/master/http.cpp d6e1f22620dfc4271244a2983195cffc36da6e8e 
  src/tests/persistent_volume_endpoints_tests.cpp 08b9102318b826bab9d2c1d389fb80b86949218c 

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


Testing
-------

make check


Thanks,

Neil Conway


Re: Review Request 44047: Added full reserved resource info to `/slaves` master endpoint.

Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44047/
-----------------------------------------------------------

(Updated Feb. 29, 2016, 7:13 a.m.)


Review request for mesos, Michael Park and Vinod Kone.


Changes
-------

Tweak per mpark.


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


Repository: mesos


Description
-------

This allows operators to list all the dynamic reservations and persistent
volumes in a cluster. This is important in itself; it also makes it easier to
use the `/unreserve` and `/destroy-volumes` endpoints.


Diffs (updated)
-----

  docs/persistent-volume.md 47ada98413f1670e9fc4ebd9d1ead6af9b120184 
  docs/reservation.md 450f4eec49d957b096df1380c3e79d5f743cc829 
  src/master/http.cpp d6e1f22620dfc4271244a2983195cffc36da6e8e 
  src/tests/persistent_volume_endpoints_tests.cpp 08b9102318b826bab9d2c1d389fb80b86949218c 

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


Testing
-------

make check


Thanks,

Neil Conway


Re: Review Request 44047: Added full reserved resource info to `/slaves` master endpoint.

Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44047/
-----------------------------------------------------------

(Updated Feb. 29, 2016, 7:01 a.m.)


Review request for mesos, Michael Park and Vinod Kone.


Changes
-------

Revert unintended change.


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


Repository: mesos


Description
-------

This allows operators to list all the dynamic reservations and persistent
volumes in a cluster. This is important in itself; it also makes it easier to
use the `/unreserve` and `/destroy-volumes` endpoints.


Diffs (updated)
-----

  docs/persistent-volume.md 47ada98413f1670e9fc4ebd9d1ead6af9b120184 
  docs/reservation.md 450f4eec49d957b096df1380c3e79d5f743cc829 
  src/master/http.cpp d6e1f22620dfc4271244a2983195cffc36da6e8e 
  src/tests/persistent_volume_endpoints_tests.cpp 08b9102318b826bab9d2c1d389fb80b86949218c 

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


Testing
-------

make check


Thanks,

Neil Conway


Re: Review Request 44047: Added full reserved resource info to `/slaves` master endpoint.

Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44047/
-----------------------------------------------------------

(Updated Feb. 29, 2016, 6:55 a.m.)


Review request for mesos, Michael Park and Vinod Kone.


Changes
-------

Rebase.


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


Repository: mesos


Description
-------

This allows operators to list all the dynamic reservations and persistent
volumes in a cluster. This is important in itself; it also makes it easier to
use the `/unreserve` and `/destroy-volumes` endpoints.


Diffs (updated)
-----

  docs/persistent-volume.md 47ada98413f1670e9fc4ebd9d1ead6af9b120184 
  docs/reservation.md 450f4eec49d957b096df1380c3e79d5f743cc829 
  src/master/http.cpp d6e1f22620dfc4271244a2983195cffc36da6e8e 
  src/tests/persistent_volume_endpoints_tests.cpp 08b9102318b826bab9d2c1d389fb80b86949218c 

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


Testing
-------

make check


Thanks,

Neil Conway


Re: Review Request 44047: Added full reserved resource info to `/slaves` master endpoint.

Posted by Neil Conway <ne...@gmail.com>.

> On Feb. 28, 2016, 3:15 a.m., Guangya Liu wrote:
> > docs/persistent-volume.md, lines 372-374
> > <https://reviews.apache.org/r/44047/diff/3/?file=1273027#file1273027line372>
> >
> >     Only the `reserved_resources_full` was mentioned here, what about `used_resources_full` and `reserved_resources_full`? I think that those new metrics should also be mentioned here to clarify the meaning of them.

In my opinion, this is okay as-is: we don't document most of the keys in the JSON objects we return (which is unfortunate and something we should fix, but that's a separate project).


> On Feb. 28, 2016, 3:15 a.m., Guangya Liu wrote:
> > src/master/http.cpp, line 1043
> > <https://reviews.apache.org/r/44047/diff/3/?file=1273029#file1273029line1043>
> >
> >     s/offered resources/offered resources per agent

I think this is implied by context (the comment occurs inside a for-each-slave loop).


- Neil


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


On Feb. 29, 2016, 7:13 a.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44047/
> -----------------------------------------------------------
> 
> (Updated Feb. 29, 2016, 7:13 a.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-4667
>     https://issues.apache.org/jira/browse/MESOS-4667
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This allows operators to list all the dynamic reservations and persistent
> volumes in a cluster. This is important in itself; it also makes it easier to
> use the `/unreserve` and `/destroy-volumes` endpoints.
> 
> 
> Diffs
> -----
> 
>   docs/persistent-volume.md 47ada98413f1670e9fc4ebd9d1ead6af9b120184 
>   docs/reservation.md 450f4eec49d957b096df1380c3e79d5f743cc829 
>   src/master/http.cpp d6e1f22620dfc4271244a2983195cffc36da6e8e 
>   src/tests/persistent_volume_endpoints_tests.cpp 08b9102318b826bab9d2c1d389fb80b86949218c 
> 
> Diff: https://reviews.apache.org/r/44047/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


Re: Review Request 44047: Added full reserved resource info to `/slaves` master endpoint.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44047/#review121114
-----------------------------------------------------------




docs/persistent-volume.md (lines 372 - 374)
<https://reviews.apache.org/r/44047/#comment182748>

    Only the `reserved_resources_full` was mentioned here, what about `used_resources_full` and `reserved_resources_full`? I think that those new metrics should also be mentioned here to clarify the meaning of them.



docs/reservation.md (lines 363 - 365)
<https://reviews.apache.org/r/44047/#comment182749>

    ditto



src/master/http.cpp (line 1043)
<https://reviews.apache.org/r/44047/#comment182750>

    s/offered resources/offered resources per agent


- Guangya Liu


On 二月 28, 2016, 12:17 a.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44047/
> -----------------------------------------------------------
> 
> (Updated 二月 28, 2016, 12:17 a.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-4667
>     https://issues.apache.org/jira/browse/MESOS-4667
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This allows operators to list all the dynamic reservations and persistent
> volumes in a cluster. This is important in itself; it also makes it easier to
> use the `/unreserve` and `/destroy-volumes` endpoints.
> 
> 
> Diffs
> -----
> 
>   docs/persistent-volume.md 47ada98413f1670e9fc4ebd9d1ead6af9b120184 
>   docs/reservation.md 450f4eec49d957b096df1380c3e79d5f743cc829 
>   src/master/http.cpp f3ce1aa22f5f753fcb254e9ecaa8ba571e3d2829 
>   src/tests/persistent_volume_endpoints_tests.cpp 08b9102318b826bab9d2c1d389fb80b86949218c 
> 
> Diff: https://reviews.apache.org/r/44047/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


Re: Review Request 44047: Added full reserved resource info to `/slaves` master endpoint.

Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44047/
-----------------------------------------------------------

(Updated Feb. 28, 2016, 12:17 a.m.)


Review request for mesos, Michael Park and Vinod Kone.


Changes
-------

Rebase.


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


Repository: mesos


Description
-------

This allows operators to list all the dynamic reservations and persistent
volumes in a cluster. This is important in itself; it also makes it easier to
use the `/unreserve` and `/destroy-volumes` endpoints.


Diffs (updated)
-----

  docs/persistent-volume.md 47ada98413f1670e9fc4ebd9d1ead6af9b120184 
  docs/reservation.md 450f4eec49d957b096df1380c3e79d5f743cc829 
  src/master/http.cpp f3ce1aa22f5f753fcb254e9ecaa8ba571e3d2829 
  src/tests/persistent_volume_endpoints_tests.cpp 08b9102318b826bab9d2c1d389fb80b86949218c 

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


Testing
-------

make check


Thanks,

Neil Conway


Re: Review Request 44047: Added full reserved resource info to `/slaves` master endpoint.

Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44047/
-----------------------------------------------------------

(Updated Feb. 26, 2016, 7:10 p.m.)


Review request for mesos, Michael Park and Vinod Kone.


Changes
-------

Tweak docs.


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


Repository: mesos


Description
-------

This allows operators to list all the dynamic reservations and persistent
volumes in a cluster. This is important in itself; it also makes it easier to
use the `/unreserve` and `/destroy-volumes` endpoints.


Diffs (updated)
-----

  docs/persistent-volume.md 2a794a572ff930aa1f95706b89fef9243be627de 
  docs/reservation.md b98ebe6df0739b48c5fa58e087fd64b1c6c5d456 
  src/master/http.cpp 950206baf7f3a1cccdc49d810126473966d8d021 
  src/tests/persistent_volume_endpoints_tests.cpp 6069ca1e9ed278459c5182e438417e95955b1924 

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


Testing
-------

make check


Thanks,

Neil Conway


Re: Review Request 44047: Added full reserved resource info to `/slaves` master endpoint.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44047/#review120845
-----------------------------------------------------------



Patch looks great!

Reviews applied: [43816, 43822, 43823, 43817, 43910, 43911, 44047]

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

- Mesos ReviewBot


On Feb. 25, 2016, 10:59 p.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44047/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2016, 10:59 p.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-4667
>     https://issues.apache.org/jira/browse/MESOS-4667
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This allows operators to list all the dynamic reservations and persistent
> volumes in a cluster. This is important in itself; it also makes it easier to
> use the `/unreserve` and `/destroy-volumes` endpoints.
> 
> 
> Diffs
> -----
> 
>   docs/persistent-volume.md 2a794a572ff930aa1f95706b89fef9243be627de 
>   docs/reservation.md b98ebe6df0739b48c5fa58e087fd64b1c6c5d456 
>   src/master/http.cpp 950206baf7f3a1cccdc49d810126473966d8d021 
>   src/tests/persistent_volume_endpoints_tests.cpp 6069ca1e9ed278459c5182e438417e95955b1924 
> 
> Diff: https://reviews.apache.org/r/44047/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>