mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Zhitao Li <zhitaoli...@gmail.com>
Subject Re: Review Request 52103: Implement quota update through `PUT` method.
Date Sat, 12 Nov 2016 08:21:58 GMT


> On Oct. 20, 2016, 3:53 p.m., Alexander Rukletsov wrote:
> > What about updating "quota.md"? How a quota update request will look like for an
operator? I assume the same as quota set, but we should call it out in the docs.

Doc update and operator API implementation will sent with a follow up patch.


> On Oct. 20, 2016, 3:53 p.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1140-1143
> > <https://reviews.apache.org/r/52103/diff/1/?file=1506783#file1506783line1140>
> >
> >     Even if we leave one function in the allocator for setting *and* updating quota
(I'm not convinced yet) we should, don't you think it is valuable to leave most of this comment
so people understand why there are two separate paths?

After some more thoughts, I think adding a new function to allocator interface for `updateQuota`
seems cleaner than reusing this function, especially if more quota related enhance in the
future could make the two case deviate further.

Do you think this is worth a new interface function on allcator interface?


> On Oct. 20, 2016, 3:53 p.m., Alexander Rukletsov wrote:
> > src/master/master.hpp, lines 1028-1032
> > <https://reviews.apache.org/r/52103/diff/1/?file=1506785#file1506785line1028>
> >
> >     We definitely need an override taking a master call, hence we should update
the operator API as well. Feel free to do it either here or as a separate commit. You can
look into https://reviews.apache.org/r/49247 for inspiration.

Will do in a follow up diff.


> On Oct. 20, 2016, 3:53 p.m., Alexander Rukletsov wrote:
> > src/master/quota_handler.cpp, lines 560-563
> > <https://reviews.apache.org/r/52103/diff/1/?file=1506786#file1506786line560>
> >
> >     This function is almost identical to `_set()`. Have you explored the possibility
to calculate the delta (new - old) and re-use the set path if the delta > 0 and remove
path otherwise?
> >     
> >     If we can't express update internally in terms of set and remove, we should
better maintain a separate update path (including allocator) for simplicity.

I perfer a separate update path. I think reusing `set` and `remove` path is somehow breaking
the current allocator interface without too much benefit, and I'm not sure we will end up
with more readable code.


- Zhitao


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


On Oct. 19, 2016, 11:28 a.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52103/
> -----------------------------------------------------------
> 
> (Updated Oct. 19, 2016, 11:28 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Xiaojian Huang.
> 
> 
> Bugs: MESOS-4941
>     https://issues.apache.org/jira/browse/MESOS-4941
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implement quota update through `PUT` method.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp 2d56bd011f2c87c67a02d0ae467a4a537d36867e

>   src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 
>   src/master/master.hpp 35db198748b8652eb53e17f592f6b40d1e6a3ed9 
>   src/master/quota_handler.cpp bf6a613a7bb3c62fd77d1ffde3170749d6c21fa2 
>   src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 
> 
> Diff: https://reviews.apache.org/r/52103/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message