You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Michael Park <mc...@gmail.com> on 2015/06/28 10:37:11 UTC

Review Request 35983: Added /unreserve HTTP endpoint to the master.

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

Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.


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


Repository: mesos


Description
-------

See summary.


Diffs
-----

  src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
  src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
  src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 

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


Testing
-------

`make check`


Thanks,

Michael Park


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On July 2, 2015, 3:39 p.m., Alexander Rukletsov wrote:
> > A high level question: do operators have the possibility to get a list of all dynamic reservations? I think about a situation, when a framework made some reservations and then quit, an operator wants to clean up those reservations, but they need a list of them, right? Since we target the endpoints for 0.24, does it make sense to include one more endpoint?
> 
> Michael Park wrote:
>     The `slaves` endpoint returns the JSON model of all of the slaves, and one of the fields of the model is `reserved_resources`. Is this sufficient?

I think it is sufficient, but it would be *nicer* to have an endpoint just for dynamic reservations. Also, this is something we planned for quota, so it would be more consistent. In any case, I think it makes sense to capture the scenarion and its solution in the doc for operators.


- Alexander


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


On June 28, 2015, 8:37 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35983/
> -----------------------------------------------------------
> 
> (Updated June 28, 2015, 8:37 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
>     https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
> 
> Diff: https://reviews.apache.org/r/35983/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

Posted by Michael Park <mc...@gmail.com>.

> On July 2, 2015, 3:39 p.m., Alexander Rukletsov wrote:
> > A high level question: do operators have the possibility to get a list of all dynamic reservations? I think about a situation, when a framework made some reservations and then quit, an operator wants to clean up those reservations, but they need a list of them, right? Since we target the endpoints for 0.24, does it make sense to include one more endpoint?

The `slaves` endpoint returns the JSON model of all of the slaves, and one of the fields of the model is `reserved_resources`. Is this sufficient?


- Michael


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


On June 28, 2015, 8:37 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35983/
> -----------------------------------------------------------
> 
> (Updated June 28, 2015, 8:37 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
>     https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
> 
> Diff: https://reviews.apache.org/r/35983/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35983/#review90235
-----------------------------------------------------------


A high level question: do operators have the possibility to get a list of all dynamic reservations? I think about a situation, when a framework made some reservations and then quit, an operator wants to clean up those reservations, but they need a list of them, right? Since we target the endpoints for 0.24, does it make sense to include one more endpoint?

- Alexander Rukletsov


On June 28, 2015, 8:37 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35983/
> -----------------------------------------------------------
> 
> (Updated June 28, 2015, 8:37 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
>     https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
> 
> Diff: https://reviews.apache.org/r/35983/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

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



src/master/http.cpp (lines 1909 - 1914)
<https://reviews.apache.org/r/35983/#comment153999>

    get ur point, here just checkig if there are offers can be unreserved.


- Guangya Liu


On 九月 4, 2015, 11:20 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35983/
> -----------------------------------------------------------
> 
> (Updated 九月 4, 2015, 11:20 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
>     https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 94e97a2898106579434e8cdec04b7b0e130a810e 
>   src/master/master.hpp e1331851c19e3372a4a525dcfd7ba2a01c3e97a6 
>   src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
>   src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
>   src/tests/master_validation_tests.cpp 3513bca6fd6773f712d1a647b1757766dc34f948 
> 
> Diff: https://reviews.apache.org/r/35983/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

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



src/master/http.cpp (lines 1909 - 1914)
<https://reviews.apache.org/r/35983/#comment153971>

    Just curious why UNRESERVE still call Http::_operation to greedily rescind offers?


- Guangya Liu


On 九月 4, 2015, 11:20 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35983/
> -----------------------------------------------------------
> 
> (Updated 九月 4, 2015, 11:20 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
>     https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 94e97a2898106579434e8cdec04b7b0e130a810e 
>   src/master/master.hpp e1331851c19e3372a4a525dcfd7ba2a01c3e97a6 
>   src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
>   src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
>   src/tests/master_validation_tests.cpp 3513bca6fd6773f712d1a647b1757766dc34f948 
> 
> Diff: https://reviews.apache.org/r/35983/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35983/
-----------------------------------------------------------

(Updated Sept. 4, 2015, 11:20 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Rebased. NNFR.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/master/http.cpp 94e97a2898106579434e8cdec04b7b0e130a810e 
  src/master/master.hpp e1331851c19e3372a4a525dcfd7ba2a01c3e97a6 
  src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
  src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
  src/tests/master_validation_tests.cpp 3513bca6fd6773f712d1a647b1757766dc34f948 

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


Testing
-------

`make check`


Thanks,

Michael Park


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35983/
-----------------------------------------------------------

(Updated Aug. 12, 2015, 2:46 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Discarded [r36987](https://reviews.apache.org/r/36987/) and rebased accordingly.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/master/http.cpp 7c650555ed15d310ad28e68239cbd56580fbae37 
  src/master/master.hpp bb7c8e9e05be829a6b9aa3100a714b2359854d96 
  src/master/master.cpp 398203d9367f85340166e66ecc34b9a33dd81048 
  src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
  src/tests/master_validation_tests.cpp 3513bca6fd6773f712d1a647b1757766dc34f948 

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


Testing
-------

`make check`


Thanks,

Michael Park


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35983/#review90790
-----------------------------------------------------------

Ship it!


LGTM!

- Jie Yu


On Aug. 5, 2015, 7:12 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35983/
> -----------------------------------------------------------
> 
> (Updated Aug. 5, 2015, 7:12 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
>     https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
>   src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
>   src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 
>   src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
>   src/tests/master_validation_tests.cpp 3513bca6fd6773f712d1a647b1757766dc34f948 
> 
> Diff: https://reviews.apache.org/r/35983/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35983/
-----------------------------------------------------------

(Updated Aug. 5, 2015, 7:12 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Removed `Nothing`.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 
  src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
  src/tests/master_validation_tests.cpp 3513bca6fd6773f712d1a647b1757766dc34f948 

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


Testing
-------

`make check`


Thanks,

Michael Park


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35983/
-----------------------------------------------------------

(Updated Aug. 5, 2015, 11:15 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf 
  src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
  src/tests/master_validation_tests.cpp 3513bca6fd6773f712d1a647b1757766dc34f948 

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


Testing
-------

`make check`


Thanks,

Michael Park


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35983/
-----------------------------------------------------------

(Updated Aug. 5, 2015, 10:44 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Removed temporary variables.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf 

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


Testing
-------

`make check`


Thanks,

Michael Park


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35983/
-----------------------------------------------------------

(Updated Aug. 5, 2015, 9:54 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Factored out `Master::Http::_operation`.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf 

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


Testing
-------

`make check`


Thanks,

Michael Park


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35983/
-----------------------------------------------------------

(Updated July 31, 2015, 9:57 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f 
  src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc 
  src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 

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


Testing
-------

`make check`


Thanks,

Michael Park


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35983/
-----------------------------------------------------------

(Updated July 28, 2015, 9:06 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Added comments and updated to only rescind offers if rescinding the offer will contribute in satisfying the request


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
  src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
  src/master/master.cpp 5b5e3c37d4433c8524db267866aebc0a35a181f1 

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


Testing
-------

`make check`


Thanks,

Michael Park


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On July 16, 2015, 3:04 p.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, lines 1325-1332
> > <https://reviews.apache.org/r/35983/diff/1/?file=994085#file994085line1325>
> >
> >     Why do we need to recover resources for unreserve?
> 
> Michael Park wrote:
>     If reserved resources are offered, we need to recover the reserved resources before we proceed to updating them to be unreserved.

But you recover all active offers on the slave, not just those with dynamic reservations, or am I missing something?


- Alexander


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


On June 28, 2015, 8:37 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35983/
> -----------------------------------------------------------
> 
> (Updated June 28, 2015, 8:37 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
>     https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
> 
> Diff: https://reviews.apache.org/r/35983/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

Posted by Michael Park <mc...@gmail.com>.

> On July 16, 2015, 3:04 p.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, lines 1325-1332
> > <https://reviews.apache.org/r/35983/diff/1/?file=994085#file994085line1325>
> >
> >     Why do we need to recover resources for unreserve?
> 
> Michael Park wrote:
>     If reserved resources are offered, we need to recover the reserved resources before we proceed to updating them to be unreserved.
> 
> Alexander Rukletsov wrote:
>     But you recover all active offers on the slave, not just those with dynamic reservations, or am I missing something?

I see what you're saying I think. I've updated the code to only rescind the offer if rescinding the offer will contribute to satisfying the request, which I think is more accurate than checking that the offer contains dynamically reserved resources.


- Michael


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


On July 28, 2015, 9:06 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35983/
> -----------------------------------------------------------
> 
> (Updated July 28, 2015, 9:06 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
>     https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
>   src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
>   src/master/master.cpp 5b5e3c37d4433c8524db267866aebc0a35a181f1 
> 
> Diff: https://reviews.apache.org/r/35983/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On July 16, 2015, 3:04 p.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, line 1291
> > <https://reviews.apache.org/r/35983/diff/1/?file=994085#file994085line1291>
> >
> >     As in https://reviews.apache.org/r/35702/, I suggest we extract validation into a separate function.
> 
> Michael Park wrote:
>     In https://reviews.apache.org/r/35702/, I suggested that we should refactor the validation logic uniformly in all of the endpoints as a separate task. Is that reasonable?

Yes, it's enough to have the discussion in one place : ).


- Alexander


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


On June 28, 2015, 8:37 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35983/
> -----------------------------------------------------------
> 
> (Updated June 28, 2015, 8:37 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
>     https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
> 
> Diff: https://reviews.apache.org/r/35983/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On July 16, 2015, 3:04 p.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, lines 1325-1332
> > <https://reviews.apache.org/r/35983/diff/1/?file=994085#file994085line1325>
> >
> >     Why do we need to recover resources for unreserve?
> 
> Michael Park wrote:
>     If reserved resources are offered, we need to recover the reserved resources before we proceed to updating them to be unreserved.
> 
> Alexander Rukletsov wrote:
>     But you recover all active offers on the slave, not just those with dynamic reservations, or am I missing something?
> 
> Michael Park wrote:
>     I see what you're saying I think. I've updated the code to only rescind the offer if rescinding the offer will contribute to satisfying the request, which I think is more accurate than checking that the offer contains dynamically reserved resources.

I think your change is fine and will do the job.


- Alexander


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


On July 28, 2015, 9:06 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35983/
> -----------------------------------------------------------
> 
> (Updated July 28, 2015, 9:06 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
>     https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
>   src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
>   src/master/master.cpp 5b5e3c37d4433c8524db267866aebc0a35a181f1 
> 
> Diff: https://reviews.apache.org/r/35983/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

Posted by Michael Park <mc...@gmail.com>.

> On July 16, 2015, 3:04 p.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, lines 1325-1332
> > <https://reviews.apache.org/r/35983/diff/1/?file=994085#file994085line1325>
> >
> >     Why do we need to recover resources for unreserve?

If reserved resources are offered, we need to recover the reserved resources before we proceed to updating them to be unreserved.


> On July 16, 2015, 3:04 p.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, line 1291
> > <https://reviews.apache.org/r/35983/diff/1/?file=994085#file994085line1291>
> >
> >     As in https://reviews.apache.org/r/35702/, I suggest we extract validation into a separate function.

In https://reviews.apache.org/r/35702/, I suggested that we should refactor the validation logic uniformly in all of the endpoints as a separate task. Is that reasonable?


- Michael


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


On June 28, 2015, 8:37 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35983/
> -----------------------------------------------------------
> 
> (Updated June 28, 2015, 8:37 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
>     https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
> 
> Diff: https://reviews.apache.org/r/35983/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 35983: Added /unreserve HTTP endpoint to the master.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35983/#review91890
-----------------------------------------------------------



src/master/http.cpp (line 1291)
<https://reviews.apache.org/r/35983/#comment145599>

    As in https://reviews.apache.org/r/35702/, I suggest we extract validation into a separate function.



src/master/http.cpp (lines 1325 - 1332)
<https://reviews.apache.org/r/35983/#comment145600>

    Why do we need to recover resources for unreserve?


- Alexander Rukletsov


On June 28, 2015, 8:37 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35983/
> -----------------------------------------------------------
> 
> (Updated June 28, 2015, 8:37 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
>     https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
> 
> Diff: https://reviews.apache.org/r/35983/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>