> On Feb. 16, 2015, 11:43 p.m., Jie Yu wrote: > > include/mesos/resources.hpp, lines 155-161 > > <https://reviews.apache.org/r/30911/diff/2/?file=863975#file863975line155> > > > > I don't think we should remove these two helper functions as they are > > quite convinent to use. (just like we still have empty() in std containers > > even if we can use size() == 0 to replace it).
Ok, initially I removed them because we only use them in tests and not actually within Mesos. We used `unreserved` in `src/master/allocator/mesos/hierarchical.hpp` but that needs to be replaced with a different predicate, `isDRFAccounted` or something like that. Note: The `hashmap<string, Resources> Resources::reserved()` function is also not really used. Just once in `src/tests/resources_tests.cpp` to test it. > On Feb. 16, 2015, 11:43 p.m., Jie Yu wrote: > > include/mesos/resources.hpp, lines 106-121 > > <https://reviews.apache.org/r/30911/diff/2/?file=863975#file863975line106> > > > > See my comments below. Can we punt this for now and leave a TODO for > > supporing filtering for RepeatedPtrField? > > > > This struct itself is confusing because we have `reserved` as well. > > > > ``` > > Resources::Reserved("role1") > > ``` > > vs > > ``` > > resources.reserved("role1") > > ``` > > > > I think being explicit and using lambda::bind is not a bad thing. As > > least the readers immediately know that `filter` takes a predicate > > function. What do you think? Removed. > On Feb. 16, 2015, 11:43 p.m., Jie Yu wrote: > > include/mesos/resources.hpp, lines 351-354 > > <https://reviews.apache.org/r/30911/diff/2/?file=863975#file863975line351> > > > > Can we make `filter` function a member of `Resources` instead. We > > usually prefer member functions than global functions. Also, looking at the > > callsites, this version does not read well. For example: > > ``` > > filter(Resources::Reserved("role2"), slave2.resources()) > > ``` > > > > It's not obvious what does this function do. For instance, Readers > > might think that `Reserved("role2")` is a function which returns all > > resources reserved for `"role2"`. > > > > I understand that you want to make filter seamlessly work for > > RepeatedPtrField. Let's punt that for now and leave a TODO. For the above > > case, simply do the following for now (i.e., still keep `reserved` and > > `unreserved`). > > ``` > > Resources(slave2.resourecs()).reserved("role2"); > > ``` Fixed. - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30911/#review72660 ----------------------------------------------------------- On Feb. 19, 2015, 2:54 a.m., Michael Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30911/ > ----------------------------------------------------------- > > (Updated Feb. 19, 2015, 2:54 a.m.) > > > Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, and > Vinod Kone. > > > Bugs: MESOS-2348 > https://issues.apache.org/jira/browse/MESOS-2348 > > > Repository: mesos > > > Description > ------- > > See [JIRA Ticket](https://issues.apache.org/jira/browse/MESOS-2348). > > > Diffs > ----- > > include/mesos/resources.hpp c7cc46e0183ea97013dd088a717da6c0e6ed5cf0 > src/common/resources.cpp 98371f6873482d0cdbefeb279b58ae6cc680a88f > src/master/allocator/mesos/hierarchical.hpp > 2680d6231927867d5a8d75cbc42b81d6c75fc7f2 > src/master/master.hpp 6a39df04514c756415354fae66c5835ada191c52 > src/master/validation.cpp acc35b25c93f2d3900d79c3070b1d681416ed66b > src/slave/slave.cpp ec7ec1356e745bb07484ae1755c9183b038043b3 > src/tests/hierarchical_allocator_tests.cpp > eeecfb64540b16666915074aaffaa5d506b203bc > src/tests/resources_tests.cpp 3f98782fd437dba808d720bf8e9b94b8fa7e0feb > > Diff: https://reviews.apache.org/r/30911/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Michael Park > >
