mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jie Yu" <yujie....@gmail.com>
Subject Re: Review Request 35260: Refactored the ResourceMonitor to get statistics from the Slave.
Date Wed, 10 Jun 2015 00:17:26 GMT


> On June 9, 2015, 11:49 p.m., Ben Mahler wrote:
> > Would be nice to split apart this diff to make it easier to review thoroughly. For
example, the following seem to fit into separate patches:
> > 
> > (1) Adding Slave::usage for resource estimators.
> > (2) Updating the monitor to use Slave::usage.
> > (3) Moving ResourceMonitorProcess into .cpp file.
> > 
> > Also curious if we can use a lambda for the '`Slave::usage`' continuation?

I did want to split the patches for this when I started. I found it quite difficult because
we are changing the protobuf `ResourceUsage` which is used by both Slave, ResourceMonitor
and ResourceEstimator. That means we cannot do (1) and (2) in separate patches.

For (3), I should have done that in a separate patch. It's a bit hard to do the split right
now given the dependencies. Since most of (3) is code movement, could you please do another
pass over this patch? Sorry about that, and Thanks for the review.


- Jie


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


On June 9, 2015, 6:54 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35260/
> -----------------------------------------------------------
> 
> (Updated June 9, 2015, 6:54 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-2818
>     https://issues.apache.org/jira/browse/MESOS-2818
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Refactored the ResourceMonitor to get statistics from the Slave.
> 
> 1) Modified ResourceUsage to include allocation information (see ticket for reaons).
> 2) Simplied the ResourceMonitor by taking a lambda from the slave to get statistics.
> 3) Adjusted (and simplified) the MonitorTest accordingly.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 7457ff11f6a55099ccb95beb2f0ccb9a2f7ccd87 
>   include/mesos/slave/resource_estimator.hpp 7f78fd86760397d5b885a2c00b41081d0b546cdd

>   src/slave/monitor.hpp bee91ba143c26059fc8badf56beccb68c6556cb7 
>   src/slave/monitor.cpp 8f7ff63b4daf0286d4cf912e2f3d2d1b68caab1c 
>   src/slave/resource_estimators/fixed.cpp 3efa18d7e3c6ac62e67f75ea45a832f3f6349036 
>   src/slave/resource_estimators/noop.hpp 7a44e6deefc9c1985c14d076a427aa5c654aa1bb 
>   src/slave/resource_estimators/noop.cpp 5f135ff37e84c248ede29bbe4a7d1b8319417e20 
>   src/slave/slave.hpp 4d2c31688b19f101ec851c0d94e7d45aa2f8a76e 
>   src/slave/slave.cpp 98036b2d5f2c765aef4a416c3cbc082df77ab3ac 
>   src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 
>   src/tests/mesos.cpp dff45b0d3bf9ef53f19575ab3d90a0b223755d6a 
>   src/tests/monitor_tests.cpp 6de8b1f65843fd7b852dfa69627a1c435b482fe0 
> 
> Diff: https://reviews.apache.org/r/35260/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message