Re: Review Request 71347: Optimized shrinkResources.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71345, 71346, 71347]

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 Aug. 22, 2019, 12:31 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71347/
> ---
> 
> (Updated Aug. 22, 2019, 12:31 a.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Meng Zhu.
> 
> 
> Bugs: MESOS-9806
> https://issues.apache.org/jira/browse/MESOS-9806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There are several optimization used here:
> 
> * Avoid unnecessary copying of Resource objects.
> * Avoid unnecessary validation.
> 
> The shrinkResources function now is a friend of Resources,
> in order to perform an in-place shuffle of the vector.
> 
> Master branch:
> HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 36.1185929secs
> Made 0 allocation in 32.62218602secs
> 
> After all patches in review chain:
> HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 21.389381617secs
> Made 0 allocation in 18.593000222secs
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_quantities.hpp 
> c861df8a8a167b19ea8374c22cdd2a8fe567a6a6 
>   include/mesos/resources.hpp a6a052ba51e13e2804eca846f08605e7f0714cfd 
>   include/mesos/v1/resources.hpp e43d1fba69771405f4575cf675d6f0cd13c2c7c9 
>   src/common/resource_quantities.cpp 9577181bc4c05214759332e41f4a7d8b8fb6db1f 
>   src/common/resources.cpp fc7e86ba5eee3deb953d85065a1192374910c5e5 
>   src/common/resources_utils.cpp 720b954b74a5db72438b1846d7df837d6a1fa4a4 
>   src/v1/resources.cpp 88da0a185bd54e7053a221fe0b3255f3c514ac60 
> 
> 
> Diff: https://reviews.apache.org/r/71347/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 71285: Fixed recovery of agent resources and operations after crash.

[James Peach]

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




src/slave/slave.cpp
Line 7888 (original)


Can you explain this more?

In `Slave::applyOperation()`, there is a call to 
`checkpointResourceState()` before the operation is applied. If the agent 
crashed after writing the operation checkpoint, wouldn't we need to apply it 
here? It's not clear to me how we can tell whether it needs to be applied or 
not ...


- James Peach


On Aug. 21, 2019, 6:29 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71285/
> ---
> 
> (Updated Aug. 21, 2019, 6:29 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, James Peach, and Joseph Wu.
> 
> 
> Bugs: MESOS-9875
> https://issues.apache.org/jira/browse/MESOS-9875
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes an issue where the agent may incorrectly send an
> OPERATION_FINISHED update for a failed offer operation
> following agent failover and recovery.
> 
> The agent previously relied on the difference between the
> set of checkpointed operations and the set of operation
> IDs recovered from the operation status update manager to
> apply any operations which had not been applied due to an
> ill-timed agent failover.
> 
> However, this approach did not work in the case where a
> persistent volume failed to be successfully created by
> syncCheckpointedResources(). In order to handle this
> case, this patch changes the agent code to continue with
> the old approach of a two-phase-commit of persistent
> volumes to disk, where the agent will fail to complete
> recovery if syncCheckpointedResources() cannot be
> executed successfully after failover.
> 
> 
> Diffs
> -
> 
>   src/slave/paths.hpp e077587fd02bd8e35fee7ce12ae436e3dca25e47 
>   src/slave/paths.cpp 28a7cf9f9c70fb31eeefe2e823cd7e19ffcf126a 
>   src/slave/slave.cpp 1d0ec9d2428c3ffa28ad3e960b74f171013cf0c2 
>   src/slave/state.cpp cd3fac72dd57da21ed5ac46b17066531af26d42a 
> 
> 
> Diff: https://reviews.apache.org/r/71285/diff/2/
> 
> 
> Testing
> ---
> 
> Tested manually by doing the following:
> 
> 1) Run a master
> 2) Run an agent with statically reserved resources for use in the following 
> step
> 3) Run the persistent volume example framework to create a volume on the 
> agent, which leads to checkpointing of resources
> 4) Use `chattr -R +i /agent/volumes/dir` to make the agent's persistent 
> volume directory immutable
> 5) Run the persistent volume example framework again; it will fail and the 
> agent will crash
> 6) Restart the agent; confirm that it continues to crash
> 7) Use `chattr -R -i /agent/volumes/dir` to remove the immutable attribute
> 8) Restart the agent; confirm that it recovers successfully, with the 
> persistent volume created on disk
> 9) Run the persistent volume example framework again; it should succeed
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 71174: Recovered network info for nested/standalone containers in CNI isolator.

[Qian Zhang]

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

(Updated Aug. 22, 2019, 9:19 a.m.)


Review request for mesos, Andrei Budnik and Gilbert Song.


Changes
---

Rebased


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


Repository: mesos


Description
---

Recovered network info for nested/standalone containers in CNI isolator.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
3e5b66702be5c042ef6c7c580cbcbf6afd3167f0 
  src/common/protobuf_utils.hpp 5d6a35d6e3bae35b83e87827724206f7c5dfb2d8 
  src/common/protobuf_utils.cpp 64777c0e2f28aba6cd34cbdd92b914485f8d73e7 
  src/slave/containerizer/mesos/containerizer.cpp 
9030af1463d7de7f339e1b39f0c4eeb24c5d3105 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
8d037334a8e449408918c584eade9ac6bc93337c 


Diff: https://reviews.apache.org/r/71174/diff/3/

Changes: https://reviews.apache.org/r/71174/diff/2-3/


Testing
---

sudo make check


Thanks,

Qian Zhang

