mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Peach <jpe...@apache.org>
Subject Re: Review Request 66173: Add test for new `disk/xfs` kill functionality.
Date Tue, 27 Mar 2018 18:44:17 GMT

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



Running the test, I get this failure:
```
../../src/tests/containerizer/xfs_quota_tests.cpp:533: Failure
Failed to wait 15secs for killedStatus
```

Which indicates that the task doesn't get killed.

We should also add the following tests:

1. Verify that `flags.xfs_kill_containers` doesn't kill containers that don't violate there
limits. You can do this by snsuring that you can kill the container after guaranteeing that
at least one disk space usage check has elapsed (this is an argument for keeping the isolator
`check` functio, since you can expect it to be called).
2. Add soft limit checks to QuotaGetSet.
3. Add soft limit checks to QuotaLimit.


src/tests/containerizer/xfs_quota_tests.cpp
Line 147 (original), 147 (patched)
<https://reviews.apache.org/r/66173/#comment280684>

    Since we are now pausing the clock, we need to make sure that it is resumed so that the
`losetup` exec works correctly:
    
    ```
        // Make sure we resume the clock so that we can wait on the
        // `losetup` process.
        if (Clock::paused()) {
          Clock::resume();
        }
    ```



src/tests/containerizer/xfs_quota_tests.cpp
Lines 457 (patched)
<https://reviews.apache.org/r/66173/#comment280679>

    Double newline above here.



src/tests/containerizer/xfs_quota_tests.cpp
Lines 460 (patched)
<https://reviews.apache.org/r/66173/#comment280677>

    Need to update this comment for accuracy.



src/tests/containerizer/xfs_quota_tests.cpp
Lines 469 (patched)
<https://reviews.apache.org/r/66173/#comment280678>

    Newline before and after this to make it stand out.



src/tests/containerizer/xfs_quota_tests.cpp
Lines 498 (patched)
<https://reviews.apache.org/r/66173/#comment280670>

    We can just sleep:
    ```
    "dd if=/dev/zero of=file bs=1048576 count=2 && sleep 100000"
    ```
    
    and update the comment.



src/tests/containerizer/xfs_quota_tests.cpp
Lines 517 (patched)
<https://reviews.apache.org/r/66173/#comment280682>

    So that you don't have to wait for the full check interval, you can advance the clock:
    ```
    // Create TaskInfo ...
    
    Clock::pause();
    
    // Expect TASK_RUNNING ...
    
    Clock::advance(flags.container_disk_watch_interval);
    Clock::settle();
    Clock::resume();
    
    // Expect more stuff ...
    
    ```



src/tests/containerizer/xfs_quota_tests.cpp
Lines 524 (patched)
<https://reviews.apache.org/r/66173/#comment280673>

    Here we should check that the resource in the limitation is what we expect:
    
    ```
     EXPECT_EQ(TaskStatus::SOURCE_SLAVE, killedStatus->source());
     EXPECT_EQ(
         TaskStatus::REASON_CONTAINER_LIMITATION_DISK, killedStatus->reason());
         
     ASSERT_TRUE(killedStatus->has_limitation()) << JSON::protobuf(killedStatus.get());
     
     Resources limit = Resources(killedStatus->limitation().resources());
     
     // Expect that we were limited on a single disk resource that represents
     // the amount of disk that the task consumed.
     EXPECT_EQ(1u, limit.size());
     EXPECT_SOME_EQ(Megabytes(2), limit.disk());
     
    ```



src/tests/containerizer/xfs_quota_tests.cpp
Lines 528 (patched)
<https://reviews.apache.org/r/66173/#comment280680>

    Double newline after here.


- James Peach


On March 22, 2018, 2:11 p.m., Harold Dost wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66173/
> -----------------------------------------------------------
> 
> (Updated March 22, 2018, 2:11 p.m.)
> 
> 
> Review request for mesos and James Peach.
> 
> 
> Bugs: MESOS-6575
>     https://issues.apache.org/jira/browse/MESOS-6575
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> MESOS-6575
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/xfs_quota_tests.cpp 64c3e1c3f0bc435897626cb0a13bc19c7cb1a4fe

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


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