You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Chi Zhang <ch...@gmail.com> on 2015/03/11 00:55:40 UTC

Review Request 31915: MemIsolator: Improved some statistics naming. (MESOS-2104)

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

Review request for mesos and Ian Downes.


Bugs: mesos-2104
    https://issues.apache.org/jira/browse/mesos-2104


Repository: mesos


Description
-------

MemIsolator: Improved some statistics naming. (MESOS-2104)


Diffs
-----

  include/mesos/mesos.proto 9df972d750ce1e4a81d2e96cc508d6f83cad2fc8 
  src/slave/containerizer/isolators/cgroups/mem.cpp 6299ca4ba01b65daa3d75c64150e2738e32b841e 

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


Testing
-------


Thanks,

Chi Zhang


Re: Review Request 31915: MemIsolator: Improved some statistics naming. (MESOS-2104)

Posted by Chi Zhang <ch...@gmail.com>.

> On March 17, 2015, 6:53 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/cgroups/mem.cpp, line 441
> > <https://reviews.apache.org/r/31915/diff/1/?file=890842#file890842line441>
> >
> >     if this conditional on limitSwap?

Do you want it more explicit? If swap is not used, option will be none.


> On March 17, 2015, 6:53 p.m., Ian Downes wrote:
> > include/mesos/mesos.proto, line 454
> > <https://reviews.apache.org/r/31915/diff/1/?file=890841#file890841line454>
> >
> >     For protobuf you cannot re-use tags, they must refer to the same field name or, if they were optional, then you can drop them.
> >     
> >     See "Assigning Tags" [here|https://developers.google.com/protocol-buffers/docs/proto]

change to minimize protobuf modification and added more comments around the area.


- Chi


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


On April 1, 2015, 10:28 p.m., Chi Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31915/
> -----------------------------------------------------------
> 
> (Updated April 1, 2015, 10:28 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Paul Brett.
> 
> 
> Bugs: mesos-2104
>     https://issues.apache.org/jira/browse/mesos-2104
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> MemIsolator: Improved some statistics naming. (MESOS-2104)
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 3a8e8bf303e0576c212951f6028af77e54d93537 
>   src/slave/containerizer/isolators/cgroups/mem.cpp a7a83ef9ad4726aa139a92fc7f5917ed687d33f5 
> 
> Diff: https://reviews.apache.org/r/31915/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>


Re: Review Request 31915: MemIsolator: Improved some statistics naming. (MESOS-2104)

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



include/mesos/mesos.proto
<https://reviews.apache.org/r/31915/#comment124434>

    For protobuf you cannot re-use tags, they must refer to the same field name or, if they were optional, then you can drop them.
    
    See "Assigning Tags" [here|https://developers.google.com/protocol-buffers/docs/proto]



include/mesos/mesos.proto
<https://reviews.apache.org/r/31915/#comment124435>

    ditto



include/mesos/mesos.proto
<https://reviews.apache.org/r/31915/#comment124436>

    ditto



include/mesos/mesos.proto
<https://reviews.apache.org/r/31915/#comment124437>

    double negatives are confusing ;-)
    // This is only set if swap is enabled.



src/slave/containerizer/isolators/cgroups/mem.cpp
<https://reviews.apache.org/r/31915/#comment124440>

    Be explicit that mem_file_bytes is deprecated, otherwise it could be interpreted as all of this block.



src/slave/containerizer/isolators/cgroups/mem.cpp
<https://reviews.apache.org/r/31915/#comment124439>

    Be explicit that mem_anon_bytes is deprecated, otherwise it could be interpreted as all of this block.



src/slave/containerizer/isolators/cgroups/mem.cpp
<https://reviews.apache.org/r/31915/#comment124441>

    if this conditional on limitSwap?


- Ian Downes


On March 10, 2015, 4:55 p.m., Chi Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31915/
> -----------------------------------------------------------
> 
> (Updated March 10, 2015, 4:55 p.m.)
> 
> 
> Review request for mesos and Ian Downes.
> 
> 
> Bugs: mesos-2104
>     https://issues.apache.org/jira/browse/mesos-2104
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> MemIsolator: Improved some statistics naming. (MESOS-2104)
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 9df972d750ce1e4a81d2e96cc508d6f83cad2fc8 
>   src/slave/containerizer/isolators/cgroups/mem.cpp 6299ca4ba01b65daa3d75c64150e2738e32b841e 
> 
> Diff: https://reviews.apache.org/r/31915/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>