Re: Review Request 71299: Added separate script to install developer setup.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On Aug. 20, 2019, 11:48 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71299/
> ---
> 
> (Updated Aug. 20, 2019, 11:48 a.m.)
> 
> 
> Review request for mesos, Benno Evers and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch breaks the installation of developer tools (i.e., linter
> configuration files and git hooks) out of `./bootstrap`. This not only
> simplifies and streamlines the setup, but will allow us to add
> developer-only features without breaking users who are just interested
> in building a distribution tarball.
> 
> 
> Diffs
> -
> 
>   bootstrap 7be2cc95c7ace55d710315427f38284cc5b2af4c 
>   bootstrap.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4 
>   docs/advanced-contribution.md 573138d3642e901f46ee35db58299627f188b94c 
>   docs/beginner-contribution.md 471f5dd0da30d921cc3b29484d2b02f164f8ca75 
>   docs/clang-format.md 4289813c2ca78e1b963aa22a596abd6e8aa3a28b 
>   support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b 
>   support/llvm/README.md 188930224ba8ff53dbef8770bc56625be936bb76 
>   support/setup-dev.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71299/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71303: Tracked frameworks in the role sorter.

[Benjamin Mahler]

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



Won't we need the allocated amount for each framework (leaf node) in the DRF 
case?

- Benjamin Mahler


On Aug. 17, 2019, midnight, Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71303/
> ---
> 
> (Updated Aug. 17, 2019, midnight)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9917
> https://issues.apache.org/jira/browse/MESOS-9917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This paves the way for removing the framework sorters.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
>   src/master/allocator/mesos/sorter/drf/sorter.hpp 
> 537086039fd804453ea8c682cda775d8fdff038f 
>   src/master/allocator/mesos/sorter/drf/sorter.cpp 
> 09889cdf3dc8c0e773b8e2e24154fb0edd2cc254 
>   src/master/allocator/mesos/sorter/random/sorter.hpp 
> f18b014ed15ff8906fbdbd3248becefde896651c 
>   src/master/allocator/mesos/sorter/random/sorter.cpp 
> 60a5797472460a8d3d9be938af9f6711ea51d484 
>   src/master/allocator/mesos/sorter/sorter.hpp 
> 52b8a7b57bf17759311b32aa56c26e614119b773 
> 
> 
> Diff: https://reviews.apache.org/r/71303/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71333: Avoided copying the clientPath in the sorter.

[Benjamin Mahler]

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


Ship it!




Should probably clarify that this doesn't eliminate a copy during sort? Only 
during client add / remove?

- Benjamin Mahler


On Aug. 21, 2019, 12:59 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71333/
> ---
> 
> (Updated Aug. 21, 2019, 12:59 a.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided copying the clientPath in the sorter.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/sorter/drf/sorter.hpp 
> 537086039fd804453ea8c682cda775d8fdff038f 
>   src/master/allocator/mesos/sorter/random/sorter.hpp 
> f18b014ed15ff8906fbdbd3248becefde896651c 
> 
> 
> Diff: https://reviews.apache.org/r/71333/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71343: Fixed out-of-order processing of terminal status updates in agent.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [71343]

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_71343"]

Error:
..
18624 http.cpp:263] Processing call DESTROY_VOLUMES
I0822 00:42:32.563313 18624 master.cpp:3998] Authorizing principal 
'test-principal' to destroy volumes 
'[{"disk":{"persistence":{"id":"cb13a84f-1888-4f0b-9042-7eb5a3f9632d","principal":"test-principal"},"source":{"id":"/tmp/CSIVersion_StorageLocalResourceProviderTest_OperatorOperationsWithResourceProviderResources_v1_ZX1WjK/2GB-c2382977-f0bf-49c9-9408-fca4f4959fbf","mount":{"root":"./csi/org.apache.mesos.csi.test/local/mounts"},"profile":"test","type":"MOUNT","vendor":"org.apache.mesos.csi.test.local"},"volume":{"container_path":"volume","mode":"RW"}},"name":"disk","provider_id":{"value":"2acdbcf4-880f-4035-9068-d76c90f7379f"},"reservations":[{"role":"storage","type":"DYNAMIC"},{"principal":"test-principal","role":"storage/default-role","type":"DYNAMIC"}],"scalar":{"value":2048.0},"type":"SCALAR"}]'
I0822 00:42:32.567185 18631 master.cpp:12724] Removing offer 
d42d1569-3454-4001-b72e-5dee6d8119c4-O4
I0822 00:42:32.569187 18631 sched.cpp:960] Rescinded offer 
d42d1569-3454-4001-b72e-5dee6d8119c4-O4
I0822 00:42:32.569320 18631 sched.cpp:971] Scheduler::offerRescinded took 
36302ns
I0822 00:42:32.570363 18628 hierarchical.cpp:1454] Recovered disk(allocated: 
storage/default-role)(reservations: 
[(DYNAMIC,storage),(DYNAMIC,storage/default-role,test-principal)])[MOUNT(org.apache.mesos.csi.test.local,/tmp/CSIVersion_StorageLocalResourceProviderTest_OperatorOperationsWithResourceProviderResources_v1_ZX1WjK/2GB-c2382977-f0bf-49c9-9408-fca4f4959fbf,test),cb13a84f-1888-4f0b-9042-7eb5a3f9632d:volume]:2048;
 cpus(allocated: storage/default-role):2; mem(allocated: 
storage/default-role):1024; disk(allocated: storage/default-role):1024; 
ports(allocated: storage/default-role):[31000-32000] (total: cpus:2; mem:1024; 
disk:1024; ports:[31000-32000]; disk(reservations: 
[(DYNAMIC,storage),(DYNAMIC,storage/default-role,test-principal)])[MOUNT(org.apache.mesos.csi.test.local,/tmp/CSIVersion_StorageLocalResourceProviderTest_OperatorOperationsWithResourceProviderResources_v1_ZX1WjK/2GB-c2382977-f0bf-49c9-9408-fca4f4959fbf,test),cb13a84f-1888-4f0b-9042-7eb5a3f9632d:volume]:2048,
 allocat
 ed: {}) on agent d42d1569-3454-4001-b72e-5dee6d8119c4-S0 from framework 
