> On Nov. 26, 2017, 4:40 a.m., Jie Yu wrote:
> > src/master/master.hpp
> > Lines 2795-2841 (original)
> > <https://reviews.apache.org/r/64054/diff/2/?file=1901164#file1901164line2795>
> >
> >     This should still be part of `addOfferOperation`. In fact, anything to 
> > do with `used` resourecs should still be part of `addOfferOperation`.
> >     
> >     Only the part that's converting `total` resources should be pulled out 
> > because the way we change total does not rely on each offer operation. 
> > instead, we rely on the snapshot value received in `UpdateSlaveMessage`.

While we do not strictly need to remove the updates to `used` here, I feel it 
might be worthwhile to nevertheless completely remove any updates to resource 
state here, and instead make this function operate exclusively on offer 
operation state (and additionally in the future e.g., trigger offer operation 
status reconciliation). To me this seems like a natural division of 
responsibilities to me.


- Benjamin


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


On Nov. 24, 2017, 12:24 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64054/
> -----------------------------------------------------------
> 
> (Updated Nov. 24, 2017, 12:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Applying operations has been refactored out of 'addOfferOperation' and
> simplified by using 'protobuf::isSpeculativeOperation'.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 2a2e830354db4a2191fb8321beb8174b80f7ba7d 
>   src/master/master.cpp 53263e499d88b906b6406c24c0dfb737e589e813 
>   src/slave/slave.cpp f93ff7b20815c3ccb274ce6990ee66a17b6ac51c 
> 
> 
> Diff: https://reviews.apache.org/r/64054/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Reply via email to