Re: Review Request 40244: Clarified comments in Master::Http::_operation.

2015-11-23 Thread Neil Conway 

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

(Updated Nov. 23, 2015, 11:45 a.m.)


Review request for mesos, Greg Mann and Michael Park.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Clarified comments in Master::Http::_operation.


Diffs (updated)
-

  src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 40244: Clarified comments in Master::Http::_operation.

2015-11-19 Thread Qian Zhang 


> On Nov. 18, 2015, 4:05 p.m., Qian Zhang wrote:
> > src/master/http.cpp, line 2097
> > 
> >
> > This comment may not be related to this patch. I am just curious, for 
> > ```required```, we have called ```flatten()``` to remove role from it when 
> > passing it into ```Master::Http::_operation()```. But here for 
> > ```offer->resources()```, we do not call ```flatten()``` to remove its 
> > role, so is it subtractable between ```required``` and 
> > ```offer->resources()```?
> 
> Michael Park wrote:
> We `flatten` the resources for `/reserve` because the `required` 
> represents the resources that we need in order to perform the reservation.
> Suppose we want to end up with 2 reserved cpus. We `flatten` it, and 
> search for 2 unreserved cpus in the received offers.
> If we were to `flatten` the `offer.resources()` also, then an offer with 
> 1 reserved cpu and 1 unreserved cpu would incorrectly satisfy our condition.
> 
> In terms of subtractibility, that is determined on a per-resource basis, 
> and non-subtractable resources are ignored.
> For example, `[(2 unreserved cpus)] - [(1 reserved cpu), (1 unreserved 
> cpu)]` == `[(1 unreserved cpu)]`.

Suppose ```offer.resources()``` is ```[(1 reserved cpu), (1 unreserved 
cpu)]```, and ```required``` is ```[(2 unreserved cpus)]```, so the result of 
```required == required - offer.resources()``` is ```false```, that mean we 
will rescind this offer, right? But it may not make sense to me, because I 
think the **reserved** resource should not be rescinded.


- Qian


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


