> On March 8, 2018, 12:20 a.m., Benjamin Mahler wrote: > > src/master/http.cpp > > Lines 3491 (patched) > > <https://reviews.apache.org/r/65939/diff/1/?file=1972146#file1972146line3491> > > > > Just to be sure, do we disallow double slashes? E.g. "a//b"? If so, > > then tokenize and split provide the same functionality here, and I would be > > inclined to just use split to avoid having to reason about what the > > implications of stripping empty tokens are (e.g. it breaks the lookup > > logic?)
We do make sure that we got no double-slashes via https://github.com/apache/mesos/blob/master/src/common/roles.cpp#L71 so we could indeed use split here. > On March 8, 2018, 12:20 a.m., Benjamin Mahler wrote: > > src/master/http.cpp > > Lines 3494-3513 (patched) > > <https://reviews.apache.org/r/65939/diff/1/?file=1972146#file1972146line3494> > > > > The way this is written the root 'Node' does not contain the sum of its > > children's allocations, unlike all other nodes. This doesn't seem like an > > issue since the API does not allow you to query for the root allocation. > > However, might as well make the root 'Node.resources' contain the right > > information. We could do this - so far I hesitated fearing that this duplicates the amount of invocations on our resource arithmetics. Let's discuss value vs. price. > On March 8, 2018, 12:20 a.m., Benjamin Mahler wrote: > > src/master/http.cpp > > Lines 3702-3710 (patched) > > <https://reviews.apache.org/r/65939/diff/1/?file=1972146#file1972146line3707> > > > > Rather than having to do this in the http code, can we have the Role > > struct directly expose the aggregated information as well? I'd also like > > the Role struct to have quota information, but that's for another patch :) That is what my updated RR now proposes. - Till ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65939/#review198755 ----------------------------------------------------------- On April 5, 2018, 4:15 p.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65939/ > ----------------------------------------------------------- > > (Updated April 5, 2018, 4:15 p.m.) > > > Review request for mesos, Benjamin Bannier, Benjamin Mahler, Kapil Arya, > Michael Park, and Meng Zhu. > > > Bugs: MESOS-8069 > https://issues.apache.org/jira/browse/MESOS-8069 > > > Repository: mesos > > > Description > ------- > > When listing role "a", resources allocated to role "a/b" are added to > those allocated to role "a". These changes affect both, the "/roles" > endpoint as well as the V1 HTTP Call "GET_ROLES". > Adds dynamic hierarchical role tracking to the master. > > > Diffs > ----- > > include/mesos/mesos.proto 676f0b090cad7ebf59eb32556f17ff8b5f247907 > include/mesos/v1/master/master.proto > ddb28f96b2a3a439bb9a829995a9a3015f65ba43 > include/mesos/v1/mesos.proto 10d506517c9f098374ab0c8f4dcfda42e1896c95 > src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 > src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 > src/master/master.cpp 18382fa93fd0c59e641e00f2028ac1ae2e67c01c > src/master/quota_handler.cpp 21bafd0064e9397f88185eaf687a58f85da94e2c > src/master/weights_handler.cpp 1053652804a8fc6af31a3b8a9f632f836c897fa9 > src/tests/api_tests.cpp dd8e221d8fd1b2a241505345337897e4ee4a6347 > src/tests/role_tests.cpp a609ed27ef828ca82efc0d269669cda92e5f50a1 > > > Diff: https://reviews.apache.org/r/65939/diff/2/ > > > Testing > ------- > > make check > > > Thanks, > > Till Toenshoff > >
