Review Request 71111: Added offer rescind logic for guarantees enforcement.

[Meng Zhu]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Outstanding offers need to be rescinded as needed to ensure
roles' requested guarantees can be satisfied.

For simplicity, and also due to the race between the master and
the allocator, we pessimistically assume that what seems like
"available" resources in the allocator are all gone. We greedily
rescind offers until we can satisfy the guarantees.

Also added a test.


Diffs
-

  src/master/quota_handler.cpp 2a3ca5640c850ec8a8aca7abed45386b5b5e1125 
  src/tests/master_quota_tests.cpp 64a9b31a38af23fb16a4afd04f6d3413c7d1ffe9 


Diff: https://reviews.apache.org/r/7/diff/1/


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 71068: Added offer rescind logic for limits enforcement.

[Meng Zhu]

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

(Updated July 17, 2019, 9:28 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Addressed Ben's comment.


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


Repository: mesos


Description
---

Outstanding offers of a role (including offers allocated to its
subroles) are rescinded until the sum of its consumed quota and
outstanding offered resources are below the requested limits.

The rescind logic here, however, overlooks the race between master
and the allocator. Namely, there might be pending offers in the master
mailbox that have yet to be processed. Here, for simplicity, we ignore
this race. To properly handle this, the plan is to move offer
management logic into the allocator and rescind offers there.

Also added a test.


Diffs (updated)
-

  src/master/quota_handler.cpp 2a3ca5640c850ec8a8aca7abed45386b5b5e1125 
  src/tests/master_quota_tests.cpp 64a9b31a38af23fb16a4afd04f6d3413c7d1ffe9 


Diff: https://reviews.apache.org/r/71068/diff/2/

Changes: https://reviews.apache.org/r/71068/diff/1-2/


Testing
---

Ran continously without failure.


Thanks,

Meng Zhu

Re: Review Request 71068: Added offer rescind logic for limits enforcement.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [71109, 71110, 71061, 71068]

Failed command: ['bash', '-c', "set -o pipefail; export OS='ubuntu:14.04' 
BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose 
--disable-libtool-wrappers --disable-parallel-test-execution' 
ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh 2>&1 | tee 
build_71068"]

Error:
..
s\" -DPACKAGE_VERSION=\"1.9.0\" "-DPACKAGE_STRING=\"mesos 1.9.0\"" 
-DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DPACKAGE=\"mesos\" 
-DVERSION=\"1.9.0\" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 
-DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 
-DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 
-DLT_OBJDIR=\".libs/\" -DHAVE_CXX11=1 -DHAVE_PTHREAD_PRIO_INHERIT=1 
-DHAVE_PTHREAD=1 -DHAVE_FTS_H=1 -DHAVE_APR_POOLS_H=1 -DHAVE_LIBAPR_1=1 
-DHAVE_LIBCURL=1 -DMESOS_HAS_JAVA=1 -DHAVE_LIBSASL2=1 -DHAVE_OPENSSL_SSL_H=1 
-DHAVE_SVN_VERSION_H=1 -DHAVE_LIBSVN_SUBR_1=1 -DHAVE_SVN_DELTA_H=1 
-DHAVE_LIBSVN_DELTA_1=1 -DHAVE_ZLIB_H=1 -DHAVE_LIBZ=1 -DHAVE_PYTHON=\"2.7\" 
-DMESOS_HAS_PYTHON=1 -I. -I../../src -Werror 
-DLIBDIR=\"/mesos/mesos-1.9.0/_inst/lib\" 
-DPKGLIBEXECDIR=\"/mesos/mesos-1.9.0/_inst/libexec/mesos\" 
-DPKGDATADIR=\"/mesos/mesos-1.9.0/_inst/share/mesos\" 
-DPKGMODULEDIR=\"/mesos/mesos-1.9.0/_inst/lib/mesos/modules\" -I../../include 
-I../i
 nclude -I../include/mesos -D__STDC_FORMAT_MACROS -I../3rdparty/boost-1.65.0 
-I../3rdparty/concurrentqueue-7b69a8f -I../3rdparty/elfio-3.2 
-I../3rdparty/glog-0.4.0/src -I../3rdparty/grpc-1.10.0/include 
-I../3rdparty/leveldb-1.19/include -I../3rdparty/libarchive-3.3.2/libarchive/ 
-I../../3rdparty/libprocess/include -I../3rdparty/nvml-352.79 
-I../3rdparty/picojson-1.3.0 -I../3rdparty/protobuf-3.5.0/src 
-I../3rdparty/rapidjson-1.1.0/include -I../../3rdparty/stout/include 
-I../3rdparty/zookeeper-3.4.8/src/c/include 
-I../3rdparty/zookeeper-3.4.8/src/c/generated -I/usr/include/subversion-1 
-I/usr/include/apr-1 -I/usr/include/apr-1.0 -pthread -Wall -Wsign-compare 
-Wformat-security -fstack-protector -fPIC -g1 -O0 -Wno-unused-local-typedefs 
-std=c++11 -c ../../src/logging/logging.cpp  -fPIC -DPIC -o 
logging/.libs/libmesos_no_3rdparty_la-logging.o
/bin/bash ../libtool  --tag=CXX   --mode=compile g++ -DPACKAGE_NAME=\"mesos\" 
-DPACKAGE_TARNAME=\"mesos\" -DPACKAGE_VERSION=\"1.9.0\" 
-DPACKAGE_STRING=\"mesos\ 1.9.0\" -DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" 
-DPACKAGE=\"mesos\" -DVERSION=\"1.9.0\" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 
-DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 
-DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 
-DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\" -DHAVE_CXX11=1 
-DHAVE_PTHREAD_PRIO_INHERIT=1 -DHAVE_PTHREAD=1 -DHAVE_FTS_H=1 
-DHAVE_APR_POOLS_H=1 -DHAVE_LIBAPR_1=1 -DHAVE_LIBCURL=1 -DMESOS_HAS_JAVA=1 
-DHAVE_LIBSASL2=1 -DHAVE_OPENSSL_SSL_H=1 -DHAVE_SVN_VERSION_H=1 
-DHAVE_LIBSVN_SUBR_1=1 -DHAVE_SVN_DELTA_H=1 -DHAVE_LIBSVN_DELTA_1=1 
-DHAVE_ZLIB_H=1 -DHAVE_LIBZ=1 -DHAVE_PYTHON=\"2.7\" -DMESOS_HAS_PYTHON=1 -I. 
-I../../src   -Werror -DLIBDIR=\"/mesos/mesos-1.9.0/_inst/lib\" 
-DPKGLIBEXECDIR=\"/mesos/mesos-1.9.0/_inst/libexec/mesos\" 
-DPKGDATADIR=\"/mesos/mesos-1.9.0/_
 inst/share/mesos\" 
-DPKGMODULEDIR=\"/mesos/mesos-1.9.0/_inst/lib/mesos/modules\" -I../../include 
-I../include -I../include/mesos -D__STDC_FORMAT_MACROS 
-I../3rdparty/boost-1.65.0 -I../3rdparty/concurrentqueue-7b69a8f 
-I../3rdparty/elfio-3.2 -I../3rdparty/glog-0.4.0/src 
-I../3rdparty/grpc-1.10.0/include -I../3rdparty/leveldb-1.19/include 
-I../3rdparty/libarchive-3.3.2/libarchive/ -I../../3rdparty/libprocess/include  
-I../3rdparty/nvml-352.79 -I../3rdparty/picojson-1.3.0 
-I../3rdparty/protobuf-3.5.0/src -I../3rdparty/rapidjson-1.1.0/include 
-I../../3rdparty/stout/include -I../3rdparty/zookeeper-3.4.8/src/c/include 
-I../3rdparty/zookeeper-3.4.8/src/c/generated -I/usr/include/subversion-1 
-I/usr/include/apr-1 -I/usr/include/apr-1.0 -pthread -Wall 
-Wsign-compare -Wformat-security -fstack-protector -fPIC -fPIE -g1 -O0 
-Wno-unused-local-typedefs -std=c++11 -c -o 
master/allocator/libmesos_no_3rdparty_la-allocator.lo `test -f 
'master/allocator/allocator.cpp' || echo '../../src/'`maste
 r/allocator/allocator.cpp
libtool: compile:  g++ -DPACKAGE_NAME=\"mesos\" -DPACKAGE_TARNAME=\"mesos\" 
-DPACKAGE_VERSION=\"1.9.0\" "-DPACKAGE_STRING=\"mesos 1.9.0\"" 
-DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DPACKAGE=\"mesos\" 
-DVERSION=\"1.9.0\" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 
-DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 
-DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 
-DLT_OBJDIR=\".libs/\" 

Re: Review Request 71061: Added limits validation for `UPDATE_QUOTA` call.

[Meng Zhu]

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

(Updated July 17, 2019, 6:56 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Updated after MESOS-9888


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


Repository: mesos


Description
---

The validation checks the newly requested limits against
its current consumption. If a role's consumption is already
more than the limits, the update request will be denied
(unless force flag is set to true).

Also added a test.


Diffs (updated)
-

  src/master/quota_handler.cpp 2a3ca5640c850ec8a8aca7abed45386b5b5e1125 
  src/tests/master_quota_tests.cpp 64a9b31a38af23fb16a4afd04f6d3413c7d1ffe9 


Diff: https://reviews.apache.org/r/71061/diff/2/

Changes: https://reviews.apache.org/r/71061/diff/1-2/


Testing
---

make check


Thanks,

Meng Zhu

Review Request 71110: Refactored `struct ResourceBreakdown`.

[Meng Zhu]

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

Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


Repository: mesos


Description
---

This breaks the computation of offered, allocated, reserved
and consumedQuota into separate getter functions which are
lazily calculated upon invoked. The struct is renamed to
`RoleResourceBreakdown`.


Diffs
-

  src/master/master.hpp ffa7423ba533725f7c1123d9aa507b1348e7f281 
  src/master/master.cpp f1ca637b4cb0382caec53b5a81f6a4eb46f4dd2d 
  src/master/readonly_handler.cpp 60dac9ae48e1e84380c7c499a367797069ca64bd 


Diff: https://reviews.apache.org/r/71110/diff/1/


Testing
---

make check


Thanks,

Meng Zhu

Review Request 71109: Added helpers to return resources allocated/reserved to a role subtree.

[Meng Zhu]

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

Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


Repository: mesos


Description
---

Added helpers to return resources allocated/reserved to a role subtree.


Diffs
-

  include/mesos/resources.hpp 3fc2c34b383cf1cabfb5367a72e0e71a1aed4238 
  include/mesos/v1/resources.hpp 39c04035263cb9f96e957d1fb80ab0cc8313f97d 
  src/common/resources.cpp 6e02d5c7cd479d4a8161260df2eba7ae0a4e9b6d 
  src/v1/resources.cpp 25fb2136c0ecae4102bc7dfc72cdb5d8ceeff836 


Diff: https://reviews.apache.org/r/71109/diff/1/


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 71107: Added docs for the NNP isolator.

[Mesos Reviewbot]

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos Reviewbot


On July 17, 2019, 8:38 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71107/
> ---
> 
> (Updated July 17, 2019, 8:38 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added docs for the NNP isolator.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 164465a71c660ab9f01fb18d43076afc4b892ad5 
>   docs/isolators/linux-nnp.md PRE-CREATION 
>   docs/mesos-containerizer.md e79976111ec8e9cc8e8d44b5f1b8d6e2c7e072d6 
> 
> 
> Diff: https://reviews.apache.org/r/71107/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 71080: Master should store the list of completed framework ids for lifecycle.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71080]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On July 17, 2019, 6:58 p.m., Xudong Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71080/
> ---
> 
> (Updated July 17, 2019, 6:58 p.m.)
> 
> 
> Review request for mesos, James Peach and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8636
> https://issues.apache.org/jira/browse/MESOS-8636
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It should be separately from that for webUI and endpoints.
> Currently the master stores the history of completed frameworks in
> a map with the full historical data of the framework, it is
> prohibitively expensive to keep a long history; In order to reject
> frameworks from reregistering if they have previously marked as
> completed, we only need to persist the framework ids and are able
> to keep long history.
> 
> 
> Diffs
> -
> 
>   docs/configuration/master.md c56ac8510ea968f9587e23e81ed310caa968ee9e 
>   docs/operator-http-api.md 2d4a9b66e20cf19eceec718b7de3d812ab285772 
>   include/mesos/allocator/allocator.hpp 
> 2bab53ab5fb25931a724c20a039e1301983ba574 
>   src/master/flags.hpp 78623d68bf428cd3f52684303d98a525d42eb878 
>   src/master/flags.cpp 74f4daadd48e8e691be43759b88dc8b3c2df489a 
>   src/master/http.cpp 765d5052ab6a8d731113f8922d20fb280b843003 
>   src/master/master.hpp ffa7423ba533725f7c1123d9aa507b1348e7f281 
>   src/master/master.cpp f1ca637b4cb0382caec53b5a81f6a4eb46f4dd2d 
> 
> 
> Diff: https://reviews.apache.org/r/71080/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Xudong Ni
> 
>

Re: Review Request 71081: Exposed agent drain information in the webui.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71081]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On July 17, 2019, 8:31 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71081/
> ---
> 
> (Updated July 17, 2019, 8:31 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Joseph Wu.
> 
> 
> Bugs: MESOS-9846
> https://issues.apache.org/jira/browse/MESOS-9846
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed agent drain information in the webui.
> 
> 
> Diffs
> -
> 
>   src/webui/app/agents/agent.html 6d50bfd320973687dcba0aaf2a2aa5d843679ea5 
>   src/webui/app/agents/agents.html 98712c619fb58acf06b465b53d4c9dee85f8841d 
>   src/webui/app/controllers.js 66cd32e666ba490b1194fc8b52bf2db199efdbc9 
> 
> 
> Diff: https://reviews.apache.org/r/71081/diff/2/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> agents summary
>   
> https://reviews.apache.org/media/uploaded/files/2019/07/16/f21569fa-fdb5-4563-ac5d-2d7392bf3914__Screen_Shot_2019-07-16_at_2.03.43_PM.png
> agent detail
>   
> https://reviews.apache.org/media/uploaded/files/2019/07/16/ae8bf9c8-584c-4afe-acbd-650258223933__Screen_Shot_2019-07-16_at_2.03.07_PM.png
> agents summary
>   
> https://reviews.apache.org/media/uploaded/files/2019/07/17/2baa2933-1e98-47f2-bdd5-1cb3a08817f2__Screen_Shot_2019-07-17_at_10.28.37_PM.png
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 71106: Moved kernelVersion check to common code.

[Jacob Janco]

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

Review request for mesos.


Repository: mesos


Description
---

Moved kernelVersion check to common code.


Diffs
-

  src/common/kernel_version.hpp PRE-CREATION 
  src/linux/ns.cpp 2440bb2232dfd9cf57a6cc36aeff874c96297c4b 


Diff: https://reviews.apache.org/r/71106/diff/1/


Testing
---


Thanks,

Jacob Janco

Re: Review Request 70757: Added a NNP isolator.

[Jacob Janco]

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

(Updated July 17, 2019, 8:38 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Changes
---

Broke this off into 3 patches, nnp docs -> this core patch -> kernel version 
change


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


Repository: mesos


Description
---

This patch adds the isolation capability of flipping the
NO_NEW_PRIVILEGES bit for process control.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
2d04f3c182a4274dda527a3da56c894c3c892a12 
  src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 
  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/slave/containerizer/mesos/containerizer.cpp 
c9a369bcdbfc1676912430c3e84fa567bbd7f766 
  src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp 
0419e8e0fbf154396ab5fb5026d77b94cc9fca5b 
  src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db 
  src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/70757/diff/14/

Changes: https://reviews.apache.org/r/70757/diff/13-14/


Testing
---


Thanks,

Jacob Janco

Review Request 71107: Added docs for the NNP isolator.

[Jacob Janco]

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

Review request for mesos.


Repository: mesos


Description
---

Added docs for the NNP isolator.


Diffs
-

  CHANGELOG 164465a71c660ab9f01fb18d43076afc4b892ad5 
  docs/isolators/linux-nnp.md PRE-CREATION 
  docs/mesos-containerizer.md e79976111ec8e9cc8e8d44b5f1b8d6e2c7e072d6 


Diff: https://reviews.apache.org/r/71107/diff/1/


Testing
---


Thanks,

Jacob Janco

Re: Review Request 71081: Exposed agent drain information in the webui.

[Benjamin Bannier]

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

(Updated July 17, 2019, 10:31 p.m.)


Review request for mesos, Greg Mann and Joseph Wu.


Changes
---

Display generic "agent state" in `agents` screen instead of just drain state


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


Repository: mesos


Description
---

Exposed agent drain information in the webui.


Diffs (updated)
-

  src/webui/app/agents/agent.html 6d50bfd320973687dcba0aaf2a2aa5d843679ea5 
  src/webui/app/agents/agents.html 98712c619fb58acf06b465b53d4c9dee85f8841d 
  src/webui/app/controllers.js 66cd32e666ba490b1194fc8b52bf2db199efdbc9 


Diff: https://reviews.apache.org/r/71081/diff/2/

Changes: https://reviews.apache.org/r/71081/diff/1-2/


Testing
---


File Attachments (updated)


agents summary
  
https://reviews.apache.org/media/uploaded/files/2019/07/16/f21569fa-fdb5-4563-ac5d-2d7392bf3914__Screen_Shot_2019-07-16_at_2.03.43_PM.png
agent detail
  
https://reviews.apache.org/media/uploaded/files/2019/07/16/ae8bf9c8-584c-4afe-acbd-650258223933__Screen_Shot_2019-07-16_at_2.03.07_PM.png
agents summary
  
https://reviews.apache.org/media/uploaded/files/2019/07/17/2baa2933-1e98-47f2-bdd5-1cb3a08817f2__Screen_Shot_2019-07-17_at_10.28.37_PM.png


Thanks,

Benjamin Bannier

Re: Review Request 70757: Added a NNP isolator.

[Jacob Janco]

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

(Updated July 17, 2019, 7:49 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Changes
---

Opening docs review in a separate patch, for some reason that is dropping the 
previous commits when posting.


Summary (updated)
-

Added a NNP isolator.


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


Repository: mesos


Description (updated)
---

This patch adds the isolation capability of flipping the
NO_NEW_PRIVILEGES bit for process control.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
2d04f3c182a4274dda527a3da56c894c3c892a12 
  src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 
  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/slave/containerizer/mesos/containerizer.cpp 
c9a369bcdbfc1676912430c3e84fa567bbd7f766 
  src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp 
0419e8e0fbf154396ab5fb5026d77b94cc9fca5b 
  src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db 
  src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/70757/diff/13/

Changes: https://reviews.apache.org/r/70757/diff/12-13/


Testing
---


Thanks,

Jacob Janco

Re: Review Request 71086: Fixed an issue where master quota state is not updated.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71086]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On July 16, 2019, 2:22 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71086/
> ---
> 
> (Updated July 16, 2019, 2:22 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also added a regression test.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 13789d8461fcf029837b15b567ac1b7b97677607 
>   src/tests/master_quota_tests.cpp a1b37693b74778ff6827c9325f1f9225ecf73e58 
> 
> 
> Diff: https://reviews.apache.org/r/71086/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 70757: Added docs for the NNP isolator.

[Jacob Janco]

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

(Updated July 17, 2019, 7:34 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Changes
---

Fixed branch.


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


Repository: mesos


Description
---

Added docs for the NNP isolator.


Diffs (updated)
-

  CHANGELOG 164465a71c660ab9f01fb18d43076afc4b892ad5 
  docs/isolators/linux-nnp.md PRE-CREATION 
  docs/mesos-containerizer.md e79976111ec8e9cc8e8d44b5f1b8d6e2c7e072d6 


Diff: https://reviews.apache.org/r/70757/diff/12/

Changes: https://reviews.apache.org/r/70757/diff/11-12/


Testing
---


Thanks,

Jacob Janco

Re: Review Request 70757: Added docs for the NNP isolator.

[Jacob Janco]

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

(Updated July 17, 2019, 7:31 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


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


Repository: mesos


Description
---

Added docs for the NNP isolator.


Diffs (updated)
-

  CHANGELOG 164465a71c660ab9f01fb18d43076afc4b892ad5 
  docs/isolators/linux-nnp.md PRE-CREATION 
  docs/mesos-containerizer.md e79976111ec8e9cc8e8d44b5f1b8d6e2c7e072d6 


Diff: https://reviews.apache.org/r/70757/diff/11/

Changes: https://reviews.apache.org/r/70757/diff/10-11/


Testing
---


Thanks,

Jacob Janco

Re: Review Request 70757: Added docs for the NNP isolator.

[Jacob Janco]

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

(Updated July 17, 2019, 7:27 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


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


Repository: mesos


Description
---

Added docs for the NNP isolator.


Diffs (updated)
-

  CHANGELOG 164465a71c660ab9f01fb18d43076afc4b892ad5 
  docs/isolators/linux-nnp.md PRE-CREATION 
  docs/mesos-containerizer.md e79976111ec8e9cc8e8d44b5f1b8d6e2c7e072d6 


Diff: https://reviews.apache.org/r/70757/diff/10/

Changes: https://reviews.apache.org/r/70757/diff/9-10/


Testing
---


Thanks,

Jacob Janco

Re: Review Request 70757: Added docs for the NNP isolator.

[Jacob Janco]

> On May 31, 2019, 10:31 a.m., Andrei Budnik wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Lines 1098 (patched)
> > 
> >
> > Can this condition ever be `false`, given the isolator assigns `true` 
> > in any case?
> > 
> > What is the semantics of `ContainerLaunchInfo::no_new_privileges` flag? 
> > How does it work in pair with the new isolator?
> 
> Jacob Janco wrote:
> I don't have a default value set in the proto so I think checking here is 
> sane. The user can set this to false/true explicitly.

Dropping this, feel free to reopen.


- Jacob


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


On July 17, 2019, 7:19 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70757/
> ---
> 
> (Updated July 17, 2019, 7:19 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James 
> Peach.
> 
> 
> Bugs: MESOS-9770
> https://issues.apache.org/jira/browse/MESOS-9770
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added docs for the NNP isolator.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 164465a71c660ab9f01fb18d43076afc4b892ad5 
>   docs/isolators/linux-nnp.md PRE-CREATION 
>   docs/mesos-containerizer.md e79976111ec8e9cc8e8d44b5f1b8d6e2c7e072d6 
> 
> 
> Diff: https://reviews.apache.org/r/70757/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 70757: Added docs for the NNP isolator.

[Jacob Janco]

> On July 12, 2019, 5:46 p.m., Andrei Budnik wrote:
> > src/slave/containerizer/mesos/isolators/linux/nnp.cpp
> > Lines 71 (patched)
> > 
> >
> > What happens if a framework explicitly set `no_new_privileges` flag to 
> > `false` in the `ContainerLaunchInfo`? Does the isolator handle such case?
> 
> James Peach wrote:
> In that case, the containerizer would do nothing (i.e. default NNP 
> status). This would have the same end result, but I agree that it's worth 
> being explicit here.
> 
> Andrei Budnik wrote:
> At this point, the NNP isolator does not support overriding of a NNP bit 
> by a framework?
> 
> Here is an example of how `linux/seccomp` isolator handles `seccomp` flag 
> provided by a framework: 
> https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/isolators/linux/seccomp.cpp#L98-L103

Dropping this issue from chat on slack: 
jpeach: 
We need the operator to take some action to enable  NNP, we can't just turn it 
on since that might break things. The way  I had been thinking about this was 
that the action would be enabling the NNP isolator. However, when we make it 
configurable  by frameworks, that action is no longer definitive (i.e. there's 
no way to be explicit about what you want the default to be for frameworks that 
don't set the field). This is why I was suggesting that the seccomp isolator 
deal with  the configurable  part, since NNP is usually associated with seccomp 
as well.


jpeach  [19 hours ago]
However, if we follow this reasoning, it takes us back to our original aim for 
the NNP isolator, which was to unconditionally set the NNP flag. Taking this 
approach, we can add the configurable parts later.


- Jacob


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


On July 17, 2019, 7:19 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70757/
> ---
> 
> (Updated July 17, 2019, 7:19 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James 
> Peach.
> 
> 
> Bugs: MESOS-9770
> https://issues.apache.org/jira/browse/MESOS-9770
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added docs for the NNP isolator.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 164465a71c660ab9f01fb18d43076afc4b892ad5 
>   docs/isolators/linux-nnp.md PRE-CREATION 
>   docs/mesos-containerizer.md e79976111ec8e9cc8e8d44b5f1b8d6e2c7e072d6 
> 
> 
> Diff: https://reviews.apache.org/r/70757/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 70757: Added docs for the NNP isolator.

[Jacob Janco]

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

(Updated July 17, 2019, 7:19 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Changes
---

Added docs and fixed patch series I think.


Summary (updated)
-

Added docs for the NNP isolator.


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


Repository: mesos


Description (updated)
---

Added docs for the NNP isolator.


Diffs (updated)
-

  CHANGELOG 164465a71c660ab9f01fb18d43076afc4b892ad5 
  docs/isolators/linux-nnp.md PRE-CREATION 
  docs/mesos-containerizer.md e79976111ec8e9cc8e8d44b5f1b8d6e2c7e072d6 


Diff: https://reviews.apache.org/r/70757/diff/9/

Changes: https://reviews.apache.org/r/70757/diff/8-9/


Testing
---


Thanks,

Jacob Janco

Re: Review Request 71080: Master should store the list of completed framework ids for lifecycle.

[Xudong Ni via Review Board]

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

(Updated July 17, 2019, 6:58 p.m.)


Review request for mesos, James Peach and Jiang Yan Xu.


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


Repository: mesos


Description
---

It should be separately from that for webUI and endpoints.
Currently the master stores the history of completed frameworks in
a map with the full historical data of the framework, it is
prohibitively expensive to keep a long history; In order to reject
frameworks from reregistering if they have previously marked as
completed, we only need to persist the framework ids and are able
to keep long history.


Diffs (updated)
-

  docs/configuration/master.md c56ac8510ea968f9587e23e81ed310caa968ee9e 
  docs/operator-http-api.md 2d4a9b66e20cf19eceec718b7de3d812ab285772 
  include/mesos/allocator/allocator.hpp 
2bab53ab5fb25931a724c20a039e1301983ba574 
  src/master/flags.hpp 78623d68bf428cd3f52684303d98a525d42eb878 
  src/master/flags.cpp 74f4daadd48e8e691be43759b88dc8b3c2df489a 
  src/master/http.cpp 765d5052ab6a8d731113f8922d20fb280b843003 
  src/master/master.hpp ffa7423ba533725f7c1123d9aa507b1348e7f281 
  src/master/master.cpp f1ca637b4cb0382caec53b5a81f6a4eb46f4dd2d 


Diff: https://reviews.apache.org/r/71080/diff/2/

Changes: https://reviews.apache.org/r/71080/diff/1-2/


Testing
---

make check


Thanks,

Xudong Ni

Re: Review Request 71082: Added a helper to create std::vector from iterable into python bindings.

[Andrei Sekretenko]

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




src/python/native_common/ext_modules.py.in
Lines 125 (patched)


This is not portable, and might cause incorrect `.egg` to be built (see 
broken CI) - I'll need to fix it.


- Andrei Sekretenko


On July 16, 2019, 3:11 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71082/
> ---
> 
> (Updated July 16, 2019, 3:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9864
> https://issues.apache.org/jira/browse/MESOS-9864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a helper to create std::vector from iterable into python bindings.
> 
> 
> Diffs
> -
> 
>   src/python/native_common/common.hpp 
> ed743aab014301981fe2526a7e81f4a35c02e19b 
>   src/python/native_common/common.cpp PRE-CREATION 
>   src/python/native_common/ext_modules.py.in 
> 38e6717ec3b7bb09cfbb9b2fa25f173a17d78f51 
> 
> 
> Diff: https://reviews.apache.org/r/71082/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 71086: Fixed an issue where master quota state is not updated.

[Benjamin Mahler]

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


Ship it!





src/tests/master_quota_tests.cpp
Lines 265-266 (patched)


Maybe wrap the first << on the next line?


- Benjamin Mahler


On July 16, 2019, 9:22 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71086/
> ---
> 
> (Updated July 16, 2019, 9:22 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also added a regression test.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 13789d8461fcf029837b15b567ac1b7b97677607 
>   src/tests/master_quota_tests.cpp a1b37693b74778ff6827c9325f1f9225ecf73e58 
> 
> 
> Diff: https://reviews.apache.org/r/71086/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71065: Implemented `FutureTrackTest` tests.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [70887, 70888, 70889, 70890, 70891, 70892, 71065]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On July 12, 2019, 9:03 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71065/
> ---
> 
> (Updated July 12, 2019, 9:03 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Qian Zhang.
> 
> 
> Bugs: MESOS-9842
> https://issues.apache.org/jira/browse/MESOS-9842
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests verify functionality provided by the
> `PendingFutureTracker` class.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
>   src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db 
>   src/tests/common/future_tracker_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71065/diff/3/
> 
> 
> Testing
> ---
> 
> `src/mesos-tests --gtest_repeat=100 --gtest_filter=FutureTrackTest.*`
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 70887: Added `PendingFutureTracker` class for tracking pending futures.

[Andrei Budnik]

> On Июль 17, 2019, 1:02 д.п., Gilbert Song wrote:
> > src/common/future_tracker.hpp
> > Lines 101-103 (patched)
> > 
> >
> > Please add some comments if possible :)

Added comments.


- Andrei


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


On Июнь 19, 2019, 2:49 п.п., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70887/
> ---
> 
> (Updated Июнь 19, 2019, 2:49 п.п.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gilbert Song, James Peach, Meng 
> Zhu, and Qian Zhang.
> 
> 
> Bugs: MESOS-9837
> https://issues.apache.org/jira/browse/MESOS-9837
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch introduces a mechanism for tracking pending futures.
> This feature allows detection of hanging operations, which get
> stuck on a blocking operation or asynchronously. However, this
> feature does not provide any mechanism for tracking pending
> promises, because `Promise` objects might not be accessible in
> various cases. Thereby, we introduce a new class that can be
> used to track pending futures, so it might facilitate debugging
> of stuck issues.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
>   src/common/future_tracker.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70887/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 71092: Added additional synchronization to a previously flaky test.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71092]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On July 17, 2019, 12:02 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71092/
> ---
> 
> (Updated July 17, 2019, 12:02 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Greg Mann, and Joseph Wu.
> 
> 
> Bugs: MESOS-9895
> https://issues.apache.org/jira/browse/MESOS-9895
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test did not make sure that the agent's task status update manager
> had received a task status update acknowledgemtn before launching a
> subsequent task. The agent then, given its state, correctly rejected the
> task.
> 
> This patch adds additional synchronization so the agent has finished
> processing the task status update like expected.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 1ed59ca5af69a6b53ceb8be035e608286299c2f1 
> 
> 
> Diff: https://reviews.apache.org/r/71092/diff/1/
> 
> 
> Testing
> ---
> 
> This test previously failed about 4% of the time in ~400 iterations (~20 
> failures) under a lot of additional system stress. With this patch it did not 
> fail once in 500 iterations.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71092: Added additionaly synchronization to a previously flaky test.

[Benno Evers]

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


Ship it!




Ship It!

- Benno Evers


On July 17, 2019, 12:02 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71092/
> ---
> 
> (Updated July 17, 2019, 12:02 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Greg Mann, and Joseph Wu.
> 
> 
> Bugs: MESOS-9895
> https://issues.apache.org/jira/browse/MESOS-9895
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test did not make sure that the agent's task status update manager
> had received a task status update acknowledgemtn before launching a
> subsequent task. The agent then, given its state, correctly rejected the
> task.
> 
> This patch adds additional synchronization so the agent has finished
> processing the task status update like expected.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 1ed59ca5af69a6b53ceb8be035e608286299c2f1 
> 
> 
> Diff: https://reviews.apache.org/r/71092/diff/1/
> 
> 
> Testing
> ---
> 
> This test previously failed about 4% of the time in ~400 iterations (~20 
> failures) under a lot of additional system stress. With this patch it did not 
> fail once in 500 iterations.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 71092: Added additionaly synchronization to a previously flaky test.

[Benjamin Bannier]

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

Review request for mesos, Benno Evers, Greg Mann, and Joseph Wu.


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


Repository: mesos


Description
---

This test did not make sure that the agent's task status update manager
had received a task status update acknowledgemtn before launching a
subsequent task. The agent then, given its state, correctly rejected the
task.

This patch adds additional synchronization so the agent has finished
processing the task status update like expected.


Diffs
-

  src/tests/slave_tests.cpp 1ed59ca5af69a6b53ceb8be035e608286299c2f1 


Diff: https://reviews.apache.org/r/71092/diff/1/


Testing
---

This test previously failed about 4% of the time in ~400 iterations (~20 
failures) under a lot of additional system stress. With this patch it did not 
fail once in 500 iterations.


Thanks,

Benjamin Bannier

Re: Review Request 70891: Wrapped launcher in `LauncherTracker`.

[Andrei Budnik]

> On Июль 17, 2019, 7:08 д.п., Gilbert Song wrote:
> > src/slave/containerizer/mesos/containerizer.cpp
> > Lines 350 (patched)
> > 
> >
> > isn't this always true?

`futureTracker` can be a `nullptr` in tests.


- Andrei


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


On Июнь 19, 2019, 2:49 п.п., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70891/
> ---
> 
> (Updated Июнь 19, 2019, 2:49 п.п.)
> 
> 
> Review request for mesos, Gilbert Song and Qian Zhang.
> 
> 
> Bugs: MESOS-9841
> https://issues.apache.org/jira/browse/MESOS-9841
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch wraps a container launcher in instance of `LauncherTracker`
> class. If the launcher gets stuck in some operation, `pendingFutures`
> will return the method name along with its arguments such as
> `containerId`, `pid`, etc.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> c9a369bcdbfc1676912430c3e84fa567bbd7f766 
> 
> 
> Diff: https://reviews.apache.org/r/70891/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 70888: Added `IsolatorTracker` for tracking calls of isolator methods.

[Andrei Budnik]

> On Июль 16, 2019, 7:23 д.п., Gilbert Song wrote:
> > src/slave/constants.hpp
> > Lines 205-207 (patched)
> > 
> >
> > are you using `COMPONENT_NAME_` prefix because `MESOS_CONTAINERIZER` is 
> > already defined in containerizer/mesos/constant.hpp?
> > 
> > probably let's figure out what `string component` stand for first. 
> > Then, we could better define the constant, and I would probably suggest to 
> > have the constant in containerizer/mesos/constant.hpp because we may have 
> > docker container stuck issue and if the docker container use this tracker 
> > process we need to distinguish two containerizers.

`/containerizer/endpoint` exposes information about both Docker and Mesos 
containerizers, so they belong to the same component.

`PendingFutureTracker` is a common code which is independent from the Mesos 
containerizer.


> On Июль 16, 2019, 7:23 д.п., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolator_tracker.hpp
> > Lines 71 (patched)
> > 
> >
> > could this be a Owned pointer?

Yes, but why? Pointer seems consistent with other subsystems, see 
https://github.com/apache/mesos/blob/633b1b5d4ef3f08b0abf057a3651f34116e1e45f/src/slave/containerizer/containerizer.hpp#L75-L78


- Andrei


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


On Июнь 19, 2019, 2:49 п.п., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70888/
> ---
> 
> (Updated Июнь 19, 2019, 2:49 п.п.)
> 
> 
> Review request for mesos, Gilbert Song and Qian Zhang.
> 
> 
> Bugs: MESOS-9839
> https://issues.apache.org/jira/browse/MESOS-9839
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a new `IsolatorTracker` class that proxies
> calls to the real isolator and keeps track of returned futures
> by employing `PendingFutureTracker` class.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 
>   src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
>   src/slave/constants.hpp cc81e5111e8c3082c940b918230f660a1121a1ac 
>   src/slave/containerizer/mesos/isolator_tracker.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolator_tracker.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70888/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 71065: Implemented `FutureTrackTest` tests.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 12, 2019, 9:03 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71065/
> ---
> 
> (Updated July 12, 2019, 9:03 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Qian Zhang.
> 
> 
> Bugs: MESOS-9842
> https://issues.apache.org/jira/browse/MESOS-9842
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests verify functionality provided by the
> `PendingFutureTracker` class.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
>   src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db 
>   src/tests/common/future_tracker_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71065/diff/3/
> 
> 
> Testing
> ---
> 
> `src/mesos-tests --gtest_repeat=100 --gtest_filter=FutureTrackTest.*`
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 70892: Added `/containerizer/debug` endpoint.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On June 19, 2019, 7:49 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70892/
> ---
> 
> (Updated June 19, 2019, 7:49 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9829
> https://issues.apache.org/jira/browse/MESOS-9829
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch introduces an agent's `/containerizer/debug` endpoint,
> which exposes the debug info related to Mesos containerizer.
> Currently, this endpoint returns a list of pending futures related to
> isolators or containerizer launcher. This endpoint is experimental,
> and the format of its output may change over time.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md 91dc03be66bdcf135610bff50e5abcfe8d95c5db 
>   src/common/authorization.cpp 89487219ef87619084663e1a0a17e75924394d38 
>   src/local/local.cpp 6ac6b027f66ef555440ca86803198bfe3227f8d7 
>   src/slave/http.hpp b8c83f1db95c2175bb94f6cd76cc2c58aa118c4e 
>   src/slave/http.cpp 321dca786f106169f88df086422f5ef817d2f098 
>   src/slave/main.cpp ef5ea02f169427d9913b807664bf3073d72cd9b6 
>   src/slave/slave.hpp 58a5608ceb53d7f75a085f1d3a4f757e8c3ddd3b 
>   src/slave/slave.cpp 247797543d9608fb212b5f09865f744052a460e3 
>   src/tests/cluster.cpp b43f806eab096b7d4e86e2b5d4b81d6baecb0ee5 
>   src/tests/mock_slave.hpp c057b40d6db433f52d1dfe10ae02a3b0bc936564 
>   src/tests/mock_slave.cpp dd458e3d09e212d6cde514d86989878954f79fc1 
> 
> 
> Diff: https://reviews.apache.org/r/70892/diff/4/
> 
> 
> Testing
> ---
> 
> Manual testing.
> 1. Add `::sleep(5)` to the beginning of the 
> `LinuxSeccompIsolatorProcess::prepare` method.
> 2. Rebuild Mesos and then spin up a local Mesos cluster.
> 3. Run simple task, e.g.:
> ```
> ./src/mesos-execute --master="`hostname`:5050" --name="test" 
> --containerizer=mesos --command="sleep 1"
> ```
> 4. In the parallel terminal session:
> ```
> curl `hostname`:5051/containerizer/debug
> # will return: 
> {"pending":[["linux/seccomp::prepare",{"containerId":""}]]}
> ```
> 5. After 5 seconds this endpoint returns an empty list of pending futures: 
> `{"pending":[]}`
> 
> Unit tests: TBD in the following patches.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 70891: Wrapped launcher in `LauncherTracker`.

[Gilbert Song]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/containerizer.cpp
Lines 350 (patched)


isn't this always true?


- Gilbert Song


On June 19, 2019, 7:49 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70891/
> ---
> 
> (Updated June 19, 2019, 7:49 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Qian Zhang.
> 
> 
> Bugs: MESOS-9841
> https://issues.apache.org/jira/browse/MESOS-9841
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch wraps a container launcher in instance of `LauncherTracker`
> class. If the launcher gets stuck in some operation, `pendingFutures`
> will return the method name along with its arguments such as
> `containerId`, `pid`, etc.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> c9a369bcdbfc1676912430c3e84fa567bbd7f766 
> 
> 
> Diff: https://reviews.apache.org/r/70891/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 70890: Added `LauncherTracker` for tracking calls of launcher methods.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On June 19, 2019, 7:49 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70890/
> ---
> 
> (Updated June 19, 2019, 7:49 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Qian Zhang.
> 
> 
> Bugs: MESOS-9840
> https://issues.apache.org/jira/browse/MESOS-9840
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a new `LauncherTracker` class that proxies
> calls to the real `Launcher` and keeps track of returned futures
> by employing `PendingFutureTracker` class.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 
>   src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
>   src/slave/containerizer/mesos/launcher_tracker.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/launcher_tracker.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70890/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 70889: Wrapped isolators in `IsolatorTracker`.

[Gilbert Song]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/containerizer.cpp
Lines 550 (patched)


could we be explicit here?

if (futuretracker != nullptr)



src/slave/containerizer/mesos/containerizer.cpp
Lines 570 (patched)


ditto


- Gilbert Song


On June 19, 2019, 7:49 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70889/
> ---
> 
> (Updated June 19, 2019, 7:49 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Qian Zhang.
> 
> 
> Bugs: MESOS-9841
> https://issues.apache.org/jira/browse/MESOS-9841
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch wraps every isolator in instance of `IsolatorTracker` class.
> If an isolator gets stuck in some operation, `pendingFutures` will
> return the isolator name, method name along with its arguments such as
> `containerId`, `pid`, etc.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.hpp 
> d33c65cd3fae9bb4d347681cb00dbdb25ecf57b8 
>   src/slave/containerizer/containerizer.cpp 
> 5ce0d9c4bfa707495ea644c90fa6e35eb0fe25cd 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 558e412b28a84ae0ca1bf05b596540072bb216d6 
>   src/slave/containerizer/mesos/containerizer.cpp 
> c9a369bcdbfc1676912430c3e84fa567bbd7f766 
>   src/slave/main.cpp ef5ea02f169427d9913b807664bf3073d72cd9b6 
>   src/tests/cluster.hpp c04ee14beafe350568a295c8258566aa3704028a 
>   src/tests/cluster.cpp b43f806eab096b7d4e86e2b5d4b81d6baecb0ee5 
> 
> 
> Diff: https://reviews.apache.org/r/70889/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 70888: Added `IsolatorTracker` for tracking calls of isolator methods.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On June 19, 2019, 7:49 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70888/
> ---
> 
> (Updated June 19, 2019, 7:49 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Qian Zhang.
> 
> 
> Bugs: MESOS-9839
> https://issues.apache.org/jira/browse/MESOS-9839
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a new `IsolatorTracker` class that proxies
> calls to the real isolator and keeps track of returned futures
> by employing `PendingFutureTracker` class.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 
>   src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
>   src/slave/constants.hpp cc81e5111e8c3082c940b918230f660a1121a1ac 
>   src/slave/containerizer/mesos/isolator_tracker.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolator_tracker.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70888/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>