You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Michael Park <mc...@gmail.com> on 2015/03/03 08:27:32 UTC

Review Request 31665: Updated master::Framework::usedResources and master::Framework::offerResources from Resources to hashmap.

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

Review request for mesos, Alexander Rukletsov and Ben Mahler.


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


Repository: mesos


Description
-------

`master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offerResources`. Both of these are aggregates of resources from multiple slaves and therefore are updated to be `hashmap<SlaveID, Resources>` instead.

There are 3 places where these variables get propagated:

(1) `allocator->addFramework(framework->id, framework->info, framework->usedResources)`
(2) `src/master/http.cpp`: exposes them to `state.json`.
(3) `master::Role::resources()`: needs to return `hashmap<SlaveID, Resources>` instead.

For (3) we can simply change the function signature since we only use it once in `http.cpp` and nowhere else.
For (1) and (2), we use the `sum(resources.values())` pattern to match the existing API in the other components.


Diffs
-----

  src/master/http.cpp 117c0ee720a60a1d8a25359028bad803f1fc2b07 
  src/master/master.hpp ce0e0b3645b099d29c6d59312a91241379d5e80c 
  src/master/master.cpp dae0955d4ab29ea74ea5ab9dd6e19b028c58c6fd 

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


Testing
-------

make check


Thanks,

Michael Park


Re: Review Request 31665: Changed '(used|offered)Resources' to 'total(Used|Offered)Resources' and added 'hashmap (used|offered)Resources' to 'master::Framework'

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31665/#review80528
-----------------------------------------------------------

Ship it!


- Ben Mahler


