You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2018/08/13 23:03:32 UTC

Re: Review Request 68334: Used state::checkpoint instead in CNI isolator.

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

(Updated Aug. 13, 2018, 11:03 p.m.)


Review request for mesos, Deepak Goel, Qian Zhang, and Sergey Urbanovich.


Summary (updated)
-----------------

Used state::checkpoint instead in CNI isolator.


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


Repository: mesos


Description
-------

This is to ensure all or nothing semantics. We don't want to deal with a
particially written file in case agent crashes.


Diffs (updated)
-----

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp f46c962d8f044092aaa113fafc536c6b25bab45c 


Diff: https://reviews.apache.org/r/68334/diff/2/

Changes: https://reviews.apache.org/r/68334/diff/1-2/


Testing
-------

sudo make check


Thanks,

Jie Yu


Re: Review Request 68334: Used state::checkpoint instead in CNI isolator.

Posted by Sergey Urbanovich <se...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68334/#review207206
-----------------------------------------------------------


Ship it!




????

- Sergey Urbanovich


On Aug. 13, 2018, 11:04 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68334/
> -----------------------------------------------------------
> 
> (Updated Aug. 13, 2018, 11:04 p.m.)
> 
> 
> Review request for mesos, Deepak Goel, Qian Zhang, and Sergey Urbanovich.
> 
> 
> Bugs: MESOS-9142
>     https://issues.apache.org/jira/browse/MESOS-9142
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is to ensure all or nothing semantics. We don't want to deal with a
> particially written file in case agent crashes.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp f46c962d8f044092aaa113fafc536c6b25bab45c 
> 
> 
> Diff: https://reviews.apache.org/r/68334/diff/3/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 68334: Used state::checkpoint instead in CNI isolator.

Posted by Qian Zhang <zh...@gmail.com>.

> On Aug. 14, 2018, 3:30 p.m., Qian Zhang wrote:
> > Ship It!

BTW, what about the other files (i.e., hosts, hostname and resolv.conf) that CNI isolator persists in the container dir? Should we change from `os::write()` to `state::checkpoint()` for them too?


- Qian


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


On Aug. 14, 2018, 7:04 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68334/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2018, 7:04 a.m.)
> 
> 
> Review request for mesos, Deepak Goel, Qian Zhang, and Sergey Urbanovich.
> 
> 
> Bugs: MESOS-9142
>     https://issues.apache.org/jira/browse/MESOS-9142
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is to ensure all or nothing semantics. We don't want to deal with a
> particially written file in case agent crashes.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp f46c962d8f044092aaa113fafc536c6b25bab45c 
> 
> 
> Diff: https://reviews.apache.org/r/68334/diff/3/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 68334: Used state::checkpoint instead in CNI isolator.

Posted by Jie Yu <yu...@gmail.com>.

> On Aug. 14, 2018, 7:30 a.m., Qian Zhang wrote:
> > Ship It!
> 
> Qian Zhang wrote:
>     BTW, what about the other files (i.e., hosts, hostname and resolv.conf) that CNI isolator persists in the container dir? Should we change from `os::write()` to `state::checkpoint()` for them too?

Those are not needed because it won't be used after recovery. Those are only used during launching.


- Jie


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


On Aug. 13, 2018, 11:04 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68334/
> -----------------------------------------------------------
> 
> (Updated Aug. 13, 2018, 11:04 p.m.)
> 
> 
> Review request for mesos, Deepak Goel, Qian Zhang, and Sergey Urbanovich.
> 
> 
> Bugs: MESOS-9142
>     https://issues.apache.org/jira/browse/MESOS-9142
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is to ensure all or nothing semantics. We don't want to deal with a
> particially written file in case agent crashes.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp f46c962d8f044092aaa113fafc536c6b25bab45c 
> 
> 
> Diff: https://reviews.apache.org/r/68334/diff/3/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 68334: Used state::checkpoint instead in CNI isolator.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68334/#review207216
-----------------------------------------------------------


Ship it!




Ship It!

- Qian Zhang


On Aug. 14, 2018, 7:04 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68334/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2018, 7:04 a.m.)
> 
> 
> Review request for mesos, Deepak Goel, Qian Zhang, and Sergey Urbanovich.
> 
> 
> Bugs: MESOS-9142
>     https://issues.apache.org/jira/browse/MESOS-9142
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is to ensure all or nothing semantics. We don't want to deal with a
> particially written file in case agent crashes.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp f46c962d8f044092aaa113fafc536c6b25bab45c 
> 
> 
> Diff: https://reviews.apache.org/r/68334/diff/3/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 68334: Used state::checkpoint instead in CNI isolator.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68334/
-----------------------------------------------------------

(Updated Aug. 13, 2018, 11:04 p.m.)


Review request for mesos, Deepak Goel, Qian Zhang, and Sergey Urbanovich.


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


Repository: mesos


Description
-------

This is to ensure all or nothing semantics. We don't want to deal with a
particially written file in case agent crashes.


Diffs (updated)
-----

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp f46c962d8f044092aaa113fafc536c6b25bab45c 


Diff: https://reviews.apache.org/r/68334/diff/3/

Changes: https://reviews.apache.org/r/68334/diff/2-3/


Testing
-------

sudo make check


Thanks,

Jie Yu