> On June 9, 2016, 5:10 p.m., Jiang Yan Xu wrote: > > I feel good about the overall structure of this change with this revision. > > I hope I have captured most things so if we do another pass after it should > > mostly be relatively minor readability/reusablity/documentation/cleanups > > etc. which is hard to do in one go due to the length. > > > > ## About v1 resources files > > In retropsect I feel like it probably works the better if it's in a > > separate patch done **after the v0 files are shipped** to avoid having them > > go through all intermediate revisions. i.e., you can update v1 files > > according to the final version of the v0 files. > > > > I have mostly skipped v1 files in this review because the comments would > > have been duplicated. Since we are still iterating on this, would you mind > > pulling them out from this review? > > > > ## Validation > > If we currently only allow persistent volumes to be shared, should we > > validate this here? > > > > ## Additional test ideas > > - Resources A + A - A == A with shared resources in it. > > - Resources B + A - A == B with shared resources in A (empty resources are > > discarded) > > - Resources r1 = A + A, r2 = A + A + A, r1 - r2 is empty (invalid entries > > are discarded) > > - Resources r1 + A == r1 (A is an invalid shared resource) (invalid entries > > are discarded) > > - Shared resources that slightly differ are not grouped together. > > - Shared resources stream output. > > - Resources contains() with shared resources. > > - Any existing resources tests interesting when shared resources are > > blended in?
About validation: sorry I didn't realize it's in a later review /r/45960/ but if you just look at this review one might wonder if you should also test shared cpus or other fungible resources. Anyways, it's fine if we don't since it's not a valid use case for now. - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/#review136417 ----------------------------------------------------------- On May 22, 2016, 12:08 a.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45959/ > ----------------------------------------------------------- > > (Updated May 22, 2016, 12:08 a.m.) > > > Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Jiang > Yan Xu. > > > Bugs: MESOS-4892 > https://issues.apache.org/jira/browse/MESOS-4892 > > > Repository: mesos > > > Description > ------- > > A new class Resource_ is added that allows 'Resources' to group > identical shared resource objects together into a single 'Resource_' > object and tracked by its shared count. Non-shared resource objects > are not grouped. > > For resource addition and subtraction, the shared count is adjusted for > shared resources as follows: > a) Addition: If shared resource is absent from original, then the > resource is added initialized with a consumer count of 1. Otherwise, > the share count for the shared resource is incremented. > b) Subtraction: If shared resource's share count is already 1, then > the shared resource is removed from the original. Otherwise, its > consumer count is decremented. > > > Diffs > ----- > > include/mesos/resources.hpp a557e97c65194d4aad879fb88d8edefd1c95b8d8 > include/mesos/v1/resources.hpp a5ba8fec4c9c3643646308f75a4b28cefe0b3df3 > src/common/resources.cpp f6ff92b591c15bc8e93fd85e1896349c3a7bb968 > src/master/validation.cpp f490b899758bdac9676a6f6939918efa6ac52781 > src/tests/mesos.hpp 79bf1ff16412ce2a510a9b75ab1ac91c1c182653 > src/tests/resources_tests.cpp dc12bd8f1e2da6972bc8aed598811c55d664036e > src/v1/resources.cpp 8c3f2d1c1529915a59d47fe37bb3fc7a3267079a > > Diff: https://reviews.apache.org/r/45959/diff/ > > > Testing > ------- > > New tests added to demonstrate arithmetic operations for shared resources > with consumer counts. > Tests successful. > > > Thanks, > > Anindya Sinha > >