d42d1569-3454-4001-b72e-5dee6d8119c4-
I0822 00:42:32.570607 18628 hierarchical.cpp:1500] Framework 
d42d1569-3454-4001-b72e-5dee6d8119c4- filtered agent 
d42d1569-3454-4001-b72e-5dee6d8119c4-S0 for 5secs
I0822 00:42:32.575932 18629 master.cpp:12615] Sending operation '' (uuid: 
74601dd1-b000-4b50-b647-26ed9934d706) to agent 
d42d1569-3454-4001-b72e-5dee6d8119c4-S0 at slave(1225)@172.17.0.4:35575 
(8c6ce722e607)
I0822 00:42:32.577000 18629 slave.cpp:4352] Ignoring new checkpointed resources 
and operations identical to the current version
I0822 00:42:32.587234 18629 hierarchical.cpp:1740] Performed allocation for 1 
agents in 2.050864ms
I0822 00:42:32.588766 18610 master.cpp:10432] Sending offers [ 
d42d1569-3454-4001-b72e-5dee6d8119c4-O5 ] to framework 
d42d1569-3454-4001-b72e-5dee6d8119c4- (default) at 
scheduler-0abc1be1-835e-4f52-8cef-a6e0fb673272@172.17.0.4:35575
I0822 00:42:32.589761 18610 sched.cpp:934] Scheduler::resourceOffers took 
110908ns
I0822 00:42:32.591498 18620 provider.cpp:498] Received APPLY_OPERATION event
I0822 00:42:32.591593 18620 provider.cpp:1329] Received DESTROY operation '' 
(uuid: 74601dd1-b000-4b50-b647-26ed9934d706)
I0822 00:42:32.692255 18620 status_update_manager_process.hpp:152] Received 
operation status update OPERATION_FINISHED (Status UUID: 
f4365977-c3f4-4564-b94c-4c3cf5fef091) for operation UUID 
74601dd1-b000-4b50-b647-26ed9934d706 on agent 
d42d1569-3454-4001-b72e-5dee6d8119c4-S0
I0822 00:42:32.692404 18620 status_update_manager_process.hpp:414] Creating 
operation status update stream 74601dd1-b000-4b50-b647-26ed9934d706 
checkpoint=true
I0822 00:42:32.693342 18620 status_update_manager_process.hpp:929] 
Checkpointing UPDATE for operation status update OPERATION_FINISHED (Status 
UUID: f4365977-c3f4-4564-b94c-4c3cf5fef091) for operation UUID 
74601dd1-b000-4b50-b647-26ed9934d706 on agent 
d42d1569-3454-4001-b72e-5dee6d8119c4-S0
I0822 00:42:32.742231 18620 status_update_manager_process.hpp:528] Forwarding 
operation status update OPERATION_FINISHED (Status UUID: 
f4365977-c3f4-4564-b94c-4c3cf5fef091) for operation UUID 
74601dd1-b000-4b50-b647-26ed9934d706 on agent 

Review Request 71347: Optimized shrinkResources.

[Benjamin Mahler]

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

Review request for mesos, Andrei Sekretenko and Meng Zhu.


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


Repository: mesos


Description
---

There are several optimization used here:

* Avoid unnecessary copying of Resource objects.
* Avoid unnecessary validation.

The shrinkResources function now is a friend of Resources,
in order to perform an in-place shuffle of the vector.

Master branch:
HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
Made 3500 allocations in 36.1185929secs
Made 0 allocation in 32.62218602secs

After all patches in review chain:
HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
Made 3500 allocations in 21.389381617secs
Made 0 allocation in 18.593000222secs


Diffs
-

  include/mesos/resource_quantities.hpp 
c861df8a8a167b19ea8374c22cdd2a8fe567a6a6 
  include/mesos/resources.hpp a6a052ba51e13e2804eca846f08605e7f0714cfd 
  include/mesos/v1/resources.hpp e43d1fba69771405f4575cf675d6f0cd13c2c7c9 
  src/common/resource_quantities.cpp 9577181bc4c05214759332e41f4a7d8b8fb6db1f 
  src/common/resources.cpp fc7e86ba5eee3deb953d85065a1192374910c5e5 
  src/common/resources_utils.cpp 720b954b74a5db72438b1846d7df837d6a1fa4a4 
  src/v1/resources.cpp 88da0a185bd54e7053a221fe0b3255f3c514ac60 


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


Testing
---

make check


Thanks,

Benjamin Mahler

Review Request 71346: Avoid duplicate allocatableTo call in the allocator.

[Benjamin Mahler]

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

Review request for mesos, Andrei Sekretenko and Meng Zhu.


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


Repository: mesos


Description
---

Avoid duplicate allocatableTo call in the allocator.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
d09f10f45b68e591940d748b778fd667b16b94b3 


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


Testing
---

make check


Thanks,

Benjamin Mahler

Review Request 71345: Eliminated double lookups in the allocator.

[Benjamin Mahler]

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

Review request for mesos, Andrei Sekretenko and Meng Zhu.


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


Repository: mesos


Description
---

Eliminated double lookups in the allocator.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
22dd881d5eb85bbfd0e9991cf1282a244932d777 
  src/master/allocator/mesos/hierarchical.cpp 
d09f10f45b68e591940d748b778fd667b16b94b3 


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


Testing
---

make check


Thanks,

Benjamin Mahler

