Re: Review Request 59088: Implemented QuotaTree structure to be used by allocator.

[Jay Guo]

> On May 15, 2017, 8:45 p.m., Alexander Rojas wrote:
> > src/master/quota.hpp
> > Lines 56 (patched)
> > 
> >
> > 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.

[Jay Guo]

> On May 15, 2017, 8:20 p.m., Alexander Rojas wrote:
> > src/master/quota.hpp
> > Lines 56 (patched)
> > 
> >
> > 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.

[Alexander Rojas]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59088/#review174943
---




src/master/quota.hpp
Lines 56 (patched)


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
> 
>

Re: Review Request 59088: Implemented QuotaTree structure to be used by allocator.

[Alexander Rojas]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59088/#review174941
---




src/master/quota.hpp
Lines 56 (patched)


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
> 
>

Review Request 59088: Implemented QuotaTree structure to be used by allocator.

[Jay Guo]

---
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