Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-11-16 Thread Gilbert Song 

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


Ship it!




Ship It!

- Gilbert Song


On Nov. 15, 2017, 8:33 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55334/
> ---
> 
> (Updated Nov. 15, 2017, 8:33 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-6894
> https://issues.apache.org/jira/browse/MESOS-6894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch includes the following change:
> - Checkpointed `ContainerConfig` used to launch a container;
> - Added helper function to read checkpointed `ContainerConfig`;
> - Recovered `ContainerConfig`.
> - Make `ContainerConfig` Option in `Container` struct to indicate
>   whether recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> f5d5146de9abdd9804ede6a4b17a965a4b27c05a 
>   src/slave/containerizer/mesos/containerizer.cpp 
> db5f044f7415386c94e4930f365dfc14d403414a 
>   src/slave/containerizer/mesos/paths.hpp 
> 7b67ccf670d991cbc339d74ebf4fccf3ae9a98a8 
>   src/slave/containerizer/mesos/paths.cpp 
> 23f1fee3667aa48e8c31c15e0b69201268e44d37 
> 
> 
> Diff: https://reviews.apache.org/r/55334/diff/16/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-11-15 Thread Zhitao Li 

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

(Updated Nov. 15, 2017, 8:33 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Fixed the comment message < 72 char.


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


Repository: mesos


Description (updated)
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate
  whether recovered.


Diffs
-

  src/slave/containerizer/mesos/containerizer.hpp 
f5d5146de9abdd9804ede6a4b17a965a4b27c05a 
  src/slave/containerizer/mesos/containerizer.cpp 
db5f044f7415386c94e4930f365dfc14d403414a 
  src/slave/containerizer/mesos/paths.hpp 
7b67ccf670d991cbc339d74ebf4fccf3ae9a98a8 
  src/slave/containerizer/mesos/paths.cpp 
23f1fee3667aa48e8c31c15e0b69201268e44d37 


Diff: https://reviews.apache.org/r/55334/diff/16/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-11-15 Thread Zhitao Li 

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

(Updated Nov. 16, 2017, 3:58 a.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Rebase after standalone container support is added.


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


Repository: mesos


Description
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate whether 
recovered.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
f5d5146de9abdd9804ede6a4b17a965a4b27c05a 
  src/slave/containerizer/mesos/containerizer.cpp 
db5f044f7415386c94e4930f365dfc14d403414a 
  src/slave/containerizer/mesos/paths.hpp 
7b67ccf670d991cbc339d74ebf4fccf3ae9a98a8 
  src/slave/containerizer/mesos/paths.cpp 
23f1fee3667aa48e8c31c15e0b69201268e44d37 


Diff: https://reviews.apache.org/r/55334/diff/16/

Changes: https://reviews.apache.org/r/55334/diff/15-16/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-11-15 Thread Gilbert Song 

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


Ship it!




Could you rebase this patch? Cannot apply.

- Gilbert Song


On Nov. 10, 2017, 11:30 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55334/
> ---
> 
> (Updated Nov. 10, 2017, 11:30 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-6894
> https://issues.apache.org/jira/browse/MESOS-6894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch includes the following change:
> - Checkpointed `ContainerConfig` used to launch a container;
> - Added helper function to read checkpointed `ContainerConfig`;
> - Recovered `ContainerConfig`.
> - Make `ContainerConfig` Option in `Container` struct to indicate whether 
> recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 100e3bbda543d87808da9ff6bea42da5099ea8c5 
>   src/slave/containerizer/mesos/paths.hpp 
> a03f15e01e8eb8a1326baad8d7db96a79d785482 
>   src/slave/containerizer/mesos/paths.cpp 
> 0c61c20c345a327ec469b382558aaeed0280e754 
> 
> 
> Diff: https://reviews.apache.org/r/55334/diff/15/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-11-10 Thread Zhitao Li 

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

(Updated Nov. 10, 2017, 7:30 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


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


Repository: mesos


Description
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate whether 
recovered.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
  src/slave/containerizer/mesos/containerizer.cpp 
100e3bbda543d87808da9ff6bea42da5099ea8c5 
  src/slave/containerizer/mesos/paths.hpp 
a03f15e01e8eb8a1326baad8d7db96a79d785482 
  src/slave/containerizer/mesos/paths.cpp 
0c61c20c345a327ec469b382558aaeed0280e754 


Diff: https://reviews.apache.org/r/55334/diff/15/

Changes: https://reviews.apache.org/r/55334/diff/14-15/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-11-10 Thread Zhitao Li 

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

(Updated Nov. 10, 2017, 7:03 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


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


Repository: mesos


Description (updated)
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate whether 
recovered.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
  src/slave/containerizer/mesos/containerizer.cpp 
100e3bbda543d87808da9ff6bea42da5099ea8c5 
  src/slave/containerizer/mesos/paths.hpp 
a03f15e01e8eb8a1326baad8d7db96a79d785482 
  src/slave/containerizer/mesos/paths.cpp 
0c61c20c345a327ec469b382558aaeed0280e754 


Diff: https://reviews.apache.org/r/55334/diff/14/

Changes: https://reviews.apache.org/r/55334/diff/13-14/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-11-06 Thread Gilbert Song 

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


Fix it, then Ship it!





src/slave/containerizer/mesos/containerizer.cpp
Lines 777 (patched)


Let's use VLOG(1) instead since there might be a lot of containers.



src/slave/containerizer/mesos/containerizer.cpp
Line 807 (original), 839-842 (patched)


Seems like you forget to remove these.



src/slave/containerizer/mesos/containerizer.cpp
Lines 878 (patched)


ditto.



src/slave/containerizer/mesos/paths.hpp
Lines 48-63 (original), 48-63 (patched)


could you update this comment?


- Gilbert Song


On Oct. 27, 2017, 11 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55334/
> ---
> 
> (Updated Oct. 27, 2017, 11 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-6894
> https://issues.apache.org/jira/browse/MESOS-6894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch includes the following change:
> - Checkpointed `ContainerConfig` used to launch a container;
> - Added helper function to read checkpointed `ContainerConfig`;
> - Recovered `ContainerConfig`.
> - Make `ContainerConfig` Option in `Container` struct to indicate whether 
> recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 100e3bbda543d87808da9ff6bea42da5099ea8c5 
>   src/slave/containerizer/mesos/paths.hpp 
> a03f15e01e8eb8a1326baad8d7db96a79d785482 
>   src/slave/containerizer/mesos/paths.cpp 
> 0c61c20c345a327ec469b382558aaeed0280e754 
> 
> 
> Diff: https://reviews.apache.org/r/55334/diff/13/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-10-27 Thread Zhitao Li 

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

(Updated Oct. 27, 2017, 6 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


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


Repository: mesos


Description
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate whether 
recovered.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
  src/slave/containerizer/mesos/containerizer.cpp 
100e3bbda543d87808da9ff6bea42da5099ea8c5 
  src/slave/containerizer/mesos/paths.hpp 
a03f15e01e8eb8a1326baad8d7db96a79d785482 
  src/slave/containerizer/mesos/paths.cpp 
0c61c20c345a327ec469b382558aaeed0280e754 


Diff: https://reviews.apache.org/r/55334/diff/13/

Changes: https://reviews.apache.org/r/55334/diff/12-13/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-10-27 Thread Zhitao Li 

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

(Updated Oct. 27, 2017, 5:57 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Review comments.


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


Repository: mesos


Description
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate whether 
recovered.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
  src/slave/containerizer/mesos/containerizer.cpp 
100e3bbda543d87808da9ff6bea42da5099ea8c5 
  src/slave/containerizer/mesos/paths.hpp 
a03f15e01e8eb8a1326baad8d7db96a79d785482 
  src/slave/containerizer/mesos/paths.cpp 
0c61c20c345a327ec469b382558aaeed0280e754 


Diff: https://reviews.apache.org/r/55334/diff/12/

Changes: https://reviews.apache.org/r/55334/diff/11-12/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-10-26 Thread Gilbert Song 


> On Oct. 26, 2017, 12:33 p.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/containerizer.cpp
> > Lines 1113-1120 (original), 1152-1164 (patched)
> > 
> >
> > After looking at the slave code. it seems to me we can remove the whole 
> > part and the TODO.
> > 
> > Not yours. this was from a tech debt when we combined the launch() 
> > method 
> > https://github.com/apache/mesos/commit/17ffb97ae7eda78edf85b204eba35bc59649a479
> 
> Zhitao Li wrote:
> Are you suggesting that a nested container's config does not need 
> `executor_info` anymore?

no, we already set it in slave.cpp before calling `containerizer::launch()`.


- Gilbert


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


On Oct. 19, 2017, 9:41 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55334/
> ---
> 
> (Updated Oct. 19, 2017, 9:41 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-6894
> https://issues.apache.org/jira/browse/MESOS-6894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch includes the following change:
> - Checkpointed `ContainerConfig` used to launch a container;
> - Added helper function to read checkpointed `ContainerConfig`;
> - Recovered `ContainerConfig`.
> - Make `ContainerConfig` Option in `Container` struct to indicate whether 
> recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 78fdd21f8b7ede4beedff31ba2b488ffebd4ea31 
>   src/slave/containerizer/mesos/paths.hpp 
> a03f15e01e8eb8a1326baad8d7db96a79d785482 
>   src/slave/containerizer/mesos/paths.cpp 
> 0c61c20c345a327ec469b382558aaeed0280e754 
> 
> 
> Diff: https://reviews.apache.org/r/55334/diff/11/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-10-26 Thread Zhitao Li 


> On Oct. 26, 2017, 7:33 p.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/containerizer.cpp
> > Lines 1113-1120 (original), 1152-1164 (patched)
> > 
> >
> > After looking at the slave code. it seems to me we can remove the whole 
> > part and the TODO.
> > 
> > Not yours. this was from a tech debt when we combined the launch() 
> > method 
> > https://github.com/apache/mesos/commit/17ffb97ae7eda78edf85b204eba35bc59649a479

Are you suggesting that a nested container's config does not need 
`executor_info` anymore?


- Zhitao


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


On Oct. 20, 2017, 4:41 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55334/
> ---
> 
> (Updated Oct. 20, 2017, 4:41 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-6894
> https://issues.apache.org/jira/browse/MESOS-6894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch includes the following change:
> - Checkpointed `ContainerConfig` used to launch a container;
> - Added helper function to read checkpointed `ContainerConfig`;
> - Recovered `ContainerConfig`.
> - Make `ContainerConfig` Option in `Container` struct to indicate whether 
> recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 78fdd21f8b7ede4beedff31ba2b488ffebd4ea31 
>   src/slave/containerizer/mesos/paths.hpp 
> a03f15e01e8eb8a1326baad8d7db96a79d785482 
>   src/slave/containerizer/mesos/paths.cpp 
> 0c61c20c345a327ec469b382558aaeed0280e754 
> 
> 
> Diff: https://reviews.apache.org/r/55334/diff/11/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-10-26 Thread Gilbert Song 

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


Fix it, then Ship it!





src/slave/containerizer/mesos/containerizer.cpp
Lines 757-758 (patched)


we don't quote the containerID since it was generated by the agent or 
executor, not by users.



src/slave/containerizer/mesos/containerizer.cpp
Lines 764-765 (patched)


ditto.



src/slave/containerizer/mesos/containerizer.cpp
Lines 764-765 (patched)


how about:
```
  LOG(INFO) << "No container config is recovered for container " << 
containerId
<< ", image pruning will be disabled.";
```



src/slave/containerizer/mesos/containerizer.cpp
Lines 765 (patched)


we don't have a period at the end for `LOG(INFO)`.



src/slave/containerizer/mesos/containerizer.cpp
Line 793 (original), 811 (patched)


not yours. mind add the containerID here?



src/slave/containerizer/mesos/containerizer.cpp
Lines 819 (patched)


add containerID?



src/slave/containerizer/mesos/containerizer.cpp
Lines 823 (patched)


do we need to mention image pruning will be disabled?



src/slave/containerizer/mesos/containerizer.cpp
Lines 860 (patched)


s/config/container config/g



src/slave/containerizer/mesos/containerizer.cpp
Lines 861 (patched)


remove `this means`.



src/slave/containerizer/mesos/containerizer.cpp
Lines 862 (patched)


ditto.



src/slave/containerizer/mesos/containerizer.cpp
Lines 1113-1120 (original), 1152-1164 (patched)


After looking at the slave code. it seems to me we can remove the whole 
part and the TODO.

Not yours. this was from a tech debt when we combined the launch() method 
https://github.com/apache/mesos/commit/17ffb97ae7eda78edf85b204eba35bc59649a479



src/slave/containerizer/mesos/containerizer.cpp
Lines 1163 (patched)


ditto.



src/slave/containerizer/mesos/containerizer.cpp
Lines 1271 (patched)


could we move it below `CHECK_EQ(container->state, PROVISIONING);`?



src/slave/containerizer/mesos/containerizer.cpp
Lines 1321 (patched)


good catch/



src/slave/containerizer/mesos/paths.cpp
Lines 279-280 (original), 282-283 (patched)


mind add the containerID?



src/slave/containerizer/mesos/paths.cpp
Lines 310-311 (patched)


ditto.



src/slave/containerizer/mesos/paths.cpp
Lines 467-468 (patched)


extra newline?


- Gilbert Song


On Oct. 19, 2017, 9:41 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55334/
> ---
> 
> (Updated Oct. 19, 2017, 9:41 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-6894
> https://issues.apache.org/jira/browse/MESOS-6894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch includes the following change:
> - Checkpointed `ContainerConfig` used to launch a container;
> - Added helper function to read checkpointed `ContainerConfig`;
> - Recovered `ContainerConfig`.
> - Make `ContainerConfig` Option in `Container` struct to indicate whether 
> recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 78fdd21f8b7ede4beedff31ba2b488ffebd4ea31 
>   src/slave/containerizer/mesos/paths.hpp 
> a03f15e01e8eb8a1326baad8d7db96a79d785482 
>   src/slave/containerizer/mesos/paths.cpp 
> 0c61c20c345a327ec469b382558aaeed0280e754 
> 
> 
> Diff: https://reviews.apache.org/r/55334/diff/11/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-10-19 Thread Zhitao Li 

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

(Updated Oct. 20, 2017, 4:41 a.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


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


Repository: mesos


Description
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate whether 
recovered.


Diffs
-

  src/slave/containerizer/mesos/containerizer.hpp 
6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
  src/slave/containerizer/mesos/containerizer.cpp 
78fdd21f8b7ede4beedff31ba2b488ffebd4ea31 
  src/slave/containerizer/mesos/paths.hpp 
a03f15e01e8eb8a1326baad8d7db96a79d785482 
  src/slave/containerizer/mesos/paths.cpp 
0c61c20c345a327ec469b382558aaeed0280e754 


Diff: https://reviews.apache.org/r/55334/diff/11/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-10-09 Thread Zhitao Li 

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

(Updated Oct. 9, 2017, 11:13 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Move checkpointing after `provisioner::provision()` finishes.


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


Repository: mesos


Description
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate whether 
recovered.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
cc23b4d91be16fc95a131c09d07378b801e34d6f 
  src/slave/containerizer/mesos/containerizer.cpp 
4d5dc13f363f5d8886983d7dd06a5cecc177c345 
  src/slave/containerizer/mesos/paths.hpp 
a03f15e01e8eb8a1326baad8d7db96a79d785482 
  src/slave/containerizer/mesos/paths.cpp 
0c61c20c345a327ec469b382558aaeed0280e754 


Diff: https://reviews.apache.org/r/55334/diff/10/

Changes: https://reviews.apache.org/r/55334/diff/9-10/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-10-05 Thread Zhitao Li 

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




src/slave/containerizer/mesos/containerizer.cpp
Lines 1186-1202 (patched)


@gilbert, I realized one thing: should this be moved to or duplicated in 
`containerizer::prepare()`, after the `provisionInfo` is returned from 
`provisioner::provision()` finishes? In the chained callback there, 
`container->config` could be updated one more time if some rootfs is 
provisioned.


- Zhitao Li


On Sept. 26, 2017, 7:09 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55334/
> ---
> 
> (Updated Sept. 26, 2017, 7:09 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-6894
> https://issues.apache.org/jira/browse/MESOS-6894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch includes the following change:
> - Checkpointed `ContainerConfig` used to launch a container;
> - Added helper function to read checkpointed `ContainerConfig`;
> - Recovered `ContainerConfig`.
> - Make `ContainerConfig` Option in `Container` struct to indicate whether 
> recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> cc23b4d91be16fc95a131c09d07378b801e34d6f 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4d5dc13f363f5d8886983d7dd06a5cecc177c345 
>   src/slave/containerizer/mesos/paths.hpp 
> a03f15e01e8eb8a1326baad8d7db96a79d785482 
>   src/slave/containerizer/mesos/paths.cpp 
> 0c61c20c345a327ec469b382558aaeed0280e754 
> 
> 
> Diff: https://reviews.apache.org/r/55334/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-09-26 Thread Zhitao Li 

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

(Updated Sept. 26, 2017, 7:09 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Added `CHECK_SOME`


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


Repository: mesos


Description
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate whether 
recovered.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
cc23b4d91be16fc95a131c09d07378b801e34d6f 
  src/slave/containerizer/mesos/containerizer.cpp 
4d5dc13f363f5d8886983d7dd06a5cecc177c345 
  src/slave/containerizer/mesos/paths.hpp 
a03f15e01e8eb8a1326baad8d7db96a79d785482 
  src/slave/containerizer/mesos/paths.cpp 
0c61c20c345a327ec469b382558aaeed0280e754 


Diff: https://reviews.apache.org/r/55334/diff/9/

Changes: https://reviews.apache.org/r/55334/diff/8-9/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-09-26 Thread Zhitao Li 

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




src/slave/containerizer/mesos/containerizer.cpp
Line 1050 (original), 1088 (patched)


Added a check for `config.isSome()` and a `warning` log line.


- Zhitao Li


On Sept. 26, 2017, 6:09 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55334/
> ---
> 
> (Updated Sept. 26, 2017, 6:09 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-6894
> https://issues.apache.org/jira/browse/MESOS-6894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch includes the following change:
> - Checkpointed `ContainerConfig` used to launch a container;
> - Added helper function to read checkpointed `ContainerConfig`;
> - Recovered `ContainerConfig`.
> - Make `ContainerConfig` Option in `Container` struct to indicate whether 
> recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> cc23b4d91be16fc95a131c09d07378b801e34d6f 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4d5dc13f363f5d8886983d7dd06a5cecc177c345 
>   src/slave/containerizer/mesos/paths.hpp 
> a03f15e01e8eb8a1326baad8d7db96a79d785482 
>   src/slave/containerizer/mesos/paths.cpp 
> 0c61c20c345a327ec469b382558aaeed0280e754 
> 
> 
> Diff: https://reviews.apache.org/r/55334/diff/8/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-09-26 Thread Zhitao Li 

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

(Updated Sept. 26, 2017, 6:09 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Rebase with Gilbert's comments addressed.


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


Repository: mesos


Description
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate whether 
recovered.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
cc23b4d91be16fc95a131c09d07378b801e34d6f 
  src/slave/containerizer/mesos/containerizer.cpp 
4d5dc13f363f5d8886983d7dd06a5cecc177c345 
  src/slave/containerizer/mesos/paths.hpp 
a03f15e01e8eb8a1326baad8d7db96a79d785482 
  src/slave/containerizer/mesos/paths.cpp 
0c61c20c345a327ec469b382558aaeed0280e754 


Diff: https://reviews.apache.org/r/55334/diff/8/

Changes: https://reviews.apache.org/r/55334/diff/7-8/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-09-25 Thread Gilbert Song 

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



could you rebase this chain? sorry for the delay, but thank you, Zhitao!


src/slave/containerizer/mesos/containerizer.cpp
Lines 689 (patched)


log the containerId?



src/slave/containerizer/mesos/containerizer.cpp
Lines 695 (patched)


should we use `WARNING` here? since everytime the containerizer recovers, 
it might log a bunch of warning if there are many legacy containers running.



src/slave/containerizer/mesos/containerizer.cpp
Lines 696 (patched)


do we still plan to back fill?



src/slave/containerizer/mesos/containerizer.cpp
Lines 755 (patched)


ditto.



src/slave/containerizer/mesos/containerizer.cpp
Lines 792-794 (patched)


identify this log for both executor container and nested container?



src/slave/containerizer/mesos/containerizer.cpp
Line 1050 (original), 1088 (patched)


this may cause segfault, right?

we should consider address this TODO in a separate patch in favor of 
checkpointing the `ContainerConfig`.



src/slave/containerizer/mesos/containerizer.cpp
Lines 1114-1117 (patched)


How about:

Checkpoint the `ContainerConfig` which includes all information to launch a 
container. Critical information (e.g., `ContainerInfo`) can be used for 
tracking container image usage.



src/slave/containerizer/mesos/containerizer.cpp
Lines 1130 (patched)


quote the path?



src/slave/containerizer/mesos/containerizer.cpp
Line 1191 (original), 1247 (patched)


add a `CHECK_SOME()` in the very beginning of `prepare()` funtion?



src/slave/containerizer/mesos/containerizer.cpp
Line 1486 (original), 1545 (patched)


fix the indentation (extra space)?



src/slave/containerizer/mesos/paths.cpp
Lines 438 (patched)


Move below `getContainerTermination()`?



src/slave/containerizer/mesos/paths.cpp
Lines 449 (patched)


quote the `path`?

and add `containerId`?



src/slave/containerizer/mesos/paths.cpp
Lines 458 (patched)


permutate the space to the line above?

mind fix getContainerTermination() as well?


- Gilbert Song


On Aug. 1, 2017, 10:10 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55334/
> ---
> 
> (Updated Aug. 1, 2017, 10:10 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-6894
> https://issues.apache.org/jira/browse/MESOS-6894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch includes the following change:
> - Checkpointed `ContainerConfig` used to launch a container;
> - Added helper function to read checkpointed `ContainerConfig`;
> - Recovered `ContainerConfig`.
> - Make `ContainerConfig` Option in `Container` struct to indicate whether 
> recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> fd586306f7a6d3d2cdb58481f0ef4906de8d0f88 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 6f100b516f2750732acaed693f7023b1e44b39d9 
>   src/slave/containerizer/mesos/paths.hpp 
> 12ae7c7329f9e8d334cb32c30cc38465bddc046c 
>   src/slave/containerizer/mesos/paths.cpp 
> 0c61c20c345a327ec469b382558aaeed0280e754 
> 
> 
> Diff: https://reviews.apache.org/r/55334/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-08-01 Thread Zhitao Li 

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

(Updated Aug. 1, 2017, 5:10 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate whether 
recovered.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
fd586306f7a6d3d2cdb58481f0ef4906de8d0f88 
  src/slave/containerizer/mesos/containerizer.cpp 
6f100b516f2750732acaed693f7023b1e44b39d9 
  src/slave/containerizer/mesos/paths.hpp 
12ae7c7329f9e8d334cb32c30cc38465bddc046c 
  src/slave/containerizer/mesos/paths.cpp 
0c61c20c345a327ec469b382558aaeed0280e754 


Diff: https://reviews.apache.org/r/55334/diff/7/

Changes: https://reviews.apache.org/r/55334/diff/6-7/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-05-31 Thread Zhitao Li 

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

(Updated May 31, 2017, 5:57 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate whether 
recovered.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
ea0691945d3450fcc336df03d9cf7b48afbd8b3d 
  src/slave/containerizer/mesos/containerizer.cpp 
f3e6210eccd4a6b445ffd4447e69526d424ea36d 
  src/slave/containerizer/mesos/paths.hpp 
12ae7c7329f9e8d334cb32c30cc38465bddc046c 
  src/slave/containerizer/mesos/paths.cpp 
0c61c20c345a327ec469b382558aaeed0280e754 


Diff: https://reviews.apache.org/r/55334/diff/5/

Changes: https://reviews.apache.org/r/55334/diff/4-5/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-04-11 Thread Zhitao Li 

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

(Updated April 11, 2017, 8:28 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate whether 
recovered.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
29a99f33e646593127b9dc126f398f7bca88e21d 
  src/slave/containerizer/mesos/containerizer.cpp 
bc611a5e085de10e9953b5f942d98f2b5747fce6 
  src/slave/containerizer/mesos/paths.hpp 
d85fd34660faacd9c73de2ba7b87b3bbd4b6007b 
  src/slave/containerizer/mesos/paths.cpp 
ed4bbd2491e71ad1e4a41e0575b514377d02da9b 


Diff: https://reviews.apache.org/r/55334/diff/4/

Changes: https://reviews.apache.org/r/55334/diff/3-4/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-02-15 Thread Zhitao Li 

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

(Updated Feb. 15, 2017, 9:14 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


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


Repository: mesos


Description (updated)
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate whether 
recovered.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
10a9b57660388ac2409458a4d07af64cc3b185e2 
  src/slave/containerizer/mesos/containerizer.cpp 
d2b4f75a55dbe4746bc2dfc180335fa831a554ef 
  src/slave/containerizer/mesos/paths.hpp 
d85fd34660faacd9c73de2ba7b87b3bbd4b6007b 
  src/slave/containerizer/mesos/paths.cpp 
c1770cefe0287ce994eec6979db41201148ef3fb 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-01-09 Thread Zhitao Li 

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

(Updated Jan. 9, 2017, 5:59 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Fix for also recovering `ContainerConfig` for alive executor containers.


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


Repository: mesos


Description (updated)
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
9fcad933e1ec3b52d4dc366ba80d282deea04c0b 
  src/slave/containerizer/mesos/containerizer.cpp 
d9d5619e45ae1199fc91878f17a33b5647f48305 
  src/slave/containerizer/mesos/paths.hpp 
d85fd34660faacd9c73de2ba7b87b3bbd4b6007b 
  src/slave/containerizer/mesos/paths.cpp 
c1770cefe0287ce994eec6979db41201148ef3fb 

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


Testing
---


Thanks,

Zhitao Li

Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-01-09 Thread Zhitao Li 

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

Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


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


Repository: mesos


Description
---

This patch includes the following changes:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.


Diffs
-

  src/slave/containerizer/mesos/containerizer.hpp 
9fcad933e1ec3b52d4dc366ba80d282deea04c0b 
  src/slave/containerizer/mesos/containerizer.cpp 
d9d5619e45ae1199fc91878f17a33b5647f48305 
  src/slave/containerizer/mesos/paths.hpp 
d85fd34660faacd9c73de2ba7b87b3bbd4b6007b 
  src/slave/containerizer/mesos/paths.cpp 
c1770cefe0287ce994eec6979db41201148ef3fb 

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


Testing
---


Thanks,

Zhitao Li