mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joseph Wu" <jos...@mesosphere.io>
Subject Re: Review Request 38059: Quota: Created a bare pipeline for quota set requests.
Date Tue, 10 Nov 2015 18:14:47 GMT


> On Nov. 5, 2015, 12:24 p.m., Joseph Wu wrote:
> > src/master/quota_handler.cpp, lines 83-88
> > <https://reviews.apache.org/r/38059/diff/13/?file=1116697#file1116697line83>
> >
> >     Why don't you just parse a QuotaInfo object instead of a form-serialized body
(with JSON components)?
> >     
> >     The maintenance endpoints do this for simplicity.
> 
> Alexander Rukletsov wrote:
>     Because we didn't want to expose the internal structure (`QuotaInfo`) to the outside
world. In the future we may want to allow operators to set quotas for multiple roles in one
call, which we can easier do if we do not tie the operator API to `QuotaInfo` protobuf. Does
it make sense?
> 
> Joseph Wu wrote:
>     `QuotaInfo` should already be exposed, since it's in the `include` folder.
>     
>     As for setting multiple `QuotaInfo`s in a single call, wouldn't that be simpler to
implement via a JSON array?
>     (Maybe you can parse the request body as an array, but reject calls with more than
one quota request in the MVP.)
> 
> Alexander Rukletsov wrote:
>     I should have been more precise and should have elaborated more in my first reply.

>     
>     `QuotaInfo` is "mainly" the storage protobuf. It is indeed in the public includes,
but the reason for this is that it's part of the allocator interface. Also we may want to
let frameworks know what the quota of their role is.
>     
>     We do not aim to document the operator API for quota by exposing `QuotaInfo` to them.
We also do not intend to make the operator API for quota 1:1 correspondent to `QuotaInfo`
message. Let's think about post MVP: quota may get optional chunks, limits; it may be set
for a framework. I envision the `QuotaInfo` message becomes a complex protobuf (think `Resource`).
The API for operators should stay simple and flexible, I am reluctant to force operators provide
a JSON view of `QuotaInfo` any time they want to set or update quota. Recalling some of our
discussions, we may even have different endpoints for setting quota (`/master/quota`, `/master/roles/<role>`)!
>     
>     Does it make sense?

That sounds similar to the motivation for having V1 protos (i.e. having a public proto that
gets converted into an internal-but-publically-exposed object).  And even if the API expects
a non-`QuotaInfo` object, it would still be helpful to have a schema, of some sort.  Based
on what's present in the review, it could just be a wrapper around a `Resource` proto.

Side note: Apparently, the `Resource` object is a bad example.  MPark found out yesterday
that you can't take a `Resource` that Mesos outputs (via `/state`) and pass it to an endpoint
that takes `Resource` as an input.  You have to reformat the `Resource`, even if both input/output
are JSON formatted.


- Joseph


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


On Nov. 10, 2015, 8:19 a.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38059/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2015, 8:19 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, and Joseph
Wu.
> 
> 
> Bugs: MESOS-3073
>     https://issues.apache.org/jira/browse/MESOS-3073
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Processing quota request consists of several stages: request validation, sanity check
and so on. This patch creates a basic workflow for quota requests, while the stages are implemented
in subsequent patches.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt c235209743de6e84deb44df31c61948f4dc8b8eb 
>   src/Makefile.am ae2740a5b56351d9fd82ae3bd5c733d10a90bf2f 
>   src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
>   src/master/quota.hpp PRE-CREATION 
>   src/master/quota.cpp PRE-CREATION 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38059/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


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