Re: Review Request 38117: Export per container SNMP statistics

2016-01-14 Thread Cong Wang 

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

(Updated Jan. 14, 2016, 7:52 p.m.)


Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone.


Changes
---

rebase


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


Repository: mesos


Description (updated)
---

Export per container SNMP statistics


Diffs (updated)
-

  docs/configuration.md 7e0eb9584f3cb24a388c44d7bc81cd87dbb4072e 
  include/mesos/mesos.proto b12e0f3eff44d90ec01360fc08bf9e597d7ed9dd 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
e3766c313a1bd2a838a73730c62c74c5ee8e1a4c 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
22a8428427b758bae4a0518356d7933c4110cd9f 
  src/slave/flags.hpp 6857fde027fd57b4934cb43ddf435d12900e0b87 
  src/slave/flags.cpp 6eea03a3e482df99c6a632585d82b13d621123d0 
  src/tests/containerizer/port_mapping_tests.cpp 
e3aea53468fa00374320a8b89bdbb64f38e44b01 

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


Testing
---

./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
--pid=


Thanks,

Cong Wang

Re: Review Request 38117: Export per container SNMP statistics

2016-01-14 Thread Mesos ReviewBot 

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


Bad patch!

Reviews applied: [38117]

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

Error:
 2016-01-14 20:29:45 URL:https://reviews.apache.org/r/38117/diff/raw/ 
[20862/20862] -> "38117.patch" [1]
Total errors found: 0
Checking 5 files
Error: Commit message summary (the first line) must end in a period.

- Mesos ReviewBot


