----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54836/#review159564 -----------------------------------------------------------
include/mesos/resources.hpp (lines 335 - 336) <https://reviews.apache.org/r/54836/#comment230600> Can you please elaborate on this? I think that we do not want to allocate a resource again if it was already allocated to some other roles? even with optimistic offer? include/mesos/resources.hpp (line 339) <https://reviews.apache.org/r/54836/#comment230598> What is the use of the return value here, can you please add more comments here for its usage? I think that the `unallocate()` will be called by `recoverResources`, and what is the problem if `unalocate` just return void for such case? src/common/resources.cpp (line 1062) <https://reviews.apache.org/r/54836/#comment230599> Shall we add a `CHECK` here to make sure this resource was not allocated to any role? src/common/resources.cpp (line 1075) <https://reviews.apache.org/r/54836/#comment230597> How about put this in the `if` block? - Guangya Liu On 十二月 17, 2016, 10:50 p.m., Benjamin Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54836/ > ----------------------------------------------------------- > > (Updated 十二月 17, 2016, 10:50 p.m.) > > > Review request for mesos, Benjamin Bannier, Jay Guo, and Guangya Liu. > > > Repository: mesos > > > Description > ------- > > Added helpers to allocate / unallocate Resources. > > > Diffs > ----- > > include/mesos/resources.hpp c2e6d6680d9687f0bbfb73e84ffb1457072c4412 > include/mesos/v1/resources.hpp 98f025b68c818c384a1174ef9b969f5c0776a7f1 > src/common/resources.cpp 16f6de6741b548a8a9fff5b70447b1a312f10efe > src/tests/resources_tests.cpp c0ff9777ceb1731086179e2966b07c46d035c110 > src/v1/resources.cpp a1adf1b27964bf6a89d97715cff05878d8c6cae9 > > Diff: https://reviews.apache.org/r/54836/diff/ > > > Testing > ------- > > Updated the existing allocation test to incorporate the new helper. > > > Thanks, > > Benjamin Mahler > >
