Re: Review Request 45605: Introduced HTB class.

[Cong Wang]

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

(Updated June 10, 2016, 5:57 p.m.)


Review request for mesos, Ian Downes and Jie Yu.


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


Repository: mesos


Description
---

Introduced HTB class API, prepare for per-container bandwidth limit.


Diffs
-

  src/linux/routing/internal.hpp 8f68119819f7c79ece1a13ac1894b1802ddc8e19 
  src/linux/routing/queueing/discipline.hpp 
54d6b214ef6a38fd8279f6d01e6f4e3ccfddf634 
  src/linux/routing/queueing/htb.hpp 857646190d21387f98832f5094128505a52a0776 
  src/linux/routing/queueing/htb.cpp faadf32bd48cc6bf968b1229789903c0d01fd75c 
  src/linux/routing/queueing/internal.hpp 
768ed325f9b259e150779eb3ad74f4e5d4bcc7a2 

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


Testing
---

make && make check


Thanks,

Cong Wang

Re: Review Request 47633: Isolation/networking: check if IPv6 is loaded before trying to disable it

[Cong Wang]

> On May 20, 2016, 5:02 a.m., Cong Wang wrote:
> > src/slave/containerizer/mesos/isolators/network/port_mapping.cpp, line 3927
> > <https://reviews.apache.org/r/47633/diff/1/?file=1388820#file1388820line3927>
> >
> > You need to move this check into the script rather than in the C++ 
> > code. Something like "test -f /proc/sys/net/ipv6/conf/all/disable_ipv6 && 
> > echo 1 > /proc/sys/net/ipv6/conf/all/disable_ipv6".
> 
> Zhengju Sha wrote:
> Thanks for the reply.
> I thought this way before, but I notice that we also use os::exists() to 
> check other file in this function, so I choose to keep in consistent. Both of 
> the manner is okay IMHO, is there other considerations to move it to bash 
> script?
> 
> haosdent huang wrote:
> Hmm, keep consistent more reasonable. Because we have
> 
> ```
>   // Enable route_localnet on lo because by default 127.0.0.1 traffic
>   // is dropped. This feature exists on 3.6 kernel or newer.
>   if (os::exists(path::join("/proc/sys/net/ipv4/conf", lo, 
> "route_localnet"))) {
> script << "echo 1 > /proc/sys/net/ipv4/conf/" << lo << 
> "/route_localnet\n";
>   }
> ```

Nope, you don't understand it. Ipv6 module could be removed during preparation 
and launch, although not likely, but still.


- Cong


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


On May 20, 2016, 3:54 a.m., Zhengju Sha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47633/
> ---
> 
> (Updated May 20, 2016, 3:54 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Cong Wang.
> 
> 
> Bugs: MESOS-5381
> https://issues.apache.org/jira/browse/MESOS-5381
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Isolation/networking: check if IPv6 is loaded before trying to disable it
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> ad792def2bb3a1614d21ca28d858e400d2e3ede1 
> 
> Diff: https://reviews.apache.org/r/47633/diff/
> 
> 
> Testing
> ---
> 
> Enniornment and steps:
> 1. Enable mesos-slave --isolation=network/port_mapping on CentOS7.2 with 
> kernel version: 3.10.0-327.10.1.el7.x86_64
> 2. Create application on marathon framework with commands such as "echo 
> hello" using MesosContainerizer
> 3. Load IPv6 module by removing "ipv6.disable=1" of GRUB_CMDLINE_LINUX in 
> /etc/default/grub
> 4. Disable IPv6 module by adding "ipv6.disable=1" of GRUB_CMDLINE_LINUX in 
> /etc/default/grub
> 
> Now mesos can run both of the testcases successfully.
> 
> 
> Thanks,
> 
> Zhengju Sha
> 
>

Re: Review Request 47633: Isolation/networking: check if IPv6 is loaded before trying to disable it

[Cong Wang]

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




src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 3927)
<https://reviews.apache.org/r/47633/#comment198745>

You need to move this check into the script rather than in the C++ code. 
Something like "test -f /proc/sys/net/ipv6/conf/all/disable_ipv6 && echo 1 > 
/proc/sys/net/ipv6/conf/all/disable_ipv6".


- Cong Wang


