Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-09-05 Thread Guangya Liu

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

Ship it!


Ship It!

- Guangya Liu


On 九月 4, 2015, 11:19 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35702/
> ---
> 
> (Updated 九月 4, 2015, 11:19 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
> ---
> 
> This involved a lot more challenges than I anticipated, I've captured the 
> various approaches and limitations and deal-breakers of those approaches 
> here: [Master Endpoint Implementation 
> Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
> 
> Key points:
> 
> * This is a stop-gap solution until we shift the offer creation/management 
> logic from the master to the allocator.
> * `updateAvailable` and `updateSlave` are kept separate because
>   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
>   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
>   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
> and must not, whereas `updateSlave` does, and can.
> * The algorithm:
> * Initially, the master pessimistically assume that what seems like 
> "available" resources will be gone.
>   This is due to the race between the allocator scheduling an `allocate` 
> call to itself vs master's
>   `allocator->updateAvailable` invocation.
>   As such, we first try to satisfy the request only with the offered 
> resources.
> * We greedily rescind one offer at a time until we've rescinded 
> sufficiently many offers.
>   IMPORTANT: We perform `recoverResources(..., Filters())` which has a 
> default `refuse_sec` of 5 seconds,
>   rather than `recoverResources(..., None())` so that we can virtually 
> always win the race against `allocate`.
>   In the rare case that we do lose, no disaster occurs. We simply fail to 
> satisfy the request.
> * If we still don't have enough resources after resciding all offers, be 
> semi-optimistic and forward the
>   request to the allocator since there may be available resources to 
> satisfy the request.
> * If the allocator returns a failure, report the error to the user with 
> `Conflict`.
> 
> This approach is clearly not ideal, since we would prefer to rescind as 
> little offers as possible.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 94e97a2898106579434e8cdec04b7b0e130a810e 
>   src/master/master.hpp e1331851c19e3372a4a525dcfd7ba2a01c3e97a6 
>   src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
>   src/master/validation.hpp 43b8d84556e7f0a891dddf6185bbce7ca50b360a 
>   src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
> 
> Diff: https://reviews.apache.org/r/35702/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>



Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-09-04 Thread Michael Park

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

(Updated Sept. 4, 2015, 11:19 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
---

This involved a lot more challenges than I anticipated, I've captured the 
various approaches and limitations and deal-breakers of those approaches here: 
[Master Endpoint Implementation 
Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)

Key points:

* This is a stop-gap solution until we shift the offer creation/management 
logic from the master to the allocator.
* `updateAvailable` and `updateSlave` are kept separate because
  (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
  (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
  (3) `updateAvailable` never leaves the allocator in an over-allocated state 
and must not, whereas `updateSlave` does, and can.
* The algorithm:
* Initially, the master pessimistically assume that what seems like 
"available" resources will be gone.
  This is due to the race between the allocator scheduling an `allocate` 
call to itself vs master's
  `allocator->updateAvailable` invocation.
  As such, we first try to satisfy the request only with the offered 
resources.
* We greedily rescind one offer at a time until we've rescinded 
sufficiently many offers.
  IMPORTANT: We perform `recoverResources(..., Filters())` which has a 
default `refuse_sec` of 5 seconds,
  rather than `recoverResources(..., None())` so that we can virtually 
always win the race against `allocate`.
  In the rare case that we do lose, no disaster occurs. We simply fail to 
satisfy the request.
* If we still don't have enough resources after resciding all offers, be 
semi-optimistic and forward the
  request to the allocator since there may be available resources to 
satisfy the request.
* If the allocator returns a failure, report the error to the user with 
`Conflict`.

This approach is clearly not ideal, since we would prefer to rescind as little 
offers as possible.


Diffs (updated)
-

  src/master/http.cpp 94e97a2898106579434e8cdec04b7b0e130a810e 
  src/master/master.hpp e1331851c19e3372a4a525dcfd7ba2a01c3e97a6 
  src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
  src/master/validation.hpp 43b8d84556e7f0a891dddf6185bbce7ca50b360a 
  src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 

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


Testing
---

`make check`


Thanks,

Michael Park



Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-08-11 Thread Michael Park

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

(Updated Aug. 12, 2015, 2:44 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
---

This involved a lot more challenges than I anticipated, I've captured the 
various approaches and limitations and deal-breakers of those approaches here: 
[Master Endpoint Implementation 
Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)

Key points:

* This is a stop-gap solution until we shift the offer creation/management 
logic from the master to the allocator.
* `updateAvailable` and `updateSlave` are kept separate because
  (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
  (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
  (3) `updateAvailable` never leaves the allocator in an over-allocated state 
and must not, whereas `updateSlave` does, and can.
* The algorithm:
* Initially, the master pessimistically assume that what seems like 
available resources will be gone.
  This is due to the race between the allocator scheduling an `allocate` 
call to itself vs master's
  `allocator-updateAvailable` invocation.
  As such, we first try to satisfy the request only with the offered 
resources.
* We greedily rescind one offer at a time until we've rescinded 
sufficiently many offers.
  IMPORTANT: We perform `recoverResources(..., Filters())` which has a 
default `refuse_sec` of 5 seconds,
  rather than `recoverResources(..., None())` so that we can virtually 
always win the race against `allocate`.
  In the rare case that we do lose, no disaster occurs. We simply fail to 
satisfy the request.
* If we still don't have enough resources after resciding all offers, be 
semi-optimistic and forward the
  request to the allocator since there may be available resources to 
satisfy the request.
* If the allocator returns a failure, report the error to the user with 
`Conflict`.

This approach is clearly not ideal, since we would prefer to rescind as little 
offers as possible.


Diffs (updated)
-

  src/master/http.cpp 7c650555ed15d310ad28e68239cbd56580fbae37 
  src/master/master.hpp bb7c8e9e05be829a6b9aa3100a714b2359854d96 
  src/master/master.cpp 398203d9367f85340166e66ecc34b9a33dd81048 
  src/master/validation.hpp 43b8d84556e7f0a891dddf6185bbce7ca50b360a 
  src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 

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


Testing
---

`make check`


Thanks,

Michael Park



Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-08-05 Thread Michael Park

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

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


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


Changes
---

Addressed Jie's comments.


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


Repository: mesos


Description
---

This involved a lot more challenges than I anticipated, I've captured the 
various approaches and limitations and deal-breakers of those approaches here: 
[Master Endpoint Implementation 
Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)

Key points:

* This is a stop-gap solution until we shift the offer creation/management 
logic from the master to the allocator.
* `updateAvailable` and `updateSlave` are kept separate because
  (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
  (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
  (3) `updateAvailable` never leaves the allocator in an over-allocated state 
and must not, whereas `updateSlave` does, and can.
* The algorithm:
* Initially, the master pessimistically assume that what seems like 
available resources will be gone.
  This is due to the race between the allocator scheduling an `allocate` 
call to itself vs master's `allocator-updateAvailable` invocation.
  As such, we first try to satisfy the request only with the offered 
resources.
* We greedily rescind one offer at a time until we've rescinded 
sufficiently many offers.
  IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
`recoverResources(..., None())` so that we can pretty much always win the race 
against `allocate`.
 In the case that we lose, no disaster occurs. We simply fail 
to satisfy the request.
* If we still don't have enough resources after resciding all offers, be 
optimistic and forward the request to the allocator since there may be 
available resources to satisfy the request.
* If the allocator returns a failure, report the error to the user with 
`PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
maybe as well. We'll pick one eventually.

This approach is clearly not ideal, since we would prefer to rescind as little 
offers as possible.
The challenges of implementing the ideal solution in the current state is 
described in the document above.

TODO(mpark): Add more comments and test cases.


Diffs (updated)
-

  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf 
  src/master/validation.hpp 43b8d84556e7f0a891dddf6185bbce7ca50b360a 
  src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 

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


Testing
---

`make check`


Thanks,

Michael Park



Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-08-05 Thread Michael Park

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

(Updated Aug. 5, 2015, 9:55 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 (updated)
---

This involved a lot more challenges than I anticipated, I've captured the 
various approaches and limitations and deal-breakers of those approaches here: 
[Master Endpoint Implementation 
Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)

Key points:

* This is a stop-gap solution until we shift the offer creation/management 
logic from the master to the allocator.
* `updateAvailable` and `updateSlave` are kept separate because
  (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
  (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
  (3) `updateAvailable` never leaves the allocator in an over-allocated state 
and must not, whereas `updateSlave` does, and can.
* The algorithm:
* Initially, the master pessimistically assume that what seems like 
available resources will be gone.
  This is due to the race between the allocator scheduling an `allocate` 
call to itself vs master's `allocator-updateAvailable` invocation.
  As such, we first try to satisfy the request only with the offered 
resources.
* We greedily rescind one offer at a time until we've rescinded 
sufficiently many offers.
  IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
`recoverResources(..., None())` so that we can pretty much always win the race 
against `allocate`.
 In the case that we lose, no disaster occurs. We simply fail 
to satisfy the request.
* If we still don't have enough resources after resciding all offers, be 
optimistic and forward the request to the allocator since there may be 
available resources to satisfy the request.
* If the allocator returns a failure, report the error to the user with 
`PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
maybe as well. We'll pick one eventually.

This approach is clearly not ideal, since we would prefer to rescind as little 
offers as possible.
The challenges of implementing the ideal solution in the current state is 
described in the document above.


Diffs
-

  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf 
  src/master/validation.hpp 43b8d84556e7f0a891dddf6185bbce7ca50b360a 
  src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 

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


Testing
---

`make check`


Thanks,

Michael Park



Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-08-05 Thread Michael Park

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

(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.


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


Repository: mesos


Description
---

This involved a lot more challenges than I anticipated, I've captured the 
various approaches and limitations and deal-breakers of those approaches here: 
[Master Endpoint Implementation 
Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)

Key points:

* This is a stop-gap solution until we shift the offer creation/management 
logic from the master to the allocator.
* `updateAvailable` and `updateSlave` are kept separate because
  (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
  (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
  (3) `updateAvailable` never leaves the allocator in an over-allocated state 
and must not, whereas `updateSlave` does, and can.
* The algorithm:
* Initially, the master pessimistically assume that what seems like 
available resources will be gone.
  This is due to the race between the allocator scheduling an `allocate` 
call to itself vs master's `allocator-updateAvailable` invocation.
  As such, we first try to satisfy the request only with the offered 
resources.
* We greedily rescind one offer at a time until we've rescinded 
sufficiently many offers.
  IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
`recoverResources(..., None())` so that we can pretty much always win the race 
against `allocate`.
 In the case that we lose, no disaster occurs. We simply fail 
to satisfy the request.
* If we still don't have enough resources after resciding all offers, be 
optimistic and forward the request to the allocator since there may be 
available resources to satisfy the request.
* If the allocator returns a failure, report the error to the user with 
`PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
maybe as well. We'll pick one eventually.

This approach is clearly not ideal, since we would prefer to rescind as little 
offers as possible.
The challenges of implementing the ideal solution in the current state is 
described in the document above.


Diffs (updated)
-

  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf 
  src/master/validation.hpp 43b8d84556e7f0a891dddf6185bbce7ca50b360a 
  src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 

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


Testing
---

`make check`


Thanks,

Michael Park



Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-08-05 Thread Michael Park


 On Aug. 5, 2015, 5:46 a.m., Jie Yu wrote:
  src/master/http.cpp, line 475
  https://reviews.apache.org/r/35702/diff/12/?file=1026443#file1026443line475
 
  We typically use leading undescore for temp variables. The tailing 
  underscore is for class members (following google style).
  
  In fact, I think the temp variable here is not necessary. There are 
  only two places where this temp variable is used. I would rather use 
  'values.get().at(...)', but this is up to you.

Removed temporary variable and used `values.get().at(...)` instead.

The reason why I did it this way is because I've been following the general 
pattern of:

```cpp
TryObj _obj = getObj(...);
if (_obj.isError()) {
  return SomeError(_obj.error());
}

const Obj obj = _obj.get();

// proceed with 'obj'.
```

This is a very common pattern for us that I would like to eventually explore 
for a cleaner solution.


 On Aug. 5, 2015, 5:46 a.m., Jie Yu wrote:
  src/master/http.cpp, line 498
  https://reviews.apache.org/r/35702/diff/12/?file=1026443#file1026443line498
 
  Do you need this temp variable. Looks like you can just do
  ```
  foreach (.. value, parse.get().values) {...
  ```

Fixed this to use your suggestion. This was another instance of the pattern I 
described above.


 On Aug. 5, 2015, 5:46 a.m., Jie Yu wrote:
  src/master/http.cpp, line 534
  https://reviews.apache.org/r/35702/diff/12/?file=1026443#file1026443line534
 
  I don't like the name 'flatten' :(
  
  Could you at least be more explicit about it (i.e., emphasize that 
  'remaining' only has unreserved resources). 
  
  ```
  Resources remaining = resources.flatten('*');
  ```

I don't like it either, but we currently have 9 instances of `flatten()` but no 
instances of `flatten(*)`. Do you think it's worth breaking consistency here? 
As far as I know, we seem to favor consistency.


 On Aug. 5, 2015, 5:46 a.m., Jie Yu wrote:
  src/master/http.cpp, line 573
  https://reviews.apache.org/r/35702/diff/12/?file=1026443#file1026443line573
 
  What is 'Nothing' here?

The `Nothing` here comes from the result of `master-apply` which returns a 
`FutureNothing`. But I feel like you're not actually asking for an answer 
here?

What would you like to see?

What I have currently is a comment above the code which reads:

```cpp
// Propogate the 'FutureNothing' as 'FutureResponse' where
// 'Nothing' - 'OK' and Failed - 'Conflict'.
```


 On Aug. 5, 2015, 5:46 a.m., Jie Yu wrote:
  src/master/master.cpp, line 5482
  https://reviews.apache.org/r/35702/diff/12/?file=1026445#file1026445line5482
 
  The name sounds weired. You are passing in an offer operation but the 
  function name is called 'applyResourceOperation'.
  
  I would suggest we create two 'Master::apply' overloads and don't worry 
  about the code duplication.
  
  ```
  void apply(framework, slave, opeartion);
  FutureNothing apply(slave, operation);
  ```

I've introduced the overloaded `Master::apply` as suggested. I renamed the 
original `Master::apply` to `Master::_apply` since I wanted to use it as the 
continuation for `Master::apply(Slave*, const Offer::Operation)`, then 
realized I could also use it at the end of `Master::apply(Framework*, Slave*, 
const Offer::Operation)` (the same way it was before). So in the end, 
functions were renamed.


- Michael


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


On Aug. 5, 2015, 10:44 a.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (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.
 
 
 Bugs: MESOS-2600
 https://issues.apache.org/jira/browse/MESOS-2600
 
 
 Repository: mesos
 
 
 Description
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, 

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-08-05 Thread Jie Yu


 On Aug. 5, 2015, 5:46 a.m., Jie Yu wrote:
  src/master/http.cpp, line 573
  https://reviews.apache.org/r/35702/diff/12/?file=1026443#file1026443line573
 
  What is 'Nothing' here?
 
 Michael Park wrote:
 The `Nothing` here comes from the result of `master-apply` which returns 
 a `FutureNothing`. But I feel like you're not actually asking for an answer 
 here?
 
 What would you like to see?
 
 What I have currently is a comment above the code which reads:
 
 ```cpp
 // Propogate the 'FutureNothing' as 'FutureResponse' where
 // 'Nothing' - 'OK' and Failed - 'Conflict'.
 ```

I mean if we remove 'Nothing' here, will it compile?


- Jie


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


On Aug. 5, 2015, 10:44 a.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (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.
 
 
 Bugs: MESOS-2600
 https://issues.apache.org/jira/browse/MESOS-2600
 
 
 Repository: mesos
 
 
 Description
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and forward the request to the allocator since there may be 
 available resources to satisfy the request.
 * If the allocator returns a failure, report the error to the user with 
 `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
 maybe as well. We'll pick one eventually.
 
 This approach is clearly not ideal, since we would prefer to rescind as 
 little offers as possible.
 The challenges of implementing the ideal solution in the current state is 
 described in the document above.
 
 
 Diffs
 -
 
   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
   src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
   src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf 
   src/master/validation.hpp 43b8d84556e7f0a891dddf6185bbce7ca50b360a 
   src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
 
 Diff: https://reviews.apache.org/r/35702/diff/
 
 
 Testing
 ---
 
 `make check`
 
 
 Thanks,
 
 Michael Park
 




Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-08-05 Thread Jie Yu


 On Aug. 5, 2015, 5:46 a.m., Jie Yu wrote:
  src/master/http.cpp, line 534
  https://reviews.apache.org/r/35702/diff/12/?file=1026443#file1026443line534
 
  I don't like the name 'flatten' :(
  
  Could you at least be more explicit about it (i.e., emphasize that 
  'remaining' only has unreserved resources). 
  
  ```
  Resources remaining = resources.flatten('*');
  ```
 
 Michael Park wrote:
 I don't like it either, but we currently have 9 instances of `flatten()` 
 but no instances of `flatten(*)`. Do you think it's worth breaking 
 consistency here? As far as I know, we seem to favor consistency.

OK, fair enough. Given the comment you just added, I think it's much more clear 
not.


- Jie


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


On Aug. 5, 2015, 10:44 a.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (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.
 
 
 Bugs: MESOS-2600
 https://issues.apache.org/jira/browse/MESOS-2600
 
 
 Repository: mesos
 
 
 Description
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and forward the request to the allocator since there may be 
 available resources to satisfy the request.
 * If the allocator returns a failure, report the error to the user with 
 `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
 maybe as well. We'll pick one eventually.
 
 This approach is clearly not ideal, since we would prefer to rescind as 
 little offers as possible.
 The challenges of implementing the ideal solution in the current state is 
 described in the document above.
 
 
 Diffs
 -
 
   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
   src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
   src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf 
   src/master/validation.hpp 43b8d84556e7f0a891dddf6185bbce7ca50b360a 
   src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
 
 Diff: https://reviews.apache.org/r/35702/diff/
 
 
 Testing
 ---
 
 `make check`
 
 
 Thanks,
 
 Michael Park
 




Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-08-05 Thread Michael Park

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

(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 (updated)
---

This involved a lot more challenges than I anticipated, I've captured the 
various approaches and limitations and deal-breakers of those approaches here: 
[Master Endpoint Implementation 
Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)

Key points:

* This is a stop-gap solution until we shift the offer creation/management 
logic from the master to the allocator.
* `updateAvailable` and `updateSlave` are kept separate because
  (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
  (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
  (3) `updateAvailable` never leaves the allocator in an over-allocated state 
and must not, whereas `updateSlave` does, and can.
* The algorithm:
* Initially, the master pessimistically assume that what seems like 
available resources will be gone.
  This is due to the race between the allocator scheduling an `allocate` 
call to itself vs master's
  `allocator-updateAvailable` invocation.
  As such, we first try to satisfy the request only with the offered 
resources.
* We greedily rescind one offer at a time until we've rescinded 
sufficiently many offers.
  IMPORTANT: We perform `recoverResources(..., Filters())` which has a 
default `refuse_sec` of 5 seconds,
  rather than `recoverResources(..., None())` so that we can virtually 
always win the race against `allocate`.
  In the rare case that we do lose, no disaster occurs. We simply fail to 
satisfy the request.
* If we still don't have enough resources after resciding all offers, be 
semi-optimistic and forward the
  request to the allocator since there may be available resources to 
satisfy the request.
* If the allocator returns a failure, report the error to the user with 
`Conflict`.

This approach is clearly not ideal, since we would prefer to rescind as little 
offers as possible.


Diffs (updated)
-

  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 
  src/master/validation.hpp 43b8d84556e7f0a891dddf6185bbce7ca50b360a 
  src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 

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


Testing
---

`make check`


Thanks,

Michael Park



Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-08-05 Thread Michael Park


 On Aug. 5, 2015, 5:46 a.m., Jie Yu wrote:
  src/master/http.cpp, line 573
  https://reviews.apache.org/r/35702/diff/12/?file=1026443#file1026443line573
 
  What is 'Nothing' here?
 
 Michael Park wrote:
 The `Nothing` here comes from the result of `master-apply` which returns 
 a `FutureNothing`. But I feel like you're not actually asking for an answer 
 here?
 
 What would you like to see?
 
 What I have currently is a comment above the code which reads:
 
 ```cpp
 // Propogate the 'FutureNothing' as 'FutureResponse' where
 // 'Nothing' - 'OK' and Failed - 'Conflict'.
 ```
 
 Jie Yu wrote:
 I mean if we remove 'Nothing' here, will it compile?

Oh, it does. I forgot about this behavior. Thanks, updated.


- Michael


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


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/35702/
 ---
 
 (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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's
   `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` which has a 
 default `refuse_sec` of 5 seconds,
   rather than `recoverResources(..., None())` so that we can virtually 
 always win the race against `allocate`.
   In the rare case that we do lose, no disaster occurs. We simply fail to 
 satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 semi-optimistic and forward the
   request to the allocator since there may be available resources to 
 satisfy the request.
 * If the allocator returns a failure, report the error to the user with 
 `Conflict`.
 
 This approach is clearly not ideal, since we would prefer to rescind as 
 little offers as possible.
 
 
 Diffs
 -
 
   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
   src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
   src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 
   src/master/validation.hpp 43b8d84556e7f0a891dddf6185bbce7ca50b360a 
   src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
 
 Diff: https://reviews.apache.org/r/35702/diff/
 
 
 Testing
 ---
 
 `make check`
 
 
 Thanks,
 
 Michael Park
 




Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-08-04 Thread Jie Yu

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

Ship it!


LGTM overall. Please address the remaining issues and commit yourself!


src/master/http.cpp (line 475)
https://reviews.apache.org/r/35702/#comment148677

We typically use leading undescore for temp variables. The tailing 
underscore is for class members (following google style).

In fact, I think the temp variable here is not necessary. There are only 
two places where this temp variable is used. I would rather use 
'values.get().at(...)', but this is up to you.



src/master/http.cpp (line 498)
https://reviews.apache.org/r/35702/#comment148678

Do you need this temp variable. Looks like you can just do
```
foreach (.. value, parse.get().values) {...
```



src/master/http.cpp (line 511)
https://reviews.apache.org/r/35702/#comment148679

Kill this line.



src/master/http.cpp (line 533)
https://reviews.apache.org/r/35702/#comment148685

What does 'recovered' mean and what does remaining mean? It'll be great if 
you comment on that to improve readability.

For example,

```
// The resources recovered by recinding outstanding offers.
...
// The unreserved resources needed to satify the RESERVE operation.
```

IIUC, the variable 'remaining' is only used for optimization, right? Could 
you please mention that in the comments that keep this variable is for 
optimization (i.e., avoid rescinding unnecessary offers).



src/master/http.cpp (line 534)
https://reviews.apache.org/r/35702/#comment148683

I don't like the name 'flatten' :(

Could you at least be more explicit about it (i.e., emphasize that 
'remaining' only has unreserved resources). 

```
Resources remaining = resources.flatten('*');
```



src/master/http.cpp (line 553)
https://reviews.apache.org/r/35702/#comment148686

you win the race because the default filter refuse_sec is 5 seconds? Worth 
mentioning that in the comment.



src/master/http.cpp (line 573)
https://reviews.apache.org/r/35702/#comment148727

What is 'Nothing' here?



src/master/master.cpp (line 5472)
https://reviews.apache.org/r/35702/#comment148729

The name sounds weired. You are passing in an offer operation but the 
function name is called 'applyResourceOperation'.

I would suggest we create two 'Master::apply' overloads and don't worry 
about the code duplication.

```
void apply(framework, slave, opeartion);
FutureNothing apply(slave, operation);
```


- Jie Yu


On July 31, 2015, 9:56 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated July 31, 2015, 9:56 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and forward the request to the allocator since there may be 
 available resources to satisfy the request.
 * If 

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-31 Thread Michael Park


 On June 22, 2015, 1:32 p.m., Alexander Rukletsov wrote:
  src/master/master.cpp, line 749
  https://reviews.apache.org/r/35702/diff/6/?file=989449#file989449line749
 
  I think reserve is too abstract and may collide with future actions 
  (think quota). How about `/dynamic/reserve`?
 
 Alexander Rukletsov wrote:
 Though we currently do not support slashes in endpoints, I think we 
 should fix that first before introducing a `/reserve` endpoint, given these 
 endpoint are not targeted for 0.23.
 
 Joris Van Remoortere wrote:
 Cody had some patches for enabling sub namespaces in endpoints (as in 
 enabling slashes). Might be worth pulling those in.
 
 Alexander Rukletsov wrote:
 Yep, it's https://issues.apache.org/jira/browse/MESOS-2130, I plan to 
 bring up the discussion today at the community sync.
 
 Michael Park wrote:
 The concensus for now seems that (1) we introduce the allocator changes, 
 but address the allocator refactor sooner rather than later, (2) go with 
 `/reserve` for now and update them once the HTTP API folks get to supporting 
 the nested endpoint stuff.
 
 Alexander Rukletsov wrote:
 And (3) we update endpoints names before the following release, i.e. 
 there is no Mesos release, where `/reserve` will exist. Correct?
 
 Michael Park wrote:
 That is the ideal outcome. But if we commit this now/soon, whether we can 
 update the names before 0.24.0 gets out entirely depends on whether the 
 nested endpoint names capabilities get committed on time.
 
 Alexander Rukletsov wrote:
 I think we agreed that we should, in order to avoid deprecation cycle for 
 old endpoints.

I synced with BenH and Jie regarding this topic, they both suggested that we 
should get this in as is and update later. Jie suggested that if the HTTP API 
doesn't make it in in time, we can either not mention it in the `CHANGELOG` or 
mention it as an alpha feature that is subject to change.


- Michael


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


On July 28, 2015, 9:03 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated July 28, 2015, 9:03 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and forward the request to the allocator since there may be 
 available resources to satisfy the request.
 * If the allocator returns a failure, report the error to the user with 
 `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
 maybe as well. We'll pick one eventually.
 
 This approach is clearly not ideal, since we would prefer to rescind as 
 little offers as possible.
 The challenges of implementing the ideal solution in the current state is 
 described in the document above.
 
 TODO(mpark): Add more comments and test cases.
 
 
 Diffs
 -
 
   src/master/http.cpp 

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-31 Thread Michael Park

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

(Updated July 31, 2015, 9:56 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
---

This involved a lot more challenges than I anticipated, I've captured the 
various approaches and limitations and deal-breakers of those approaches here: 
[Master Endpoint Implementation 
Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)

Key points:

* This is a stop-gap solution until we shift the offer creation/management 
logic from the master to the allocator.
* `updateAvailable` and `updateSlave` are kept separate because
  (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
  (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
  (3) `updateAvailable` never leaves the allocator in an over-allocated state 
and must not, whereas `updateSlave` does, and can.
* The algorithm:
* Initially, the master pessimistically assume that what seems like 
available resources will be gone.
  This is due to the race between the allocator scheduling an `allocate` 
call to itself vs master's `allocator-updateAvailable` invocation.
  As such, we first try to satisfy the request only with the offered 
resources.
* We greedily rescind one offer at a time until we've rescinded 
sufficiently many offers.
  IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
`recoverResources(..., None())` so that we can pretty much always win the race 
against `allocate`.
 In the case that we lose, no disaster occurs. We simply fail 
to satisfy the request.
* If we still don't have enough resources after resciding all offers, be 
optimistic and forward the request to the allocator since there may be 
available resources to satisfy the request.
* If the allocator returns a failure, report the error to the user with 
`PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
maybe as well. We'll pick one eventually.

This approach is clearly not ideal, since we would prefer to rescind as little 
offers as possible.
The challenges of implementing the ideal solution in the current state is 
described in the document above.

TODO(mpark): Add more comments and test cases.


Diffs (updated)
-

  src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f 
  src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc 
  src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 
  src/master/validation.hpp 43b8d84556e7f0a891dddf6185bbce7ca50b360a 
  src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 

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


Testing
---

`make check`


Thanks,

Michael Park



Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-31 Thread Michael Park


 On July 13, 2015, 4:46 p.m., Alexander Rukletsov wrote:
  src/master/http.cpp, line 507
  https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line507
 
  The code until this line is basically request validation and 
  authorization. Though it's not how we do it now, do you think it makes 
  sense to split the function into smaller logical parts?
  
  How about something like this:
  
  ```
  FutureResponse Master::Http::reserve(const Request request) const
  {
return Master::Http::reserveValidate();
  }
  
  FutureResponse Master::Http::reserveValidate(const Request request) 
  const
  {
...
return Master::Http::reserveAuthorize();
  }
  
  ...
  ```
 
 Michael Park wrote:
 Yeah, I think it does make sense to break huge functions down to the 
 smaller logical pieces. I think we can do a more general refactoring for the 
 validation pattern, since they all pretty much do the same thing. But I think 
 we can consider doing that uniformly, outside of this patch. What do you 
 think?
 
 Alexander Rukletsov wrote:
 I personally prefer sacrificing consistency, but write new code right. 
 However, generally we tend to favour consistency over local improvements, so 
 feel free to fix the issue by creating a JIRA : ).

I've filed a JIRA ticket for this here: 
https://issues.apache.org/jira/browse/MESOS-3186


- Michael


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


On July 31, 2015, 9:56 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated July 31, 2015, 9:56 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and forward the request to the allocator since there may be 
 available resources to satisfy the request.
 * If the allocator returns a failure, report the error to the user with 
 `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
 maybe as well. We'll pick one eventually.
 
 This approach is clearly not ideal, since we would prefer to rescind as 
 little offers as possible.
 The challenges of implementing the ideal solution in the current state is 
 described in the document above.
 
 TODO(mpark): Add more comments and test cases.
 
 
 Diffs
 -
 
   src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f 
   src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc 
   src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 
   src/master/validation.hpp 43b8d84556e7f0a891dddf6185bbce7ca50b360a 
   src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
 
 Diff: https://reviews.apache.org/r/35702/diff/
 
 
 Testing
 ---
 
 `make check`
 
 
 Thanks,
 
 Michael Park
 




Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-29 Thread Alexander Rukletsov


 On July 13, 2015, 4:46 p.m., Alexander Rukletsov wrote:
  src/master/http.cpp, lines 515-516
  https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line515
 
  It looks like we actually have the role, but it's buried in resources. 
  Do you envision having resources collection with various roles in one 
  request? Maybe it makes sense to add a validation step which ensures there 
  is just one role per request and use it here, also avoiding changes in the 
  `validate()`function.
 
 Michael Park wrote:
 I didn't see a good reason to require a one role per request condition. 
 The current interface accurately models the fact that an operator does not 
 have a role associated to it like a framework does, and I don't think 
 avoiding changes in the `validate()` function should have any influence in 
 deciding how an interface behaves.
 
 If we required such a condition, the per-request atomicity guarantee 
 comes with a limitation that it can only be for a single role. While I'm not 
 sure of its value, I'm also not sure what we gain by requiring it from the 
 user's perspective?
 
 Alexander Rukletsov wrote:
 I think I'm missing something, my understanding is that each dynamic 
 reservation is associated with a role, regardless, who issues a reservation 
 request. I don't think limiting users to one role per request gives them any 
 benefit, but it looks like we can be closer to framework-issued request if we 
 do so. What am I missing?
 
 Michael Park wrote:
 Your understanding is correct. Aside from the resources being associated 
 with a role, frameworks are also associated with a role. We check that every 
 resource is being reserved for the framework's role because a framework is 
 associated with a role and it wouldn't make sense to allow a framework to 
 reserve resources for a role that does not match its role. On the contrary, 
 the same rule doesn't apply for an operator since there's no such thing as an 
 operator's role.
 
 Alexander Rukletsov wrote:
 That's right. Let me try to reformulate my proposal. If we require an 
 operator to reserve resources for one role per request, it can be interpreted 
 as an operator role. An advantage here is that `validate()` method doesn't 
 need to be changed, while a disadvantage is that this approach is a bit 
 artificial and can lead to confusion. What do you think?
 
 Michael Park wrote:
 As I mentioned in my initial comment, I think that the `validate()` 
 function is an implementation detail and shouldn't be a source of motivation 
 in deciding how an API should behave.

Fair enough, feel free to drop.


- Alexander


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


On July 28, 2015, 9:03 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated July 28, 2015, 9:03 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have 

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-29 Thread Michael Park


 On July 13, 2015, 4:46 p.m., Alexander Rukletsov wrote:
  src/master/http.cpp, lines 515-516
  https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line515
 
  It looks like we actually have the role, but it's buried in resources. 
  Do you envision having resources collection with various roles in one 
  request? Maybe it makes sense to add a validation step which ensures there 
  is just one role per request and use it here, also avoiding changes in the 
  `validate()`function.
 
 Michael Park wrote:
 I didn't see a good reason to require a one role per request condition. 
 The current interface accurately models the fact that an operator does not 
 have a role associated to it like a framework does, and I don't think 
 avoiding changes in the `validate()` function should have any influence in 
 deciding how an interface behaves.
 
 If we required such a condition, the per-request atomicity guarantee 
 comes with a limitation that it can only be for a single role. While I'm not 
 sure of its value, I'm also not sure what we gain by requiring it from the 
 user's perspective?
 
 Alexander Rukletsov wrote:
 I think I'm missing something, my understanding is that each dynamic 
 reservation is associated with a role, regardless, who issues a reservation 
 request. I don't think limiting users to one role per request gives them any 
 benefit, but it looks like we can be closer to framework-issued request if we 
 do so. What am I missing?
 
 Michael Park wrote:
 Your understanding is correct. Aside from the resources being associated 
 with a role, frameworks are also associated with a role. We check that every 
 resource is being reserved for the framework's role because a framework is 
 associated with a role and it wouldn't make sense to allow a framework to 
 reserve resources for a role that does not match its role. On the contrary, 
 the same rule doesn't apply for an operator since there's no such thing as an 
 operator's role.
 
 Alexander Rukletsov wrote:
 That's right. Let me try to reformulate my proposal. If we require an 
 operator to reserve resources for one role per request, it can be interpreted 
 as an operator role. An advantage here is that `validate()` method doesn't 
 need to be changed, while a disadvantage is that this approach is a bit 
 artificial and can lead to confusion. What do you think?

As I mentioned in my initial comment, I think that the `validate()` function is 
an implementation detail and shouldn't be a source of motivation in deciding 
how an API should behave.


- Michael


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


On July 28, 2015, 9:03 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated July 28, 2015, 9:03 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and 

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-29 Thread Alexander Rukletsov


 On June 22, 2015, 1:32 p.m., Alexander Rukletsov wrote:
  src/master/master.cpp, line 749
  https://reviews.apache.org/r/35702/diff/6/?file=989449#file989449line749
 
  I think reserve is too abstract and may collide with future actions 
  (think quota). How about `/dynamic/reserve`?
 
 Alexander Rukletsov wrote:
 Though we currently do not support slashes in endpoints, I think we 
 should fix that first before introducing a `/reserve` endpoint, given these 
 endpoint are not targeted for 0.23.
 
 Joris Van Remoortere wrote:
 Cody had some patches for enabling sub namespaces in endpoints (as in 
 enabling slashes). Might be worth pulling those in.
 
 Alexander Rukletsov wrote:
 Yep, it's https://issues.apache.org/jira/browse/MESOS-2130, I plan to 
 bring up the discussion today at the community sync.
 
 Michael Park wrote:
 The concensus for now seems that (1) we introduce the allocator changes, 
 but address the allocator refactor sooner rather than later, (2) go with 
 `/reserve` for now and update them once the HTTP API folks get to supporting 
 the nested endpoint stuff.
 
 Alexander Rukletsov wrote:
 And (3) we update endpoints names before the following release, i.e. 
 there is no Mesos release, where `/reserve` will exist. Correct?
 
 Michael Park wrote:
 That is the ideal outcome. But if we commit this now/soon, whether we can 
 update the names before 0.24.0 gets out entirely depends on whether the 
 nested endpoint names capabilities get committed on time.

I think we agreed that we should, in order to avoid deprecation cycle for old 
endpoints.


- Alexander


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


On July 28, 2015, 9:03 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated July 28, 2015, 9:03 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and forward the request to the allocator since there may be 
 available resources to satisfy the request.
 * If the allocator returns a failure, report the error to the user with 
 `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
 maybe as well. We'll pick one eventually.
 
 This approach is clearly not ideal, since we would prefer to rescind as 
 little offers as possible.
 The challenges of implementing the ideal solution in the current state is 
 described in the document above.
 
 TODO(mpark): Add more comments and test cases.
 
 
 Diffs
 -
 
   src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
   src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
   src/master/master.cpp 5b5e3c37d4433c8524db267866aebc0a35a181f1 
   src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
   src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 
 
 Diff: 

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-29 Thread Alexander Rukletsov


 On July 13, 2015, 4:46 p.m., Alexander Rukletsov wrote:
  src/master/http.cpp, lines 515-516
  https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line515
 
  It looks like we actually have the role, but it's buried in resources. 
  Do you envision having resources collection with various roles in one 
  request? Maybe it makes sense to add a validation step which ensures there 
  is just one role per request and use it here, also avoiding changes in the 
  `validate()`function.
 
 Michael Park wrote:
 I didn't see a good reason to require a one role per request condition. 
 The current interface accurately models the fact that an operator does not 
 have a role associated to it like a framework does, and I don't think 
 avoiding changes in the `validate()` function should have any influence in 
 deciding how an interface behaves.
 
 If we required such a condition, the per-request atomicity guarantee 
 comes with a limitation that it can only be for a single role. While I'm not 
 sure of its value, I'm also not sure what we gain by requiring it from the 
 user's perspective?
 
 Alexander Rukletsov wrote:
 I think I'm missing something, my understanding is that each dynamic 
 reservation is associated with a role, regardless, who issues a reservation 
 request. I don't think limiting users to one role per request gives them any 
 benefit, but it looks like we can be closer to framework-issued request if we 
 do so. What am I missing?
 
 Michael Park wrote:
 Your understanding is correct. Aside from the resources being associated 
 with a role, frameworks are also associated with a role. We check that every 
 resource is being reserved for the framework's role because a framework is 
 associated with a role and it wouldn't make sense to allow a framework to 
 reserve resources for a role that does not match its role. On the contrary, 
 the same rule doesn't apply for an operator since there's no such thing as an 
 operator's role.

That's right. Let me try to reformulate my proposal. If we require an operator 
to reserve resources for one role per request, it can be interpreted as an 
operator role. An advantage here is that `validate()` method doesn't need to 
be changed, while a disadvantage is that this approach is a bit artificial and 
can lead to confusion. What do you think?


- Alexander


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


On July 28, 2015, 9:03 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated July 28, 2015, 9:03 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and forward the request to the allocator since there may be 
 available resources to satisfy the request.
 * If the allocator returns a failure, report the error to the user with 
 `PreconditionFailed`. This could be updated 

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-28 Thread Alexander Rukletsov


 On July 13, 2015, 4:46 p.m., Alexander Rukletsov wrote:
  src/master/http.cpp, line 507
  https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line507
 
  The code until this line is basically request validation and 
  authorization. Though it's not how we do it now, do you think it makes 
  sense to split the function into smaller logical parts?
  
  How about something like this:
  
  ```
  FutureResponse Master::Http::reserve(const Request request) const
  {
return Master::Http::reserveValidate();
  }
  
  FutureResponse Master::Http::reserveValidate(const Request request) 
  const
  {
...
return Master::Http::reserveAuthorize();
  }
  
  ...
  ```
 
 Michael Park wrote:
 Yeah, I think it does make sense to break huge functions down to the 
 smaller logical pieces. I think we can do a more general refactoring for the 
 validation pattern, since they all pretty much do the same thing. But I think 
 we can consider doing that uniformly, outside of this patch. What do you 
 think?

I personally prefer sacrificing consistency, but write new code right. 
However, generally we tend to favour consistency over local improvements, so 
feel free to fix the issue by creating a JIRA : ).


 On July 13, 2015, 4:46 p.m., Alexander Rukletsov wrote:
  src/master/http.cpp, lines 515-516
  https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line515
 
  It looks like we actually have the role, but it's buried in resources. 
  Do you envision having resources collection with various roles in one 
  request? Maybe it makes sense to add a validation step which ensures there 
  is just one role per request and use it here, also avoiding changes in the 
  `validate()`function.
 
 Michael Park wrote:
 I didn't see a good reason to require a one role per request condition. 
 The current interface accurately models the fact that an operator does not 
 have a role associated to it like a framework does, and I don't think 
 avoiding changes in the `validate()` function should have any influence in 
 deciding how an interface behaves.
 
 If we required such a condition, the per-request atomicity guarantee 
 comes with a limitation that it can only be for a single role. While I'm not 
 sure of its value, I'm also not sure what we gain by requiring it from the 
 user's perspective?

I think I'm missing something, my understanding is that each dynamic 
reservation is associated with a role, regardless, who issues a reservation 
request. I don't think limiting users to one role per request gives them any 
benefit, but it looks like we can be closer to framework-issued request if we 
do so. What am I missing?


- Alexander


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


On July 27, 2015, 11:30 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated July 27, 2015, 11:30 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the 

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-28 Thread Alexander Rukletsov


 On June 22, 2015, 1:32 p.m., Alexander Rukletsov wrote:
  src/master/master.cpp, line 749
  https://reviews.apache.org/r/35702/diff/6/?file=989449#file989449line749
 
  I think reserve is too abstract and may collide with future actions 
  (think quota). How about `/dynamic/reserve`?
 
 Alexander Rukletsov wrote:
 Though we currently do not support slashes in endpoints, I think we 
 should fix that first before introducing a `/reserve` endpoint, given these 
 endpoint are not targeted for 0.23.
 
 Joris Van Remoortere wrote:
 Cody had some patches for enabling sub namespaces in endpoints (as in 
 enabling slashes). Might be worth pulling those in.
 
 Alexander Rukletsov wrote:
 Yep, it's https://issues.apache.org/jira/browse/MESOS-2130, I plan to 
 bring up the discussion today at the community sync.
 
 Michael Park wrote:
 The concensus for now seems that (1) we introduce the allocator changes, 
 but address the allocator refactor sooner rather than later, (2) go with 
 `/reserve` for now and update them once the HTTP API folks get to supporting 
 the nested endpoint stuff.

And (3) we update endpoints names before the following release, i.e. there is 
no Mesos release, where `/reserve` will exist. Correct?


- Alexander


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


On July 27, 2015, 11:30 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated July 27, 2015, 11:30 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and forward the request to the allocator since there may be 
 available resources to satisfy the request.
 * If the allocator returns a failure, report the error to the user with 
 `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
 maybe as well. We'll pick one eventually.
 
 This approach is clearly not ideal, since we would prefer to rescind as 
 little offers as possible.
 The challenges of implementing the ideal solution in the current state is 
 described in the document above.
 
 TODO(mpark): Add more comments and test cases.
 
 
 Diffs
 -
 
   src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
   src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
   src/master/master.cpp 5b5e3c37d4433c8524db267866aebc0a35a181f1 
   src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
   src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 
 
 Diff: https://reviews.apache.org/r/35702/diff/
 
 
 Testing
 ---
 
 `make check`
 
 
 Thanks,
 
 Michael Park
 




Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-28 Thread Alexander Rukletsov


 On July 16, 2015, 2:54 p.m., Alexander Rukletsov wrote:
  src/master/http.cpp, line 447
  https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line447
 
  Not directly related to endpoints, but to dynamic reservations in 
  general. Do you think it makes sense to bookkeep dynamic reservation or 
  have an aggregating method in `mesos::internal::master::Role`?
 
 Michael Park wrote:
 We have a `Role::resources()` function which aggregates all resources, 
 and we can filter for dynamically reserved ones by doing something like: 
 `resources.filter(Resources::isDynamicallyReserved)`. Is this sufficient for 
 what you're asking about? or is there more?

Good point! I think this is close to what I had in mind. However, one thing 
still bothers me: how can we hint somebody who is not very familiar with the 
codebase, that they can do tricks like this? Maybe a comment in `Role` struct 
like

```
NOTE: You can use filters to extract specific resources, e.g. 
Role::resources().filter(Resources::isDynamicallyReserved).
```
But maybe it's too much (why putting such comment into the `Role` struct), what 
do you think?


- Alexander


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


On July 27, 2015, 11:30 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated July 27, 2015, 11:30 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and forward the request to the allocator since there may be 
 available resources to satisfy the request.
 * If the allocator returns a failure, report the error to the user with 
 `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
 maybe as well. We'll pick one eventually.
 
 This approach is clearly not ideal, since we would prefer to rescind as 
 little offers as possible.
 The challenges of implementing the ideal solution in the current state is 
 described in the document above.
 
 TODO(mpark): Add more comments and test cases.
 
 
 Diffs
 -
 
   src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
   src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
   src/master/master.cpp 5b5e3c37d4433c8524db267866aebc0a35a181f1 
   src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
   src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 
 
 Diff: https://reviews.apache.org/r/35702/diff/
 
 
 Testing
 ---
 
 `make check`
 
 
 Thanks,
 
 Michael Park
 




Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-28 Thread Michael Park


 On July 13, 2015, 4:46 p.m., Alexander Rukletsov wrote:
  src/master/http.cpp, lines 515-516
  https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line515
 
  It looks like we actually have the role, but it's buried in resources. 
  Do you envision having resources collection with various roles in one 
  request? Maybe it makes sense to add a validation step which ensures there 
  is just one role per request and use it here, also avoiding changes in the 
  `validate()`function.
 
 Michael Park wrote:
 I didn't see a good reason to require a one role per request condition. 
 The current interface accurately models the fact that an operator does not 
 have a role associated to it like a framework does, and I don't think 
 avoiding changes in the `validate()` function should have any influence in 
 deciding how an interface behaves.
 
 If we required such a condition, the per-request atomicity guarantee 
 comes with a limitation that it can only be for a single role. While I'm not 
 sure of its value, I'm also not sure what we gain by requiring it from the 
 user's perspective?
 
 Alexander Rukletsov wrote:
 I think I'm missing something, my understanding is that each dynamic 
 reservation is associated with a role, regardless, who issues a reservation 
 request. I don't think limiting users to one role per request gives them any 
 benefit, but it looks like we can be closer to framework-issued request if we 
 do so. What am I missing?

Your understanding is correct. Aside from the resources being associated with a 
role, frameworks are also associated with a role. We check that every resource 
is being reserved for the framework's role because a framework is associated 
with a role and it wouldn't make sense to allow a framework to reserve 
resources for a role that does not match its role. On the contrary, the same 
rule doesn't apply for an operator since there's no such thing as an 
operator's role.


- Michael


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


On July 27, 2015, 11:30 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated July 27, 2015, 11:30 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and forward the request to the allocator since there may be 
 available resources to satisfy the request.
 * If the allocator returns a failure, report the error to the user with 
 `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
 maybe as well. We'll pick one eventually.
 
 This approach is clearly not ideal, since we would prefer to rescind as 
 little offers as possible.
 The challenges of implementing the ideal solution in the current state is 
 described in the document above.
 
 TODO(mpark): Add more comments and test cases.
 
 
 Diffs
 -
 
   src/master/http.cpp 

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-28 Thread Michael Park


 On July 16, 2015, 2:54 p.m., Alexander Rukletsov wrote:
  src/master/http.cpp, line 447
  https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line447
 
  Not directly related to endpoints, but to dynamic reservations in 
  general. Do you think it makes sense to bookkeep dynamic reservation or 
  have an aggregating method in `mesos::internal::master::Role`?
 
 Michael Park wrote:
 We have a `Role::resources()` function which aggregates all resources, 
 and we can filter for dynamically reserved ones by doing something like: 
 `resources.filter(Resources::isDynamicallyReserved)`. Is this sufficient for 
 what you're asking about? or is there more?
 
 Alexander Rukletsov wrote:
 Good point! I think this is close to what I had in mind. However, one 
 thing still bothers me: how can we hint somebody who is not very familiar 
 with the codebase, that they can do tricks like this? Maybe a comment in 
 `Role` struct like
 
 ```
 NOTE: You can use filters to extract specific resources, e.g. 
 Role::resources().filter(Resources::isDynamicallyReserved).
 ```
 But maybe it's too much (why putting such comment into the `Role` 
 struct), what do you think?

Yeah, I'm not sure about a seemingly unrelated comment living at the `Role` 
struct. But it would be useful if it's something that people ask about often.


- Michael


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


On July 27, 2015, 11:30 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated July 27, 2015, 11:30 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and forward the request to the allocator since there may be 
 available resources to satisfy the request.
 * If the allocator returns a failure, report the error to the user with 
 `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
 maybe as well. We'll pick one eventually.
 
 This approach is clearly not ideal, since we would prefer to rescind as 
 little offers as possible.
 The challenges of implementing the ideal solution in the current state is 
 described in the document above.
 
 TODO(mpark): Add more comments and test cases.
 
 
 Diffs
 -
 
   src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
   src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
   src/master/master.cpp 5b5e3c37d4433c8524db267866aebc0a35a181f1 
   src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
   src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 
 
 Diff: https://reviews.apache.org/r/35702/diff/
 
 
 Testing
 ---
 
 `make check`
 
 
 Thanks,
 
 Michael Park
 




Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-28 Thread Michael Park

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

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


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


Changes
---

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
---

This involved a lot more challenges than I anticipated, I've captured the 
various approaches and limitations and deal-breakers of those approaches here: 
[Master Endpoint Implementation 
Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)

Key points:

* This is a stop-gap solution until we shift the offer creation/management 
logic from the master to the allocator.
* `updateAvailable` and `updateSlave` are kept separate because
  (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
  (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
  (3) `updateAvailable` never leaves the allocator in an over-allocated state 
and must not, whereas `updateSlave` does, and can.
* The algorithm:
* Initially, the master pessimistically assume that what seems like 
available resources will be gone.
  This is due to the race between the allocator scheduling an `allocate` 
call to itself vs master's `allocator-updateAvailable` invocation.
  As such, we first try to satisfy the request only with the offered 
resources.
* We greedily rescind one offer at a time until we've rescinded 
sufficiently many offers.
  IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
`recoverResources(..., None())` so that we can pretty much always win the race 
against `allocate`.
 In the case that we lose, no disaster occurs. We simply fail 
to satisfy the request.
* If we still don't have enough resources after resciding all offers, be 
optimistic and forward the request to the allocator since there may be 
available resources to satisfy the request.
* If the allocator returns a failure, report the error to the user with 
`PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
maybe as well. We'll pick one eventually.

This approach is clearly not ideal, since we would prefer to rescind as little 
offers as possible.
The challenges of implementing the ideal solution in the current state is 
described in the document above.

TODO(mpark): Add more comments and test cases.


Diffs (updated)
-

  src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
  src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
  src/master/master.cpp 5b5e3c37d4433c8524db267866aebc0a35a181f1 
  src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
  src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 

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


Testing
---

`make check`


Thanks,

Michael Park



Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-28 Thread Michael Park


 On June 22, 2015, 1:32 p.m., Alexander Rukletsov wrote:
  src/master/master.cpp, line 749
  https://reviews.apache.org/r/35702/diff/6/?file=989449#file989449line749
 
  I think reserve is too abstract and may collide with future actions 
  (think quota). How about `/dynamic/reserve`?
 
 Alexander Rukletsov wrote:
 Though we currently do not support slashes in endpoints, I think we 
 should fix that first before introducing a `/reserve` endpoint, given these 
 endpoint are not targeted for 0.23.
 
 Joris Van Remoortere wrote:
 Cody had some patches for enabling sub namespaces in endpoints (as in 
 enabling slashes). Might be worth pulling those in.
 
 Alexander Rukletsov wrote:
 Yep, it's https://issues.apache.org/jira/browse/MESOS-2130, I plan to 
 bring up the discussion today at the community sync.
 
 Michael Park wrote:
 The concensus for now seems that (1) we introduce the allocator changes, 
 but address the allocator refactor sooner rather than later, (2) go with 
 `/reserve` for now and update them once the HTTP API folks get to supporting 
 the nested endpoint stuff.
 
 Alexander Rukletsov wrote:
 And (3) we update endpoints names before the following release, i.e. 
 there is no Mesos release, where `/reserve` will exist. Correct?

That is the ideal outcome. But if we commit this now/soon, whether we can 
update the names before 0.24.0 gets out entirely depends on whether the nested 
endpoint names capabilities get committed on time.


- Michael


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


On July 28, 2015, 9:03 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated July 28, 2015, 9:03 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and forward the request to the allocator since there may be 
 available resources to satisfy the request.
 * If the allocator returns a failure, report the error to the user with 
 `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
 maybe as well. We'll pick one eventually.
 
 This approach is clearly not ideal, since we would prefer to rescind as 
 little offers as possible.
 The challenges of implementing the ideal solution in the current state is 
 described in the document above.
 
 TODO(mpark): Add more comments and test cases.
 
 
 Diffs
 -
 
   src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
   src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
   src/master/master.cpp 5b5e3c37d4433c8524db267866aebc0a35a181f1 
   src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
   src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 
 
 Diff: https://reviews.apache.org/r/35702/diff/
 
 
 Testing
 ---
 
 `make check`
 
 
 Thanks,
 
 Michael Park
 




Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-27 Thread Michael Park


 On July 13, 2015, 4:46 p.m., Alexander Rukletsov wrote:
  A high level question: do you think rescinding offers is a big deal for now?

I don't believe it's a big deal for now because frameworks need to deal with 
rescinded offers regardless, and I imagine the frequency of operators using 
these endpoints will probably be low. Of course we'll monitor whether these 
expectations are true as we go forward, but I think for now it's ok.


 On July 13, 2015, 4:46 p.m., Alexander Rukletsov wrote:
  src/master/http.cpp, line 507
  https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line507
 
  The code until this line is basically request validation and 
  authorization. Though it's not how we do it now, do you think it makes 
  sense to split the function into smaller logical parts?
  
  How about something like this:
  
  ```
  FutureResponse Master::Http::reserve(const Request request) const
  {
return Master::Http::reserveValidate();
  }
  
  FutureResponse Master::Http::reserveValidate(const Request request) 
  const
  {
...
return Master::Http::reserveAuthorize();
  }
  
  ...
  ```

Yeah, I think it does make sense to break huge functions down to the smaller 
logical pieces. I think we can do a more general refactoring for the validation 
pattern, since they all pretty much do the same thing. But I think we can 
consider doing that uniformly, outside of this patch. What do you think?


 On July 13, 2015, 4:46 p.m., Alexander Rukletsov wrote:
  src/master/http.cpp, lines 515-516
  https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line515
 
  It looks like we actually have the role, but it's buried in resources. 
  Do you envision having resources collection with various roles in one 
  request? Maybe it makes sense to add a validation step which ensures there 
  is just one role per request and use it here, also avoiding changes in the 
  `validate()`function.

I didn't see a good reason to require a one role per request condition. The 
current interface accurately models the fact that an operator does not have a 
role associated to it like a framework does, and I don't think avoiding 
changes in the `validate()` function should have any influence in deciding how 
an interface behaves.

If we required such a condition, the per-request atomicity guarantee comes with 
a limitation that it can only be for a single role. While I'm not sure of its 
value, I'm also not sure what we gain by requiring it from the user's 
perspective?


- Michael


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


On June 28, 2015, 8:36 a.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated June 28, 2015, 8:36 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and 

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-27 Thread Michael Park

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

(Updated July 27, 2015, 11:30 p.m.)


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


Changes
---

Addressed AlexR's comments.


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


Repository: mesos


Description
---

This involved a lot more challenges than I anticipated, I've captured the 
various approaches and limitations and deal-breakers of those approaches here: 
[Master Endpoint Implementation 
Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)

Key points:

* This is a stop-gap solution until we shift the offer creation/management 
logic from the master to the allocator.
* `updateAvailable` and `updateSlave` are kept separate because
  (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
  (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
  (3) `updateAvailable` never leaves the allocator in an over-allocated state 
and must not, whereas `updateSlave` does, and can.
* The algorithm:
* Initially, the master pessimistically assume that what seems like 
available resources will be gone.
  This is due to the race between the allocator scheduling an `allocate` 
call to itself vs master's `allocator-updateAvailable` invocation.
  As such, we first try to satisfy the request only with the offered 
resources.
* We greedily rescind one offer at a time until we've rescinded 
sufficiently many offers.
  IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
`recoverResources(..., None())` so that we can pretty much always win the race 
against `allocate`.
 In the case that we lose, no disaster occurs. We simply fail 
to satisfy the request.
* If we still don't have enough resources after resciding all offers, be 
optimistic and forward the request to the allocator since there may be 
available resources to satisfy the request.
* If the allocator returns a failure, report the error to the user with 
`PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
maybe as well. We'll pick one eventually.

This approach is clearly not ideal, since we would prefer to rescind as little 
offers as possible.
The challenges of implementing the ideal solution in the current state is 
described in the document above.

TODO(mpark): Add more comments and test cases.


Diffs (updated)
-

  src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
  src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
  src/master/master.cpp 5b5e3c37d4433c8524db267866aebc0a35a181f1 
  src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
  src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 

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


Testing
---

`make check`


Thanks,

Michael Park



Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-27 Thread Michael Park


 On July 16, 2015, 2:54 p.m., Alexander Rukletsov wrote:
  src/master/http.cpp, line 447
  https://reviews.apache.org/r/35702/diff/9/?file=994080#file994080line447
 
  Not directly related to endpoints, but to dynamic reservations in 
  general. Do you think it makes sense to bookkeep dynamic reservation or 
  have an aggregating method in `mesos::internal::master::Role`?

We have a `Role::resources()` function which aggregates all resources, and we 
can filter for dynamically reserved ones by doing something like: 
`resources.filter(Resources::isDynamicallyReserved)`. Is this sufficient for 
what you're asking about? or is there more?


- Michael


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


On July 27, 2015, 11:30 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated July 27, 2015, 11:30 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and forward the request to the allocator since there may be 
 available resources to satisfy the request.
 * If the allocator returns a failure, report the error to the user with 
 `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
 maybe as well. We'll pick one eventually.
 
 This approach is clearly not ideal, since we would prefer to rescind as 
 little offers as possible.
 The challenges of implementing the ideal solution in the current state is 
 described in the document above.
 
 TODO(mpark): Add more comments and test cases.
 
 
 Diffs
 -
 
   src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
   src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
   src/master/master.cpp 5b5e3c37d4433c8524db267866aebc0a35a181f1 
   src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
   src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 
 
 Diff: https://reviews.apache.org/r/35702/diff/
 
 
 Testing
 ---
 
 `make check`
 
 
 Thanks,
 
 Michael Park
 




Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-24 Thread Michael Park


 On June 22, 2015, 1:32 p.m., Alexander Rukletsov wrote:
  src/master/master.cpp, line 749
  https://reviews.apache.org/r/35702/diff/6/?file=989449#file989449line749
 
  I think reserve is too abstract and may collide with future actions 
  (think quota). How about `/dynamic/reserve`?
 
 Alexander Rukletsov wrote:
 Though we currently do not support slashes in endpoints, I think we 
 should fix that first before introducing a `/reserve` endpoint, given these 
 endpoint are not targeted for 0.23.
 
 Joris Van Remoortere wrote:
 Cody had some patches for enabling sub namespaces in endpoints (as in 
 enabling slashes). Might be worth pulling those in.
 
 Alexander Rukletsov wrote:
 Yep, it's https://issues.apache.org/jira/browse/MESOS-2130, I plan to 
 bring up the discussion today at the community sync.

The concensus for now seems that (1) we introduce the allocator changes, but 
address the allocator refactor sooner rather than later, (2) go with `/reserve` 
for now and update them once the HTTP API folks get to supporting the nested 
endpoint stuff.


- Michael


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


On June 28, 2015, 8:36 a.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated June 28, 2015, 8:36 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and forward the request to the allocator since there may be 
 available resources to satisfy the request.
 * If the allocator returns a failure, report the error to the user with 
 `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
 maybe as well. We'll pick one eventually.
 
 This approach is clearly not ideal, since we would prefer to rescind as 
 little offers as possible.
 The challenges of implementing the ideal solution in the current state is 
 described in the document above.
 
 TODO(mpark): Add more comments and test cases.
 
 
 Diffs
 -
 
   src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
   src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
   src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
   src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 
 
 Diff: https://reviews.apache.org/r/35702/diff/
 
 
 Testing
 ---
 
 `make check`
 
 
 Thanks,
 
 Michael Park
 




Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-16 Thread Alexander Rukletsov

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



src/master/http.cpp (line 447)
https://reviews.apache.org/r/35702/#comment145598

Not directly related to endpoints, but to dynamic reservations in general. 
Do you think it makes sense to bookkeep dynamic reservation or have an 
aggregating method in `mesos::internal::master::Role`?


- Alexander Rukletsov


On June 28, 2015, 8:36 a.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated June 28, 2015, 8:36 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and forward the request to the allocator since there may be 
 available resources to satisfy the request.
 * If the allocator returns a failure, report the error to the user with 
 `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
 maybe as well. We'll pick one eventually.
 
 This approach is clearly not ideal, since we would prefer to rescind as 
 little offers as possible.
 The challenges of implementing the ideal solution in the current state is 
 described in the document above.
 
 TODO(mpark): Add more comments and test cases.
 
 
 Diffs
 -
 
   src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
   src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
   src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
   src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 
 
 Diff: https://reviews.apache.org/r/35702/diff/
 
 
 Testing
 ---
 
 `make check`
 
 
 Thanks,
 
 Michael Park
 




Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-14 Thread Alexander Rukletsov


 On June 22, 2015, 1:32 p.m., Alexander Rukletsov wrote:
  src/master/master.cpp, line 749
  https://reviews.apache.org/r/35702/diff/6/?file=989449#file989449line749
 
  I think reserve is too abstract and may collide with future actions 
  (think quota). How about `/dynamic/reserve`?
 
 Alexander Rukletsov wrote:
 Though we currently do not support slashes in endpoints, I think we 
 should fix that first before introducing a `/reserve` endpoint, given these 
 endpoint are not targeted for 0.23.
 
 Joris Van Remoortere wrote:
 Cody had some patches for enabling sub namespaces in endpoints (as in 
 enabling slashes). Might be worth pulling those in.

Yep, it's https://issues.apache.org/jira/browse/MESOS-2130, I plan to bring up 
the discussion today at the community sync.


- Alexander


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


On June 28, 2015, 8:36 a.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated June 28, 2015, 8:36 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and forward the request to the allocator since there may be 
 available resources to satisfy the request.
 * If the allocator returns a failure, report the error to the user with 
 `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
 maybe as well. We'll pick one eventually.
 
 This approach is clearly not ideal, since we would prefer to rescind as 
 little offers as possible.
 The challenges of implementing the ideal solution in the current state is 
 described in the document above.
 
 TODO(mpark): Add more comments and test cases.
 
 
 Diffs
 -
 
   src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
   src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
   src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
   src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 
 
 Diff: https://reviews.apache.org/r/35702/diff/
 
 
 Testing
 ---
 
 `make check`
 
 
 Thanks,
 
 Michael Park
 




Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-07-13 Thread Alexander Rukletsov

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


A high level question: do you think rescinding offers is a big deal for now?


src/master/http.cpp (line 507)
https://reviews.apache.org/r/35702/#comment144877

The code until this line is basically request validation and authorization. 
Though it's not how we do it now, do you think it makes sense to split the 
function into smaller logical parts?

How about something like this:

```
FutureResponse Master::Http::reserve(const Request request) const
{
  return Master::Http::reserveValidate();
}

FutureResponse Master::Http::reserveValidate(const Request request) const
{
  ...
  return Master::Http::reserveAuthorize();
}

...
```



src/master/http.cpp (lines 515 - 516)
https://reviews.apache.org/r/35702/#comment144878

It looks like we actually have the role, but it's buried in resources. Do 
you envision having resources collection with various roles in one request? 
Maybe it makes sense to add a validation step which ensures there is just one 
role per request and use it here, also avoiding changes in the 
`validate()`function.



src/master/http.cpp (lines 523 - 524)
https://reviews.apache.org/r/35702/#comment144880

Let's leave a comment here, that we do want to defer the decision 
completely to an allocator, but can do it currently because offers are issued 
and handled by the master.



src/master/http.cpp (lines 541 - 545)
https://reviews.apache.org/r/35702/#comment144879

We basically defer the decision whether request can be granted or not to an 
allocator (up to rescinding). Let's capture it in a comment!



src/master/master.hpp (line 962)
https://reviews.apache.org/r/35702/#comment144851

I see that you extracted this function for code reusal, but let's document 
it. You may want to add a comment for `applyResourceOperation()`and update the 
comment for `applyOfferOperation()` as well.


- Alexander Rukletsov


On June 28, 2015, 8:36 a.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated June 28, 2015, 8:36 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and forward the request to the allocator since there may be 
 available resources to satisfy the request.
 * If the allocator returns a failure, report the error to the user with 
 `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
 maybe as well. We'll pick one eventually.
 
 This approach is clearly not ideal, since we would prefer to rescind as 
 little offers as possible.
 The challenges of implementing the ideal solution in the current state is 
 described in the document above.
 
 TODO(mpark): Add more comments and test cases.
 
 
 Diffs
 -
 
   src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
   

Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-06-28 Thread Michael Park

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

(Updated June 28, 2015, 8:35 a.m.)


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


Changes
---

Split tests out.


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


Repository: mesos


Description
---

This involved a lot more challenges than I anticipated, I've captured the 
various approaches and limitations and deal-breakers of those approaches here: 
[Master Endpoint Implementation 
Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)

Key points:

* This is a stop-gap solution until we shift the offer creation/management 
logic from the master to the allocator.
* `updateAvailable` and `updateSlave` are kept separate because
  (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
  (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
  (3) `updateAvailable` never leaves the allocator in an over-allocated state 
and must not, whereas `updateSlave` does, and can.
* The algorithm:
* Initially, the master pessimistically assume that what seems like 
available resources will be gone.
  This is due to the race between the allocator scheduling an `allocate` 
call to itself vs master's `allocator-updateAvailable` invocation.
  As such, we first try to satisfy the request only with the offered 
resources.
* We greedily rescind one offer at a time until we've rescinded 
sufficiently many offers.
  IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
`recoverResources(..., None())` so that we can pretty much always win the race 
against `allocate`.
 In the case that we lose, no disaster occurs. We simply fail 
to satisfy the request.
* If we still don't have enough resources after resciding all offers, be 
optimistic and forward the request to the allocator since there may be 
available resources to satisfy the request.
* If the allocator returns a failure, report the error to the user with 
`PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
maybe as well. We'll pick one eventually.

This approach is clearly not ideal, since we would prefer to rescind as little 
offers as possible.
The challenges of implementing the ideal solution in the current state is 
described in the document above.

TODO(mpark): Add more comments and test cases.


Diffs (updated)
-

  src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
  src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
  src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
  src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
  src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 

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


Testing
---

(1) Added `src/tests/reserve_tests.cpp`.
(2) `make check`


Thanks,

Michael Park



Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-06-28 Thread Michael Park

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

(Updated June 28, 2015, 8:36 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
---

This involved a lot more challenges than I anticipated, I've captured the 
various approaches and limitations and deal-breakers of those approaches here: 
[Master Endpoint Implementation 
Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)

Key points:

* This is a stop-gap solution until we shift the offer creation/management 
logic from the master to the allocator.
* `updateAvailable` and `updateSlave` are kept separate because
  (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
  (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
  (3) `updateAvailable` never leaves the allocator in an over-allocated state 
and must not, whereas `updateSlave` does, and can.
* The algorithm:
* Initially, the master pessimistically assume that what seems like 
available resources will be gone.
  This is due to the race between the allocator scheduling an `allocate` 
call to itself vs master's `allocator-updateAvailable` invocation.
  As such, we first try to satisfy the request only with the offered 
resources.
* We greedily rescind one offer at a time until we've rescinded 
sufficiently many offers.
  IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
`recoverResources(..., None())` so that we can pretty much always win the race 
against `allocate`.
 In the case that we lose, no disaster occurs. We simply fail 
to satisfy the request.
* If we still don't have enough resources after resciding all offers, be 
optimistic and forward the request to the allocator since there may be 
available resources to satisfy the request.
* If the allocator returns a failure, report the error to the user with 
`PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
maybe as well. We'll pick one eventually.

This approach is clearly not ideal, since we would prefer to rescind as little 
offers as possible.
The challenges of implementing the ideal solution in the current state is 
described in the document above.

TODO(mpark): Add more comments and test cases.


Diffs
-

  src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
  src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
  src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
  src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
  src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 

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


Testing (updated)
---

`make check`


Thanks,

Michael Park



Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-06-26 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [35934, 35939, 35947, 35702]

All tests passed.

- Mesos ReviewBot


On June 26, 2015, 10:56 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35702/
 ---
 
 (Updated June 26, 2015, 10:56 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
 ---
 
 This involved a lot more challenges than I anticipated, I've captured the 
 various approaches and limitations and deal-breakers of those approaches 
 here: [Master Endpoint Implementation 
 Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)
 
 Key points:
 
 * This is a stop-gap solution until we shift the offer creation/management 
 logic from the master to the allocator.
 * `updateAvailable` and `updateSlave` are kept separate because
   (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
   (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
   (3) `updateAvailable` never leaves the allocator in an over-allocated state 
 and must not, whereas `updateSlave` does, and can.
 * The algorithm:
 * Initially, the master pessimistically assume that what seems like 
 available resources will be gone.
   This is due to the race between the allocator scheduling an `allocate` 
 call to itself vs master's `allocator-updateAvailable` invocation.
   As such, we first try to satisfy the request only with the offered 
 resources.
 * We greedily rescind one offer at a time until we've rescinded 
 sufficiently many offers.
   IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
 `recoverResources(..., None())` so that we can pretty much always win the 
 race against `allocate`.
  In the case that we lose, no disaster occurs. We simply fail 
 to satisfy the request.
 * If we still don't have enough resources after resciding all offers, be 
 optimistic and forward the request to the allocator since there may be 
 available resources to satisfy the request.
 * If the allocator returns a failure, report the error to the user with 
 `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
 maybe as well. We'll pick one eventually.
 
 This approach is clearly not ideal, since we would prefer to rescind as 
 little offers as possible.
 The challenges of implementing the ideal solution in the current state is 
 described in the document above.
 
 TODO(mpark): Add more comments and test cases.
 
 
 Diffs
 -
 
   src/Makefile.am a064d17a6b62e6e3c8e190135bcc8cbbb0051ed5 
   src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
   src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
   src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
   src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 
   src/tests/reserve_tests.cpp PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/35702/diff/
 
 
 Testing
 ---
 
 Added `src/tests/reserve_tests.cpp`.
 
 
 Thanks,
 
 Michael Park
 




Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-06-26 Thread Michael Park

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

(Updated June 26, 2015, 10:56 p.m.)


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


Changes
---

Ready for review.


Summary (updated)
-

Added /reserve HTTP endpoint to the master.


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


Repository: mesos


Description
---

This involved a lot more challenges than I anticipated, I've captured the 
various approaches and limitations and deal-breakers of those approaches here: 
[Master Endpoint Implementation 
Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)

Key points:

* This is a stop-gap solution until we shift the offer creation/management 
logic from the master to the allocator.
* `updateAvailable` and `updateSlave` are kept separate because
  (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
  (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
  (3) `updateAvailable` never leaves the allocator in an over-allocated state 
and must not, whereas `updateSlave` does, and can.
* The algorithm:
* Initially, the master pessimistically assume that what seems like 
available resources will be gone.
  This is due to the race between the allocator scheduling an `allocate` 
call to itself vs master's `allocator-updateAvailable` invocation.
  As such, we first try to satisfy the request only with the offered 
resources.
* We greedily rescind one offer at a time until we've rescinded 
sufficiently many offers.
  IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
`recoverResources(..., None())` so that we can pretty much always win the race 
against `allocate`.
 In the case that we lose, no disaster occurs. We simply fail 
to satisfy the request.
* If we still don't have enough resources after resciding all offers, be 
optimistic and forward the request to the allocator since there may be 
available resources to satisfy the request.
* If the allocator returns a failure, report the error to the user with 
`PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
maybe as well. We'll pick one eventually.

This approach is clearly not ideal, since we would prefer to rescind as little 
offers as possible.
The challenges of implementing the ideal solution in the current state is 
described in the document above.

TODO(mpark): Add more comments and test cases.


Diffs
-

  src/Makefile.am a064d17a6b62e6e3c8e190135bcc8cbbb0051ed5 
  src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
  src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
  src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
  src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
  src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 
  src/tests/reserve_tests.cpp PRE-CREATION 

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


Testing
---

Added `src/tests/reserve_tests.cpp`.


Thanks,

Michael Park



Re: Review Request 35702: Added /reserve HTTP endpoint to the master.

2015-06-26 Thread Michael Park

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

(Updated June 27, 2015, 2:23 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
---

This involved a lot more challenges than I anticipated, I've captured the 
various approaches and limitations and deal-breakers of those approaches here: 
[Master Endpoint Implementation 
Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#)

Key points:

* This is a stop-gap solution until we shift the offer creation/management 
logic from the master to the allocator.
* `updateAvailable` and `updateSlave` are kept separate because
  (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not.
  (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not.
  (3) `updateAvailable` never leaves the allocator in an over-allocated state 
and must not, whereas `updateSlave` does, and can.
* The algorithm:
* Initially, the master pessimistically assume that what seems like 
available resources will be gone.
  This is due to the race between the allocator scheduling an `allocate` 
call to itself vs master's `allocator-updateAvailable` invocation.
  As such, we first try to satisfy the request only with the offered 
resources.
* We greedily rescind one offer at a time until we've rescinded 
sufficiently many offers.
  IMPORTANT: We perform `recoverResources(..., Filters())` rather than 
`recoverResources(..., None())` so that we can pretty much always win the race 
against `allocate`.
 In the case that we lose, no disaster occurs. We simply fail 
to satisfy the request.
* If we still don't have enough resources after resciding all offers, be 
optimistic and forward the request to the allocator since there may be 
available resources to satisfy the request.
* If the allocator returns a failure, report the error to the user with 
`PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` 
maybe as well. We'll pick one eventually.

This approach is clearly not ideal, since we would prefer to rescind as little 
offers as possible.
The challenges of implementing the ideal solution in the current state is 
described in the document above.

TODO(mpark): Add more comments and test cases.


Diffs
-

  src/Makefile.am a064d17a6b62e6e3c8e190135bcc8cbbb0051ed5 
  src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
  src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
  src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
  src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 
  src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 
  src/tests/reserve_tests.cpp PRE-CREATION 

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


Testing (updated)
---

(1) Added `src/tests/reserve_tests.cpp`.
(2) `make check`


Thanks,

Michael Park