Re: Review Request 69839: Fixed scheduler library on multiple SUBSCRIBE requests per connection.

2019-01-29 Thread Mesos Reviewbot Windows 

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['69839']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2830/mesos-review-69839

Relevant logs:

- 
[mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2830/mesos-review-69839/logs/mesos-tests.log):

```
I0130 05:42:03.596961 22928 master.cpp:11288] Removing task 
19ffc0d7-0a0f-41a2-99f1-90db569a057c with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework 325a24e2-09bf-4319-8075-512a05075e2c- on 
agent 325a24e2-09bf-4319-8075-512a05075e2c-S0 at slave(473)@192.10.1.6:60486 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0130 05:42:03.599964 22928 master.cpp:1269] Agent 
325a24e2-09bf-4319-8075-512a05075e2c-S0 at slave(473)@192.10.1.6:60486 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net) disconnected
I0130 05:42:03.599964 22928 master.cpp:3272] Disconnecting agent 
325a24e2-09bf-4319-8075-512a05075e2c-S0 at slave(473)@192.10.1.6:60486 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0130 05:42:03.599964 22928 master.cpp:3291] Deactivating agent 
325a24e2-09bf-4319-8075-512a05075e2c-S0 at slave(473)@192.10.1.6:60486 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0130 05:42:03.600972 20728 hierarchical.cpp:358] Removed framework 
325a24e2-09bf-4319-8075-512a05075e2c-
I0130 05:42:03.600972 20728 hierarchical.cpp:793] Agent 
325a24e2-09bf-4319-8075-512a05075e2c-S0 deactivated
I0130 05:42:03.600972 12804 containerizer.cpp:2477] Destroying container 
d44b872d-7a49-4804-ad73-b1c1238f925f in RUNNING state
I0130 05:42:03.600972 12804 containerizer.cpp:3144] Transitioning the state of 
container d44b872d-7a49-4804-ad73-b1c1238f925f from RUNNING to DESTROYING
I0130 05:42:03.601981 12804 launcher.cpp:161] Asked to destroy container 
d44b872d-7a49-4804-ad73-b1c1238f925f
W0130 05:42:03.602957 22940 process.cpp:1423] Failed to recv on socket 
WindowsFD::Type::SOCKET=4548 to peer '192.10.1.6:62375': IO failed with error 
code: The specified network name is no longer available.

W0130 05:42:03.602957 22940 process.cpp:838] Failed to recv [   OK ] 
IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (686 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (703 ms total)

[--] Global test environment tear-down
[==] 1092 tests from 104 test cases ran. (497127 ms total)
[  PASSED  ] 1091 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName

 1 FAILED TEST
  YOU HAVE 231 DISABLED TESTS

on socket WindowsFD::Type::SOCKET=4524 to peer '192.10.1.6:62376': IO failed 
with error code: The specified network name is no longer available.

I0130 05:42:03.659978 20728 containerizer.cpp:2983] Container 
d44b872d-7a49-4804-ad73-b1c1238f925f has exited
I0130 05:42:03.690968 25564 master.cpp:1109] Master terminating
I0130 05:42:03.691982 23232 hierarchical.cpp:644] Removed agent 
325a24e2-09bf-4319-8075-512a05075e2c-S0
I0130 05:42:04.053977 22940 process.cpp:927] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Jan. 30, 2019, 4:34 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69839/
> ---
> 
> (Updated Jan. 30, 2019, 4:34 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Bugs: MESOS-9210
> https://issues.apache.org/jira/browse/MESOS-9210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The HTTP scheduler API dictates that on a single connection, the
> scheduler may only send a single SUBSCRIBE request. Due to recent
> authentication related changes, this contract got broken. This patch
> restores the contract and adds a test validating that the library is
> enforcing it.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp cb24ba9c8e1d04b8c62bdf07b12758a61b3bf036 
>   src/tests/scheduler_tests.cpp b571bb1d20744b943580677a26db4c12c7c311d1 
> 
> 
> Diff: https://reviews.apache.org/r/69839/diff/2/
> 
> 
> Testing
> ---
> 
> manual testing; 
> Running the included test without patching `scheduler.cpp` -> fails as the 
> master does in fact receive two SUBSCRIBE requests.
> 
> `make check`
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 69858: Persisted intentionally dropped operations in SLRP.

2019-01-29 Thread Chun-Hung Hsiao 

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

(Updated Jan. 30, 2019, 5:38 a.m.)


Review request for mesos, Benjamin Bannier, Gastón Kleiman, and Greg Mann.


Changes
---

Added a missing checkpointing call.


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


Repository: mesos


Description
---

If an operation is dropped intentionally (e.g., because of a resource
version mismatch), the operation should be persisted so no conflicting
status update would be generated for operation reconciliation.


Diffs (updated)
-

  src/resource_provider/storage/provider.cpp 
d6e20a549ede189c757ae3ae922ab7cb86d2be2c 
  src/resource_provider/storage/provider_process.hpp PRE-CREATION 


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

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


Testing
---

Test will be added later in chain.


Thanks,

Chun-Hung Hsiao

Review Request 69858: Persisted intentionally dropped operations in SLRP.

2019-01-29 Thread Chun-Hung Hsiao 

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

Review request for mesos, Benjamin Bannier, Gastón Kleiman, and Greg Mann.


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


Repository: mesos


Description
---

If an operation is dropped intentionally (e.g., because of a resource
version mismatch), the operation should be persisted so no conflicting
status update would be generated for operation reconciliation.


Diffs
-

  src/resource_provider/storage/provider.cpp 
d6e20a549ede189c757ae3ae922ab7cb86d2be2c 
  src/resource_provider/storage/provider_process.hpp PRE-CREATION 


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


Testing
---

Test will be added later in chain.


Thanks,

Chun-Hung Hsiao

Re: Review Request 69839: Fixed scheduler library on multiple SUBSCRIBE requests per connection.

2019-01-29 Thread Till Toenshoff via Review Board 

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

(Updated Jan. 30, 2019, 4:34 a.m.)


Review request for mesos, Alexander Rukletsov and Vinod Kone.


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


Repository: mesos


Description (updated)
---

The HTTP scheduler API dictates that on a single connection, the
scheduler may only send a single SUBSCRIBE request. Due to recent
authentication related changes, this contract got broken. This patch
restores the contract and adds a test validating that the library is
enforcing it.


Diffs (updated)
-

  src/scheduler/scheduler.cpp cb24ba9c8e1d04b8c62bdf07b12758a61b3bf036 
  src/tests/scheduler_tests.cpp b571bb1d20744b943580677a26db4c12c7c311d1 


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

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


Testing
---

manual testing; 
Running the included test without patching `scheduler.cpp` -> fails as the 
master does in fact receive two SUBSCRIBE requests.

`make check`


Thanks,

Till Toenshoff

Re: Review Request 69843: Track operations on agent default resources in master state.

2019-01-29 Thread Mesos Reviewbot Windows 

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['69786', '69723', '69790', '69791', '69792', '69793', 
'69794', '69795', '69825', '69851', '69843']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2829/mesos-review-69843

Relevant logs:

- 
[mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2829/mesos-review-69843/logs/mesos-tests.log):

```
W0130 00:42:31.749464 23388 slave.cpp:3934] Ignoring shutdown framework 
f4443635-00b3-40d9-b169-11c709f6a25f- because it is terminating
I0130 00:42:31.751462 16092 master.cpp:1269] Agent 
f4443635-00b3-40d9-b169-11c709f6a25f-S0 at slave(477)@192.10.1.6:58193 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net) disconnected
I0130 00:42:31.752456 16092 master.cpp:3272] Disconnecting agent 
f4443635-00b3-40d9-b169-11c709f6a25f-S0 at slave(477)@192.10.1.6:58193 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0130 00:42:31.752456 16092 master.cpp:3291] Deactivating agent 
f4443635-00b3-40d9-b169-11c709f6a25f-S0 at slave(477)@192.10.1.6:58193 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0130 00:42:31.752456 10424 hierarchical.cpp:358] Removed framework 
f4443635-00b3-40d9-b169-11c709f6a25f-
I0130 00:42:31.752456 10424 hierarchical.cpp:793] Agent 
f4443635-00b3-40d9-b169-11c709f6a25f-S0 deactivated
I0130 00:42:31.753477 25528 containerizer.cpp:2477] Destroying container 
e3e37928-0e93-4a4f-b8aa-6c363e8956cd in RUNNING state
I0130 00:42:31.753477 25528 containerizer.cpp:3144] Transitioning the state of 
container e3e37928-0e93-4a4f-b8aa-6c363e8956cd from RUNNING to DESTROYING
I0130 00:42:31.754835 25528 launcher.cpp:161] Asked to destroy container 
e3e37928-0e93-4a4f-b8aa-6c363e8956cd
W0130 00:42:31.755467 24792 process.cpp:838] Failed to recv on socket 
WindowsFD::Type::SOCKET=9276 to peer '192.10.1.6:60086': IO failed with error 
code: The specified network name [   OK ] 
IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (688 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (705 ms total)

[--] Global test environment tear-down
[==] 1093 tests from 104 test cases ran. (494677 ms total)
[  PASSED  ] 1092 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage

 1 FAILED TEST
  YOU HAVE 231 DISABLED TESTS

is no longer available.

W0130 00:42:31.756462 24792 process.cpp:1423] Failed to recv on socket 
WindowsFD::Type::SOCKET=9400 to peer '192.10.1.6:60085': IO failed with error 
code: The specified network name is no longer available.