On May 20, 2016, 3:54 a.m., Zhengju Sha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47633/
> ---
> 
> (Updated May 20, 2016, 3:54 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Cong Wang.
> 
> 
> Bugs: MESOS-5381
> https://issues.apache.org/jira/browse/MESOS-5381
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Isolation/networking: check if IPv6 is loaded before trying to disable it
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> ad792def2bb3a1614d21ca28d858e400d2e3ede1 
> 
> Diff: https://reviews.apache.org/r/47633/diff/
> 
> 
> Testing
> ---
> 
> Enniornment and steps:
> 1. Enable mesos-slave --isolation=network/port_mapping on CentOS7.2 with 
> kernel version: 3.10.0-327.10.1.el7.x86_64
> 2. Create application on marathon framework with commands such as "echo 
> hello" using MesosContainerizer
> 3. Load IPv6 module by removing "ipv6.disable=1" of GRUB_CMDLINE_LINUX in 
> /etc/default/grub
> 4. Disable IPv6 module by adding "ipv6.disable=1" of GRUB_CMDLINE_LINUX in 
> /etc/default/grub
> 
> Now mesos can run both of the testcases successfully.
> 
> 
> Thanks,
> 
> Zhengju Sha
> 
>

Re: Review Request 47209: Establish TCP connection after backing off.

[Cong Wang]

> On May 14, 2016, 2:01 a.m., Qian Zhang wrote:
> > src/slave/slave.cpp, line 1020
> > <https://reviews.apache.org/r/47209/diff/3/?file=1382234#file1382234line1020>
> >
> > If we do the link here, that means we still establish TCP connection 
> > with master right after a new master is detected, right? But I think we 
> > want to do the link after the initial backoff to avoid SYN flood.
> 
> Cong Wang wrote:
> Authentication code path doesn't have a backoff, so we don't want to 
> touch it in this patch.
> 
> Qian Zhang wrote:
> Thanks, so that means if there are a large number of agents in the 
> cluster and each agent has authentication enabled, when new master is 
> detected, it is still possible all those agents try to connect with master at 
> the same time?

If they use TCP, I think so. Also, there is already a TODO saying we need a 
backoff for authentication too.


- Cong


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


On May 13, 2016, 3:37 a.m., David Robinson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47209/
> ---
> 
> (Updated May 13, 2016, 3:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Ian Downes, and Cong Wang.
> 
> 
> Bugs: MESOS-5330
> https://issues.apache.org/jira/browse/MESOS-5330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Establish TCP connection after backing off.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 
> 
> Diff: https://reviews.apache.org/r/47209/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Also started a master and agent and captured a tcpdump to verify the 
> connection is only established _after_ the registration backoff.
> 
> 
> Thanks,
> 
> David Robinson
> 
>

Re: Review Request 47209: Establish TCP connection after backing off.

[Cong Wang]

> On May 14, 2016, 2:01 a.m., Qian Zhang wrote:
> > src/slave/slave.cpp, line 1020
> > <https://reviews.apache.org/r/47209/diff/3/?file=1382234#file1382234line1020>
> >
> > If we do the link here, that means we still establish TCP connection 
> > with master right after a new master is detected, right? But I think we 
> > want to do the link after the initial backoff to avoid SYN flood.

Authentication code path doesn't have a backoff, so we don't want to touch it 
in this patch.


- Cong


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


On May 13, 2016, 3:37 a.m., David Robinson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47209/
> ---
> 
> (Updated May 13, 2016, 3:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Ian Downes, and Cong Wang.
> 
> 
> Bugs: MESOS-5330
> https://issues.apache.org/jira/browse/MESOS-5330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Establish TCP connection after backing off.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 
> 
> Diff: https://reviews.apache.org/r/47209/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Also started a master and agent and captured a tcpdump to verify the 
> connection is only established _after_ the registration backoff.
> 
> 
> Thanks,
> 
> David Robinson
> 
>

Re: Review Request 47209: Establish TCP connection after backing off.

[Cong Wang]

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


Ship it!




Ship It!

- Cong Wang


On May 11, 2016, 12:58 a.m., David Robinson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47209/
> ---
> 
> (Updated May 11, 2016, 12:58 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Cong Wang.
> 
> 
> Bugs: MESOS-5330
> https://issues.apache.org/jira/browse/MESOS-5330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Establish TCP connection after backing off.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 
> 
> Diff: https://reviews.apache.org/r/47209/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Also started a master and agent and captured a tcpdump to verify the 
> connection is only established _after_ the registration backoff.
> 
> 
> Thanks,
> 
> David Robinson
> 
>

Re: Review Request 47080: Agent should backoff when establishing a socket.

[Cong Wang]

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




src/slave/slave.cpp 
<https://reviews.apache.org/r/47080/#comment196366>

One thing to note is the authentication could need a connect with master, 
so this could impact it. But essentially we need a backoff for authentication 
too.


- Cong Wang


On May 6, 2016, 10:51 p.m., David Robinson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47080/
> ---
> 
> (Updated May 6, 2016, 10:51 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Cong Wang.
> 
> 
> Bugs: MESOS-5330
> https://issues.apache.org/jira/browse/MESOS-5330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Agent should backoff when establishing a socket.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 
> 
> Diff: https://reviews.apache.org/r/47080/diff/
> 
> 
> Testing
> ---
> 
> make tests
> make check
> 
> Also started a master and agent and captured a tcpdump to verify the 
> connection is only established _after_ the registration backoff.
> 
> 
> Thanks,
> 
> David Robinson
> 
>

Re: Review Request 47080: Agent should backoff when establishing a socket.

[Cong Wang]

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


Ship it!




Ship It!

- Cong Wang


On May 6, 2016, 10:51 p.m., David Robinson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47080/
> ---
> 
> (Updated May 6, 2016, 10:51 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Cong Wang.
> 
> 
> Bugs: MESOS-5330
> https://issues.apache.org/jira/browse/MESOS-5330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Agent should backoff when establishing a socket.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 
> 
> Diff: https://reviews.apache.org/r/47080/diff/
> 
> 
> Testing
> ---
> 
> make tests
> make check
> 
> Also started a master and agent and captured a tcpdump to verify the 
> connection is only established _after_ the registration backoff.
> 
> 
> Thanks,
> 
> David Robinson
> 
>

Re: Review Request 45690: Ensured the bind mount root is a shared mount in its own peer group.

[Cong Wang]

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


Ship it!




Ship It!

- Cong Wang


On April 5, 2016, 6:25 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45690/
> ---
> 
> (Updated April 5, 2016, 6:25 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Cong Wang.
> 
> 
> Bugs: MESOS-4662
> https://issues.apache.org/jira/browse/MESOS-4662
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is for the port mapping isolator.
> 
> This is a simple test I did on a fresh Fedora23 box:
> ```
> Jies-MacBook-Pro:fedora23 jie$ vagrant ssh
> Last login: Mon Apr  4 17:27:28 2016 from 10.0.2.2
> [vagrant@localhost ~]$ sudo mkdir /var/run/netns
> [vagrant@localhost ~]$ cat /proc/self/mountinfo
> ...
> 23 58 0:20 / /run rw,nosuid,nodev shared:22 - tmpfs tmpfs rw,seclabel,mode=755
> ...
> [vagrant@localhost ~]$ sudo mount --bind /var/run/netns /var/run/netns
> [vagrant@localhost ~]$ cat /proc/self/mountinfo
> ...
> 23 58 0:20 / /run rw,nosuid,nodev shared:22 - tmpfs tmpfs rw,seclabel,mode=755
> 72 23 0:20 /netns /run/netns rw,nosuid,nodev shared:22 - tmpfs tmpfs 
> rw,seclabel,mode=755
> ...
> [vagrant@localhost ~]$ sudo mount --make-shared /var/run/netns
> [vagrant@localhost ~]$ cat /proc/self/mountinfo
> ...
> 23 58 0:20 / /run rw,nosuid,nodev shared:22 - tmpfs tmpfs rw,seclabel,mode=755
> 72 23 0:20 /netns /run/netns rw,nosuid,nodev shared:22 - tmpfs tmpfs 
> rw,seclabel,mode=755
> ...
> [vagrant@localhost ~]$ sudo touch /var/run/netns/$$ && sudo mount --bind 
> /proc/self/ns/net /var/run/netns/$$
> [vagrant@localhost ~]$ cat /proc/self/mountinfo
> ...
> 23 58 0:20 / /run rw,nosuid,nodev shared:22 - tmpfs tmpfs rw,seclabel,mode=755
> 72 23 0:20 /netns /run/netns rw,nosuid,nodev shared:22 - tmpfs tmpfs 
> rw,seclabel,mode=755
> 74 72 0:3 / /run/netns/1526 rw shared:28 - nsfs nsfs rw
> 75 23 0:3 / /run/netns/1526 rw shared:28 - nsfs nsfs rw
> ```
> 
> As you can see above, a single bind mount creates two entries in the mount 
> table (`/run/netns/1526`). This is because /run/netns is in the same peer 
> group as /run. So a single mount operation under /run/netns will be 
> propergated to /run as well, creating two mounts. This will confuse the 
> recovery logic in the port mapping isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 323c84a3d960a196d8ba87f753814e9d43a07957 
> 
> Diff: https://reviews.apache.org/r/45690/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on Fedora 23
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 45690: Ensured the bind mount root is a shared mount in its own peer group.

[Cong Wang]

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




src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 1963)
<https://reviews.apache.org/r/45690/#comment190275>

It is still not clear why we need to handle this case, at least I don't see 
Fedora needs it. If it is really needed by some distro, please add some 
comments to explain this.


- Cong Wang


On April 4, 2016, 5:30 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45690/
> ---
> 
> (Updated April 4, 2016, 5:30 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Cong Wang.
> 
> 
> Bugs: MESOS-4662
> https://issues.apache.org/jira/browse/MESOS-4662
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is for the port mapping isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 323c84a3d960a196d8ba87f753814e9d43a07957 
> 
> Diff: https://reviews.apache.org/r/45690/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on Fedora 23
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 45520: Used realpath for the bind mount root in port mapping isolator.

[Cong Wang]

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


Ship it!




You don't have to pass the real path to all the places, other places than the 
mount table checking should be fine. But I am totally fine with this patch too.

Please consider to backport this to other releases.

- Cong Wang


On April 4, 2016, 5:29 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45520/
> ---
> 
> (Updated April 4, 2016, 5:29 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Cong Wang.
> 
> 
> Bugs: MESOS-4662
> https://issues.apache.org/jira/browse/MESOS-4662
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used realpath for the bind mount root in port mapping isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> 0fe2f486eb733acf738c1c61fc44f820d7401afc 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 323c84a3d960a196d8ba87f753814e9d43a07957 
>   src/tests/containerizer/port_mapping_tests.cpp 
> e062daa9fcfc776144b48325daa1f1284c5e59a4 
> 
> Diff: https://reviews.apache.org/r/45520/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on Fedora23
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 45607: Move HTB qdisc out of containers.

[Cong Wang]

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

(Updated April 1, 2016, 11:05 p.m.)


Review request for mesos, Ian Downes and Jie Yu.


Changes
---

Updated counters


Summary (updated)
-

Move HTB qdisc out of containers.


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


Repository: mesos


Description (updated)
---

Move HTB qdisc out of containers.


Diffs (updated)
-

  src/linux/routing/queueing/fq_codel.hpp 
6fcd9a21e821da476e9c16f50ed781424338022a 
  src/linux/routing/queueing/fq_codel.cpp 
26860d1552839b47be2b4cbdc575c0d20e6fcdc5 
  src/linux/routing/queueing/htb.hpp 857646190d21387f98832f5094128505a52a0776 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
0fe2f486eb733acf738c1c61fc44f820d7401afc 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
323c84a3d960a196d8ba87f753814e9d43a07957 

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


Testing
---

Manual test with --egress_unique_flow_per_container and/or 
--egress_rate_limit_per_container


Thanks,

Cong Wang

Review Request 45607: Moved HTB out of containers.

[Cong Wang]

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

Review request for mesos, Ian Downes and Jie Yu.


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


Repository: mesos


Description
---

Move HTB out of containers so that containers could borrow bandwidth when idle.


Diffs
-

  src/linux/routing/queueing/fq_codel.hpp 
6fcd9a21e821da476e9c16f50ed781424338022a 
  src/linux/routing/queueing/fq_codel.cpp 
26860d1552839b47be2b4cbdc575c0d20e6fcdc5 
  src/linux/routing/queueing/htb.hpp 857646190d21387f98832f5094128505a52a0776 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
0fe2f486eb733acf738c1c61fc44f820d7401afc 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
323c84a3d960a196d8ba87f753814e9d43a07957 

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


Testing
---

Manual test with --egress_unique_flow_per_container and/or 
--egress_rate_limit_per_container


Thanks,

Cong Wang

Review Request 45606: Added default class to htb qdisc.

[Cong Wang]

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

Review request for mesos, Ian Downes and Jie Yu.


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


Repository: mesos


Description
---

Added default class to htb::create().


Diffs
-

  src/linux/routing/queueing/htb.hpp 857646190d21387f98832f5094128505a52a0776 
  src/linux/routing/queueing/htb.cpp faadf32bd48cc6bf968b1229789903c0d01fd75c 

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


Testing
---

make && make check


Thanks,

Cong Wang

Review Request 45605: Introduced HTB class.

[Cong Wang]

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

Review request for mesos, Ian Downes and Jie Yu.


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


Repository: mesos


Description
---

Introduced HTB class API, prepare for per-container bandwidth limit.


Diffs
-

  src/linux/routing/internal.hpp 8f68119819f7c79ece1a13ac1894b1802ddc8e19 
  src/linux/routing/queueing/discipline.hpp 
54d6b214ef6a38fd8279f6d01e6f4e3ccfddf634 
  src/linux/routing/queueing/htb.hpp 857646190d21387f98832f5094128505a52a0776 
  src/linux/routing/queueing/htb.cpp faadf32bd48cc6bf968b1229789903c0d01fd75c 
  src/linux/routing/queueing/internal.hpp 
768ed325f9b259e150779eb3ad74f4e5d4bcc7a2 

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


Testing
---

make && make check


Thanks,

Cong Wang

Re: Review Request 45520: Fixed the bind mount root issue in port mapping isolator.

[Cong Wang]

> On March 31, 2016, 5:59 p.m., Cong Wang wrote:
> > Why /var/run/netns could be in the same mount peer group with its parent? 
> > At least on fedora21 this is not the case.
> > 
> > Also, why do you fix two bugs in one patch? I know you don't care about 
> > bisect, but even so this is still not a good practice at all.
> 
> Jie Yu wrote:
> I'll split the patch. Regarding the mount peer groups issue, here is the 
> test I did on fedora23:
> ```
> [vagrant@localhost build]$ cat /proc/self/mountinfo 
> 17 58 0:17 / /sys rw,nosuid,nodev,noexec,relatime shared:6 - sysfs sysfs 
> rw,seclabel
> 18 58 0:4 / /proc rw,nosuid,nodev,noexec,relatime shared:5 - proc proc rw
> 19 58 0:6 / /dev rw,nosuid shared:2 - devtmpfs devtmpfs 
> rw,seclabel,size=4076012k,nr_inodes=1019003,mode=755
> 20 17 0:18 / /sys/kernel/security rw,nosuid,nodev,noexec,relatime 
> shared:7 - securityfs securityfs rw
> 21 19 0:19 / /dev/shm rw,nosuid,nodev shared:3 - tmpfs tmpfs rw,seclabel
> 22 19 0:13 / /dev/pts rw,nosuid,noexec,relatime shared:4 - devpts devpts 
> rw,seclabel,gid=5,mode=620,ptmxmode=000
> 23 58 0:20 / /run rw,nosuid,nodev shared:22 - tmpfs tmpfs 
> rw,seclabel,mode=755
> 24 17 0:21 / /sys/fs/cgroup ro,nosuid,nodev,noexec shared:8 - tmpfs tmpfs 
> ro,seclabel,mode=755
> 25 24 0:22 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime 
> shared:9 - cgroup cgroup 
> rw,xattr,release_agent=/usr/lib/systemd/systemd-cgroups-agent,name=systemd
> 26 17 0:23 / /sys/fs/pstore rw,nosuid,nodev,noexec,relatime shared:19 - 
> pstore pstore rw,seclabel
> 27 24 0:24 / /sys/fs/cgroup/blkio rw,nosuid,nodev,noexec,relatime 
> shared:10 - cgroup cgroup rw,blkio
> 28 24 0:25 / /sys/fs/cgroup/net_cls,net_prio 
> rw,nosuid,nodev,noexec,relatime shared:11 - cgroup cgroup rw,net_cls,net_prio
> 29 24 0:26 / /sys/fs/cgroup/freezer rw,nosuid,nodev,noexec,relatime 
> shared:12 - cgroup cgroup rw,freezer
> 30 24 0:27 / /sys/fs/cgroup/memory rw,nosuid,nodev,noexec,relatime 
> shared:13 - cgroup cgroup rw,memory
> 31 24 0:28 / /sys/fs/cgroup/perf_event rw,nosuid,nodev,noexec,relatime 
> shared:14 - cgroup cgroup rw,perf_event
> 32 24 0:29 / /sys/fs/cgroup/cpu,cpuacct rw,nosuid,nodev,noexec,relatime 
> shared:15 - cgroup cgroup rw,cpu,cpuacct
> 33 24 0:30 / /sys/fs/cgroup/devices rw,nosuid,nodev,noexec,relatime 
> shared:16 - cgroup cgroup rw,devices
> 34 24 0:31 / /sys/fs/cgroup/hugetlb rw,nosuid,nodev,noexec,relatime 
> shared:17 - cgroup cgroup rw,hugetlb
> 35 24 0:32 / /sys/fs/cgroup/cpuset rw,nosuid,nodev,noexec,relatime 
> shared:18 - cgroup cgroup rw,cpuset
> 56 17 0:33 / /sys/kernel/config rw,relatime shared:20 - configfs configfs 
> rw
> 58 0 8:1 / / rw,relatime shared:1 - ext4 /dev/sda1 
> rw,seclabel,data=ordered
> 36 17 0:16 / /sys/fs/selinux rw,relatime shared:21 - selinuxfs selinuxfs 
> rw
> 37 18 0:34 / /proc/sys/fs/binfmt_misc rw,relatime shared:23 - autofs 
> systemd-1 rw,fd=30,pgrp=1,timeout=0,minproto=5,maxproto=5,direct
> 38 19 0:35 / /dev/hugepages rw,relatime shared:24 - hugetlbfs hugetlbfs 
> rw,seclabel
> 39 19 0:15 / /dev/mqueue rw,relatime shared:25 - mqueue mqueue rw,seclabel
> 40 17 0:7 / /sys/kernel/debug rw,relatime shared:26 - debugfs debugfs 
> rw,seclabel
> 70 23 0:36 / /run/user/1001 rw,nosuid,nodev,relatime shared:27 - tmpfs 
> tmpfs rw,seclabel,size=817560k,mode=700,uid=1001,gid=1001
> [vagrant@localhost build]$ sudo mount^C
> [vagrant@localhost build]$ sudo mkdir /run/netns
> [vagrant@localhost build]$ sudo mount --bind /run/netns /run/netns
> [vagrant@localhost build]$ cat /proc/self/mountinfo 
> 17 58 0:17 / /sys rw,nosuid,nodev,noexec,relatime shared:6 - sysfs sysfs 
> rw,seclabel
> 18 58 0:4 / /proc rw,nosuid,nodev,noexec,relatime shared:5 - proc proc rw
> 19 58 0:6 / /dev rw,nosuid shared:2 - devtmpfs devtmpfs 
> rw,seclabel,size=4076012k,nr_inodes=1019003,mode=755
> 20 17 0:18 / /sys/kernel/security rw,nosuid,nodev,noexec,relatime 
> shared:7 - securityfs securityfs rw
> 21 19 0:19 / /dev/shm rw,nosuid,nodev shared:3 - tmpfs tmpfs rw,seclabel
> 22 19 0:13 / /dev/pts rw,nosuid,noexec,relatime shared:4 - devpts devpts 
> rw,seclabel,gid=5,mode=620,ptmxmode=000
> 23 58 0:20 / /run rw,nosuid,nodev shared:22 - tmpfs tmpfs 
> rw,seclabel,mode=755
> 24 17 0:21 / /sys/fs/cgroup ro,nosuid,nodev,noexec shared:8 - tmpfs tmpfs 
> ro,seclabel,mode=755
> 25 24 0:22 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime 
> shared:9 - cgroup cgroup 
> rw,xattr,release_agent=/usr/lib/systemd/systemd-cgroups-agent,name=systemd
> 26 17 0:23 / /sys/fs/pstore rw,nosuid,node

Re: Review Request 45520: Fixed the bind mount root issue in port mapping isolator.

[Cong Wang]

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



Why /var/run/netns could be in the same mount peer group with its parent? At 
least on fedora21 this is not the case.

Also, why do you fix two bugs in one patch? I know you don't care about bisect, 
but even so this is still not a good practice at all.

- Cong Wang


On March 31, 2016, 1:47 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45520/
> ---
> 
> (Updated March 31, 2016, 1:47 a.m.)
> 
> 
> Review request for mesos, Ian Downes and Cong Wang.
> 
> 
> Bugs: MESOS-4662
> https://issues.apache.org/jira/browse/MESOS-4662
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the bind mount root issue in port mapping isolator. This patch fixed 
> two issues:
> 1) no long assume /var/run/netns is a realpath
> 2) made sure /var/run/netns is a shared mount in its own mount peer group
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> 0fe2f486eb733acf738c1c61fc44f820d7401afc 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 323c84a3d960a196d8ba87f753814e9d43a07957 
>   src/tests/containerizer/port_mapping_tests.cpp 
> e062daa9fcfc776144b48325daa1f1284c5e59a4 
> 
> Diff: https://reviews.apache.org/r/45520/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on Fedora23
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 45513: Fixed mount point check in port mapping isolator.

[Cong Wang]

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

Review request for mesos, Ian Downes and Jie Yu.


Repository: mesos


Description
---

On Fedora, /var/run is a symlink to /run, which causes the mount point check 
code fail to detect it therefore keeps mounting it each time when we restart 
the slave.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
323c84a3d960a196d8ba87f753814e9d43a07957 

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


Testing
---

Manually start mesos slave and verify there is no more leak in /proc/mounts.


Thanks,

Cong Wang

Re: Review Request 45500: Fix Mesos-1104, linux/fs.hpp remove mesos::internal:: should be enough.

[Cong Wang]

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



You must be joking when you say you tested this Linux change on your osx...

- Cong Wang


On March 30, 2016, 6:34 p.m., Deshi Xiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45500/
> ---
> 
> (Updated March 30, 2016, 6:34 p.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Bugs: MESOS-1104
> https://issues.apache.org/jira/browse/MESOS-1104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> other exists code under src/linux/, e.g. perf.hpp, ns.hpp
> I think remove mesos::internal:: should be enough.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp 4525a5d5566e2bc913894e993ac7350f1bbd9cc0 
> 
> Diff: https://reviews.apache.org/r/45500/diff/
> 
> 
> Testing
> ---
> 
> make check on mac_osx 11
> 
> 
> Thanks,
> 
> Deshi Xiao
> 
>

Re: Review Request 44421: Added support for "overlay" keyword.

[Cong Wang]

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




src/slave/containerizer/mesos/provisioner/backend.cpp (line 50)
<https://reviews.apache.org/r/44421/#comment188687>

Why do you need to test overlay fs support in create()? If kernel doesn't 
support this, the mount in provision() will simply fail.


- Cong Wang


On March 18, 2016, 4:22 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44421/
> ---
> 
> (Updated March 18, 2016, 4:22 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Shuai Lin.
> 
> 
> Bugs: MESOS-4874
> https://issues.apache.org/jira/browse/MESOS-4874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The "overlayfs" was renamed to "overlay" in kernel 4.2, for overlay
> support check function, it should check both "overlay" and "overlayfs"
> in "/proc/filesystems".
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp 4525a5d5566e2bc913894e993ac7350f1bbd9cc0 
>   src/linux/fs.cpp dbf94759b6382bfafd3f3b8b4c2047af36a53ad5 
>   src/slave/containerizer/mesos/provisioner/backend.cpp 
> 55652540e35f9c451ad85cfead575a788aa3eba1 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
> 5cc0f8b5a8cd4c945023f874056a8184113186c5 
>   src/tests/environment.cpp ee1bbe6b4e3dda1e27b63d71a08ef0d1d254741a 
> 
> Diff: https://reviews.apache.org/r/44421/diff/
> 
> 
> Testing
> ---
> 
> root@mesos002:~/src/mesos/m1/mesos/build# uname -r
> 4.2.3-040203-generic
> root@mesos002:~/src/mesos/m1/mesos/build# lsmod  | grep over
> overlay45056  1 
> root@mesos002:~/src/mesos/m1/mesos/build# cat /proc/filesystems | grep over
> nodev overlay
> root@mesos002:~/src/
> 
> make
> make check
> ./bin/mesos-tests.sh 
> --gtest_filter="OverlayBackendTest.ROOT_OVERLAYFS_OverlayFSBackend" --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45401: Fixed typo in subprocess doxygen comments.

[Cong Wang]

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


Ship it!




Ship It!

- Cong Wang


On March 28, 2016, 9:42 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45401/
> ---
> 
> (Updated March 28, 2016, 9:42 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typo in subprocess doxygen comments.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> da806640a238e168bd24ea837cbb711041fe1d12 
> 
> Diff: https://reviews.apache.org/r/45401/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45399: Fixed capitalization of Watchdog enum.

[Cong Wang]

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


Ship it!




Ship It!

- Cong Wang


On March 28, 2016, 9:41 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45399/
> ---
> 
> (Updated March 28, 2016, 9:41 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed capitalization of Watchdog enum.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> da806640a238e168bd24ea837cbb711041fe1d12 
>   3rdparty/libprocess/src/subprocess.cpp 
> be32962e94b605ee2e15d35010acd05d58c2b2d3 
> 
> Diff: https://reviews.apache.org/r/45399/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45400: Adapted port_mapping with missing subprocess parameter.

[Cong Wang]

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


Ship it!




Ship It!

- Cong Wang


On March 28, 2016, 9:41 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45400/
> ---
> 
> (Updated March 28, 2016, 9:41 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adapted port_mapping with missing subprocess parameter.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 134b6c759b769cf335539e49eff817973c7f96a4 
>   src/tests/containerizer/port_mapping_tests.cpp 
> de4b6f99f3a994bcedafa801eed9c4a7b79bac23 
> 
> Diff: https://reviews.apache.org/r/45400/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on with port_isolator enabled.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45230: Refactored subproces setup functions [1/7].

[Cong Wang]

I am wondering how does it pass your test since you did `make check`
on both Linux and OSX? It fails immediately for me on Linux...


On Mon, Mar 28, 2016 at 10:09 AM, Joris Van Remoortere
 wrote:
> Joerg will fix these.
> Thanks!
>
> On Mon, Mar 28, 2016 at 7:06 PM, Cong Wang  wrote:
>
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/45230/
>> 3rdparty/libprocess/include/process/subprocess.hpp
>> <https://reviews.apache.org/r/45230/diff/2/?file=1316622#file1316622line311> 
>> (Diff
>> revision 2)
>>
>> public:
>>
>> 301
>>
>> const Setsid setsid = NO_SETSID,
>>
>> This one breaks build even after all of your 7 patches are committed, 
>> because port_mapping.cpp passes flags but no setsid to subprocess(), this is 
>> not allowed by C++.
>>
>>
>> 3rdparty/libprocess/include/process/subprocess.hpp
>> <https://reviews.apache.org/r/45230/diff/2/?file=1316622#file1316622line356> 
>> (Diff
>> revision 2)
>>
>> public:
>>
>> 342
>>
>> const Setsid setsid = NO_SETSID,
>>
>> Ditto
>>
>>
>> - Cong Wang
>>
>> On March 28th, 2016, 4:51 p.m. UTC, Joerg Schad wrote:
>> Review request for mesos and Joris Van Remoortere.
>> By Joerg Schad.
>>
>> *Updated March 28, 2016, 4:51 p.m.*
>> *Bugs: * MESOS-5049 <https://issues.apache.org/jira/browse/MESOS-5049>
>> *Repository: * mesos
>> Description
>>
>> Executing arbitrary setup functions while creating new processes is
>> dangerous as all functions called have to be async safe. As setup
>> functions are used for only very few purposes (setsid, chdir, monitoring
>> and killing a process (see upcoming review) it makes sense to support
>> them safely via parameters to subprocess. Note this review by itself
>> -without the following ones- removing the uses of the old interface will
>> break the build.
>>
>> Testing
>>
>> tested entire chain (see https://reviews.apache.org/r/45236/).
>>
>> Diffs
>>
>>- 3rdparty/libprocess/include/process/ssl/gtest.hpp
>>(2ca705524c8f9bba3c03eef296dc04a353dd236c)
>>- 3rdparty/libprocess/include/process/subprocess.hpp
>>(e0c306aa5cf5f393abb73768bbd287c45730f076)
>>- 3rdparty/libprocess/src/subprocess.cpp
>>(b99bad04f7251169df3bfcec5dee459977440997)
>>
>> View Diff <https://reviews.apache.org/r/45230/diff/>
>>

Re: Review Request 45230: Refactored subproces setup functions [1/7].

[Cong Wang]

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




3rdparty/libprocess/include/process/subprocess.hpp (line 301)
<https://reviews.apache.org/r/45230/#comment188545>

This one breaks build even after all of your 7 patches are committed, 
because port_mapping.cpp passes flags but no setsid to subprocess(), this is 
not allowed by C++.



3rdparty/libprocess/include/process/subprocess.hpp (line 342)
<https://reviews.apache.org/r/45230/#comment188546>

    Ditto


- Cong Wang


On March 28, 2016, 4:51 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45230/
> ---
> 
> (Updated March 28, 2016, 4:51 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executing arbitrary setup functions while creating new processes is
> dangerous as all functions called have to be async safe. As setup
> functions are used for only very few purposes (setsid, chdir, monitoring
> and killing a process (see upcoming review) it makes sense to support
> them safely via parameters to subprocess. Note this review by itself
> -without the following ones- removing the uses of the old interface will
> break the build.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> 2ca705524c8f9bba3c03eef296dc04a353dd236c 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> e0c306aa5cf5f393abb73768bbd287c45730f076 
>   3rdparty/libprocess/src/subprocess.cpp 
> b99bad04f7251169df3bfcec5dee459977440997 
> 
> Diff: https://reviews.apache.org/r/45230/diff/
> 
> 
> Testing
> ---
> 
> tested entire chain (see https://reviews.apache.org/r/45236/).
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 44514: Implemented prepare() method of "network/cni" isolator.

[Cong Wang]

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




src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 228)
<https://reviews.apache.org/r/44514/#comment187871>

You always have ifIndex==0 here, it is never changed. So either you miss 
some code or it is not needed at all.


- Cong Wang


On March 23, 2016, 5:10 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44514/
> ---
> 
> (Updated March 23, 2016, 5:10 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented prepare() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 3878a7e85fe24175b3bd5e3a6268cf32a07f2d8b 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 9552312f9ba1e4df6564cfb737cc41e041cf4407 
> 
> Diff: https://reviews.apache.org/r/44514/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44514: Implemented prepare() method of "network/cni" isolator.

[Cong Wang]

> On March 11, 2016, 6:19 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 228
> > 
> >
> > Can there be a use case where you want multiple NICs to be attached to 
> > the same network? Servers use this configuration when they want to utilize 
> > NIC bonding. To aggregate the bandwidth available on the NICs
> 
> Qian Zhang wrote:
> Good point! I think it makes sense that user enables NIC bonding by 
> creating a bond device (e.g., bond0) as the master of the normal ethernet 
> devices (e.g., eth0 and eth1), and both eth0 and eth1 are set up by CNI 
> plugin and get assigned IP from CNI plugin in the same subnet. My only 
> concern is, how to configure the IP for bond0, maybe just use IP of either 
> eth0 or eth1 as its IP?
> 
> Avinash sridharan wrote:
> I think for the time being we can leave the behavior as is. To support 
> something like NIC bonding, I think we will need to introduce the concept of 
> `repeated` devices in `NetworkInfo`, which is definitely out of the scope of 
> this work.

Bonding is a L2 device as a whole, therefore you should just set IP address on 
the bonding master, its slaves don't need IP addresses.


- Cong


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


On March 23, 2016, 5:10 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44514/
> ---
> 
> (Updated March 23, 2016, 5:10 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented prepare() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 3878a7e85fe24175b3bd5e3a6268cf32a07f2d8b 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 9552312f9ba1e4df6564cfb737cc41e041cf4407 
> 
> Diff: https://reviews.apache.org/r/44514/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 45002: Added FTS_PHYSICAL option to fts_open for rmdir.

[Cong Wang]

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



Not related to your patch, but it also makes sense to add FTS_NOSTAT flag to 
speed up? We don't need statp anyway.

- Cong Wang


On March 18, 2016, 12:16 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45002/
> ---
> 
> (Updated March 18, 2016, 12:16 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> According to the documentation for fts_open, either FTS_PHYSICAL or 
> FTS_LOGICAL
> SHOULD be provided. We need FTS_PHYSICAL for the case of rmdir as we dont want
> to resolve the symlink targets while deleting them.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> daa46e05d113fd62ea9dad042ec41aaab28ad003 
> 
> Diff: https://reviews.apache.org/r/45002/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 45002: Added FTS_PHYSICAL option to fts_open for rmdir.

[Cong Wang]

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


Ship it!




Ship It!

- Cong Wang


On March 18, 2016, 12:16 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45002/
> ---
> 
> (Updated March 18, 2016, 12:16 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> According to the documentation for fts_open, either FTS_PHYSICAL or 
> FTS_LOGICAL
> SHOULD be provided. We need FTS_PHYSICAL for the case of rmdir as we dont want
> to resolve the symlink targets while deleting them.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> daa46e05d113fd62ea9dad042ec41aaab28ad003 
> 
> Diff: https://reviews.apache.org/r/45002/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 45003: Fixed rmdir comment for FTS_SLNONE as per coding guidelines.

[Cong Wang]

> On March 20, 2016, 2:33 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp, line 71
> > 
> >
> > This comment seems incorrect:
> > 
> > (1) The relevant parameters are `FTS_PHYSICAL` vs. `FTS_LOGICAL`, as 
> > well as `FTS_COMFOLLOW`.
> > 
> > (2) Since we _do_ set `FTS_PHYSICAL`, isn't it possible for 
> > `FTS_SLNONE` to be returned?
> 
> Jojy Varghese wrote:
> I believe FTS_LOGICAL or FTS_COMFOLLOW needs to be set in order for 
> FTS_SLNONE to be returned.
> 
> Jie Yu wrote:
> Can you do a simple test to validate that (see if FTS_SLNONE is hit or 
> not)? My understanding is that FTS_SLNONE will still be hit if we're using 
> FTS_PHYSICAL.

Looking at 
https://sourceware.org/git/?p=glibc.git;a=blob;f=io/fts.c;h=0c5abfcdd660871d876751fba351ab25b921e88a;hb=HEAD#l901
 I think Jojy is correct here, I don't see FTS_SLNONE could be set without 
FTS_LOGICAL or FTS_COMFOLLOW.


- Cong


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


On March 18, 2016, 12:17 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45003/
> ---
> 
> (Updated March 18, 2016, 12:17 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed rmdir comment for FTS_SLNONE as per coding guidelines.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
> 
> Diff: https://reviews.apache.org/r/45003/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 40988: stout: Fixed comments for numify(), cleaned up some test code.

[Cong Wang]

> On March 18, 2016, 11:13 p.m., Cong Wang wrote:
> > Can you separate the non-numify() change from the numify() one? It is best 
> > to do one thing per commit.
> 
> Neil Conway wrote:
> This change was committed a few months ago.

I know it, I noticed this one because of 45011. And my comment is for Jie. I 
know that Mesos is still too young to care about bisect, but it is a best 
practice to check in one thing within one commit, this patch/commit does at 
least two things: one for numify(), one for the rest; it also hurts the 
readablity of this patch a little.


- Cong


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


On Dec. 4, 2015, 9:50 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40988/
> ---
> 
> (Updated Dec. 4, 2015, 9:50 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> stout: Fixed comments for numify(), cleaned up some test code.
> 
> Note that numify() handles negative numbers inconsistently, depending on 
> whether they are specified in hex or decimal (see test case). Can you guys 
> take a look at fixing?
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp 
> 322a89946befb0d7006124a08b415e2ef0a03e97 
>   3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp 
> 522fc3470eb3496f7dfe1eeb814b171bcff21f38 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> fc5a821bf3bb1416b67103d0ffd7ab5b88f45cca 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
> 7fa06a980b58040f68bf92d217c866f9e48a57d3 
>   3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 
> 7c0309c41ee5ad18bed30aa31a8361f11bca23a1 
> 
> Diff: https://reviews.apache.org/r/40988/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 40988: stout: Fixed comments for numify(), cleaned up some test code.

[Cong Wang]

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



Can you separate the non-numify() change from the numify() one? It is best to 
do one thing per commit.

- Cong Wang


On Dec. 4, 2015, 9:50 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40988/
> ---
> 
> (Updated Dec. 4, 2015, 9:50 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Cong Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> stout: Fixed comments for numify(), cleaned up some test code.
> 
> Note that numify() handles negative numbers inconsistently, depending on 
> whether they are specified in hex or decimal (see test case). Can you guys 
> take a look at fixing?
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp 
> 322a89946befb0d7006124a08b415e2ef0a03e97 
>   3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp 
> 522fc3470eb3496f7dfe1eeb814b171bcff21f38 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> fc5a821bf3bb1416b67103d0ffd7ab5b88f45cca 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
> 7fa06a980b58040f68bf92d217c866f9e48a57d3 
>   3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 
> 7c0309c41ee5ad18bed30aa31a8361f11bca23a1 
> 
> Diff: https://reviews.apache.org/r/40988/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44230: Added FS_DEFAULT case in rmdir.

[Cong Wang]

> On March 15, 2016, 8:02 p.m., Neil Conway wrote:
> > Is it feasible/portable to have a test case for this change?
> 
> Cong Wang wrote:
> Yes, like in our case, you can create some socket or device file and try 
> to remove the directory contains it, it would fail without this patch.
> 
> David Robinson wrote:
> AFAICT tests have already been added:
> 
> 
> https://github.com/apache/mesos/commit/c55ca2941b0119c13764bccdebcea46119e69e4e

None of these tests covers this bug.


- Cong


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


On March 1, 2016, 10:01 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44230/
> ---
> 
> (Updated March 1, 2016, 10:01 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We currently dont handle special files like device files in rmdir. This change
> adds FS_DEFAULT as one of the cases where we try to unlink a file.
> 
> Reference: http://man7.org/linux/man-pages/man3/fts.3.html
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> bc420c9c10d93ddd619a9eb2c5f4db67f31d722f 
> 
> Diff: https://reviews.apache.org/r/44230/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44230: Added FS_DEFAULT case in rmdir.

[Cong Wang]

> On March 15, 2016, 8:02 p.m., Neil Conway wrote:
> > Is it feasible/portable to have a test case for this change?

Yes, like in our case, you can create some socket or device file and try to 
remove the directory contains it, it would fail without this patch.


- Cong


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


On March 1, 2016, 10:01 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44230/
> ---
> 
> (Updated March 1, 2016, 10:01 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We currently dont handle special files like device files in rmdir. This change
> adds FS_DEFAULT as one of the cases where we try to unlink a file.
> 
> Reference: http://man7.org/linux/man-pages/man3/fts.3.html
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> bc420c9c10d93ddd619a9eb2c5f4db67f31d722f 
> 
> Diff: https://reviews.apache.org/r/44230/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44473: Added decrement operator to Counter.

[Cong Wang]

> On March 10, 2016, 10:30 p.m., Ben Mahler wrote:
> > It looks like we need to update the documentation at the top of Counter to 
> > reflect that we intentionally left out decrementing, because we found that 
> > some metrics libraries and systems have a counter type that only supports 
> > going up or potentially being reset to 0. For example:
> > 
> > http://lymph.readthedocs.org/en/latest/api/metrics_api.html#lymph.core.monitoring.metrics.Counter
> > https://prometheus.io/docs/concepts/metric_types/#counter
> > https://blog.pkhamre.com/understanding-statsd-and-graphite/
> > 
> > Think of a counter as something that just counts events that go by, in 
> > which context decrementing doesn't apply. If we want this decrementing 
> > behavior, we probably want to distinguish it with a new type of Metric. 
> > This is because users have asked for the ability to distinguish different 
> > metric types (e.g. counter vs gauge) because some monitoring systems need 
> > to be told whether it can be decremented.
> > 
> > Make sense?

Yeah, kinda. But I am not sure if it worth the effort, at least the comments 
there already make sense for me:

Counter:

847db528 (Dominic Hamon 2014-04-10 14:04:53 -0700  24) // A Metric that 
represents an integer value that can be incremented and
847db528 (Dominic Hamon 2014-04-10 14:04:53 -0700  25) // decremented.

Gauge:

847db528 (Dominic Hamon2014-04-10 14:04:53 -0700 26) // A Metric that 
represents an instantaneous value evaluated when
847db528 (Dominic Hamon2014-04-10 14:04:53 -0700 27) // 'value' is called.

We are free to define our own Counter or Gauge, we don't have to follow any 
standard here.


- Cong


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


On March 9, 2016, 7:07 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44473/
> ---
> 
> (Updated March 9, 2016, 7:07 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, Vinod Kone, and Jiang Yan 
> Xu.
> 
> 
> Bugs: MESOS-4740
> https://issues.apache.org/jira/browse/MESOS-4740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added decrement operator to Counter.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/counter.hpp 
> a13cc7e18c8b23eae83c326d63874d9d2aaedc0d 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> b84dc8d858f58bc9f52b218b7153510417cf34c2 
> 
> Diff: https://reviews.apache.org/r/44473/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 44675: Updated `/metrics/snapshot` endpoint to use `jsonify`.

[Cong Wang]

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

(Updated March 10, 2016, 9:59 p.m.)


Review request for mesos, Michael Park, Vinod Kone, and Jiang Yan Xu.


Changes
---

Address review comments


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


Repository: mesos


Description
---

Updated `/metrics/snapshot` endpoint to use `jsonify`.


Diffs (updated)
-

  3rdparty/libprocess/src/metrics/metrics.cpp 
f1e6774ebf8670b006ba6ea181439d0ef1529b40 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 44675: Updated `/metrics/snapshot` endpoint to use `jsonify`.

[Cong Wang]

> On March 10, 2016, 9:32 p.m., Michael Park wrote:
> > 3rdparty/libprocess/src/metrics/metrics.cpp, line 234
> > <https://reviews.apache.org/r/44675/diff/1/?file=1294670#file1294670line234>
> >
> > (1) Why not just `snapshot`?
> > (2) We don't use default capture by reference. Could you explicitly 
> > list the variables that need to be captured here?
> 
> Cong Wang wrote:
> 1. Fixed
> 
> 2. We need to capture all the function parameters, and in this context it 
> equals to capture all. So I don't think it makes sense to list them one by 
> one?

Oh, we can actually save one here... I will change as you suggest.


- Cong


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


On March 10, 2016, 8:19 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44675/
> ---
> 
> (Updated March 10, 2016, 8:19 p.m.)
> 
> 
> Review request for mesos, Michael Park, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4740
> https://issues.apache.org/jira/browse/MESOS-4740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `/metrics/snapshot` endpoint to use `jsonify`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> f1e6774ebf8670b006ba6ea181439d0ef1529b40 
> 
> Diff: https://reviews.apache.org/r/44675/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 44675: Updated `/metrics/snapshot` endpoint to use `jsonify`.

[Cong Wang]

> On March 10, 2016, 9:32 p.m., Michael Park wrote:
> > 3rdparty/libprocess/src/metrics/metrics.cpp, line 234
> > <https://reviews.apache.org/r/44675/diff/1/?file=1294670#file1294670line234>
> >
> > (1) Why not just `snapshot`?
> > (2) We don't use default capture by reference. Could you explicitly 
> > list the variables that need to be captured here?

1. Fixed

2. We need to capture all the function parameters, and in this context it 
equals to capture all. So I don't think it makes sense to list them one by one?


- Cong


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


On March 10, 2016, 8:19 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44675/
> ---
> 
> (Updated March 10, 2016, 8:19 p.m.)
> 
> 
> Review request for mesos, Michael Park, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4740
> https://issues.apache.org/jira/browse/MESOS-4740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `/metrics/snapshot` endpoint to use `jsonify`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> f1e6774ebf8670b006ba6ea181439d0ef1529b40 
> 
> Diff: https://reviews.apache.org/r/44675/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Review Request 44675: Updated `/metrics/snapshot` endpoint to use `jsonify`.

[Cong Wang]

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

Review request for mesos, Michael Park, Vinod Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Updated `/metrics/snapshot` endpoint to use `jsonify`.


Diffs
-

  3rdparty/libprocess/src/metrics/metrics.cpp 
f1e6774ebf8670b006ba6ea181439d0ef1529b40 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 44666: Fixed typos in comments in libprocess.

[Cong Wang]

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


Ship it!




Ship It!

- Cong Wang


On March 10, 2016, 6:13 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44666/
> ---
> 
> (Updated March 10, 2016, 6:13 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typos in comments in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 5dae1e71f0cdedd22028bb3657ffad35f776b1c7 
> 
> Diff: https://reviews.apache.org/r/44666/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44026: Moved future tests into future_tests.cpp.

[Cong Wang]

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

(Updated March 9, 2016, 7:25 p.m.)


Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone.


Changes
---

Move FromTry test as suggested by Anand


Repository: mesos


Description
---

Moved future tests into future_tests.cpp.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/future_tests.cpp 
ded1aaed25876d45e33b4a27fffc6a5c46ca92f5 
  3rdparty/libprocess/src/tests/process_tests.cpp 
e9bf80ee69f4add299cb828ed3245ac07398943c 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 44474: Improve master tasks metrics.

[Cong Wang]

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

(Updated March 9, 2016, 7:08 p.m.)


Review request for mesos, Ben Mahler, Ian Downes, Vinod Kone, and Jiang Yan Xu.


Changes
---

Address review comments


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


Repository: mesos


Description (updated)
---

Improve master tasks metrics.


Diffs (updated)
-

  src/master/master.hpp 4fa88f1968c92c1216c0af22a476ef7aa57ae961 
  src/master/master.cpp 249e82ffcef35aa8df3c5b9faef5b9b25d68facc 
  src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
  src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 

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


Testing
---

Manual check


Thanks,

Cong Wang

Re: Review Request 44475: Improve master slave metrics.

[Cong Wang]

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

(Updated March 9, 2016, 7:08 p.m.)


Review request for mesos, Ben Mahler, Ian Downes, Vinod Kone, and Jiang Yan Xu.


Changes
---

Rebase


Summary (updated)
-

Improve master slave metrics.


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


Repository: mesos


Description (updated)
---

Improve master slave metrics.


Diffs (updated)
-

  src/master/master.hpp 4fa88f1968c92c1216c0af22a476ef7aa57ae961 
  src/master/master.cpp 249e82ffcef35aa8df3c5b9faef5b9b25d68facc 
  src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
  src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 

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


Testing
---

Manual check


Thanks,

Cong Wang

Re: Review Request 44473: Added decrement operator to Counter.

[Cong Wang]

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

(Updated March 9, 2016, 7:07 p.m.)


Review request for mesos, Ben Mahler, Ian Downes, Vinod Kone, and Jiang Yan Xu.


Changes
---

Rebase


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


Repository: mesos


Description
---

Added decrement operator to Counter.


Diffs (updated)
-

  3rdparty/libprocess/include/process/metrics/counter.hpp 
a13cc7e18c8b23eae83c326d63874d9d2aaedc0d 
  3rdparty/libprocess/src/tests/metrics_tests.cpp 
b84dc8d858f58bc9f52b218b7153510417cf34c2 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 44026: Moved future tests into future_tests.cpp.

[Cong Wang]

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

(Updated March 9, 2016, 7:01 p.m.)


Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone.


Changes
---

Rebase


Repository: mesos


Description
---

Moved future tests into future_tests.cpp.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/future_tests.cpp 
ded1aaed25876d45e33b4a27fffc6a5c46ca92f5 
  3rdparty/libprocess/src/tests/process_tests.cpp 
e9bf80ee69f4add299cb828ed3245ac07398943c 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 44474: Improve master tasks metrics.

[Cong Wang]

> On March 7, 2016, 11:17 p.m., Neil Conway wrote:
> > src/master/master.cpp, line 6461
> > <https://reviews.apache.org/r/44474/diff/1/?file=1283503#file1283503line6461>
> >
> > Seems like we use post-increment elsewhere in this RR?

I am not sure I understand your question, but post-increment is supported by 
Counter even before my patches.


- Cong


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


On March 7, 2016, 10:51 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44474/
> ---
> 
> (Updated March 7, 2016, 10:51 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Vinod Kone.
> 
> 
> Bugs: MESOS-4740
> https://issues.apache.org/jira/browse/MESOS-4740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoid iterate the list of slaves, instead just maintain some counters.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
>   src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
>   src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
> 
> Diff: https://reviews.apache.org/r/44474/diff/
> 
> 
> Testing
> ---
> 
> Manual check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 44474: Improve master tasks metrics.

[Cong Wang]

> On March 8, 2016, 12:37 a.m., Ian Downes wrote:
> > src/master/metrics.cpp, line 215
> > <https://reviews.apache.org/r/44474/diff/1/?file=1283505#file1283505line215>
> >
> > foreach?

I don't think we can use foreach over enum, can we?


- Cong


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


On March 7, 2016, 10:51 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44474/
> ---
> 
> (Updated March 7, 2016, 10:51 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Vinod Kone.
> 
> 
> Bugs: MESOS-4740
> https://issues.apache.org/jira/browse/MESOS-4740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoid iterate the list of slaves, instead just maintain some counters.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
>   src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
>   src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
> 
> Diff: https://reviews.apache.org/r/44474/diff/
> 
> 
> Testing
> ---
> 
> Manual check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 44474: Improve master tasks metrics.

[Cong Wang]

> On March 8, 2016, 12:37 a.m., Ian Downes wrote:
> > src/master/master.cpp, line 6461
> > <https://reviews.apache.org/r/44474/diff/1/?file=1283503#file1283503line6461>
> >
> > I'm not familiar with this code but it appears to be changing the 
> > behavior substantially. 
> > 
> > The original code sums across all slaves that are registered at the 
> > time of the metric call. The new code tracks counters for the various 
> > states when a task on a registered slave changes. IIUC, this is a 
> > fundamental change in how the metrics are determined because the slave 
> > registration state is considered at different times. For example, a task 
> > may change state on a registered slave and it gets counted, then the slave 
> > becomes unregistered, then the metric endpoint is queried. Old and new code 
> > will give different numbers?
> > 
> > If my understanding is correct, then perhaps (somewhat) more performant 
> > code could be achieved by maintaining counters at the slave level, and then 
> > aggregating the counters from *registered* slaves?

So are you saying I miss the slave unregistration event? But when we unregister 
a slave, we remove all the tasks from that slave too, and the counter is 
updated in removeTask(). Or I misunderstand your point?

As the slaves/tasks states are complicated, I am not surprised at all I miss 
some case, please point it out explicitly so that I can fix it. ;)


> On March 8, 2016, 12:37 a.m., Ian Downes wrote:
> > src/master/metrics.cpp, lines 38-46
> > <https://reviews.apache.org/r/44474/diff/1/?file=1283505#file1283505line38>
> >
> > Please keep the original order which more closely matches the 
> > progression of states, i.e., staging before starting.

Nope, we use TASK_* as an index for this array, therefore it has to be in this 
order.


- Cong


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


On March 7, 2016, 10:51 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44474/
> ---
> 
> (Updated March 7, 2016, 10:51 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Vinod Kone.
> 
> 
> Bugs: MESOS-4740
> https://issues.apache.org/jira/browse/MESOS-4740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoid iterate the list of slaves, instead just maintain some counters.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
>   src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
>   src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
> 
> Diff: https://reviews.apache.org/r/44474/diff/
> 
> 
> Testing
> ---
> 
> Manual check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Review Request 44473: Added decrement operator to Counter.

[Cong Wang]

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

Review request for mesos, Ian Downes and Vinod Kone.


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


Repository: mesos


Description
---

Added decrement operator to Counter.


Diffs
-

  3rdparty/libprocess/include/process/metrics/counter.hpp 
a13cc7e18c8b23eae83c326d63874d9d2aaedc0d 
  3rdparty/libprocess/src/tests/metrics_tests.cpp 
b84dc8d858f58bc9f52b218b7153510417cf34c2 

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


Testing
---

make check


Thanks,

Cong Wang

Review Request 44475: Improve master slaves metrics.

[Cong Wang]

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

Review request for mesos, Ian Downes and Vinod Kone.


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


Repository: mesos


Description
---

Avoid iterate the list of slaves, instead just maintain some counters.


Diffs
-

  src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
  src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
  src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
  src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 

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


Testing
---

Manual check


Thanks,

Cong Wang

Review Request 44474: Improve master tasks metrics.

[Cong Wang]

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

Review request for mesos, Ian Downes and Vinod Kone.


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


Repository: mesos


Description
---

Avoid iterate the list of slaves, instead just maintain some counters.


Diffs
-

  src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
  src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
  src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
  src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 

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


Testing
---

Manual check


Thanks,

Cong Wang

Re: Review Request 44153: Obtained uid/gids before changing filesystem root.

[Cong Wang]

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



1. Looks like after this patch all os::su() callers are gone, can we remove it?
2. How about making getuid()+getgid()+getgrouplist() a helper function to 
reduce code duplication? Same for setgroups()+setuid().

- Cong Wang


On Feb. 29, 2016, 12:09 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44153/
> ---
> 
> (Updated Feb. 29, 2016, 12:09 a.m.)
> 
> 
> Review request for mesos, Ian Downes and Timothy Chen.
> 
> 
> Bugs: MESOS-4757
> https://issues.apache.org/jira/browse/MESOS-4757
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, we call os::su(user) after pivot_root. This is problematic
> because /etc/passwd and /etc/group might be missing in container's root
> filesystem. We should instead, get the uid/gids before pivot_root, and
> call setuid/setgroups after pivot_root.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 4149f084b0b234fb995df04e9d1ca704feec082e 
>   src/slave/containerizer/mesos/launch.cpp 
> 6b3bf163e2a577e6318a4a62f96d6bfd98ef9ae9 
> 
> Diff: https://reviews.apache.org/r/44153/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 44151: Added stout functions to get and set supplementary groud ids.

[Cong Wang]

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




3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp (line 322)
<https://reviews.apache.org/r/44151/#comment182954>

You can construct vector with array, no need to bother a loop.



3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp (line 335)
<https://reviews.apache.org/r/44151/#comment182956>

Use vector::data()?


- Cong Wang


On Feb. 29, 2016, 12:08 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44151/
> ---
> 
> (Updated Feb. 29, 2016, 12:08 a.m.)
> 
> 
> Review request for mesos, Ian Downes and Timothy Chen.
> 
> 
> Bugs: MESOS-4757
> https://issues.apache.org/jira/browse/MESOS-4757
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added stout functions to get and set supplementary groud ids.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> 84a2a021859d4e5c8547ad2a509eebda428a8255 
> 
> Diff: https://reviews.apache.org/r/44151/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 44026: Moved future tests into future_tests.cpp.

[Cong Wang]

> On Feb. 26, 2016, 1:45 a.m., Anand Mazumdar wrote:
> > 3rdparty/libprocess/src/tests/process_tests.cpp, lines 1764-1776
> > <https://reviews.apache.org/r/44026/diff/1/?file=1271139#file1271139line1764>
> >
> > Should this be moved to `future_tests.cpp` too?

Yes, it should, because this is a pure Future test, not related to process.


- Cong


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


On Feb. 25, 2016, 5:59 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44026/
> ---
> 
> (Updated Feb. 25, 2016, 5:59 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved future tests into future_tests.cpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/future_tests.cpp 
> ded1aaed25876d45e33b4a27fffc6a5c46ca92f5 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> e9bf80ee69f4add299cb828ed3245ac07398943c 
> 
> Diff: https://reviews.apache.org/r/44026/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 38074: Calculate schedule latency with trace events

[Cong Wang]

> On Nov. 18, 2015, 12:51 a.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/cgroups/perf_event.hpp, line 97
> > <https://reviews.apache.org/r/38074/diff/5/?file=1087487#file1087487line97>
> >
> > linux/cgroups has an internal {{Result cgroup(pid_t pid, const 
> > string& subsystem)}} that is wrapped for cpu, cpuacct, etc. Can you add an 
> > wrapper for the perf_event cgroup and use that? Yes, it may be less 
> > efficient than looking at the hashmap, but it'll reuse the code and keep 
> > consistency.

We need to cache that value before sampling the events, we can't call cgroup() 
each time when we lookup the pid, because cgroup() always reads from the cgroup 
tasks file.


> On Nov. 18, 2015, 12:51 a.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/cgroups/perf_event.cpp, lines 472-482
> > <https://reviews.apache.org/r/38074/diff/5/?file=1087488#file1087488line472>
> >
> > remove this, see earlier comment.

Removed after switching to pid -> cgroup mapping.


> On Nov. 18, 2015, 12:51 a.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/cgroups/perf_event.cpp, line 486
> > <https://reviews.apache.org/r/38074/diff/5/?file=1087488#file1087488line486>
> >
> > Suggest that you use a hashmap, i.e., just directly 
> > store the cgroup for each pid. Slight tradeoff in size but then you can 
> > avoid findCgroupByPid() and simplify the code.

Done. But note that with pid namespace enabled we could have duplicated pid's 
from different containers. I just add a TODO there.


> On Nov. 18, 2015, 12:51 a.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/cgroups/perf_event.cpp, line 502
> > <https://reviews.apache.org/r/38074/diff/5/?file=1087488#file1087488line502>
> >
> > You don't actually use this as a map? It would be much clearer to use a 
> > vector and explicitly sort with a comparision function on the timestamp. 
> > Suggest just calling it events.
> > 
> > ```
> >   events.push_back(event);
> > }
> > 
> > std::sort(begin(events), end(events), [](...){});
> > ```

I don't see any advantage of sorting it to just using std::map, therefore I 
keep as it is.


> On Nov. 18, 2015, 12:51 a.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/cgroups/perf_event.cpp, line 547
> > <https://reviews.apache.org/r/38074/diff/5/?file=1087488#file1087488line547>
> >
> > Are states enumerated somewhere?

Ideally kernel should export these values, but it doesn't. We could enumerate 
by ourselves, but here we only care about RUNNING which is 0, so...


> On Nov. 18, 2015, 12:51 a.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/cgroups/perf_event.cpp, line 560
> > <https://reviews.apache.org/r/38074/diff/5/?file=1087488#file1087488line560>
> >
> > sort the latencies in schedLatency here?

I don't see any advantage to sort them here than later.


> On Nov. 18, 2015, 12:51 a.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/cgroups/perf_event.cpp, lines 625-627
> > <https://reviews.apache.org/r/38074/diff/5/?file=1087488#file1087488line625>
> >
> > How expensive is this...?

Depends on the workload, we could have tens of thousands events on a busy 
system.


> On Nov. 18, 2015, 12:51 a.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/cgroups/perf_event.cpp, lines 629-632
> > <https://reviews.apache.org/r/38074/diff/5/?file=1087488#file1087488line629>
> >
> > Users have different kernels, code should determine the version at run 
> > time and act accordingly.

That means we would need two separated code to handle two kernel versions.


- Cong


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


On Sept. 30, 2015, 12:15 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38074/
> ---
> 
> (Updated Sept. 30, 2015, 12:15 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2769
> https://issues.apache.org/jira/browse/MESOS-2769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Finally, calculate schedule latency with the sched trace events, and add it 
> to our statistics
> 
> 
> 

Review Request 44026: Moved future tests into future_tests.cpp.

[Cong Wang]

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

Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone.


Repository: mesos


Description
---

Moved future tests into future_tests.cpp.


Diffs
-

  3rdparty/libprocess/src/tests/future_tests.cpp 
ded1aaed25876d45e33b4a27fffc6a5c46ca92f5 
  3rdparty/libprocess/src/tests/process_tests.cpp 
e9bf80ee69f4add299cb828ed3245ac07398943c 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 43932: Added overlayfs provisioning backend.

[Cong Wang]

> On Feb. 25, 2016, 2:52 a.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/provisioner/backends/overlay.cpp, line 106
> > 
> >
> > just a question, does there are any policy when to use `Failure` and 
> > `Error`

Failure is for Future, Error is for Try/Result.


- Cong


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


On Feb. 24, 2016, 4:38 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43932/
> ---
> 
> (Updated Feb. 24, 2016, 4:38 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-2971
> https://issues.apache.org/jira/browse/MESOS-2971
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added overlayfs provisioning backend.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt b13fb23219ebb23bcfd6db062e1c814ca2114aa4 
>   src/Makefile.am 2a26261b513bb7c03437ed8e850c3b36b93d82f5 
>   src/slave/containerizer/mesos/provisioner/backend.cpp 
> 01d06ebc67e259272ee57ea5c75bf7077ede65c4 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp PRE-CREATION 
>   src/slave/flags.cpp 1c6a87b670efde2deab4d6e3f24fd6eb3704a47d 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> 25b28ef8fa5aae81e8dd0c9e33df4160dd912ce8 
>   src/tests/environment.cpp 6cd295f76496770774d090e0485ff87be378f74c 
> 
> Diff: https://reviews.apache.org/r/43932/diff/
> 
> 
> Testing
> ---
> 
> sudo modprobe overlayfs
> sudo make check -j4 
> GTEST_FILTER='OverlayBackendTest.ROOT_OVERLAYFS_OverlayFSBackend'
> 
> - OS: ubuntu 14.04 64bit vm
> - Kernel: 4.2.0-27-generic
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 43461: Used foreach loop to iterate std list/set.

[Cong Wang]

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

(Updated Feb. 25, 2016, 4:11 a.m.)


Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.


Changes
---

Address review comments


Summary (updated)
-

Used foreach loop to iterate std list/set.


Repository: mesos


Description (updated)
---

Used foreach loop to iterate std list/set.


Diffs (updated)
-

  3rdparty/libprocess/include/process/future.hpp 
77655f22ad4ce6e81fd46bcdebbf2a82786e8f49 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 43461: Used C++11 for loop to iterate std list/set.

[Cong Wang]

> On Feb. 24, 2016, 7:29 p.m., Michael Park wrote:
> > 3rdparty/libprocess/include/process/future.hpp, line 791
> > <https://reviews.apache.org/r/43461/diff/2/?file=1268386#file1268386line791>
> >
> > * Mesos currently uses `foreach` from stout rather than 
> > range-based`for`.
> > * Mesos generally prefers explicit types than `auto`.
> > 
> > ```
> > foreach (Future future, futures) {
> >   ...
> > }
> > ```
> > 
> > Here and below.
> 
> Cong Wang wrote:
> Is that a strict rule? At least 
> src/tests/hierarchical_allocator_tests.cpp uses for(auto...) too.
> 
> Michael Park wrote:
> Yeah, those shouldn't have made it into the codebase for consistency. The 
> migration strategy is being discussed/tracked here: 
> [MESOS-3214](https://issues.apache.org/jira/browse/MESOS-3214).

OK. I will use foreach before we migrate to C++11 for loop.


- Cong


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


On Feb. 24, 2016, 7:19 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43461/
> ---
> 
> (Updated Feb. 24, 2016, 7:19 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used C++11 for loop to iterate std list/set.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 77655f22ad4ce6e81fd46bcdebbf2a82786e8f49 
> 
> Diff: https://reviews.apache.org/r/43461/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 43461: Used C++11 for loop to iterate std list/set.

[Cong Wang]

> On Feb. 24, 2016, 7:29 p.m., Michael Park wrote:
> > 3rdparty/libprocess/include/process/future.hpp, line 791
> > <https://reviews.apache.org/r/43461/diff/2/?file=1268386#file1268386line791>
> >
> > * Mesos currently uses `foreach` from stout rather than 
> > range-based`for`.
> > * Mesos generally prefers explicit types than `auto`.
> > 
> > ```
> > foreach (Future future, futures) {
> >   ...
> > }
> > ```
> > 
> > Here and below.

Is that a strict rule? At least src/tests/hierarchical_allocator_tests.cpp uses 
for(auto...) too.


- Cong


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


On Feb. 24, 2016, 7:19 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43461/
> ---
> 
> (Updated Feb. 24, 2016, 7:19 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used C++11 for loop to iterate std list/set.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 77655f22ad4ce6e81fd46bcdebbf2a82786e8f49 
> 
> Diff: https://reviews.apache.org/r/43461/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 43462: Used list::splice() for clock::tick().

[Cong Wang]

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

(Updated Feb. 24, 2016, 7:20 p.m.)


Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.


Changes
---

rebase


Repository: mesos


Description
---

Used list::splice() for clock::tick().


Diffs (updated)
-

  3rdparty/libprocess/src/clock.cpp 6c32792aa2c00a4ff672c5aa0f784304b64d43bf 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 43461: Used C++11 for loop to iterate std list/set.

[Cong Wang]

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

(Updated Feb. 24, 2016, 7:19 p.m.)


Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.


Changes
---

rebase


Repository: mesos


Description
---

Used C++11 for loop to iterate std list/set.


Diffs (updated)
-

  3rdparty/libprocess/include/process/future.hpp 
77655f22ad4ce6e81fd46bcdebbf2a82786e8f49 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 43718: Added fs::supported() function.

[Cong Wang]

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




src/tests/containerizer/fs_tests.cpp (line 48)
<https://reviews.apache.org/r/43718/#comment181441>

Well, not all kernels compile ext4 module, you can actually test "procfs" 
and "sysfs" here, they are required by Mesos.


- Cong Wang


On Feb. 21, 2016, 12:24 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> ---
> 
> (Updated Feb. 21, 2016, 12:24 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added fs::supported() function.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595 
>   src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e 
>   src/tests/containerizer/fs_tests.cpp 
> 29e43877612fa151e6f6d79268a7411272a7bfeb 
> 
> Diff: https://reviews.apache.org/r/43718/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu 14.04 64bit vm
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 43718: Added fs::supported() function.

[Cong Wang]

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


Ship it!




Ship It!

- Cong Wang


On Feb. 19, 2016, 2:02 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> ---
> 
> (Updated Feb. 19, 2016, 2:02 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added fs::supported() function.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595 
>   src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e 
>   src/tests/containerizer/fs_tests.cpp 
> 29e43877612fa151e6f6d79268a7411272a7bfeb 
> 
> Diff: https://reviews.apache.org/r/43718/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 38117: Added per container SNMP statistics.

[Cong Wang]

> On Feb. 18, 2016, 5:49 p.m., Neil Conway wrote:
> > Seems like this commit should have updated the `v1` version of 
> > `mesos.proto`.
> 
> Ian Downes wrote:
> Ahh, you're probably right. @cwang could you please submit a fix.

Sure. https://reviews.apache.org/r/43730/


- Cong


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


On Feb. 11, 2016, 12:48 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38117/
> ---
> 
> (Updated Feb. 11, 2016, 12:48 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
> ---
> 
> Added per container SNMP statistics.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md b26a058001dd8419b701a3cbea063a9d58b9 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> ebf820a2813eef32416f1465eff3f6eea153492f 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 1c2fbe934baabd1d816018de0c077bc9f63e9d33 
>   src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d 
>   src/slave/flags.cpp 2ce7f921c9f53f360143b6927d0aaf78a8b958e7 
>   src/tests/containerizer/port_mapping_tests.cpp 
> aa9846097feda1a82131b67aa4c782ec3625d049 
> 
> Diff: https://reviews.apache.org/r/38117/diff/
> 
> 
> Testing
> ---
> 
> ./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
> --pid=
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Review Request 43730: Added SNMP statistics to v1 mesos.proto too.

[Cong Wang]

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

Review request for mesos, Ian Downes, Jie Yu, and Neil Conway.


Repository: mesos


Description
---

As Neil Conway noticed, we should add these SNMP statistics to v1 mesos.proto 
too.


Diffs
-

  include/mesos/v1/mesos.proto d909e60ddfd8e3ba2075f82c372edde04cd99d54 

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


Testing
---

make


Thanks,

Cong Wang

Re: Review Request 43718: Added fs::supported() function.

[Cong Wang]

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




src/linux/fs.cpp (line 411)
<https://reviews.apache.org/r/43718/#comment180989>

Use os::read() which handles error nicely.



src/linux/fs.cpp (line 413)
<https://reviews.apache.org/r/43718/#comment180990>

Use tokenize("\n")


- Cong Wang


On Feb. 18, 2016, 4:01 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> ---
> 
> (Updated Feb. 18, 2016, 4:01 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added fs::supported() function.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595 
>   src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e 
>   src/tests/containerizer/fs_tests.cpp 
> 29e43877612fa151e6f6d79268a7411272a7bfeb 
> 
> Diff: https://reviews.apache.org/r/43718/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 43535: Fixed name server parsing for PortMappingIsolatorTest.

[Cong Wang]

> On Feb. 12, 2016, 7:05 p.m., Jie Yu wrote:
> > we should ignore the comment lines, instead of relax this check

No, the comment is at the end of the "nameserver" line, not separated.


- Cong


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


On Feb. 12, 2016, 6:57 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43535/
> ---
> 
> (Updated Feb. 12, 2016, 6:57 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, Jie Yu, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It is valid to contain comments in /etc/resolv.conf, so the quickest fix for 
> this is to relax the ASSERT.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/port_mapping_tests.cpp 
> aa9846097feda1a82131b67aa4c782ec3625d049 
> 
> Diff: https://reviews.apache.org/r/43535/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Review Request 43535: Fixed name server parsing for PortMappingIsolatorTest.

[Cong Wang]

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

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


Repository: mesos


Description
---

It is valid to contain comments in /etc/resolv.conf, so the quickest fix for 
this is to relax the ASSERT.


Diffs
-

  src/tests/containerizer/port_mapping_tests.cpp 
aa9846097feda1a82131b67aa4c782ec3625d049 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 38117: Added per container SNMP statistics.

[Cong Wang]

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

(Updated Feb. 11, 2016, 12:48 a.m.)


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


Changes
---

rebase


Summary (updated)
-

Added per container SNMP statistics.


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


Repository: mesos


Description (updated)
---

Added per container SNMP statistics.


Diffs (updated)
-

  docs/configuration.md b26a058001dd8419b701a3cbea063a9d58b9 
  include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
ebf820a2813eef32416f1465eff3f6eea153492f 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
1c2fbe934baabd1d816018de0c077bc9f63e9d33 
  src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d 
  src/slave/flags.cpp 2ce7f921c9f53f360143b6927d0aaf78a8b958e7 
  src/tests/containerizer/port_mapping_tests.cpp 
aa9846097feda1a82131b67aa4c782ec3625d049 

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


Testing
---

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


Thanks,

Cong Wang

Review Request 43461: Used C++11 for loop to iterate std list/set.

[Cong Wang]

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

Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.


Repository: mesos


Description
---

Used C++11 for loop to iterate std list/set.


Diffs
-

  3rdparty/libprocess/include/process/future.hpp 
77655f22ad4ce6e81fd46bcdebbf2a82786e8f49 

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


Testing
---

make check


Thanks,

Cong Wang

Review Request 43462: Used list::splice() for clock::tick().

[Cong Wang]

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

Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.


Repository: mesos


Description
---

Used list::splice() for clock::tick().


Diffs
-

  3rdparty/libprocess/src/clock.cpp 6c32792aa2c00a4ff672c5aa0f784304b64d43bf 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 42047: Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to isolate a mesos container using the net_cls cgroup subsystem.

[Cong Wang]

> On Jan. 15, 2016, 6 a.m., Cong Wang wrote:
> > Why do we need netcls to regulate framework traffic on a per-container 
> > basis? Given the fact that a) the port range based filters already work and 
> > the code (see egress fq_codel) already exists b) we only have port range 
> > based network isolation so far.
> > 
> > I see no point of this. Please describe your use case with details, just 
> > pointing to netcls kernel doc doesn't help at all.
> 
> Cong Wang wrote:
> Since no one answers this, I assume no one in Mesosphere actually 
> understands it... So looks like you are pushing something no one is actually 
> going to use.
> 
> Avinash sridharan wrote:
> The egress_fq_codel that you are pointing too (I am assuming this is the 
> jira you are refferring to https://issues.apache.org/jira/browse/MESOS-2422) 
> needs port mapping isolator to enforce QoS on any egress traffic 
> shaping/policing, and for that matter any network policy enforcement.  
> 
> The net_cls cgroup is a linux kernel construct that allows operators to 
> support traffic shapping/policing and any network policy enforcement using 
> existing networking tools like tc and iptables. By enabling net_cls cgroup it 
> gives mesos a more generalized way of allowing operators to enforce network 
> policy irrespective of whether the task is running in the global namespace or 
> in a specific network namespace. In other words it will allow network policy 
> enforcement to take place irrespective of the type of network isolator you 
> are using. For e.g., if someone wants to use ip-per-container (MESOS-2044) vs 
> the port mapping isolator, operators would still be able to perform policy 
> enforcement without relying on the network isolator to provide those 
> constructs.
> 
> Cong Wang wrote:
> True, I know what netcls is more than you do, but you just ignore the 
> fact that we _only_ have port mapping isolator in our _current_ code, that is 
> my whole point. We can always add this _after_ ip-per-container work is 
> merged in upstream, it is never too late.
> 
> No need to mention this is hard to work together with the fq_codel 
> filters on egress. This is why I ask for more details, but you still don't 
> give any detail so far.
> 
> Jie Yu wrote:
> Cong, we already have other network isolators (e.g., Calico has one for 
> ip per container) through modules. I don't think the operator will allow two 
> network isolators to co-exists in production so I am not so worried about the 
> egress filter conflicts.

I know Calico is ip-per-container, but my point is Calico module is 
out-of-tree, why do we need this in tree just for an out-of-tree module? IOW, 
why not just let Calico carry this code?

Also, you think the operator will not allow that, but you don't document it at 
all...


- Cong


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


On Jan. 20, 2016, 5:05 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42047/
> ---
> 
> (Updated Jan. 20, 2016, 5:05 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4262
> https://issues.apache.org/jira/browse/MESOS-4262
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to 
> isolate a mesos container using the net_cls cgroup subsystem.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt a52203ab9aa47315e6e5c58cc283a7b5df597c76 
>   src/Makefile.am 4fabe600d4ba38c95a777d622b0b675dd5811a53 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42047/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 41911: Added a test case for corrupt packets.

[Cong Wang]

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

(Updated Jan. 25, 2016, 11:01 p.m.)


Review request for mesos, Ian Downes and Jie Yu.


Changes
---

Address review comments and rebase


Summary (updated)
-

Added a test case for corrupt packets.


Repository: mesos


Description (updated)
---

Added a test case for corrupt packets.


Diffs (updated)
-

  src/tests/containerizer/port_mapping_tests.cpp 
182fe9217a5da9af603d6f9c203a1689eff4ca1b 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 41911: Added a test case for corrupt packets

[Cong Wang]

> On Jan. 20, 2016, 6:12 p.m., Ian Downes wrote:
> > src/tests/containerizer/port_mapping_tests.cpp, line 1027
> > <https://reviews.apache.org/r/41911/diff/3/?file=1197167#file1197167line1027>
> >
> > This function is doing a lot, both constructing the packet, opening a 
> > socket and sending the packet. What about splitting this functionality? One 
> > function to construct, another to open/send/close?

I tried, but both constructing the packet and sending the packet require almost 
same parameters, so I am not sure if splitting in this way really improves 
anything here.


- Cong


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


On Jan. 14, 2016, 8:03 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41911/
> ---
> 
> (Updated Jan. 14, 2016, 8:03 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a test case to ensure no corrupt packet could be delivered to application.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/port_mapping_tests.cpp 
> e3aea53468fa00374320a8b89bdbb64f38e44b01 
> 
> Diff: https://reviews.apache.org/r/41911/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 42047: Specified the CgroupsNetClsIsolatorProcess class.

[Cong Wang]

> On Jan. 15, 2016, 6 a.m., Cong Wang wrote:
> > Why do we need netcls to regulate framework traffic on a per-container 
> > basis? Given the fact that a) the port range based filters already work and 
> > the code (see egress fq_codel) already exists b) we only have port range 
> > based network isolation so far.
> > 
> > I see no point of this. Please describe your use case with details, just 
> > pointing to netcls kernel doc doesn't help at all.
> 
> Cong Wang wrote:
> Since no one answers this, I assume no one in Mesosphere actually 
> understands it... So looks like you are pushing something no one is actually 
> going to use.
> 
> Avinash sridharan wrote:
> The egress_fq_codel that you are pointing too (I am assuming this is the 
> jira you are refferring to https://issues.apache.org/jira/browse/MESOS-2422) 
> needs port mapping isolator to enforce QoS on any egress traffic 
> shaping/policing, and for that matter any network policy enforcement.  
> 
> The net_cls cgroup is a linux kernel construct that allows operators to 
> support traffic shapping/policing and any network policy enforcement using 
> existing networking tools like tc and iptables. By enabling net_cls cgroup it 
> gives mesos a more generalized way of allowing operators to enforce network 
> policy irrespective of whether the task is running in the global namespace or 
> in a specific network namespace. In other words it will allow network policy 
> enforcement to take place irrespective of the type of network isolator you 
> are using. For e.g., if someone wants to use ip-per-container (MESOS-2044) vs 
> the port mapping isolator, operators would still be able to perform policy 
> enforcement without relying on the network isolator to provide those 
> constructs.

True, I know what netcls is more than you do, but you just ignore the fact that 
we _only_ have port mapping isolator in our _current_ code, that is my whole 
point. We can always add this _after_ ip-per-container work is merged in 
upstream, it is never too late.

No need to mention this is hard to work together with the fq_codel filters on 
egress. This is why I ask for more details, but you still don't give any detail 
so far.


- Cong


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


On Jan. 15, 2016, 5:42 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42047/
> ---
> 
> (Updated Jan. 15, 2016, 5:42 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4262
> https://issues.apache.org/jira/browse/MESOS-4262
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to 
> isolate a mesos container using the net_cls cgroup subsystem.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 39a23df3227a4f524ea0d408dc894fa5bbab7d10 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42047/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42047: Specified the CgroupsNetClsIsolatorProcess class.

[Cong Wang]

> On Jan. 15, 2016, 6 a.m., Cong Wang wrote:
> > Why do we need netcls to regulate framework traffic on a per-container 
> > basis? Given the fact that a) the port range based filters already work and 
> > the code (see egress fq_codel) already exists b) we only have port range 
> > based network isolation so far.
> > 
> > I see no point of this. Please describe your use case with details, just 
> > pointing to netcls kernel doc doesn't help at all.

Since no one answers this, I assume no one in Mesosphere actually understands 
it... So looks like you are pushing something no one is actually going to use.


- Cong


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


On Jan. 15, 2016, 5:42 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42047/
> ---
> 
> (Updated Jan. 15, 2016, 5:42 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4262
> https://issues.apache.org/jira/browse/MESOS-4262
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to 
> isolate a mesos container using the net_cls cgroup subsystem.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 39a23df3227a4f524ea0d408dc894fa5bbab7d10 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42047/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42047: Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to isolate a mesos container using the net_cls cgroup subsystem.

[Cong Wang]

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


Why do we need netcls to regulate framework traffic on a per-container basis? 
Given the fact that a) the port range based filters already work and the code 
(see egress fq_codel) already exists b) we only have port range based network 
isolation so far.

I see no point of this. Please describe your use case with details, just 
pointing to netcls kernel doc doesn't help at all.

- Cong Wang


On Jan. 15, 2016, 5:44 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42047/
> ---
> 
> (Updated Jan. 15, 2016, 5:44 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4262
> https://issues.apache.org/jira/browse/MESOS-4262
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Specified the CgroupsNetClsIsolatorProcess class. This adds the ability to 
> isolate a mesos container using the net_cls cgroup subsystem.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 39a23df3227a4f524ea0d408dc894fa5bbab7d10 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42047/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 38117: Export per container SNMP statistics

[Cong Wang]

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

(Updated Jan. 14, 2016, 9:02 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 (updated)
---

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 41911: Added a test case for corrupt packets

[Cong Wang]

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

(Updated Jan. 14, 2016, 8:03 p.m.)


Review request for mesos, Ian Downes and Jie Yu.


Changes
---

rebase and cleanup


Repository: mesos


Description
---

Add a test case to ensure no corrupt packet could be delivered to application.


Diffs (updated)
-

  src/tests/containerizer/port_mapping_tests.cpp 
e3aea53468fa00374320a8b89bdbb64f38e44b01 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 38117: Export per container SNMP statistics

[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 41648: Used initializer list c-tor for brevity.

[Cong Wang]

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

Ship it!


Ship It!

- Cong Wang


On Jan. 5, 2016, 11:45 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41648/
> ---
> 
> (Updated Jan. 5, 2016, 11:45 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used initializer list c-tor for brevity.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
> 
> Diff: https://reviews.apache.org/r/41648/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 38117: Export per container SNMP statistics

[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

[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
> > <https://reviews.apache.org/r/38117/diff/7/?file=1164004#file1164004line1131>
> >
> > 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 41911: Added a test case for corrupt packets

[Cong Wang]

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

(Updated Jan. 5, 2016, 6:03 p.m.)


Review request for mesos, Ian Downes and Jie Yu.


Repository: mesos


Description (updated)
---

Add a test case to ensure no corrupt packet could be delivered to application.


Diffs
-

  src/tests/containerizer/port_mapping_tests.cpp 
9bcf05ec071b44156b57d8515f47ee6a8bbfdfa0 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 41911: Added a test case for corrupt packets

[Cong Wang]

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

(Updated Jan. 5, 2016, 3:59 a.m.)


Review request for mesos, Ian Downes and Jie Yu.


Changes
---

Cleanup


Repository: mesos


Description
---

Add a test case to ensure no corrupt packet could be delivered to application


Diffs (updated)
-

  src/tests/containerizer/port_mapping_tests.cpp 
9bcf05ec071b44156b57d8515f47ee6a8bbfdfa0 

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


Testing
---

make check


Thanks,

Cong Wang

Review Request 41911: Added a test case for corrupt packets

[Cong Wang]

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

Review request for mesos, Ian Downes and Jie Yu.


Repository: mesos


Description
---

Add a test case to ensure no corrupt packet could be delivered to application


Diffs
-

  src/tests/containerizer/port_mapping_tests.cpp 
9bcf05ec071b44156b57d8515f47ee6a8bbfdfa0 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 41414: Exit nicely when a pid gets reused

[Cong Wang]

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

(Updated Dec. 15, 2015, 11:48 p.m.)


Review request for mesos, Ian Downes and Jie Yu.


Changes
---

Remove eth0 from error message.


Repository: mesos


Description
---

We saw the following assertion failure when PID got reused before the container 
got cleaned up.


F1110 20:30:17.903465 21542 port_mapping.cpp:2241] Check failed: 
createQdisc.get() 

*** Check failure stack trace: ***

@ 0x7f1d5598c82d  google::LogMessage::Fail()
@ 0x7f1d5598e674  google::LogMessage::SendToLog()
@ 0x7f1d5598c41c  google::LogMessage::Flush()
@ 0x7f1d5598ef69  google::LogMessageFatal::~LogMessageFatal()
@ 0x7f1d55674621  
mesos::internal::slave::PortMappingIsolatorProcess::isolate()
@ 0x7f1d5547f286  
_ZNSt17_Function_handlerIFvPN7process11ProcessBaseEEZNS0_8dispatchI7NothingN5mesos8internal5slave20MesosIsolatorProcessERKNS6_11ContainerIDEiSA_iEENS0_6F
utureIT_EERKNS0_3PIDIT0_EEMSH_FSF_T1_T2_ET3_T4_EUlS2_E_E9_M_invokeERKSt9_Any_dataS2_
@ 0x7f1d5593db91  process::ProcessManager::resume()
@ 0x7f1d5593de97  
_ZNSt6thread5_ImplISt12_Bind_simpleIFSt5_BindIFZN7process14ProcessManager12init_threadsEvEUlRKSt11atomic_boolE_St17reference_wrapperIS6_EEEvEEE6_M_runEv
@ 0x7f1d55a4edf0  execute_native_thread_routine
@ 0x7f1d545c883d  start_thread
@ 0x7f1d5403dfdd  clone
/usr/local/bin/mesos-slave.sh: line 104: 21519 Aborted (core 
dumped) $debug /usr/local/sbin/mesos-slave "${MESOS_FLAGS[@]}"
Slave Exit Status: 134


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
49e215ba3502bba029956fedfc8bd828c3b4a028 

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


Testing
---

Run recovery tests


Thanks,

Cong Wang

Review Request 41414: Exit nicely when a pid gets reused

[Cong Wang]

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

Review request for mesos, Ian Downes and Jie Yu.


Repository: mesos


Description
---

We saw the following assertion failure when PID got reused before the container 
got cleaned up.


F1110 20:30:17.903465 21542 port_mapping.cpp:2241] Check failed: 
createQdisc.get() 

*** Check failure stack trace: ***

@ 0x7f1d5598c82d  google::LogMessage::Fail()
@ 0x7f1d5598e674  google::LogMessage::SendToLog()
@ 0x7f1d5598c41c  google::LogMessage::Flush()
@ 0x7f1d5598ef69  google::LogMessageFatal::~LogMessageFatal()
@ 0x7f1d55674621  
mesos::internal::slave::PortMappingIsolatorProcess::isolate()
@ 0x7f1d5547f286  
_ZNSt17_Function_handlerIFvPN7process11ProcessBaseEEZNS0_8dispatchI7NothingN5mesos8internal5slave20MesosIsolatorProcessERKNS6_11ContainerIDEiSA_iEENS0_6F
utureIT_EERKNS0_3PIDIT0_EEMSH_FSF_T1_T2_ET3_T4_EUlS2_E_E9_M_invokeERKSt9_Any_dataS2_
@ 0x7f1d5593db91  process::ProcessManager::resume()
@ 0x7f1d5593de97  
_ZNSt6thread5_ImplISt12_Bind_simpleIFSt5_BindIFZN7process14ProcessManager12init_threadsEvEUlRKSt11atomic_boolE_St17reference_wrapperIS6_EEEvEEE6_M_runEv
@ 0x7f1d55a4edf0  execute_native_thread_routine
@ 0x7f1d545c883d  start_thread
@ 0x7f1d5403dfdd  clone
/usr/local/bin/mesos-slave.sh: line 104: 21519 Aborted (core 
dumped) $debug /usr/local/sbin/mesos-slave "${MESOS_FLAGS[@]}"
Slave Exit Status: 134


Diffs
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
49e215ba3502bba029956fedfc8bd828c3b4a028 

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


Testing
---

Run recovery tests


Thanks,

Cong Wang

Re: Review Request 39850: Process: Added headers to make `process/mime.hpp` standalone.

[Cong Wang]

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

Ship it!


Ship It!

- Cong Wang


On Nov. 16, 2015, 9:14 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39850/
> ---
> 
> (Updated Nov. 16, 2015, 9:14 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Process: Added headers to make `process/mime.hpp` standalone.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/mime.hpp 
> decdfb6bc2eb60bfc6b25bc7227b11e8a11d5aff 
> 
> Diff: https://reviews.apache.org/r/39850/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38117: Export per container SNMP statistics

[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

Review Request 41245: Use ethtool -k lo to check ethtool command

[Cong Wang]

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

Review request for mesos, David Robinson, Ian Downes, and Jie Yu.


Repository: mesos


Description
---

David reported that on CentOS5 part of ethtool --version output sends to stderr 
instead of stdout, which leads this output in our log.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
7c1724a4ca5c45bb214e5129743e0644c9ca9661 

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


Testing
---

Manually start mesos slave


Thanks,

Cong Wang

Re: Review Request 41158: Turn off rx checksum offloading for veth in container

[Cong Wang]

> On Dec. 11, 2015, 1:16 a.m., David Robinson wrote:
> > src/slave/containerizer/mesos/isolators/network/port_mapping.cpp, lines 
> > 1097-1098
> > <https://reviews.apache.org/r/41158/diff/1/?file=1157658#file1157658line1097>
> >
> > This writes to stderr, which can end up in the logs.
> > 
> > [root@server ~]# ethtool --version 1> /dev/null
> > ethtool version 6
> > Usage:
> > ethtool DEVNAME Display standard information about device
> > 
> > 
> > Log snippet:
> > 
> > I1211 01:05:13.215730 10885 main.cpp:190] Build: 2015-12-10 22:54:33 by 
> > mockbuild
> > I1211 01:05:13.215859 10885 main.cpp:192] Version: 0.26.0-tw5
> > I1211 01:05:13.215996 10885 containerizer.cpp:142] Using isolation: 
> > cgroups/cpu,cgroups/mem,network/port_mapping,posix/disk,cgroups/perf_event,filesystem/posix
> > ethtool version 6
> > Usage:
> > ethtool DEVNAME Display standard information about device
> > I1211 01:05:13.251729 10885 port_mapping.cpp:1255] Using eth0 as the 
> > public interface
> > I1211 01:05:13.252707 10885 port_mapping.cpp:1280] Using lo as the 
> > loopback interface

Ah, yet another difference on Fedora...

$ ethtool --version | grep v
ethtool version 3.8

Let me fix it.


- Cong


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


On Dec. 9, 2015, 11:15 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41158/
> ---
> 
> (Updated Dec. 9, 2015, 11:15 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Bugs: MESOS-4105
> https://issues.apache.org/jira/browse/MESOS-4105
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We noticed that in some cases we delivered some corrupt packets to 
> applications running in our containers. This is clearly wrong. 
> 
> Here is what happens:
> 
> 1) We receive a corrupt packet externally
> 2) The hardware driver is able to checksum it and notices it has a bad 
> checksum
> 3) The driver delivers this packet anyway to wait for TCP layer to checksum 
> it again and then drop it
> 4) This packet is moved to a veth interface because it is for a container
> 5) Both sides of the veth pair have RX checksum offloading by default
> 6) The veth_xmit() marks the packet's checksum as UNNECESSARY since its peer 
> device has rx checksum offloading
> 7) Packet is moved into the container TCP/IP stack
> 8) TCP layer is not going to checksum it since it is not necessary
> 9) The packet gets delivered to application layer
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 89bb36f936417de8169a2442729fbd7c9d60acb7 
> 
> Diff: https://reviews.apache.org/r/41158/diff/
> 
> 
> Testing
> ---
> 
> 1) Turn rx checksum off manually and the bug is gone
> 2) Test this patch and verify rx checksum is turned off as expected.
> 3) I don't see any noticable performance issue after turning this off
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 41158: Turn off rx checksum offloading for veth in container

[Cong Wang]

> On Dec. 10, 2015, 12:27 a.m., Jie Yu wrote:
> > This is more like a question: do we need to turn off tx side as well?

This is not needed, because 1) the physical interface can finally checksum it 
after it moves out of the container to the gateway interface; 2) if the 
physcial interface is not able to do it, the kernel can do it right before 
delivering it.


- Cong


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


On Dec. 9, 2015, 11:15 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41158/
> ---
> 
> (Updated Dec. 9, 2015, 11:15 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Bugs: MESOS-4105
> https://issues.apache.org/jira/browse/MESOS-4105
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We noticed that in some cases we delivered some corrupt packets to 
> applications running in our containers. This is clearly wrong. 
> 
> Here is what happens:
> 
> 1) We receive a corrupt packet externally
> 2) The hardware driver is able to checksum it and notices it has a bad 
> checksum
> 3) The driver delivers this packet anyway to wait for TCP layer to checksum 
> it again and then drop it
> 4) This packet is moved to a veth interface because it is for a container
> 5) Both sides of the veth pair have RX checksum offloading by default
> 6) The veth_xmit() marks the packet's checksum as UNNECESSARY since its peer 
> device has rx checksum offloading
> 7) Packet is moved into the container TCP/IP stack
> 8) TCP layer is not going to checksum it since it is not necessary
> 9) The packet gets delivered to application layer
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 89bb36f936417de8169a2442729fbd7c9d60acb7 
> 
> Diff: https://reviews.apache.org/r/41158/diff/
> 
> 
> Testing
> ---
> 
> 1) Turn rx checksum off manually and the bug is gone
> 2) Test this patch and verify rx checksum is turned off as expected.
> 3) I don't see any noticable performance issue after turning this off
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 41158: Turn off rx checksum offloading for veth in container

[Cong Wang]

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

(Updated Dec. 9, 2015, 3:15 p.m.)


Review request for mesos, Ian Downes and Jie Yu.


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


Repository: mesos


Description
---

We noticed that in some cases we delivered some corrupt packets to applications 
running in our containers. This is clearly wrong. 

Here is what happens:

1) We receive a corrupt packet externally
2) The hardware driver is able to checksum it and notices it has a bad checksum
3) The driver delivers this packet anyway to wait for TCP layer to checksum it 
again and then drop it
4) This packet is moved to a veth interface because it is for a container
5) Both sides of the veth pair have RX checksum offloading by default
6) The veth_xmit() marks the packet's checksum as UNNECESSARY since its peer 
device has rx checksum offloading
7) Packet is moved into the container TCP/IP stack
8) TCP layer is not going to checksum it since it is not necessary
9) The packet gets delivered to application layer


Diffs
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
89bb36f936417de8169a2442729fbd7c9d60acb7 

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


Testing
---

1) Turn rx checksum off manually and the bug is gone
2) Test this patch and verify rx checksum is turned off as expected.
3) I don't see any noticable performance issue after turning this off


Thanks,

Cong Wang

Review Request 41158: Turn off rx checksum offloading for veth in container

[Cong Wang]

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

Review request for mesos, Ian Downes and Jie Yu.


Repository: mesos


Description
---

We noticed that in some cases we delivered some corrupt packets to applications 
running in our containers. This is clearly wrong. 

Here is what happens:

1) We receive a corrupt packet externally
2) The hardware driver is able to checksum it and notices it has a bad checksum
3) The driver delivers this packet anyway to wait for TCP layer to checksum it 
again and then drop it
4) This packet is moved to a veth interface because it is for a container
5) Both sides of the veth pair have RX checksum offloading by default
6) The veth_xmit() marks the packet's checksum as UNNECESSARY since its peer 
device has rx checksum offloading
7) Packet is moved into the container TCP/IP stack
8) TCP layer is not going to checksum it since it is not necessary
9) The packet gets delivered to application layer


Diffs
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
89bb36f936417de8169a2442729fbd7c9d60acb7 

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


Testing
---

1) Turn rx checksum off manually and the bug is gone
2) Test this patch and verify rx checksum is turned off as expected.
3) I don't see any noticable performance issue after turning this off


Thanks,

Cong Wang

Re: Review Request 37541: Add trace event API

[Cong Wang]

> On Nov. 24, 2015, 12:53 a.m., Vinod Kone wrote:
> > src/linux/perf.hpp, lines 108-110
> > <https://reviews.apache.org/r/37541/diff/9/?file=1087482#file1087482line108>
> >
> > why are these public?

Easy to access, instead of adding a get/set for each of them.


> On Nov. 24, 2015, 12:53 a.m., Vinod Kone wrote:
> > src/linux/perf.cpp, lines 1125-1128
> > <https://reviews.apache.org/r/37541/diff/9/?file=1087483#file1087483line1125>
> >
> > this could use some comments.

What comments do you expect here? Some comment to explain what ID is? I thought 
the code is clear.


> On Nov. 24, 2015, 12:53 a.m., Vinod Kone wrote:
> > src/tests/containerizer/cgroups_tests.cpp, line 1105
> > <https://reviews.apache.org/r/37541/diff/9/?file=1087484#file1087484line1105>
> >
> > no need for this?

We need this, otherwise we would still hold a ref to the cgroup which causes a 
failure to destroy it later.


> On Nov. 24, 2015, 12:53 a.m., Vinod Kone wrote:
> > src/linux/perf.cpp, lines 974-1029
> > <https://reviews.apache.org/r/37541/diff/9/?file=1087483#file1087483line974>
> >
> > this could use some comments.

I will add some ASCII arts here to draw the ring buffer.


- Cong


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


On Sept. 30, 2015, 12:14 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37541/
> ---
> 
> (Updated Sept. 30, 2015, 12:14 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2769
> https://issues.apache.org/jira/browse/MESOS-2769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Based on the PerfEvent API's, add API for Linux kernel trace events, 
> especially the schedule trace events.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp d10968ca670eb516bae08385fd0ddde8e8ad83b5 
>   src/linux/perf.cpp f7035ddb2507a7646d88dd517d048018f695448a 
>   src/tests/containerizer/cgroups_tests.cpp 
> 75a3bc0009c037dc18ce319db2eb44630f083e8c 
>   src/tests/containerizer/perf_tests.cpp 
> ed5212ee31b8aa47339b8b8fab184bbdf85be82a 
> 
> Diff: https://reviews.apache.org/r/37541/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 36610: Add explicit syscall header file to linux fs

[Cong Wang]

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


Isn't SYS_xxx in sys/syscall.h instead of syscall.h?

- Cong Wang


On July 20, 2015, 4:21 a.m., Jihun Kang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36610/
> ---
> 
> (Updated July 20, 2015, 4:21 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3085
> https://issues.apache.org/jira/browse/MESOS-3085
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add explicit syscall header file to linux fs
> 
> 
> Diffs
> -
> 
>   src/linux/fs.cpp ea0891e320154b85a21ed2d138c192821efae9cd 
> 
> Diff: https://reviews.apache.org/r/36610/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jihun Kang
> 
>

Re: Review Request 37540: Add perf event API

[Cong Wang]

> On Dec. 2, 2015, 6:13 p.m., Niklas Nielsen wrote:
> > Ping - Cong, is there anything you need to close the last issues? :)

I was blocked at the second to the last issue above and switched to something 
else, now I can think more about it. Thanks!


- Cong


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


On Sept. 30, 2015, 12:12 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37540/
> ---
> 
> (Updated Sept. 30, 2015, 12:12 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2769
> https://issues.apache.org/jira/browse/MESOS-2769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Abstract Linux kernel perf event API and provide API to collect schedule 
> events.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp d10968ca670eb516bae08385fd0ddde8e8ad83b5 
>   src/linux/perf.cpp f7035ddb2507a7646d88dd517d048018f695448a 
>   src/tests/containerizer/perf_tests.cpp 
> ed5212ee31b8aa47339b8b8fab184bbdf85be82a 
> 
> Diff: https://reviews.apache.org/r/37540/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 40953: Avoid accepting hex float literals

[Cong Wang]

> On Dec. 4, 2015, 11:13 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp, line 37
> > <https://reviews.apache.org/r/40953/diff/1/?file=1153565#file1153565line37>
> >
> > Just noticed that this fails to parse negative hex numbers like `-0x2`. 
> > This seems overly restrictive to me.

If you have any use case for that, sure, please do make it better. But I don't 
have any.


> On Dec. 4, 2015, 11:13 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp, line 38
> > <https://reviews.apache.org/r/40953/diff/1/?file=1153565#file1153565line38>
> >
> > Instead of this comment we should specify what *we* support (we are not 
> > writing a C++ lexer, but interpreting user input here).
> > 
> > My expectation for this would be to support what e.g., a `stringstream` 
> > can parse with `hex` or `hexfloat` (or `stoi`, `strtof`, ...).

Yeah we can add doc for this function. Or even better: fix boost::lexical_cast.


> On Dec. 4, 2015, 11:13 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp, line 42
> > <https://reviews.apache.org/r/40953/diff/1/?file=1153566#file1153566line42>
> >
> > As currently implemented we should test that mixed float & exponent 
> > forms also fail, e.g. `0xF.Ep-1`.
> > 
> > I do not agree that being so limited is needed.
> > 
> > Note that e.g., `strtof` would fail to parse `0x10.9` since it lacks 
> > the required exponent.

Ditto, if you need that case, please go for it.


- Cong


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


On Dec. 4, 2015, 5:23 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40953/
> ---
> 
> (Updated Dec. 4, 2015, 5:23 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Ben Mahler, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-4045
> https://issues.apache.org/jira/browse/MESOS-4045
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> GCC, as an extension, accepts hex float literals even for C++, this is not 
> allowed by standard C++. So we disallow it explicitly.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp 
> 2035653d578497e88ef465dc6cd49e2c0fc53366 
>   3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp 
> 26fa7a4a06b36f9e7490e3274264f84b0369d412 
> 
> Diff: https://reviews.apache.org/r/40953/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>