----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65095/#review195246 -----------------------------------------------------------
src/master/master.hpp Lines 881-885 (original), 881-888 (patched) <https://reviews.apache.org/r/65095/#comment274420> I'm thinking that `convertResources` might be a more intuitive name for the boolean parameter here? In all cases, resources associated with the operation are recovered. In only some cases, we must first convert the consumed resources to converted resources so that we can then recover them. So, the variable behavior seems to be whether or not we are converting resources before we recover them. WDYT? I think the comment here could be tweaked a bit to better reflect this as well. Perhaps: "Transitions the operation and, if the operation becomes terminal, recovers the resources associate with the operation. If `convertResources` is false, then no conversion from consumed to converted resources is applied in the allocator prior to resource recovery." src/master/master.cpp Lines 10409-10416 (original), 10418-10427 (patched) <https://reviews.apache.org/r/65095/#comment274421> Is there any reason not to move all this into the first branch of the above conditional? - Greg Mann On Jan. 11, 2018, 1:53 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65095/ > ----------------------------------------------------------- > > (Updated Jan. 11, 2018, 1:53 p.m.) > > > Review request for mesos, Gaston Kleiman, Greg Mann, and Jan Schlicht. > > > Bugs: MESOS-8422 > https://issues.apache.org/jira/browse/MESOS-8422 > > > Repository: mesos > > > Description > ------- > > In certain situations it can make sense to update the state of an > operation without also wanting to update resources. In this patch we > modify the master's `updateOperation` function to take an additional > parameter signifying whether resources should be updated, or whether > we only care about updating the operation and tracking of used > resources. > > We will use this functionality in a subsequent patch to perform more > contained updates to agent state when processing `UpdateSlaveMessages` > which contain both resources and operations (and where any terminal > operations were already applied to the agent's resources). > > > Diffs > ----- > > src/master/master.hpp cdfd06ceb2a8eafa60d0af382b628b165e4ddcb5 > src/master/master.cpp 7ed15e4ba2a31c5fe4b8571f645cdca69a3e82f4 > > > Diff: https://reviews.apache.org/r/65095/diff/1/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
