You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Paul Brett <pa...@twopensource.com> on 2015/07/07 23:54:05 UTC

Review Request 36281: Document per-container unique egress flows and network queueing statistics.

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

Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.


Repository: mesos


Description
-------

Document per-container unique egress flows and network queueing statistics.


Diffs
-----

  docs/home.md bc277910907c381c08835b6e9d485b27d6da5002 
  docs/network-isolation.md PRE-CREATION 
  docs/network-monitoring.md 8889fb165cc70bc382be0c99de8d7748328abf57 

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


Testing
-------

Rendered at https://www.notehub.org/2015/7/7/network-isolation for review.


Thanks,

Paul Brett


Re: Review Request 36281: Document per-container unique egress flows and network queueing statistics.

Posted by Paul Brett <pa...@twopensource.com>.

> On July 7, 2015, 11:31 p.m., Jie Yu wrote:
> > docs/network-isolation.md, line 7
> > <https://reviews.apache.org/r/36281/diff/1/?file=1001779#file1001779line7>
> >
> >     Please do not delete the version information. Network monitoring is added in mesos 0.20 and network isolation is added in mesos 0.23.
> >     
> >     I suggest the following layout for the summary paragraph:
> >     
> >     ```
> >     # Network Monitoring and Isolation.
> >     
> >     Mesos 0.20.0 adds the support for per container network monitoring. ...
> >     
> >     Mesos 0.23.0 adds the support for per container network isolation. ...
> >     
> >     Our solution is transparent to the tasks running on the slave ...
> >     ```

We should address the changes in the changelog and publish historical documentation on the website for those running older releaes (raised MESOS-3011 to addresss).


- Paul


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


On July 8, 2015, 12:03 a.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36281/
> -----------------------------------------------------------
> 
> (Updated July 8, 2015, 12:03 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Document per-container unique egress flows and network queueing statistics.
> 
> 
> Diffs
> -----
> 
>   docs/home.md bc277910907c381c08835b6e9d485b27d6da5002 
>   docs/network-isolation.md PRE-CREATION 
>   docs/network-monitoring.md 8889fb165cc70bc382be0c99de8d7748328abf57 
> 
> Diff: https://reviews.apache.org/r/36281/diff/
> 
> 
> Testing
> -------
> 
> Rendered at https://www.notehub.org/2015/7/7/network-isolation for review.
> 
> 
> Thanks,
> 
> Paul Brett
> 
>


Re: Review Request 36281: Document per-container unique egress flows and network queueing statistics.

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

Ship it!


Thanks Paul! This is great!

It's a little hard to review this change because you change the file name (I won't be able to see the diff). It would be better if you change the file name in a separate patch. Next time!


docs/home.md (line 24)
<https://reviews.apache.org/r/36281/#comment143953>

    I would suggest calling it 'Network Monitoring and Isolation'.
    
    Do not forget to change the file name as well.



docs/network-isolation.md (line 5)
<https://reviews.apache.org/r/36281/#comment143954>

    Ditto. Calling Network Monitoring and Isolation is more appropriate.
    
    Please to a sweep to fix all occurances in this doc.



docs/network-isolation.md (line 7)
<https://reviews.apache.org/r/36281/#comment143956>

    Please do not delete the version information. Network monitoring is added in mesos 0.20 and network isolation is added in mesos 0.23.
    
    I suggest the following layout for the summary paragraph:
    
    ```
    # Network Monitoring and Isolation.
    
    Mesos 0.20.0 adds the support for per container network monitoring. ...
    
    Mesos 0.23.0 adds the support for per container network isolation. ...
    
    Our solution is transparent to the tasks running on the slave ...
    ```



docs/network-isolation.md (line 98)
<https://reviews.apache.org/r/36281/#comment143973>

    Container Network Statistics?


- Jie Yu


