> On Jan. 30, 2017, 1:46 a.m., Guangya Liu wrote: > > src/slave/slave.cpp, lines 1874-1878 > > <https://reviews.apache.org/r/55971/diff/2/?file=1617172#file1617172line1874> > > > > Ditto as comments in previous patches, move this to a resources util.
Added a TODO for now. > On Jan. 30, 2017, 1:46 a.m., Guangya Liu wrote: > > src/slave/slave.cpp, lines 5241-5249 > > <https://reviews.apache.org/r/55971/diff/2/?file=1617172#file1617172line5241> > > > > 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()); > > ``` I will drop for now since if we wrote this code we should probably be CHECKing the invariant, no? Would like to figure out what to do about this! :) > On Jan. 30, 2017, 1:46 a.m., Guangya Liu wrote: > > src/slave/slave.cpp, line 5244 > > <https://reviews.apache.org/r/55971/diff/2/?file=1617172#file1617172line5244> > > > > How about include `resources` in the log message? Hm.. I'm having a hard time seeing how that would be helpful here, any thoughts? - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55971/#review163428 ----------------------------------------------------------- On Jan. 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 Jan. 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 > >
