You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2015/01/08 01:45:05 UTC
Review Request 29689: Added tests for verifying DiskUsageChecker.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29689/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Ben Mahler, and Ian Downes.
Bugs: MESOS-1588
https://issues.apache.org/jira/browse/MESOS-1588
Repository: mesos-git
Description
-------
Added tests for verifying DiskUsageChecker.
Diffs
-----
src/Makefile.am 0521f5849acc3237a8fa3970c983beab74441586
src/tests/disk_quota_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/29689/diff/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 29689: Added tests for verifying DiskUsageChecker.
Posted by Jie Yu <yu...@gmail.com>.
> On Jan. 12, 2015, 11:34 p.m., Ben Mahler wrote:
> > Could you pause the clock in these tests? That might help you test that the interval is respected (if you want to add a test for that).
Added a TODO. Pausing the clock is problematic for subprocess related tests.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29689/#review67745
-----------------------------------------------------------
On Jan. 12, 2015, 6:47 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29689/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2015, 6:47 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Ian Downes.
>
>
> Bugs: MESOS-1588
> https://issues.apache.org/jira/browse/MESOS-1588
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added tests for verifying DiskUsageChecker.
>
>
> Diffs
> -----
>
> src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683
> src/tests/disk_quota_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/29689/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 29689: Added tests for verifying DiskUsageChecker.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29689/#review67745
-----------------------------------------------------------
Could you pause the clock in these tests? That might help you test that the interval is respected (if you want to add a test for that).
- Ben Mahler
On Jan. 12, 2015, 6:47 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29689/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2015, 6:47 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Ian Downes.
>
>
> Bugs: MESOS-1588
> https://issues.apache.org/jira/browse/MESOS-1588
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added tests for verifying DiskUsageChecker.
>
>
> Diffs
> -----
>
> src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683
> src/tests/disk_quota_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/29689/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 29689: Added tests for verifying DiskUsageChecker.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29689/#review68373
-----------------------------------------------------------
Ship it!
src/tests/disk_quota_tests.cpp
<https://reviews.apache.org/r/29689/#comment112564>
Kilobytes(8).bytes()?
src/tests/disk_quota_tests.cpp
<https://reviews.apache.org/r/29689/#comment112563>
EXPECT_GE(usage.get(), Kilobytes(8));
Note that for EXPECT_LE, you don't have to put an "expected" as the first argument, unlike EXPECT_EQ or EXPECT_NE.
src/tests/disk_quota_tests.cpp
<https://reviews.apache.org/r/29689/#comment112565>
Use Kilobytes().bytes() here?
src/tests/disk_quota_tests.cpp
<https://reviews.apache.org/r/29689/#comment112566>
EXPECT_GE(usage.get(), Kilobytes(15));
src/tests/disk_quota_tests.cpp
<https://reviews.apache.org/r/29689/#comment112567>
Ditto for using Kilobytes here.
src/tests/disk_quota_tests.cpp
<https://reviews.apache.org/r/29689/#comment112568>
Could you move this towards the top of the file, before any of the tests?
- Ben Mahler
On Jan. 14, 2015, 7:28 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29689/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2015, 7:28 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Ian Downes.
>
>
> Bugs: MESOS-1588
> https://issues.apache.org/jira/browse/MESOS-1588
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added tests for verifying DiskUsageChecker.
>
>
> Diffs
> -----
>
> src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683
> src/tests/disk_quota_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/29689/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 29689: Added tests for verifying DiskUsageChecker.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29689/
-----------------------------------------------------------
(Updated Jan. 14, 2015, 7:28 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, and Ian Downes.
Changes
-------
Review comments.
Bugs: MESOS-1588
https://issues.apache.org/jira/browse/MESOS-1588
Repository: mesos-git
Description
-------
Added tests for verifying DiskUsageChecker.
Diffs (updated)
-----
src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683
src/tests/disk_quota_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/29689/diff/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 29689: Added tests for verifying DiskUsageChecker.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29689/
-----------------------------------------------------------
(Updated Jan. 12, 2015, 6:47 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, and Ian Downes.
Changes
-------
Removed inconsistent tests.
Bugs: MESOS-1588
https://issues.apache.org/jira/browse/MESOS-1588
Repository: mesos-git
Description
-------
Added tests for verifying DiskUsageChecker.
Diffs (updated)
-----
src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683
src/tests/disk_quota_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/29689/diff/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 29689: Added tests for verifying DiskUsageChecker.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29689/
-----------------------------------------------------------
(Updated Jan. 9, 2015, 10:05 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, and Ian Downes.
Changes
-------
Review comments.
Bugs: MESOS-1588
https://issues.apache.org/jira/browse/MESOS-1588
Repository: mesos-git
Description
-------
Added tests for verifying DiskUsageChecker.
Diffs (updated)
-----
src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683
src/tests/disk_quota_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/29689/diff/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 29689: Added tests for verifying DiskUsageChecker.
Posted by Jie Yu <yu...@gmail.com>.
> On Jan. 8, 2015, 7:40 p.m., Ian Downes wrote:
> > src/tests/disk_quota_tests.cpp, line 41
> > <https://reviews.apache.org/r/29689/diff/1/?file=810332#file810332line41>
> >
> > Suggest adding the following tests:
> > 1) Test that an empty directory is > 0
> > [root@hostname test]# ls -la
> > total 8
> > drwxr-xr-x. 2 root root 4096 Jan 8 19:31 .
> > drwxr-xr-x. 5 idownes employee 4096 Jan 8 19:31 ..
> > [root@hostname test]# du -k -s
> > 4 .
> >
> > 2) Test that a directory with empty files is > 0
> >
> > 3) Test that a job that exceeds the quota causes a Limitation and the MesosContainerizer acts upon it.
>
> Ian Downes wrote:
> Oops, missed seeing (3) from the next review, ignore.
In fact, on os x, both 1) and 2) are not true (just tested that).
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29689/#review67263
-----------------------------------------------------------
On Jan. 9, 2015, 10:05 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29689/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2015, 10:05 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Ian Downes.
>
>
> Bugs: MESOS-1588
> https://issues.apache.org/jira/browse/MESOS-1588
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added tests for verifying DiskUsageChecker.
>
>
> Diffs
> -----
>
> src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683
> src/tests/disk_quota_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/29689/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 29689: Added tests for verifying DiskUsageChecker.
Posted by Ian Downes <ia...@gmail.com>.
> On Jan. 8, 2015, 11:40 a.m., Ian Downes wrote:
> > src/tests/disk_quota_tests.cpp, line 41
> > <https://reviews.apache.org/r/29689/diff/1/?file=810332#file810332line41>
> >
> > Suggest adding the following tests:
> > 1) Test that an empty directory is > 0
> > [root@hostname test]# ls -la
> > total 8
> > drwxr-xr-x. 2 root root 4096 Jan 8 19:31 .
> > drwxr-xr-x. 5 idownes employee 4096 Jan 8 19:31 ..
> > [root@hostname test]# du -k -s
> > 4 .
> >
> > 2) Test that a directory with empty files is > 0
> >
> > 3) Test that a job that exceeds the quota causes a Limitation and the MesosContainerizer acts upon it.
Oops, missed seeing (3) from the next review, ignore.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29689/#review67263
-----------------------------------------------------------
On Jan. 7, 2015, 4:45 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29689/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2015, 4:45 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Ian Downes.
>
>
> Bugs: MESOS-1588
> https://issues.apache.org/jira/browse/MESOS-1588
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added tests for verifying DiskUsageChecker.
>
>
> Diffs
> -----
>
> src/Makefile.am 0521f5849acc3237a8fa3970c983beab74441586
> src/tests/disk_quota_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/29689/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 29689: Added tests for verifying DiskUsageChecker.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29689/#review67263
-----------------------------------------------------------
src/tests/disk_quota_tests.cpp
<https://reviews.apache.org/r/29689/#comment111258>
Suggest adding the following tests:
1) Test that an empty directory is > 0
[root@hostname test]# ls -la
total 8
drwxr-xr-x. 2 root root 4096 Jan 8 19:31 .
drwxr-xr-x. 5 idownes employee 4096 Jan 8 19:31 ..
[root@hostname test]# du -k -s
4 .
2) Test that a directory with empty files is > 0
3) Test that a job that exceeds the quota causes a Limitation and the MesosContainerizer acts upon it.
- Ian Downes
On Jan. 7, 2015, 4:45 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29689/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2015, 4:45 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Ian Downes.
>
>
> Bugs: MESOS-1588
> https://issues.apache.org/jira/browse/MESOS-1588
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added tests for verifying DiskUsageChecker.
>
>
> Diffs
> -----
>
> src/Makefile.am 0521f5849acc3237a8fa3970c983beab74441586
> src/tests/disk_quota_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/29689/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 29689: Added tests for verifying DiskUsageChecker.
Posted by Jie Yu <yu...@gmail.com>.
> On Jan. 8, 2015, 8:03 a.m., Timothy Chen wrote:
> > src/tests/disk_quota_tests.cpp, line 55
> > <https://reviews.apache.org/r/29689/diff/1/?file=810332#file810332line55>
> >
> > Why would this be less?
This is a bit tricky because some file system might use more blocks to keep meta data. I don't want to be over restrictive.
(BTW, on the system I've tested, the size if actually 8192B).
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29689/#review67177
-----------------------------------------------------------
On Jan. 8, 2015, 12:45 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29689/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2015, 12:45 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Ian Downes.
>
>
> Bugs: MESOS-1588
> https://issues.apache.org/jira/browse/MESOS-1588
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added tests for verifying DiskUsageChecker.
>
>
> Diffs
> -----
>
> src/Makefile.am 0521f5849acc3237a8fa3970c983beab74441586
> src/tests/disk_quota_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/29689/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 29689: Added tests for verifying DiskUsageChecker.
Posted by Timothy Chen <tn...@apache.org>.
> On Jan. 8, 2015, 8:03 a.m., Timothy Chen wrote:
> > src/tests/disk_quota_tests.cpp, line 55
> > <https://reviews.apache.org/r/29689/diff/1/?file=810332#file810332line55>
> >
> > Why would this be less?
>
> Jie Yu wrote:
> This is a bit tricky because some file system might use more blocks to keep meta data. I don't want to be over restrictive.
>
> (BTW, on the system I've tested, the size if actually 8192B).
I see, maybe add a comment since it wasn't obvious :)
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29689/#review67177
-----------------------------------------------------------
On Jan. 8, 2015, 12:45 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29689/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2015, 12:45 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Ian Downes.
>
>
> Bugs: MESOS-1588
> https://issues.apache.org/jira/browse/MESOS-1588
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added tests for verifying DiskUsageChecker.
>
>
> Diffs
> -----
>
> src/Makefile.am 0521f5849acc3237a8fa3970c983beab74441586
> src/tests/disk_quota_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/29689/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 29689: Added tests for verifying DiskUsageChecker.
Posted by Jie Yu <yu...@gmail.com>.
> On Jan. 8, 2015, 8:03 a.m., Timothy Chen wrote:
> > src/tests/disk_quota_tests.cpp, line 55
> > <https://reviews.apache.org/r/29689/diff/1/?file=810332#file810332line55>
> >
> > Why would this be less?
>
> Jie Yu wrote:
> This is a bit tricky because some file system might use more blocks to keep meta data. I don't want to be over restrictive.
>
> (BTW, on the system I've tested, the size if actually 8192B).
>
> Timothy Chen wrote:
> I see, maybe add a comment since it wasn't obvious :)
Added a comment.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29689/#review67177
-----------------------------------------------------------
On Jan. 8, 2015, 12:45 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29689/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2015, 12:45 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Ian Downes.
>
>
> Bugs: MESOS-1588
> https://issues.apache.org/jira/browse/MESOS-1588
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added tests for verifying DiskUsageChecker.
>
>
> Diffs
> -----
>
> src/Makefile.am 0521f5849acc3237a8fa3970c983beab74441586
> src/tests/disk_quota_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/29689/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 29689: Added tests for verifying DiskUsageChecker.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29689/#review67177
-----------------------------------------------------------
src/tests/disk_quota_tests.cpp
<https://reviews.apache.org/r/29689/#comment111040>
Why would this be less?
- Timothy Chen
On Jan. 8, 2015, 12:45 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29689/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2015, 12:45 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Ian Downes.
>
>
> Bugs: MESOS-1588
> https://issues.apache.org/jira/browse/MESOS-1588
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added tests for verifying DiskUsageChecker.
>
>
> Diffs
> -----
>
> src/Makefile.am 0521f5849acc3237a8fa3970c983beab74441586
> src/tests/disk_quota_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/29689/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>