Re: Review Request 44949: Add XFS disk isolator tests.

[Jiang Yan Xu]

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


Ship it!




Only some nits which I'll fix when committing.


src/tests/containerizer/xfs_quota_tests.cpp (line 344)


s/that/than/.



src/tests/containerizer/xfs_quota_tests.cpp (line 345)


s/consumed/consume/.



src/tests/containerizer/xfs_quota_tests.cpp (line 597)


A little explanaion of `2u` would be make it clearer.



src/tests/containerizer/xfs_quota_tests.cpp (line 654)


Let's do `count=1`, same as the previous test.


- Jiang Yan Xu


On April 7, 2016, 2:50 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44949/
> ---
> 
> (Updated April 7, 2016, 2:50 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
> https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add basic XFS disk isolator tests by cloning the POSIX disk isolator
> tests and making minor changes for the differences in semantics.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44949/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44949: Add XFS disk isolator tests.

[Jiang Yan Xu]

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


Ship it!




Ship It!

- Jiang Yan Xu


On April 7, 2016, 2:50 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44949/
> ---
> 
> (Updated April 7, 2016, 2:50 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
> https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add basic XFS disk isolator tests by cloning the POSIX disk isolator
> tests and making minor changes for the differences in semantics.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44949/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated April 7, 2016, 9:50 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


Bugs: MESOS-4828
https://issues.apache.org/jira/browse/MESOS-4828


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44949/diff/


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44949: Add XFS disk isolator tests.

[Jiang Yan Xu]

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




src/tests/containerizer/xfs_quota_tests.cpp (line 338)


These more complex and integration oriented tests should all have a header 
comment so it makes it easier for 

Something like:
```
// This test verifies that a task fails to write to disk when it 
// exceeds the requested disk quota.
```



src/tests/containerizer/xfs_quota_tests.cpp (line 345)


Two spaces to continue after `=`.

Here and elsewhere in this test.



src/tests/containerizer/xfs_quota_tests.cpp (line 357)


One space between `;` and `//`.

I saw some other test code that does this but it's probably due to copying 
and pasting of bad examples. Unless it's to align with some other comments, use 
one space.

Here and elsewhere.



src/tests/containerizer/xfs_quota_tests.cpp (line 390)


s/a/an.



src/tests/containerizer/xfs_quota_tests.cpp (line 425)


`s/Future/Future/` here and elsewhere.



src/tests/containerizer/xfs_quota_tests.cpp (line 461)


`Timeout::in()` and `Timeout::expired()` is more suitable for this.



src/tests/containerizer/xfs_quota_tests.cpp (line 486)


With Timeout you don't need to do this.



src/tests/containerizer/xfs_quota_tests.cpp (lines 526 - 531)


Here we seem to be implying that we are expecting the task to die because 
it exceeds the disk quota but in fact we are just testing slave recovery and 
the task doesn't die because of the `;`.

How about we just request 1MB and use 1MB (or not use anything at all (just 
sleep) since we are mainly verifying if the project ID is cleared afterwards.



src/tests/containerizer/xfs_quota_tests.cpp (line 545)


Two spaces.



src/tests/containerizer/xfs_quota_tests.cpp (line 551)


This is not guaranteed if `dd` is slow to write to the disk and the rest of 
the test proceeds quickly.

I think it's OK if we don't verify this here because the two tests above 
already verified that disk usage is correctly reported and correctly capped at 
the limit.



src/tests/containerizer/xfs_quota_tests.cpp (lines 565 - 566)


This is not necessary if we

```
AWAIT_READY(slaveReregisteredMessage);
```

because reregistration only happens after `Slave::_recover`.



src/tests/containerizer/xfs_quota_tests.cpp (line 590)


Additionally we should assert there is exactly one container. Otherwise the 
foreach below can fall through without triggering any expectations if 
`sandboxees` is empty.



src/tests/containerizer/xfs_quota_tests.cpp (line 593)


`s/foreach(/foreach (/`



src/tests/containerizer/xfs_quota_tests.cpp (lines 680 - 691)


This part is not necessary. Slave considers itself fully recovered when the 
containers are fully recovered, not when the executors have all reregistered. 
Also the exchange of reconnect and reregister messages doesn't go through a 
delay so no need to advance (only the `reregisterExecutorTimeout` goes through 
a delay).

Simply removing these lines should do it.



src/tests/containerizer/xfs_quota_tests.cpp (line 702)


This is again not guaranteed. I think it's good enough if we just verify 
the limits.



src/tests/containerizer/xfs_quota_tests.cpp (line 718)


`s/foreach(/foreach (/`


- Jiang Yan Xu


On April 6, 2016, 3:37 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44949/
> ---
> 
> (Updated April 6, 2016, 3:37 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
> https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add basic XFS disk isolator tests by cloning the POSIX disk isolator
> tests and making minor changes for the differences in 

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated April 6, 2016, 10:37 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


Bugs: MESOS-4828
https://issues.apache.org/jira/browse/MESOS-4828


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44949/diff/


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated April 6, 2016, 6:43 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


Bugs: MESOS-4828
https://issues.apache.org/jira/browse/MESOS-4828


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44949/diff/


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated April 5, 2016, 11:41 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


Bugs: MESOS-4828
https://issues.apache.org/jira/browse/MESOS-4828


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44949/diff/


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated April 5, 2016, 11:15 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


Bugs: MESOS-4828
https://issues.apache.org/jira/browse/MESOS-4828


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44949/diff/


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated April 5, 2016, 10:59 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


Bugs: MESOS-4828
https://issues.apache.org/jira/browse/MESOS-4828


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44949/diff/


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated April 4, 2016, 5:28 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased and addressed some review comments.


Bugs: MESOS-4828
https://issues.apache.org/jira/browse/MESOS-4828


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44949/diff/


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated April 1, 2016, 12:01 a.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


Bugs: MESOS-4828
https://issues.apache.org/jira/browse/MESOS-4828


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44949/diff/


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated March 29, 2016, 8:52 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased and updated.


Bugs: MESOS-4828
https://issues.apache.org/jira/browse/MESOS-4828


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44949/diff/


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated March 26, 2016, 5:06 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


Bugs: MESOS-4828
https://issues.apache.org/jira/browse/MESOS-4828


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44949/diff/


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated March 22, 2016, 11:24 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


Bugs: MESOS-4828
https://issues.apache.org/jira/browse/MESOS-4828


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44949/diff/


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated March 21, 2016, 9:46 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased and fixed review comments.


Bugs: MESOS-4828
https://issues.apache.org/jira/browse/MESOS-4828


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44949/diff/


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated March 21, 2016, 6:20 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased. Added --enable-xfs-disk-isolator.


Bugs: MESOS-4828
https://issues.apache.org/jira/browse/MESOS-4828


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44949/diff/


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated March 21, 2016, 2 a.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


Bugs: MESOS-4828
https://issues.apache.org/jira/browse/MESOS-4828


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44949/diff/


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated March 18, 2016, 3:46 a.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


Bugs: MESOS-4828
https://issues.apache.org/jira/browse/MESOS-4828


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44949/diff/


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated March 17, 2016, 10:42 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


Bugs: MESOS-4828
https://issues.apache.org/jira/browse/MESOS-4828


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44949/diff/


Testing
---

Make check. Manual testing.


Thanks,

James Peach