Re: Review Request 39456: Documentation: added containerizer internals

[Jojy Varghese]

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

(Updated Dec. 16, 2015, 9:48 p.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Changes
---

addressed reviews.


Repository: mesos


Description
---

Documentation: added containerizer internals


Diffs (updated)
-

  docs/containerizer-internals.md PRE-CREATION 
  docs/home.md a01612ec8347eb8a7a9277b829365b6c3a1fe9e3 

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


Testing
---


Thanks,

Jojy Varghese

Re: Review Request 39456: Documentation: added containerizer internals

[Jie Yu]

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

Ship it!



docs/containerizer-internals.md (lines 54 - 55)


Do you need to remove the second half of this sentense?


- Jie Yu


On Dec. 16, 2015, 9:48 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39456/
> ---
> 
> (Updated Dec. 16, 2015, 9:48 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation: added containerizer internals
> 
> 
> Diffs
> -
> 
>   docs/containerizer-internals.md PRE-CREATION 
>   docs/home.md a01612ec8347eb8a7a9277b829365b6c3a1fe9e3 
> 
> Diff: https://reviews.apache.org/r/39456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39456: Documentation: added containerizer internals

[Jojy Varghese]

> On Dec. 15, 2015, 12:35 a.m., Guangya Liu wrote:
> > docs/containerizer-internals.md, lines 15-23
> > 
> >
> > What about moveing this after ### Type of containerizers
> 
> Jojy Varghese wrote:
> Since this section is common to all containerizers, I liked it here. WDYT?
> 
> Thanks Guangya.
> 
> Guangya Liu wrote:
> It is ok to put it here, my thinking is that it would be better if we put 
> this after giving end user a concept introduciton for all of the 
> containerizers. I do not have strong opinion on this and this depends on you 
> ;-)

dropping this as per our comments.


- Jojy


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


On Dec. 15, 2015, 6:22 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39456/
> ---
> 
> (Updated Dec. 15, 2015, 6:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation: added containerizer internals
> 
> 
> Diffs
> -
> 
>   docs/containerizer-internals.md PRE-CREATION 
>   docs/home.md a01612ec8347eb8a7a9277b829365b6c3a1fe9e3 
> 
> Diff: https://reviews.apache.org/r/39456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39456: Documentation: added containerizer internals

[Jojy Varghese]

> On Dec. 15, 2015, 12:35 a.m., Guangya Liu wrote:
> > docs/containerizer-internals.md, lines 6-10
> > 
> >
> > Does this still needed? As this was already mentioned in 
> > https://github.com/apache/mesos/blob/master/docs/containerizer.md#mesos-containerizer

This was added as per review comments from other reviewers. The idea was to add 
a little note about what containerizers are.


- Jojy


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


On Dec. 15, 2015, 6:22 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39456/
> ---
> 
> (Updated Dec. 15, 2015, 6:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation: added containerizer internals
> 
> 
> Diffs
> -
> 
>   docs/containerizer-internals.md PRE-CREATION 
>   docs/home.md a01612ec8347eb8a7a9277b829365b6c3a1fe9e3 
> 
> Diff: https://reviews.apache.org/r/39456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39456: Documentation: added containerizer internals

[Jojy Varghese]

> On Dec. 15, 2015, 12:35 a.m., Guangya Liu wrote:
> > docs/containerizer-internals.md, lines 6-10
> > 
> >
> > Does this still needed? As this was already mentioned in 
> > https://github.com/apache/mesos/blob/master/docs/containerizer.md#mesos-containerizer
> 
> Jojy Varghese wrote:
> This was added as per review comments from other reviewers. The idea was 
> to add a little note about what containerizers are.

dropping this as per the comments. Thanks again.


- Jojy


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


On Dec. 15, 2015, 6:22 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39456/
> ---
> 
> (Updated Dec. 15, 2015, 6:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation: added containerizer internals
> 
> 
> Diffs
> -
> 
>   docs/containerizer-internals.md PRE-CREATION 
>   docs/home.md a01612ec8347eb8a7a9277b829365b6c3a1fe9e3 
> 
> Diff: https://reviews.apache.org/r/39456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39456: Documentation: added containerizer internals

[Jie Yu]

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



docs/containerizer-internals.md (lines 39 - 43)


This is not true. Composing containerizer will compose the specified 
containerizers together and act like a single containerizer.



docs/containerizer-internals.md (lines 55 - 61)


I would mention two cases here:

1) User specifies a TaskInfo without an executor
2) User specifies a TaskInfo with an executor

The flow of the above 2) is different. Please read the code to understand 
the difference.



docs/containerizer-internals.md (line 72)


For the executor using Launcher (link to the following section).



docs/containerizer-internals.md (line 82)


I would just add a short summary about Launcher:

```
Launcher is responsible for forking/destroying containers.
```



docs/containerizer-internals.md (lines 83 - 87)


I would remove this.



docs/containerizer-internals.md (lines 86 - 87)


User is not allowed to set the 'setup' function.



docs/containerizer-internals.md (line 94)


using 'clone' system call


- Jie Yu


On Dec. 15, 2015, 6:22 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39456/
> ---
> 
> (Updated Dec. 15, 2015, 6:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation: added containerizer internals
> 
> 
> Diffs
> -
> 
>   docs/containerizer-internals.md PRE-CREATION 
>   docs/home.md a01612ec8347eb8a7a9277b829365b6c3a1fe9e3 
> 
> Diff: https://reviews.apache.org/r/39456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39456: Documentation: added containerizer internals

[Guangya Liu]

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



docs/containerizer-internals.md (lines 6 - 10)


Does this still needed? As this was already mentioned in 
https://github.com/apache/mesos/blob/master/docs/containerizer.md#mesos-containerizer



docs/containerizer-internals.md (line 10)


s/eg/eg,



docs/containerizer-internals.md (lines 15 - 23)


What about moveing this after ### Type of containerizers



docs/containerizer-internals.md (line 18)


s/eg./eg,



docs/containerizer-internals.md (lines 21 - 23)


Just a kind reminder, https://issues.apache.org/jira/browse/MESOS-1718 is 
planning to move get exeucutor info from slave to master, after this is fixed, 
it will be master who will generate an executor.


- Guangya Liu


On Dec. 14, 2015, 8:37 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39456/
> ---
> 
> (Updated Dec. 14, 2015, 8:37 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation: added containerizer internals
> 
> 
> Diffs
> -
> 
>   docs/containerizer-internals.md PRE-CREATION 
>   docs/home.md a01612ec8347eb8a7a9277b829365b6c3a1fe9e3 
> 
> Diff: https://reviews.apache.org/r/39456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39456: Documentation: added containerizer internals

[Jojy Varghese]

> On Dec. 15, 2015, 12:35 a.m., Guangya Liu wrote:
> > docs/containerizer-internals.md, lines 15-23
> > 
> >
> > What about moveing this after ### Type of containerizers

Since this section is common to all containerizers, I liked it here. WDYT?

Thanks Guangya.


> On Dec. 15, 2015, 12:35 a.m., Guangya Liu wrote:
> > docs/containerizer-internals.md, lines 21-23
> > 
> >
> > Just a kind reminder, https://issues.apache.org/jira/browse/MESOS-1718 
> > is planning to move get exeucutor info from slave to master, after this is 
> > fixed, it will be master who will generate an executor.

Thanks again for the reminder. Do you think we should have this in the 
documentation now or update the documentation when the change goes in?


- Jojy


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


On Dec. 14, 2015, 8:37 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39456/
> ---
> 
> (Updated Dec. 14, 2015, 8:37 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation: added containerizer internals
> 
> 
> Diffs
> -
> 
>   docs/containerizer-internals.md PRE-CREATION 
>   docs/home.md a01612ec8347eb8a7a9277b829365b6c3a1fe9e3 
> 
> Diff: https://reviews.apache.org/r/39456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39456: Documentation: added containerizer internals

[Guangya Liu]

> On Dec. 15, 2015, 12:35 a.m., Guangya Liu wrote:
> > docs/containerizer-internals.md, lines 21-23
> > 
> >
> > Just a kind reminder, https://issues.apache.org/jira/browse/MESOS-1718 
> > is planning to move get exeucutor info from slave to master, after this is 
> > fixed, it will be master who will generate an executor.
> 
> Jojy Varghese wrote:
> Thanks again for the reminder. Do you think we should have this in the 
> documentation now or update the documentation when the change goes in?

I think that adding a TODO here by refering to MESOS-1718 is good enough.


> On Dec. 15, 2015, 12:35 a.m., Guangya Liu wrote:
> > docs/containerizer-internals.md, lines 15-23
> > 
> >
> > What about moveing this after ### Type of containerizers
> 
> Jojy Varghese wrote:
> Since this section is common to all containerizers, I liked it here. WDYT?
> 
> Thanks Guangya.

It is ok to put it here, my thinking is that it would be better if we put this 
after giving end user a concept introduciton for all of the containerizers. I 
do not have strong opinion on this and this depends on you ;-)


- Guangya


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


On Dec. 14, 2015, 8:37 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39456/
> ---
> 
> (Updated Dec. 14, 2015, 8:37 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation: added containerizer internals
> 
> 
> Diffs
> -
> 
>   docs/containerizer-internals.md PRE-CREATION 
>   docs/home.md a01612ec8347eb8a7a9277b829365b6c3a1fe9e3 
> 
> Diff: https://reviews.apache.org/r/39456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39456: Documentation: added containerizer internals

[Jojy Varghese]

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

(Updated Dec. 14, 2015, 8:37 p.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Changes
---

post review changes.


Repository: mesos


Description
---

Documentation: added containerizer internals


Diffs (updated)
-

  docs/containerizer-internals.md PRE-CREATION 
  docs/home.md a01612ec8347eb8a7a9277b829365b6c3a1fe9e3 

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


Testing
---


Thanks,

Jojy Varghese

Re: Review Request 39456: Documentation: added containerizer internals

[Jojy Varghese]

> On Nov. 21, 2015, 2:19 a.m., Timothy Chen wrote:
> > Are you still working on this? This doesn't look complete to me?

Thanks for taking a look Tim.Since the area is too big to document all in one 
go, do you have any particular topics you would like to be in the first 
iteration?


- Jojy


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


On Oct. 21, 2015, 9:25 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39456/
> ---
> 
> (Updated Oct. 21, 2015, 9:25 p.m.)
> 
> 
> Review request for mesos, Connor Doyle and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation: added containerizer internals
> 
> 
> Diffs
> -
> 
>   docs/containerizer-internals.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39456: Documentation: added containerizer internals

[Timothy Chen]

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


Are you still working on this? This doesn't look complete to me?

- Timothy Chen


On Oct. 21, 2015, 9:25 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39456/
> ---
> 
> (Updated Oct. 21, 2015, 9:25 p.m.)
> 
> 
> Review request for mesos, Connor Doyle and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation: added containerizer internals
> 
> 
> Diffs
> -
> 
>   docs/containerizer-internals.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39456: Documentation: added containerizer internals

[Jojy Varghese]

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

(Updated Oct. 21, 2015, 9:25 p.m.)


Review request for mesos, Connor Doyle and Timothy Chen.


Changes
---

made more updates


Repository: mesos


Description
---

Documentation: added containerizer internals


Diffs (updated)
-

  docs/containerizer-internals.md PRE-CREATION 

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


Testing
---


Thanks,

Jojy Varghese

Re: Review Request 39456: Documentation: added containerizer internals

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39456]

All tests passed.

- Mesos ReviewBot


On Oct. 21, 2015, 9:25 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39456/
> ---
> 
> (Updated Oct. 21, 2015, 9:25 p.m.)
> 
> 
> Review request for mesos, Connor Doyle and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation: added containerizer internals
> 
> 
> Diffs
> -
> 
>   docs/containerizer-internals.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Review Request 39456: Documentation: added containerizer internals

[Jojy Varghese]

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

Review request for mesos and Timothy Chen.


Repository: mesos


Description
---

Documentation: added containerizer internals


Diffs
-

  docs/containerizer-internals.md PRE-CREATION 

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


Testing
---


Thanks,

Jojy Varghese

Re: Review Request 39456: Documentation: added containerizer internals

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39456]

All tests passed.

- Mesos ReviewBot


On Oct. 19, 2015, 10:18 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39456/
> ---
> 
> (Updated Oct. 19, 2015, 10:18 p.m.)
> 
> 
> Review request for mesos, Connor Doyle and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation: added containerizer internals
> 
> 
> Diffs
> -
> 
>   docs/containerizer-internals.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>