----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55971/#review163428 -----------------------------------------------------------
src/slave/slave.cpp (lines 1874 - 1878) <https://reviews.apache.org/r/55971/#comment234888> Ditto as comments in previous patches, move this to a resources util. src/slave/slave.cpp (lines 5241 - 5249) <https://reviews.apache.org/r/55971/#comment234887> Since we do not support store mix allocated and unallocated resources, how about optimize the logic a big as following? ``` if (resource->has_allocation_info()) { break; } if (roles.size() != 1) { LOG(FATAL) << "Missing 'Resource.AllocationInfo' for resources" << " allocated to MULTI_ROLE framework" << " '" << frameworkInfo.name() << "'"; } resource->mutable_allocation_info()->set_role(*roles.begin()); ``` src/slave/slave.cpp (line 5244) <https://reviews.apache.org/r/55971/#comment234886> How about include `resources` in the log message? - Guangya Liu On 一月 27, 2017, 12:27 a.m., Benjamin Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55971/ > ----------------------------------------------------------- > > (Updated 一月 27, 2017, 12:27 a.m.) > > > Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael > Park. > > > Bugs: MESOS-6964 > https://issues.apache.org/jira/browse/MESOS-6964 > > > Repository: mesos > > > Description > ------- > > With the addition of MULTI_ROLE support, the agent needs to ensure > that allocated resources reported to the master have the > `Resource.AllocationInfo` set. The approach taken here is to set > it only in memory upon recovering tasks and executors. Note that > once we allow a framework to modify its roles, we need to update > the agent to re-persist the resources with injected allocation > info (rather than just setting it in memory). > > > Diffs > ----- > > src/slave/resource_estimators/fixed.cpp > 2c1268c3423091c6a419020a3af97255de55db0a > src/slave/slave.cpp 0548b04073c0ba4adfc4433d75fd18c2ba79d891 > > Diff: https://reviews.apache.org/r/55971/diff/ > > > Testing > ------- > > The tests pass at the end of this review chain. > > > Thanks, > > Benjamin Mahler > >