Review Request 60423: Renamed method `resources` to `allocatedResources` in master::Role.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60423/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- This method collects resources allocated to the particular role. It's more precise to rename it as such. Diffs - src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 src/master/master.hpp 9dd6a530c373516dc81bfd51ee6e95f25588148f Diff: https://reviews.apache.org/r/60423/diff/1/ Testing --- no functional change Thanks, Jay Guo
Re: Review Request 58552: Resolved a TODO of MULTI_ROLE.
> On June 26, 2017, 2:55 p.m., Benjamin Mahler wrote: > > src/master/master.hpp > > Line 2838 (original), 2838-2839 (patched) > > <https://reviews.apache.org/r/58552/diff/1/?file=1695283#file1695283line2838> > > > > Rather than the comment, we should just call this function > > allocatedResources so that there's no need for the comment. > > > > I'll remove the comment in the commit, do you want to send another > > patch to rename it to `allocatedResources()` for clarity? https://reviews.apache.org/r/60423/ - Jay --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58552/#review178864 ----------- On April 20, 2017, 11 a.m., Jay Guo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58552/ > --- > > (Updated April 20, 2017, 11 a.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > --- > > After the completion of MULTI_ROLE support, allocation_info is always > populated, some redundant logic for resources of old format are not > necessary anymore, therefore removed. > > > Diffs > - > > src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034 > > > Diff: https://reviews.apache.org/r/58552/diff/1/ > > > Testing > --- > > make check > > > Thanks, > > Jay Guo > >
Re: Review Request 58096: Added authorization for frameworks in /roles endpoint.
> On May 17, 2017, 4:08 p.m., Adam B wrote: > > src/master/http.cpp > > Lines 3524 (patched) > > <https://reviews.apache.org/r/58096/diff/7/?file=1714048#file1714048line3524> > > > > `futures` is an over-vague variable name, especially since neither are > > Futures by this point. Can we do better? I'm running out of vocabulary here... two very different thing to be captured in one word. Maybe `collection`? - Jay --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58096/#review175213 ----------- On June 22, 2017, 2 a.m., Jay Guo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58096/ > --- > > (Updated June 22, 2017, 2 a.m.) > > > Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. > > > Bugs: MESOS-7260 > https://issues.apache.org/jira/browse/MESOS-7260 > > > Repository: mesos > > > Description > --- > > While /roles displays a list of frameworksIds that register with > a role, it did NOT filter them based on VIEW_FRAMEWORK ACL, which > impose a security risk. This patch fixed this issue by taking a > frameworksApprover in `Master::Http::roles()` which is used to > filter framework IDs. > > > Diffs > - > > src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 > > > Diff: https://reviews.apache.org/r/58096/diff/8/ > > > Testing > --- > > see next patch in the chain. > > > Thanks, > > Jay Guo > >
Re: Review Request 58099: Added authorization for frameworks in `GetRoles` v1 API.
> On May 17, 2017, 4:29 p.m., Adam B wrote: > > Seems like we're adding even more duplicate code into this v1 clone of > > `roles()`. Can you find a way to reduce the redundance? OK, let me take look and may submit some follow-up patches for it. - Jay --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58099/#review175219 --- On June 22, 2017, 2:02 a.m., Jay Guo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58099/ > --- > > (Updated June 22, 2017, 2:02 a.m.) > > > Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. > > > Bugs: MESOS-7260 > https://issues.apache.org/jira/browse/MESOS-7260 > > > Repository: mesos > > > Description > --- > > Added authorization for frameworks in `GetRoles` v1 API. > > > Diffs > - > > src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 > src/tests/api_tests.cpp cdaa72462eac1687185115771265539d8c2b09a9 > > > Diff: https://reviews.apache.org/r/58099/diff/7/ > > > Testing > --- > > > Thanks, > > Jay Guo > >
Re: Review Request 58099: Added authorization for frameworks in `GetRoles` v1 API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58099/ --- (Updated June 22, 2017, 2:02 a.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- rebase and address comments Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Added authorization for frameworks in `GetRoles` v1 API. Diffs (updated) - src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 src/tests/api_tests.cpp cdaa72462eac1687185115771265539d8c2b09a9 Diff: https://reviews.apache.org/r/58099/diff/7/ Changes: https://reviews.apache.org/r/58099/diff/6-7/ Testing --- Thanks, Jay Guo
Re: Review Request 58095: Refactored `Master::Http::roles` to use `jsonify`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58095/ --- (Updated June 22, 2017, 2:01 a.m.) Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and Michael Park. Changes --- minor tweak Bugs: MESOS-4732 and MESOS-7260 https://issues.apache.org/jira/browse/MESOS-4732 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Refactored `Master::Http::roles` to use `jsonify`. Diffs (updated) - src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 Diff: https://reviews.apache.org/r/58095/diff/9/ Changes: https://reviews.apache.org/r/58095/diff/8-9/ Testing --- no functional changes make check Thanks, Jay Guo
Re: Review Request 58097: Added a test to check framework filtering in /roles endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58097/ --- (Updated June 22, 2017, 2:01 a.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- rebase and address comments Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Added a test to check framework filtering in /roles endpoint. Diffs (updated) - src/tests/master_authorization_tests.cpp 8d54472d8a31956e6e8bb5fe7ffbf47dc793c0bc Diff: https://reviews.apache.org/r/58097/diff/7/ Changes: https://reviews.apache.org/r/58097/diff/6-7/ Testing --- make check Thanks, Jay Guo
Re: Review Request 58096: Added authorization for frameworks in /roles endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58096/ --- (Updated June 22, 2017, 2 a.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- rebase & address comments Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- While /roles displays a list of frameworksIds that register with a role, it did NOT filter them based on VIEW_FRAMEWORK ACL, which impose a security risk. This patch fixed this issue by taking a frameworksApprover in `Master::Http::roles()` which is used to filter framework IDs. Diffs (updated) - src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 Diff: https://reviews.apache.org/r/58096/diff/8/ Changes: https://reviews.apache.org/r/58096/diff/7-8/ Testing --- see next patch in the chain. Thanks, Jay Guo
Re: Review Request 58095: Refactored `Master::Http::roles` to use `jsonify`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58095/ --- (Updated June 22, 2017, 1:57 a.m.) Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and Michael Park. Changes --- rebase and address comments Bugs: MESOS-4732 and MESOS-7260 https://issues.apache.org/jira/browse/MESOS-4732 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Refactored `Master::Http::roles` to use `jsonify`. Diffs (updated) - src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 Diff: https://reviews.apache.org/r/58095/diff/8/ Changes: https://reviews.apache.org/r/58095/diff/7-8/ Testing --- no functional changes make check Thanks, Jay Guo
Re: Review Request 58508: Introduced method `Resources::allocatableTo` to aggregate reservations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58508/ --- (Updated June 13, 2017, 10:35 a.m.) Review request for mesos and Michael Park. Changes --- Updated dependency. Bugs: MESOS-7149 https://issues.apache.org/jira/browse/MESOS-7149 Repository: mesos Description --- A reservation is allocatable to 'role' iff: - it is reserved to ancestor of 'role' in hierarchy, OR - it is reserved to 'role' itself, OR - it is unreserved. This is a helper function for reservation delegate, where reserved resources can be 'delegated' downwards in the hierarchy. Diffs - include/mesos/resources.hpp 451b0cef6f84743e3d630104ba6f55665793f2bc include/mesos/v1/resources.hpp c790927353122a4eb2bbbee31c6df57d97f3994b src/common/resources.cpp f89f1aae7845f1e93dd5947de2e7a8e2bfce8bc4 src/tests/resources_tests.cpp 71c7544470f44f84ef90f9b31afcd81d46493981 src/v1/resources.cpp 24240eedc4b2747ca02998534437318c3396db4d Diff: https://reviews.apache.org/r/58508/diff/3/ Testing --- comment out https://github.com/apache/mesos/blob/master/src/common/roles.cpp#L71-L74 and `./bin/mesos-tests.sh --gtest_filter="ResourcesTest.DISABLED_HierarchicalReservations" --gtest_also_run_disabled_tests` Thanks, Jay Guo
Re: Review Request 58510: Added a test for hierarchical reservation.
> On June 10, 2017, 3:22 a.m., Benjamin Mahler wrote: > > src/tests/hierarchical_allocator_tests.cpp > > Lines 4606-4607 (patched) > > <https://reviews.apache.org/r/58510/diff/1/?file=1693724#file1693724line4606> > > > > I was a bit confused about why this test was using quota, then I > > figured out that this is how you wanted to ensure that the allocation goes > > to the descendant. > > > > We should document this at the top of the test, or here, to describe > > that we leverage quota to test this. > > > > Simpler to understand would be to not use quota and add two agents, > > expecting one agent to have been allocated to the ancestor and descendant, > > each. Does that not work? Since we made changes for both quota and fair share stage of allocation, I'm using quota to make sure we cover both logic paths. I updated the test comments, please take a look. > On June 10, 2017, 3:22 a.m., Benjamin Mahler wrote: > > src/tests/reservation_tests.cpp > > Lines 2399-2401 (patched) > > <https://reviews.apache.org/r/58510/diff/1/?file=1693725#file1693725line2399> > > > > These two tests are very similar, the only difference seems to be that > > one tests that the reservations are allocated using "fair sharing" between > > the roles (or at least if you removed quota, that would be the case), and > > the other just tests that it is allocatable to the role? > > > > That seems like a reasonable split to ensure that there is a > > ReservationTest covering this, but it's only accurate if you remove the > > quota from the first test (otherwise these tests are essentially the same?) Dropped the test. - Jay --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58510/#review177509 --- On June 13, 2017, 11:40 p.m., Jay Guo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58510/ > --- > > (Updated June 13, 2017, 11:40 p.m.) > > > Review request for mesos and Michael Park. > > > Bugs: MESOS-7149 > https://issues.apache.org/jira/browse/MESOS-7149 > > > Repository: mesos > > > Description > --- > > Added a test for hierarchical reservation. > > > Diffs > - > > src/tests/hierarchical_allocator_tests.cpp > 631e800e6566847e015ea2638cc1c2fe673855be > > > Diff: https://reviews.apache.org/r/58510/diff/2/ > > > Testing > --- > > make check > > comment out > https://github.com/apache/mesos/blob/master/src/common/roles.cpp#L71-L74 and > `./bin/mesos-tests.sh > --gtest_filter="HierarchicalAllocatorTest.DISABLED_OfferAncestorReservationsToDescendantChild" > --gtest_also_run_disabled_tests` > > > Thanks, > > Jay Guo > >
Re: Review Request 58510: Added a test for hierarchical reservation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58510/ --- (Updated June 13, 2017, 11:40 p.m.) Review request for mesos and Michael Park. Bugs: MESOS-7149 https://issues.apache.org/jira/browse/MESOS-7149 Repository: mesos Description --- Added a test for hierarchical reservation. Diffs - src/tests/hierarchical_allocator_tests.cpp 631e800e6566847e015ea2638cc1c2fe673855be Diff: https://reviews.apache.org/r/58510/diff/2/ Testing (updated) --- make check comment out https://github.com/apache/mesos/blob/master/src/common/roles.cpp#L71-L74 and `./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.DISABLED_OfferAncestorReservationsToDescendantChild" --gtest_also_run_disabled_tests` Thanks, Jay Guo
Re: Review Request 58508: Introduced method `Resources::allocatableTo` to aggregate reservations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58508/ --- (Updated June 13, 2017, 11:38 p.m.) Review request for mesos and Michael Park. Bugs: MESOS-7149 https://issues.apache.org/jira/browse/MESOS-7149 Repository: mesos Description --- A reservation is allocatable to 'role' iff: - it is reserved to ancestor of 'role' in hierarchy, OR - it is reserved to 'role' itself, OR - it is unreserved. This is a helper function for reservation delegate, where reserved resources can be 'delegated' downwards in the hierarchy. Diffs - include/mesos/resources.hpp 451b0cef6f84743e3d630104ba6f55665793f2bc include/mesos/v1/resources.hpp c790927353122a4eb2bbbee31c6df57d97f3994b src/common/resources.cpp f89f1aae7845f1e93dd5947de2e7a8e2bfce8bc4 src/tests/resources_tests.cpp 71c7544470f44f84ef90f9b31afcd81d46493981 src/v1/resources.cpp 24240eedc4b2747ca02998534437318c3396db4d Diff: https://reviews.apache.org/r/58508/diff/3/ Testing (updated) --- comment out https://github.com/apache/mesos/blob/master/src/common/roles.cpp#L71-L74 and `./bin/mesos-tests.sh --gtest_filter="ResourcesTest.DISABLED_HierarchicalReservations" --gtest_also_run_disabled_tests` Thanks, Jay Guo
Re: Review Request 58510: Added a test for hierarchical reservation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58510/ --- (Updated June 13, 2017, 11:31 p.m.) Review request for mesos and Michael Park. Changes --- rebase and address comments. Summary (updated) - Added a test for hierarchical reservation. Bugs: MESOS-7149 https://issues.apache.org/jira/browse/MESOS-7149 Repository: mesos Description (updated) --- Added a test for hierarchical reservation. Diffs (updated) - src/tests/hierarchical_allocator_tests.cpp 631e800e6566847e015ea2638cc1c2fe673855be Diff: https://reviews.apache.org/r/58510/diff/2/ Changes: https://reviews.apache.org/r/58510/diff/1-2/ Testing --- make check Thanks, Jay Guo
Re: Review Request 58510: Added two tests for hierarchical reservation.
> On June 9, 2017, 2:23 a.m., Neil Conway wrote: > > src/tests/reservation_tests.cpp > > Lines 2401 (patched) > > <https://reviews.apache.org/r/58510/diff/1/?file=1693725#file1693725line2401> > > > > I have various minor gripes about how this test is written :) > > > > When the clock is paused, I think it is better to first advance the > > clock to ensure the agent has registered. Then register the framework; we > > should then get an offer at precisely that point (w/o waiting for batch > > alloc). As written, advancing the clock for the batch alloc is actually > > what triggers the agent to register, which is fragile (what if agent > > registration backoff is > batch alloc interval?). > > > > i.e., I'd adjust the test as follows: > > https://gist.github.com/neilconway/2f11988222cd8fb9583905624cb4ddd5 Thank you for great advice! Although I removed this test per @bmahler's comment, I'm sure this will help me in the future :) - Jay --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58510/#review177346 --- On April 18, 2017, 11:45 p.m., Jay Guo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58510/ > --- > > (Updated April 18, 2017, 11:45 p.m.) > > > Review request for mesos and Michael Park. > > > Bugs: MESOS-7149 > https://issues.apache.org/jira/browse/MESOS-7149 > > > Repository: mesos > > > Description > --- > > Added two tests for hierarchical reservation. > > > Diffs > - > > src/tests/hierarchical_allocator_tests.cpp > 33e7b455f8664858eb4f03727b076a10c80cd6e0 > src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad > > > Diff: https://reviews.apache.org/r/58510/diff/1/ > > > Testing > --- > > make check > > > Thanks, > > Jay Guo > >
Re: Review Request 58509: Enabled allocator to handle hierarchical reservation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58509/ --- (Updated June 13, 2017, 11:27 p.m.) Review request for mesos and Michael Park. Changes --- rebase and address comments Bugs: MESOS-7149 https://issues.apache.org/jira/browse/MESOS-7149 Repository: mesos Description (updated) --- When collecting resources allocatable to a role, allocator should aggregate not only reservations of the role itself, but also that of all its ancestors. Diffs (updated) - src/master/allocator/mesos/hierarchical.cpp 73d0b42433a1ff3e853e851b8191864b4e233666 Diff: https://reviews.apache.org/r/58509/diff/2/ Changes: https://reviews.apache.org/r/58509/diff/1-2/ Testing --- see next patch in chain Thanks, Jay Guo
Re: Review Request 58508: Introduced method `Resources::allocatableTo` to aggregate reservations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58508/ --- (Updated June 13, 2017, 11:23 p.m.) Review request for mesos and Michael Park. Changes --- rebase and address comments Summary (updated) - Introduced method `Resources::allocatableTo` to aggregate reservations. Bugs: MESOS-7149 https://issues.apache.org/jira/browse/MESOS-7149 Repository: mesos Description (updated) --- A reservation is allocatable to 'role' iff: - it is reserved to ancestor of 'role' in hierarchy, OR - it is reserved to 'role' itself, OR - it is unreserved. This is a helper function for reservation delegate, where reserved resources can be 'delegated' downwards in the hierarchy. Diffs (updated) - include/mesos/resources.hpp 451b0cef6f84743e3d630104ba6f55665793f2bc include/mesos/v1/resources.hpp c790927353122a4eb2bbbee31c6df57d97f3994b src/common/resources.cpp f89f1aae7845f1e93dd5947de2e7a8e2bfce8bc4 src/tests/resources_tests.cpp 71c7544470f44f84ef90f9b31afcd81d46493981 src/v1/resources.cpp 24240eedc4b2747ca02998534437318c3396db4d Diff: https://reviews.apache.org/r/58508/diff/3/ Changes: https://reviews.apache.org/r/58508/diff/2-3/ Testing --- make check Thanks, Jay Guo
Re: Review Request 59088: Implemented QuotaTree structure to be used by allocator.
> On May 15, 2017, 8:45 p.m., Alexander Rojas wrote: > > src/master/quota.hpp > > Lines 56 (patched) > > <https://reviews.apache.org/r/59088/diff/1/?file=1711225#file1711225line57> > > > > So after looking more closely I realized that in no small part this is > > a copy paste of the other `QuotaTree`, is there a reason for that? > > > > Likewise, I am not a big fan of using the word `Tree` in the name since > > it mixes intend with implementation details. If you really want to create a > > quota validator, then called that: `QuotaValidator` or `QuotaRegistry` or > > `QuotaBookeeper` all which reflect intent without telling you unecesary > > details about how the implementation works. I agree that name should be chosen more wisely. I like `QuotaBookeeper` - Jay --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59088/#review174943 ------- On May 9, 2017, 5:49 p.m., Jay Guo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59088/ > --- > > (Updated May 9, 2017, 5:49 p.m.) > > > Review request for mesos, Benjamin Mahler, Michael Park, and Neil Conway. > > > Bugs: MESOS-7402 > https://issues.apache.org/jira/browse/MESOS-7402 > > > Repository: mesos > > > Description > --- > > Implemented QuotaTree structure to be used by allocator. > > > Diffs > - > > src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a > src/master/quota.hpp 479112d1a94ede34e2d46a27ff03999fc1634b8c > src/master/quota.cpp 3e0d280efff69904b74e5d4c71985ab44ffa34a7 > src/tests/quota_tests.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/59088/diff/1/ > > > Testing > --- > > see end of chain. > > > Thanks, > > Jay Guo > >
Re: Review Request 59088: Implemented QuotaTree structure to be used by allocator.
> On May 15, 2017, 8:20 p.m., Alexander Rojas wrote: > > src/master/quota.hpp > > Lines 56 (patched) > > <https://reviews.apache.org/r/59088/diff/1/?file=1711225#file1711225line57> > > > > why do we need a new class called `QuotaTree` when we have already one > > in `quota_handler.cpp`? > > > > Why not fixing stuff there instead than here? This is a PoC patch, which is intended to replace `QuotaTree` in `quota_handler.cpp`, and should be used by both `HierarchicalAllocatorProcess` and `QuotaHandler`. I just want to show a possible approach to solve MESOS-7402. - Jay --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59088/#review174941 ------- On May 9, 2017, 5:49 p.m., Jay Guo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59088/ > --- > > (Updated May 9, 2017, 5:49 p.m.) > > > Review request for mesos, Benjamin Mahler, Michael Park, and Neil Conway. > > > Bugs: MESOS-7402 > https://issues.apache.org/jira/browse/MESOS-7402 > > > Repository: mesos > > > Description > --- > > Implemented QuotaTree structure to be used by allocator. > > > Diffs > - > > src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a > src/master/quota.hpp 479112d1a94ede34e2d46a27ff03999fc1634b8c > src/master/quota.cpp 3e0d280efff69904b74e5d4c71985ab44ffa34a7 > src/tests/quota_tests.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/59088/diff/1/ > > > Testing > --- > > see end of chain. > > > Thanks, > > Jay Guo > >
Re: Review Request 58096: Added authorization for frameworks in /roles endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58096/ --- (Updated May 11, 2017, 12:33 a.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- While /roles displays a list of frameworksIds that register with a role, it did NOT filter them based on VIEW_FRAMEWORK ACL, which impose a security risk. This patch fixed this issue by taking a frameworksApprover in `Master::Http::roles()` which is used to filter framework IDs. Diffs (updated) - src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 Diff: https://reviews.apache.org/r/58096/diff/7/ Changes: https://reviews.apache.org/r/58096/diff/6-7/ Testing --- see next patch in the chain. Thanks, Jay Guo
Re: Review Request 58095: Refactored `Master::Http::roles` to use `jsonify`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58095/ --- (Updated May 11, 2017, 12:32 a.m.) Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and Michael Park. Changes --- rebase Bugs: MESOS-4732 and MESOS-7260 https://issues.apache.org/jira/browse/MESOS-4732 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Refactored `Master::Http::roles` to use `jsonify`. Diffs (updated) - src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e src/common/http.cpp 167dce2b9a2d3b68a1df5b4079f701482d34db28 src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 Diff: https://reviews.apache.org/r/58095/diff/7/ Changes: https://reviews.apache.org/r/58095/diff/6-7/ Testing --- no functional changes make check Thanks, Jay Guo
Re: Review Request 58099: Added authorization for frameworks in `GetRoles` v1 API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58099/ --- (Updated May 10, 2017, 9:52 p.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- address comments. Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Added authorization for frameworks in `GetRoles` v1 API. Diffs (updated) - src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 Diff: https://reviews.apache.org/r/58099/diff/6/ Changes: https://reviews.apache.org/r/58099/diff/5-6/ Testing --- Thanks, Jay Guo
Re: Review Request 58097: Added a test to check framework filtering in /roles endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58097/ --- (Updated May 10, 2017, 9:47 p.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Added a test to check framework filtering in /roles endpoint. Diffs (updated) - src/tests/master_authorization_tests.cpp e4233c19b1d9e3e2734259503d0daec4ce243667 Diff: https://reviews.apache.org/r/58097/diff/6/ Changes: https://reviews.apache.org/r/58097/diff/5-6/ Testing --- make check Thanks, Jay Guo
Re: Review Request 58096: Added authorization for frameworks in /roles endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58096/ --- (Updated May 10, 2017, 9:46 p.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- address comments. Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- While /roles displays a list of frameworksIds that register with a role, it did NOT filter them based on VIEW_FRAMEWORK ACL, which impose a security risk. This patch fixed this issue by taking a frameworksApprover in `Master::Http::roles()` which is used to filter framework IDs. Diffs (updated) - src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 Diff: https://reviews.apache.org/r/58096/diff/6/ Changes: https://reviews.apache.org/r/58096/diff/5-6/ Testing --- see next patch in the chain. Thanks, Jay Guo
Re: Review Request 58095: Refactored `Master::Http::roles` to use `jsonify`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58095/ --- (Updated May 10, 2017, 9:42 p.m.) Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and Michael Park. Changes --- address comments. Bugs: MESOS-4732 and MESOS-7260 https://issues.apache.org/jira/browse/MESOS-4732 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Refactored `Master::Http::roles` to use `jsonify`. Diffs (updated) - src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e src/common/http.cpp 167dce2b9a2d3b68a1df5b4079f701482d34db28 src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 Diff: https://reviews.apache.org/r/58095/diff/6/ Changes: https://reviews.apache.org/r/58095/diff/5-6/ Testing --- no functional changes make check Thanks, Jay Guo
Review Request 59090: Renabled some tests, fixed some errors.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59090/ --- Review request for mesos, Benjamin Mahler, Michael Park, and Neil Conway. Repository: mesos Description --- Renabled some tests, fixed some errors. Diffs - src/tests/hierarchical_allocator_tests.cpp 08180b9975869de328f0c095dd3cddf0c84fbecf Diff: https://reviews.apache.org/r/59090/diff/1/ Testing --- make check Thanks, Jay Guo
Review Request 59089: Changed hierarchical allocator to use QuotaTree.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59089/ --- Review request for mesos, Benjamin Mahler, Michael Park, and Neil Conway. Bugs: MESOS-7402 https://issues.apache.org/jira/browse/MESOS-7402 Repository: mesos Description --- Changed hierarchical allocator to use QuotaTree to keep track of quotas instead of using hashmap. This allows us to correctly account hierarchical quota, see MESOS-7402. Diffs - src/master/allocator/mesos/hierarchical.hpp 123f97cf495bff0f822838e09df0d88818f04da6 src/master/allocator/mesos/hierarchical.cpp b75ed9a20a9a42f958cebbacd91e5e15b0d21394 Diff: https://reviews.apache.org/r/59089/diff/1/ Testing --- Thanks, Jay Guo
Review Request 59088: Implemented QuotaTree structure to be used by allocator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59088/ --- Review request for mesos, Benjamin Mahler, Michael Park, and Neil Conway. Bugs: MESOS-7402 https://issues.apache.org/jira/browse/MESOS-7402 Repository: mesos Description --- Implemented QuotaTree structure to be used by allocator. Diffs - src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a src/master/quota.hpp 479112d1a94ede34e2d46a27ff03999fc1634b8c src/master/quota.cpp 3e0d280efff69904b74e5d4c71985ab44ffa34a7 src/tests/quota_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/59088/diff/1/ Testing --- see end of chain. Thanks, Jay Guo
Re: Review Request 58097: Added a test to check framework filtering in /roles endpoint.
> On April 27, 2017, 11:05 p.m., Alexander Rojas wrote: > > src/tests/master_authorization_tests.cpp > > Line 2063 (original), 2062 (patched) > > <https://reviews.apache.org/r/58097/diff/4/?file=1694001#file1694001line2063> > > > > Not yours, but no test verifies that the serialzation of quotas/weights > > is done correctly. Since you are changing how it is performed, coul you > > test that too? I feel `RoleTest` is sufficient, no? - Jay --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58097/#review173207 ----------- On May 8, 2017, 3:56 p.m., Jay Guo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58097/ > --- > > (Updated May 8, 2017, 3:56 p.m.) > > > Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. > > > Bugs: MESOS-7260 > https://issues.apache.org/jira/browse/MESOS-7260 > > > Repository: mesos > > > Description > --- > > Added a test to check framework filtering in /roles endpoint. > > > Diffs > - > > src/tests/master_authorization_tests.cpp > e4233c19b1d9e3e2734259503d0daec4ce243667 > > > Diff: https://reviews.apache.org/r/58097/diff/5/ > > > Testing > --- > > make check > > > Thanks, > > Jay Guo > >
Re: Review Request 58099: Added authorization for frameworks in `GetRoles` v1 API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58099/ --- (Updated May 8, 2017, 3:56 p.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Added authorization for frameworks in `GetRoles` v1 API. Diffs (updated) - src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 Diff: https://reviews.apache.org/r/58099/diff/5/ Changes: https://reviews.apache.org/r/58099/diff/4-5/ Testing --- Thanks, Jay Guo
Re: Review Request 58097: Added a test to check framework filtering in /roles endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58097/ --- (Updated May 8, 2017, 3:56 p.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Added a test to check framework filtering in /roles endpoint. Diffs (updated) - src/tests/master_authorization_tests.cpp e4233c19b1d9e3e2734259503d0daec4ce243667 Diff: https://reviews.apache.org/r/58097/diff/5/ Changes: https://reviews.apache.org/r/58097/diff/4-5/ Testing --- make check Thanks, Jay Guo
Re: Review Request 58096: Added authorization for frameworks in /roles endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58096/ --- (Updated May 8, 2017, 3:56 p.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- While /roles displays a list of frameworksIds that register with a role, it did NOT filter them based on VIEW_FRAMEWORK ACL, which impose a security risk. This patch fixed this issue by taking a frameworksApprover in `Master::Http::roles()` which is used to filter framework IDs. Diffs (updated) - src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 Diff: https://reviews.apache.org/r/58096/diff/5/ Changes: https://reviews.apache.org/r/58096/diff/4-5/ Testing --- see next patch in the chain. Thanks, Jay Guo
Re: Review Request 58095: Refactored `Master::Http::roles` to use `jsonify`.
> On April 27, 2017, 10:58 p.m., Alexander Rojas wrote: > > src/master/http.cpp > > Line 3401 (original), 3403 (patched) > > <https://reviews.apache.org/r/58095/diff/4/?file=1693998#file1693998line3403> > > > > your changes here make this function non safe thread. Notice that the > > original would run in the context of the `MasterProcess` thread, while this > > will do so in the caller's thread, causing a possible race condition. Reverted to keep using `_roles()`. > On April 27, 2017, 10:58 p.m., Alexander Rojas wrote: > > src/master/http.cpp > > Lines 3481-3482 (original), 3456-3457 (patched) > > <https://reviews.apache.org/r/58095/diff/4/?file=1693998#file1693998line3495> > > > > Not yours, but I feel the formatting of this lambda is really off. The > > body of the lambda starts a couple of columns earlier than declaration. Other indents in this file are off too, e.g. `state()`, `frameworks()` > On April 27, 2017, 10:58 p.m., Alexander Rojas wrote: > > src/master/http.cpp > > Line 3504 (original), 3505 (patched) > > <https://reviews.apache.org/r/58095/diff/4/?file=1693998#file1693998line3548> > > > > Is ist really necesary to add empty fields? Without this, `RoleTest.EndpointNoFrameworks` would be broken. I suppose we need empty fields for backwards compatibility? > On April 27, 2017, 10:58 p.m., Alexander Rojas wrote: > > src/master/master.hpp > > Lines 1399-1401 (original), 1399-1405 (patched) > > <https://reviews.apache.org/r/58095/diff/4/?file=1693999#file1693999line1399> > > > > How about rewording this to: > > > > ```c++ > > // List of active roles, i.e. roles which are > > // explicitly white listed, plus roles with one or > > // more registered frameworks, plus all roles with a > > // non default weight or quota. > > ``` > > > > This gives you a clear understanding of what the method returns without > > describing how you do it (which is non interesting for API documentation). > > Also, the first paragraph doesn't really says anything useful. Left original method untouched. - Jay ----------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58095/#review173186 --- On May 8, 2017, 3:41 p.m., Jay Guo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58095/ > --- > > (Updated May 8, 2017, 3:41 p.m.) > > > Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and > Michael Park. > > > Bugs: MESOS-4732 and MESOS-7260 > https://issues.apache.org/jira/browse/MESOS-4732 > https://issues.apache.org/jira/browse/MESOS-7260 > > > Repository: mesos > > > Description > --- > > Refactored `Master::Http::roles` to use `jsonify`. > > > Diffs > - > > src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e > src/common/http.cpp 167dce2b9a2d3b68a1df5b4079f701482d34db28 > src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 > > > Diff: https://reviews.apache.org/r/58095/diff/5/ > > > Testing > --- > > no functional changes > make check > > > Thanks, > > Jay Guo > >
Re: Review Request 58095: Refactored `Master::Http::roles` to use `jsonify`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58095/ --- (Updated May 8, 2017, 3:41 p.m.) Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and Michael Park. Changes --- address comments. Summary (updated) - Refactored `Master::Http::roles` to use `jsonify`. Bugs: MESOS-4732 and MESOS-7260 https://issues.apache.org/jira/browse/MESOS-4732 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description (updated) --- Refactored `Master::Http::roles` to use `jsonify`. Diffs (updated) - src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e src/common/http.cpp 167dce2b9a2d3b68a1df5b4079f701482d34db28 src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 Diff: https://reviews.apache.org/r/58095/diff/5/ Changes: https://reviews.apache.org/r/58095/diff/4-5/ Testing --- no functional changes make check Thanks, Jay Guo
Re: Review Request 58509: Enabled allocator to handle hierarchical reservation.
> On April 24, 2017, 3:25 p.m., Qian Zhang wrote: > > After this change is introduced, I think we may need to update some docs, > > e.g., in > > https://github.com/apache/mesos/blob/master/docs/persistent-volume.md, it > > is mentioned that a persistent volume might be created by one framework and > > then offered to a different framework subscribed to the `same role`, but > > now I think it is not just `same role`, instead it should be the role > > itself and any its descendant roles. Good catch! I actually wonder if this is correct at all... need some further discussion. cc @mcypark @bmahler - Jay --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58509/#review172760 ------- On April 18, 2017, 11:44 p.m., Jay Guo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58509/ > --- > > (Updated April 18, 2017, 11:44 p.m.) > > > Review request for mesos and Michael Park. > > > Bugs: MESOS-7149 > https://issues.apache.org/jira/browse/MESOS-7149 > > > Repository: mesos > > > Description > --- > > Other than looking at reservation made for a role, now allocator also > aggregates reservations of all its ancestor in the hierarchy. > > > Diffs > - > > src/master/allocator/mesos/hierarchical.cpp > 051f749dd5921a322ca930a042c31814616d38f9 > > > Diff: https://reviews.apache.org/r/58509/diff/1/ > > > Testing > --- > > see next patch in chain > > > Thanks, > > Jay Guo > >
Re: Review Request 58584: Disabled support for setting quota on nested roles.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58584/#review172587 --- LGTM src/master/quota_handler.cpp Lines 518-521 (patched) <https://reviews.apache.org/r/58584/#comment245683> How about putting this before building `QuotaTree` so we don't bother to validate nested quota for now. Also, how about changing error message to something like: `"Setting quota on nested role '" + quotaInfo.role() + "' is not supported yet"`, so we don't mislead users to believe that we don't plan to. src/tests/hierarchical_allocator_tests.cpp Lines 4610 (patched) <https://reviews.apache.org/r/58584/#comment245686> How about `NON_QUOTA_ROLE` to be explicit. src/tests/hierarchical_allocator_tests.cpp Lines 4613 (patched) <https://reviews.apache.org/r/58584/#comment245684> I like indexing from zero too, but I think we start from one in most of the tests. Also the agent starts from one in this test case :P src/tests/hierarchical_allocator_tests.cpp Lines 4617 (patched) <https://reviews.apache.org/r/58584/#comment245685> Let's make value of mem greater than `MIN_MEM` to be consistent with other tests. - Jay Guo On April 21, 2017, 2:18 a.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58584/ > --- > > (Updated April 21, 2017, 2:18 a.m.) > > > Review request for mesos, Benjamin Mahler, Jay Guo, and Michael Park. > > > Repository: mesos > > > Description > --- > > Correct support for quota on nested roles will require further work in > the allocator (see MESOS-7402 for details). For now, setting quota on > nested roles is disabled until MESOS-7402 can be fixed. This commit > disables any tests that rely on setting quota on nested roles; it also > adds a (disabled) test to cover the behavior that will be fixed as part > of MESOS-7402. > > > Diffs > - > > src/master/quota_handler.cpp 7ff43a048e17b9e9ac0ceed248f7b3fd56b007d6 > src/tests/hierarchical_allocator_tests.cpp > 33e7b455f8664858eb4f03727b076a10c80cd6e0 > src/tests/master_quota_tests.cpp 1714ba13ea63bae05448d0898bf722ef472c672b > > > Diff: https://reviews.apache.org/r/58584/diff/2/ > > > Testing > --- > > `make check` > > > Thanks, > > Neil Conway > >
Re: Review Request 58508: Introduced method `Resources::inheritable` to aggregate reservations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58508/ --- (Updated April 21, 2017, 12:18 a.m.) Review request for mesos and Michael Park. Changes --- address comments from mcypark. Bugs: MESOS-7149 https://issues.apache.org/jira/browse/MESOS-7149 Repository: mesos Description --- A reservation is inheritable by 'role' iff: - it is reserved to ancestor of 'role' in hierarchy, OR - it is reserved to 'role' itself. This is a helper function for reservation delegate, where reserved resources can be 'delegated' downwards in the hierarchy. Diffs (updated) - include/mesos/resources.hpp 74583e3c784f22b45cce81f07c5a69b339e1684d src/common/resources.cpp 77bac0c4390da905016a942991b14a79877f8455 src/tests/resources_tests.cpp 4cf320d802a749f1419ac5b9f63b6c73b0c974be Diff: https://reviews.apache.org/r/58508/diff/2/ Changes: https://reviews.apache.org/r/58508/diff/1-2/ Testing --- make check Thanks, Jay Guo
Re: Review Request 58533: Added a benchmark test for hierarchical sorter.
> On April 20, 2017, 3:04 a.m., Neil Conway wrote: > > src/tests/sorter_tests.cpp > > Lines 1570 (patched) > > <https://reviews.apache.org/r/58533/diff/1/?file=1694263#file1694263line1570> > > > > I wonder if an iterative solution would be clearer. I benchmarked both iterative and recursive solutions, and recursive one seems to be much more efficient. - Jay --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58533/#review172377 ------- On April 20, 2017, 11:54 a.m., Jay Guo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58533/ > --- > > (Updated April 20, 2017, 11:54 a.m.) > > > Review request for mesos, Benjamin Mahler and Neil Conway. > > > Bugs: MESOS-7395 > https://issues.apache.org/jira/browse/MESOS-7395 > > > Repository: mesos > > > Description > --- > > This test is very similar to Sorter_BENCHMARK_Test.FullSort, except > that hierarchical role is built instead of flat one. > > > Diffs > - > > src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a > > > Diff: https://reviews.apache.org/r/58533/diff/2/ > > > Testing > --- > > `./bin/mesos-tests.sh --benchmark > --gtest_filter="AgentAndClientCount/HSorter_BENCHMARK_Test.FullSort*"` > > > Thanks, > > Jay Guo > >
Re: Review Request 58533: Added a benchmark test for hierarchical sorter.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58533/ --- (Updated April 20, 2017, 11:54 a.m.) Review request for mesos, Benjamin Mahler and Neil Conway. Changes --- address comments from neilc Bugs: MESOS-7395 https://issues.apache.org/jira/browse/MESOS-7395 Repository: mesos Description --- This test is very similar to Sorter_BENCHMARK_Test.FullSort, except that hierarchical role is built instead of flat one. Diffs (updated) - src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a Diff: https://reviews.apache.org/r/58533/diff/2/ Changes: https://reviews.apache.org/r/58533/diff/1-2/ Testing --- `./bin/mesos-tests.sh --benchmark --gtest_filter="AgentAndClientCount/HSorter_BENCHMARK_Test.FullSort*"` Thanks, Jay Guo
Review Request 58552: Resolved a TODO of MULTI_ROLE.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58552/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description --- After the completion of MULTI_ROLE support, allocation_info is always populated, some redundant logic for resources of old format are not necessary anymore, therefore removed. Diffs - src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034 Diff: https://reviews.apache.org/r/58552/diff/1/ Testing --- make check Thanks, Jay Guo
Re: Review Request 58545: Removed NOTE about incomplete implementation of FrameworkInfo.roles.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58545/#review172437 --- Ship it! LGTM. I'm looking for MultioRole-related TODOs and see what could be resolved now. In the future, I feel labeling TODO seems to be a good practice worth calling out in the dev community. - Jay Guo On April 20, 2017, 6:32 a.m., Benjamin Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58545/ > --- > > (Updated April 20, 2017, 6:32 a.m.) > > > Review request for mesos, Benjamin Bannier and Jay Guo. > > > Bugs: MESOS-7368 > https://issues.apache.org/jira/browse/MESOS-7368 > > > Repository: mesos > > > Description > --- > > Now that the support for frameworks with multiple roles has been > implemented, we can remove the note about the `FrameworkInfo.roles` > field having an incomplete implementation. > > > Diffs > - > > include/mesos/mesos.proto eaa2d2ac697cfc4f5aa56db0fb37363339608f43 > include/mesos/v1/mesos.proto 1a32a7bdc991c77b35a988bf8a34cee936c97608 > > > Diff: https://reviews.apache.org/r/58545/diff/1/ > > > Testing > --- > > N/A > > > Thanks, > > Benjamin Mahler > >
Review Request 58533: Added a benchmark test for hierarchical sorter.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58533/ --- Review request for mesos, Benjamin Mahler and Neil Conway. Bugs: MESOS-7395 https://issues.apache.org/jira/browse/MESOS-7395 Repository: mesos Description --- This test is very similar to Sorter_BENCHMARK_Test.FullSort, except that hierarchical role is built instead of flat one. Diffs - src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a Diff: https://reviews.apache.org/r/58533/diff/1/ Testing --- `./bin/mesos-tests.sh --benchmark --gtest_filter="AgentAndClientCount/HSorter_BENCHMARK_Test.FullSort*"` Thanks, Jay Guo
Re: Review Request 58099: Added authorization for frameworks in `GetRoles` v1 API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58099/ --- (Updated April 19, 2017, 11:38 a.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Added authorization for frameworks in `GetRoles` v1 API. Diffs (updated) - src/master/http.cpp 5aae52870451d883ef1ea1fda5a27764d7f318e8 Diff: https://reviews.apache.org/r/58099/diff/4/ Changes: https://reviews.apache.org/r/58099/diff/3-4/ Testing --- Thanks, Jay Guo
Re: Review Request 58097: Added a test to check framework filtering in /roles endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58097/ --- (Updated April 19, 2017, 11:38 a.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Added a test to check framework filtering in /roles endpoint. Diffs (updated) - src/tests/master_authorization_tests.cpp 1a0285a3f345ef21a8256d4123d8bb684f538da4 Diff: https://reviews.apache.org/r/58097/diff/4/ Changes: https://reviews.apache.org/r/58097/diff/3-4/ Testing --- make check Thanks, Jay Guo
Re: Review Request 58096: Added authorization for frameworks in /roles endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58096/ --- (Updated April 19, 2017, 11:37 a.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- While /roles displays a list of frameworksIds that register with a role, it did NOT filter them based on VIEW_FRAMEWORK ACL, which impose a security risk. This patch fixed this issue by taking a frameworksApprover in `Master::Http::roles()` which is used to filter framework IDs. Diffs (updated) - src/master/http.cpp 5aae52870451d883ef1ea1fda5a27764d7f318e8 Diff: https://reviews.apache.org/r/58096/diff/4/ Changes: https://reviews.apache.org/r/58096/diff/3-4/ Testing --- see next patch in the chain. Thanks, Jay Guo
Re: Review Request 58095: Refactored functions that render /roles and GetRoles endpoints.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58095/ --- (Updated April 19, 2017, 11:36 a.m.) Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and Michael Park. Changes --- rebase Bugs: MESOS-4732 and MESOS-7260 https://issues.apache.org/jira/browse/MESOS-4732 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Instead of generating JSON object, `Master::Http::roles()` now leverages `jsonify` to compute output. Also approver is taken out from its continuation function. This is for easier and cleaner implementation of framework authorization in /roles endpoint, see MESOS-7260. Diffs (updated) - src/master/http.cpp 5aae52870451d883ef1ea1fda5a27764d7f318e8 src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034 Diff: https://reviews.apache.org/r/58095/diff/4/ Changes: https://reviews.apache.org/r/58095/diff/3-4/ Testing --- no functional changes make check Thanks, Jay Guo
Review Request 58510: Added two tests for hierarchical reservation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58510/ --- Review request for mesos and Michael Park. Bugs: MESOS-7149 https://issues.apache.org/jira/browse/MESOS-7149 Repository: mesos Description --- Added two tests for hierarchical reservation. Diffs - src/tests/hierarchical_allocator_tests.cpp 33e7b455f8664858eb4f03727b076a10c80cd6e0 src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad Diff: https://reviews.apache.org/r/58510/diff/1/ Testing --- make check Thanks, Jay Guo
Review Request 58509: Enabled allocator to handle hierarchical reservation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58509/ --- Review request for mesos and Michael Park. Bugs: MESOS-7149 https://issues.apache.org/jira/browse/MESOS-7149 Repository: mesos Description --- Other than looking at reservation made for a role, now allocator also aggregates reservations of all its ancestor in the hierarchy. Diffs - src/master/allocator/mesos/hierarchical.cpp 051f749dd5921a322ca930a042c31814616d38f9 Diff: https://reviews.apache.org/r/58509/diff/1/ Testing --- see next patch in chain Thanks, Jay Guo
Review Request 58508: Introduced method `Resources::inheritable` to aggregate reservations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58508/ --- Review request for mesos and Michael Park. Bugs: MESOS-7149 https://issues.apache.org/jira/browse/MESOS-7149 Repository: mesos Description --- A reservation is inheritable by 'role' iff: - it is reserved to ancestor of 'role' in hierarchy, OR - it is reserved to 'role' itself. This is a helper function for reservation delegate, where reserved resources can be 'delegated' downwards in the hierarchy. Diffs - include/mesos/resources.hpp 74583e3c784f22b45cce81f07c5a69b339e1684d src/common/resources.cpp 77bac0c4390da905016a942991b14a79877f8455 src/tests/resources_tests.cpp 4cf320d802a749f1419ac5b9f63b6c73b0c974be Diff: https://reviews.apache.org/r/58508/diff/1/ Testing --- make check Thanks, Jay Guo
Re: Review Request 57254: Added support for hierarchical roles to DRFSorter.
> On April 17, 2017, 11:09 p.m., Jay Guo wrote: > > src/master/allocator/sorter/drf/sorter.cpp > > Line 333 (original), 509 (patched) > > <https://reviews.apache.org/r/57254/diff/17/?file=1692350#file1692350line536> > > > > In case of `dirty`, how about insert logic of `listClients` there to > > avoid traversing tree twice for the sake of performance? I tried a > > benchmark test which does suggest a performance benefit. > > Neil Conway wrote: > That could be a useful improvement. When you benchmarked this, what % > performance improvement did you observe? Approximately 10% - Jay --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57254/#review172083 --- On April 18, 2017, 12:58 p.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57254/ > --- > > (Updated April 18, 2017, 12:58 p.m.) > > > Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Michael Park. > > > Repository: mesos > > > Description > --- > > This commit replaces the sorter's flat list of clients with a tree; the > tree represents the hierarchical relationship between sorter clients. > Each node in the tree contains a vector of pointers to child nodes. The > tree might contain nodes that do not correspond directly to sorter > clients. For example, adding clients "a/b" and "c/d" results in the > following tree: > > root > -> a >-> b > -> c >-> d > > The `sort` member function still only returns one result for each active > client in the sorter. This is implemented by ensuring that each sorter > client is associated with a leaf node in the tree. Note that it is > possible for a leaf node to be transformed into an internal node by a > subsequent insertion; to handle this case, we "implicitly" create an > extra child node, which maintains the invariant that each client has a > leaf node. For example, if the client "a/b/x" is added to the tree > above, the result is: > > root > -> a >-> b > -> . > -> x > -> c >-> d > > The "." leaf node holds the allocation that has been made to the "a/b" > client itself; the "a/b" node holds the sum of all the allocations that > have been made to the subtree rooted at "a/b", which also includes > "a/b/x". > > This commit also introduces a new approach to sorting: rather than > keeping an `std::set` of sorter clients, we now keep a tree of > `std::vector`, which is sorted explicitly via `std::sort`. The previous > implementation tried to optimize the sorting process by updating the > sort order incrementally when a single sorter client was updated; this > commit removes that optimization, and instead re-sorts the entire tree > whenever the sort order is changed. > > Re-introducing a version of this optimization would make sense in the > future (MESOS-7390), but benchmarking suggests that this commit results > in a net improvement in sorter performance anyway. The sorter perf > improvement is likely due to the introduction of a secondary hashmap > that allows the leaf node associated with a tree path to be find > efficiently; the previous implementation required a linear scan of a > `std::set`. > > > Diffs > - > > src/master/allocator/sorter/drf/metrics.cpp > 15aab32db5ca1a7a14080e9bbb7c65283be3ec20 > src/master/allocator/sorter/drf/sorter.hpp > 76329220e1115c1de7810fb69b943c78c078be59 > src/master/allocator/sorter/drf/sorter.cpp > ed54680cecb637931fc344fbcf8fd3b14cc24295 > src/master/allocator/sorter/sorter.hpp > b3029fcf7342406955760da53f1ae736769f308c > src/tests/hierarchical_allocator_tests.cpp > 33e7b455f8664858eb4f03727b076a10c80cd6e0 > src/tests/master_allocator_tests.cpp > 119e318f8a01d50e8dae5c30cf5fa6a017c3c625 > src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a > > > Diff: https://reviews.apache.org/r/57254/diff/19/ > > > Testing > --- > > > Thanks, > > Neil Conway > >
Re: Review Request 57254: Added support for hierarchical roles to DRFSorter.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57254/#review172083 --- I tried to modify `Sorter_BENCHMARK_Test.FullSort` to build tree of roles instead of flat roles. And the benchmark result suggests a significant performance downgrade during `allocated`/`unallocated`, which I guess is inevitable due to tree traversal. However, should we capture this in the benchmark test? I left a comment in MESOS-7078 as well. src/master/allocator/sorter/drf/sorter.cpp Line 333 (original), 509 (patched) <https://reviews.apache.org/r/57254/#comment245162> In case of `dirty`, how about insert logic of `listClients` there to avoid traversing tree twice for the sake of performance? I tried a benchmark test which does suggest a performance benefit. src/master/allocator/sorter/drf/sorter.cpp Lines 513 (patched) <https://reviews.apache.org/r/57254/#comment245160> Indent of this lambda is off. src/master/allocator/sorter/drf/sorter.cpp Line 355 (original), 537 (patched) <https://reviews.apache.org/r/57254/#comment245161> indent of this lambda is off. - Jay Guo On April 14, 2017, 5:42 a.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57254/ > --- > > (Updated April 14, 2017, 5:42 a.m.) > > > Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Michael Park. > > > Repository: mesos > > > Description > --- > > This commit replaces the sorter's flat list of clients with a tree; the > tree represents the hierarchical relationship between sorter clients. > Each node in the tree contains a vector of pointers to child nodes. The > tree might contain nodes that do not correspond directly to sorter > clients. For example, adding clients "a/b" and "c/d" results in the > following tree: > > root > -> a >-> b > -> c >-> d > > The `sort` member function still only returns one result for each active > client in the sorter. This is implemented by ensuring that each sorter > client is associated with a leaf node in the tree. Note that it is > possible for a leaf node to be transformed into an internal node by a > subsequent insertion; to handle this case, we "implicitly" create an > extra child node, which maintains the invariant that each client has a > leaf node. For example, if the client "a/b/x" is added to the tree > above, the result is: > > root > -> a >-> b > -> . > -> x > -> c >-> d > > The "." leaf node holds the allocation that has been made to the "a/b" > client itself; the "a/b" node holds the sum of all the allocations that > have been made to the subtree rooted at "a/b", which also includes > "a/b/x". > > This commit also introduces a new approach to sorting: rather than > keeping an `std::set` of sorter clients, we now keep a tree of > `std::vector`, which is sorted explicitly via `std::sort`. The previous > implementation tried to optimize the sorting process by updating the > sort order incrementally when a single sorter client was updated; this > commit removes that optimization, and instead re-sorts the entire tree > whenever the sort order is changed. > > Re-introducing a version of this optimization would make sense in the > future (MESOS-7390), but benchmarking suggests that this commit results > in a net improvement in sorter performance anyway. The sorter perf > improvement is likely due to the introduction of a secondary hashmap > that allows the leaf node associated with a tree path to be find > efficiently; the previous implementation required a linear scan of a > `std::set`. > > > Diffs > - > > src/master/allocator/sorter/drf/metrics.cpp > 15aab32db5ca1a7a14080e9bbb7c65283be3ec20 > src/master/allocator/sorter/drf/sorter.hpp > 76329220e1115c1de7810fb69b943c78c078be59 > src/master/allocator/sorter/drf/sorter.cpp > ed54680cecb637931fc344fbcf8fd3b14cc24295 > src/master/allocator/sorter/sorter.hpp > b3029fcf7342406955760da53f1ae736769f308c > src/tests/hierarchical_allocator_tests.cpp > 33e7b455f8664858eb4f03727b076a10c80cd6e0 > src/tests/master_allocator_tests.cpp > 119e318f8a01d50e8dae5c30cf5fa6a017c3c625 > src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a > > > Diff: https://reviews.apache.org/r/57254/diff/17/ > > > Testing > --- > > > Thanks, > > Neil Conway > >
Review Request 58379: Fixed a typo in persistent volume doc.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58379/ --- Review request for mesos and Jie Yu. Repository: mesos Description --- Fixed a typo in persistent volume doc. Diffs - docs/persistent-volume.md bd2f5391e0e6df5155b2c5644eddb3ca861108e9 Diff: https://reviews.apache.org/r/58379/diff/1/ Testing --- N/A Thanks, Jay Guo
Re: Review Request 58095: Refactored functions that render /roles and GetRoles endpoints.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58095/#review171674 --- src/master/http.cpp Lines 3504-3507 (patched) <https://reviews.apache.org/r/58095/#comment244692> Is there a better way to write empty array to json? - Jay Guo On April 12, 2017, 1:35 p.m., Jay Guo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58095/ > --- > > (Updated April 12, 2017, 1:35 p.m.) > > > Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and > Michael Park. > > > Bugs: MESOS-4732 and MESOS-7260 > https://issues.apache.org/jira/browse/MESOS-4732 > https://issues.apache.org/jira/browse/MESOS-7260 > > > Repository: mesos > > > Description > --- > > Instead of generating JSON object, `Master::Http::roles()` now > leverages `jsonify` to compute output. Also approver is taken > out from its continuation function. This is for easier and cleaner > implementation of framework authorization in /roles endpoint, > see MESOS-7260. > > > Diffs > - > > src/master/http.cpp 0e84d363b346de1ef05101dac066c2f0cf31609d > src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034 > > > Diff: https://reviews.apache.org/r/58095/diff/3/ > > > Testing > --- > > no functional changes > make check > > > Thanks, > > Jay Guo > >
Re: Review Request 58097: Added a test to check framework filtering in /roles endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58097/ --- (Updated April 12, 2017, 1:38 p.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Added a test to check framework filtering in /roles endpoint. Diffs (updated) - src/tests/master_authorization_tests.cpp 1a0285a3f345ef21a8256d4123d8bb684f538da4 Diff: https://reviews.apache.org/r/58097/diff/3/ Changes: https://reviews.apache.org/r/58097/diff/2-3/ Testing --- make check Thanks, Jay Guo
Re: Review Request 58096: Added authorization for frameworks in /roles endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58096/ --- (Updated April 12, 2017, 1:38 p.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- While /roles displays a list of frameworksIds that register with a role, it did NOT filter them based on VIEW_FRAMEWORK ACL, which impose a security risk. This patch fixed this issue by taking a frameworksApprover in `Master::Http::roles()` which is used to filter framework IDs. Diffs (updated) - src/master/http.cpp 0e84d363b346de1ef05101dac066c2f0cf31609d Diff: https://reviews.apache.org/r/58096/diff/3/ Changes: https://reviews.apache.org/r/58096/diff/2-3/ Testing --- see next patch in the chain. Thanks, Jay Guo
Re: Review Request 58099: Added authorization for frameworks in `GetRoles` v1 API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58099/ --- (Updated April 12, 2017, 1:38 p.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Added authorization for frameworks in `GetRoles` v1 API. Diffs (updated) - src/master/http.cpp 0e84d363b346de1ef05101dac066c2f0cf31609d Diff: https://reviews.apache.org/r/58099/diff/3/ Changes: https://reviews.apache.org/r/58099/diff/2-3/ Testing --- Thanks, Jay Guo
Re: Review Request 58095: Refactored functions that render /roles and GetRoles endpoints.
> On April 7, 2017, 9:11 p.m., Alexander Rojas wrote: > > src/master/http.cpp > > Line 3383 (original), 3385 (patched) > > <https://reviews.apache.org/r/58095/diff/2/?file=1684472#file1684472line3385> > > > > With the proposed changes, there's no reason to have this function in > > `Master::Http` and can be move directly to `Master` IMHO. > > > > Probably the name can be change to `activeRoles` too, since it makes > > more sense given the description of _interesting roles_. Thanks for your review. I changed method name to be more expressive. However, this method is only used by `Master::Http` and not intended for `Master`, I feel we should let it stay in `Master::Http`. What do you think. - Jay --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58095/#review171337 ----------- On April 12, 2017, 1:35 p.m., Jay Guo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58095/ > --- > > (Updated April 12, 2017, 1:35 p.m.) > > > Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and > Michael Park. > > > Bugs: MESOS-4732 and MESOS-7260 > https://issues.apache.org/jira/browse/MESOS-4732 > https://issues.apache.org/jira/browse/MESOS-7260 > > > Repository: mesos > > > Description > --- > > Instead of generating JSON object, `Master::Http::roles()` now > leverages `jsonify` to compute output. Also approver is taken > out from its continuation function. This is for easier and cleaner > implementation of framework authorization in /roles endpoint, > see MESOS-7260. > > > Diffs > - > > src/master/http.cpp 0e84d363b346de1ef05101dac066c2f0cf31609d > src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034 > > > Diff: https://reviews.apache.org/r/58095/diff/3/ > > > Testing > --- > > no functional changes > make check > > > Thanks, > > Jay Guo > >
Re: Review Request 58095: Refactored functions that render /roles and GetRoles endpoints.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58095/ --- (Updated April 12, 2017, 1:35 p.m.) Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and Michael Park. Changes --- tweak indentation and method name to address arojas's comments. Bugs: MESOS-4732 and MESOS-7260 https://issues.apache.org/jira/browse/MESOS-4732 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Instead of generating JSON object, `Master::Http::roles()` now leverages `jsonify` to compute output. Also approver is taken out from its continuation function. This is for easier and cleaner implementation of framework authorization in /roles endpoint, see MESOS-7260. Diffs (updated) - src/master/http.cpp 0e84d363b346de1ef05101dac066c2f0cf31609d src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034 Diff: https://reviews.apache.org/r/58095/diff/3/ Changes: https://reviews.apache.org/r/58095/diff/2-3/ Testing --- no functional changes make check Thanks, Jay Guo
Re: Review Request 57254: Added support for hierarchical roles to DRFSorter.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57254/#review171424 --- src/tests/hierarchical_allocator_tests.cpp Lines 4582 (patched) <https://reviews.apache.org/r/57254/#comment244357> s/framework3/framework2/ - Jay Guo On April 1, 2017, 4:59 a.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57254/ > --- > > (Updated April 1, 2017, 4:59 a.m.) > > > Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Michael Park. > > > Repository: mesos > > > Description > --- > > This commit replaces the sorter's flat list of clients with a tree of > client names; this tree represents the hierarchical relationship between > sorter clients. Each node in the tree contains an (ordered) list of > pointers to child nodes. The tree might contain nodes that do not > correspond directly to sorter clients. For example, adding clients "a/b" > and "c/d" results in the following tree: > > root > -> a >-> b > -> c >-> d > > The `sort` member function still only returns one result for each active > client in the sorter. This is implemented by ensuring that each sorter > client is associated with a leaf node in the tree. Note that it is > possible for a leaf node to be transformed into an internal node by a > subsequent insertion; to handle this case, we "implicitly" create an > extra child node, which maintains the invariant that each client has a > leaf node. For example, if the client "a/b/x" is added to the tree > above, the result is: > > root > -> a >-> b > -> . > -> x > -> c >-> d > > The "." leaf node holds the allocation that has been made to the "a/b" > client itself; the "a/b" node holds the sum of all the allocations that > have been made to the subtree rooted at "a/b", which also includes > "a/b/x". > > > Diffs > - > > src/master/allocator/sorter/drf/metrics.cpp > 15aab32db5ca1a7a14080e9bbb7c65283be3ec20 > src/master/allocator/sorter/drf/sorter.hpp > 76329220e1115c1de7810fb69b943c78c078be59 > src/master/allocator/sorter/drf/sorter.cpp > ed54680cecb637931fc344fbcf8fd3b14cc24295 > src/master/allocator/sorter/sorter.hpp > b3029fcf7342406955760da53f1ae736769f308c > src/tests/hierarchical_allocator_tests.cpp > e343dc37bd7136f0f6dd5dbc22a25cabe715038d > src/tests/master_allocator_tests.cpp > 9f3750215f2b72f6148d0c47cdde6a3f7dfb1b50 > src/tests/sorter_tests.cpp ec0636beb936d46a253d19322f2157abe95156b6 > > > Diff: https://reviews.apache.org/r/57254/diff/16/ > > > Testing > --- > > > Thanks, > > Neil Conway > >
Review Request 58202: Tweaked an incorrect comment in allocator test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58202/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-6940 https://issues.apache.org/jira/browse/MESOS-6940 Repository: mesos Description --- Tweaked an incorrect comment in allocator test. Diffs - src/tests/hierarchical_allocator_tests.cpp e343dc37bd7136f0f6dd5dbc22a25cabe715038d Diff: https://reviews.apache.org/r/58202/diff/1/ Testing --- no functional change Thanks, Jay Guo
Re: Review Request 58095: Refactored functions that render /roles and GetRoles endpoints.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58095/ --- (Updated April 5, 2017, 10:24 a.m.) Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and Michael Park. Changes --- add `MESOS-4732`. add mcypark as reviewer. Bugs: MESOS-4732 and MESOS-7260 https://issues.apache.org/jira/browse/MESOS-4732 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Instead of generating JSON object, `Master::Http::roles()` now leverages `jsonify` to compute output. Also approver is taken out from its continuation function. This is for easier and cleaner implementation of framework authorization in /roles endpoint, see MESOS-7260. Diffs - src/master/http.cpp 6cf9d350446d1b2d4a6e67d552217daff32657ff src/master/master.hpp 1b077424373d6e195e4ab29e150dedbc3f3f95ab Diff: https://reviews.apache.org/r/58095/diff/2/ Testing --- no functional changes make check Thanks, Jay Guo
Re: Review Request 58099: Added authorization for frameworks in `GetRoles` v1 API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58099/ --- (Updated April 5, 2017, 1:47 a.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Added authorization for frameworks in `GetRoles` v1 API. Diffs (updated) - src/master/http.cpp 6cf9d350446d1b2d4a6e67d552217daff32657ff Diff: https://reviews.apache.org/r/58099/diff/2/ Changes: https://reviews.apache.org/r/58099/diff/1-2/ Testing --- Thanks, Jay Guo
Re: Review Request 58097: Added a test to check framework filtering in /roles endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58097/ --- (Updated April 5, 2017, 1:46 a.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Added a test to check framework filtering in /roles endpoint. Diffs (updated) - src/tests/master_authorization_tests.cpp 1a0285a3f345ef21a8256d4123d8bb684f538da4 Diff: https://reviews.apache.org/r/58097/diff/2/ Changes: https://reviews.apache.org/r/58097/diff/1-2/ Testing --- make check Thanks, Jay Guo
Re: Review Request 58096: Added authorization for frameworks in /roles endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58096/ --- (Updated April 5, 2017, 1:46 a.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- While /roles displays a list of frameworksIds that register with a role, it did NOT filter them based on VIEW_FRAMEWORK ACL, which impose a security risk. This patch fixed this issue by taking a frameworksApprover in `Master::Http::roles()` which is used to filter framework IDs. Diffs (updated) - src/master/http.cpp 6cf9d350446d1b2d4a6e67d552217daff32657ff Diff: https://reviews.apache.org/r/58096/diff/2/ Changes: https://reviews.apache.org/r/58096/diff/1-2/ Testing --- see next patch in the chain. Thanks, Jay Guo
Re: Review Request 58095: Refactored functions that render /roles and GetRoles endpoints.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58095/ --- (Updated April 5, 2017, 1:46 a.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Instead of generating JSON object, `Master::Http::roles()` now leverages `jsonify` to compute output. Also approver is taken out from its continuation function. This is for easier and cleaner implementation of framework authorization in /roles endpoint, see MESOS-7260. Diffs (updated) - src/master/http.cpp 6cf9d350446d1b2d4a6e67d552217daff32657ff src/master/master.hpp 1b077424373d6e195e4ab29e150dedbc3f3f95ab Diff: https://reviews.apache.org/r/58095/diff/2/ Changes: https://reviews.apache.org/r/58095/diff/1-2/ Testing --- no functional changes make check Thanks, Jay Guo
Review Request 58099: Added authorization for frameworks in `GetRoles` v1 API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58099/ --- Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Added authorization for frameworks in `GetRoles` v1 API. Diffs - src/master/http.cpp 6cf9d350446d1b2d4a6e67d552217daff32657ff Diff: https://reviews.apache.org/r/58099/diff/1/ Testing --- Thanks, Jay Guo
Review Request 58096: Added authorization for frameworks in /roles endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58096/ --- Review request for mesos, Adam B and Benjamin Mahler. Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- While /roles displays a list of frameworksIds that register with a role, it did NOT filter them based on VIEW_FRAMEWORK ACL, which impose a security risk. This patch fixed this issue by taking a frameworksApprover in `Master::Http::roles()` which is used to filter framework IDs. Diffs - src/master/http.cpp 6cf9d350446d1b2d4a6e67d552217daff32657ff Diff: https://reviews.apache.org/r/58096/diff/1/ Testing --- see next patch in the chain. Thanks, Jay Guo
Review Request 58097: Added a test to check framework filtering in /roles endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58097/ --- Review request for mesos, Adam B and Benjamin Mahler. Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Added a test to check framework filtering in /roles endpoint. Diffs - src/tests/master_authorization_tests.cpp 1a0285a3f345ef21a8256d4123d8bb684f538da4 Diff: https://reviews.apache.org/r/58097/diff/1/ Testing --- make check Thanks, Jay Guo
Review Request 58095: Refactored functions that render /roles and GetRoles endpoints.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58095/ --- Review request for mesos, Adam B and Benjamin Mahler. Bugs: MESOS-7260 https://issues.apache.org/jira/browse/MESOS-7260 Repository: mesos Description --- Instead of generating JSON object, `Master::Http::roles()` now leverages `jsonify` to compute output. Also approver is taken out from its continuation function. This is for easier and cleaner implementation of framework authorization in /roles endpoint, see MESOS-7260. Diffs - src/master/http.cpp 6cf9d350446d1b2d4a6e67d552217daff32657ff src/master/master.hpp 98364d736522cd3c7b1b406dda42d114cd1808e9 Diff: https://reviews.apache.org/r/58095/diff/1/ Testing --- no functional changes make check Thanks, Jay Guo
Review Request 57828: Modified WebUI to display quota under `roles` tab.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57828/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-6995 https://issues.apache.org/jira/browse/MESOS-6995 Repository: mesos Description --- Modified WebUI to display quota under `roles` tab. Diffs - src/webui/master/static/roles.html a8ca03e2533d7bcf5596673c1cd99676bcfb2fae Diff: https://reviews.apache.org/r/57828/diff/1/ Testing --- File Attachments roles quota https://reviews.apache.org/media/uploaded/files/2017/03/22/c26212e8-b28f-439f-b756-1114561a935c__Screen_Shot_2017-03-22_at_14.28.10.png Thanks, Jay Guo
Re: Review Request 57768: Added a test to check /roles endpoint of master includes quota info.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57768/ --- (Updated March 22, 2017, 12:46 p.m.) Review request for mesos and Benjamin Mahler. Changes --- rebase Bugs: MESOS-6995 https://issues.apache.org/jira/browse/MESOS-6995 Repository: mesos Description --- Added a test to check /roles endpoint of master includes quota info. Diffs (updated) - src/tests/role_tests.cpp 1d58d7b37be23d30a37d405c762b274e4e53b409 Diff: https://reviews.apache.org/r/57768/diff/2/ Changes: https://reviews.apache.org/r/57768/diff/1-2/ Testing --- added a test `RoleTest.RolesEndpointContainsQuota` make check on Ubuntu 14.04 Thanks, Jay Guo
Review Request 57768: Added a test to check /roles endpoint of master includes quota info.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57768/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-6995 https://issues.apache.org/jira/browse/MESOS-6995 Repository: mesos Description --- Added a test to check /roles endpoint of master includes quota info. Diffs - src/tests/role_tests.cpp 1d58d7b37be23d30a37d405c762b274e4e53b409 Diff: https://reviews.apache.org/r/57768/diff/1/ Testing --- added a test `RoleTest.RolesEndpointContainsQuota` make check on Ubuntu 14.04 Thanks, Jay Guo
Review Request 57767: Added quota information to /roles endpoint of master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57767/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-6995 https://issues.apache.org/jira/browse/MESOS-6995 Repository: mesos Description --- Added quota information to /roles endpoint of master. Diffs - src/common/http.hpp a3cfc5d8f0b2e453d5f6c3e485e92dbd643737a3 src/common/http.cpp ce32ff36ee58b19f2cb11d80e69ab1ff007e75ef src/master/http.cpp 862b68ff2e8846d6fb968556a6612ec7dab2a61a Diff: https://reviews.apache.org/r/57767/diff/1/ Testing --- see next review in this chain Thanks, Jay Guo
Re: Review Request 57622: Introduced a Roles tab in the webui.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57622/#review169246 --- Ship it! Ship It! - Jay Guo On March 16, 2017, 9:28 a.m., Benjamin Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57622/ > --- > > (Updated March 16, 2017, 9:28 a.m.) > > > Review request for mesos, Jay Guo, haosdent huang, Michael Park, and Neil > Conway. > > > Bugs: MESOS-6995 > https://issues.apache.org/jira/browse/MESOS-6995 > > > Repository: mesos > > > Description > --- > > Initially, this includes the weight, number of frameworks involved > with the role, and the resource allocation. Longer term this should > include the quota information and the revocable resources. > > > Diffs > - > > src/Makefile.am 8c67e472ec2e973b66c0004f6bf58bd8b0e4fad3 > src/webui/master/static/index.html 7811ecb2c8c4fc5d14c4d5ca690b611290b07c83 > src/webui/master/static/js/app.js 73043a8521d2931b639ece11bf3f2b8bccba83d6 > src/webui/master/static/js/controllers.js > ae3e7875258fed9e70b8d15ac82b3e8ffadc6522 > src/webui/master/static/roles.html PRE-CREATION > > > Diff: https://reviews.apache.org/r/57622/diff/4/ > > > Testing > --- > > manual testing. > > > File Attachments > > > screenshot > > https://reviews.apache.org/media/uploaded/files/2017/03/14/2670df08-47f2-4cbf-8c61-1f897c19fa1c__Screen_Shot_2017-03-14_at_3.26.06_PM.png > > > Thanks, > > Benjamin Mahler > >
Re: Review Request 57167: Updated quota handler logic for hierarchical roles.
> On March 16, 2017, 2:54 p.m., Jay Guo wrote: > > What should be the correct behavior for following case: > > - parent role `/a` is quota'd with `cpus:2;mem:1024` > > - child role `/a/b` is quota'd with `cpus:1;mem:512` > > - `Framework1` subscribes to role `/a` > > - `Framework2` subscribes to role `/a/b` > > > > From my understanding: > > - `cpus:1;mem:512` should be offered to `Framework2` > > - remaining `cpus:1;mem:512` should be fairly shared by two frameworks, > > resulting in `cpus:0.5;mem:256` for `Framework1` and `cpus:1.5;mem:768` for > > `Framework2`, since virtual role (which `Framework1` subscribes to) always > > has default quota of zero. > > > > Am I missing something? > > > > Another thought is that we should document the correct way to set quota for > > hierarchical roles (always set quota for parent and then children). My question is that how to quota an internal node, if virtual node always has zero quota. - Jay --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57167/#review169098 --- On March 11, 2017, 6:03 a.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57167/ > --- > > (Updated March 11, 2017, 6:03 a.m.) > > > Review request for mesos and Michael Park. > > > Repository: mesos > > > Description > --- > > The quota'd resources for a nested role are "included" within the > quota'd resources for that role's parent. Hence, the quota of a node > must always be greater than or equal to the sum of the quota'd resources > of that role's children. > > When creating and removing quota, we must ensure that this invariant is > not violated. > > When computing the cluster capacity heuristic, we must ensure that we do > not "double-count" quota'd resources: e.g., if the cluster has a total > capacity of 100 CPUs, role "x" has a quota guarantee of 80 CPUs, and > role "x/y" has a quota guarantee of 40 CPUs, this does NOT violate the > cluster capacity heuristic. > > > Diffs > - > > src/master/quota_handler.cpp 36ea1acca47014b2fb7a3b597b857c8ec9e2ab67 > src/tests/hierarchical_allocator_tests.cpp > dce619ec49db480685deb1bf8f7faeebe02e25b5 > src/tests/master_quota_tests.cpp e418f22ea1773f4356ced44b8d57a80e826c8837 > > > Diff: https://reviews.apache.org/r/57167/diff/4/ > > > Testing > --- > > `make check` > > > Thanks, > > Neil Conway > >
Re: Review Request 57167: Updated quota handler logic for hierarchical roles.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57167/#review169098 --- What should be the correct behavior for following case: - parent role `/a` is quota'd with `cpus:2;mem:1024` - child role `/a/b` is quota'd with `cpus:1;mem:512` - `Framework1` subscribes to role `/a` - `Framework2` subscribes to role `/a/b` >From my understanding: - `cpus:1;mem:512` should be offered to `Framework2` - remaining `cpus:1;mem:512` should be fairly shared by two frameworks, resulting in `cpus:0.5;mem:256` for `Framework1` and `cpus:1.5;mem:768` for `Framework2`, since virtual role (which `Framework1` subscribes to) always has default quota of zero. Am I missing something? Another thought is that we should document the correct way to set quota for hierarchical roles (always set quota for parent and then children). src/tests/master_quota_tests.cpp Lines 1918 (patched) <https://reviews.apache.org/r/57167/#comment241421> s/first/second/ - Jay Guo On March 11, 2017, 6:03 a.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57167/ > --- > > (Updated March 11, 2017, 6:03 a.m.) > > > Review request for mesos and Michael Park. > > > Repository: mesos > > > Description > --- > > The quota'd resources for a nested role are "included" within the > quota'd resources for that role's parent. Hence, the quota of a node > must always be greater than or equal to the sum of the quota'd resources > of that role's children. > > When creating and removing quota, we must ensure that this invariant is > not violated. > > When computing the cluster capacity heuristic, we must ensure that we do > not "double-count" quota'd resources: e.g., if the cluster has a total > capacity of 100 CPUs, role "x" has a quota guarantee of 80 CPUs, and > role "x/y" has a quota guarantee of 40 CPUs, this does NOT violate the > cluster capacity heuristic. > > > Diffs > - > > src/master/quota_handler.cpp 36ea1acca47014b2fb7a3b597b857c8ec9e2ab67 > src/tests/hierarchical_allocator_tests.cpp > dce619ec49db480685deb1bf8f7faeebe02e25b5 > src/tests/master_quota_tests.cpp e418f22ea1773f4356ced44b8d57a80e826c8837 > > > Diff: https://reviews.apache.org/r/57167/diff/4/ > > > Testing > --- > > `make check` > > > Thanks, > > Neil Conway > >
Re: Review Request 57622: Introduced a Roles tab in the webui.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57622/#review168986
---
src/webui/master/static/roles.html
Lines 30-31 (patched)
<https://reviews.apache.org/r/57622/#comment241321>
```
{{role.resources.mem * (1024 * 1024) | dataSize}}
{{role.resources.disk * (1024 * 1024) | dataSize}}
```
- Jay Guo
On March 15, 2017, 10:05 a.m., Benjamin Mahler wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57622/
> ---
>
> (Updated March 15, 2017, 10:05 a.m.)
>
>
> Review request for mesos, Jay Guo, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-6995
> https://issues.apache.org/jira/browse/MESOS-6995
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Initially, this includes the weight, number of frameworks involved
> with the role, and the resource allocation. Longer term this should
> include the quota information and the revocable resources.
>
>
> Diffs
> -
>
> src/Makefile.am 2eea11a4f18f5d0e4ac3cc6bffc8b80f00556d01
> src/webui/master/static/index.html 7811ecb2c8c4fc5d14c4d5ca690b611290b07c83
> src/webui/master/static/js/app.js 73043a8521d2931b639ece11bf3f2b8bccba83d6
> src/webui/master/static/js/controllers.js
> 4e1d07eb6155301c7d7d2b3e030c38156e8210ad
> src/webui/master/static/roles.html PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/57622/diff/3/
>
>
> Testing
> ---
>
> manual testing.
>
>
> File Attachments
>
>
> screenshot
>
> https://reviews.apache.org/media/uploaded/files/2017/03/14/2670df08-47f2-4cbf-8c61-1f897c19fa1c__Screen_Shot_2017-03-14_at_3.26.06_PM.png
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 57254: Updated DRFSorter to support hierarchical roles.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57254/#review168984 --- src/master/allocator/sorter/drf/sorter.cpp Line 72 (original), 95-97 (patched) <https://reviews.apache.org/r/57254/#comment241320> Maybe a `CHECK(!clients.contains(name))` here? - Jay Guo On March 14, 2017, 1:59 p.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57254/ > --- > > (Updated March 14, 2017, 1:59 p.m.) > > > Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Michael Park. > > > Repository: mesos > > > Description > --- > > This commit replaces the sorter's flat list of clients with a tree of > client names; this tree represents the hierarchical relationship between > sorter clients. Each node in the tree contains an (ordered) list of > pointers to child nodes. The tree might contain nodes that do not > correspond directly to sorter clients. For example, adding clients "a/b" > and "c/d" results in the following tree: > > root > -> a >-> b > -> c >-> d > > The `sort` member function still only returns one result for each > (active) client in the sorter. This is implemented by ensuring that each > sorter client is associated with a leaf node in the tree. Note that it > is possible for a leaf node to be transformed into an internal node by a > subsequent insertion; to handle this case, we "implicitly" create an > extra child node, which maintains the invariant that each client has a > leaf node. For example, if the client "a/b/x" is added to the tree > above, the result is: > > root > -> a >-> b > -> . > -> x > -> c >-> d > > The "." leaf node holds the allocation that has been made to the "a/b" > client itself; the "a/b" node holds the sum of all the allocations that > have been made to the subtree rooted at "a/b", which also includes > "a/b/x". > > > Diffs > - > > src/master/allocator/sorter/drf/metrics.cpp > 15aab32db5ca1a7a14080e9bbb7c65283be3ec20 > src/master/allocator/sorter/drf/sorter.hpp > 76329220e1115c1de7810fb69b943c78c078be59 > src/master/allocator/sorter/drf/sorter.cpp > ed54680cecb637931fc344fbcf8fd3b14cc24295 > src/master/allocator/sorter/sorter.hpp > b3029fcf7342406955760da53f1ae736769f308c > src/tests/hierarchical_allocator_tests.cpp > dce619ec49db480685deb1bf8f7faeebe02e25b5 > src/tests/master_allocator_tests.cpp > 9f3750215f2b72f6148d0c47cdde6a3f7dfb1b50 > src/tests/sorter_tests.cpp ec0636beb936d46a253d19322f2157abe95156b6 > > > Diff: https://reviews.apache.org/r/57254/diff/9/ > > > Testing > --- > > > Thanks, > > Neil Conway > >
Re: Review Request 57254: Updated DRFSorter to support hierarchical roles.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57254/#review168982 --- Per design doc, we always associate framework to a virtual role. In this implementation, however, virtual role is created ONLY when the leaf node is turned into internal node. Could you clarify a bit? src/master/allocator/sorter/drf/sorter.cpp Lines 101 (patched) <https://reviews.apache.org/r/57254/#comment241319> It would be nice to add some comments here to help reader understand that we are traversing the tree using each element of `roles`, in order to position it in the tree, much like what `mkdir -p /path/to/create/dir` would do. - Jay Guo On March 14, 2017, 1:59 p.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57254/ > --- > > (Updated March 14, 2017, 1:59 p.m.) > > > Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Michael Park. > > > Repository: mesos > > > Description > --- > > This commit replaces the sorter's flat list of clients with a tree of > client names; this tree represents the hierarchical relationship between > sorter clients. Each node in the tree contains an (ordered) list of > pointers to child nodes. The tree might contain nodes that do not > correspond directly to sorter clients. For example, adding clients "a/b" > and "c/d" results in the following tree: > > root > -> a >-> b > -> c >-> d > > The `sort` member function still only returns one result for each > (active) client in the sorter. This is implemented by ensuring that each > sorter client is associated with a leaf node in the tree. Note that it > is possible for a leaf node to be transformed into an internal node by a > subsequent insertion; to handle this case, we "implicitly" create an > extra child node, which maintains the invariant that each client has a > leaf node. For example, if the client "a/b/x" is added to the tree > above, the result is: > > root > -> a >-> b > -> . > -> x > -> c >-> d > > The "." leaf node holds the allocation that has been made to the "a/b" > client itself; the "a/b" node holds the sum of all the allocations that > have been made to the subtree rooted at "a/b", which also includes > "a/b/x". > > > Diffs > - > > src/master/allocator/sorter/drf/metrics.cpp > 15aab32db5ca1a7a14080e9bbb7c65283be3ec20 > src/master/allocator/sorter/drf/sorter.hpp > 76329220e1115c1de7810fb69b943c78c078be59 > src/master/allocator/sorter/drf/sorter.cpp > ed54680cecb637931fc344fbcf8fd3b14cc24295 > src/master/allocator/sorter/sorter.hpp > b3029fcf7342406955760da53f1ae736769f308c > src/tests/hierarchical_allocator_tests.cpp > dce619ec49db480685deb1bf8f7faeebe02e25b5 > src/tests/master_allocator_tests.cpp > 9f3750215f2b72f6148d0c47cdde6a3f7dfb1b50 > src/tests/sorter_tests.cpp ec0636beb936d46a253d19322f2157abe95156b6 > > > Diff: https://reviews.apache.org/r/57254/diff/9/ > > > Testing > --- > > > Thanks, > > Neil Conway > >
Review Request 57631: Augmented a test to check protobuf::stripAllocationInfo.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57631/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-7048 https://issues.apache.org/jira/browse/MESOS-7048 Repository: mesos Description --- An unit test is augmented to check both injectAllocationInfo and stripAllocationInfo. Diffs - src/tests/protobuf_utils_tests.cpp 2c32d56e41d12a26824aaf22ef78f8c0048acc00 Diff: https://reviews.apache.org/r/57631/diff/1/ Testing --- make check Thanks, Jay Guo
Review Request 57630: Renamed `adjustOfferOperation` to `injectAllocationInfo`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57630/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-7048 https://issues.apache.org/jira/browse/MESOS-7048 Repository: mesos Description --- Method name `protobuf::adjustOfferOperation` is renamed to `protobuf::injectAllocationInfo`. Diffs - src/common/protobuf_utils.hpp 09e468c77f0cdd931302d1bdcc192370b6ce3340 src/common/protobuf_utils.cpp 34c14e8ebd7b575627704c7edebcbb0458eeb3b1 src/master/allocator/mesos/hierarchical.cpp 37fb7a62ef95c2f6e5a9e9a4ab49260332e2b03b src/master/master.cpp d43350d08ddd14fb7ba2a79c899abda6a864038c src/tests/hierarchical_allocator_tests.cpp dce619ec49db480685deb1bf8f7faeebe02e25b5 src/tests/protobuf_utils_tests.cpp 2c32d56e41d12a26824aaf22ef78f8c0048acc00 Diff: https://reviews.apache.org/r/57630/diff/1/ Testing --- Thanks, Jay Guo
Re: Review Request 57340: Remove adjustment code within Resources::apply.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57340/ --- (Updated March 15, 2017, 11:26 a.m.) Review request for mesos and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7048 https://issues.apache.org/jira/browse/MESOS-7048 Repository: mesos Description --- Remove adjustment code within Resources::apply. Diffs (updated) - src/common/protobuf_utils.hpp 09e468c77f0cdd931302d1bdcc192370b6ce3340 src/common/protobuf_utils.cpp 34c14e8ebd7b575627704c7edebcbb0458eeb3b1 src/common/resources.cpp 388e3ef3eabea0dd8d1300e56d493b92b70c75e3 src/master/allocator/mesos/hierarchical.cpp 37fb7a62ef95c2f6e5a9e9a4ab49260332e2b03b src/master/master.cpp d43350d08ddd14fb7ba2a79c899abda6a864038c src/tests/resources_tests.cpp 5ffc9e7111c91157b79a38a1cbd8d58a0565e450 src/v1/resources.cpp e47c4d49e69b915d37e40a91fef69f75dff52463 Diff: https://reviews.apache.org/r/57340/diff/3/ Changes: https://reviews.apache.org/r/57340/diff/2-3/ Testing --- WIP Thanks, Jay Guo
Re: Review Request 57340: Remove adjustment code within Resources::apply.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57340/ --- (Updated March 15, 2017, 1:39 a.m.) Review request for mesos and Benjamin Mahler. Changes --- Address bmahler's comments. Bugs: MESOS-7048 https://issues.apache.org/jira/browse/MESOS-7048 Repository: mesos Description --- Remove adjustment code within Resources::apply. Diffs (updated) - src/common/protobuf_utils.hpp 09e468c77f0cdd931302d1bdcc192370b6ce3340 src/common/protobuf_utils.cpp 34c14e8ebd7b575627704c7edebcbb0458eeb3b1 src/common/resources.cpp 388e3ef3eabea0dd8d1300e56d493b92b70c75e3 src/master/allocator/mesos/hierarchical.cpp 37fb7a62ef95c2f6e5a9e9a4ab49260332e2b03b src/master/master.cpp d43350d08ddd14fb7ba2a79c899abda6a864038c src/tests/resources_tests.cpp 5ffc9e7111c91157b79a38a1cbd8d58a0565e450 src/v1/resources.cpp e47c4d49e69b915d37e40a91fef69f75dff52463 Diff: https://reviews.apache.org/r/57340/diff/2/ Changes: https://reviews.apache.org/r/57340/diff/1-2/ Testing --- WIP Thanks, Jay Guo
Re: Review Request 57340: Remove adjustment code within Resources::apply.
> On March 14, 2017, 7:03 a.m., Benjamin Mahler wrote: > > Looks good, the changes to resources.cpp were done how? Are they a direct > > reversion to the old code? Yes, I did a `git revert` and apply changes based on that. > On March 14, 2017, 7:03 a.m., Benjamin Mahler wrote: > > src/common/protobuf_utils.hpp > > Lines 126-128 (original), 126-128 (patched) > > <https://reviews.apache.org/r/57340/diff/1/?file=1656720#file1656720line126> > > > > I like the naming you used for the strip function, mind also clarifying > > the naming of this one in a separate patch? > > > > ``` > > void injectAllocationInfo( > > Offer::Operation* operation, > > const Resource::AllocationInfo& allocationInfo); > > ``` Will do. - Jay --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57340/#review168539 --- On March 15, 2017, 1:39 a.m., Jay Guo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57340/ > --- > > (Updated March 15, 2017, 1:39 a.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-7048 > https://issues.apache.org/jira/browse/MESOS-7048 > > > Repository: mesos > > > Description > --- > > Remove adjustment code within Resources::apply. > > > Diffs > - > > src/common/protobuf_utils.hpp 09e468c77f0cdd931302d1bdcc192370b6ce3340 > src/common/protobuf_utils.cpp 34c14e8ebd7b575627704c7edebcbb0458eeb3b1 > src/common/resources.cpp 388e3ef3eabea0dd8d1300e56d493b92b70c75e3 > src/master/allocator/mesos/hierarchical.cpp > 37fb7a62ef95c2f6e5a9e9a4ab49260332e2b03b > src/master/master.cpp d43350d08ddd14fb7ba2a79c899abda6a864038c > src/tests/resources_tests.cpp 5ffc9e7111c91157b79a38a1cbd8d58a0565e450 > src/v1/resources.cpp e47c4d49e69b915d37e40a91fef69f75dff52463 > > > Diff: https://reviews.apache.org/r/57340/diff/2/ > > > Testing > --- > > WIP > > > Thanks, > > Jay Guo > >
Review Request 57607: Added `GetRoles` to `GetState` API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57607/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-6855 https://issues.apache.org/jira/browse/MESOS-6855 Repository: mesos Description --- Added `GetRoles` to `GetState` API. Diffs - src/master/http.cpp 862b68ff2e8846d6fb968556a6612ec7dab2a61a src/master/master.hpp d92c8adef79d997f255cf26ebd10ab0e87da8413 Diff: https://reviews.apache.org/r/57607/diff/1/ Testing --- Thanks, Jay Guo
Review Request 57608: Augmented master api test to check `GetRoles` is included.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57608/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-6855 https://issues.apache.org/jira/browse/MESOS-6855 Repository: mesos Description --- Checks that `GetRoles` is included in `GetState` API. Diffs - src/tests/api_tests.cpp 29ae1bcf660fb0e03af1d2192484c9ec739f3ef6 Diff: https://reviews.apache.org/r/57608/diff/1/ Testing --- Thanks, Jay Guo
Review Request 57606: Extracted some logic from `getRoles` into `_getRoles`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57606/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-6855 https://issues.apache.org/jira/browse/MESOS-6855 Repository: mesos Description --- Some logic from `getRoles` is extracted into `_getRoles` so that it could be reused by `_getState`. This is a step toward adding roles to `GetState` V1 API. Diffs - src/master/http.cpp 862b68ff2e8846d6fb968556a6612ec7dab2a61a src/master/master.hpp d92c8adef79d997f255cf26ebd10ab0e87da8413 Diff: https://reviews.apache.org/r/57606/diff/1/ Testing --- Thanks, Jay Guo
Review Request 57605: Added `GetRoles` to `GetState` in master.proto.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57605/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-6855 https://issues.apache.org/jira/browse/MESOS-6855 Repository: mesos Description --- Added `GetRoles` to `GetState` in master.proto. Diffs - include/mesos/master/master.proto 0d8d58983c1ce5553845b0a01df33dca3ac90433 include/mesos/v1/master/master.proto da6f1104fabfe6c09a4cb4ae09ed91bc70827a71 Diff: https://reviews.apache.org/r/57605/diff/1/ Testing --- Thanks, Jay Guo
Review Request 57592: Augmented a master test to check `roles` in `/state` endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57592/ --- Review request for mesos, Benjamin Mahler and Michael Park. Bugs: MESOS-6855 https://issues.apache.org/jira/browse/MESOS-6855 Repository: mesos Description --- Augmented a master test to check `roles` in `/state` endpoint. Diffs - src/tests/master_tests.cpp 5fdb9493c9aed31b90e03062544c1446eb200040 Diff: https://reviews.apache.org/r/57592/diff/1/ Testing --- make check [==] Running 1 test from 1 test case. [--] Global test environment set-up. [--] 1 test from MasterTest [ RUN ] MasterTest.StateEndpointRoles [ OK ] MasterTest.StateEndpointRoles (111 ms) [--] 1 test from MasterTest (112 ms total) [--] Global test environment tear-down [==] 1 test from 1 test case ran. (143 ms total) [ PASSED ] 1 test. Thanks, Jay Guo
Re: Review Request 55253: Added `roles` to `/state` endpoint of master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55253/ --- (Updated March 14, 2017, 6:29 p.m.) Review request for mesos, Benjamin Mahler and Michael Park. Summary (updated) - Added `roles` to `/state` endpoint of master. Bugs: MESOS-6855 https://issues.apache.org/jira/browse/MESOS-6855 Repository: mesos Description (updated) --- `/state` endpoint of master is augmented to include `roles`, which is identical to the response of `/roles`. Diffs (updated) - src/master/http.cpp 862b68ff2e8846d6fb968556a6612ec7dab2a61a Diff: https://reviews.apache.org/r/55253/diff/4/ Changes: https://reviews.apache.org/r/55253/diff/3-4/ Testing --- Thanks, Jay Guo
Re: Review Request 55252: Moved some logic from `Master::Http::_roles` into `filterRoles`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55252/ --- (Updated March 14, 2017, 6:29 p.m.) Review request for mesos, Benjamin Mahler and Michael Park. Summary (updated) - Moved some logic from `Master::Http::_roles` into `filterRoles`. Bugs: MESOS-6855 https://issues.apache.org/jira/browse/MESOS-6855 Repository: mesos Description (updated) --- Moved some logic in `Master::Http::_roles` into a new method `filterRoles`, which could be reused by `Master::Http::state`. Diffs (updated) - src/master/http.cpp 862b68ff2e8846d6fb968556a6612ec7dab2a61a src/master/master.hpp d92c8adef79d997f255cf26ebd10ab0e87da8413 Diff: https://reviews.apache.org/r/55252/diff/3/ Changes: https://reviews.apache.org/r/55252/diff/2-3/ Testing --- Thanks, Jay Guo
Re: Review Request 57269: Added a test to ensure allocation roles are exposed in master API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57269/ --- (Updated March 13, 2017, 10:43 a.m.) Review request for mesos and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7158 https://issues.apache.org/jira/browse/MESOS-7158 Repository: mesos Description --- This test ensures that allocation roles of tasks and executors are exposed via `/state` endpoint of master v0 API. Diffs (updated) - src/tests/master_tests.cpp cacb5566cd40ab57a9c72ad02f75e84c451d08b6 Diff: https://reviews.apache.org/r/57269/diff/5/ Changes: https://reviews.apache.org/r/57269/diff/4-5/ Testing --- Added a test `MasterTest.StateEndpointAllocationRole` make check Thanks, Jay Guo
Re: Review Request 57360: Replace `.get().` in favor of `->` in tests.
sts/master_tests.cpp cacb5566cd40ab57a9c72ad02f75e84c451d08b6 src/tests/master_validation_tests.cpp 11dfbc612ee2c9a39a17dc1ca9bde04eeb65b550 src/tests/metrics_tests.cpp 27fc35f3bc47fc2905f4ce400b1051307dcacb2e src/tests/oversubscription_tests.cpp e57fcc6354b6c5bea4499094513851cd2fd358a0 src/tests/partition_tests.cpp 2f5c6946970c15750ade75002568184db1267ba6 src/tests/persistent_volume_endpoints_tests.cpp 741d62e4a80f34754436e42c6357374bcbb87e32 src/tests/persistent_volume_tests.cpp 7ac82862363af7a33a52b8570149cf2237b3519b src/tests/protobuf_io_tests.cpp 82bffb814a979eceb46531f981053495c7b2bdbc src/tests/rate_limiting_tests.cpp 646848e20ecd0d4934c8b6a7001a229a3fb2935a src/tests/reconciliation_tests.cpp b84a1ab3dfc8a27ccee1ce844b12c646ef2145db src/tests/registrar_tests.cpp fb693ea24ae0786cdb3fc09b4cc9f47d754d4c6a src/tests/registrar_zookeeper_tests.cpp a31942d2bc49821d1d9af070c92cebbe85d79a9f src/tests/reservation_endpoints_tests.cpp ddf279b0177b21a7c369d6a4cb46d86cceec0f98 src/tests/reservation_tests.cpp 95cc9118d437a325141e09e925033604f869abb5 src/tests/resource_offers_tests.cpp 02fb248cc6c9c083b5492d85e8270c826ffd6d68 src/tests/resources_tests.cpp 2bdce3c496108a66308ab6c8484dd171cc6c019f src/tests/role_tests.cpp 77f3d46a544a51ba71476e2f0735bb32758dd9e1 src/tests/scheduler_driver_tests.cpp 3705d13c420e1715645bdef5da6bd2b8763359f4 src/tests/scheduler_event_call_tests.cpp 8ea5eb802b194fc149994560cfc8a6212d351087 src/tests/scheduler_http_api_tests.cpp 083cc3eb2d2701e25b3d6492593ea9ce52a8978a src/tests/scheduler_tests.cpp 1ff423fd5abaffc0e6a0f7fb0edc3e1d5c85fb59 src/tests/script.cpp deec24886526a3a8355ec7cd863ca2236fae5829 src/tests/slave_authorization_tests.cpp 61c44460eb79931dd77c5100eb33a31a182c1710 src/tests/slave_recovery_tests.cpp dc82db990b00b4d6227ab5d8fcc78c1546a52fa1 src/tests/slave_tests.cpp 61f4f42c88ffb18dd0428e9e73a5336314877179 src/tests/state_tests.cpp d200768d536820f4119d3099a60746d5a14ff02d src/tests/status_update_manager_tests.cpp bfb471ff0f6938e112cf7c413fc09c40512318a2 src/tests/upgrade_tests.cpp baf16f8922bfa3e308f359b17233f73b14a28847 src/tests/utils.cpp 0a9e5a867a46795f01fcf7030f50581b5ef1341f src/tests/values_tests.cpp cc17b675d5d3768685b44a1cea64264dcbca80ba src/tests/zookeeper_tests.cpp 72279a3397af7fa15f6ecb3c7adb9d149439e8c8 src/tests/zookeeper_url_tests.cpp 2b6345fd65addcae7d2ed5cc47f1f7396017fb7e Diff: https://reviews.apache.org/r/57360/diff/4/ Changes: https://reviews.apache.org/r/57360/diff/3-4/ Testing --- `s/.get()./->/` under `src/tests/` make check Thanks, Jay Guo
Re: Review Request 57269: Added a test to ensure allocation roles are exposed in master API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57269/ --- (Updated March 10, 2017, 10:18 a.m.) Review request for mesos and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7158 https://issues.apache.org/jira/browse/MESOS-7158 Repository: mesos Description --- This test ensures that allocation roles of tasks and executors are exposed via `/state` endpoint of master v0 API. Diffs (updated) - src/tests/master_tests.cpp cacb5566cd40ab57a9c72ad02f75e84c451d08b6 Diff: https://reviews.apache.org/r/57269/diff/4/ Changes: https://reviews.apache.org/r/57269/diff/3-4/ Testing --- Added a test `MasterTest.StateEndpointAllocationRole` make check Thanks, Jay Guo
Re: Review Request 57360: Replace `.get().` in favor of `->` in tests.
sts/master_tests.cpp cacb5566cd40ab57a9c72ad02f75e84c451d08b6 src/tests/master_validation_tests.cpp 11dfbc612ee2c9a39a17dc1ca9bde04eeb65b550 src/tests/metrics_tests.cpp 27fc35f3bc47fc2905f4ce400b1051307dcacb2e src/tests/oversubscription_tests.cpp e57fcc6354b6c5bea4499094513851cd2fd358a0 src/tests/partition_tests.cpp 2f5c6946970c15750ade75002568184db1267ba6 src/tests/persistent_volume_endpoints_tests.cpp 741d62e4a80f34754436e42c6357374bcbb87e32 src/tests/persistent_volume_tests.cpp 7ac82862363af7a33a52b8570149cf2237b3519b src/tests/protobuf_io_tests.cpp 82bffb814a979eceb46531f981053495c7b2bdbc src/tests/rate_limiting_tests.cpp 646848e20ecd0d4934c8b6a7001a229a3fb2935a src/tests/reconciliation_tests.cpp b84a1ab3dfc8a27ccee1ce844b12c646ef2145db src/tests/registrar_tests.cpp fb693ea24ae0786cdb3fc09b4cc9f47d754d4c6a src/tests/registrar_zookeeper_tests.cpp a31942d2bc49821d1d9af070c92cebbe85d79a9f src/tests/reservation_endpoints_tests.cpp ddf279b0177b21a7c369d6a4cb46d86cceec0f98 src/tests/reservation_tests.cpp 95cc9118d437a325141e09e925033604f869abb5 src/tests/resource_offers_tests.cpp 02fb248cc6c9c083b5492d85e8270c826ffd6d68 src/tests/resources_tests.cpp 2bdce3c496108a66308ab6c8484dd171cc6c019f src/tests/role_tests.cpp 77f3d46a544a51ba71476e2f0735bb32758dd9e1 src/tests/scheduler_driver_tests.cpp 3705d13c420e1715645bdef5da6bd2b8763359f4 src/tests/scheduler_event_call_tests.cpp 8ea5eb802b194fc149994560cfc8a6212d351087 src/tests/scheduler_http_api_tests.cpp 083cc3eb2d2701e25b3d6492593ea9ce52a8978a src/tests/scheduler_tests.cpp 1ff423fd5abaffc0e6a0f7fb0edc3e1d5c85fb59 src/tests/script.cpp deec24886526a3a8355ec7cd863ca2236fae5829 src/tests/slave_authorization_tests.cpp 61c44460eb79931dd77c5100eb33a31a182c1710 src/tests/slave_recovery_tests.cpp dc82db990b00b4d6227ab5d8fcc78c1546a52fa1 src/tests/slave_tests.cpp 61f4f42c88ffb18dd0428e9e73a5336314877179 src/tests/state_tests.cpp d200768d536820f4119d3099a60746d5a14ff02d src/tests/status_update_manager_tests.cpp bfb471ff0f6938e112cf7c413fc09c40512318a2 src/tests/upgrade_tests.cpp baf16f8922bfa3e308f359b17233f73b14a28847 src/tests/utils.cpp 0a9e5a867a46795f01fcf7030f50581b5ef1341f src/tests/values_tests.cpp cc17b675d5d3768685b44a1cea64264dcbca80ba src/tests/zookeeper_tests.cpp 72279a3397af7fa15f6ecb3c7adb9d149439e8c8 src/tests/zookeeper_url_tests.cpp 2b6345fd65addcae7d2ed5cc47f1f7396017fb7e Diff: https://reviews.apache.org/r/57360/diff/3/ Changes: https://reviews.apache.org/r/57360/diff/2-3/ Testing --- `s/.get()./->/` under `src/tests/` make check Thanks, Jay Guo
Re: Review Request 57360: Replace `.get().` in favor of `->` in tests.
sts/master_tests.cpp cacb5566cd40ab57a9c72ad02f75e84c451d08b6 src/tests/master_validation_tests.cpp 11dfbc612ee2c9a39a17dc1ca9bde04eeb65b550 src/tests/metrics_tests.cpp 27fc35f3bc47fc2905f4ce400b1051307dcacb2e src/tests/oversubscription_tests.cpp e57fcc6354b6c5bea4499094513851cd2fd358a0 src/tests/partition_tests.cpp 2f5c6946970c15750ade75002568184db1267ba6 src/tests/persistent_volume_endpoints_tests.cpp 34d569c520db6dfd5da36e5c5931592f6fedbbe0 src/tests/persistent_volume_tests.cpp 7ac82862363af7a33a52b8570149cf2237b3519b src/tests/protobuf_io_tests.cpp 82bffb814a979eceb46531f981053495c7b2bdbc src/tests/rate_limiting_tests.cpp 646848e20ecd0d4934c8b6a7001a229a3fb2935a src/tests/reconciliation_tests.cpp 3573d558ee656ee3e10a25c3b3ed1c36f9fe6a07 src/tests/registrar_tests.cpp fb693ea24ae0786cdb3fc09b4cc9f47d754d4c6a src/tests/registrar_zookeeper_tests.cpp a31942d2bc49821d1d9af070c92cebbe85d79a9f src/tests/reservation_endpoints_tests.cpp 345f0457ec1fc00b7033d71227ff178c14e015bb src/tests/reservation_tests.cpp 5c0d01483efb4561b8c0016c3a2fa6ea5574196e src/tests/resource_offers_tests.cpp 74dacf140e49e402a4ad02ce7751e7c7b2f78ee1 src/tests/resources_tests.cpp 2bdce3c496108a66308ab6c8484dd171cc6c019f src/tests/role_tests.cpp 77f3d46a544a51ba71476e2f0735bb32758dd9e1 src/tests/scheduler_driver_tests.cpp 3705d13c420e1715645bdef5da6bd2b8763359f4 src/tests/scheduler_event_call_tests.cpp 8ea5eb802b194fc149994560cfc8a6212d351087 src/tests/scheduler_http_api_tests.cpp 083cc3eb2d2701e25b3d6492593ea9ce52a8978a src/tests/scheduler_tests.cpp 1ff423fd5abaffc0e6a0f7fb0edc3e1d5c85fb59 src/tests/script.cpp deec24886526a3a8355ec7cd863ca2236fae5829 src/tests/slave_authorization_tests.cpp 61c44460eb79931dd77c5100eb33a31a182c1710 src/tests/slave_recovery_tests.cpp a29b29c5ee31e79174347fae50a2279ca749c0c3 src/tests/slave_tests.cpp ec2cd348412e85c45782c2bdb371e7bd0bb3673d src/tests/state_tests.cpp d200768d536820f4119d3099a60746d5a14ff02d src/tests/status_update_manager_tests.cpp bfb471ff0f6938e112cf7c413fc09c40512318a2 src/tests/upgrade_tests.cpp 0b51a2cd1be39ef4898989c646bebbd83ab4d239 src/tests/utils.cpp 0a9e5a867a46795f01fcf7030f50581b5ef1341f src/tests/values_tests.cpp cc17b675d5d3768685b44a1cea64264dcbca80ba src/tests/zookeeper_tests.cpp 72279a3397af7fa15f6ecb3c7adb9d149439e8c8 src/tests/zookeeper_url_tests.cpp 2b6345fd65addcae7d2ed5cc47f1f7396017fb7e Diff: https://reviews.apache.org/r/57360/diff/2/ Changes: https://reviews.apache.org/r/57360/diff/1-2/ Testing --- `s/.get()./->/` under `src/tests/` make check Thanks, Jay Guo
Re: Review Request 57269: Added a test to ensure allocation roles are exposed in master API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57269/ --- (Updated March 9, 2017, 5:28 p.m.) Review request for mesos and Benjamin Mahler. Changes --- rebase Bugs: MESOS-7158 https://issues.apache.org/jira/browse/MESOS-7158 Repository: mesos Description --- This test ensures that allocation roles of tasks and executors are exposed via `/state` endpoint of master v0 API. Diffs (updated) - src/tests/master_tests.cpp cacb5566cd40ab57a9c72ad02f75e84c451d08b6 Diff: https://reviews.apache.org/r/57269/diff/3/ Changes: https://reviews.apache.org/r/57269/diff/2-3/ Testing --- Added a test `MasterTest.StateEndpointAllocationRole` make check Thanks, Jay Guo
Re: Review Request 57364: Fixed flaky test FaultToleranceTest.FrameworkReregister.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57364/ --- (Updated March 9, 2017, 2:01 p.m.) Review request for mesos and Neil Conway. Changes --- address neilc's comments. Bugs: MESOS-7029 https://issues.apache.org/jira/browse/MESOS-7029 Repository: mesos Description --- Fixed flaky test FaultToleranceTest.FrameworkReregister. Diffs (updated) - src/tests/fault_tolerance_tests.cpp b13a7e2527189931b733fb4f188b1463fe1f919a Diff: https://reviews.apache.org/r/57364/diff/3/ Changes: https://reviews.apache.org/r/57364/diff/2-3/ Testing --- ./bin/mesos-tests.sh --gtest_filter="FaultToleranceTest.FrameworkReregister" --gtest_break_on_failure --gtest_repeat=-1 Thanks, Jay Guo
Re: Review Request 57364: Fixed flaky test FaultToleranceTest.FrameworkReregister.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57364/ --- (Updated March 9, 2017, 10:15 a.m.) Review request for mesos and Neil Conway. Changes --- address neilc's comments. Bugs: MESOS-7029 https://issues.apache.org/jira/browse/MESOS-7029 Repository: mesos Description --- Fixed flaky test FaultToleranceTest.FrameworkReregister. Diffs (updated) - src/tests/fault_tolerance_tests.cpp b13a7e2527189931b733fb4f188b1463fe1f919a Diff: https://reviews.apache.org/r/57364/diff/2/ Changes: https://reviews.apache.org/r/57364/diff/1-2/ Testing --- ./bin/mesos-tests.sh --gtest_filter="FaultToleranceTest.FrameworkReregister" --gtest_break_on_failure --gtest_repeat=-1 Thanks, Jay Guo
Review Request 57364: Fixed flaky test FaultToleranceTest.FrameworkReregister.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57364/ --- Review request for mesos and Neil Conway. Bugs: MESOS-7029 https://issues.apache.org/jira/browse/MESOS-7029 Repository: mesos Description --- Fixed flaky test FaultToleranceTest.FrameworkReregister. Diffs - src/tests/fault_tolerance_tests.cpp b13a7e2527189931b733fb4f188b1463fe1f919a Diff: https://reviews.apache.org/r/57364/diff/1/ Testing --- ./bin/mesos-tests.sh --gtest_filter="FaultToleranceTest.FrameworkReregister" --gtest_break_on_failure --gtest_repeat=-1 Thanks, Jay Guo
