> On May 12, 2017, 10:13 a.m., Jiang Yan Xu wrote: > > src/slave/containerizer/mesos/isolators/xfs/utils.cpp > > Lines 157-158 (original), 155-156 (patched) > > <https://reviews.apache.org/r/55895/diff/6/?file=1713122#file1713122line157> > > > > Sorry I overlooked this in my last around of review but I had this > > question on > > https://reviews.apache.org/r/55897/diff/2/?file=1667426#file1667426line185: > > > > ``` > > I have questions about the need for soft limit below but I don't recall > > the reason for setting the soft limits earlier "just for consistency". Soft > > limits is one of the low-level system's funtionality that we didn't use. If > > we didn't use it, I am not sure about the need for our util or the reader > > to be aware of it (the concept of soft limit)? > > ``` > > > > Can we get rid of the soft limits? > > James Peach wrote: > We set the soft limit because otherwise it would look weird to have a > hard limit but no soft limit. It's easy to set and I don't see any upside in > leaving it unset, do you?
How is it weird? These are two different concepts and we are not using soft limits. IIUC to soft limits depend on hard limits but not vice versa. I as a reader needed to read additional docs to understand what it is and as a reviewer had to reason about whether the code is using soft limits correctly. The behavior of soft limits depend on other varibles which are not explicitly set or documented here and seemingly it's only because we are setting the two with the same value that the soft limits don't kick in. Code with no effect means redundancy but my point is more this is an additional concept that we don't need to understand. There are other fields and concepts in http://xfs.org/docs/xfsdocs-xml-dev/XFS_Filesystem_Structure/tmp/en-US/html/Internal_Inodes.html and other features provieded by XFS that I'd be happy to be ignorant of. :) - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55895/#review174763 ----------------------------------------------------------- On May 9, 2017, 4:59 p.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55895/ > ----------------------------------------------------------- > > (Updated May 9, 2017, 4:59 p.m.) > > > Review request for mesos, Jie Yu and Jiang Yan Xu. > > > Bugs: MESOS-5116 > https://issues.apache.org/jira/browse/MESOS-5116 > > > Repository: mesos > > > Description > ------- > > Extract a BasicBlocks class to handle disk blocks to clarify block-based > arithmetic in the XFS disk isolator. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/xfs/utils.hpp > eddd4c37fb42339ca21ecb392dea47acf6b277bb > src/slave/containerizer/mesos/isolators/xfs/utils.cpp > 8018ad348d26bd962357543a5fb9f6cb43ff13b1 > src/tests/containerizer/xfs_quota_tests.cpp > 7beb60b059910a0d4451b1ace895a35dc974a043 > > > Diff: https://reviews.apache.org/r/55895/diff/6/ > > > Testing > ------- > > sudo make check (Fedora 25) > > > Thanks, > > James Peach > >
