You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jay Guo <gu...@gmail.com> on 2017/05/09 09:49:48 UTC
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 59088: Implemented QuotaTree structure to be used
by allocator.
Posted by Jay Guo <gu...@gmail.com>.
> 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 59088: Implemented QuotaTree structure to be used
by allocator.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59088/#review174941
-----------------------------------------------------------
src/master/quota.hpp
Lines 56 (patched)
<https://reviews.apache.org/r/59088/#comment248232>
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?
- Alexander Rojas
On May 9, 2017, 11:49 a.m., Jay Guo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59088/
> -----------------------------------------------------------
>
> (Updated May 9, 2017, 11:49 a.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.
Posted by Jay Guo <gu...@gmail.com>.
> 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.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59088/#review174943
-----------------------------------------------------------
src/master/quota.hpp
Lines 56 (patched)
<https://reviews.apache.org/r/59088/#comment248235>
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.
- Alexander Rojas
On May 9, 2017, 11:49 a.m., Jay Guo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59088/
> -----------------------------------------------------------
>
> (Updated May 9, 2017, 11:49 a.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
>
>