On April 17, 2015, 5:31 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31665/
> -----------------------------------------------------------
> 
> (Updated April 17, 2015, 5:31 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-2373
>     https://issues.apache.org/jira/browse/MESOS-2373
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offeredResources`. Both of these are aggregates of resources from multiple slaves. We add the `hashmap<SlaveID, Resources>` versions since we can lose non-scalar resources by summing them up across multiple slaves. For further details refer to [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).
> 
> In [r31666](https://reviews.apache.org/r/31666/), we report `usedResourcesBySlaveId` rather than `usedResources` when adding the framework to the allocator.
> 
> We don't actually use `offeredResourcesBySlaveId` currently, but it should be kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as well as `offeredResourcesBySlaveId` via the HTTP endpoint.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp f2b123d7f84eafc792849ad4630c2e3b63be000b 
>   src/master/master.hpp 4a1df58e6756bff7d5f292c0542023a9215e33d8 
>   src/master/master.cpp 02f35ac2d98d288bf9d2b9195770b839bbfe6d7a 
> 
> Diff: https://reviews.apache.org/r/31665/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 31665: Changed '(used|offered)Resources' to 'total(Used|Offered)Resources' and added 'hashmap (used|offered)Resources' to 'master::Framework'

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

(Updated April 17, 2015, 5:31 a.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
-------

(1) `s/usedResources/totalUsedResources/`
(2) `s/offeredResources/totalOfferedResources/`
(3) `s/usedResourcesBySlaveId/usedResources/`
(4) `s/offeredResourcesBySlaveId/offeredResources/`
(5) Added a `NOTE` regarding why keeping totals is safe followed by a `TODO` to address [MESOS-2623](https://issues.apache.org/jira/browse/MESOS-2373).
(6) Added a `TODO` for the call to `allocator->addFramework` for what needs to be done in the next step.


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


Repository: mesos


Description
-------

`master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offeredResources`. Both of these are aggregates of resources from multiple slaves. We add the `hashmap<SlaveID, Resources>` versions since we can lose non-scalar resources by summing them up across multiple slaves. For further details refer to [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).

In [r31666](https://reviews.apache.org/r/31666/), we report `usedResourcesBySlaveId` rather than `usedResources` when adding the framework to the allocator.

We don't actually use `offeredResourcesBySlaveId` currently, but it should be kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as well as `offeredResourcesBySlaveId` via the HTTP endpoint.


Diffs (updated)
-----

  src/master/http.cpp f2b123d7f84eafc792849ad4630c2e3b63be000b 
  src/master/master.hpp 4a1df58e6756bff7d5f292c0542023a9215e33d8 
  src/master/master.cpp 02f35ac2d98d288bf9d2b9195770b839bbfe6d7a 

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


Testing
-------

make check


Thanks,

Michael Park


Re: Review Request 31665: Changed '(used|offered)Resources' to 'total(Used|Offered)Resources' and added 'hashmap (used|offered)Resources' to 'master::Framework'

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

(Updated April 17, 2015, 5:22 a.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Summary (updated)
-----------------

Changed '(used|offered)Resources' to 'total(Used|Offered)Resources' and added 'hashmap<SlaveID, Resources> (used|offered)Resources' to 'master::Framework'


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


Repository: mesos


Description
-------

`master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offeredResources`. Both of these are aggregates of resources from multiple slaves. We add the `hashmap<SlaveID, Resources>` versions since we can lose non-scalar resources by summing them up across multiple slaves. For further details refer to [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).

In [r31666](https://reviews.apache.org/r/31666/), we report `usedResourcesBySlaveId` rather than `usedResources` when adding the framework to the allocator.

We don't actually use `offeredResourcesBySlaveId` currently, but it should be kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as well as `offeredResourcesBySlaveId` via the HTTP endpoint.


Diffs
-----

  src/master/master.hpp 4a1df58e6756bff7d5f292c0542023a9215e33d8 

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


Testing
-------

make check


Thanks,

Michael Park


Re: Review Request 31665: Changed '(used|offered)Resources' to 'total(Used|Offered)Resources' and added 'hashmap (used|offered)Resources' in 'master::Framework'

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

(Updated April 17, 2015, 5:22 a.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Summary (updated)
-----------------

Changed '(used|offered)Resources' to 'total(Used|Offered)Resources' and added 'hashmap<SlaveID, Resources> (used|offered)Resources' in 'master::Framework'


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


Repository: mesos


Description
-------

`master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offeredResources`. Both of these are aggregates of resources from multiple slaves. We add the `hashmap<SlaveID, Resources>` versions since we can lose non-scalar resources by summing them up across multiple slaves. For further details refer to [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).

In [r31666](https://reviews.apache.org/r/31666/), we report `usedResourcesBySlaveId` rather than `usedResources` when adding the framework to the allocator.

We don't actually use `offeredResourcesBySlaveId` currently, but it should be kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as well as `offeredResourcesBySlaveId` via the HTTP endpoint.


Diffs
-----

  src/master/master.hpp 4a1df58e6756bff7d5f292c0542023a9215e33d8 

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


Testing
-------

make check


Thanks,

Michael Park


Re: Review Request 31665: Added 'hashmap (used|offered)ResourcesBySlaveId' to master::Framework.

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

(Updated April 17, 2015, 5:02 a.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


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


Repository: mesos


Description
-------

`master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offeredResources`. Both of these are aggregates of resources from multiple slaves. We add the `hashmap<SlaveID, Resources>` versions since we can lose non-scalar resources by summing them up across multiple slaves. For further details refer to [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).

In [r31666](https://reviews.apache.org/r/31666/), we report `usedResourcesBySlaveId` rather than `usedResources` when adding the framework to the allocator.

We don't actually use `offeredResourcesBySlaveId` currently, but it should be kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as well as `offeredResourcesBySlaveId` via the HTTP endpoint.


Diffs (updated)
-----

  src/master/master.hpp 4a1df58e6756bff7d5f292c0542023a9215e33d8 

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


Testing
-------

make check


Thanks,

Michael Park


Re: Review Request 31665: Added 'hashmap (used|offered)ResourcesBySlaveId' to master::Framework.

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

> On April 16, 2015, 7:27 p.m., Ben Mahler wrote:
> > src/master/master.hpp, line 1071
> > <https://reviews.apache.org/r/31665/diff/6/?file=931105#file931105line1071>
> >
> >     For example, could we document in NOTE on these total variables that subtraction is safe even though non-scalars lose information?
> >     
> >     Specifically, is my understanding correct here: 
> >     
> >     (1) For overlapping set items / ranges across slaves, these will get added N times but only represented 1 time in the Resources. 
> >     (2) So, when an initial subtraction occurs (N-1), the resource is no longer represented. We don't expose non-scalars for now, so this is safe for now.
> >     (3) When any further subtractions occur (N-(1+M)), the Resources simply ignores the subtraction since there's nothing to remove, so this is safe for now.
> 
> Michael Park wrote:
>     I think this was meant as a subsequent comment to the one above, so I'll answer here.
>     
>     I'm not sure if I correctly understand your definition of __safety__ and what's considered a __bug__.
>     > (1) For overlapping set items / ranges across slaves, these will get added N times but only represented 1 time in the Resources.
>     
>     Correct.
>     > (2) So, when an initial subtraction occurs (N-1), the resource is no longer represented.
>     
>     Correct.
>     > We don't expose non-scalars for now, so this is safe for now.
>     
>     We actually do expose non-scalars through the master HTTP endpoint. We used to only expose `cpus`, `mem`, `disk`, and `ports` but since [r31082](https://reviews.apache.org/r/31082) we expose all resources. But even before, `ports` was reported incorrectly. If we mean __safe__ as in we don't crash, then yes it's safe. But I would probably consider this to be at least a __bug__.
>     > (3) When any further subtractions occur (N-(1+M)), the Resources simply ignores the subtraction since there's nothing to remove, so this is safe for now.
>     
>     Correct.
>     
>     After the sequence of patches land, the totals (used and offered) are merely used to be exposed via `state.json`. The non-scalars are still incorrect, but I'm not sure that we can assume that nobody's using them. If we decide to only expose the scalar resources since the non-scalar information was wrong anyway, I think that should definitely be done in a separate patch at least to keep the commits clean (as I mentioned, MESOS-2623 will track that). Furthermore, I think changing what resources get exposed via `state.json` is considered an API change and therefore is probably worthwhile mentioning in the `CHANGELOG` for the next release.

I'll add a `NOTE` for why keeping the totals is __safe__ followed by a `TODO` to remove the non-scalars to avoid reporting incorrect totals.


- Michael


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


On April 15, 2015, 10:45 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31665/
> -----------------------------------------------------------
> 
> (Updated April 15, 2015, 10:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-2373
>     https://issues.apache.org/jira/browse/MESOS-2373
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offeredResources`. Both of these are aggregates of resources from multiple slaves. We add the `hashmap<SlaveID, Resources>` versions since we can lose non-scalar resources by summing them up across multiple slaves. For further details refer to [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).
> 
> In [r31666](https://reviews.apache.org/r/31666/), we report `usedResourcesBySlaveId` rather than `usedResources` when adding the framework to the allocator.
> 
> We don't actually use `offeredResourcesBySlaveId` currently, but it should be kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as well as `offeredResourcesBySlaveId` via the HTTP endpoint.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 
> 
> Diff: https://reviews.apache.org/r/31665/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 31665: Added 'hashmap (used|offered)ResourcesBySlaveId' to master::Framework.

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

> On April 16, 2015, 7:27 p.m., Ben Mahler wrote:
> > src/master/master.hpp, line 1071
> > <https://reviews.apache.org/r/31665/diff/6/?file=931105#file931105line1071>
> >
> >     For example, could we document in NOTE on these total variables that subtraction is safe even though non-scalars lose information?
> >     
> >     Specifically, is my understanding correct here: 
> >     
> >     (1) For overlapping set items / ranges across slaves, these will get added N times but only represented 1 time in the Resources. 
> >     (2) So, when an initial subtraction occurs (N-1), the resource is no longer represented. We don't expose non-scalars for now, so this is safe for now.
> >     (3) When any further subtractions occur (N-(1+M)), the Resources simply ignores the subtraction since there's nothing to remove, so this is safe for now.

I think this was meant as a subsequent comment to the one above, so I'll answer here.

I'm not sure if I correctly understand your definition of __safety__ and what's considered a __bug__.
> (1) For overlapping set items / ranges across slaves, these will get added N times but only represented 1 time in the Resources.

Correct.
> (2) So, when an initial subtraction occurs (N-1), the resource is no longer represented.

Correct.
> We don't expose non-scalars for now, so this is safe for now.

We actually do expose non-scalars through the master HTTP endpoint. We used to only expose `cpus`, `mem`, `disk`, and `ports` but since [r31082](https://reviews.apache.org/r/31082) we expose all resources. But even before, `ports` was reported incorrectly. If we mean __safe__ as in we don't crash, then yes it's safe. But I would probably consider this to be at least a __bug__.
> (3) When any further subtractions occur (N-(1+M)), the Resources simply ignores the subtraction since there's nothing to remove, so this is safe for now.

Correct.

After the sequence of patches land, the totals (used and offered) are merely used to be exposed via `state.json`. The non-scalars are still incorrect, but I'm not sure that we can assume that nobody's using them. If we decide to only expose the scalar resources since the non-scalar information was wrong anyway, I think that should definitely be done in a separate patch at least to keep the commits clean (as I mentioned, MESOS-2623 will track that). Furthermore, I think changing what resources get exposed via `state.json` is considered an API change and therefore is probably worthwhile mentioning in the `CHANGELOG` for the next release.


- Michael


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


On April 15, 2015, 10:45 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31665/
> -----------------------------------------------------------
> 
> (Updated April 15, 2015, 10:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-2373
>     https://issues.apache.org/jira/browse/MESOS-2373
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offeredResources`. Both of these are aggregates of resources from multiple slaves. We add the `hashmap<SlaveID, Resources>` versions since we can lose non-scalar resources by summing them up across multiple slaves. For further details refer to [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).
> 
> In [r31666](https://reviews.apache.org/r/31666/), we report `usedResourcesBySlaveId` rather than `usedResources` when adding the framework to the allocator.
> 
> We don't actually use `offeredResourcesBySlaveId` currently, but it should be kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as well as `offeredResourcesBySlaveId` via the HTTP endpoint.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 
> 
> Diff: https://reviews.apache.org/r/31665/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 31665: Added 'hashmap (used|offered)ResourcesBySlaveId' to master::Framework.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31665/#review80370
-----------------------------------------------------------



src/master/master.hpp
<https://reviews.apache.org/r/31665/#comment130237>

    For example, could we document in NOTE on these total variables that subtraction is safe even though non-scalars lose information?
    
    Specifically, is my understanding correct here: 
    
    (1) For overlapping set items / ranges across slaves, these will get added N times but only represented 1 time in the Resources. 
    (2) So, when an initial subtraction occurs (N-1), the resource is no longer represented. We don't expose non-scalars for now, so this is safe for now.
    (3) When any further subtractions occur (N-(1+M)), the Resources simply ignores the subtraction since there's nothing to remove, so this is safe for now.


- Ben Mahler


On April 15, 2015, 10:45 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31665/
> -----------------------------------------------------------
> 
> (Updated April 15, 2015, 10:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-2373
>     https://issues.apache.org/jira/browse/MESOS-2373
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offeredResources`. Both of these are aggregates of resources from multiple slaves. We add the `hashmap<SlaveID, Resources>` versions since we can lose non-scalar resources by summing them up across multiple slaves. For further details refer to [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).
> 
> In [r31666](https://reviews.apache.org/r/31666/), we report `usedResourcesBySlaveId` rather than `usedResources` when adding the framework to the allocator.
> 
> We don't actually use `offeredResourcesBySlaveId` currently, but it should be kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as well as `offeredResourcesBySlaveId` via the HTTP endpoint.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 
> 
> Diff: https://reviews.apache.org/r/31665/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 31665: Added 'hashmap (used|offered)ResourcesBySlaveId' to master::Framework.

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

(Updated April 15, 2015, 10:45 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
-------

Remove the `SlaveID` from `usedResourcesBySlaveId` if `usedResourcesBySlaveId[slaveId]` is empty.


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


Repository: mesos


Description
-------

`master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offeredResources`. Both of these are aggregates of resources from multiple slaves. We add the `hashmap<SlaveID, Resources>` versions since we can lose non-scalar resources by summing them up across multiple slaves. For further details refer to [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).

In [r31666](https://reviews.apache.org/r/31666/), we report `usedResourcesBySlaveId` rather than `usedResources` when adding the framework to the allocator.

We don't actually use `offeredResourcesBySlaveId` currently, but it should be kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as well as `offeredResourcesBySlaveId` via the HTTP endpoint.


Diffs (updated)
-----

  src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 

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


Testing
-------

make check


Thanks,

Michael Park


Re: Review Request 31665: Added 'hashmap (used|offered)ResourcesBySlaveId' to master::Framework.

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

> On April 15, 2015, 7:20 p.m., Ben Mahler wrote:
> > src/master/master.hpp, lines 1138-1143
> > <https://reviews.apache.org/r/31665/diff/5/?file=927382#file927382line1138>
> >
> >     Thanks for this NOTE! We might want to take this opportunity to articulate why we can safely keep the totals. Or, should we strip non-scalars and store a 'totalUsedScalarResources'?

We can't do the former since we can't safely keep the totals. We might want to do something like the latter but I think we may break some people that way, and therefore should be done as a subsequent task. I've created [MESOS-2623](https://issues.apache.org/jira/browse/MESOS-2623) to track this.


- Michael


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


On April 15, 2015, 10:45 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31665/
> -----------------------------------------------------------
> 
> (Updated April 15, 2015, 10:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-2373
>     https://issues.apache.org/jira/browse/MESOS-2373
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offeredResources`. Both of these are aggregates of resources from multiple slaves. We add the `hashmap<SlaveID, Resources>` versions since we can lose non-scalar resources by summing them up across multiple slaves. For further details refer to [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).
> 
> In [r31666](https://reviews.apache.org/r/31666/), we report `usedResourcesBySlaveId` rather than `usedResources` when adding the framework to the allocator.
> 
> We don't actually use `offeredResourcesBySlaveId` currently, but it should be kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as well as `offeredResourcesBySlaveId` via the HTTP endpoint.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 
> 
> Diff: https://reviews.apache.org/r/31665/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 31665: Added 'hashmap (used|offered)ResourcesBySlaveId' to master::Framework.

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

> On April 15, 2015, 7:20 p.m., Ben Mahler wrote:
> > src/master/master.hpp, lines 1145-1147
> > <https://reviews.apache.org/r/31665/diff/5/?file=927382#file927382line1145>
> >
> >     How about s/usedResources/totalUsedResources/ and s/usedResourcesBySlaveId/usedResources/ ?
> >     
> >     This might also make the "total" referred to in your note a bit clearer. Ditto for offered.
> 
> Michael Park wrote:
>     Yeah, I considered `totalUsedResources` and `usedResources` and I liked those. I went with `usedResources` and `usedResourcesBySlaveId` because I thought exposing `totalUsedResources` via `used_resources`, then having to expose `usedResources` via some other key like `used_resources_by_slave_id` would create confusion later on. This way `usedResources` maps to `used_resources` and `usedResourcesBySlaveId` can map to `used_resources_by_slave_id`. Unless we decide that we're never going to expose `usedResourcesBySlaveId`, in which case we'll have `totalUsedResources` mapped to `used_resources` and that'll be it.
>     
>     What do you think?
> 
> Ben Mahler wrote:
>     That makes sense, although, since we both had the same initial intuition, others may as well. I think it's ok not to allow the JSON format to affect our variable naming, since we want to make sure that the code is as readable / understandable as possible.
>     
>     I like the clarified total and used naming scheme you considered initially because it allows us to make it clear that "used resources" have to be namespaced by slaves to be correctly represented, whereas the total only makes sense for scalars (we can have a note about that). Someone is more likely to ponder this, especially if it's called `'totalUsedScalarResources'`. Feel free to punt on stripping non-scalars if you want to tackle it later!

> I think it's ok not to allow the JSON format to affect our variable naming, since we want to make sure that the code is as readable / understandable as possible.

I agree. To be specific, my worry is that it'll be confusing for `usedResources` to be not exposed as "used_resources".
If you think that wouldn't be a big deal, I'm happy to change the naming scheme. Perhaps we can simply have a `NOTE` to help avoid the confusion.


- Michael


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


On April 15, 2015, 10:45 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31665/
> -----------------------------------------------------------
> 
> (Updated April 15, 2015, 10:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-2373
>     https://issues.apache.org/jira/browse/MESOS-2373
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offeredResources`. Both of these are aggregates of resources from multiple slaves. We add the `hashmap<SlaveID, Resources>` versions since we can lose non-scalar resources by summing them up across multiple slaves. For further details refer to [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).
> 
> In [r31666](https://reviews.apache.org/r/31666/), we report `usedResourcesBySlaveId` rather than `usedResources` when adding the framework to the allocator.
> 
> We don't actually use `offeredResourcesBySlaveId` currently, but it should be kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as well as `offeredResourcesBySlaveId` via the HTTP endpoint.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 
> 
> Diff: https://reviews.apache.org/r/31665/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 31665: Added 'hashmap (used|offered)ResourcesBySlaveId' to master::Framework.

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

> On April 15, 2015, 7:20 p.m., Ben Mahler wrote:
> > src/master/master.hpp, lines 1138-1143
> > <https://reviews.apache.org/r/31665/diff/5/?file=927382#file927382line1138>
> >
> >     Thanks for this NOTE! We might want to take this opportunity to articulate why we can safely keep the totals. Or, should we strip non-scalars and store a 'totalUsedScalarResources'?
> 
> Michael Park wrote:
>     We can't do the former since we can't safely keep the totals. We might want to do something like the latter but I think we may break some people that way, and therefore should be done as a subsequent task. I've created [MESOS-2623](https://issues.apache.org/jira/browse/MESOS-2623) to track this.
> 
> Ben Mahler wrote:
>     > We can't do the former since we can't safely keep the totals.
>     
>     Ah ok great, can we articulate why for posterity? It's not immediately obvious to me. As in, is there a bug due to this? Or is it just that the non-scalar information is not correct, and there's not yet a bug due to this since we only expose scalars? Something else?

Answered to your comment below. In short, if you mean __safe__ as in we don't crash then yes it's safe. There __is__ a bug due to this since the non-scalar information is not correct and we do expose non-scalar resources.


- Michael


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


On April 15, 2015, 10:45 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31665/
> -----------------------------------------------------------
> 
> (Updated April 15, 2015, 10:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-2373
>     https://issues.apache.org/jira/browse/MESOS-2373
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offeredResources`. Both of these are aggregates of resources from multiple slaves. We add the `hashmap<SlaveID, Resources>` versions since we can lose non-scalar resources by summing them up across multiple slaves. For further details refer to [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).
> 
> In [r31666](https://reviews.apache.org/r/31666/), we report `usedResourcesBySlaveId` rather than `usedResources` when adding the framework to the allocator.
> 
> We don't actually use `offeredResourcesBySlaveId` currently, but it should be kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as well as `offeredResourcesBySlaveId` via the HTTP endpoint.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 
> 
> Diff: https://reviews.apache.org/r/31665/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 31665: Added 'hashmap (used|offered)ResourcesBySlaveId' to master::Framework.

Posted by Ben Mahler <be...@gmail.com>.

> On April 15, 2015, 7:20 p.m., Ben Mahler wrote:
> > src/master/master.hpp, lines 1138-1143
> > <https://reviews.apache.org/r/31665/diff/5/?file=927382#file927382line1138>
> >
> >     Thanks for this NOTE! We might want to take this opportunity to articulate why we can safely keep the totals. Or, should we strip non-scalars and store a 'totalUsedScalarResources'?
> 
> Michael Park wrote:
>     We can't do the former since we can't safely keep the totals. We might want to do something like the latter but I think we may break some people that way, and therefore should be done as a subsequent task. I've created [MESOS-2623](https://issues.apache.org/jira/browse/MESOS-2623) to track this.

> We can't do the former since we can't safely keep the totals.

Ah ok great, can we articulate why for posterity? It's not immediately obvious to me. As in, is there a bug due to this? Or is it just that the non-scalar information is not correct, and there's not yet a bug due to this since we only expose scalars? Something else?


> On April 15, 2015, 7:20 p.m., Ben Mahler wrote:
> > src/master/master.hpp, lines 1145-1147
> > <https://reviews.apache.org/r/31665/diff/5/?file=927382#file927382line1145>
> >
> >     How about s/usedResources/totalUsedResources/ and s/usedResourcesBySlaveId/usedResources/ ?
> >     
> >     This might also make the "total" referred to in your note a bit clearer. Ditto for offered.
> 
> Michael Park wrote:
>     Yeah, I considered `totalUsedResources` and `usedResources` and I liked those. I went with `usedResources` and `usedResourcesBySlaveId` because I thought exposing `totalUsedResources` via `used_resources`, then having to expose `usedResources` via some other key like `used_resources_by_slave_id` would create confusion later on. This way `usedResources` maps to `used_resources` and `usedResourcesBySlaveId` can map to `used_resources_by_slave_id`. Unless we decide that we're never going to expose `usedResourcesBySlaveId`, in which case we'll have `totalUsedResources` mapped to `used_resources` and that'll be it.
>     
>     What do you think?

That makes sense, although, since we both had the same initial intuition, others may as well. I think it's ok not to allow the JSON format to affect our variable naming, since we want to make sure that the code is as readable / understandable as possible.

I like the clarified total and used naming scheme you considered initially because it allows us to make it clear that "used resources" have to be namespaced by slaves to be correctly represented, whereas the total only makes sense for scalars (we can have a note about that). Someone is more likely to ponder this, especially if it's called `'totalUsedScalarResources'`. Feel free to punt on stripping non-scalars if you want to tackle it later!


- Ben


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


On April 15, 2015, 10:45 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31665/
> -----------------------------------------------------------
> 
> (Updated April 15, 2015, 10:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-2373
>     https://issues.apache.org/jira/browse/MESOS-2373
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offeredResources`. Both of these are aggregates of resources from multiple slaves. We add the `hashmap<SlaveID, Resources>` versions since we can lose non-scalar resources by summing them up across multiple slaves. For further details refer to [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).
> 
> In [r31666](https://reviews.apache.org/r/31666/), we report `usedResourcesBySlaveId` rather than `usedResources` when adding the framework to the allocator.
> 
> We don't actually use `offeredResourcesBySlaveId` currently, but it should be kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as well as `offeredResourcesBySlaveId` via the HTTP endpoint.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 
> 
> Diff: https://reviews.apache.org/r/31665/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 31665: Added 'hashmap (used|offered)ResourcesBySlaveId' to master::Framework.

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

> On April 15, 2015, 7:20 p.m., Ben Mahler wrote:
> > There are no erase calls on these new maps? When are you removing the slave?

Ah, good catch! I'll add the calls to `erase` when removing resources.


> On April 15, 2015, 7:20 p.m., Ben Mahler wrote:
> > src/master/master.hpp, lines 1145-1147
> > <https://reviews.apache.org/r/31665/diff/5/?file=927382#file927382line1145>
> >
> >     How about s/usedResources/totalUsedResources/ and s/usedResourcesBySlaveId/usedResources/ ?
> >     
> >     This might also make the "total" referred to in your note a bit clearer. Ditto for offered.

Yeah, I considered `totalUsedResources` and `usedResources` and I liked those. I went with `usedResources` and `usedResourcesBySlaveId` because I thought exposing `totalUsedResources` via `used_resources`, then having to expose `usedResources` via some other key like `used_resources_by_slave_id` would create confusion later on. This way `usedResources` maps to `used_resources` and `usedResourcesBySlaveId` can map to `used_resources_by_slave_id`. Unless we decide that we're never going to expose `usedResourcesBySlaveId`, in which case we'll have `totalUsedResources` mapped to `used_resources` and that'll be it.

What do you think?


- Michael


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


On April 14, 2015, 7:40 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31665/
> -----------------------------------------------------------
> 
> (Updated April 14, 2015, 7:40 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-2373
>     https://issues.apache.org/jira/browse/MESOS-2373
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offeredResources`. Both of these are aggregates of resources from multiple slaves. We add the `hashmap<SlaveID, Resources>` versions since we can lose non-scalar resources by summing them up across multiple slaves. For further details refer to [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).
> 
> In [r31666](https://reviews.apache.org/r/31666/), we report `usedResourcesBySlaveId` rather than `usedResources` when adding the framework to the allocator.
> 
> We don't actually use `offeredResourcesBySlaveId` currently, but it should be kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as well as `offeredResourcesBySlaveId` via the HTTP endpoint.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 
> 
> Diff: https://reviews.apache.org/r/31665/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 31665: Added 'hashmap (used|offered)ResourcesBySlaveId' to master::Framework.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31665/#review80252
-----------------------------------------------------------


There are no erase calls on these new maps? When are you removing the slave?


src/master/master.hpp
<https://reviews.apache.org/r/31665/#comment130057>

    Thanks for this NOTE! We might want to take this opportunity to articulate why we can safely keep the totals. Or, should we strip non-scalars and store a 'totalUsedScalarResources'?



src/master/master.hpp
<https://reviews.apache.org/r/31665/#comment130056>

    How about s/usedResources/totalUsedResources/ and s/usedResourcesBySlaveId/usedResources/ ?
    
    This might also make the "total" referred to in your note a bit clearer. Ditto for offered.


- Ben Mahler


On April 14, 2015, 7:40 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31665/
> -----------------------------------------------------------
> 
> (Updated April 14, 2015, 7:40 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-2373
>     https://issues.apache.org/jira/browse/MESOS-2373
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offeredResources`. Both of these are aggregates of resources from multiple slaves. We add the `hashmap<SlaveID, Resources>` versions since we can lose non-scalar resources by summing them up across multiple slaves. For further details refer to [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).
> 
> In [r31666](https://reviews.apache.org/r/31666/), we report `usedResourcesBySlaveId` rather than `usedResources` when adding the framework to the allocator.
> 
> We don't actually use `offeredResourcesBySlaveId` currently, but it should be kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as well as `offeredResourcesBySlaveId` via the HTTP endpoint.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 
> 
> Diff: https://reviews.apache.org/r/31665/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 31665: Added 'hashmap (used|offered)ResourcesBySlaveId' to master::Framework.

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

(Updated April 14, 2015, 7:40 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Summary (updated)
-----------------

Added 'hashmap<SlaveID, Resources> (used|offered)ResourcesBySlaveId' to master::Framework.


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


Repository: mesos


Description
-------

`master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offeredResources`. Both of these are aggregates of resources from multiple slaves. We add the `hashmap<SlaveID, Resources>` versions since we can lose non-scalar resources by summing them up across multiple slaves. For further details refer to [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).

In [r31666](https://reviews.apache.org/r/31666/), we report `usedResourcesBySlaveId` rather than `usedResources` when adding the framework to the allocator.

We don't actually use `offeredResourcesBySlaveId` currently, but it should be kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as well as `offeredResourcesBySlaveId` via the HTTP endpoint.


Diffs
-----

  src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 

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


Testing
-------

make check


Thanks,

Michael Park


Re: Review Request 31665: Added 'hashmap usedResourcesBySlaveId' to master::Framework.

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

(Updated April 14, 2015, 7:40 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Summary (updated)
-----------------

Added 'hashmap<SlaveID, Resources> usedResourcesBySlaveId' to master::Framework.


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


Repository: mesos


Description (updated)
-------

`master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offeredResources`. Both of these are aggregates of resources from multiple slaves. We add the `hashmap<SlaveID, Resources>` versions since we can lose non-scalar resources by summing them up across multiple slaves. For further details refer to [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373).

In [r31666](https://reviews.apache.org/r/31666/), we report `usedResourcesBySlaveId` rather than `usedResources` when adding the framework to the allocator.

We don't actually use `offeredResourcesBySlaveId` currently, but it should be kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` as well as `offeredResourcesBySlaveId` via the HTTP endpoint.


Diffs (updated)
-----

  src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 

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


Testing
-------

make check


Thanks,

Michael Park


Re: Review Request 31665: Updated master::Framework::usedResources and master::Framework::offerResources from Resources to hashmap.

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

Ship it!


Minor nits, otherwise looks good to me!


src/master/http.cpp
<https://reviews.apache.org/r/31665/#comment128602>

    This comment should go down, `Resources::sum()` is problematic, not `operator +` for hashmap, right?



src/master/http.cpp
<https://reviews.apache.org/r/31665/#comment128603>

    Same here. It's about deprecating the JSON entry, not the intermediate variable.



src/master/http.cpp
<https://reviews.apache.org/r/31665/#comment128604>

    Not sure what you mean here. Is it worth creating a JIRA ticket?


- Alexander Rukletsov


On March 7, 2015, 10:02 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31665/
> -----------------------------------------------------------
> 
> (Updated March 7, 2015, 10:02 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-2373
>     https://issues.apache.org/jira/browse/MESOS-2373
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offerResources`. Both of these are aggregates of resources from multiple slaves and therefore are updated to be `hashmap<SlaveID, Resources>` instead.
> 
> There are 3 places where these variables get propagated:
> 
> (1) `allocator->addFramework(framework->id, framework->info, framework->usedResources)`
> (2) `src/master/http.cpp`: exposes them to `state.json`.
> (3) `master::Role::resources()`: needs to return `hashmap<SlaveID, Resources>` instead.
> 
> For (3) we can simply change the function signature since we only use it once in `http.cpp` and nowhere else.
> For (1) and (2), we use the `sum(resources.values())` pattern to match the existing API in the other components.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp b8eef69505b147d4c8a0e005dff545b9fc12a220 
>   src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d 
>   src/master/master.cpp 68ca19a9ae680e3ae5bd433a9842baf69f2360ec 
> 
> Diff: https://reviews.apache.org/r/31665/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Re: Review Request 31665: Updated master::Framework::usedResources and master::Framework::offerResources from Resources to hashmap.

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

(Updated March 7, 2015, 10:02 a.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

`master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offerResources`. Both of these are aggregates of resources from multiple slaves and therefore are updated to be `hashmap<SlaveID, Resources>` instead.

There are 3 places where these variables get propagated:

(1) `allocator->addFramework(framework->id, framework->info, framework->usedResources)`
(2) `src/master/http.cpp`: exposes them to `state.json`.
(3) `master::Role::resources()`: needs to return `hashmap<SlaveID, Resources>` instead.

For (3) we can simply change the function signature since we only use it once in `http.cpp` and nowhere else.
For (1) and (2), we use the `sum(resources.values())` pattern to match the existing API in the other components.


Diffs (updated)
-----

  src/master/http.cpp b8eef69505b147d4c8a0e005dff545b9fc12a220 
  src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d 
  src/master/master.cpp 68ca19a9ae680e3ae5bd433a9842baf69f2360ec 

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


Testing
-------

make check


Thanks,

Michael Park


Re: Review Request 31665: Updated master::Framework::usedResources and master::Framework::offerResources from Resources to hashmap.

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

(Updated March 7, 2015, 9:52 a.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

`master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offerResources`. Both of these are aggregates of resources from multiple slaves and therefore are updated to be `hashmap<SlaveID, Resources>` instead.

There are 3 places where these variables get propagated:

(1) `allocator->addFramework(framework->id, framework->info, framework->usedResources)`
(2) `src/master/http.cpp`: exposes them to `state.json`.
(3) `master::Role::resources()`: needs to return `hashmap<SlaveID, Resources>` instead.

For (3) we can simply change the function signature since we only use it once in `http.cpp` and nowhere else.
For (1) and (2), we use the `sum(resources.values())` pattern to match the existing API in the other components.


Diffs (updated)
-----

  src/master/http.cpp b8eef69505b147d4c8a0e005dff545b9fc12a220 
  src/master/master.hpp 3c957abcb54a0c23b8549c1d21d2d9277791938d 
  src/master/master.cpp 68ca19a9ae680e3ae5bd433a9842baf69f2360ec 

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


Testing
-------

make check


Thanks,

Michael Park


Re: Review Request 31665: Updated master::Framework::usedResources and master::Framework::offerResources from Resources to hashmap.

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

(Updated March 3, 2015, 8:28 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
-------

Moved `sum` into `Resources`.


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


Repository: mesos


Description
-------

`master::Framework` holds 2 member variables of type `Resources`: `usedResources` and `offerResources`. Both of these are aggregates of resources from multiple slaves and therefore are updated to be `hashmap<SlaveID, Resources>` instead.

There are 3 places where these variables get propagated:

(1) `allocator->addFramework(framework->id, framework->info, framework->usedResources)`
(2) `src/master/http.cpp`: exposes them to `state.json`.
(3) `master::Role::resources()`: needs to return `hashmap<SlaveID, Resources>` instead.

For (3) we can simply change the function signature since we only use it once in `http.cpp` and nowhere else.
For (1) and (2), we use the `sum(resources.values())` pattern to match the existing API in the other components.


Diffs (updated)
-----

  src/master/http.cpp 117c0ee720a60a1d8a25359028bad803f1fc2b07 
  src/master/master.hpp ce0e0b3645b099d29c6d59312a91241379d5e80c 
  src/master/master.cpp dae0955d4ab29ea74ea5ab9dd6e19b028c58c6fd 

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


Testing
-------

make check


Thanks,

Michael Park