On July 7, 2015, 9:54 p.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36281/
> -----------------------------------------------------------
> 
> (Updated July 7, 2015, 9:54 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Document per-container unique egress flows and network queueing statistics.
> 
> 
> Diffs
> -----
> 
>   docs/home.md bc277910907c381c08835b6e9d485b27d6da5002 
>   docs/network-isolation.md PRE-CREATION 
>   docs/network-monitoring.md 8889fb165cc70bc382be0c99de8d7748328abf57 
> 
> Diff: https://reviews.apache.org/r/36281/diff/
> 
> 
> Testing
> -------
> 
> Rendered at https://www.notehub.org/2015/7/7/network-isolation for review.
> 
> 
> Thanks,
> 
> Paul Brett
> 
>


Re: Review Request 36281: Document per-container unique egress flows and network queueing statistics.

Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36281/
-----------------------------------------------------------

(Updated July 8, 2015, 5:23 p.m.)


Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.


Changes
-------

Incorporate Adam's comments.


Repository: mesos


Description
-------

Document per-container unique egress flows and network queueing statistics.


Diffs (updated)
-----

  docs/home.md bc277910907c381c08835b6e9d485b27d6da5002 
  docs/network-monitoring.md 8889fb165cc70bc382be0c99de8d7748328abf57 

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


Testing
-------

Rendered at https://www.notehub.org/2015/7/7/network-isolation for review.


Thanks,

Paul Brett


Re: Review Request 36281: Document per-container unique egress flows and network queueing statistics.

Posted by Paul Brett <pa...@twopensource.com>.

> On July 8, 2015, 3:18 a.m., Adam B wrote:
> > docs/network-monitoring.md, line 17
> > <https://reviews.apache.org/r/36281/diff/3/?file=1001910#file1001910line17>
> >
> >     > "Mesos will automatically check for those kernel functionalities and will abort if they are not supported"
> >     Is this still true?

Yes


- Paul


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


On July 8, 2015, 12:59 a.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36281/
> -----------------------------------------------------------
> 
> (Updated July 8, 2015, 12:59 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Document per-container unique egress flows and network queueing statistics.
> 
> 
> Diffs
> -----
> 
>   docs/home.md bc277910907c381c08835b6e9d485b27d6da5002 
>   docs/network-monitoring.md 8889fb165cc70bc382be0c99de8d7748328abf57 
> 
> Diff: https://reviews.apache.org/r/36281/diff/
> 
> 
> Testing
> -------
> 
> Rendered at https://www.notehub.org/2015/7/7/network-isolation for review.
> 
> 
> Thanks,
> 
> Paul Brett
> 
>


Re: Review Request 36281: Document per-container unique egress flows and network queueing statistics.

Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36281/#review90857
-----------------------------------------------------------

Ship it!


A few other minor nits, but I think we can commit this for 0.23.0-rc2.
I'd even be willing to make some of the changes myself before committing.


docs/network-monitoring.md (line 7)
<https://reviews.apache.org/r/36281/#comment144013>

    s/running under/on/?
    s/which bind/those that bind/



docs/network-monitoring.md (line 15)
<https://reviews.apache.org/r/36281/#comment144014>

    > "Mesos will automatically check for those kernel functionalities and will abort if they are not supported"
    Is this still true?



docs/network-monitoring.md (line 67)
<https://reviews.apache.org/r/36281/#comment144016>

    > "If it is necessary to reduce this range to free ports to be allocated by the slave, it can be configured by defining the new range in `etc/sysctl.conf` and rebooting to eliminate"
    
    In "free ports", I couldn't tell if "free" was supposed to be a verb or adjective. 
    How about "If ports need to be set aside for slave containers, the host ephemeral port range can be updated in `etc/sysctl.conf`. Rebooting after the update will eliminate"



docs/network-monitoring.md (lines 69 - 71)
<https://reviews.apache.org/r/36281/#comment144015>

    This comment confused me. What you really mean is that the previous range was 32768-61000, but then you took away the first chunk for Mesos to use for its containers.
    ```
    # Set aside 32768-57344 for Mesos containers.
    # net.ipv4.ip_local_port_range = 32768 61000
    net.ipv4.ip_local_port_range = 57345 61000
    ```



docs/network-monitoring.md (line 77)
<https://reviews.apache.org/r/36281/#comment144017>

    Maybe insert a <br> before "`number of ephemeral_ports / ephemeral_ports_per_container`" so that it goes on a single line.



docs/network-monitoring.md (line 79)
<https://reviews.apache.org/r/36281/#comment144020>

    What is the default value of ephemeral_ports_per_container? (1024)



docs/network-monitoring.md (line 92)
<https://reviews.apache.org/r/36281/#comment144018>

    Remove indentation if you want this to render as a header.



docs/network-monitoring.md (line 106)
<https://reviews.apache.org/r/36281/#comment144021>

    We should only bother to include this flag if we set a non-default value.



docs/network-monitoring.md (line 107)
<https://reviews.apache.org/r/36281/#comment144019>

    Can't you just say `300MB` like in your description? Or does this flag only handle KB?


- Adam B


On July 7, 2015, 5:59 p.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36281/
> -----------------------------------------------------------
> 
> (Updated July 7, 2015, 5:59 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Document per-container unique egress flows and network queueing statistics.
> 
> 
> Diffs
> -----
> 
>   docs/home.md bc277910907c381c08835b6e9d485b27d6da5002 
>   docs/network-monitoring.md 8889fb165cc70bc382be0c99de8d7748328abf57 
> 
> Diff: https://reviews.apache.org/r/36281/diff/
> 
> 
> Testing
> -------
> 
> Rendered at https://www.notehub.org/2015/7/7/network-isolation for review.
> 
> 
> Thanks,
> 
> Paul Brett
> 
>


Re: Review Request 36281: Document per-container unique egress flows and network queueing statistics.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36281/#review90858
-----------------------------------------------------------


Patch looks great!

Reviews applied: [36281]

All tests passed.

- Mesos ReviewBot


On July 8, 2015, 12:59 a.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36281/
> -----------------------------------------------------------
> 
> (Updated July 8, 2015, 12:59 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Document per-container unique egress flows and network queueing statistics.
> 
> 
> Diffs
> -----
> 
>   docs/home.md bc277910907c381c08835b6e9d485b27d6da5002 
>   docs/network-monitoring.md 8889fb165cc70bc382be0c99de8d7748328abf57 
> 
> Diff: https://reviews.apache.org/r/36281/diff/
> 
> 
> Testing
> -------
> 
> Rendered at https://www.notehub.org/2015/7/7/network-isolation for review.
> 
> 
> Thanks,
> 
> Paul Brett
> 
>


Re: Review Request 36281: Document per-container unique egress flows and network queueing statistics.

Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36281/
-----------------------------------------------------------

(Updated July 8, 2015, 12:59 a.m.)


Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.


Changes
-------

Incorporate remaining reviewer comments.


Repository: mesos


Description
-------

Document per-container unique egress flows and network queueing statistics.


Diffs (updated)
-----

  docs/home.md bc277910907c381c08835b6e9d485b27d6da5002 
  docs/network-monitoring.md 8889fb165cc70bc382be0c99de8d7748328abf57 

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


Testing
-------

Rendered at https://www.notehub.org/2015/7/7/network-isolation for review.


Thanks,

Paul Brett


Re: Review Request 36281: Document per-container unique egress flows and network queueing statistics.

Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36281/
-----------------------------------------------------------

(Updated July 8, 2015, 12:03 a.m.)


Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.


Changes
-------

Incorporate comments from Ian Downes


Repository: mesos


Description
-------

Document per-container unique egress flows and network queueing statistics.


Diffs (updated)
-----

  docs/home.md bc277910907c381c08835b6e9d485b27d6da5002 
  docs/network-isolation.md PRE-CREATION 
  docs/network-monitoring.md 8889fb165cc70bc382be0c99de8d7748328abf57 

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


Testing
-------

Rendered at https://www.notehub.org/2015/7/7/network-isolation for review.


Thanks,

Paul Brett


Re: Review Request 36281: Document per-container unique egress flows and network queueing statistics.

Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36281/#review90819
-----------------------------------------------------------



docs/network-isolation.md (line 29)
<https://reviews.apache.org/r/36281/#comment143960>

    libnl3-devel is the package on Red Hat distribution, not for other disto.



docs/network-isolation.md (line 48)
<https://reviews.apache.org/r/36281/#comment143963>

    Please explicitly mention that currently binding to an unassigned port is allowed by kernel too (we need a patch to disallow this), just that packets will be dropped silently.



docs/network-isolation.md (line 81)
<https://reviews.apache.org/r/36281/#comment143964>

    s/Separating container traffic/Egress traffic isolation/



docs/network-isolation.md (line 83)
<https://reviews.apache.org/r/36281/#comment143969>

    That is called flow classification and isolation. Please also mention that flow is classified based on port ranges.


- Cong Wang


On July 7, 2015, 9:54 p.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36281/
> -----------------------------------------------------------
> 
> (Updated July 7, 2015, 9:54 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Document per-container unique egress flows and network queueing statistics.
> 
> 
> Diffs
> -----
> 
>   docs/home.md bc277910907c381c08835b6e9d485b27d6da5002 
>   docs/network-isolation.md PRE-CREATION 
>   docs/network-monitoring.md 8889fb165cc70bc382be0c99de8d7748328abf57 
> 
> Diff: https://reviews.apache.org/r/36281/diff/
> 
> 
> Testing
> -------
> 
> Rendered at https://www.notehub.org/2015/7/7/network-isolation for review.
> 
> 
> Thanks,
> 
> Paul Brett
> 
>


Re: Review Request 36281: Document per-container unique egress flows and network queueing statistics.

Posted by Paul Brett <pa...@twopensource.com>.

> On July 8, 2015, 12:01 a.m., Adam B wrote:
> > docs/network-isolation.md, line 79
> > <https://reviews.apache.org/r/36281/diff/1/?file=1001779#file1001779line79>
> >
> >     Why is this a fixed constant limit? Seems like we might want to adjust this depending on how many containers are running, or give some containers more bandwidth than others. Why isn't this just another resource type (outbound network bandwidth) with a fixed total, where subsets can be reserved/claimed by different frameworks/tasks.

Outbound network bandwidth sounds like a reasonable future enhancement - do you want to raise a ticket?


> On July 8, 2015, 12:01 a.m., Adam B wrote:
> > docs/network-isolation.md, lines 70-73
> > <https://reviews.apache.org/r/36281/diff/1/?file=1001779#file1001779line70>
> >
> >     Is 1024 a reasonable example ephemeral_ports_per_container? Your current example only allows 24 containers on that slave, which is extremely low. Maybe 16 ports per container would make more sense, yielding 1536 possible containers.

Its a reasonable number for the kinds of services I see running on mesos, after all we don't churn through containers very quickly.


> On July 8, 2015, 12:01 a.m., Adam B wrote:
> > docs/network-isolation.md, lines 24-29
> > <https://reviews.apache.org/r/36281/diff/1/?file=1001779#file1001779line24>
> >
> >     Do these dependencies need to be added to getting-started.md?

I don't believe so since this is not a default configuration.


- Paul


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


On July 8, 2015, 12:03 a.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36281/
> -----------------------------------------------------------
> 
> (Updated July 8, 2015, 12:03 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Document per-container unique egress flows and network queueing statistics.
> 
> 
> Diffs
> -----
> 
>   docs/home.md bc277910907c381c08835b6e9d485b27d6da5002 
>   docs/network-isolation.md PRE-CREATION 
>   docs/network-monitoring.md 8889fb165cc70bc382be0c99de8d7748328abf57 
> 
> Diff: https://reviews.apache.org/r/36281/diff/
> 
> 
> Testing
> -------
> 
> Rendered at https://www.notehub.org/2015/7/7/network-isolation for review.
> 
> 
> Thanks,
> 
> Paul Brett
> 
>


Re: Review Request 36281: Document per-container unique egress flows and network queueing statistics.

Posted by Adam B <ad...@mesosphere.io>.

> On July 7, 2015, 5:01 p.m., Adam B wrote:
> > docs/network-isolation.md, line 79
> > <https://reviews.apache.org/r/36281/diff/1/?file=1001779#file1001779line79>
> >
> >     Why is this a fixed constant limit? Seems like we might want to adjust this depending on how many containers are running, or give some containers more bandwidth than others. Why isn't this just another resource type (outbound network bandwidth) with a fixed total, where subsets can be reserved/claimed by different frameworks/tasks.
> 
> Paul Brett wrote:
>     Outbound network bandwidth sounds like a reasonable future enhancement - do you want to raise a ticket?

Sure. Created https://issues.apache.org/jira/browse/MESOS-3014


- Adam


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


On July 7, 2015, 5:59 p.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36281/
> -----------------------------------------------------------
> 
> (Updated July 7, 2015, 5:59 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Document per-container unique egress flows and network queueing statistics.
> 
> 
> Diffs
> -----
> 
>   docs/home.md bc277910907c381c08835b6e9d485b27d6da5002 
>   docs/network-monitoring.md 8889fb165cc70bc382be0c99de8d7748328abf57 
> 
> Diff: https://reviews.apache.org/r/36281/diff/
> 
> 
> Testing
> -------
> 
> Rendered at https://www.notehub.org/2015/7/7/network-isolation for review.
> 
> 
> Thanks,
> 
> Paul Brett
> 
>


Re: Review Request 36281: Document per-container unique egress flows and network queueing statistics.

Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36281/#review90827
-----------------------------------------------------------


Thanks for writing this up! I had a few minor suggestions and some questions.


docs/network-isolation.md (line 11)
<https://reviews.apache.org/r/36281/#comment143970>

    "follow the following" is a little redundant.



docs/network-isolation.md (line 15)
<https://reviews.apache.org/r/36281/#comment143971>

    s/kernels versions/kernel versions/



docs/network-isolation.md (lines 24 - 29)
<https://reviews.apache.org/r/36281/#comment143974>

    Do these dependencies need to be added to getting-started.md?



docs/network-isolation.md (line 31)
<https://reviews.apache.org/r/36281/#comment143975>

    `### Build Configuration`?



docs/network-isolation.md (lines 40 - 42)
<https://reviews.apache.org/r/36281/#comment143976>

    I would put the parameter example between these two sentences. Where it is now, I expect to see the "appropriate error" just described.



docs/network-isolation.md (line 48)
<https://reviews.apache.org/r/36281/#comment143978>

    s/so that service discovery/so that the service discovery/



docs/network-isolation.md (line 68)
<https://reviews.apache.org/r/36281/#comment143981>

    s/ephemerlal/ephemeral/



docs/network-isolation.md (lines 70 - 73)
<https://reviews.apache.org/r/36281/#comment143983>

    Is 1024 a reasonable example ephemeral_ports_per_container? Your current example only allows 24 containers on that slave, which is extremely low. Maybe 16 ports per container would make more sense, yielding 1536 possible containers.



docs/network-isolation.md (line 72)
<https://reviews.apache.org/r/36281/#comment143987>

    `s/\/\/`?



docs/network-isolation.md (line 79)
<https://reviews.apache.org/r/36281/#comment143986>

    Why is this a fixed constant limit? Seems like we might want to adjust this depending on how many containers are running, or give some containers more bandwidth than others. Why isn't this just another resource type (outbound network bandwidth) with a fixed total, where subsets can be reserved/claimed by different frameworks/tasks.



docs/network-isolation.md (line 85)
<https://reviews.apache.org/r/36281/#comment143989>

    Is the `="true"` necessary, or would `--egress_unique_flow_per_container` work on its own?



docs/network-isolation.md (line 93)
<https://reviews.apache.org/r/36281/#comment143988>

    Maybe leave out cpu/mem/disk to let the slave auto-detect them (and shorten this line)?



docs/network-isolation.md (line 178)
<https://reviews.apache.org/r/36281/#comment143991>

    lowercase?



docs/network-isolation.md (line 198)
<https://reviews.apache.org/r/36281/#comment143992>

    lowercase?



docs/network-isolation.md (line 204)
<https://reviews.apache.org/r/36281/#comment143990>

    This footnote `[1]` is never referenced above


- Adam B


On July 7, 2015, 2:54 p.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36281/
> -----------------------------------------------------------
> 
> (Updated July 7, 2015, 2:54 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Document per-container unique egress flows and network queueing statistics.
> 
> 
> Diffs
> -----
> 
>   docs/home.md bc277910907c381c08835b6e9d485b27d6da5002 
>   docs/network-isolation.md PRE-CREATION 
>   docs/network-monitoring.md 8889fb165cc70bc382be0c99de8d7748328abf57 
> 
> Diff: https://reviews.apache.org/r/36281/diff/
> 
> 
> Testing
> -------
> 
> Rendered at https://www.notehub.org/2015/7/7/network-isolation for review.
> 
> 
> Thanks,
> 
> Paul Brett
> 
>


Re: Review Request 36281: Document per-container unique egress flows and network queueing statistics.

Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36281/#review90798
-----------------------------------------------------------



docs/network-isolation.md (line 7)
<https://reviews.apache.org/r/36281/#comment143942>

    "which require to listen" is incorrect
    
    "a minimum choose a port in the Linux host ephemeral port range" hmmmm, what? I presume you really mean they should bind(0) and use what the kernel tells them to...
    
    saying "specific ports" here will be interpreted as 'I want my container to listen to port 80', not that I want to bind some specific port that I can then discover.



docs/network-isolation.md (line 25)
<https://reviews.apache.org/r/36281/#comment143925>

    This sentence is not clear, suggest rewording as "> 2.6.39 is advised for debugging purposes but is not required"



docs/network-isolation.md (line 27)
<https://reviews.apache.org/r/36281/#comment143926>

    s/development package for libnl3/libnl3 development package/



docs/network-isolation.md (line 48)
<https://reviews.apache.org/r/36281/#comment143927>

    what does "but only assigned ports will be allocated by the kernel mean"? this is not clear. Please also state here that the ephemeral range is split and assigned.



docs/network-isolation.md (line 52)
<https://reviews.apache.org/r/36281/#comment143930>

    I think it's potentially confusing to call them short-lived (yes, I know that's historically how they've been used and how wikipedia categorizes them), since applications are free to bind to them as use them for the entirety of the job lifetime.



docs/network-isolation.md (line 60)
<https://reviews.apache.org/r/36281/#comment143933>

    You can write directly to `/proc/sys/net/ipv4/ip_local_port_range`. Please state why the reboot is (strongly) advised.



docs/network-isolation.md (line 70)
<https://reviews.apache.org/r/36281/#comment143935>

    Is it also recommended that ephemeral_ports per container be power-2 sized and aligned?
    
    Can you be precise in the limit on the number of containers? Can you document here the master flag to set a global limit to the number of containers to each slave used as a workaround because ephemeral ports are not exposed to the master.
    
    s/packets/packet/



docs/network-isolation.md (line 77)
<https://reviews.apache.org/r/36281/#comment143938>

    Can you state and explain why there's no shaping/limit on ingress?
    
    State explicitly that shaping delays traffic and will not drop packets.


- Ian Downes


On July 7, 2015, 2:54 p.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36281/
> -----------------------------------------------------------
> 
> (Updated July 7, 2015, 2:54 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Document per-container unique egress flows and network queueing statistics.
> 
> 
> Diffs
> -----
> 
>   docs/home.md bc277910907c381c08835b6e9d485b27d6da5002 
>   docs/network-isolation.md PRE-CREATION 
>   docs/network-monitoring.md 8889fb165cc70bc382be0c99de8d7748328abf57 
> 
> Diff: https://reviews.apache.org/r/36281/diff/
> 
> 
> Testing
> -------
> 
> Rendered at https://www.notehub.org/2015/7/7/network-isolation for review.
> 
> 
> Thanks,
> 
> Paul Brett
> 
>


Re: Review Request 36281: Document per-container unique egress flows and network queueing statistics.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36281/#review90828
-----------------------------------------------------------


Bad patch!

Reviews applied: [36281]

Failed command: ./support/apply-review.sh -n -r 36281

Error:
 2015-07-07 23:20:54 URL:https://reviews.apache.org/r/36281/diff/raw/ [21625/21625] -> "36281.patch" [1]
36281.patch:59: trailing whitespace.
Network isolation is enabled on the slave by appending `network/port_mapping` to the slave command line. If the slave has not been compiled with network isolation support, it will refuse to start and print an appropriate error. 
36281.patch:297: new blank line at EOF.
+
warning: 2 lines add whitespace errors.
Successfully applied: Document per-container unique egress flows and network queueing statistics.

Document per-container unique egress flows and network queueing statistics.


Review: https://reviews.apache.org/r/36281
docs/network-isolation.md:40: trailing whitespace.
+Network isolation is enabled on the slave by appending `network/port_mapping` to the slave command line. If the slave has not been compiled with network isolation support, it will refuse to start and print an appropriate error. 
docs/network-isolation.md:278: new blank line at EOF.
Failed to commit patch

- Mesos ReviewBot


On July 7, 2015, 9:54 p.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36281/
> -----------------------------------------------------------
> 
> (Updated July 7, 2015, 9:54 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Document per-container unique egress flows and network queueing statistics.
> 
> 
> Diffs
> -----
> 
>   docs/home.md bc277910907c381c08835b6e9d485b27d6da5002 
>   docs/network-isolation.md PRE-CREATION 
>   docs/network-monitoring.md 8889fb165cc70bc382be0c99de8d7748328abf57 
> 
> Diff: https://reviews.apache.org/r/36281/diff/
> 
> 
> Testing
> -------
> 
> Rendered at https://www.notehub.org/2015/7/7/network-isolation for review.
> 
> 
> Thanks,
> 
> Paul Brett
> 
>