You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Qian Zhang <zh...@gmail.com> on 2018/12/10 17:07:44 UTC
Review Request 69543: Implemented recovery for volume gid manager.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69543/
-----------------------------------------------------------
Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.
Bugs: MESOS-8810
https://issues.apache.org/jira/browse/MESOS-8810
Repository: mesos
Description
-------
Implemented recovery for volume gid manager.
Diffs
-----
src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8
src/Makefile.am 35500516e18ff251357b9e8d6bba894c44a9253b
src/slave/containerizer/mesos/containerizer.cpp a5cf2da55c046c5c45e0c2ca3400f64de12de62b
src/slave/containerizer/mesos/volume_gid_manager/state.hpp PRE-CREATION
src/slave/containerizer/mesos/volume_gid_manager/state.proto PRE-CREATION
src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.hpp PRE-CREATION
src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/69543/diff/1/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 69543: Implemented recovery for volume gid manager.
Posted by Qian Zhang <zh...@gmail.com>.
> On Dec. 12, 2018, 2:38 a.m., Andrei Budnik wrote:
> > src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp
> > Lines 168 (patched)
> > <https://reviews.apache.org/r/69543/diff/1/?file=2112801#file2112801line168>
> >
> > Consider adding `gidRanges` to the `message VolumeGidInfos` (rename it?), so that the whole state will be stored in a single file. This requires storing original gid ranges as a const class variable and modifying `persist()` method.
I updated the patches by not checkpointing the gid ranges, we should allow operators to specify any gid ranges as they want.
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69543/#review211200
-----------------------------------------------------------
On Dec. 18, 2018, 4:09 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69543/
> -----------------------------------------------------------
>
> (Updated Dec. 18, 2018, 4:09 p.m.)
>
>
> Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.
>
>
> Bugs: MESOS-8810
> https://issues.apache.org/jira/browse/MESOS-8810
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Implemented recovery for volume gid manager.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8
> src/Makefile.am 35500516e18ff251357b9e8d6bba894c44a9253b
> src/slave/containerizer/mesos/containerizer.cpp a5cf2da55c046c5c45e0c2ca3400f64de12de62b
> src/slave/containerizer/mesos/volume_gid_manager/state.hpp PRE-CREATION
> src/slave/containerizer/mesos/volume_gid_manager/state.proto PRE-CREATION
> src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.hpp PRE-CREATION
> src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69543/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 69543: Implemented recovery for volume gid manager.
Posted by Andrei Budnik <ab...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69543/#review211200
-----------------------------------------------------------
src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp
Lines 168 (patched)
<https://reviews.apache.org/r/69543/#comment296131>
Consider adding `gidRanges` to the `message VolumeGidInfos` (rename it?), so that the whole state will be stored in a single file. This requires storing original gid ranges as a const class variable and modifying `persist()` method.
src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp
Lines 224 (patched)
<https://reviews.apache.org/r/69543/#comment296130>
Can we add `CHECK(!volumeGidInfo.container_ids().empty());`?
- Andrei Budnik
On Dec. 10, 2018, 5:07 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69543/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2018, 5:07 p.m.)
>
>
> Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.
>
>
> Bugs: MESOS-8810
> https://issues.apache.org/jira/browse/MESOS-8810
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Implemented recovery for volume gid manager.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8
> src/Makefile.am 35500516e18ff251357b9e8d6bba894c44a9253b
> src/slave/containerizer/mesos/containerizer.cpp a5cf2da55c046c5c45e0c2ca3400f64de12de62b
> src/slave/containerizer/mesos/volume_gid_manager/state.hpp PRE-CREATION
> src/slave/containerizer/mesos/volume_gid_manager/state.proto PRE-CREATION
> src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.hpp PRE-CREATION
> src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69543/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 69543: Implemented recovery for volume gid manager.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69543/
-----------------------------------------------------------
(Updated Dec. 20, 2018, 9:36 a.m.)
Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.
Changes
-------
Addressed review comments.
Bugs: MESOS-8810
https://issues.apache.org/jira/browse/MESOS-8810
Repository: mesos
Description
-------
Implemented recovery for volume gid manager.
Diffs (updated)
-----
src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8
src/Makefile.am 7a4904a3d67479267087fd2313a263d8218843fa
src/slave/containerizer/mesos/containerizer.cpp a5cf2da55c046c5c45e0c2ca3400f64de12de62b
src/slave/containerizer/mesos/volume_gid_manager/state.hpp PRE-CREATION
src/slave/containerizer/mesos/volume_gid_manager/state.proto PRE-CREATION
src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.hpp PRE-CREATION
src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/69543/diff/3/
Changes: https://reviews.apache.org/r/69543/diff/2-3/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 69543: Implemented recovery for volume gid manager.
Posted by Andrei Budnik <ab...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69543/#review211409
-----------------------------------------------------------
Fix it, then Ship it!
Ship It!
src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp
Lines 52 (patched)
<https://reviews.apache.org/r/69543/#comment296488>
We should either get rid of `VOLUME_GIDS_FILE` constant or introduce a new constant for `volume_gid_manager`.
src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp
Lines 338 (patched)
<https://reviews.apache.org/r/69543/#comment296487>
Can we add `const` qualifier to `persist()`?
`Try<Nothing> persist() const`
- Andrei Budnik
On Dec. 18, 2018, 8:09 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69543/
> -----------------------------------------------------------
>
> (Updated Dec. 18, 2018, 8:09 a.m.)
>
>
> Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.
>
>
> Bugs: MESOS-8810
> https://issues.apache.org/jira/browse/MESOS-8810
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Implemented recovery for volume gid manager.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8
> src/Makefile.am 35500516e18ff251357b9e8d6bba894c44a9253b
> src/slave/containerizer/mesos/containerizer.cpp a5cf2da55c046c5c45e0c2ca3400f64de12de62b
> src/slave/containerizer/mesos/volume_gid_manager/state.hpp PRE-CREATION
> src/slave/containerizer/mesos/volume_gid_manager/state.proto PRE-CREATION
> src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.hpp PRE-CREATION
> src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69543/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 69543: Implemented recovery for volume gid manager.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69543/
-----------------------------------------------------------
(Updated Dec. 18, 2018, 4:09 p.m.)
Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.
Changes
-------
Do not checkpoint gid ranges.
Bugs: MESOS-8810
https://issues.apache.org/jira/browse/MESOS-8810
Repository: mesos
Description
-------
Implemented recovery for volume gid manager.
Diffs (updated)
-----
src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8
src/Makefile.am 35500516e18ff251357b9e8d6bba894c44a9253b
src/slave/containerizer/mesos/containerizer.cpp a5cf2da55c046c5c45e0c2ca3400f64de12de62b
src/slave/containerizer/mesos/volume_gid_manager/state.hpp PRE-CREATION
src/slave/containerizer/mesos/volume_gid_manager/state.proto PRE-CREATION
src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.hpp PRE-CREATION
src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/69543/diff/2/
Changes: https://reviews.apache.org/r/69543/diff/1-2/
Testing
-------
Thanks,
Qian Zhang