mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Harold Dost <h.d...@criteo.com>
Subject Re: Review Request 66001: MESOS-6575: Add soft limit and kill to disk/xfs.
Date Wed, 04 Apr 2018 13:04:29 GMT


> On March 27, 2018, 4:35 p.m., James Peach wrote:
> > src/slave/containerizer/mesos/isolators/xfs/disk.cpp
> > Line 362 (original), 414 (patched)
> > <https://reviews.apache.org/r/66001/diff/11/?file=1986464#file1986464line418>
> >
> >     `needed` is still the allocated quota so you don't need to use `hardLimit` here.

So the reason I did it this way was that the quota which is being shown should (I believe)
reflect the amount of space which is actually being provided vs what is being requested. If
not then I can change this.


> On March 27, 2018, 4:35 p.m., James Peach wrote:
> > src/slave/containerizer/mesos/isolators/xfs/disk.cpp
> > Lines 440 (patched)
> > <https://reviews.apache.org/r/66001/diff/11/?file=1986464#file1986464line444>
> >
> >     I don't think we need this log message.

Yea, this was not supposed to get committed.


> On March 27, 2018, 4:35 p.m., James Peach wrote:
> > src/slave/containerizer/mesos/isolators/xfs/utils.cpp
> > Lines 274 (patched)
> > <https://reviews.apache.org/r/66001/diff/11/?file=1986466#file1986466line278>
> >
> >     Remove whitespace between `(` and `"`. Use a consistent error message, i.e.:
> >     
> >     ```
> >     if (hardLimit == 0) {
> >       return Error("Quota hard limit must be greater than 0");
> >     }
> >     
> >     if (softLimit == 0) {
> >       return Error("Quota soft limit must be greater than 0");
> >     }
> >     ```

So I know that we aren't using it this way, but I was more thinking from an API perspective
that it might make to have one or the other to be 0, but not both, but I take your point.


- Harold


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


On March 23, 2018, 4:13 p.m., Harold Dost wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66001/
> -----------------------------------------------------------
> 
> (Updated March 23, 2018, 4:13 p.m.)
> 
> 
> Review request for mesos and James Peach.
> 
> 
> Bugs: MESOS-6575
>     https://issues.apache.org/jira/browse/MESOS-6575
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> New Flags for disk/xfs isolator:
> - xfs_kill_containers - This will create a 10 MB buffer between the soft
>   and hard limits, and when the soft limit is exceeded it will
>   subsequently be killed.
> 
> Functionality
> - This by default disabled behavior allows for the `disk/xfs` isolator
>   to kill containers which surpass their limit.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp 07e68a777aefba4dd35066f2eb207bba7f199d83

>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp 8d9f8f846866f9de377c59cb7fb311041283ba70

>   src/slave/containerizer/mesos/isolators/xfs/utils.hpp e034133629a9c1cf58b776f8da2a93421332cee0

>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp 2708524add1ff693b616d4fb241c4a0a3070520b

>   src/slave/flags.hpp 949a4783caf8aac9a246a98525a5287b0f8256d8 
>   src/slave/flags.cpp dd8dfb7a8a9f7c6030939c9eea841eb47deadfc4 
>   src/tests/containerizer/xfs_quota_tests.cpp 64c3e1c3f0bc435897626cb0a13bc19c7cb1a4fe

> 
> 
> Diff: https://reviews.apache.org/r/66001/diff/11/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Harold Dost
> 
>


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