Re: Review Request 71301: Added a framework id field to the allocator Framework struct.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On Aug. 20, 2019, 11:10 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71301/
> ---
> 
> (Updated Aug. 20, 2019, 11:10 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This provides easy lookup.
> 
> Also refactored related functions to take in `Framework&`
> instead of `FrameworkId&`.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 035fd3d7892d4280e299a29f99e8cf6a4c2afb30 
>   src/master/allocator/mesos/hierarchical.cpp 
> b8b9241eae20691ed52c9b5759b17f88a0d8931f 
> 
> 
> Diff: https://reviews.apache.org/r/71301/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71318: Added agent reactivations to the existing agent draining tests.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Aug. 20, 2019, 5:28 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71318/
> ---
> 
> (Updated Aug. 20, 2019, 5:28 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Greg Mann.
> 
> 
> Bugs: MESOS-9892
> https://issues.apache.org/jira/browse/MESOS-9892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds an extra step to a couple of the agent draining tests,
> which calls REACTIVATE_AGENT at the end.
> 
> 
> Diffs
> -
> 
>   src/tests/master_draining_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71318/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 71317: Added draining test for momentarily disconnected agents.

[Greg Mann]

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




src/tests/master_draining_tests.cpp
Lines 313-318 (patched)


Is this used?



src/tests/master_draining_tests.cpp
Lines 324 (patched)


Is the agent unreachable? Can we verify that the agent is in the correct 
state via master API output? Ditto in the second test below.



src/tests/master_draining_tests.cpp
Lines 357-358 (patched)


Can we verify that the agent is in the correct drain state via master API 
output? Here and below.



src/tests/master_draining_tests.cpp
Lines 713-721 (patched)


Could you move this expectation down above the agent restart?


- Greg Mann


On Aug. 19, 2019, 9:58 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71317/
> ---
> 
> (Updated Aug. 19, 2019, 9:58 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Greg Mann.
> 
> 
> Bugs: MESOS-9892
> https://issues.apache.org/jira/browse/MESOS-9892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This exercises the agent draining code when the agent is disconnected
> from the master at the time of starting draining.  Draining is expected
> to proceed once the agent reregisters.
> 
> 
> Diffs
> -
> 
>   src/tests/master_draining_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71317/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 71341: Validated provider ID use in some resource provider calls.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71339, 71340, 71341]

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 Aug. 21, 2019, 6:22 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71341/
> ---
> 
> (Updated Aug. 21, 2019, 6:22 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-9482
> https://issues.apache.org/jira/browse/MESOS-9482
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For some calls we expect resource providers to set provider IDs with the
> calls. While the resource provider manager has always asserted that the
> calls were correct we never validated this.
> 
> With this patch we perform additional validation for calls taking a
> `ResourceProviderInfo` into account. We add both unit tests for the
> validation code and an integration test confirming that the validation
> is actually triggered.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp ceed1225b37d23998f523afcc2184dfaaad60636 
>   src/resource_provider/validation.cpp 
> df55b5efe3543c1dfd8441997302ab76fdd4bcc1 
>   src/tests/resource_provider_manager_tests.cpp 
> bcf6a03aa5d4931feff0299c811faa216efd95b6 
>   src/tests/resource_provider_validation_tests.cpp 
> a9989412ae30bd8244be808fc88fbe70f47d6ad9 
> 
> 
> Diff: https://reviews.apache.org/r/71341/diff/1/
> 
> 
> Testing
> ---
> 
> `ninja check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71316: Added draining tests for empty agents.

[Greg Mann]

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




src/tests/master_draining_tests.cpp
Lines 178 (patched)


In this case, we probably want to either not set the max grace period, or 
set it to something long, since we're trying to verify that an agent with 
nothing running will immediately be marked drained. While the agent won't be 
_immediately_ marked gone if a task is running and the max grace period is set 
to zero, it would still happen very quickly.



src/tests/master_draining_tests.cpp
Lines 222-250 (patched)


We probably don't need to verify the state output of all 3 endpoints in 
both versions of this test, I think just one will do. WDYT?



src/tests/master_draining_tests.cpp
Lines 281 (patched)


This test is so short now :heart_eyes:



src/tests/master_draining_tests.cpp
Lines 313-325 (patched)


Can we do something to verify that the agent has been marked unreachable 
here? Either specifying the type of the registry operation in the EXPECT_CALL, 
or inspecting some API output afterward?



src/tests/master_draining_tests.cpp
Lines 360-361 (patched)


Can we also verify via master API output that the agent is DRAINED after it 
reregisters?


- Greg Mann


On Aug. 19, 2019, 9:57 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71316/
> ---
> 
> (Updated Aug. 19, 2019, 9:57 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Greg Mann.
> 
> 
> Bugs: MESOS-9892
> https://issues.apache.org/jira/browse/MESOS-9892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This splits the existing agent draining tests into two variants:
> 1) where the agent has nothing running, and
> 2) where the agent has one task running.
> 
> 
> Diffs
> -
> 
>   src/tests/master_draining_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71316/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 71310: Stripped metadata and non-scalars from GET_ROLES resources.

[Meng Zhu]

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


Ship it!




I think we should at least try to expose QuotaConfig atm.


src/master/http.cpp
Lines 2725-2726 (patched)


This makes me wonder if we should make `RoleResourceBreakdown` scalar only.


- Meng Zhu


On Aug. 19, 2019, 1:34 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71310/
> ---
> 
> (Updated Aug. 19, 2019, 1:34 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Meng Zhu.
> 
> 
> Bugs: MESOS-9758
> https://issues.apache.org/jira/browse/MESOS-9758
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This endpoint is meant to expose resource statistics, and only
> accidentally exposed the metadata and non-scalar resources. It
> does not make sense to expose non-scalar resources in this way.
> 
> The `resources` field will be deprecated in favor of a breakdown
> between offered, allocated, and reserved resources (similar to
> the /roles endpoint).
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 6400771acd2ad7d8948bad7ff8e1eb7fe4546347 
>   src/master/master.hpp 09a70dfa7ae306b4de4c688e3b4b4576b610e351 
>   src/master/readonly_handler.cpp b226ae1fb1c4ff8ae2cde7e359d4fb1eb5b1b163 
>   src/tests/api_tests.cpp e202cd330d424efef783d39b74db5f856bd34895 
> 
> 
> Diff: https://reviews.apache.org/r/71310/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 71315: Refactored master draining test setup.

[Greg Mann]

> On Aug. 21, 2019, 3:59 p.m., Greg Mann wrote:
> > src/tests/master_draining_tests.cpp
> > Lines 206 (patched)
> > 
> >
> > Should we make this value larger? We advance the clock by the agent 
> > reregistration timeout in these tests, which could be large?
> 
> Joseph Wu wrote:
> I don't think it makes too much of a difference how large or small this 
> value is.  Larger values mean that advancing the clock by this interval may 
> trigger different intervals too.  The tests will need to account for this 
> either way.
> 
> Note: Later in the chain, I add some clock advances by this value, to 
> test offers after reactivating agents.

kk, sg


- Greg


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


On Aug. 19, 2019, 9:55 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71315/
> ---
> 
> (Updated Aug. 19, 2019, 9:55 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Greg Mann.
> 
> 
> Bugs: MESOS-9892
> https://issues.apache.org/jira/browse/MESOS-9892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tests of this feature will generally require a master, agent, framework,
> and a single task to be launched at the beginning of the test.
> This moves this common code into the test SetUp.
> 
> This also changes the `post(...)` helper to return the http::Response
> object instead of parsing it.  The response for DRAIN_AGENT calls
> does not return an object, so the tests were not checking the
> response before.
> 
> 
> Diffs
> -
> 
>   src/tests/master_draining_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71315/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 71315: Refactored master draining test setup.

[Greg Mann]

> On Aug. 21, 2019, 3:59 p.m., Greg Mann wrote:
> > src/tests/master_draining_tests.cpp
> > Lines 249 (patched)
> > 
> >
> > Do we need to parametrize these by content type? I suspect we have 
> > enough coverage of the API's handling of different content types elsewhere.
> 
> Joseph Wu wrote:
> I'd lean towards keeping this parameterization.  The two variants mean 
> we'll be serializing the agent draining protobufs to/from JSON.  Most of 
> these protobufs are not used anywhere else too.

kk, sgtm


- Greg


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


On Aug. 19, 2019, 9:55 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71315/
> ---
> 
> (Updated Aug. 19, 2019, 9:55 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Greg Mann.
> 
> 
> Bugs: MESOS-9892
> https://issues.apache.org/jira/browse/MESOS-9892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tests of this feature will generally require a master, agent, framework,
> and a single task to be launched at the beginning of the test.
> This moves this common code into the test SetUp.
> 
> This also changes the `post(...)` helper to return the http::Response
> object instead of parsing it.  The response for DRAIN_AGENT calls
> does not return an object, so the tests were not checking the
> response before.
> 
> 
> Diffs
> -
> 
>   src/tests/master_draining_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71315/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 71285: Fixed recovery of agent resources and operations after crash.

[Greg Mann]

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

(Updated Aug. 21, 2019, 6:29 p.m.)


Review request for mesos, Gastón Kleiman, James Peach, and Joseph Wu.


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


Repository: mesos


Description (updated)
---

Fixes an issue where the agent may incorrectly send an
OPERATION_FINISHED update for a failed offer operation
following agent failover and recovery.

The agent previously relied on the difference between the
set of checkpointed operations and the set of operation
IDs recovered from the operation status update manager to
apply any operations which had not been applied due to an
ill-timed agent failover.

However, this approach did not work in the case where a
persistent volume failed to be successfully created by
syncCheckpointedResources(). In order to handle this
case, this patch changes the agent code to continue with
the old approach of a two-phase-commit of persistent
volumes to disk, where the agent will fail to complete
recovery if syncCheckpointedResources() cannot be
executed successfully after failover.


Diffs
-

  src/slave/paths.hpp e077587fd02bd8e35fee7ce12ae436e3dca25e47 
  src/slave/paths.cpp 28a7cf9f9c70fb31eeefe2e823cd7e19ffcf126a 
  src/slave/slave.cpp 1d0ec9d2428c3ffa28ad3e960b74f171013cf0c2 
  src/slave/state.cpp cd3fac72dd57da21ed5ac46b17066531af26d42a 


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


Testing
---

Tested manually by doing the following:

1) Run a master
2) Run an agent with statically reserved resources for use in the following step
3) Run the persistent volume example framework to create a volume on the agent, 
which leads to checkpointing of resources
4) Use `chattr -R +i /agent/volumes/dir` to make the agent's persistent volume 
directory immutable
5) Run the persistent volume example framework again; it will fail and the 
agent will crash
6) Restart the agent; confirm that it continues to crash
7) Use `chattr -R -i /agent/volumes/dir` to remove the immutable attribute
8) Restart the agent; confirm that it recovers successfully, with the 
persistent volume created on disk
9) Run the persistent volume example framework again; it should succeed


Thanks,

Greg Mann

Re: Review Request 71335: Used cached cgroups for updating resources in Docker containerizer.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71335]

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 Aug. 21, 2019, 5:24 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71335/
> ---
> 
> (Updated Aug. 21, 2019, 5:24 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9836
> https://issues.apache.org/jira/browse/MESOS-9836
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used cached cgroups for updating resources in Docker containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp ca41f3b6f48f583b0aa1eb07df01d3872b80fa6c 
>   src/slave/containerizer/docker.cpp e4ad94572d34d49d599197afc08fd937253ecb0f 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> e25acfda336f10b2e3d6a79d1a336a290dc5b407 
> 
> 
> Diff: https://reviews.apache.org/r/71335/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 71315: Refactored master draining test setup.

[Joseph Wu]

> On Aug. 21, 2019, 8:59 a.m., Greg Mann wrote:
> > src/tests/master_draining_tests.cpp
> > Lines 206 (patched)
> > 
> >
> > Should we make this value larger? We advance the clock by the agent 
> > reregistration timeout in these tests, which could be large?

I don't think it makes too much of a difference how large or small this value 
is.  Larger values mean that advancing the clock by this interval may trigger 
different intervals too.  The tests will need to account for this either way.

Note: Later in the chain, I add some clock advances by this value, to test 
offers after reactivating agents.


> On Aug. 21, 2019, 8:59 a.m., Greg Mann wrote:
> > src/tests/master_draining_tests.cpp
> > Lines 216 (patched)
> > 
> >
> > Is this argument ever used?

Nope, I can remove it.


> On Aug. 21, 2019, 8:59 a.m., Greg Mann wrote:
> > src/tests/master_draining_tests.cpp
> > Lines 249 (patched)
> > 
> >
> > Do we need to parametrize these by content type? I suspect we have 
> > enough coverage of the API's handling of different content types elsewhere.

I'd lean towards keeping this parameterization.  The two variants mean we'll be 
serializing the agent draining protobufs to/from JSON.  Most of these protobufs 
are not used anywhere else too.


- Joseph


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


On Aug. 19, 2019, 2:55 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71315/
> ---
> 
> (Updated Aug. 19, 2019, 2:55 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Greg Mann.
> 
> 
> Bugs: MESOS-9892
> https://issues.apache.org/jira/browse/MESOS-9892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tests of this feature will generally require a master, agent, framework,
> and a single task to be launched at the beginning of the test.
> This moves this common code into the test SetUp.
> 
> This also changes the `post(...)` helper to return the http::Response
> object instead of parsing it.  The response for DRAIN_AGENT calls
> does not return an object, so the tests were not checking the
> response before.
> 
> 
> Diffs
> -
> 
>   src/tests/master_draining_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71315/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 71343: Fixed out-of-order processing of terminal status updates in agent.

[Andrei Budnik]

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

Review request for mesos, Gilbert Song, Greg Mann, and Qian Zhang.


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


Repository: mesos


Description
---

Previously, Mesos agent could send TASK_FAILED status update on
executor termination while processing of TASK_FINISHED status update
was in progress. Processing of task status updates involves sending
requests to the containerizer, which might finish processing of these
requests out-of-order, e.g. `MesosContainerizer::status`. Also,
the agent does not overwrite status of the terminal status update once
it's stored in the `terminatedTasks`. Hence, there was a race condition
between two terminal status updates.

Note that V1 Executors are not affected by this problem because they
wait for an acknowledgement of the terminal status update by the agent
before terminating.

This patch introduces a new data structure `pendingStatusUpdates`,
which holds a list of status updates that are being processed. This
data structure allows validating the order of processing of status
updates by the agent.


Diffs
-

  src/slave/slave.hpp a17bbee13cb8291ad694f1520b613764b57b046b 
  src/slave/slave.cpp 1d0ec9d2428c3ffa28ad3e960b74f171013cf0c2 


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


Testing
---

1. manual testing described in MESOS-9887
2. internal CI


Thanks,

Andrei Budnik

Re: Review Request 71315: Refactored master draining test setup.

[Greg Mann]

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



Nice refactor!! Love it


src/tests/master_draining_tests.cpp
Lines 206 (patched)


Should we make this value larger? We advance the clock by the agent 
reregistration timeout in these tests, which could be large?



src/tests/master_draining_tests.cpp
Lines 216 (patched)


Is this argument ever used?



src/tests/master_draining_tests.cpp
Lines 249 (patched)


Do we need to parametrize these by content type? I suspect we have enough 
coverage of the API's handling of different content types elsewhere.


- Greg Mann


On Aug. 19, 2019, 9:55 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71315/
> ---
> 
> (Updated Aug. 19, 2019, 9:55 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Greg Mann.
> 
> 
> Bugs: MESOS-9892
> https://issues.apache.org/jira/browse/MESOS-9892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tests of this feature will generally require a master, agent, framework,
> and a single task to be launched at the beginning of the test.
> This moves this common code into the test SetUp.
> 
> This also changes the `post(...)` helper to return the http::Response
> object instead of parsing it.  The response for DRAIN_AGENT calls
> does not return an object, so the tests were not checking the
> response before.
> 
> 
> Diffs
> -
> 
>   src/tests/master_draining_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71315/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 71314: Moved master-side agent draining tests into a separate file.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Aug. 19, 2019, 9:53 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71314/
> ---
> 
> (Updated Aug. 19, 2019, 9:53 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Greg Mann.
> 
> 
> Bugs: MESOS-9892
> https://issues.apache.org/jira/browse/MESOS-9892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The test bodies were not changed, besides renaming the test class.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 0377c8dc4a0f876f972797556c98d88aaab86589 
>   src/tests/CMakeLists.txt 04c552a828c6e262bcb66f552b4e42772e33623c 
>   src/tests/api_tests.cpp e202cd330d424efef783d39b74db5f856bd34895 
>   src/tests/master_draining_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71314/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 71338: Moved OpenSSL-related ifdef to a central location.

[Till Toenshoff via Review Board]

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


Fix it, then Ship it!




Thanks Benno for fixing my ugly patch.


3rdparty/libprocess/src/openssl.cpp
Lines 55 (patched)


As nicely noted by BenjaminB in my previous less beautiful RR, we might 
want to explain what version this was; "(OpenSSL 1.1.0)" as done above.


- Till Toenshoff


On Aug. 21, 2019, 1:34 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71338/
> ---
> 
> (Updated Aug. 21, 2019, 1:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A previous commit introduced an preprocessor directive that would
> split up code between brackets, confusing syntax highlighters and
> making the logic hard to read.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> 4aeab855b29837dc6bf93104afc62f111c38626c 
> 
> 
> Diff: https://reviews.apache.org/r/71338/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 71338: Moved OpenSSL-related ifdef to a central location.

[Benno Evers]

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

(Updated Aug. 21, 2019, 1:34 p.m.)


Review request for mesos, Benjamin Bannier and Till Toenshoff.


Repository: mesos


Description
---

A previous commit introduced an preprocessor directive that would
split up code between brackets, confusing syntax highlighters and
making the logic hard to read.


Diffs (updated)
-

  3rdparty/libprocess/src/openssl.cpp 4aeab855b29837dc6bf93104afc62f111c38626c 


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

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


Testing
---


Thanks,

Benno Evers

Review Request 71340: Allowed passing resource provider infos into call validation.

[Benjamin Bannier]

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

Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.


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


Repository: mesos


Description
---

With this patch it becomes possible to take a `ResourceProviderInfo`
into account when validating calls. While this feature is currently
unused, we will add additional validations in a follow-up patch.


Diffs
-

  src/resource_provider/driver.cpp eda019c1ad8893e14acc5979b350af4a099138be 
  src/resource_provider/manager.cpp ceed1225b37d23998f523afcc2184dfaaad60636 
  src/resource_provider/validation.hpp 0068f565b945fae7caf45955caf1067e0a7b7f26 
  src/resource_provider/validation.cpp df55b5efe3543c1dfd8441997302ab76fdd4bcc1 
  src/tests/resource_provider_validation_tests.cpp 
a9989412ae30bd8244be808fc88fbe70f47d6ad9 


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


Testing
---

`ninja check`


Thanks,

Benjamin Bannier

Review Request 71341: Validated provider ID use in some resource provider calls.

[Benjamin Bannier]

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

Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.


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


Repository: mesos


Description
---

For some calls we expect resource providers to set provider IDs with the
calls. While the resource provider manager has always asserted that the
calls were correct we never validated this.

With this patch we perform additional validation for calls taking a
`ResourceProviderInfo` into account. We add both unit tests for the
validation code and an integration test confirming that the validation
is actually triggered.


Diffs
-

  src/resource_provider/manager.cpp ceed1225b37d23998f523afcc2184dfaaad60636 
  src/resource_provider/validation.cpp df55b5efe3543c1dfd8441997302ab76fdd4bcc1 
  src/tests/resource_provider_manager_tests.cpp 
bcf6a03aa5d4931feff0299c811faa216efd95b6 
  src/tests/resource_provider_validation_tests.cpp 
a9989412ae30bd8244be808fc88fbe70f47d6ad9 


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


Testing
---

`ninja check`


Thanks,

Benjamin Bannier

Review Request 71339: Refactored resource provider call validation.

[Benjamin Bannier]

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

Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.


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


Repository: mesos


Description
---

This patch collects validation of the presence of a resource provider ID
in a resource provider call into a single place.


Diffs
-

  src/resource_provider/validation.cpp df55b5efe3543c1dfd8441997302ab76fdd4bcc1 


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


Testing
---

`ninja check`


Thanks,

Benjamin Bannier

Review Request 71338: Moved OpenSSL-related ifdef to a central location.

[Benno Evers]

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

Review request for mesos, Benjamin Bannier and Till Toenshoff.


Repository: mesos


Description
---

A previous commit introduced an preprocessor directive that would
split up code between brackets, confusing syntax highlighters and
making the logic hard to read.


Diffs
-

  3rdparty/libprocess/src/openssl.cpp 4aeab855b29837dc6bf93104afc62f111c38626c 


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


Testing
---


Thanks,

Benno Evers

Re: Review Request 71337: Switched on full debug logging and stack trace reporting for Maven.

[Benno Evers]

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


Ship it!




Ship It!

- Benno Evers


On Aug. 21, 2019, 12:50 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71337/
> ---
> 
> (Updated Aug. 21, 2019, 12:50 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Benno Evers.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is done in attempt to track down the origin of Java build errors
> sporadically observed on CIs. The added volume of logs is around 15K
> lines, which is still much smaller than that of the logs of the tests.
> Developers who perform local build without Java bindings should
> not be affected.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 0377c8dc4a0f876f972797556c98d88aaab86589 
>   src/java/CMakeLists.txt 29422e90b10647dc1bf78237725e89b15c69e6aa 
> 
> 
> Diff: https://reviews.apache.org/r/71337/diff/1/
> 
> 
> Testing
> ---
> 
> Ran tests in CI.
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Review Request 71337: Switched on full debug logging and stack trace reporting for Maven.

[Andrei Sekretenko]

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

Review request for mesos, Benjamin Bannier and Benno Evers.


Repository: mesos


Description
---

This is done in attempt to track down the origin of Java build errors
sporadically observed on CIs. The added volume of logs is around 15K
lines, which is still much smaller than that of the logs of the tests.
Developers who perform local build without Java bindings should
not be affected.


Diffs
-

  src/Makefile.am 0377c8dc4a0f876f972797556c98d88aaab86589 
  src/java/CMakeLists.txt 29422e90b10647dc1bf78237725e89b15c69e6aa 


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


Testing
---

Ran tests in CI.


Thanks,

Andrei Sekretenko

Re: Review Request 71174: Recovered network info for nested/standalone containers in CNI isolator.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [71174]

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_71174"]

Error:
..
cess'
/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_OPENSSL_SSL_H=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_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../../../3rdparty/libprocess  
-DBUILD_DIR=\"/mesos/mesos-1.9.0/_build/3rdparty/libprocess\" 
-I../../../3rdparty/libprocess/include -I../../../3rdparty/li
 bprocess/src -I../boost-1.65.0 -I../concurrentqueue-7b69a8f -I../elfio-3.2 
-I../glog-0.4.0/src  -I../grpc-1.10.0/include -I../http-parser-2.6.2 
-I../libev-4.22  -D__STDC_FORMAT_MACROS -I../picojson-1.3.0 
-I../protobuf-3.5.0/src -I../rapidjson-1.1.0/include 
-I../../../3rdparty/libprocess/../stout/include  -DLIBPROCESS_ALLOW_JEMALLOC 
-I/usr/include/subversion-1 -I/usr/include/apr-1 -I/usr/include/apr-1.0 
-Wall -Wsign-compare -Wformat-security -fstack-protector -fPIC -fPIE -g1 -O0 
-Wno-unused-local-typedefs -std=c++11 -c -o src/libprocess_la-authenticator.lo 
`test -f 'src/authenticator.cpp' || echo 
'../../../3rdparty/libprocess/'`src/authenticator.cpp
/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_OPENSSL_SSL_H=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_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../../../3rdparty/libprocess  
-DBUILD_DIR=\"/mesos/mesos-1.9.0/_build/3rdparty/libprocess\" 
-I../../../3rdparty/libprocess/include -I../../../3rdparty/li
 bprocess/src -I../boost-1.65.0 -I../concurrentqueue-7b69a8f -I../elfio-3.2 
-I../glog-0.4.0/src  -I../grpc-1.10.0/include -I../http-parser-2.6.2 
-I../libev-4.22  -D__STDC_FORMAT_MACROS -I../picojson-1.3.0 
-I../protobuf-3.5.0/src -I../rapidjson-1.1.0/include 
-I../../../3rdparty/libprocess/../stout/include  -DLIBPROCESS_ALLOW_JEMALLOC 
-I/usr/include/subversion-1 -I/usr/include/apr-1 -I/usr/include/apr-1.0 
-Wall -Wsign-compare -Wformat-security -fstack-protector -fPIC -fPIE -g1 -O0 
-Wno-unused-local-typedefs -std=c++11 -c -o 
src/libprocess_la-authenticator_manager.lo `test -f 
'src/authenticator_manager.cpp' || echo 
'../../../3rdparty/libprocess/'`src/authenticator_manager.cpp
/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_OPENSSL_SSL_H=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_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../../../3rdparty/libprocess  
-DBUILD_DIR=\"/mesos/mesos-1.9.0/_build/3rdparty/libprocess\" 
-I../../../3rdparty/libprocess/include -I../../../3rdparty/li

Review Request 71335: Used cached cgroups for updating resources in Docker containerizer.

[Qian Zhang]

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

Review request for mesos, Andrei Budnik and Gilbert Song.


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


Repository: mesos


Description
---

Used cached cgroups for updating resources in Docker containerizer.


Diffs
-

  src/slave/containerizer/docker.hpp ca41f3b6f48f583b0aa1eb07df01d3872b80fa6c 
  src/slave/containerizer/docker.cpp e4ad94572d34d49d599197afc08fd937253ecb0f 
  src/tests/containerizer/docker_containerizer_tests.cpp 
e25acfda336f10b2e3d6a79d1a336a290dc5b407 


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


Testing
---

sudo make check


Thanks,

Qian Zhang

Re: Review Request 71336: Fixed deprecation warning when building against OpenSSL 1.1.x.

[Till Toenshoff via Review Board]

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

(Updated Aug. 21, 2019, 11:45 a.m.)


Review request for mesos, Benno Evers and Benjamin Mahler.


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


Repository: mesos


Description
---

OpenSSL 1.1.x deprecates 'ASN1_STRING_get_data' and replaces it
by 'ASN1_STRING_get0_data'.


Diffs
-

  3rdparty/libprocess/src/openssl.cpp ee9a049d06bdf6365930aa92873dd97af197eba0 


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


Testing
---

```
cmake .. -DUNBUNDLED_ZOOKEEPER=ON -DUNBUNDLED_LIBEVENT=ON 
-DUNBUNDLED_LEVELDB=ON -DUNBUNDLED_LIBARCHIVE=ON -DENABLE_SSL=ON 
-DENABLE_LIBEVENT=ON -DOPENSSL_ROOT_DIR=/usr/local/opt/openssl@1.1 
-DLIBEVENT_ROOT_DIR=/Users/till/usr -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -G Ninja
ninja tests -j12
```


Thanks,

Till Toenshoff

Re: Review Request 71336: Fixed deprecation warning when building against OpenSSL 1.1.x.

[Benjamin Bannier]

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


Fix it, then Ship it!





3rdparty/libprocess/src/openssl.cpp
Line 889 (original), 889 (patched)


Let's add a comment here correlating this with an "normal" version number. 
Is this 1.0.1 (what patch level)?


- Benjamin Bannier


On Aug. 21, 2019, 1:16 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71336/
> ---
> 
> (Updated Aug. 21, 2019, 1:16 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> OpenSSL 1.1.x deprecates 'ASN1_STRING_get_data' and replaces it
> by 'ASN1_STRING_get0_data'.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> ee9a049d06bdf6365930aa92873dd97af197eba0 
> 
> 
> Diff: https://reviews.apache.org/r/71336/diff/1/
> 
> 
> Testing
> ---
> 
> ```
> cmake .. -DUNBUNDLED_ZOOKEEPER=ON -DUNBUNDLED_LIBEVENT=ON 
> -DUNBUNDLED_LEVELDB=ON -DUNBUNDLED_LIBARCHIVE=ON -DENABLE_SSL=ON 
> -DENABLE_LIBEVENT=ON -DOPENSSL_ROOT_DIR=/usr/local/opt/openssl@1.1 
> -DLIBEVENT_ROOT_DIR=/Users/till/usr -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -G Ninja
> ninja tests -j12
> ```
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Review Request 71336: Fixed deprecation warning when building against OpenSSL 1.1.x.

[Till Toenshoff via Review Board]

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

Review request for mesos, Benno Evers and Benjamin Mahler.


Repository: mesos


Description
---

OpenSSL 1.1.x deprecates 'ASN1_STRING_get_data' and replaces it
by 'ASN1_STRING_get0_data'.


Diffs
-

  3rdparty/libprocess/src/openssl.cpp ee9a049d06bdf6365930aa92873dd97af197eba0 


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


Testing
---

```
cmake .. -DUNBUNDLED_ZOOKEEPER=ON -DUNBUNDLED_LIBEVENT=ON 
-DUNBUNDLED_LEVELDB=ON -DUNBUNDLED_LIBARCHIVE=ON -DENABLE_SSL=ON 
-DENABLE_LIBEVENT=ON -DOPENSSL_ROOT_DIR=/usr/local/opt/openssl@1.1 
-DLIBEVENT_ROOT_DIR=/Users/till/usr -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -G Ninja
ninja tests -j12
```


Thanks,

Till Toenshoff

Re: Review Request 71174: Recovered network info for nested/standalone containers in CNI isolator.

[Qian Zhang]

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

(Updated Aug. 21, 2019, 6:43 p.m.)


Review request for mesos, Andrei Budnik and Gilbert Song.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Recovered network info for nested/standalone containers in CNI isolator.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
3e5b66702be5c042ef6c7c580cbcbf6afd3167f0 
  src/common/protobuf_utils.hpp 5d6a35d6e3bae35b83e87827724206f7c5dfb2d8 
  src/common/protobuf_utils.cpp 64777c0e2f28aba6cd34cbdd92b914485f8d73e7 
  src/slave/containerizer/mesos/containerizer.cpp 
9030af1463d7de7f339e1b39f0c4eeb24c5d3105 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
8d037334a8e449408918c584eade9ac6bc93337c 


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

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


Testing
---

sudo make check


Thanks,

Qian Zhang