You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Brenden Matthews <br...@diddyinc.com> on 2013/08/15 01:18:11 UTC

Review Request 13599: Use libstout cache for slave completedFrameworks.

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

Review request for mesos.


Repository: mesos-git


Description
-------

Use libstout cache for slave completedFrameworks.

The list of retired (completed) frameworks is now a stout cache rather
than a boost circular buffer.  There are now key iterator accessors for
the cache class, and a removal member.


Diffs
-----

  3rdparty/libprocess/3rdparty/stout/include/stout/cache.hpp 653507c5f1293acb929d2ffa33738fd830541c84 
  src/slave/http.cpp 073d0923febf697dec2a9bbfa6e3b243bcab7316 
  src/slave/slave.hpp ef8b64f952b7335b7f20f6c69e49450eecf563ce 
  src/slave/slave.cpp b41f2bd6ec0cdf00b0ff166d5089a984b2707ac2 

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


Testing
-------

Running on staging cluster.

`make check`


Thanks,

Brenden Matthews


Re: Review Request 13599: Use libstout cache for slave completedFrameworks.

Posted by Brenden Matthews <br...@diddyinc.com>.

> On Oct. 15, 2013, 2:36 a.m., Ben Mahler wrote:
> > Hey Brenden, I somehow missed this change, sorry for not giving a review here!
> > 
> > It would be really great if this also made the corresponding change to the master which would fix MESOS-417 and avoid the need for Ross' change here: https://reviews.apache.org/r/14644/

Hey Ben, I need to catch up on things here.  These two patches do indeed resolve MESOS-417.  I'm going to update according to your comments below in the next couple days and get back to you.


- Brenden


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


On Aug. 14, 2013, 11:18 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13599/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2013, 11:18 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Use libstout cache for slave completedFrameworks.
> 
> The list of retired (completed) frameworks is now a stout cache rather
> than a boost circular buffer.  There are now key iterator accessors for
> the cache class, and a removal member.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/cache.hpp 653507c5f1293acb929d2ffa33738fd830541c84 
>   src/slave/http.cpp 073d0923febf697dec2a9bbfa6e3b243bcab7316 
>   src/slave/slave.hpp ef8b64f952b7335b7f20f6c69e49450eecf563ce 
>   src/slave/slave.cpp b41f2bd6ec0cdf00b0ff166d5089a984b2707ac2 
> 
> Diff: https://reviews.apache.org/r/13599/diff/
> 
> 
> Testing
> -------
> 
> Running on staging cluster.
> 
> `make check`
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 13599: Use libstout cache for slave completedFrameworks.

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


Hey Brenden, I somehow missed this change, sorry for not giving a review here!

It would be really great if this also made the corresponding change to the master which would fix MESOS-417 and avoid the need for Ross' change here: https://reviews.apache.org/r/14644/


src/slave/slave.hpp
<https://reviews.apache.org/r/13599/#comment52583>

    Since we don't really want the LRU behavior for cache, and rather we want LRI (least recently inserted), we could just leave cache.hpp unchanged and leave this as a circular_buffer. Then when we need to lookup the framework id, we can loop over to find framework, leaving a TODO to address the inefficiency.
    
    Vinod and I had discussed adding LRI to cache (at construction time) or adding a capacity to the new LinkedHashMap (think BoundedLinkedHashMap or the like). I think benh had also thought about an Archive abstraction that provides some kind of indexed LRI interface. But let's punt on solving that and we can come back to it and update completedFrameworks later! :)
    
    So to simplify this change perhaps we can just leave this as circular_buffer and loop over it to lookup framework ids? Then we can solve efficiency later :)


- Ben Mahler


On Aug. 14, 2013, 11:18 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13599/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2013, 11:18 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Use libstout cache for slave completedFrameworks.
> 
> The list of retired (completed) frameworks is now a stout cache rather
> than a boost circular buffer.  There are now key iterator accessors for
> the cache class, and a removal member.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/cache.hpp 653507c5f1293acb929d2ffa33738fd830541c84 
>   src/slave/http.cpp 073d0923febf697dec2a9bbfa6e3b243bcab7316 
>   src/slave/slave.hpp ef8b64f952b7335b7f20f6c69e49450eecf563ce 
>   src/slave/slave.cpp b41f2bd6ec0cdf00b0ff166d5089a984b2707ac2 
> 
> Diff: https://reviews.apache.org/r/13599/diff/
> 
> 
> Testing
> -------
> 
> Running on staging cluster.
> 
> `make check`
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>