Re: Review Request 26199: Eliminated redundant resource accounting in the master.

[Vinod Kone]

> On Oct. 1, 2014, 11:05 p.m., Vinod Kone wrote:
> > src/master/master.cpp, line 4293
> > 
> >
> > hmmm. slave->used() doesn't really tell what is being returned here. 
> > how about calling it slave->resources() instead. do you think that's less 
> > confusing?
> > 
> > slave->resources() # used resources.
> > 
> > slave->info.resources() # total resources.

yea. not a big deal. that's why didn't raise an issue. s/used/allocated/ sounds 
good though, unless 'used' is already used in that context elsewhere.


- Vinod


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


On Sept. 30, 2014, 11:30 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26199/
> ---
> 
> (Updated Sept. 30, 2014, 11:30 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> This avoids the need for the per-framework resource accounting in addTask, 
> which is error prone given it may diverge from the slave->used().
> 
> Rather, have Slave::used just return this mapping directly.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 41d91c842456b6d8e23b45be24210c966c287e24 
>   src/master/master.hpp d6380199421840aa17d4ce2725dcbcf4a11ce85f 
>   src/master/master.cpp a60308f912a1ed81ecd51c677461a8f591d9eb8e 
> 
> Diff: https://reviews.apache.org/r/26199/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 26199: Eliminated redundant resource accounting in the master.

[Benjamin Mahler]

A bit worried that slave->resources() could be confused with
slave->info.resources().

Maybe slave->allocated() feels better to you?

Callers will have the type available at the call-site, since we have static
typing to force them to understand what type is being returned:

hashmap allocated = slave->allocated();
hashmap used = slave->used();

Could go this route to make the non-local reasoning a bit easier, but seems
unnecessary to me:

hashmap allocated = slave->allocatedResources();
hashmap used = slave->usedResources();

On Wed, Oct 1, 2014 at 4:05 PM, Vinod Kone  wrote:

>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26199/#review55156
> ---
>
> Ship it!
>
>
>
> src/master/master.cpp
> 
>
> hmmm. slave->used() doesn't really tell what is being returned here.
> how about calling it slave->resources() instead. do you think that's less
> confusing?
>
> slave->resources() # used resources.
>
> slave->info.resources() # total resources.
>
>
> - Vinod Kone
>
>
> On Sept. 30, 2014, 11:30 p.m., Ben Mahler wrote:
> >
> > ---
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/26199/
> > ---
> >
> > (Updated Sept. 30, 2014, 11:30 p.m.)
> >
> >
> > Review request for mesos and Vinod Kone.
> >
> >
> > Repository: mesos-git
> >
> >
> > Description
> > ---
> >
> > This avoids the need for the per-framework resource accounting in
> addTask, which is error prone given it may diverge from the slave->used().
> >
> > Rather, have Slave::used just return this mapping directly.
> >
> >
> > Diffs
> > -
> >
> >   src/master/http.cpp 41d91c842456b6d8e23b45be24210c966c287e24
> >   src/master/master.hpp d6380199421840aa17d4ce2725dcbcf4a11ce85f
> >   src/master/master.cpp a60308f912a1ed81ecd51c677461a8f591d9eb8e
> >
> > Diff: https://reviews.apache.org/r/26199/diff/
> >
> >
> > Testing
> > ---
> >
> > make check
> >
> >
> > Thanks,
> >
> > Ben Mahler
> >
> >
>
>

Re: Review Request 26199: Eliminated redundant resource accounting in the master.

[Vinod Kone]

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

Ship it!



src/master/master.cpp


hmmm. slave->used() doesn't really tell what is being returned here. how 
about calling it slave->resources() instead. do you think that's less confusing?

slave->resources() # used resources.

slave->info.resources() # total resources.


- Vinod Kone


On Sept. 30, 2014, 11:30 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26199/
> ---
> 
> (Updated Sept. 30, 2014, 11:30 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> This avoids the need for the per-framework resource accounting in addTask, 
> which is error prone given it may diverge from the slave->used().
> 
> Rather, have Slave::used just return this mapping directly.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 41d91c842456b6d8e23b45be24210c966c287e24 
>   src/master/master.hpp d6380199421840aa17d4ce2725dcbcf4a11ce85f 
>   src/master/master.cpp a60308f912a1ed81ecd51c677461a8f591d9eb8e 
> 
> Diff: https://reviews.apache.org/r/26199/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 26199: Eliminated redundant resource accounting in the master.

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On Sept. 30, 2014, 11:30 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26199/
> ---
> 
> (Updated Sept. 30, 2014, 11:30 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> This avoids the need for the per-framework resource accounting in addTask, 
> which is error prone given it may diverge from the slave->used().
> 
> Rather, have Slave::used just return this mapping directly.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 41d91c842456b6d8e23b45be24210c966c287e24 
>   src/master/master.hpp d6380199421840aa17d4ce2725dcbcf4a11ce85f 
>   src/master/master.cpp a60308f912a1ed81ecd51c677461a8f591d9eb8e 
> 
> Diff: https://reviews.apache.org/r/26199/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Review Request 26199: Eliminated redundant resource accounting in the master.

[Ben Mahler]

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

Review request for mesos and Vinod Kone.


Repository: mesos-git


Description
---

This avoids the need for the per-framework resource accounting in addTask, 
which is error prone given it may diverge from the slave->used().

Rather, have Slave::used just return this mapping directly.


Diffs
-

  src/master/http.cpp 41d91c842456b6d8e23b45be24210c966c287e24 
  src/master/master.hpp d6380199421840aa17d4ce2725dcbcf4a11ce85f 
  src/master/master.cpp a60308f912a1ed81ecd51c677461a8f591d9eb8e 

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


Testing
---

make check


Thanks,

Ben Mahler