----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59294/#review178796 -----------------------------------------------------------
src/slave/containerizer/mesos/isolators/network/port_mapping.cpp Lines 423-427 (original), 424-428 (patched) <https://reviews.apache.org/r/59294/#comment253023> I think this flag's semantics needs additional explanation. If it is set, but contains an empty JSON, then an existing HTB class is removed. src/slave/containerizer/mesos/isolators/network/port_mapping.cpp Lines 576 (patched) <https://reviews.apache.org/r/59294/#comment253002> Style: For functions the opening brace must be on a separate line. src/slave/containerizer/mesos/isolators/network/port_mapping.cpp Lines 617 (patched) <https://reviews.apache.org/r/59294/#comment253003> Above there's a check that verifies that `rate.isSome()`. src/slave/containerizer/mesos/isolators/network/port_mapping.cpp Lines 604-606 (original), 642-644 (patched) <https://reviews.apache.org/r/59294/#comment253005> The comment is a bit misleading. We're removing the whole qdisc here, not just a class. src/slave/containerizer/mesos/isolators/network/port_mapping.cpp Lines 2209-2210 (original), 2301-2302 (patched) <https://reviews.apache.org/r/59294/#comment253204> Do we expect the helper to hang? Is this a safeguard for the new code deployment? src/slave/containerizer/mesos/isolators/network/port_mapping.cpp Line 2231 (original), 2318 (patched) <https://reviews.apache.org/r/59294/#comment253219> Spelling: s/stdout>/stdout/. src/slave/containerizer/mesos/isolators/network/port_mapping.cpp Lines 4315-4317 (patched) <https://reviews.apache.org/r/59294/#comment253211> If we set `ceil` low, it will prevent heavy CPU users' from bursting. Wouldn't it be more flexible to make `egress_ceil_limit` flag additive instead of absolute? - Ilya Pronin On June 21, 2017, 9:48 p.m., Ian Downes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59294/ > ----------------------------------------------------------- > > (Updated June 21, 2017, 9:48 p.m.) > > > Review request for mesos, Dmitry Zhuk, Ilya Pronin, and Jie Yu. > > > Bugs: MESOS-7508 > https://issues.apache.org/jira/browse/MESOS-7508 > > > Repository: mesos > > > Description > ------- > > Add support to isolators/port_mapping for optionally scaling egress bandwidth > with CPU and with minimum and maximum limits. > > > Diffs > ----- > > include/mesos/mesos.proto e2502acc3542d64c2b2b13f8c0363a3c8372e20e > src/slave/containerizer/mesos/isolators/network/helper.cpp > 4ed879dca42f85fc9dd7638e763822cf10fa8405 > src/slave/containerizer/mesos/isolators/network/port_mapping.hpp > 9d38289c7161d5e931053b587d115684ccc44c94 > src/slave/containerizer/mesos/isolators/network/port_mapping.cpp > cd008aaebcd42554a9a81d2b059269546f59c966 > src/slave/flags.hpp e75c1b4227b443aedf445921b3f2108d930c112c > src/slave/flags.cpp c84aa6724170bba46b4444be8410b71d42a1626e > src/tests/containerizer/port_mapping_tests.cpp > 16e015a8ac53a4aa5336b60c40228720b5f6910a > > > Diff: https://reviews.apache.org/r/59294/diff/4/ > > > Testing > ------- > > # added a new test > $ make check > > > Thanks, > > Ian Downes > >
