You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by fei long <ca...@gmail.com> on 2018/03/13 13:05:11 UTC
Review Request 66038: Add usageWithInode to check both disk and inode
usage.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66038/
-----------------------------------------------------------
Review request for mesos and Gilbert Song.
Bugs: MESOS-8665
https://issues.apache.org/jira/browse/MESOS-8665
Repository: mesos
Description
-------
Add fs::usageWithInode to check both disk and inode usage, which returns max(diskUsage, inodeUsage).
Diffs
-----
3rdparty/stout/include/stout/posix/fs.hpp 269a4f50f1df8d68be9e11030f885cf2c254c9d8
Diff: https://reviews.apache.org/r/66038/diff/1/
Testing
-------
No new tests are added and all "make check" tests are passed.
Thanks,
fei long
Re: Review Request 66038: Add inodeUsage to provide inode usage.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66038/#review201170
-----------------------------------------------------------
Ship it!
Ship It!
- Andrew Schwartzmeyer
On April 14, 2018, 1:43 a.m., fei long wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66038/
> -----------------------------------------------------------
>
> (Updated April 14, 2018, 1:43 a.m.)
>
>
> Review request for mesos and Gilbert Song.
>
>
> Bugs: MESOS-8665
> https://issues.apache.org/jira/browse/MESOS-8665
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add inodeUsage to provide inode usage.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/posix/fs.hpp 269a4f50f1df8d68be9e11030f885cf2c254c9d8
>
>
> Diff: https://reviews.apache.org/r/66038/diff/3/
>
>
> Testing
> -------
>
> No new tests are added and all "make check" tests are passed.
>
>
> Thanks,
>
> fei long
>
>
Re: Review Request 66038: Add inodeUsage to provide inode usage.
Posted by fei long <ca...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66038/
-----------------------------------------------------------
(Updated April 14, 2018, 8:43 a.m.)
Review request for mesos and Gilbert Song.
Changes
-------
Use inodeUsage instead of usageWithInode. Return 0 if f_files is 0.
Summary (updated)
-----------------
Add inodeUsage to provide inode usage.
Bugs: MESOS-8665
https://issues.apache.org/jira/browse/MESOS-8665
Repository: mesos
Description (updated)
-------
Add inodeUsage to provide inode usage.
Diffs (updated)
-----
3rdparty/stout/include/stout/posix/fs.hpp 269a4f50f1df8d68be9e11030f885cf2c254c9d8
Diff: https://reviews.apache.org/r/66038/diff/3/
Changes: https://reviews.apache.org/r/66038/diff/2-3/
Testing
-------
No new tests are added and all "make check" tests are passed.
Thanks,
fei long
Re: Review Request 66038: Add usageWithInode to check both disk and
inode usage.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66038/#review200427
-----------------------------------------------------------
3rdparty/stout/include/stout/posix/fs.hpp
Lines 55 (patched)
<https://reviews.apache.org/r/66038/#comment281190>
as a library in stout, I would suggest to avoid adding `std::max`. Instead, we may want to just implement a function called `inodeUsage()`.
We could move the std::max to the caller size, which is more explicit.
Thoughts?
- Gilbert Song
On March 13, 2018, 7:08 a.m., fei long wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66038/
> -----------------------------------------------------------
>
> (Updated March 13, 2018, 7:08 a.m.)
>
>
> Review request for mesos and Gilbert Song.
>
>
> Bugs: MESOS-8665
> https://issues.apache.org/jira/browse/MESOS-8665
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add usageWithInode to check both disk and inode usage.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/posix/fs.hpp 269a4f50f1df8d68be9e11030f885cf2c254c9d8
>
>
> Diff: https://reviews.apache.org/r/66038/diff/2/
>
>
> Testing
> -------
>
> No new tests are added and all "make check" tests are passed.
>
>
> Thanks,
>
> fei long
>
>
Re: Review Request 66038: Add usageWithInode to check both disk and
inode usage.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66038/#review200751
-----------------------------------------------------------
3rdparty/stout/include/stout/posix/fs.hpp
Lines 61-62 (patched)
<https://reviews.apache.org/r/66038/#comment281603>
Nit-picking but `static_cast` please to avoid warnings :)
- Andrew Schwartzmeyer
On March 13, 2018, 7:08 a.m., fei long wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66038/
> -----------------------------------------------------------
>
> (Updated March 13, 2018, 7:08 a.m.)
>
>
> Review request for mesos and Gilbert Song.
>
>
> Bugs: MESOS-8665
> https://issues.apache.org/jira/browse/MESOS-8665
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add usageWithInode to check both disk and inode usage.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/posix/fs.hpp 269a4f50f1df8d68be9e11030f885cf2c254c9d8
>
>
> Diff: https://reviews.apache.org/r/66038/diff/2/
>
>
> Testing
> -------
>
> No new tests are added and all "make check" tests are passed.
>
>
> Thanks,
>
> fei long
>
>
Re: Review Request 66038: Add usageWithInode to check both disk and
inode usage.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66038/#review200754
-----------------------------------------------------------
3rdparty/stout/include/stout/posix/fs.hpp
Lines 62 (patched)
<https://reviews.apache.org/r/66038/#comment281607>
I was curious, and checked what this would do for an NTFS volume mounted via cifs on Linux. It reports 0/0, and this would then divide by zero.
```
> df -i
Filesystem Inodes IUsed IFree IUse% Mounted on
/dev/mapper/andschwa--ubuntu--vg-root 16482304 379480 16102824 3% /
//andschwa-pc/public 0 0 0 - /mnt/public
```
Note the last line; total/used/free inodes are all zero. I could see this being the case for other non-POSIX filesystems (or network shares) that get mounted in Linux anyway.
We should check if `f_files` is zero before dividing by it.
- Andrew Schwartzmeyer
On March 13, 2018, 7:08 a.m., fei long wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66038/
> -----------------------------------------------------------
>
> (Updated March 13, 2018, 7:08 a.m.)
>
>
> Review request for mesos and Gilbert Song.
>
>
> Bugs: MESOS-8665
> https://issues.apache.org/jira/browse/MESOS-8665
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add usageWithInode to check both disk and inode usage.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/posix/fs.hpp 269a4f50f1df8d68be9e11030f885cf2c254c9d8
>
>
> Diff: https://reviews.apache.org/r/66038/diff/2/
>
>
> Testing
> -------
>
> No new tests are added and all "make check" tests are passed.
>
>
> Thanks,
>
> fei long
>
>
Re: Review Request 66038: Add usageWithInode to check both disk and
inode usage.
Posted by fei long <ca...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66038/
-----------------------------------------------------------
(Updated March 13, 2018, 2:08 p.m.)
Review request for mesos and Gilbert Song.
Bugs: MESOS-8665
https://issues.apache.org/jira/browse/MESOS-8665
Repository: mesos
Description (updated)
-------
Add usageWithInode to check both disk and inode usage.
Diffs (updated)
-----
3rdparty/stout/include/stout/posix/fs.hpp 269a4f50f1df8d68be9e11030f885cf2c254c9d8
Diff: https://reviews.apache.org/r/66038/diff/2/
Changes: https://reviews.apache.org/r/66038/diff/1-2/
Testing
-------
No new tests are added and all "make check" tests are passed.
Thanks,
fei long