Re: Review Request 44948: Add XFS disk resource isolator.

2016-04-07 Thread James Peach 

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

(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
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am 71c4308cccfa5c56b93f6c3928dd2a1cf3ba9741 
  src/slave/containerizer/mesos/containerizer.cpp 
a5dd22380066aa85de04d485052084e2629681c0 
  src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
  src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6 
  src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

2016-04-07 Thread Jiang Yan Xu 

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


Ship it!




Ship It!

- Jiang Yan Xu


On April 6, 2016, 3:36 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44948/
> ---
> 
> (Updated April 6, 2016, 3:36 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
> ---
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -
> 
>   src/Makefile.am ba9cc8b683bba2ae8fe9d9c58642690f5b80afaf 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a5dd22380066aa85de04d485052084e2629681c0 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
>   src/slave/flags.hpp 69e1b01e09d2a15bee5e0745b751f47aaefe3fbe 
>   src/slave/flags.cpp 315cf47d268bce0a0255a061d64e414c736c8125 
> 
> Diff: https://reviews.apache.org/r/44948/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44948: Add XFS disk resource isolator.

2016-04-06 Thread James Peach 


> On April 6, 2016, 9:07 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/xfs/disk.cpp, line 407
> > 
> >
> > Should we assume that freeProjectId already doesn't include 
> > `info->projectId`?

No, ``freeProjectIds`` is initialized from ``totalProjectIds``, so we have to 
remove it because it is still considered in use.


- James


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


On April 6, 2016, 6:43 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44948/
> ---
> 
> (Updated April 6, 2016, 6:43 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
> ---
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 55d3b341361bed25f3aa966d77060c88be29e5b0 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a5dd22380066aa85de04d485052084e2629681c0 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
>   src/slave/flags.hpp 69e1b01e09d2a15bee5e0745b751f47aaefe3fbe 
>   src/slave/flags.cpp 315cf47d268bce0a0255a061d64e414c736c8125 
> 
> Diff: https://reviews.apache.org/r/44948/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44948: Add XFS disk resource isolator.

2016-04-06 Thread James Peach 

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

(Updated April 6, 2016, 10:36 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
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am ba9cc8b683bba2ae8fe9d9c58642690f5b80afaf 
  src/slave/containerizer/mesos/containerizer.cpp 
a5dd22380066aa85de04d485052084e2629681c0 
  src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
  src/slave/flags.hpp 69e1b01e09d2a15bee5e0745b751f47aaefe3fbe 
  src/slave/flags.cpp 315cf47d268bce0a0255a061d64e414c736c8125 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

2016-04-06 Thread Jiang Yan Xu 

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



Mostly LGTM. Just a few additional comments.


src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 21 - 22)


Not used anymore.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 26)


This is not used.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 28 - 30)


One blank line above and below as per convention.

And I don't think any of the three are still used?



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 31)


This is not used either.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 38 - 39)


Not used anymore.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 63 - 66)


There is some awkwardness with the -1: we don't use the last element in the 
range so the check needs to be `ranges.range(i).end() - 1` as well.

However, I am thinking if we can just use the inclusive right bounds. It 
seems to me the open right bound is because of the empty interval set check, 
see my comments below.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 69)


s/open/closed/ if we agree on the comment on `nextProjectId()`.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 116)


We should always add a CHECK if we expect it to never be none. In fact 
it'll never be error either because it simply returns `::getuid()`.

It's just being clear that we intentionally don't check other conditions: 
`CHECK_SOME(uid);`, Kinda like "This page is intentionally left blank".

At this point it seems there is not much value to use `os::getuid()` rather 
than `::getuid()` but let's just keep the use of `os::getuid()` with this CHECK 
for the sake of "There should be one-- and preferably only one --obvious way" 
to get uid.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 199)


s/known/alive/

Otherwise it sounds like it encompasses both alive and known orphans and we 
should merge it with `orphans`. Keeping them separate is more explict so we 
should keep `alive` IMO.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 214 - 217)


Unless there is a collision in UUID this should never happen. I trust UUID 
but we can do a CHECK here instead. :)

```
CHECK(!infos.contains(containerId)) << "ContainerIDs should never collide";
```



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 221)


