----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69157/#review210692 -----------------------------------------------------------
src/slave/slave.cpp Line 8017 (original), 8017 (patched) <https://reviews.apache.org/r/69157/#comment295414> If the `latest_status` is not in sync with `status`, then this would lead to an inconcistency between the resource bookkeeping of the master and the agent: https://github.com/apache/mesos/blob/master/src/master/master.cpp#L11212 Since the master recovers and re-offers the converted resources based on the latest status, the framework might get the converted resources and use them before it acks any intermediate nonterminal status, which would lead to a failure at the agent side since the agent's bookkeeping is not based on the latest status. Instead we should do the following since `latest_status` has been updated in Line 7964--7976: ``` switch (operation->latest_status().state()) { ``` - Chun-Hung Hsiao On Nov. 13, 2018, 9:10 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69157/ > ----------------------------------------------------------- > > (Updated Nov. 13, 2018, 9:10 p.m.) > > > Review request for mesos, Chun-Hung Hsiao and Jan Schlicht. > > > Bugs: MESOS-8403 > https://issues.apache.org/jira/browse/MESOS-8403 > > > Repository: mesos > > > Description > ------- > > The handling of offer operation updates introduced in `c946615ec6d` > made use of an update's `latest_status` without making sure that any > value was set. This could lead to situation where an uninitialized > enum value was switched on which would have caused a fatal error at > runtime. > > This patch replaces uses of `latest_status` with `state` which does > contain the information we care about. We also adjust the error > logging so we log the value that lead to the error, not some other > value. > > > Diffs > ----- > > src/slave/slave.cpp 74f6fb9036a9ac4f587f53ec2df04eeb4c167bfb > > > Diff: https://reviews.apache.org/r/69157/diff/3/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