On Nov. 18, 2015, 7:27 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40244/
> ---
> 
> (Updated Nov. 18, 2015, 7:27 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Bugs: MESOS-2455
> https://issues.apache.org/jira/browse/MESOS-2455
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Clarified comments in Master::Http::_operation.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 
> 
> Diff: https://reviews.apache.org/r/40244/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 40244: Clarified comments in Master::Http::_operation.

2015-11-19 Thread Michael Park 


> On Nov. 18, 2015, 8:05 a.m., Qian Zhang wrote:
> > src/master/http.cpp, line 2097
> > 
> >
> > This comment may not be related to this patch. I am just curious, for 
> > ```required```, we have called ```flatten()``` to remove role from it when 
> > passing it into ```Master::Http::_operation()```. But here for 
> > ```offer->resources()```, we do not call ```flatten()``` to remove its 
> > role, so is it subtractable between ```required``` and 
> > ```offer->resources()```?
> 
> Michael Park wrote:
> We `flatten` the resources for `/reserve` because the `required` 
> represents the resources that we need in order to perform the reservation.
> Suppose we want to end up with 2 reserved cpus. We `flatten` it, and 
> search for 2 unreserved cpus in the received offers.
> If we were to `flatten` the `offer.resources()` also, then an offer with 
> 1 reserved cpu and 1 unreserved cpu would incorrectly satisfy our condition.
> 
> In terms of subtractibility, that is determined on a per-resource basis, 
> and non-subtractable resources are ignored.
> For example, `[(2 unreserved cpus)] - [(1 reserved cpu), (1 unreserved 
> cpu)]` == `[(1 unreserved cpu)]`.
> 
> Qian Zhang wrote:
> Suppose ```offer.resources()``` is ```[(1 reserved cpu), (1 unreserved 
> cpu)]```, and ```required``` is ```[(2 unreserved cpus)]```, so the result of 
> ```required == required - offer.resources()``` is ```false```, that mean we 
> will rescind this offer, right? But it may not make sense to me, because I 
> think the **reserved** resource should not be rescinded.

Yes, it's clear that this approach has limitations. It was just the best known 
approach given the current state of things since we cannot currently rescind 
offers partially. While rescinding the __reserved__ resource is undesirable, it 
still meets the requirements as the resource will still be offered to the role.


- Michael


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


On Nov. 18, 2015, 11:27 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40244/
> ---
> 
> (Updated Nov. 18, 2015, 11:27 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Bugs: MESOS-2455
> https://issues.apache.org/jira/browse/MESOS-2455
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Clarified comments in Master::Http::_operation.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 
> 
> Diff: https://reviews.apache.org/r/40244/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 40244: Clarified comments in Master::Http::_operation.

2015-11-19 Thread Michael Park 


> On Nov. 18, 2015, 8:05 a.m., Qian Zhang wrote:
> > src/master/http.cpp, line 2097
> > 
> >
> > This comment may not be related to this patch. I am just curious, for 
> > ```required```, we have called ```flatten()``` to remove role from it when 
> > passing it into ```Master::Http::_operation()```. But here for 
> > ```offer->resources()```, we do not call ```flatten()``` to remove its 
> > role, so is it subtractable between ```required``` and 
> > ```offer->resources()```?

We `flatten` the resources for `/reserve` because the `required` represents the 
resources that we need in order to perform the reservation.
Suppose we want to end up with 2 reserved cpus. We `flatten` it, and search for 
2 unreserved cpus in the received offers.
If we were to `flatten` the `offer.resources()` also, then an offer with 1 
reserved cpu and 1 unreserved cpu would incorrectly satisfy our condition.

In terms of subtractibility, that is determined on a per-resource basis, and 
non-subtractable resources are ignored.
For example, `[(2 unreserved cpus)] - [(1 reserved cpu), (1 unreserved cpu)]` 
== `[(1 unreserved cpu)]`.


- Michael


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


On Nov. 18, 2015, 11:27 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40244/
> ---
> 
> (Updated Nov. 18, 2015, 11:27 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Bugs: MESOS-2455
> https://issues.apache.org/jira/browse/MESOS-2455
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Clarified comments in Master::Http::_operation.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 
> 
> Diff: https://reviews.apache.org/r/40244/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 40244: Clarified comments in Master::Http::_operation.

2015-11-18 Thread Neil Conway 

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

(Updated Nov. 18, 2015, 11:27 a.m.)


Review request for mesos, Greg Mann and Michael Park.


Changes
---

Address mpark's comments.


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


Repository: mesos


Description
---

Clarified comments in Master::Http::_operation.


Diffs (updated)
-

  src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 40244: Clarified comments in Master::Http::_operation.

2015-11-18 Thread Qian Zhang 

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



src/master/http.cpp (line 2096)


This comment may not be related to this patch. I am just curious, for 
```required```, we have called ```flatten()``` to remove role from it when 
passing it into ```Master::Http::_operation()```. But here for 
```offer->resources()```, we do not call ```flatten()``` to remove its role, so 
is it subtractable between ```required``` and ```offer->resources()```?


- Qian Zhang


On Nov. 13, 2015, 8:45 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40244/
> ---
> 
> (Updated Nov. 13, 2015, 8:45 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Bugs: MESOS-2455
> https://issues.apache.org/jira/browse/MESOS-2455
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Clarified comments in Master::Http::_operation.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp f4ec23d74e203b2a8f2af187f0e56fbde7d9b3e5 
> 
> Diff: https://reviews.apache.org/r/40244/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 40244: Clarified comments in Master::Http::_operation.

2015-11-17 Thread Michael Park 

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

Ship it!



src/master/http.cpp 


Restore new line



src/master/http.cpp (line 2095)


`s/remaining/required/`


- Michael Park


On Nov. 13, 2015, 12:45 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40244/
> ---
> 
> (Updated Nov. 13, 2015, 12:45 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Bugs: MESOS-2455
> https://issues.apache.org/jira/browse/MESOS-2455
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Clarified comments in Master::Http::_operation.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp f4ec23d74e203b2a8f2af187f0e56fbde7d9b3e5 
> 
> Diff: https://reviews.apache.org/r/40244/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 40244: Clarified comments in Master::Http::_operation.

2015-11-13 Thread Neil Conway 

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

(Updated Nov. 13, 2015, 12:45 p.m.)


Review request for mesos, Greg Mann and Michael Park.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Clarified comments in Master::Http::_operation.


Diffs (updated)
-

  src/master/http.cpp f4ec23d74e203b2a8f2af187f0e56fbde7d9b3e5 

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 40244: Clarified comments in Master::Http::_operation.

2015-11-12 Thread Guangya Liu 

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

Ship it!


Ship It!

- Guangya Liu


On 十一月 12, 2015, 5:59 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40244/
> ---
> 
> (Updated 十一月 12, 2015, 5:59 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Bugs: MESOS-2455
> https://issues.apache.org/jira/browse/MESOS-2455
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Clarified comments in Master::Http::_operation.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b0bec97ee69413bb70c2673c4ae49e74988796bf 
> 
> Diff: https://reviews.apache.org/r/40244/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>