I0130 00:42:31.840862 22188 containerizer.cpp:2983] Container 
e3e37928-0e93-4a4f-b8aa-6c363e8956cd has exited
I0130 00:42:31.873888 23388 master.cpp:1109] Master terminating
I0130 00:42:31.874857 25528 hierarchical.cpp:644] Removed agent 
f4443635-00b3-40d9-b169-11c709f6a25f-S0
I0130 00:42:32.242929 24792 process.cpp:927] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Jan. 29, 2019, 3:30 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69843/
> ---
> 
> (Updated Jan. 29, 2019, 3:30 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Gastón 
> Kleiman.
> 
> 
> Bugs: MESOS-9471
> https://issues.apache.org/jira/browse/MESOS-9471
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the master to add operations on agent
> default resources to its in-memory state upon receipt of
> UpdateSlaveMessages.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 2e0a0de4d745112011370741b8cedbb6db17b915 
>   src/tests/master_tests.cpp 1f8da63aa313c4679c0d7a3934802af1474e8f28 
> 
> 
> Diff: https://reviews.apache.org/r/69843/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> `bin/mesos-tests.sh --gtest_filter="*OperationUpdateDuringFailover*" 
> --gtest_repeat=-1 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 69851: Added end-to-end test for operations affecting agent default resources.

2019-01-29 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 69851 was successfully built and tested.

Reviews applied: `['69786', '69723', '69790', '69791', '69792', '69793', 
'69794', '69795', '69825', '69851']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2828/mesos-review-69851

- Mesos Reviewbot Windows


On Jan. 29, 2019, 9:56 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69851/
> ---
> 
> (Updated Jan. 29, 2019, 9:56 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Greg Mann.
> 
> 
> Bugs: MESOS-9356
> https://issues.apache.org/jira/browse/MESOS-9356
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added end-to-end test for operations affecting agent default resources.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 9168e06348a83dcb20400a2fe0e3bb1f26e6ff1b 
> 
> 
> Diff: https://reviews.apache.org/r/69851/diff/2/
> 
> 
> Testing
> ---
> 
> `GLOG_v=1 bin/mesos-tests.sh --verbose 
> --gtest_filter="*Slave*RetryOperationStatusUpdateAfterRecovery*" 
> --gtest_repeat=5000 --gtest_break_on_failure` passed on CentOS 7.4.1708.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 69843: Track operations on agent default resources in master state.

2019-01-29 Thread Greg Mann 

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

(Updated Jan. 29, 2019, 11:30 p.m.)


Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Gastón Kleiman.


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


Repository: mesos


Description
---

This patch updates the master to add operations on agent
default resources to its in-memory state upon receipt of
UpdateSlaveMessages.


Diffs
-

  src/master/master.cpp 2e0a0de4d745112011370741b8cedbb6db17b915 
  src/tests/master_tests.cpp 1f8da63aa313c4679c0d7a3934802af1474e8f28 


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


Testing
---

`make check`
`bin/mesos-tests.sh --gtest_filter="*OperationUpdateDuringFailover*" 
--gtest_repeat=-1 --gtest_break_on_failure`


Thanks,

Greg Mann

Re: Review Request 69795: Made agent recover atomically checkpointed resources and operations.

2019-01-29 Thread Gastón Kleiman 

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

(Updated Jan. 29, 2019, 3:33 p.m.)


Review request for mesos, Benno Evers and Greg Mann.


Changes
---

Fixed indentation.


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


Repository: mesos


Description
---

Made agent recover atomically checkpointed resources and operations.


Diffs (updated)
-

  src/slave/slave.hpp 2bcd7a93a8f25b77c71c7f931bfaac87649f987c 
  src/slave/slave.cpp ed92f672f5155d70a36ba3619bb6f06fa09bc836 
  src/slave/state.hpp e2180ae37a65a57d1edb29d2ad6cc8232029906e 
  src/slave/state.cpp ae16d6f656cff71bcbdd4ffb02888024530b9274 


Diff: https://reviews.apache.org/r/69795/diff/8/

Changes: https://reviews.apache.org/r/69795/diff/7-8/


Testing
---

Current tests still pass.


Thanks,

Gastón Kleiman

Re: Review Request 69795: Made agent recover atomically checkpointed resources and operations.

2019-01-29 Thread Greg Mann 

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


Fix it, then Ship it!





src/slave/slave.cpp
Lines 7385-7388 (patched)


Nit: indented too far.


- Greg Mann


On Jan. 29, 2019, 11:17 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69795/
> ---
> 
> (Updated Jan. 29, 2019, 11:17 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Greg Mann.
> 
> 
> Bugs: MESOS-9356
> https://issues.apache.org/jira/browse/MESOS-9356
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made agent recover atomically checkpointed resources and operations.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 2bcd7a93a8f25b77c71c7f931bfaac87649f987c 
>   src/slave/slave.cpp ed92f672f5155d70a36ba3619bb6f06fa09bc836 
>   src/slave/state.hpp e2180ae37a65a57d1edb29d2ad6cc8232029906e 
>   src/slave/state.cpp ae16d6f656cff71bcbdd4ffb02888024530b9274 
> 
> 
> Diff: https://reviews.apache.org/r/69795/diff/7/
> 
> 
> Testing
> ---
> 
> Current tests still pass.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 69851: Added end-to-end test for operations affecting agent default resources.

2019-01-29 Thread Greg Mann 

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


Ship it!




Ship It!

- Greg Mann


On Jan. 29, 2019, 9:56 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69851/
> ---
> 
> (Updated Jan. 29, 2019, 9:56 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Greg Mann.
> 
> 
> Bugs: MESOS-9356
> https://issues.apache.org/jira/browse/MESOS-9356
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added end-to-end test for operations affecting agent default resources.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 9168e06348a83dcb20400a2fe0e3bb1f26e6ff1b 
> 
> 
> Diff: https://reviews.apache.org/r/69851/diff/2/
> 
> 
> Testing
> ---
> 
> `GLOG_v=1 bin/mesos-tests.sh --verbose 
> --gtest_filter="*Slave*RetryOperationStatusUpdateAfterRecovery*" 
> --gtest_repeat=5000 --gtest_break_on_failure` passed on CentOS 7.4.1708.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 69843: Track operations on agent default resources in master state.

2019-01-29 Thread Gastón Kleiman 

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


Ship it!




Ship It!

- Gastón Kleiman


On Jan. 25, 2019, 6:22 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69843/
> ---
> 
> (Updated Jan. 25, 2019, 6:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Gastón 
> Kleiman.
> 
> 
> Bugs: MESOS-9471
> https://issues.apache.org/jira/browse/MESOS-9471
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the master to add operations on agent
> default resources to its in-memory state upon receipt of
> UpdateSlaveMessages.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 2e0a0de4d745112011370741b8cedbb6db17b915 
>   src/tests/master_tests.cpp 1f8da63aa313c4679c0d7a3934802af1474e8f28 
> 
> 
> Diff: https://reviews.apache.org/r/69843/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> `bin/mesos-tests.sh --gtest_filter="*OperationUpdateDuringFailover*" 
> --gtest_repeat=-1 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 69795: Made agent recover atomically checkpointed resources and operations.

2019-01-29 Thread Gastón Kleiman 

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

(Updated Jan. 29, 2019, 3:17 p.m.)


Review request for mesos, Benno Evers and Greg Mann.


Changes
---

Fixed bugs.


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


Repository: mesos


Description
---

Made agent recover atomically checkpointed resources and operations.


Diffs (updated)
-

  src/slave/slave.hpp 2bcd7a93a8f25b77c71c7f931bfaac87649f987c 
  src/slave/slave.cpp ed92f672f5155d70a36ba3619bb6f06fa09bc836 
  src/slave/state.hpp e2180ae37a65a57d1edb29d2ad6cc8232029906e 
  src/slave/state.cpp ae16d6f656cff71bcbdd4ffb02888024530b9274 


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

Changes: https://reviews.apache.org/r/69795/diff/6-7/


Testing
---

Current tests still pass.


Thanks,

Gastón Kleiman

Re: Review Request 69851: Added end-to-end test for operations affecting agent default resources.

2019-01-29 Thread Gastón Kleiman 

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

(Updated Jan. 29, 2019, 1:56 p.m.)


Review request for mesos, Benno Evers and Greg Mann.


Changes
---

Addressed feedback.


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


Repository: mesos


Description
---

Added end-to-end test for operations affecting agent default resources.


Diffs (updated)
-

  src/tests/slave_tests.cpp 9168e06348a83dcb20400a2fe0e3bb1f26e6ff1b 


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

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


Testing
---

`GLOG_v=1 bin/mesos-tests.sh --verbose 
--gtest_filter="*Slave*RetryOperationStatusUpdateAfterRecovery*" 
--gtest_repeat=5000 --gtest_break_on_failure` passed on CentOS 7.4.1708.


Thanks,

Gastón Kleiman

Re: Review Request 69723: Enabled operation feedback on agent default resources.

2019-01-29 Thread Greg Mann 


> On Jan. 18, 2019, 1:51 a.m., Greg Mann wrote:
> > src/tests/api_tests.cpp
> > Lines 5260 (patched)
> > 
> >
> > s/that the we/that schedulers/

We can fix this while committing.


- Greg


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


On Jan. 18, 2019, 1:45 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69723/
> ---
> 
> (Updated Jan. 18, 2019, 1:45 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Joseph Wu.
> 
> 
> Bugs: MESOS-9472
> https://issues.apache.org/jira/browse/MESOS-9472
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Up to now, the master would validate that any operation
> requesting operation feedback would only act on resources
> belonging to a resource provider.
> 
> This patch removes this restriction and enables frameworks
> to request operation feedback on an agent's default
> resources.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 2339207149a85578ea47cf66f28392182f9075f2 
>   src/master/validation.cpp a71edeb7827b534e27ad3e3398abe555bf36e741 
>   src/slave/slave.cpp 10cbc190acc7eea5734efa0066541168545a66b6 
>   src/tests/api_tests.cpp 79a2d2966ff6533629aeeefaaaf19e849b76e0e6 
>   src/tests/master_tests.cpp 1f8da63aa313c4679c0d7a3934802af1474e8f28 
>   src/tests/mesos.hpp c2a5e5531de7498241e537ef1699e1a5e669b550 
> 
> 
> Diff: https://reviews.apache.org/r/69723/diff/3/
> 
> 
> Testing
> ---
> 
> ./src/mesos-tests
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 69723: Enabled operation feedback on agent default resources.

2019-01-29 Thread Greg Mann 

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


Ship it!




Ship It!

- Greg Mann


On Jan. 18, 2019, 1:45 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69723/
> ---
> 
> (Updated Jan. 18, 2019, 1:45 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Joseph Wu.
> 
> 
> Bugs: MESOS-9472
> https://issues.apache.org/jira/browse/MESOS-9472
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Up to now, the master would validate that any operation
> requesting operation feedback would only act on resources
> belonging to a resource provider.
> 
> This patch removes this restriction and enables frameworks
> to request operation feedback on an agent's default
> resources.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 2339207149a85578ea47cf66f28392182f9075f2 
>   src/master/validation.cpp a71edeb7827b534e27ad3e3398abe555bf36e741 
>   src/slave/slave.cpp 10cbc190acc7eea5734efa0066541168545a66b6 
>   src/tests/api_tests.cpp 79a2d2966ff6533629aeeefaaaf19e849b76e0e6 
>   src/tests/master_tests.cpp 1f8da63aa313c4679c0d7a3934802af1474e8f28 
>   src/tests/mesos.hpp c2a5e5531de7498241e537ef1699e1a5e669b550 
> 
> 
> Diff: https://reviews.apache.org/r/69723/diff/3/
> 
> 
> Testing
> ---
> 
> ./src/mesos-tests
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 69815: Added a unit test for RPC retry in SLRP.

2019-01-29 Thread Benjamin Bannier 

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


Ship it!




Ship It!

- Benjamin Bannier


On Jan. 26, 2019, 1:47 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69815/
> ---
> 
> (Updated Jan. 26, 2019, 1:47 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-9517
> https://issues.apache.org/jira/browse/MESOS-9517
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a unit test to verify that SLRP will retry
> `CreateVolume` and `DeleteVolume` CSI calls with a random exponential
> backoff upon receiving `DEADLINE_EXCEEDED` or `UNAVAILABLE`.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> e8ed20f818ed7f1a3ce15758ea3c366520443377 
> 
> 
> Diff: https://reviews.apache.org/r/69815/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-01-29 Thread Benjamin Mahler 

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



The commit description makes it sound like the filter is using the **union** of 
the flag and the per-framework/role override:

> This set is populated with any minimal allocatable resources specified in the 
> allocator's options and any framework's minimal allocatable resources.

However, it should be an override (which is what the patch correctly does 
AFAICT). Probably, just using "override" in the explanation would make this 
very clear to the reader.

IMO, we only really need the first paragraph of the description:

> This patch modifies the hierarchical allocator to take framework-specified 
> minimal allocatable resources into account.

We could make it a bit clearer:

> This patch modifies the allocator to take the framework-specified minimum 
> allocatable resources into account. These act as an override of the existing 
> `min_allocatable_resources` flag.

What more needs to be said?


src/master/allocator/mesos/hierarchical.cpp
Lines 1952-1968 (original), 1978-1987 (patched)


This makes it look like we should consolidate into the `isFiltered` 
function?



src/master/allocator/mesos/hierarchical.cpp
Line 2069 (original), 2082 (patched)


Now that this is a continue instead of a break, it seems they can be 
consolidated into 1 below?

Also, can you post the benchmark numbers before and after this patch to 
make sure we're not accidentally regressing pretty badly on performance?



src/master/allocator/mesos/hierarchical.cpp
Lines 2400-2409 (patched)


Hm.. what's this? Are you suggesting that the allocatable check should be 
moved within isFiltered? (seems like a good idea to me)



src/tests/hierarchical_allocator_tests.cpp
Lines 2287-2289 (patched)


Can you pull the tests into a separate review to simplify reviewing?



src/tests/hierarchical_allocator_tests.cpp
Lines 2289 (patched)


FrameworkMinAllocatable



src/tests/hierarchical_allocator_tests.cpp
Lines 2336 (patched)


FrameworkEmptyMinAllocatable



src/tests/hierarchical_allocator_tests.cpp
Lines 2378 (patched)


Hm.. this doesn't quite look right to me, it seems if a framework is 
setting the `min_allocatable_resources` field, it should override, but that's 
not happening here?


- Benjamin Mahler


On Jan. 29, 2019, 4:20 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Jan. 29, 2019, 4:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its options, it now inspects a dynamically
> calculated set of minimal allocatable resources. This set is populated
> with any minimal allocatable resources specified in the allocator's
> options and any framework's minimal allocatable resources. For a
> resource to be allocatable it needs to contain any of these minimal
> requirements.
> 
> If a framework does not specify minimal allocatable resource
> requirements, its minimal requirements are set to the globally
> configured option.
> 
> We also adjust the allocator to take a framework's minimal resource
> requirements into account when checking whether a resource can be
> allocated to a particular framework. The check is performed at the same
> time we check whether a framework filtered a particular resource. This
> avoids offering resources to frameworks which the framework would never
> have considered allocatable itself (e.g., given the global option of the
> allocator).
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> ca1638390d89e2a81efd9d6d4a28b863c79723c4 
>   src/master/allocator/mesos/hierarchical.cpp 
> f1f3894058a8e3f008013cb269744bd36c0e31b3 
>   src/tests/hierarchical_allocator_tests.cpp 
> cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69818: Added offer filters resources to static framework configuration.

2019-01-29 Thread Benjamin Mahler 

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



I think you missed my top level comment from the last review to add context to 
the commit about the min allocatable flag and how this is providing a 
per-framework/role override of that global flag?

Also, we're missing a patch for the master to allow changing the value upon 
framework re-registration?


src/common/resource_quantities.cpp
Lines 82-90 (patched)


Can we add some validation somewhere to ensure that negative value scalars 
are not provided?


- Benjamin Mahler


On Jan. 29, 2019, 4:24 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69818/
> ---
> 
> (Updated Jan. 29, 2019, 4:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds new fields to `FrameworkInfo` which can be used by
> frameworks to configure static, per-role resource requirements. We
> currently support specifying minimal allocatable resources which can
> e.g., be interpreted by allocators to filter out resources not relevant
> to frameworks, or make allocations below globally configured minimal
> allocatable resources for particular frameworks.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3477b7db4dc64187aa55e8e4feb66a62f7951429 
>   include/mesos/v1/mesos.proto 692940bc862e61b9899aa159f29b662f3aade033 
>   src/common/resource_quantities.hpp 11eb426104577b7977c2307df3e4917085cd 
>   src/common/resource_quantities.cpp 320983929cd7d14973c4b98d6ed5338de690ff5f 
> 
> 
> Diff: https://reviews.apache.org/r/69818/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69855: Added an example of Seccomp profile.

2019-01-29 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 69855 was successfully built and tested.

Reviews applied: `['69855']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2827/mesos-review-69855

- Mesos Reviewbot Windows


On Jan. 29, 2019, 6:32 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69855/
> ---
> 
> (Updated Jan. 29, 2019, 6:32 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/examples/seccomp_default.json PRE-CREATION 
>   docs/isolators/linux-seccomp.md fa11feaeada2bcc6b9c782fa0e2ec8adad70e10f 
> 
> 
> Diff: https://reviews.apache.org/r/69855/diff/1/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 69818: Added offer filters resources to static framework configuration.

2019-01-29 Thread Benjamin Mahler 


> On Jan. 29, 2019, 3:09 a.m., Benjamin Mahler wrote:
> > include/mesos/mesos.proto
> > Lines 1519 (patched)
> > 
> >
> > I'm not sure if this gives much information to the reader, thoughts on 
> > the following?
> > 
> > ```
> > /**
> >  * Represents filters that allow a framework to control the
> >  * shape of offers that will be sent to its role(s). These
> >  * filters apply globally to any agent (unlike the existing
> >  * `DECLINE` filter, which is a time based resource subset
> >  * filter that only applies to the agent that was declined).
> >  */
> > ```
> 
> Benjamin Bannier wrote:
> I thought about something like that as well, but there is nothing in the 
> allocator interface enforcing it, e.g., an allocator could just ignore this 
> information.
> 
> I'd suggest to not make promises here where it isn't much more than a 
> container.

>From a practical standpoint, there are 0 alternative allocator implementations 
>(and that makes sense since it's not really the right interface point). We 
>already document a lot based on the assumption that an allocator (i.e ours) is 
>enforcing things like quota, drf, etc etc. So I think we should just assume it 
>and document accordingly.


- Benjamin


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


On Jan. 29, 2019, 4:24 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69818/
> ---
> 
> (Updated Jan. 29, 2019, 4:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds new fields to `FrameworkInfo` which can be used by
> frameworks to configure static, per-role resource requirements. We
> currently support specifying minimal allocatable resources which can
> e.g., be interpreted by allocators to filter out resources not relevant
> to frameworks, or make allocations below globally configured minimal
> allocatable resources for particular frameworks.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3477b7db4dc64187aa55e8e4feb66a62f7951429 
>   include/mesos/v1/mesos.proto 692940bc862e61b9899aa159f29b662f3aade033 
>   src/common/resource_quantities.hpp 11eb426104577b7977c2307df3e4917085cd 
>   src/common/resource_quantities.cpp 320983929cd7d14973c4b98d6ed5338de690ff5f 
> 
> 
> Diff: https://reviews.apache.org/r/69818/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 69855: Added an example of Seccomp profile.

2019-01-29 Thread Andrei Budnik 

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

Review request for mesos, Gilbert Song, Jie Yu, and Qian Zhang.


Repository: mesos


Description
---

See summary.


Diffs
-

  docs/examples/seccomp_default.json PRE-CREATION 
  docs/isolators/linux-seccomp.md fa11feaeada2bcc6b9c782fa0e2ec8adad70e10f 


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


Testing
---

None: not a functional change.


Thanks,

Andrei Budnik

Re: Review Request 69851: Added end-to-end test for operations affecting agent default resources.

2019-01-29 Thread Greg Mann 

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




src/tests/slave_tests.cpp
Lines 11436-11437 (patched)


Let's be explicit in this comment that we are verifying that the agent's 
checkpointing/recovery logic works.



src/tests/slave_tests.cpp
Lines 11437 (patched)


Add a period.



src/tests/slave_tests.cpp
Lines 11440 (patched)


s/Send/Sends/



src/tests/slave_tests.cpp
Lines 11441-11442 (patched)


This is incorrect, right? Looks like the test allows the update to go 
through but uses the acknowledgement to control the removal of the operation 
from the agent's state.



src/tests/slave_tests.cpp
Lines 11464 (patched)


Let's use `DEFAULT_TEST_ROLE` here.


- Greg Mann


On Jan. 28, 2019, 11:24 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69851/
> ---
> 
> (Updated Jan. 28, 2019, 11:24 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Greg Mann.
> 
> 
> Bugs: MESOS-9356
> https://issues.apache.org/jira/browse/MESOS-9356
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added end-to-end test for operations affecting agent default resources.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 9168e06348a83dcb20400a2fe0e3bb1f26e6ff1b 
> 
> 
> Diff: https://reviews.apache.org/r/69851/diff/1/
> 
> 
> Testing
> ---
> 
> `GLOG_v=1 bin/mesos-tests.sh --verbose 
> --gtest_filter="*Slave*RetryOperationStatusUpdateAfterRecovery*" 
> --gtest_repeat=5000 --gtest_break_on_failure` passed on CentOS 7.4.1708.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-01-29 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 69821 was successfully built and tested.

Reviews applied: `['69818', '69819', '69820', '69821']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2826/mesos-review-69821

- Mesos Reviewbot Windows


On Jan. 29, 2019, 4:20 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69821/
> ---
> 
> (Updated Jan. 29, 2019, 4:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the hierarchical allocator to take
> framework-specified minimal allocatable resources into account.
> 
> While previously the allocator was inspecting the minimal allocatable
> resources specified in its options, it now inspects a dynamically
> calculated set of minimal allocatable resources. This set is populated
> with any minimal allocatable resources specified in the allocator's
> options and any framework's minimal allocatable resources. For a
> resource to be allocatable it needs to contain any of these minimal
> requirements.
> 
> If a framework does not specify minimal allocatable resource
> requirements, its minimal requirements are set to the globally
> configured option.
> 
> We also adjust the allocator to take a framework's minimal resource
> requirements into account when checking whether a resource can be
> allocated to a particular framework. The check is performed at the same
> time we check whether a framework filtered a particular resource. This
> avoids offering resources to frameworks which the framework would never
> have considered allocatable itself (e.g., given the global option of the
> allocator).
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> ca1638390d89e2a81efd9d6d4a28b863c79723c4 
>   src/master/allocator/mesos/hierarchical.cpp 
> f1f3894058a8e3f008013cb269744bd36c0e31b3 
>   src/tests/hierarchical_allocator_tests.cpp 
> cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee 
> 
> 
> Diff: https://reviews.apache.org/r/69821/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69818: Added offer filters resources to static framework configuration.

2019-01-29 Thread Benjamin Bannier 

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

(Updated Jan. 29, 2019, 5:24 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Summary (updated)
-

Added offer filters resources to static framework configuration.


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


Repository: mesos


Description (updated)
---

This patch adds new fields to `FrameworkInfo` which can be used by
frameworks to configure static, per-role resource requirements. We
currently support specifying minimal allocatable resources which can
e.g., be interpreted by allocators to filter out resources not relevant
to frameworks, or make allocations below globally configured minimal
allocatable resources for particular frameworks.


Diffs (updated)
-

  include/mesos/mesos.proto 3477b7db4dc64187aa55e8e4feb66a62f7951429 
  include/mesos/v1/mesos.proto 692940bc862e61b9899aa159f29b662f3aade033 
  src/common/resource_quantities.hpp 11eb426104577b7977c2307df3e4917085cd 
  src/common/resource_quantities.cpp 320983929cd7d14973c4b98d6ed5338de690ff5f 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 69818: Added minimal allocatable resources to framework information.

2019-01-29 Thread Benjamin Bannier 


> On Jan. 29, 2019, 4:09 a.m., Benjamin Mahler wrote:
> > include/mesos/mesos.proto
> > Lines 1519 (patched)
> > 
> >
> > I'm not sure if this gives much information to the reader, thoughts on 
> > the following?
> > 
> > ```
> > /**
> >  * Represents filters that allow a framework to control the
> >  * shape of offers that will be sent to its role(s). These
> >  * filters apply globally to any agent (unlike the existing
> >  * `DECLINE` filter, which is a time based resource subset
> >  * filter that only applies to the agent that was declined).
> >  */
> > ```

I thought about something like that as well, but there is nothing in the 
allocator interface enforcing it, e.g., an allocator could just ignore this 
information.

I'd suggest to not make promises here where it isn't much more than a container.


- Benjamin


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


On Jan. 29, 2019, 5:20 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69818/
> ---
> 
> (Updated Jan. 29, 2019, 5:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds minimal allocatable resources to a framework's
> `FrameworkInfo`. We introduce a dedicated type to model allocatable
> resources which is composed of types loosly based on the internal
> `ResourceQuantity` class.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3477b7db4dc64187aa55e8e4feb66a62f7951429 
>   include/mesos/v1/mesos.proto 692940bc862e61b9899aa159f29b662f3aade033 
>   src/common/resource_quantities.hpp 11eb426104577b7977c2307df3e4917085cd 
>   src/common/resource_quantities.cpp 320983929cd7d14973c4b98d6ed5338de690ff5f 
> 
> 
> Diff: https://reviews.apache.org/r/69818/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69820: Added containment check for `ResourceQuantities`.

2019-01-29 Thread Benjamin Bannier 

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

(Updated Jan. 29, 2019, 5:20 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This patch adds `contains` function for `ResourceQuantities`. The
function has the same semantics as `Resources::contains`.


Diffs (updated)
-

  src/common/resource_quantities.hpp 11eb426104577b7977c2307df3e4917085cd 
  src/common/resource_quantities.cpp 320983929cd7d14973c4b98d6ed5338de690ff5f 
  src/tests/resource_quantities_tests.cpp 
435a4949b95e9a83be73781388eb4be9c7da695b 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 69821: Enforced minimal allocatable resources in the hierarchical allocator.

2019-01-29 Thread Benjamin Bannier 

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

(Updated Jan. 29, 2019, 5:20 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This patch modifies the hierarchical allocator to take
framework-specified minimal allocatable resources into account.

While previously the allocator was inspecting the minimal allocatable
resources specified in its options, it now inspects a dynamically
calculated set of minimal allocatable resources. This set is populated
with any minimal allocatable resources specified in the allocator's
options and any framework's minimal allocatable resources. For a
resource to be allocatable it needs to contain any of these minimal
requirements.

If a framework does not specify minimal allocatable resource
requirements, its minimal requirements are set to the globally
configured option.

We also adjust the allocator to take a framework's minimal resource
requirements into account when checking whether a resource can be
allocated to a particular framework. The check is performed at the same
time we check whether a framework filtered a particular resource. This
avoids offering resources to frameworks which the framework would never
have considered allocatable itself (e.g., given the global option of the
allocator).


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
ca1638390d89e2a81efd9d6d4a28b863c79723c4 
  src/master/allocator/mesos/hierarchical.cpp 
f1f3894058a8e3f008013cb269744bd36c0e31b3 
  src/tests/hierarchical_allocator_tests.cpp 
cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 69819: Added equality operator for `ResourceQuantities`.

2019-01-29 Thread Benjamin Bannier 

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

(Updated Jan. 29, 2019, 5:20 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This patch adds an `operator==` for `ResourceQuantities`. The operator
is declared as a `friend` of `ResourceQuantities` so we can directly
compare the two `ResourceQuantities::quantities` which can be less
expensive.


Diffs (updated)
-

  src/common/resource_quantities.hpp 11eb426104577b7977c2307df3e4917085cd 
  src/common/resource_quantities.cpp 320983929cd7d14973c4b98d6ed5338de690ff5f 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 69818: Added minimal allocatable resources to framework information.

2019-01-29 Thread Benjamin Bannier 

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

(Updated Jan. 29, 2019, 5:20 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Addressed comments from Ben.


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


Repository: mesos


Description
---

This patch adds minimal allocatable resources to a framework's
`FrameworkInfo`. We introduce a dedicated type to model allocatable
resources which is composed of types loosly based on the internal
`ResourceQuantity` class.


Diffs (updated)
-

  include/mesos/mesos.proto 3477b7db4dc64187aa55e8e4feb66a62f7951429 
  include/mesos/v1/mesos.proto 692940bc862e61b9899aa159f29b662f3aade033 
  src/common/resource_quantities.hpp 11eb426104577b7977c2307df3e4917085cd 
  src/common/resource_quantities.cpp 320983929cd7d14973c4b98d6ed5338de690ff5f 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 69680: Have master acknowledge operation updates of completed frameworks.

2019-01-29 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 69680 was successfully built and tested.

Reviews applied: `['69680']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2825/mesos-review-69680

- Mesos Reviewbot Windows


On Jan. 29, 2019, 1:21 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69680/
> ---
> 
> (Updated Jan. 29, 2019, 1:21 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Greg Mann.
> 
> 
> Bugs: MESOS-9434
> https://issues.apache.org/jira/browse/MESOS-9434
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After a framework was removed and has unacknowledged operations status
> updates, it was impossible to remove terminal operations as nobody could
> acknowledge them.
> 
> In this patch we make the master acknowledge operation status updates
> for frameworks it knows are removed so that e.g., terminal operations
> can be removed. Since masters do not persist completed frameworks this
> is not reliable (e.g., an agent was partitioned for a long time and
> still tracks a completed framework's `FrameworkInfo`, and comes back
> only after the master knowing about the framework's completion has
> failed over). We merely extend the existing master behavior (e.g., send
> `ShutdownFrameworkMessage` to all currently registered agents) to
> operations.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 2e0a0de4d745112011370741b8cedbb6db17b915 
>   src/tests/master_tests.cpp 1f8da63aa313c4679c0d7a3934802af1474e8f28 
> 
> 
> Diff: https://reviews.apache.org/r/69680/diff/2/
> 
> 
> Testing
> ---
> 
> * `make check`
> * tested on a number of configurations in internal CI
> * ran added test in repetition, both with and without additional stress
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69854: Set status update UUID in MockResourceProvider.

2019-01-29 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 69854 was successfully built and tested.

Reviews applied: `['69854']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2824/mesos-review-69854

- Mesos Reviewbot Windows


On Jan. 29, 2019, 1:20 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69854/
> ---
> 
> (Updated Jan. 29, 2019, 1:20 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set status update UUID in MockResourceProvider.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp c2a5e5531de7498241e537ef1699e1a5e669b550 
> 
> 
> Diff: https://reviews.apache.org/r/69854/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69680: Have master acknowledge operation updates of completed frameworks.

2019-01-29 Thread Benjamin Bannier 


> On Jan. 28, 2019, 9:37 p.m., Greg Mann wrote:
> > src/tests/master_tests.cpp
> > Lines 9469-9491 (patched)
> > 
> >
> > Can you use the mock RP's `operationDefault` method to accomplish this?
> > 
> > 
> > https://github.com/apache/mesos/blob/d838f2958e18c1a75594ca4f10df132670fcd11e/src/tests/master_tests.cpp#L9200-L9201

This mock method needed to send a update UUID which I added in 
https://reviews.apache.org/r/69854/.

I also tried to make this us a non-speculated operation like `CREATE_DISK` 
instead of `RESERVE` which caused me to run into 
https://issues.apache.org/jira/browse/MESOS-9542.


- Benjamin


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


On Jan. 29, 2019, 2:21 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69680/
> ---
> 
> (Updated Jan. 29, 2019, 2:21 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Greg Mann.
> 
> 
> Bugs: MESOS-9434
> https://issues.apache.org/jira/browse/MESOS-9434
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After a framework was removed and has unacknowledged operations status
> updates, it was impossible to remove terminal operations as nobody could
> acknowledge them.
> 
> In this patch we make the master acknowledge operation status updates
> for frameworks it knows are removed so that e.g., terminal operations
> can be removed. Since masters do not persist completed frameworks this
> is not reliable (e.g., an agent was partitioned for a long time and
> still tracks a completed framework's `FrameworkInfo`, and comes back
> only after the master knowing about the framework's completion has
> failed over). We merely extend the existing master behavior (e.g., send
> `ShutdownFrameworkMessage` to all currently registered agents) to
> operations.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 2e0a0de4d745112011370741b8cedbb6db17b915 
>   src/tests/master_tests.cpp 1f8da63aa313c4679c0d7a3934802af1474e8f28 
> 
> 
> Diff: https://reviews.apache.org/r/69680/diff/2/
> 
> 
> Testing
> ---
> 
> * `make check`
> * tested on a number of configurations in internal CI
> * ran added test in repetition, both with and without additional stress
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 69854: Set status update UUID in MockResourceProvider.

2019-01-29 Thread Benjamin Bannier 

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

Review request for mesos, Greg Mann and Jan Schlicht.


Repository: mesos


Description
---

Set status update UUID in MockResourceProvider.


Diffs
-

  src/tests/mesos.hpp c2a5e5531de7498241e537ef1699e1a5e669b550 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 69680: Have master acknowledge operation updates of completed frameworks.

2019-01-29 Thread Benjamin Bannier 

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

(Updated Jan. 29, 2019, 2:21 p.m.)


Review request for mesos, Gastón Kleiman and Greg Mann.


Changes
---

Addressed Greg's comments.


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


Repository: mesos


Description
---

After a framework was removed and has unacknowledged operations status
updates, it was impossible to remove terminal operations as nobody could
acknowledge them.

In this patch we make the master acknowledge operation status updates
for frameworks it knows are removed so that e.g., terminal operations
can be removed. Since masters do not persist completed frameworks this
is not reliable (e.g., an agent was partitioned for a long time and
still tracks a completed framework's `FrameworkInfo`, and comes back
only after the master knowing about the framework's completion has
failed over). We merely extend the existing master behavior (e.g., send
`ShutdownFrameworkMessage` to all currently registered agents) to
operations.


Diffs (updated)
-

  src/master/master.cpp 2e0a0de4d745112011370741b8cedbb6db17b915 
  src/tests/master_tests.cpp 1f8da63aa313c4679c0d7a3934802af1474e8f28 


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

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


Testing
---

* `make check`
* tested on a number of configurations in internal CI
* ran added test in repetition, both with and without additional stress


Thanks,

Benjamin Bannier