----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64589/#review193772 -----------------------------------------------------------
Fix it, then Ship it! src/slave/slave.cpp Lines 7501 (patched) <https://reviews.apache.org/r/64589/#comment272418> Let's `CHECK_SOME(_conversions)` explicitly here. This should be true since the operation is speculated. src/slave/slave.cpp Lines 7503 (patched) <https://reviews.apache.org/r/64589/#comment272421> nit: non-speculative src/slave/slave.cpp Lines 7506 (patched) <https://reviews.apache.org/r/64589/#comment272419> This `CHECK` is not sufficient to make sure that `converted_resources` is set -- the operation could e.g., be terminal and failed. Could we instead assert that the operation is `OFFER_OPERTATION_FINISHED`? Alternatively we might also make sure that every terminal operation sets `converted_resources`, in case of failures e.g., to `consumed`. src/slave/slave.cpp Lines 7532 (patched) <https://reviews.apache.org/r/64589/#comment272420> Let's add a comment here that after updating `slave`'s `totalResources` we also need to update the resource provider's `totalResources`. - Benjamin Bannier On Dec. 14, 2017, 2:38 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64589/ > ----------------------------------------------------------- > > (Updated Dec. 14, 2017, 2:38 a.m.) > > > Review request for mesos, Benjamin Bannier, Greg Mann, and Jan Schlicht. > > > Repository: mesos > > > Description > ------- > > The bug is introduced in this patch: > https://reviews.apache.org/r/64477/ > > Given that we also keep track of each resource provider's total > resources in addition to the total resources of the agent, we need to > make sure we update those totals after applying an operation. > > The bug may manifest as a CHECK failure in the agent that checks if > `totalResources` of the agent is a super set of all the resource > provider resources. > > > Diffs > ----- > > src/slave/slave.hpp de2b2e2e81ed860e6a33ce1b93d859d816ba1021 > src/slave/slave.cpp e8f7591dc0d57ca8a0eb72f6c1c008d4005a524d > > > Diff: https://reviews.apache.org/r/64589/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
