You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by James Peach <jp...@apache.org> on 2018/08/16 23:51:57 UTC
Review Request 68399: Exposed a constant for the agent persistent
volumes directory.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68399/
-----------------------------------------------------------
Review request for mesos, Chun-Hung Hsiao, Ilya Pronin, Jie Yu, Joseph Wu, and Jiang Yan Xu.
Bugs: MESOS-5158
https://issues.apache.org/jira/browse/MESOS-5158
Repository: mesos
Description
-------
The `disk/xfs` isolator needs to scan persistent volumes at recovery
time. Add a `VOLUMES_DIR` constant to make it easier to find the
relevant isolator code.
Diffs
-----
src/slave/paths.hpp 015896453410a33923eed07b3e676be19af62a48
src/slave/paths.cpp ed0b1276908f4990ce7a24c96aea20e8c79d3126
Diff: https://reviews.apache.org/r/68399/diff/1/
Testing
-------
sudo make check (Fedora 28)
Thanks,
James Peach
Re: Review Request 68399: Exposed a helper to access for the agent
persistent volumes directories.
Posted by Ilya Pronin <ip...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68399/#review208008
-----------------------------------------------------------
Ship it!
Ship It!
- Ilya Pronin
On Aug. 24, 2018, 4:24 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68399/
> -----------------------------------------------------------
>
> (Updated Aug. 24, 2018, 4:24 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao, Ilya Pronin, Jie Yu, Joseph Wu, and Jiang Yan Xu.
>
>
> Bugs: MESOS-5158
> https://issues.apache.org/jira/browse/MESOS-5158
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The `disk/xfs` isolator needs to scan persistent volumes at recovery
> time. Add a helper function to return the relevant set of directories.
>
>
> Diffs
> -----
>
> src/slave/paths.hpp 015896453410a33923eed07b3e676be19af62a48
> src/slave/paths.cpp ed0b1276908f4990ce7a24c96aea20e8c79d3126
>
>
> Diff: https://reviews.apache.org/r/68399/diff/2/
>
>
> Testing
> -------
>
> sudo make check (Fedora 28)
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 68399: Exposed a helper to access for the agent
persistent volumes directories.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68399/#review208005
-----------------------------------------------------------
Ship it!
Ship It!
- Chun-Hung Hsiao
On Aug. 24, 2018, 11:24 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68399/
> -----------------------------------------------------------
>
> (Updated Aug. 24, 2018, 11:24 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao, Ilya Pronin, Jie Yu, Joseph Wu, and Jiang Yan Xu.
>
>
> Bugs: MESOS-5158
> https://issues.apache.org/jira/browse/MESOS-5158
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The `disk/xfs` isolator needs to scan persistent volumes at recovery
> time. Add a helper function to return the relevant set of directories.
>
>
> Diffs
> -----
>
> src/slave/paths.hpp 015896453410a33923eed07b3e676be19af62a48
> src/slave/paths.cpp ed0b1276908f4990ce7a24c96aea20e8c79d3126
>
>
> Diff: https://reviews.apache.org/r/68399/diff/2/
>
>
> Testing
> -------
>
> sudo make check (Fedora 28)
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 68399: Exposed a helper to access for the agent
persistent volumes directories.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68399/
-----------------------------------------------------------
(Updated Aug. 24, 2018, 11:24 p.m.)
Review request for mesos, Chun-Hung Hsiao, Ilya Pronin, Jie Yu, Joseph Wu, and Jiang Yan Xu.
Summary (updated)
-----------------
Exposed a helper to access for the agent persistent volumes directories.
Bugs: MESOS-5158
https://issues.apache.org/jira/browse/MESOS-5158
Repository: mesos
Description (updated)
-------
The `disk/xfs` isolator needs to scan persistent volumes at recovery
time. Add a helper function to return the relevant set of directories.
Diffs (updated)
-----
src/slave/paths.hpp 015896453410a33923eed07b3e676be19af62a48
src/slave/paths.cpp ed0b1276908f4990ce7a24c96aea20e8c79d3126
Diff: https://reviews.apache.org/r/68399/diff/2/
Changes: https://reviews.apache.org/r/68399/diff/1-2/
Testing
-------
sudo make check (Fedora 28)
Thanks,
James Peach
Re: Review Request 68399: Exposed a constant for the agent persistent
volumes directory.
Posted by James Peach <jp...@apache.org>.
> On Aug. 23, 2018, 1:33 a.m., Chun-Hung Hsiao wrote:
> > src/slave/paths.hpp
> > Lines 424 (patched)
> > <https://reviews.apache.org/r/68399/diff/1/?file=2074096#file2074096line424>
> >
> > It looks good to me to introduce this new constant, but instead of exporting it, it seems better to me if we keep this constant internal and introduce the following function:
> >
> > ```
> > Try<list<string>> getPersistentVolumePaths(
> > const string& workDir)
> > {
> > return fs::list(path::join(
> > workDir,
> > VOLUMES_DIR,
> > ROLES_DIR,
> > "*", // Role.
> > "*")); // Persistence ID.
> > }
> > ```
> > So we could have a single place to control the filesystem layout.
> >
> > Also, it seems you need `"roles"` in the subsequent patch and I was wondering why there is no corresponding constant. ;)
Yeh I considered that approach but arbitrarily went with the constant. I'll make this change.
- James
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68399/#review207787
-----------------------------------------------------------
On Aug. 16, 2018, 11:51 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68399/
> -----------------------------------------------------------
>
> (Updated Aug. 16, 2018, 11:51 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao, Ilya Pronin, Jie Yu, Joseph Wu, and Jiang Yan Xu.
>
>
> Bugs: MESOS-5158
> https://issues.apache.org/jira/browse/MESOS-5158
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The `disk/xfs` isolator needs to scan persistent volumes at recovery
> time. Add a `VOLUMES_DIR` constant to make it easier to find the
> relevant isolator code.
>
>
> Diffs
> -----
>
> src/slave/paths.hpp 015896453410a33923eed07b3e676be19af62a48
> src/slave/paths.cpp ed0b1276908f4990ce7a24c96aea20e8c79d3126
>
>
> Diff: https://reviews.apache.org/r/68399/diff/1/
>
>
> Testing
> -------
>
> sudo make check (Fedora 28)
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 68399: Exposed a constant for the agent persistent
volumes directory.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68399/#review207787
-----------------------------------------------------------
src/slave/paths.hpp
Lines 424 (patched)
<https://reviews.apache.org/r/68399/#comment291288>
It looks good to me to introduce this new constant, but instead of exporting it, it seems better to me if we keep this constant internal and introduce the following function:
```
Try<list<string>> getPersistentVolumePaths(
const string& workDir)
{
return fs::list(path::join(
workDir,
VOLUMES_DIR,
ROLES_DIR,
"*", // Role.
"*")); // Persistence ID.
}
```
So we could have a single place to control the filesystem layout.
Also, it seems you need `"roles"` in the subsequent patch and I was wondering why there is no corresponding constant. ;)
- Chun-Hung Hsiao
On Aug. 16, 2018, 11:51 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68399/
> -----------------------------------------------------------
>
> (Updated Aug. 16, 2018, 11:51 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao, Ilya Pronin, Jie Yu, Joseph Wu, and Jiang Yan Xu.
>
>
> Bugs: MESOS-5158
> https://issues.apache.org/jira/browse/MESOS-5158
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The `disk/xfs` isolator needs to scan persistent volumes at recovery
> time. Add a `VOLUMES_DIR` constant to make it easier to find the
> relevant isolator code.
>
>
> Diffs
> -----
>
> src/slave/paths.hpp 015896453410a33923eed07b3e676be19af62a48
> src/slave/paths.cpp ed0b1276908f4990ce7a24c96aea20e8c79d3126
>
>
> Diff: https://reviews.apache.org/r/68399/diff/1/
>
>
> Testing
> -------
>
> sudo make check (Fedora 28)
>
>
> Thanks,
>
> James Peach
>
>