Add a comment here. Somthing along the lines of:

```
// We fail the isolator recovery upon failure in any container because
// failing to get the project ID usually suggests some fatal issue
// on the host.
```



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 234)


s/message/call later/ because it's not a 'message'.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 236)


Add 

```
// Note that we don't wait for the result of the cleanups as we don't 
// want to block agent recovery for unknown orphans.
```



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 406 - 413)


We make a best effort to cleanup but we should still propagate the failure 
to the containerizer so it can do some containerizer-wide logging and metrics 
update.

How about:
```
infos.erase(containerId);

if (quotaStatus.isError() || projectStatus.isError()) {
  return Failure("Failed to cleanup '" + info->directory + "'");
} else {
  returnProjectId(info->projectId);
  return Nothing();
}
```

?



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 407)


Should we assume that freeProjectId already doesn't include 
`info->projectId`?



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 419 - 430)


This seems better:

```
if (freeProjectIds.empty()) {
  return None();
}

prid_t projectId = freeProjectIds.begin()->lower();

Re: Review Request 44948: Add XFS disk resource isolator.

2016-04-06 Thread James Peach 

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

(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
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am 55d3b341361bed25f3aa966d77060c88be29e5b0 
  src/slave/containerizer/mesos/containerizer.cpp 
a5dd22380066aa85de04d485052084e2629681c0 
  src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
  src/slave/flags.hpp 69e1b01e09d2a15bee5e0745b751f47aaefe3fbe 
  src/slave/flags.cpp 315cf47d268bce0a0255a061d64e414c736c8125 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

2016-04-05 Thread James Peach 

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

(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
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/slave/containerizer/mesos/containerizer.cpp 
a5dd22380066aa85de04d485052084e2629681c0 
  src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
  src/slave/flags.hpp 69e1b01e09d2a15bee5e0745b751f47aaefe3fbe 
  src/slave/flags.cpp 315cf47d268bce0a0255a061d64e414c736c8125 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

2016-04-05 Thread James Peach 

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

(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
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/slave/containerizer/mesos/containerizer.cpp 
a5dd22380066aa85de04d485052084e2629681c0 
  src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
  src/slave/flags.hpp 69e1b01e09d2a15bee5e0745b751f47aaefe3fbe 
  src/slave/flags.cpp 315cf47d268bce0a0255a061d64e414c736c8125 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

2016-04-05 Thread James Peach 

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

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


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


Changes
---

Rebased and addresses review comments.


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


Repository: mesos


Description
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/slave/containerizer/mesos/containerizer.cpp 
a5dd22380066aa85de04d485052084e2629681c0 
  src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
  src/slave/flags.hpp d0c606eea74e1a2e69067c43a267047e65a22a04 
  src/slave/flags.cpp 0551ec334c6747507bf7bb068d27d67f3fdd6c83 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

2016-04-05 Thread James Peach 


> On April 5, 2016, 4:52 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/xfs/disk.hpp, line 33
> > 
> >
> > Doesn't look like the utils header is used in this header?
> > 
> > Otherwise add a blank line above.

Added the blank. The header is needed for the definition of ``prid_t``.


> On April 5, 2016, 4:52 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/xfs/disk.cpp, lines 112-114
> > 
> >
> > There is also the `uid.isNone()` case.
> > 
> > We can just 
> > 
> > ```
> > if (!user.isSome()) {
> >   return Error("Failed to determine user: " +
> >(uid.isError() ? uid.error() : "uid not found"));
> > }
> > ```

``getuid(2)`` can't fail or return ``None``.


> On April 5, 2016, 4:52 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/xfs/disk.cpp, lines 281-286
> > 
> >
> > This is sort of safe in our case because we expect an empty sandbox. 
> > 
> > In a general case `xfs::setProjectId()` could partially fail which 
> > makes it unsafe to return the project ID.
> > 
> > I think we can just do a hard CHECK before calling to make sure the 
> > directory is empty. If no such method is directory usable right now, add a 
> > TODO and let's add one to stout.

As discussed, we keep the container Info record and depend on cleanup to return 
the project ID.


- James


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


On April 5, 2016, 10:59 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44948/
> ---
> 
> (Updated April 5, 2016, 10:59 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
> ---
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -
> 
>   src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a5dd22380066aa85de04d485052084e2629681c0 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
>   src/slave/flags.hpp d0c606eea74e1a2e69067c43a267047e65a22a04 
>   src/slave/flags.cpp 0551ec334c6747507bf7bb068d27d67f3fdd6c83 
> 
> Diff: https://reviews.apache.org/r/44948/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44948: Add XFS disk resource isolator.

2016-04-05 Thread Jiang Yan Xu 


> On March 24, 2016, 9:54 a.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.cpp, line 568
> > 
> >
> > It seems that we don't need to modify `totalProjectIds` (which can be a 
> > const) here, this change doesn't survive agent restart anyways and we use 
> > `freeProjectIds` to assign IDs.
> 
> James Peach wrote:
> I think it is better to handle this consistently. We use 
> ``totalProjectIds`` to bound the set of possible IDs and this ID is not 
> longer possible.

I am not sure about the intended consistency here because I can't think of a 
case where in the following place:

```
if (totalProjectIds.contains(projectId)) {
  freeProjectIds += projectId;
}
```

`projectId` could be in `totalProjectIds` (and therefore incorrectly returned) 
if `totalProjectIds` was always the initial value but would not be in 
`totalProjectIds` if we modify `totalProjectIds` like we currently do in the 
code.

Can you think of any?


> On March 24, 2016, 9:54 a.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.cpp, lines 501-503
> > 
> >
> > If the new resources have no disk resources, shouldn't we cleanup the 
> > sandbox?
> 
> James Peach wrote:
> It's not clear to me what this means. If you had a disk resource and now 
> you don't are you unrestricted or do you have zero disk? I think it is better 
> to only remove the project ID in cleanup, but maybe we should still adjust 
> the quota here?

Dropping this for a new suggestion in the new review.


- Jiang Yan


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


On April 4, 2016, 10:28 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44948/
> ---
> 
> (Updated April 4, 2016, 10:28 a.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
> ---
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -
> 
>   src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a5dd22380066aa85de04d485052084e2629681c0 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
>   src/slave/flags.hpp d0c606eea74e1a2e69067c43a267047e65a22a04 
>   src/slave/flags.cpp 0551ec334c6747507bf7bb068d27d67f3fdd6c83 
> 
> Diff: https://reviews.apache.org/r/44948/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44948: Add XFS disk resource isolator.

2016-04-05 Thread Jiang Yan Xu 

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




src/slave/containerizer/mesos/isolators/xfs/disk.hpp (line 33)


Doesn't look like the utils header is used in this header?

Otherwise add a blank line above.



src/slave/containerizer/mesos/isolators/xfs/disk.hpp (line 100)


const Flags flags;



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 57 - 58)


We should probably also validate the upper bound of the ranges because they 
are `uint64`.

```
static Try getIntervalSet(
const Value::Ranges& ranges)
```

?



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 112 - 114)


There is also the `uid.isNone()` case.

We can just 

```
if (!user.isSome()) {
  return Error("Failed to determine user: " +
   (uid.isError() ? uid.error() : "uid not found"));
}
```



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 140 - 144)


I suggested this earlier and this was marked as resolved:

If the utils provdes 

```
Option validateProjectIds(IntervalSet);
```

Then the isolator doesn't need to know about `NON_PROJECT_ID` anymore and 
it can stay as a constant in utils.cpp.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 172 - 199)


We can consolidate this with the orphan case because in both cases we need 
to do all of these to read its project ID and construct an `info`.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 216 - 217)


This fits in one line.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 222)


IMO it's pretty clean and explicit to consolicate the recovery for both 
live containers and known orphans if we add comments here to make it clear that

```
// We recover the sandboxes for both live containers and known orphans 
// and only clean up the unknown orphans here.
```



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 231 - 234)


This case shouldn't happen anymore if we consolidate. We can do a CHECK 
here.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 237)


In the code above you had

```
// We should always be able to get a project ID. Maybe the directory
// is not on an XFS filesystem, or something equally fatal happened.
```

And you return a failure. I think this applies here as well regarless of 
whether the sandbox is alive or known/unkown orphans because this indicates 
problems that affect more than just this one container.

We should return a failure here as well.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 238 - 239)


Single quote the sandbox.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 239)


Move one space to the right.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 243)


Let's move over the comments you had above.

```
// If there is no project ID, don't worry about it. This can happen the
// first time an operator enables the XFS disk isolator and we recover a
// set of containers that we did not isolate.
```



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 253)


I don't think we need to wait for the result of the unknown orphan recovery 
because the cleanup could be expensive as it processes every file in the 
sandbox.

We can call `cleanup(containerId)` for every unknown orphan without 
collecting them. At the bottom we just `return Nothing()`.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 258)


2 space indentation here.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 281 - 286)


This is sort of safe in our case because we expect an empty sandbox. 

In a general case `xfs::setProjectId()` could partially fail which makes it 
unsafe to return the project ID.

I think we can just do a hard CHECK before

Re: Review Request 44948: Add XFS disk resource isolator.

2016-04-04 Thread James Peach 

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

(Updated April 4, 2016, 5:28 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
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/slave/containerizer/mesos/containerizer.cpp 
a5dd22380066aa85de04d485052084e2629681c0 
  src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
  src/slave/flags.hpp d0c606eea74e1a2e69067c43a267047e65a22a04 
  src/slave/flags.cpp 0551ec334c6747507bf7bb068d27d67f3fdd6c83 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

2016-04-01 Thread Jiang Yan Xu 

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




src/slave/containerizer/mesos/isolators/disk/xfs.cpp (lines 501 - 503)


Chatted offline. 

Feels like the right thing to do here is to treat a container with no disk 
resource as `limit == 0`, whether the container started with zero disk resource 
or is updated to zero.

Given XFS' lack of ability to enforce disk quota below a basic block 
(512bytes), I think it's OK to do so and in the user doc make it clear that 
"with XFS isolator when no disk resource or zero disk resource is given, XFS 
will limit it at 512bytes (the size of a basic block)." In fact, XFS cannot do 
enforcement at a granularity below 512bytes. However since Mesos' disk resource 
granularity is more coarse than it (at 1KB, or 0.001 of 1MB), this shouldn't 
violate any expectations.

Given that this is a pretty strict behavior (in most cases the task will 
fail immediately because of stdout/stderr/downloads) I think we should act this 
way based on a flag, maybe only if `flags.enforce_container_disk_quota == 
true`, but it raises another question of whether XFS isolator should support 
monitoring-only mode. Therefore I think it can be punted to a later review, 
I'll raise a JIRA intead.


- Jiang Yan Xu


On March 31, 2016, 5:01 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44948/
> ---
> 
> (Updated March 31, 2016, 5:01 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
> ---
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -
> 
>   src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a5dd22380066aa85de04d485052084e2629681c0 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
>   src/slave/flags.hpp d0c606eea74e1a2e69067c43a267047e65a22a04 
>   src/slave/flags.cpp 0551ec334c6747507bf7bb068d27d67f3fdd6c83 
> 
> Diff: https://reviews.apache.org/r/44948/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44948: Add XFS disk resource isolator.

2016-03-31 Thread James Peach 

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

(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
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/slave/containerizer/mesos/containerizer.cpp 
a5dd22380066aa85de04d485052084e2629681c0 
  src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
  src/slave/flags.hpp d0c606eea74e1a2e69067c43a267047e65a22a04 
  src/slave/flags.cpp 0551ec334c6747507bf7bb068d27d67f3fdd6c83 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

2016-03-30 Thread James Peach 

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

(Updated March 30, 2016, 10:19 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
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/slave/containerizer/mesos/containerizer.cpp 
a5dd22380066aa85de04d485052084e2629681c0 
  src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
  src/slave/flags.hpp 345a2256c8dfcd94ce8233f270b450364b27cf37 
  src/slave/flags.cpp 8868e1ec2db93fa8587e095617548fb923db1fb0 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

2016-03-29 Thread James Peach 


> On March 24, 2016, 4:54 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.cpp, line 95
> > 
> >
> > This method aborts early when a error occurs, I think it's useful to 
> > note this.
> > 
> > Plus, I still think what we need in the isolator is a generic 
> > filesystem walk functionality. I'll help with creating something that 
> > utilize `nftw`: 
> > http://pubs.opengroup.org/onlinepubs/9699919799/functions/nftw.html

Replaced with ``fts(3)``.


> On March 24, 2016, 4:54 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.cpp, lines 501-503
> > 
> >
> > If the new resources have no disk resources, shouldn't we cleanup the 
> > sandbox?

It's not clear to me what this means. If you had a disk resource and now you 
don't are you unrestricted or do you have zero disk? I think it is better to 
only remove the project ID in cleanup, but maybe we should still adjust the 
quota here?


> On March 24, 2016, 4:54 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.cpp, line 568
> > 
> >
> > It seems that we don't need to modify `totalProjectIds` (which can be a 
> > const) here, this change doesn't survive agent restart anyways and we use 
> > `freeProjectIds` to assign IDs.

I think it is better to handle this consistently. We use ``totalProjectIds`` to 
bound the set of possible IDs and this ID is not longer possible.


- James


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


On March 22, 2016, 11:24 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44948/
> ---
> 
> (Updated March 22, 2016, 11:24 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
> ---
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 6552e48eab2708a28dd69adba3ec759cb5aeca4c 
>   src/slave/containerizer/mesos/containerizer.cpp 
> ee7a265975323ca891114a286357c8e42901560c 
>   src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
>   src/slave/flags.hpp 3067c128f1fded4f130f4d45f191584c2f30ad9c 
>   src/slave/flags.cpp ce028825ad99f54a231b4b18dde277b63aa0525c 
> 
> Diff: https://reviews.apache.org/r/44948/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44948: Add XFS disk resource isolator.

2016-03-29 Thread James Peach 

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

(Updated March 29, 2016, 8:54 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
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am 21e2965bb7cd9b88a5c787dd3efe0673c71cdc4f 
  src/slave/containerizer/mesos/containerizer.cpp 
605c52e1118f35232ddf59ee90d2343569061a29 
  src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
  src/slave/flags.hpp 9ee7f349cc2bb3fa76141645f4a06fad57664367 
  src/slave/flags.cpp fd9fbbaa911cc77a21574ba314c50ac226fa49ce 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

2016-03-26 Thread James Peach 


> On March 24, 2016, 4:54 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.cpp, lines 156-157
> > 
> >
> > How does `openat` facilitate DFS better?

If we used ``openat(2)`` we would not need to construct the full path name.


- James


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


On March 22, 2016, 11:24 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44948/
> ---
> 
> (Updated March 22, 2016, 11:24 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
> ---
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 6552e48eab2708a28dd69adba3ec759cb5aeca4c 
>   src/slave/containerizer/mesos/containerizer.cpp 
> ee7a265975323ca891114a286357c8e42901560c 
>   src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
>   src/slave/flags.hpp 3067c128f1fded4f130f4d45f191584c2f30ad9c 
>   src/slave/flags.cpp ce028825ad99f54a231b4b18dde277b63aa0525c 
> 
> Diff: https://reviews.apache.org/r/44948/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44948: Add XFS disk resource isolator.

2016-03-26 Thread James Peach 


> On March 24, 2016, 4:54 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 217
> > 
> >
> > IMO "xfs/disk" is probably better than "disk/xfs" (compared with 
> > "posix/disk") and "xfs/quota" is even better ("posix/quota" would have been 
> > more confusing but xfs being a filesystem, "xfs/quota" seem pretty clear to 
> > me what it means)

I agree that "xfs/disk" would be a reasonable choice.


- James


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


On March 22, 2016, 11:24 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44948/
> ---
> 
> (Updated March 22, 2016, 11:24 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
> ---
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 6552e48eab2708a28dd69adba3ec759cb5aeca4c 
>   src/slave/containerizer/mesos/containerizer.cpp 
> ee7a265975323ca891114a286357c8e42901560c 
>   src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
>   src/slave/flags.hpp 3067c128f1fded4f130f4d45f191584c2f30ad9c 
>   src/slave/flags.cpp ce028825ad99f54a231b4b18dde277b63aa0525c 
> 
> Diff: https://reviews.apache.org/r/44948/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44948: Add XFS disk resource isolator.

2016-03-24 Thread Jiang Yan Xu 

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




src/slave/containerizer/mesos/containerizer.cpp (line 217)


IMO "xfs/disk" is probably better than "disk/xfs" (compared with 
"posix/disk") and "xfs/quota" is even better ("posix/quota" would have been 
more confusing but xfs being a filesystem, "xfs/quota" seem pretty clear to me 
what it means)



src/slave/containerizer/mesos/isolators/disk/xfs.hpp (line 68)


It fits in one line.



src/slave/containerizer/mesos/isolators/disk/xfs.hpp (line 80)


We usually don't omit argument names.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 95)


This method aborts early when a error occurs, I think it's useful to note 
this.

Plus, I still think what we need in the isolator is a generic filesystem 
walk functionality. I'll help with creating something that utilize `nftw`: 
http://pubs.opengroup.org/onlinepubs/9699919799/functions/nftw.html



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 128)


`::dirfd` specially because you use dirfd as function arguments as well.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (lines 156 - 157)


How does `openat` facilitate DFS better?



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (lines 212 - 214)


This helper method tries to do two things and it continues to do the second 
even if the first fails but the error reflects the first if it fails.

It's a bit weird and we don't need this: we can do all this directly in:

```
Future XfsDiskIsolatorProcess::cleanup(
const ContainerID& containerId) {...}
```

And in `recover()` we can call `cleanup(containerId)` directly.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 247)


What's `nbytes`, number of bytes? We could simply call it `result`, to 
avoid name abbreviation.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 278)


We should check if the user is root and the filesystem `flags.work_dir` is 
on is XFS here.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 301)


To eliminate the need to use the sentinel value in the isolator we can add 
a 

```
bool validateProjectIds(IntervalSet);
```

to the utils.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 305)


s/process::Owned/Owned/



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (lines 310 - 312)


Jie mentioned this too: it's customary for the isolators to take and store 
the flags directly, even if we have already derived `projectIds` from the flags.

For `workDir` because it can be used directly from `flags.work_dir` with no 
conversion, we'd just use that in recovery.

The argument for it is not strong but I would advocate for it.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 335)


Single quotes around `state.directory()`.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (lines 337 - 352)


Jie mentioned this and I concur. The XFS API should interpret the sentinel 
value for us.

i.e.

```
Result getProjectId(directory);
```



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 350)


If this is not necessarily an error, I think LOG(WARNING) is more 
appropriate.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (lines 366 - 367)


I don't think this is strictly true because the known orphans in the 
`orphans` list have not fully died yet and could in theory be manipulating 
files and causing race conditions that result in errors in cleanup operations.

I'd follow the commonly used pattern to cleanup only unknown orphans (i.e., 
sandboxes that are neither in `states` or `orphans`) here and leave the 
`orphans` cleanup for the containerizer once it successfully kills the cgroup.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (lines 381 - 382)


This is

Re: Review Request 44948: Add XFS disk resource isolator.

2016-03-22 Thread James Peach 

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

(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
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am 6552e48eab2708a28dd69adba3ec759cb5aeca4c 
  src/slave/containerizer/mesos/containerizer.cpp 
ee7a265975323ca891114a286357c8e42901560c 
  src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
  src/slave/flags.hpp 3067c128f1fded4f130f4d45f191584c2f30ad9c 
  src/slave/flags.cpp ce028825ad99f54a231b4b18dde277b63aa0525c 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

2016-03-21 Thread James Peach 

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

(Updated March 22, 2016, 1:21 a.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
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
  src/slave/containerizer/mesos/containerizer.cpp 
ee7a265975323ca891114a286357c8e42901560c 
  src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

2016-03-21 Thread Jie Yu 


> On March 21, 2016, 3:51 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.cpp, line 320
> > 
> >
> > I am wondering if it's possible to distinguish the Error case from the 
> > case where there's no project ID assigned to the directory.
> > 
> > Can we make xfs::getProjectId(...) return a Result?
> 
> James Peach wrote:
> If there's no project ID assigned, you get a successful ``Try`` with a 
> value of ``0``.

Yes, i understand that part. I think we should make the 0 case more explicit 
(using None() instead of Try<...>(0))


- Jie


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


On March 21, 2016, 9:47 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44948/
> ---
> 
> (Updated March 21, 2016, 9:47 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
> ---
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/slave/containerizer/mesos/containerizer.cpp 
> ee7a265975323ca891114a286357c8e42901560c 
>   src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
> 
> Diff: https://reviews.apache.org/r/44948/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44948: Add XFS disk resource isolator.

2016-03-21 Thread James Peach 


> On March 21, 2016, 3:51 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.cpp, lines 302-303
> > 
> >
> > We typically store the 'flags' directory in case yo u need other 
> > parameters in the future.

Since I don't need all the flags, it seems better to just store the one I want. 
If I needed more than one flag, I agree that storing the whole thing probably 
makes sense.


> On March 21, 2016, 3:51 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.cpp, line 394
> > 
> >
> > Can you have a LOG(ERROR) here?

``cleanupDirectory`` has already logged this error.


> On March 21, 2016, 3:51 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.cpp, line 320
> > 
> >
> > I am wondering if it's possible to distinguish the Error case from the 
> > case where there's no project ID assigned to the directory.
> > 
> > Can we make xfs::getProjectId(...) return a Result?

If there's no project ID assigned, you get a successful ``Try`` with a value of 
``0``.


- James


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


On March 21, 2016, 9:47 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44948/
> ---
> 
> (Updated March 21, 2016, 9:47 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
> ---
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/slave/containerizer/mesos/containerizer.cpp 
> ee7a265975323ca891114a286357c8e42901560c 
>   src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
> 
> Diff: https://reviews.apache.org/r/44948/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44948: Add XFS disk resource isolator.

2016-03-21 Thread James Peach 

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

(Updated March 21, 2016, 9:47 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
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
  src/slave/containerizer/mesos/containerizer.cpp 
ee7a265975323ca891114a286357c8e42901560c 
  src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

2016-03-21 Thread James Peach 

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

(Updated March 21, 2016, 6:21 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
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
  src/slave/containerizer/mesos/containerizer.cpp 
ee7a265975323ca891114a286357c8e42901560c 
  src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

2016-03-20 Thread James Peach 

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

(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
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
  src/slave/containerizer/mesos/containerizer.cpp 
ee7a265975323ca891114a286357c8e42901560c 
  src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

2016-03-19 Thread James Peach 

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

(Updated March 17, 2016, 10:42 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
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
  src/slave/containerizer/mesos/containerizer.cpp 
4638d08328127a8c4ae37555f2be74fe9695da31 
  src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

2016-03-19 Thread James Peach 

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

(Updated March 18, 2016, 3:47 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
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
  src/slave/containerizer/mesos/containerizer.cpp 
4638d08328127a8c4ae37555f2be74fe9695da31 
  src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

2016-03-19 Thread Avinash sridharan 

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


Fix it, then Ship it!




This is just a flyby review. Was looking at it to understand how the xfs disk 
quota isolator works.


src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 511)


This breaks the `Owned` semantics. We shouldn't be copying `Owned` 
pointers. This seems to be working for the time being since we are using 
`shared_ptr` to implement `Owned`. 

May be s/Owned/const Owned&/ ?


- Avinash sridharan


On March 17, 2016, 10:42 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44948/
> ---
> 
> (Updated March 17, 2016, 10:42 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
> ---
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4638d08328127a8c4ae37555f2be74fe9695da31 
>   src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
> 
> Diff: https://reviews.apache.org/r/44948/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44948: Add XFS disk resource isolator.

2016-03-19 Thread Timothy Chen 

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




src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 220)


you should also add notes that we failed to do this operation, otherwise it 
won't be obvious from the log. Also wrap the directories/paths with single 
quotes:

Failed to clear quota for directory '" << directory << "'



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 226)


ditto


- Timothy Chen


On March 17, 2016, 3:38 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44948/
> ---
> 
> (Updated March 17, 2016, 3:38 a.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
> ---
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4638d08328127a8c4ae37555f2be74fe9695da31 
>   src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
> 
> Diff: https://reviews.apache.org/r/44948/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>