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


Can we link this page from `home.md`?

Overall: looks great! Only major feedback is to move the implementation details 
down to the bottom (or remove them), and refactor things slightly so that any 
user-visible behavior (e.g., failover and capacity check heuristic) are 
discussed outside the "implementation details" section.


docs/quota.md (line 7)
<https://reviews.apache.org/r/42040/#comment173907>

    This seems a confusing way to start the document. How can a single 
framework "hog all of the resources?" While I can see how this is possible, I 
think it will be hard for many readers to grasp.
    
    I wonder if we should begin by quickly reviewing the quota-less DRF 
behavior, and showing how one framework can hog all the resoures. Then say how 
quota provides a means to avoid this.



docs/quota.md (line 21)
<https://reviews.apache.org/r/42040/#comment173906>

    Is the analogy between quota and dynamic reservation accurate/helpful? A 
dynamic reservation reserves _particular_ resources on a _specific_ agent, and 
the reservation fails if the resources are unavailable; quota just ensures that 
_sufficient_ resources in one or more agents are dedicated for the role. 
Calling quota a "cluster-wide dynamic reservation" doesn't capture this, IMO.



docs/quota.md (line 26)
<https://reviews.apache.org/r/42040/#comment173908>

    "that manages a Mesos cluster."



docs/quota.md (line 28)
<https://reviews.apache.org/r/42040/#comment173909>

    'In computer science, a "quota" usually..."



docs/quota.md (line 44)
<https://reviews.apache.org/r/42040/#comment173910>

    Period after "100 CPUs", not comma.



docs/quota.md (line 45)
<https://reviews.apache.org/r/42040/#comment173911>

    ", while fB is greedy..."



docs/quota.md (line 46)
<https://reviews.apache.org/r/42040/#comment173912>

    "fair share"
    
    "hogging the remaining 90 CPUs"



docs/quota.md (line 47)
<https://reviews.apache.org/r/42040/#comment173914>

    "the additional"



docs/quota.md (line 52)
<https://reviews.apache.org/r/42040/#comment173916>

    I think we want em-dash ("---") here, without spaces on either side. i.e., 
"resources---100 CPUs."



docs/quota.md (line 53)
<https://reviews.apache.org/r/42040/#comment173917>

    I'd put a comma after "joins the cluster".
    
    I'd say "it will not receive its fair share of the cluster resources (50 
CPUs) until ..."



docs/quota.md (line 56)
<https://reviews.apache.org/r/42040/#comment173918>

    Comma after "Scenario 2".



docs/quota.md (line 57)
<https://reviews.apache.org/r/42040/#comment173919>

    In Scenario 2, it is fB that uses all the resources, not fA.
    
    Remove "the" from "after the fA"



docs/quota.md (line 61)
<https://reviews.apache.org/r/42040/#comment173920>

    These kinds of implementation details belong at the bottom of the document, 
I think -- it is more important to tell the user/operator how to define quota 
before we worry about allocator details.
    
    We could also remove a lot of this information -- the specific steps we 
take to implement a set/remove quota request are not an important thing to 
document (and might change over time).



docs/quota.md (line 76)
<https://reviews.apache.org/r/42040/#comment173923>

    Why capitalize Quota here?
    
    Typically we want "e.g.," not "e.g."



docs/quota.md (line 82)
<https://reviews.apache.org/r/42040/#comment173924>

    You haven't told the reader what "the master endpoint" is.



docs/quota.md (line 106)
<https://reviews.apache.org/r/42040/#comment173925>

    Not sure this belongs in the implementation details section, because it 
influences user-facing behavior.



docs/quota.md (line 133)
<https://reviews.apache.org/r/42040/#comment173926>

    I found this sentence confusing.



docs/quota.md (line 180)
<https://reviews.apache.org/r/42040/#comment173927>

    "enables operators to configure quotas."



docs/quota.md (line 190)
<https://reviews.apache.org/r/42040/#comment173928>

    "set a new one."



docs/quota.md (line 192)
<https://reviews.apache.org/r/42040/#comment173929>

    "See the [...](auth...) for details."



docs/quota.md (line 198)
<https://reviews.apache.org/r/42040/#comment173930>

    "an HTTP", I'd think.



docs/quota.md (line 206)
<https://reviews.apache.org/r/42040/#comment173931>

    "12 CPUs", for consistency with the preceding text in the doc, I'd think.



docs/quota.md (line 226)
<https://reviews.apache.org/r/42040/#comment173933>

    Fix this link -- probably just link to roles.md. Although we don't really 
call the feature "implicit roles" in the user-facing docs (we just talk about 
whether a "role whitelist" has been configured), so maybe we can just remove 
this.



docs/quota.md (line 240)
<https://reviews.apache.org/r/42040/#comment173935>

    "will receive"
    
    "HTTP response codes"



docs/quota.md (line 253)
<https://reviews.apache.org/r/42040/#comment173937>

    "an HTTP"



docs/quota.md (line 254)
<https://reviews.apache.org/r/42040/#comment173938>

    I'd just say "endpoint", we said "HTTP" in this sentence already.



docs/quota.md (line 259)
<https://reviews.apache.org/r/42040/#comment173940>

    "will receive"
    
    "HTTP response codes"



docs/quota.md (line 270)
<https://reviews.apache.org/r/42040/#comment173936>

    "query the configured quotas"



docs/quota.md (line 299)
<https://reviews.apache.org/r/42040/#comment173939>

    "will receive"
    
    "HTTP response codes"



docs/quota.md (line 306)
<https://reviews.apache.org/r/42040/#comment173941>

    "does not allow specifying" ("does not allow to specify" is ungrammatical). 
Similarly below.


- Neil Conway


On Jan. 7, 2016, 11:17 p.m., Joerg Schad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42040/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2016, 11:17 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, Joris Van 
> Remoortere, and Neil Conway.
> 
> 
> Bugs: MESOS-3877
>     https://issues.apache.org/jira/browse/MESOS-3877
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added Quota Operator Documentation.
> 
> 
> Diffs
> -----
> 
>   docs/quota.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42040/diff/
> 
> 
> Testing
> -------
> 
> Rendered version: https://gist.github.com/joerg84/a2c32e25d91e33045b56
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Reply via email to