On Jan. 14, 2016, 7:52 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38117/
> ---
> 
> (Updated Jan. 14, 2016, 7:52 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3365
> https://issues.apache.org/jira/browse/MESOS-3365
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Export per container SNMP statistics
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 7e0eb9584f3cb24a388c44d7bc81cd87dbb4072e 
>   include/mesos/mesos.proto b12e0f3eff44d90ec01360fc08bf9e597d7ed9dd 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> e3766c313a1bd2a838a73730c62c74c5ee8e1a4c 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 22a8428427b758bae4a0518356d7933c4110cd9f 
>   src/slave/flags.hpp 6857fde027fd57b4934cb43ddf435d12900e0b87 
>   src/slave/flags.cpp 6eea03a3e482df99c6a632585d82b13d621123d0 
>   src/tests/containerizer/port_mapping_tests.cpp 
> e3aea53468fa00374320a8b89bdbb64f38e44b01 
> 
> Diff: https://reviews.apache.org/r/38117/diff/
> 
> 
> Testing
> ---
> 
> ./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
> --pid=
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 38117: Export per container SNMP statistics

2016-01-05 Thread Cong Wang 


> On Dec. 15, 2015, 9:18 p.m., Ian Downes wrote:
> > src/slave/containerizer/mesos/isolators/network/port_mapping.cpp, lines 
> > 1131-1173
> > 
> >
> > Can you please pull the parsing code out into a function and add some 
> > tests around it? One test could read the host's /proc/net/snmp and test 
> > parsing is sucessful, further tests should ensure known values are parsed 
> > correctly.

I just expand the PortMappingStatistics test case for this, because it is a bit 
hard to expose this code to tests.


- Cong


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


On Dec. 15, 2015, 12:05 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38117/
> ---
> 
> (Updated Dec. 15, 2015, 12:05 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3365
> https://issues.apache.org/jira/browse/MESOS-3365
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These stats are those we get by `netstat -s`, they are important for diagnose 
> networking issues.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md c75f56ce849f6960b3b6246bfa6949156a82eabb 
>   include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> cbb94077d46d7b87ffc09b72e02269bc16f25f92 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 49e215ba3502bba029956fedfc8bd828c3b4a028 
>   src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9 
>   src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 
> 
> Diff: https://reviews.apache.org/r/38117/diff/
> 
> 
> Testing
> ---
> 
> ./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
> --pid=
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 38117: Export per container SNMP statistics

2016-01-05 Thread Cong Wang 

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

(Updated Jan. 5, 2016, 7:53 p.m.)


Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone.


Changes
---

Added SNMP into a test case


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


Repository: mesos


Description
---

These stats are those we get by `netstat -s`, they are important for diagnose 
networking issues.


Diffs (updated)
-

  docs/configuration.md a33e802a3ff1246d25f52b15da7905c5b22e339d 
  include/mesos/mesos.proto 158e08774c4a4fa5ec667388c61e55dbdafc7f67 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
e3766c313a1bd2a838a73730c62c74c5ee8e1a4c 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
22a8428427b758bae4a0518356d7933c4110cd9f 
  src/slave/flags.hpp 2b2679c1ae68d120756eaf81e5728d20791d6746 
  src/slave/flags.cpp a60d3c8022aba93fbd17a46dfff601fb1b25bbee 
  src/tests/containerizer/port_mapping_tests.cpp 
9bcf05ec071b44156b57d8515f47ee6a8bbfdfa0 

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


Testing
---

./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
--pid=


Thanks,

Cong Wang

Re: Review Request 38117: Export per container SNMP statistics

2015-12-15 Thread Ian Downes 

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


Looks good, I just want to pull that parsing code out so it can be tested.


src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (lines 1131 - 
1173)


Can you please pull the parsing code out into a function and add some tests 
around it? One test could read the host's /proc/net/snmp and test parsing is 
sucessful, further tests should ensure known values are parsed correctly.


- Ian Downes


On Dec. 14, 2015, 4:05 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38117/
> ---
> 
> (Updated Dec. 14, 2015, 4:05 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3365
> https://issues.apache.org/jira/browse/MESOS-3365
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These stats are those we get by `netstat -s`, they are important for diagnose 
> networking issues.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md c75f56ce849f6960b3b6246bfa6949156a82eabb 
>   include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> cbb94077d46d7b87ffc09b72e02269bc16f25f92 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 49e215ba3502bba029956fedfc8bd828c3b4a028 
>   src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9 
>   src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 
> 
> Diff: https://reviews.apache.org/r/38117/diff/
> 
> 
> Testing
> ---
> 
> ./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
> --pid=
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 38117: Export per container SNMP statistics

2015-12-15 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [38117]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 15, 2015, 12:05 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38117/
> ---
> 
> (Updated Dec. 15, 2015, 12:05 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3365
> https://issues.apache.org/jira/browse/MESOS-3365
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These stats are those we get by `netstat -s`, they are important for diagnose 
> networking issues.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md c75f56ce849f6960b3b6246bfa6949156a82eabb 
>   include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> cbb94077d46d7b87ffc09b72e02269bc16f25f92 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 49e215ba3502bba029956fedfc8bd828c3b4a028 
>   src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9 
>   src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 
> 
> Diff: https://reviews.apache.org/r/38117/diff/
> 
> 
> Testing
> ---
> 
> ./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
> --pid=
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 38117: Export per container SNMP statistics

2015-12-14 Thread Cong Wang 

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

(Updated Dec. 15, 2015, 12:05 a.m.)


Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone.


Changes
---

Rebase and clean up some comments


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


Repository: mesos


Description
---

These stats are those we get by `netstat -s`, they are important for diagnose 
networking issues.


Diffs (updated)
-

  docs/configuration.md c75f56ce849f6960b3b6246bfa6949156a82eabb 
  include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
cbb94077d46d7b87ffc09b72e02269bc16f25f92 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
49e215ba3502bba029956fedfc8bd828c3b4a028 
  src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9 
  src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 

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


Testing
---

./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
--pid=


Thanks,

Cong Wang

Re: Review Request 38117: Export per container SNMP statistics

2015-12-04 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [38117]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 4, 2015, 7:18 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38117/
> ---
> 
> (Updated Dec. 4, 2015, 7:18 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3365
> https://issues.apache.org/jira/browse/MESOS-3365
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These stats are those we get by `netstat -s`, they are important for diagnose 
> networking issues.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 3498368ef2ebdbf814eb4fd2e500461974ed4f13 
>   include/mesos/mesos.proto 001e6430681e6112abe2202c6a61298464044b0d 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> cbb94077d46d7b87ffc09b72e02269bc16f25f92 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 89bb36f936417de8169a2442729fbd7c9d60acb7 
>   src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9 
>   src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 
> 
> Diff: https://reviews.apache.org/r/38117/diff/
> 
> 
> Testing
> ---
> 
> ./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
> --pid=
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 38117: Export per container SNMP statistics

2015-12-04 Thread Cong Wang 

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

(Updated Dec. 4, 2015, 7:18 p.m.)


Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone.


Changes
---

Fix doc


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


Repository: mesos


Description
---

These stats are those we get by `netstat -s`, they are important for diagnose 
networking issues.


Diffs (updated)
-

  docs/configuration.md 3498368ef2ebdbf814eb4fd2e500461974ed4f13 
  include/mesos/mesos.proto 001e6430681e6112abe2202c6a61298464044b0d 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
cbb94077d46d7b87ffc09b72e02269bc16f25f92 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
89bb36f936417de8169a2442729fbd7c9d60acb7 
  src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9 
  src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 

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


Testing
---

./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
--pid=


Thanks,

Cong Wang

Re: Review Request 38117: Export per container SNMP statistics

2015-12-04 Thread Cong Wang 

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

(Updated Dec. 4, 2015, 7:01 p.m.)


Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone.


Changes
---

Rebase and add doc


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


Repository: mesos


Description
---

These stats are those we get by `netstat -s`, they are important for diagnose 
networking issues.


Diffs (updated)
-

  docs/configuration.md 3498368ef2ebdbf814eb4fd2e500461974ed4f13 
  include/mesos/mesos.proto 001e6430681e6112abe2202c6a61298464044b0d 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
cbb94077d46d7b87ffc09b72e02269bc16f25f92 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
89bb36f936417de8169a2442729fbd7c9d60acb7 
  src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9 
  src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 

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


Testing
---

./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
--pid=


Thanks,

Cong Wang

Re: Review Request 38117: Export per container SNMP statistics

2015-11-21 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [38117]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 20, 2015, 11:25 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38117/
> ---
> 
> (Updated Nov. 20, 2015, 11:25 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-3365
> https://issues.apache.org/jira/browse/MESOS-3365
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These stats are those we get by `netstat -s`, they are important for diagnose 
> networking issues.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5ad48bd376c34dd495399b62fa0bd37ddcc5518b 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> ae53c1b1716a7ad9c6e64f9079c972bae31e4ed2 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> e50616fd609588c547c90bba6d7b3b9b3eb4c6a9 
>   src/slave/flags.hpp 6ae7c94d2e05d81c8b970e7dcaa82d8aa4de7936 
>   src/slave/flags.cpp 26f554e1a73f1f7f3d7baec7bfc1a7f456c5677c 
> 
> Diff: https://reviews.apache.org/r/38117/diff/
> 
> 
> Testing
> ---
> 
> ./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
> --pid=
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 38117: Export per container SNMP statistics

2015-11-20 Thread Cong Wang 

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

(Updated Nov. 20, 2015, 11:25 p.m.)


Review request for mesos, Jie Yu, Paul Brett, and Vinod Kone.


Changes
---

Rebase


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


Repository: mesos


Description
---

These stats are those we get by `netstat -s`, they are important for diagnose 
networking issues.


Diffs (updated)
-

  include/mesos/mesos.proto 5ad48bd376c34dd495399b62fa0bd37ddcc5518b 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
ae53c1b1716a7ad9c6e64f9079c972bae31e4ed2 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
e50616fd609588c547c90bba6d7b3b9b3eb4c6a9 
  src/slave/flags.hpp 6ae7c94d2e05d81c8b970e7dcaa82d8aa4de7936 
  src/slave/flags.cpp 26f554e1a73f1f7f3d7baec7bfc1a7f456c5677c 

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


Testing
---

./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
--pid=


Thanks,

Cong Wang

Re: Review Request 38117: Export per container SNMP statistics

2015-10-19 Thread Cong Wang 

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

(Updated Oct. 19, 2015, 6:49 p.m.)


Review request for mesos, Jie Yu, Paul Brett, and Vinod Kone.


Changes
---

Address review comments from Ian


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


Repository: mesos


Description
---

These stats are those we get by `netstat -s`, they are important for diagnose 
networking issues.


Diffs (updated)
-

  include/mesos/mesos.proto 94004343ea615d87d7c7d63a6a811cf8002e 
  src/slave/containerizer/isolators/network/port_mapping.hpp 
ae53c1b1716a7ad9c6e64f9079c972bae31e4ed2 
  src/slave/containerizer/isolators/network/port_mapping.cpp 
e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef 
  src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
  src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2 

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


Testing (updated)
---

./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
--pid=


Thanks,

Cong Wang

Re: Review Request 38117: Export per container SNMP statistics

2015-10-19 Thread Cong Wang 


> On Oct. 16, 2015, 6 p.m., Ian Downes wrote:
> > include/mesos/mesos.proto, lines 702-720
> > 
> >
> > Are the statistics signed? If not, suggest using uint64 type.

Yes, I used uint64 initially and then noticed some TCP stat is -1. :)


> On Oct. 16, 2015, 6 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, lines 1171-1174
> > 
> >
> > Do these inside the parsing loop as you parse each line pair for a 
> > statistics type.

The problem is we don't expose all SNMP stats from kernel here, for example we 
don't care about UDPlite.


> On Oct. 16, 2015, 6 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, lines 1140-1141
> > 
> >
> > Clearer to just declare these inside the loop scope (but my suggestions 
> > below obviate them anyway).

If declared inside the loop, its value can't be kept for the next loop, right? 
We need to at least keep keys across one loop.


- Cong


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


On Sept. 8, 2015, 9:12 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38117/
> ---
> 
> (Updated Sept. 8, 2015, 9:12 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-3365
> https://issues.apache.org/jira/browse/MESOS-3365
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These stats are those we get by `netstat -s`, they are important for diagnose 
> networking issues.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> 4bca0b81bf69fb4cd75e05aacd02d3e818e32d09 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 34ba2294b0bd7d57aa9de073692a2ea8ec62681d 
>   src/slave/flags.hpp b8335aa585376d27b71897f8cbaefaa12f4b3a5c 
>   src/slave/flags.cpp 7539441c685828027db07173e62a4e5fc1e8b54d 
> 
> Diff: https://reviews.apache.org/r/38117/diff/
> 
> 
> Testing
> ---
> 
> Manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 38117: Export per container SNMP statistics

2015-10-16 Thread Ian Downes 

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



include/mesos/mesos.proto (lines 702 - 720)


Are the statistics signed? If not, suggest using uint64 type.



src/slave/containerizer/isolators/network/port_mapping.cpp (line 1139)


No need to store all statistics until the end. Key/value lines for a set of 
statistics are paired so you can add*Statistics after processing each line 
pair. Then you just need a scoped {{hashmap statistics}}.



src/slave/containerizer/isolators/network/port_mapping.cpp (lines 1140 - 1141)


Clearer to just declare these inside the loop scope (but my suggestions 
below obviate them anyway).



src/slave/containerizer/isolators/network/port_mapping.cpp (line 1142)


you don't care about the actual line number, just that you want to toggle 
between "key" and "value" lines, suggest something like "bool isKeyLine" and 
toggle that?



src/slave/containerizer/isolators/network/port_mapping.cpp (line 1144)


Suggest splitting first on ":" to get the statistics type and keys/values, 
then tokenize on " ". You can also validate the type then as well.



src/slave/containerizer/isolators/network/port_mapping.cpp (line 1146)


If you split first on ":" then you don't need a separate keys vector.



src/slave/containerizer/isolators/network/port_mapping.cpp (lines 1150 - 1159)


Assuming you've got the type from splitting on ":" and then you've got the 
keys, you can directly insert val into a statistics hashmap after successfully 
numifying it, i.e., no need for an intermediate values vector.



src/slave/containerizer/isolators/network/port_mapping.cpp (lines 1161 - 1165)


This is an unnecessary mix of indexing and foreach looping. Comments above 
make this unnecessary but just FYI.



src/slave/containerizer/isolators/network/port_mapping.cpp (line 1162)


This should be a {{const string&}} (if kept).



src/slave/containerizer/isolators/network/port_mapping.cpp (line 1169)


Toggle line type here.



src/slave/containerizer/isolators/network/port_mapping.cpp (lines 1171 - 1174)


Do these inside the parsing loop as you parse each line pair for a 
statistics type.


- Ian Downes


On Sept. 8, 2015, 2:12 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38117/
> ---
> 
> (Updated Sept. 8, 2015, 2:12 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-3365
> https://issues.apache.org/jira/browse/MESOS-3365
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These stats are those we get by `netstat -s`, they are important for diagnose 
> networking issues.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> 4bca0b81bf69fb4cd75e05aacd02d3e818e32d09 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 34ba2294b0bd7d57aa9de073692a2ea8ec62681d 
>   src/slave/flags.hpp b8335aa585376d27b71897f8cbaefaa12f4b3a5c 
>   src/slave/flags.cpp 7539441c685828027db07173e62a4e5fc1e8b54d 
> 
> Diff: https://reviews.apache.org/r/38117/diff/
> 
> 
> Testing
> ---
> 
> Manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 38117: Export per container SNMP statistics

2015-09-08 Thread Cong Wang 

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

(Updated Sept. 8, 2015, 9:12 p.m.)


Review request for mesos, Jie Yu, Paul Brett, and Vinod Kone.


Changes
---

Address review comment


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


Repository: mesos


Description
---

These stats are those we get by `netstat -s`, they are important for diagnose 
networking issues.


Diffs (updated)
-

  include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
  src/slave/containerizer/isolators/network/port_mapping.hpp 
4bca0b81bf69fb4cd75e05aacd02d3e818e32d09 
  src/slave/containerizer/isolators/network/port_mapping.cpp 
34ba2294b0bd7d57aa9de073692a2ea8ec62681d 
  src/slave/flags.hpp b8335aa585376d27b71897f8cbaefaa12f4b3a5c 
  src/slave/flags.cpp 7539441c685828027db07173e62a4e5fc1e8b54d 

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


Testing
---

Manual tests


Thanks,

Cong Wang

Re: Review Request 38117: Export per container SNMP statistics

2015-09-08 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [38117]

All tests passed.

- Mesos ReviewBot


On Sept. 8, 2015, 9:12 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38117/
> ---
> 
> (Updated Sept. 8, 2015, 9:12 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-3365
> https://issues.apache.org/jira/browse/MESOS-3365
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These stats are those we get by `netstat -s`, they are important for diagnose 
> networking issues.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> 4bca0b81bf69fb4cd75e05aacd02d3e818e32d09 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 34ba2294b0bd7d57aa9de073692a2ea8ec62681d 
>   src/slave/flags.hpp b8335aa585376d27b71897f8cbaefaa12f4b3a5c 
>   src/slave/flags.cpp 7539441c685828027db07173e62a4e5fc1e8b54d 
> 
> Diff: https://reviews.apache.org/r/38117/diff/
> 
> 
> Testing
> ---
> 
> Manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 38117: Export per container SNMP statistics

2015-09-05 Thread haosdent huang 

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



src/slave/containerizer/isolators/network/port_mapping.cpp (line 1148)


A bit duplicate code here, is it possible to determine which hashmap should 
use, and then fill it?


- haosdent huang


On Sept. 4, 2015, 1:17 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38117/
> ---
> 
> (Updated Sept. 4, 2015, 1:17 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-3365
> https://issues.apache.org/jira/browse/MESOS-3365
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These stats are those we get by `netstat -s`, they are important for diagnose 
> networking issues.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> 4bca0b81bf69fb4cd75e05aacd02d3e818e32d09 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 34ba2294b0bd7d57aa9de073692a2ea8ec62681d 
>   src/slave/flags.hpp b8335aa585376d27b71897f8cbaefaa12f4b3a5c 
>   src/slave/flags.cpp 49fb0007356eedf7095d34a1312e99bafa8fdc4d 
> 
> Diff: https://reviews.apache.org/r/38117/diff/
> 
> 
> Testing
> ---
> 
> Manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 38117: Export per container SNMP statistics

2015-09-03 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [38117]

All tests passed.

- Mesos ReviewBot


On Sept. 4, 2015, 1:17 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38117/
> ---
> 
> (Updated Sept. 4, 2015, 1:17 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-3365
> https://issues.apache.org/jira/browse/MESOS-3365
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These stats are those we get by `netstat -s`, they are important for diagnose 
> networking issues.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> 4bca0b81bf69fb4cd75e05aacd02d3e818e32d09 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 34ba2294b0bd7d57aa9de073692a2ea8ec62681d 
>   src/slave/flags.hpp b8335aa585376d27b71897f8cbaefaa12f4b3a5c 
>   src/slave/flags.cpp 49fb0007356eedf7095d34a1312e99bafa8fdc4d 
> 
> Diff: https://reviews.apache.org/r/38117/diff/
> 
> 
> Testing
> ---
> 
> Manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>