mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Neil Conway <neil.con...@gmail.com>
Subject Re: Review Request 57564: Changed DRFSorter's representation of inactive clients.
Date Thu, 30 Mar 2017 15:55:50 GMT


> On March 29, 2017, 10:42 p.m., Benjamin Mahler wrote:
> > src/master/allocator/sorter/drf/sorter.hpp
> > Lines 214-216 (original), 215-217 (patched)
> > <https://reviews.apache.org/r/57564/diff/1/?file=1662777#file1662777line215>
> >
> >     This comment is no longer accurate, right? We used to store a distinct structure
purely because of the deactivated clients being absent from the `clients` map. Now that this
is no longer the case, it seems like this patch should be removing this additional data structure
(which needs to be kept in sync) and just storing `Allocation` within Client.
> >     
> >     Won't we need to do this anyway for the hierarchy change? Seems clearer to me
to make that change in this patch.

Right, the comment was stale (although it was removed in the next review in the chain anyway);
I updated it.

I'll look into splitting the `Allocation` change out into a separate review.


- Neil


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


On March 13, 2017, 6:04 p.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57564/
> -----------------------------------------------------------
> 
> (Updated March 13, 2017, 6:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> DRFSorter previously removed inactive clients from the `clients`
> collection, and then re-added clients when they were reactivated. This
> resulted in resetting the allocation count for the client, which is
> unfortunate. This scheme would also be more difficult to adapt to
> hierarchical sorting.
> 
> This commit changes DRFSorter to continue to store inactive clients in
> the `clients`; inactive clients are indicated by a new field in the
> `Client` struct, and are omitted from the return value of
> `DRFSorter::sort`.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/sorter/drf/sorter.hpp 76329220e1115c1de7810fb69b943c78c078be59

>   src/master/allocator/sorter/drf/sorter.cpp ed54680cecb637931fc344fbcf8fd3b14cc24295

>   src/tests/sorter_tests.cpp ec0636beb936d46a253d19322f2157abe95156b6 
> 
> 
> Diff: https://reviews.apache.org/r/57564/diff/1/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


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