date:20180423

Re: Review Request 66293: Tested default executor support of `max_completion_time`.

2018-04-23 Thread Mesos Reviewbot Windows


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



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

Reviews applied: `['66258', '66591', '66259', '66260', '66283', '66284', 
'66291', '66293']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66293

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66293/logs/mesos-tests-stdout.log):

```
[ RUN  ] ContentType/SchedulerTest.KillTask/1
[   OK ] ContentType/SchedulerTest.KillTask/1 (14927 ms)
[ RUN  ] 
ContentType/SchedulerTest.OperationFeedbackValidationWithResourceProviderCapability/0
[   OK ] 
ContentType/SchedulerTest.OperationFeedbackValidationWithResourceProviderCapability/0
 (14682 ms)
[ RUN  ] 
ContentType/SchedulerTest.OperationFeedbackValidationWithResourceProviderCapability/1
[   OK ] 
ContentType/SchedulerTest.OperationFeedbackValidationWithResourceProviderCapability/1
 (14627 ms)
[ RUN  ] 
ContentType/SchedulerTest.OperationFeedbackValidationNoResourceProviderCapability/0
[   OK ] 
ContentType/SchedulerTest.OperationFeedbackValidationNoResourceProviderCapability/0
 (14801 ms)
[ RUN  ] 
ContentType/SchedulerTest.OperationFeedbackValidationNoResourceProviderCapability/1
[   OK ] 
ContentType/SchedulerTest.OperationFeedbackValidationNoResourceProviderCapability/1
 (14179 ms)
[ RUN  ] 
ContentType/SchedulerTest.OperationFeedbackValidationSchedulerDriverFramework/0
[   OK ] 
ContentType/SchedulerTest.OperationFeedbackValidationSchedulerDriverFramework/0 
(14767 ms)
[ RUN  ] 
ContentType/SchedulerTest.OperationFeedbackValidationSchedulerDriverFramework/1
[   OK ] 
ContentType/SchedulerTest.OperationFeedbackValidationSchedulerDriverFramework/1 
(14841 ms)
[ RUN  ] ContentType/SchedulerTest.ShutdownExecutor/0
[   OK ] ContentType/SchedulerTest.ShutdownExecutor/0 (14660 ms)
[ RUN  ] ContentType/SchedulerTest.ShutdownExecutor/1
[   OK ] ContentType/SchedulerTest.ShutdownExecutor/1 (14903 ms)
[ RUN  ] ContentType/SchedulerTest.Decline/0
[   OK ] ContentType/SchedulerTest.Decline/0 (14539 ms)
[ RUN  ] ContentType/SchedulerTest.Decline/1
[   OK ] ContentType/SchedulerTest.Decline/1 (14427 ms)
[ RUN  ] ContentType/SchedulerTest.Revive/0
[   OK ] ContentType/SchedulerTest.Revive/0 (14650 ms)
[ RUN  ] ContentType/SchedulerTest.Revive/1
[   OK ] ContentType/SchedulerTest.Revive/1 (14386 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/0
[   OK ] ContentType/SchedulerTest.Suppress/0 (14642 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/1
```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66293/logs/mesos-tests-stderr.log):

```
I0424 05:04:34.089884  8184 master.cpp:1411] Framework 
364e13be-0829-4622-a1d3-aafe15544912- (default) disconnected
I0424 05:04:34.089884  8184 master.cpp:3259] Deactivating framework 
364e13be-0829-4622-a1d3-aafe15544912- (default)
I0424 05:04:34.090879 13968 hierarchical.cpp:405] Deactivated framework 
364e13be-0829-4622-a1d3-aafe15544912-
W0424 05:04:34.091893  8184 master.hpp:2342] Unable to send event to framework 
364e13be-0829-4622-a1d3-aafe15544912- (default): connection closed
I0424 05:04:34.091893  8184 master.cpp:11069] Removing offer 
364e13be-0829-4622-a1d3-aafe15544912-O1
I0424 05:04:34.091893  8184 master.cpp:3236] Disconnecting framework 
364e13be-0829-4622-a1d3-aafe15544912- (default)
I0424 05:04:34.092893  8184 master.cpp:1426] Giving framework 
364e13be-0829-4622-a1d3-aafe15544912- (default) 0ns to failover
I0424 05:04:34.094152 12436 master.cpp:8935] Framework failover timeout, 
removing framework 364e13be-0829-4622-a1d3-aafe15544912- (default)
I0424 05:04:34.094152 12436 master.cpp:9829] Removing framework 
364e13be-0829-4622-a1d3-aafe15544912- (default)
W0424 05:04:34.094152 11476 slave.cpp:3902] Ignoring shutdown framework message 
for 364e13be-0829-4622-a1d3-aafe15544912- because the agent has not yet 
registered with the master
I0424 05:04:34.094894  7956 hierarchical.cpp:344] Removed framework 
364e13be-0829-4622-a1d3-aafe15544912-
I0424 05:04:34.096894 15552 slave.cpp:919] Agent terminating
I0424 05:04:34.096894  7292 master.cpp:1296] Agent 
364e13be-0829-4622-a1d3-aafe15544912-S0 at slave(419)@172.27.128.1:59222 
(winbldsrv-02) disconnected
I0424 05:04:34.096894  7292 master.cpp:3296] Disconnecting agent 
364e13be-0829-4622-a1d3-aafe15544912-S0 at slave(419)@172.27.128.1:59222 
(winbldsrv-02)
I0424 05:04:34.097995  7292 master.cpp:3315] Deactivating agent 
364e13be-0829-4622-a1d3-aafe15544912-S0 at slave(419)@172.27.128.1:59222 
(winbldsrv-02)
I0424 05:04:34.098883 11476

Re: Review Request 66749: Added more logging to agent recovery path.

2018-04-23 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [66749]

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

- Mesos Reviewbot


On April 24, 2018, 12:30 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66749/
> ---
> 
> (Updated April 24, 2018, 12:30 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-8793
> https://issues.apache.org/jira/browse/MESOS-8793
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added logging in some agent recovery continuations to
> make analyzing agent recovery related issue less painful.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> 186102c66d373dcd799cadd9fed7d1c8cb894971 
>   src/slave/containerizer/docker.cpp a4c9c10e91e75f406329cbb2086f39b549cbeed0 
>   src/slave/containerizer/mesos/containerizer.cpp 
> def09f1104213bf73d9f95cb5ad2fb80e3bdb04a 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> af34a856e092a880a0809da34ead9d8588b0ac8f 
>   src/slave/slave.cpp 9d2d1928b231044988f1855eb518448db38ff04f 
> 
> 
> Diff: https://reviews.apache.org/r/66749/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66749: Added more logging to agent recovery path.

2018-04-23 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 66749 was successfully built and tested.

Reviews applied: `['66749']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66749

- Mesos Reviewbot Windows


On April 24, 2018, 12:30 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66749/
> ---
> 
> (Updated April 24, 2018, 12:30 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-8793
> https://issues.apache.org/jira/browse/MESOS-8793
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added logging in some agent recovery continuations to
> make analyzing agent recovery related issue less painful.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> 186102c66d373dcd799cadd9fed7d1c8cb894971 
>   src/slave/containerizer/docker.cpp a4c9c10e91e75f406329cbb2086f39b549cbeed0 
>   src/slave/containerizer/mesos/containerizer.cpp 
> def09f1104213bf73d9f95cb5ad2fb80e3bdb04a 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> af34a856e092a880a0809da34ead9d8588b0ac8f 
>   src/slave/slave.cpp 9d2d1928b231044988f1855eb518448db38ff04f 
> 
> 
> Diff: https://reviews.apache.org/r/66749/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66769: Fixed flaky ReconciliationTest.ReconcileStatusUpdateTaskState.

2018-04-23 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [66769]

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

- Mesos Reviewbot


On April 23, 2018, 10:23 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66769/
> ---
> 
> (Updated April 23, 2018, 10:23 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Alexander Rukletsov, and Megha 
> Sharma.
> 
> 
> Bugs: MESOS-8618
> https://issues.apache.org/jira/browse/MESOS-8618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To simulate a master failover we need to use `replicated_log` as the
> registry otherwise the master loses persisted info about the agents.
> 
> 
> Diffs
> -
> 
>   src/tests/reconciliation_tests.cpp b06244b5f3efad3a88e367cd8e26cebd3d9f2e43 
> 
> 
> Diff: https://reviews.apache.org/r/66769/diff/1/
> 
> 
> Testing
> ---
> 
> Make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 66769: Fixed flaky ReconciliationTest.ReconcileStatusUpdateTaskState.

2018-04-23 Thread Mesos Reviewbot Windows


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



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

Reviews applied: `['66769']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66769

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66769/logs/mesos-tests-stdout.log):

```
[ RUN  ] 
ContentType/SchedulerTest.OperationFeedbackValidationNoResourceProviderCapability/1
[   OK ] 
ContentType/SchedulerTest.OperationFeedbackValidationNoResourceProviderCapability/1
 (14582 ms)
[ RUN  ] 
ContentType/SchedulerTest.OperationFeedbackValidationSchedulerDriverFramework/0
[   OK ] 
ContentType/SchedulerTest.OperationFeedbackValidationSchedulerDriverFramework/0 
(14634 ms)
[ RUN  ] 
ContentType/SchedulerTest.OperationFeedbackValidationSchedulerDriverFramework/1
[   OK ] 
ContentType/SchedulerTest.OperationFeedbackValidationSchedulerDriverFramework/1 
(14614 ms)
[ RUN  ] ContentType/SchedulerTest.ShutdownExecutor/0
[   OK ] ContentType/SchedulerTest.ShutdownExecutor/0 (14296 ms)
[ RUN  ] ContentType/SchedulerTest.ShutdownExecutor/1
[   OK ] ContentType/SchedulerTest.ShutdownExecutor/1 (14701 ms)
[ RUN  ] ContentType/SchedulerTest.Decline/0
[   OK ] ContentType/SchedulerTest.Decline/0 (14596 ms)
[ RUN  ] ContentType/SchedulerTest.Decline/1
[   OK ] ContentType/SchedulerTest.Decline/1 (14588 ms)
[ RUN  ] ContentType/SchedulerTest.Revive/0
[   OK ] ContentType/SchedulerTest.Revive/0 (14445 ms)
[ RUN  ] ContentType/SchedulerTest.Revive/1
[   OK ] ContentType/SchedulerTest.Revive/1 (14773 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/0
[   OK ] ContentType/SchedulerTest.Suppress/0 (14383 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/1
[   OK ] ContentType/SchedulerTest.Suppress/1 (14499 ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0 (19771 
ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1 (19163 
ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0
[   OK ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0 
(19115 ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/1
```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66769/logs/mesos-tests-stderr.log):

```
I0424 02:07:55.174295  5316 master.cpp:515] Master only allowing authenticated 
frameworks to register
I0424 02:07:55.174295  5316 master.cpp:521] Master only allowing authenticated 
agents to register
I0424 02:07:55.174295  5316 master.cpp:527] Master only allowing authenticated 
HTTP frameworks to register
I0424 02:07:55.175309  5316 credentials.hpp:37] Loading credentials for 
authentication from 'C:\Users\mesos\AppData\Local\Temp\v9N7gu\credentials'
I0424 02:07:55.176336  5316 master.cpp:571] Using default 'crammd5' 
authenticator
I0424 02:07:55.177335  5316 http.cpp:959] Creating default 'basic' HTTP 
authenticator for realm 'mesos-master-readonly'
I0424 02:07:55.178337  5316 http.cpp:959] Creating default 'basic' HTTP 
authenticator for realm 'mesos-master-readwrite'
I0424 02:07:55.178337  5316 http.cpp:959] Creating default 'basic' HTTP 
authenticator for realm 'mesos-master-scheduler'
I0424 02:07:55.179302  5316 master.cpp:652] Authorization enabled
I0424 02:07:55.190335  9356 master.cpp:2127] Elected as the leading master!
I0424 02:07:55.190335  9356 master.cpp:1683] Recovering from registrar
I0424 02:07:55.192330 11052 registrar.cpp:383] Successfully fetched the 
registry (0B) in 995072ns
I0424 02:07:55.192330 11052 registrar.cpp:487] Applied 1 operations in 0ns; 
attempting to update the registry
I0424 02:07:55.195006 10084 registrar.cpp:544] Successfully updated the 
registry in 1.068032ms
I0424 02:07:55.195006 10084 registrar.cpp:416] Successfully recovered registrar
I0424 02:07:55.195991  8460 master.cpp:1797] Recovered 0 agents from the 
registry (139B); allowing 10mins for agents to reregister
W0424 02:07:59.792044  4876 process.cpp:2821] Attempted to spawn already 
running process files@172.27.128.1:56750
I0424 02:07:59.794049  4876 containerizer.cpp:296] Using isolation { 
windows/cpu, filesystem/windows, windows/mem, environment_secret }
I0424 02:07:59.797027  4876 provisioner.cpp:299] Using default backend 'copy'
I0424 02:07:59.803056  4876 cluster.cpp:460] Creating default 'local' authorizer
I0424 02:07:59.810024 15748 slave.cpp:261] Mesos agent started on

Re: Review Request 65995: Declined unwanted offers in `RetryOperationStatusUpdate*` SLRP tests.

2018-04-23 Thread Chun-Hung Hsiao


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

(Updated April 24, 2018, 1:12 a.m.)


Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jie 
Yu.


Changes
---

Addressed Benjamin's comment and removed unnecessary `std::bind`.


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


Repository: mesos


Description
---

The two SLRP tests assume that SLRP will send out a RAW resource in its
first `UPDATE_STATE` message, and expect that the test framework would
receive an offer containing the RAW resource in its first offer. However
this behavior is not guaranteed and should not be relied on. This patch
makes the tests decline unwanted offers by default so they no longer
rely on SLRP's internal behavior.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
ccb114aacc904998dfeef4128f6d38dfb3c865c7 


Diff: https://reviews.apache.org/r/65995/diff/6/

Changes: https://reviews.apache.org/r/65995/diff/5-6/


Testing
---

sudo make check
Ran the two tests in repitition.


Thanks,

Chun-Hung Hsiao

Re: Review Request 65594: Checkpointed profiles in storage local resource provider.

2018-04-23 Thread Chun-Hung Hsiao


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

(Updated April 24, 2018, 1:06 a.m.)


Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Joseph 
Wu.


Changes
---

Addressed Benjamin's comments.


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


Repository: mesos


Description
---

SLRP now checkpoints profiles associated with storage pools, and does
not depend on the `DiskProfileAdaptor` module to return the set of
previously-known profiles during recovery.


Diffs (updated)
-

  src/resource_provider/state.proto 8577b58b8cdb63b3daddf06ea5d12f80f9d42f2b 
  src/resource_provider/storage/provider.cpp 
8ca2d3a98858940e6e027becefb53c2f00b4ae43 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 65974: Added comments and made some renaming in SLRP.

2018-04-23 Thread Chun-Hung Hsiao


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

(Updated April 24, 2018, 1:04 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Fixed the type pointed out by Benjamin.


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


Repository: mesos


Description
---

Added comments and made some renaming in SLRP.


Diffs (updated)
-

  src/resource_provider/storage/provider.cpp 
8ca2d3a98858940e6e027becefb53c2f00b4ae43 


Diff: https://reviews.apache.org/r/65974/diff/4/

Changes: https://reviews.apache.org/r/65974/diff/3-4/


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 65594: Checkpointed profiles in storage local resource provider.

2018-04-23 Thread Chun-Hung Hsiao



> On April 20, 2018, 12:57 p.m., Benjamin Bannier wrote:
> > src/resource_provider/storage/provider.cpp
> > Lines 3276 (patched)
> > 
> >
> > Should we capture this?
> > 
> > ProfileInfo& profile_ = *(storage->mutable_profiles())[profile];
> > 
> > No strong opinion, it seems hard to make interacting with proto maps 
> > nice.
> 
> Chun-Hung Hsiao wrote:
> I'll do `ProfileInfo& profileInfo_ = ...`.
> 
> Chun-Hung Hsiao wrote:
> BTW the deference operator (`*`) applies on `mutable_profiles()` not `[]`.

Typo in the above comment: `s/mutable_profiles/mutable_capability/`.


- Chun-Hung


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


On April 12, 2018, 3:35 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65594/
> ---
> 
> (Updated April 12, 2018, 3:35 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> SLRP now checkpoints profiles associated with storage pools, and does
> not depend on the `DiskProfileAdaptor` module to return the set of
> previously-known profiles during recovery.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/state.proto 8577b58b8cdb63b3daddf06ea5d12f80f9d42f2b 
>   src/resource_provider/storage/provider.cpp 
> a07620d1c4bf618f669575b3e183bf598da905a6 
> 
> 
> Diff: https://reviews.apache.org/r/65594/diff/8/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65594: Checkpointed profiles in storage local resource provider.

2018-04-23 Thread Chun-Hung Hsiao



> On April 20, 2018, 12:57 p.m., Benjamin Bannier wrote:
> > src/resource_provider/storage/provider.cpp
> > Lines 3276 (patched)
> > 
> >
> > Should we capture this?
> > 
> > ProfileInfo& profile_ = *(storage->mutable_profiles())[profile];
> > 
> > No strong opinion, it seems hard to make interacting with proto maps 
> > nice.
> 
> Chun-Hung Hsiao wrote:
> I'll do `ProfileInfo& profileInfo_ = ...`.

BTW the deference operator (`*`) applies on `mutable_profiles()` not `[]`.


- Chun-Hung


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


On April 12, 2018, 3:35 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65594/
> ---
> 
> (Updated April 12, 2018, 3:35 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> SLRP now checkpoints profiles associated with storage pools, and does
> not depend on the `DiskProfileAdaptor` module to return the set of
> previously-known profiles during recovery.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/state.proto 8577b58b8cdb63b3daddf06ea5d12f80f9d42f2b 
>   src/resource_provider/storage/provider.cpp 
> a07620d1c4bf618f669575b3e183bf598da905a6 
> 
> 
> Diff: https://reviews.apache.org/r/65594/diff/8/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65594: Checkpointed profiles in storage local resource provider.

2018-04-23 Thread Chun-Hung Hsiao



> On April 20, 2018, 12:57 p.m., Benjamin Bannier wrote:
> > src/resource_provider/storage/provider.cpp
> > Lines 1003 (patched)
> > 
> >
> > Even thought an `auto` might make sense in such a place _in general_, 
> > due to the interface of `MapInfo` it leads to pretty dense code and maybe 
> > repeating a type here might make this easier to read. How about
> > 
> > // NOTE: this form requires a `using` decl above.
> > using Profile =
> >   MapPair;
> >   
> > foreach (const Profile& profile, storage.profiles()) {
> >   profileInfos.put(
> >   profile.first,
> >   {profile.second.capability(), profile.second.parameters()});
> > }

Adopted your suggestion, but I use `ProfileEntry` instead of `Profile` and keep 
the variable name `entry`. I'd like to make it consistent across the code that 
`profile` indicates a name, `profileInfo` indicates the CSI data, and `entry` 
indicates a map pair.


> On April 20, 2018, 12:57 p.m., Benjamin Bannier wrote:
> > src/resource_provider/storage/provider.cpp
> > Line 1010 (original), 1011 (patched)
> > 
> >
> > Nit: _pending operations_ should be specific enough as this list is 
> > neither exhaustive today nor will remain in the future.
> 
> Chun-Hung Hsiao wrote:
> I'm specifically talking about `CREATE_VOLUME` and `CREATE_BLOCK` here. 
> `DESTROY_VOLUME` and `DESTROY_BLOCK` don't need the profile info.

Removed them from the comments as suggested as the explicitness here doesn't 
bring much more clarity.


> On April 20, 2018, 12:57 p.m., Benjamin Bannier wrote:
> > src/resource_provider/storage/provider.cpp
> > Lines 3276 (patched)
> > 
> >
> > Should we capture this?
> > 
> > ProfileInfo& profile_ = *(storage->mutable_profiles())[profile];
> > 
> > No strong opinion, it seems hard to make interacting with proto maps 
> > nice.

I'll do `ProfileInfo& profileInfo_ = ...`.


- Chun-Hung


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


On April 12, 2018, 3:35 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65594/
> ---
> 
> (Updated April 12, 2018, 3:35 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> SLRP now checkpoints profiles associated with storage pools, and does
> not depend on the `DiskProfileAdaptor` module to return the set of
> previously-known profiles during recovery.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/state.proto 8577b58b8cdb63b3daddf06ea5d12f80f9d42f2b 
>   src/resource_provider/storage/provider.cpp 
> a07620d1c4bf618f669575b3e183bf598da905a6 
> 
> 
> Diff: https://reviews.apache.org/r/65594/diff/8/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66746: Replaced protobuf-specific comparators with MessageDifferencer.

2018-04-23 Thread Benjamin Mahler


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



I'm not sure this approach is tenable, we may in the future have two different 
`foo` fields with differing equality semantics.

I was expecting that this patch would add an equality operator for specific 
types we want to check. Do we need the `Message&` level equality operator?

- Benjamin Mahler


On April 20, 2018, 8:19 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66746/
> ---
> 
> (Updated April 20, 2018, 8:19 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MessageDifferencer utility has been available since protobuf 3.0.0.
> When comparing most repeated fields, we treat them as sets so the order
> of elements doesn't matter. In some exceptional cases, we treat them as
> list so the order becomes important. Further, fields with default
> purposes are considered to be set for comparison purposes.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp 19ea81716496bcc0117a1b0ff157a0374f38bbfa 
>   include/mesos/v1/mesos.hpp fda3eb42061f820869a2d8da939fccadc4e5ddfb 
>   src/common/type_utils.cpp 33d63809b61a18e03ff745c88f024c71dd221ca2 
>   src/tests/master_allocator_tests.cpp 
> e1aef8a9625a805e7ad2dfad37bfeedee82f160d 
>   src/v1/mesos.cpp 9b2df2dd798dff24a91a2604ab53c7fbb5ecfbcf 
> 
> 
> Diff: https://reviews.apache.org/r/66746/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` currently fails with:
> DefaultExecutorCheckTest.CommandCheckSharesWorkDirWithTask
> DefaultExecutorCheckTest.MultipleTasksWithChecks
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 66666: Added MemoryProfiler to CHANGELOG.

2018-04-23 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [63370, 63372, 6]

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

- Mesos Reviewbot


On April 23, 2018, 11:09 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6/
> ---
> 
> (Updated April 23, 2018, 11:09 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added changelog entry for the MemoryProfiler class that was
> added to libprocess.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 09fff1d50504de158b6c77f51bd7968746e31371 
> 
> 
> Diff: https://reviews.apache.org/r/6/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 66445: Windows: Cleaned up included CRT headers.

2018-04-23 Thread Joseph Wu


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


Ship it!




Ship It!

- Joseph Wu


On April 6, 2018, 4:18 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66445/
> ---
> 
> (Updated April 6, 2018, 4:18 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8759
> https://issues.apache.org/jira/browse/MESOS-8759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The set `errno` value in `os::kill()` is never checked (especially on
> Windows), so `_set_errno()` and thus `errno.h` were removed.
> 
> The `fcntl.h` is used only to provide `O_CREAT` etc., and so belonged
> in `open.hpp`, not `windows.hpp`.
> 
> The remaining headers, `direct.h`, `io.h`, `process.h`, and `stdlib.h`
> were no longer used or needed as the respective CRT APIs were
> replaced.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/kill.hpp 
> 9cec1117fac3cf6bd624fc7db524ef1ad10cd55d 
>   3rdparty/stout/include/stout/os/windows/mkdtemp.hpp 
> 9181429383a991fe2b87701d2bfd0e858ac2537b 
>   3rdparty/stout/include/stout/os/windows/open.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> a2926dab2c8e219cf5938c4df27f83488198dd6b 
>   3rdparty/stout/include/stout/os/windows/sendfile.hpp 
> fff5872db6b3f69464e41e6d108a107e4eeabd12 
>   3rdparty/stout/include/stout/windows.hpp 
> 1bfcdf4a5c097cc6d2293396ce39c8ad2c9ec993 
>   3rdparty/stout/include/stout/windows/dynamiclibrary.hpp 
> 5b3cbf4f36ea9ac0411df52b4cfea8ef75fecbb5 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 900baf9be4e3089cb43e444b14b155a80bcd1591 
> 
> 
> Diff: https://reviews.apache.org/r/66445/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66291: Added support to `max_completion_time` in default executor.

2018-04-23 Thread Zhitao Li


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

(Updated April 23, 2018, 4:56 p.m.)


Review request for mesos, Jason Lai and James Peach.


Changes
---

Use `killedByCompletionTimeout` to indicate kill reason, and refactor `kill()` 
to reuse a grace period.


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


Repository: mesos


Description
---

When a task group has multiple tasks:
- each task can have its own `max_completion_time`, or not have one;
- if a task succeeds before its `max_completion_time`, all other tasks
will keep running;
- if a task fails, all other tasks in the same group will fail (as
before);
- if a task does not succeed before its `max_completion_time`, it will
fail with `TASK_FAILED` and reason `REASON_MAX_COMPLETION_TIME_REACHED`,
while other tasks will be killed without above reason.


Diffs (updated)
-

  src/launcher/default_executor.cpp ea0d425bb60e970f209f411632e1d486c279259b 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66608: Improved support for move-only types in `hashmap`.

2018-04-23 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 66608 was successfully built and tested.

Reviews applied: `['66608']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66608

- Mesos Reviewbot Windows


On April 23, 2018, 9:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66608/
> ---
> 
> (Updated April 23, 2018, 9:52 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While it was already possible to create a `hashmap` over move-only
> values, we still performed a copy in `put`, making it hard to
> dynamically add elements with the expected stout semantics.
> 
> This patch relaxes the requirements on the value argument to `put` so
> that instead of copyable we now only require move-constructible types.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/hashmap.hpp 
> 91085b8d8ad5d35c39c8cc95e3d4765d82d9a8db 
> 
> 
> Diff: https://reviews.apache.org/r/66608/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66568: Dropped combined operations with GROW and SHRINK volumes.

2018-04-23 Thread Zhitao Li



> On April 23, 2018, 3:43 p.m., Greg Mann wrote:
> > src/master/master.cpp
> > Line 4512 (original), 4515-4527 (patched)
> > 
> >
> > I was just wondering - perhaps this code belongs in the validation 
> > function for the ACCEPT call? Since this is something that we can easily 
> > confirm synchronously in the top-level scheduler API handler and return 
> > 400, rather than calling into the accept handlers.
> 
> Zhitao Li wrote:
> I'm fine in either case as long as it provides better consistency and 
> less code duplication (which I think is a problem). I guess the reason there 
> is a split is due to that AuthZ is async right now.
> 
> I actualy wonder what happens if we move the slave connectivity checks to 
> `Master::accept()`: since master to agent connection through network can be 
> broken at any moment, I do not see checking that in `_accept()` is 100% safe 
> anyway, and `send()` calls to agent need to handle failures anyway.
> 
> Alternatively, we can consider moving authorization checks as early as we 
> can, and condense all other validations after AuthZ returned (assuming AuthZ 
> do not depends on these validations).

(replied too fast) I feel both work can reduce the code duplicate, and makes it 
less ambiguous for future development.


- Zhitao


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


On April 23, 2018, 1:07 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66568/
> ---
> 
> (Updated April 23, 2018, 1:07 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These two operations are intended to be non-speculative eventually but
> are implemented as speculative right now. To avoid frameworks opt-in to
> dangerous behavior, we require that accept can only contain one
> `GROW_VOLUME` or `SHRINK_VOLUME` and no other operations.
> 
> This is implemented by reuse existing code which already drops `LAUNCH`
> or `LAUNCH_GROUP` with proper reason and message.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 
> 
> 
> Diff: https://reviews.apache.org/r/66568/diff/5/
> 
> 
> Testing
> ---
> 
> Behavior tested in https://reviews.apache.org/r/66569/.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66568: Dropped combined operations with GROW and SHRINK volumes.

2018-04-23 Thread Zhitao Li



> On April 23, 2018, 3:43 p.m., Greg Mann wrote:
> > src/master/master.cpp
> > Line 4512 (original), 4515-4527 (patched)
> > 
> >
> > I was just wondering - perhaps this code belongs in the validation 
> > function for the ACCEPT call? Since this is something that we can easily 
> > confirm synchronously in the top-level scheduler API handler and return 
> > 400, rather than calling into the accept handlers.

I'm fine in either case as long as it provides better consistency and less code 
duplication (which I think is a problem). I guess the reason there is a split 
is due to that AuthZ is async right now.

I actualy wonder what happens if we move the slave connectivity checks to 
`Master::accept()`: since master to agent connection through network can be 
broken at any moment, I do not see checking that in `_accept()` is 100% safe 
anyway, and `send()` calls to agent need to handle failures anyway.

Alternatively, we can consider moving authorization checks as early as we can, 
and condense all other validations after AuthZ returned (assuming AuthZ do not 
depends on these validations).


- Zhitao


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


On April 23, 2018, 1:07 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66568/
> ---
> 
> (Updated April 23, 2018, 1:07 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These two operations are intended to be non-speculative eventually but
> are implemented as speculative right now. To avoid frameworks opt-in to
> dangerous behavior, we require that accept can only contain one
> `GROW_VOLUME` or `SHRINK_VOLUME` and no other operations.
> 
> This is implemented by reuse existing code which already drops `LAUNCH`
> or `LAUNCH_GROUP` with proper reason and message.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 
> 
> 
> Diff: https://reviews.apache.org/r/66568/diff/5/
> 
> 
> Testing
> ---
> 
> Behavior tested in https://reviews.apache.org/r/66569/.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66444: Windows: Made `signals.hpp` compile.

2018-04-23 Thread Joseph Wu



> On April 4, 2018, 10:22 a.m., Andrew Schwartzmeyer wrote:
> > I think our other option is to just delete this file, and have 
> > `os/signals.hpp` include the POSIX version, but not the (deleted) Windows 
> > version, with a comment that none of it is (or will be) implemented.

I'm in favor of this option.  There are only a few places in the code that use 
signals, but most of these are restricted to linux code only (the notable 
exception being glog).  We don't gain anything by leaving a bunch of 
unimplemented stubs while ifdef-ing all occurences of said stubs.


- Joseph


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


On April 3, 2018, 10:58 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66444/
> ---
> 
> (Updated April 3, 2018, 10:58 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8682
> https://issues.apache.org/jira/browse/MESOS-8682
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This file had never been included before, so it didn't compile. It
> needed to include `unimplemented.hpp`, and because `Suppressor` isn't
> implemented, the initializers had to be deleted.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/signals.hpp 
> 0ed24771625e58c1de8b1aa96b70f5aae1638bd4 
> 
> 
> Diff: https://reviews.apache.org/r/66444/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66443: Fixed `Subprocess::ChildHook::CHDIR()` to use `os::chdir()`.

2018-04-23 Thread Joseph Wu


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


Ship it!




Ship It!

- Joseph Wu


On April 3, 2018, 10:58 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66443/
> ---
> 
> (Updated April 3, 2018, 10:58 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8682
> https://issues.apache.org/jira/browse/MESOS-8682
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This needed to use the Stout API so that the correct Windows
> implementation is used, as `::chdir` is part of the CRT.
> 
> Also included used but not included `stout/os/*` headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> 898326360d6b4f0a50d6ef3f7c86141d0aa70438 
> 
> 
> Diff: https://reviews.apache.org/r/66443/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66568: Dropped combined operations with GROW and SHRINK volumes.

2018-04-23 Thread Greg Mann


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




src/master/master.cpp
Line 4512 (original), 4515-4527 (patched)


I was just wondering - perhaps this code belongs in the validation function 
for the ACCEPT call? Since this is something that we can easily confirm 
synchronously in the top-level scheduler API handler and return 400, rather 
than calling into the accept handlers.


- Greg Mann


On April 23, 2018, 8:07 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66568/
> ---
> 
> (Updated April 23, 2018, 8:07 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These two operations are intended to be non-speculative eventually but
> are implemented as speculative right now. To avoid frameworks opt-in to
> dangerous behavior, we require that accept can only contain one
> `GROW_VOLUME` or `SHRINK_VOLUME` and no other operations.
> 
> This is implemented by reuse existing code which already drops `LAUNCH`
> or `LAUNCH_GROUP` with proper reason and message.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 
> 
> 
> Diff: https://reviews.apache.org/r/66568/diff/5/
> 
> 
> Testing
> ---
> 
> Behavior tested in https://reviews.apache.org/r/66569/.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 65995: Declined unwanted offers in `RetryOperationStatusUpdate*` SLRP tests.

2018-04-23 Thread Chun-Hung Hsiao



> On April 20, 2018, 11:26 a.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 2573-2574 (patched)
> > 
> >
> > I feel this leads to slightly involved state transitions below (e.g., 
> > requiring explicit clock manipulations to restore sane state).
> > 
> > How about introducing a
> > 
> > auto isNotRaw = [isRaw](const Resource& r) { return !isRaw(r); };
> > 
> > and then making dedicated declines,
> > 
> > EXPECT_CALL(sched, resourceOffers(, OffersHaveAnyResource(
> > std::bind(isNotRaw, lambda::_1
> >   .WillRepeatedly(DeclineOffers());
> >   
> > We should be able to remove the new clock manipulations then.

I could do the dedicated decline. However it doesn't resolve the issue I 
described in the comments for the explicit clock manipulation. We still need to 
ensure that a new offer is sent after tho unwanted offer has been declined.


- Chun-Hung


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


On April 12, 2018, 3:31 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65995/
> ---
> 
> (Updated April 12, 2018, 3:31 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The two SLRP tests assume that SLRP will send out a RAW resource in its
> first `UPDATE_STATE` message, and expect that the test framework would
> receive an offer containing the RAW resource in its first offer. However
> this behavior is not guaranteed and should not be relied on. This patch
> makes the tests decline unwanted offers by default so they no longer
> rely on SLRP's internal behavior.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 2872f1aec1a7b94fc302a533f5ae9e1be9658087 
> 
> 
> Diff: https://reviews.apache.org/r/65995/diff/5/
> 
> 
> Testing
> ---
> 
> sudo make check
> Ran the two tests in repitition.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66749: Added more logging to agent recovery path.

2018-04-23 Thread Meng Zhu


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

(Updated April 23, 2018, 3:30 p.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


Changes
---

Thanks! Comments addressed.


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


Repository: mesos


Description
---

Added logging in some agent recovery continuations to
make analyzing agent recovery related issue less painful.


Diffs (updated)
-

  src/slave/containerizer/composing.cpp 
186102c66d373dcd799cadd9fed7d1c8cb894971 
  src/slave/containerizer/docker.cpp a4c9c10e91e75f406329cbb2086f39b549cbeed0 
  src/slave/containerizer/mesos/containerizer.cpp 
def09f1104213bf73d9f95cb5ad2fb80e3bdb04a 
  src/slave/containerizer/mesos/linux_launcher.cpp 
af34a856e092a880a0809da34ead9d8588b0ac8f 
  src/slave/slave.cpp 9d2d1928b231044988f1855eb518448db38ff04f 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66666: Added MemoryProfiler to CHANGELOG.

2018-04-23 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 6 was successfully built and tested.

Reviews applied: `['63372', '6']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/6

- Mesos Reviewbot Windows


On April 23, 2018, 2:09 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6/
> ---
> 
> (Updated April 23, 2018, 2:09 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added changelog entry for the MemoryProfiler class that was
> added to libprocess.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 09fff1d50504de158b6c77f51bd7968746e31371 
> 
> 
> Diff: https://reviews.apache.org/r/6/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 66431: Windows: Fixed `os::read()` to use `ReadFile()`.

2018-04-23 Thread Joseph Wu



> On April 18, 2018, 11:55 a.m., Joseph Wu wrote:
> > 3rdparty/stout/include/stout/os/windows/read.hpp
> > Lines 39-44 (patched)
> > 
> >
> > So you're saying that `ReadFile` reads all the existing data on the 
> > handle prior to returning False?It would feel safer if we verify this 
> > with a unit test.
> 
> Andrew Schwartzmeyer wrote:
> I think that `TEST_F(SubprocessTest, PipeOutputToFileDescriptor)` 
> adequately ensures this is working as inspected. Let me know what you think...

I don't think piping output to an FD is quite what I'm thinking of.

Perhaps a test should:
(1) Open a subprocess that writes to a pipe held by the test.
(2) The subprocess should write a substantial amount of data (more than a 
memory page).
(3) Close the subprocess before reading any data from the pipe.  Presumably 
this is where the broken pipe error should occur?


- Joseph


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


On April 9, 2018, 3:53 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66431/
> ---
> 
> (Updated April 9, 2018, 3:53 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8676
> https://issues.apache.org/jira/browse/MESOS-8676
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This can eventually support overlapped I/O.
> 
> The Windows API `ReadFile()` returns an error if the pipe is broken,
> where `_read()` did not, but this is not an error for us as the data
> is still read correctly. So we ignore it.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/read.hpp 
> 49878e499209fa2f91fede0ebdabb8f088a9d018 
>   3rdparty/stout/include/stout/os/windows/read.hpp 
> 8047ad590fcc46d3ec46b551472d8c518ae49cc1 
> 
> 
> Diff: https://reviews.apache.org/r/66431/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66283: Added support of `max_completion_time` in docker executor.

2018-04-23 Thread Zhitao Li


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

(Updated April 23, 2018, 3:25 p.m.)


Review request for mesos, Jason Lai and James Peach.


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


Repository: mesos


Description
---

If `TaskInfo.max_completion_time` is set, docker executor will kill
the task immediately. We reuse the `shutdown` method to achieve a
forced kill ignoring any `KillPolicy`.

Framework should only received a `TASK_FAILED` state with
`REASON_MAX_COMPLETION_TIME_REACHED` reason.


Diffs (updated)
-

  src/docker/executor.cpp 1d672112bae889cc9d19343a59e0ff66569785c4 


Diff: https://reviews.apache.org/r/66283/diff/4/

Changes: https://reviews.apache.org/r/66283/diff/3-4/


Testing
---


Thanks,

Zhitao Li

Review Request 66769: Fixed flaky ReconciliationTest.ReconcileStatusUpdateTaskState.

2018-04-23 Thread Jiang Yan Xu


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

Review request for mesos, Andrei Budnik, Alexander Rukletsov, and Megha Sharma.


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


Repository: mesos


Description
---

To simulate a master failover we need to use `replicated_log` as the
registry otherwiser the master loses persisted info about the agents.


Diffs
-

  src/tests/reconciliation_tests.cpp b06244b5f3efad3a88e367cd8e26cebd3d9f2e43 


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


Testing
---

Make check.


Thanks,

Jiang Yan Xu

Re: Review Request 66283: Added support of `max_completion_time` in docker executor.

2018-04-23 Thread Zhitao Li


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

(Updated April 23, 2018, 3:19 p.m.)


Review request for mesos, Jason Lai and James Peach.


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


Repository: mesos


Description
---

If `TaskInfo.max_completion_time` is set, docker executor will kill
the task immediately. We reuse the `shutdown` method to achieve a
forced kill ignoring any `KillPolicy`.

Framework should only received a `TASK_FAILED` state with
`REASON_MAX_COMPLETION_TIME_REACHED` reason.


Diffs (updated)
-

  src/docker/executor.cpp 1d672112bae889cc9d19343a59e0ff66569785c4 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66260: Tested `max_completion_time` support in command executor.

2018-04-23 Thread Zhitao Li


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

(Updated April 23, 2018, 3:18 p.m.)


Review request for mesos, Jason Lai and James Peach.


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


Repository: mesos


Description
---

Tested `max_completion_time` support in command executor.


Diffs (updated)
-

  src/tests/command_executor_tests.cpp 3c5687f918deb8cdda8a97abb0fbd8d9b089926a 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66259: Added `max_completion_time` support to command executor.

2018-04-23 Thread Zhitao Li


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

(Updated April 23, 2018, 3:18 p.m.)


Review request for mesos, Jason Lai and James Peach.


Changes
---

Review comments.


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


Repository: mesos


Description
---

If `TaskInfo.max_completion_time` is set, command executor will kill
the task with `SIGKILL` immediately. Note that no KillPolicy will be
observed. Framework should only received a `TASK_FAILED` state with
`REASON_MAX_COMPLETION_TIME_REACHED` reason.


Diffs (updated)
-

  src/launcher/executor.cpp 8d0869cfcdfc693301d0e185a036e97204bad17f 


Diff: https://reviews.apache.org/r/66259/diff/5/

Changes: https://reviews.apache.org/r/66259/diff/4-5/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66591: Added a validation that `max_completion_time` must be non-negative.

2018-04-23 Thread Zhitao Li


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

(Updated April 23, 2018, 3:17 p.m.)


Review request for mesos, Jason Lai and James Peach.


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


Repository: mesos


Description
---

Added a validation that `max_completion_time` must be non-negative.


Diffs (updated)
-

  src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
  src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66258: Added `max_completion_time` to `TaskInfo` and new reason.

2018-04-23 Thread Zhitao Li


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

(Updated April 23, 2018, 3:17 p.m.)


Review request for mesos, Jason Lai and James Peach.


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


Repository: mesos


Description
---

This new field can be used support tasks which have a maximum duration
scheduling requirement. The new reason is added to distinguish from
normal task kills.


Diffs (updated)
-

  include/mesos/mesos.proto 9e24d3ea46edc21572e9226e2e76c7d55618db24 
  include/mesos/v1/mesos.proto 0f3fd8a2608b5edabc21f5fe5df9b70fc0fa8dc2 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66439: Windows: Made `protobuf::write()` use CRT file descriptor explicitly.

2018-04-23 Thread Joseph Wu


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


Ship it!




Ship It!

- Joseph Wu


On April 6, 2018, 4:16 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66439/
> ---
> 
> (Updated April 6, 2018, 4:16 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8675
> https://issues.apache.org/jira/browse/MESOS-8675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is another edge case where a third-party library (protobuf)
> requires a CRT integer file descriptor. Thus we duplicate the `int_fd`
> and then explicitly allocate via `crt()`, which requires that we also
> manually close it via `_close()`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/protobuf.hpp 
> 2fa5072e3c62c487da0dccffdd38d2fa1a615dc0 
> 
> 
> Diff: https://reviews.apache.org/r/66439/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66442: Windows: Fixed `os::abort()` to use `WriteFile()`.

2018-04-23 Thread Joseph Wu


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


Ship it!




Ship It!

- Joseph Wu


On April 4, 2018, 7:06 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66442/
> ---
> 
> (Updated April 4, 2018, 7:06 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8682
> https://issues.apache.org/jira/browse/MESOS-8682
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Fixed `os::abort()` to use `WriteFile()`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/abort.hpp 
> 4fd233dfcd4359791dd176820f3a6040947bb291 
> 
> 
> Diff: https://reviews.apache.org/r/66442/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66050: Implemented grow and shrink of persistent volumes.

2018-04-23 Thread Greg Mann


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




src/common/resources_utils.cpp
Lines 708-709 (patched)


Fits on one line.


- Greg Mann


On April 20, 2018, 6:40 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66050/
> ---
> 
> (Updated April 20, 2018, 6:40 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new offer operations are implemented as speculative operations, but
> we will use validation to make them non-speculative on API level so that
> we can transition later without a breaking change.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
>   src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
>   src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 
>   src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
>   src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
> 
> 
> Diff: https://reviews.apache.org/r/66050/diff/12/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 63372: Added documentation for memory profiling.

2018-04-23 Thread Alexander Rukletsov


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




docs/memory-profiling.md
Lines 6 (patched)


IIRC, the convention is to use `#` only for the title, i.e., this line, 
meaning you should add one `#` to each section below. Also, the title above 
should coincide with this line.



docs/memory-profiling.md
Lines 22 (patched)


Please links in "[]()" format.



docs/memory-profiling.md
Lines 27 (patched)


You mention here memory profiling but actually speak about jemalloc. How 
about the following.

"To make memory profiling possible, jemalloc memory allocator is required. 
There are two ways..." 

or

"A prerequisite for memory profiling is a suitable allocator. We currently 
support jemalloc, which can be connected via one of the following ways..."



docs/memory-profiling.md
Lines 31 (patched)


the bundled version?



docs/memory-profiling.md
Lines 31-32 (patched)


The end of the sentence sounds a bit weird. Can you please rephrase?



docs/memory-profiling.md
Lines 34 (patched)


What do you mean under present? On the build host? On the target host? 
Maybe rephrasing this saying something like "Users can provide their own 
jemalloc build via..."



docs/memory-profiling.md
Lines 36-37 (patched)


Maybe decorate this as a `**NOTE:**` to capture people's attention?



docs/memory-profiling.md
Lines 41 (patched)


Please back tick types.



docs/memory-profiling.md
Lines 50 (patched)


s/,/:/



docs/memory-profiling.md
Lines 53-54 (patched)


Let's explain a bit more, users will appreciate! Something like this: 
"Switching to the jemalloc allocator alone is not enough to get memory dumps, 
the binaries itself should have memory profiling enabled. To do this, start..."



docs/memory-profiling.md
Lines 71 (patched)


This section lacks some guidance regarding the UX. Please mention somewhere 
in this section things like
1. There can only be one active profiling run.
2. Data from a single, most recently finished profiling run are stored.
3. Fumbling with jemalloc memory profiling out of band is unsupported and 
can lead to weird results.



docs/memory-profiling.md
Lines 78 (patched)


What are the other ways? Are they alternatives or you discourage using them?



docs/memory-profiling.md
Lines 82-84 (patched)


Please tell people they will get an `id` of the progiling run that can be 
used to uniqly identify it.



docs/memory-profiling.md
Lines 98 (patched)


comma after i.e.



docs/memory-profiling.md
Lines 100-102 (patched)


No need to put this link onto a separate line. "Check out [the official 
jemalloc documentation](link) for the description of the file format"



docs/memory-profiling.md
Lines 108 (patched)


Back tick `jeprof`



docs/memory-profiling.md
Lines 109 (patched)


Period at the end.



docs/memory-profiling.md
Lines 111 (patched)


Dito re `jeprof`



docs/memory-profiling.md
Lines 122-124 (patched)


Is `dot` tool required as well?



docs/memory-profiling.md
Lines 125 (patched)


Is this blank line necessary?



docs/memory-profiling.md
Lines 127-128 (patched)


I don't think this adds much value : ), you can directly start with the 
next paragraph.



docs/memory-profiling.md
Lines 151 (patched)


maybe prepend the line with `$` to indicate it is a copypasteable command?



docs/memory-profiling.md
Lines 167 (patched)


this?



docs/memory-profiling.md
Lines 168 (patched)


Does \~\~\~{.bash} work?



docs/memory-profiling.md
Lines 183 (patched)

Re: Review Request 66608: Improved support for move-only types in `hashmap`.

2018-04-23 Thread Alexander Rukletsov


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


Ship it!




Ship It!

- Alexander Rukletsov


On April 23, 2018, 9:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66608/
> ---
> 
> (Updated April 23, 2018, 9:52 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While it was already possible to create a `hashmap` over move-only
> values, we still performed a copy in `put`, making it hard to
> dynamically add elements with the expected stout semantics.
> 
> This patch relaxes the requirements on the value argument to `put` so
> that instead of copyable we now only require move-constructible types.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/hashmap.hpp 
> 91085b8d8ad5d35c39c8cc95e3d4765d82d9a8db 
> 
> 
> Diff: https://reviews.apache.org/r/66608/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66608: Improved support for move-only types in `hashmap`.

2018-04-23 Thread Benjamin Bannier


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

(Updated April 23, 2018, 11:52 p.m.)


Review request for mesos, Alexander Rukletsov and Michael Park.


Changes
---

Documented overload as suggested offline by a reviewer.


Repository: mesos


Description
---

While it was already possible to create a `hashmap` over move-only
values, we still performed a copy in `put`, making it hard to
dynamically add elements with the expected stout semantics.

This patch relaxes the requirements on the value argument to `put` so
that instead of copyable we now only require move-constructible types.


Diffs (updated)
-

  3rdparty/stout/include/stout/hashmap.hpp 
91085b8d8ad5d35c39c8cc95e3d4765d82d9a8db 


Diff: https://reviews.apache.org/r/66608/diff/4/

Changes: https://reviews.apache.org/r/66608/diff/3-4/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 66546: Prevent resubscription of resource providers with unknown IDs.

2018-04-23 Thread Chun-Hung Hsiao


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



Please update the commit summary accordingly if you move the check in 
`subscribe()` to the previous patch.


src/resource_provider/manager.cpp
Lines 683-692 (patched)


Let's move this to r/66545.



src/tests/resource_provider_manager_tests.cpp
Lines 1160 (patched)


No need to create the master flag first since it is not used elsewhere.

It also seems that we have this redundant step in all other tests.



src/tests/resource_provider_manager_tests.cpp
Lines 1197-1198 (patched)


The `disconnected` callback is run in another actor (the `Driver`) so let's 
swap the order of the two actions to make sure that `Invoke` is executed before 
the `resourceProvider` pointer itself is destructed at the end of its scope.


- Chun-Hung Hsiao


On April 12, 2018, 9:48 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66546/
> ---
> 
> (Updated April 12, 2018, 9:48 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-7558
> https://issues.apache.org/jira/browse/MESOS-7558
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a check to the resource provider manager's subscribe
> functionality making sure that any the ID sent by a resubscribing
> resource provider corresponds to some previously known resource
> provider.
> 
> This not only serves as convenient validation of user-provided data,
> but also makes sure that the internal state of the resource provider
> manager remains consistent.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66546/diff/5/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66749: Added more logging to agent recovery path.

2018-04-23 Thread Greg Mann


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




src/slave/containerizer/docker.cpp
Lines 913 (patched)


s/containers/container/



src/slave/slave.cpp
Lines 6889 (patched)


I think this line might be a bit misleading - we have finished recovering 
checkpointed state from disk, but we are just beginning recovering the agent 
state from that.

Perhaps "Finished recovering checkpointed state from '', beginning 
agent recovery"?



src/slave/slave.cpp
Lines 7117 (patched)


Since we do things other than just reconnect to executors here, how about 
"Recovering executors" instead?


- Greg Mann


On April 20, 2018, 10:32 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66749/
> ---
> 
> (Updated April 20, 2018, 10:32 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-8793
> https://issues.apache.org/jira/browse/MESOS-8793
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added logging in some agent recovery continuations to
> make analyzing agent recovery related issue less painful.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> 186102c66d373dcd799cadd9fed7d1c8cb894971 
>   src/slave/containerizer/docker.cpp a4c9c10e91e75f406329cbb2086f39b549cbeed0 
>   src/slave/containerizer/mesos/containerizer.cpp 
> def09f1104213bf73d9f95cb5ad2fb80e3bdb04a 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> af34a856e092a880a0809da34ead9d8588b0ac8f 
>   src/slave/slave.cpp 9d2d1928b231044988f1855eb518448db38ff04f 
> 
> 
> Diff: https://reviews.apache.org/r/66749/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66692: Fixed an documentation error in `monitoring.md`.

2018-04-23 Thread Benjamin Mahler


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


Ship it!




Ship It!

- Benjamin Mahler


On April 18, 2018, 5:37 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66692/
> ---
> 
> (Updated April 18, 2018, 5:37 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See diff.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 12e2103eb041e3e1b99bddafafcf4c615205fb0c 
> 
> 
> Diff: https://reviews.apache.org/r/66692/diff/1/
> 
> 
> Testing
> ---
> 
> Not needed.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66545: Added admitted resource providers to the manager's registry.

2018-04-23 Thread Chun-Hung Hsiao


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




src/resource_provider/manager.cpp
Lines 685 (patched)


Let's move the actual logic from r/66546 to this patch to make it 
self-contained.

Also, what's your opinion on having a different error message for 
re-subscribing with an ID that has been removed?



src/resource_provider/manager.cpp
Lines 688 (patched)


It's probably just my personal preference, but based on the experience 
working on the codebase, i'd prefer `then` over `onAny` because the former 
gives the caller more control on error handling, rather than relying on the 
callee checking states.

Please feel free to drop this if you think `onAny` is a better construct 
here.


- Chun-Hung Hsiao


On April 23, 2018, 11:19 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66545/
> ---
> 
> (Updated April 23, 2018, 11:19 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8402
> https://issues.apache.org/jira/browse/MESOS-8402
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added admitted resource providers to the manager's registry.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
>   src/resource_provider/registrar.cpp 
> 92ef9aecb1e93d10f46e53984391558f9901a60b 
>   src/resource_provider/registry.proto 
> 14bd433ef056f8891e9a38153fdb06c39cee8f62 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66545/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65640: Fixed leaks and a race condition in `UriDiskProfileAdaptorTests`.

2018-04-23 Thread Joseph Wu


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




src/tests/disk_profile_adaptor_tests.cpp
Lines 453-457 (original), 451-457 (patched)


FYI: I'm going to commit the leak fixes immediately as these leaks are 
causing ~200 extranous error log lines per build/test run.


- Joseph Wu


On April 11, 2018, 8:37 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65640/
> ---
> 
> (Updated April 11, 2018, 8:37 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8567
> https://issues.apache.org/jira/browse/MESOS-8567
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There was a race between `Clock::advance()` in the `FetchFromHTTP` test
> and `delay()` in `UriDiskProfileAdaptorProcess::_poll`. This patch
> avoids the race by enforcing an order between the dispatch of the
> `__poll` function (previously `_poll`) and the clock manipulation
> in the test. It also fixed two memory leaks in the test.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 257ff0e8d21e5240c27a123ed0cd563214e24fce 
>   src/resource_provider/storage/uri_disk_profile_adaptor.hpp 
> 0484933b42d0bd66c689b06cb48f492eef7bc606 
>   src/resource_provider/storage/uri_disk_profile_adaptor.cpp 
> 300ea12687de487737ce91066ab4e74d9b3430e6 
>   src/tests/disk_profile_adaptor_tests.cpp 
> 948f6efc916d6fad0703944ad9acb77d85e4c25e 
> 
> 
> Diff: https://reviews.apache.org/r/65640/diff/4/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66666: Added MemoryProfiler to CHANGELOG.

2018-04-23 Thread Benno Evers


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

(Updated April 23, 2018, 9:09 p.m.)


Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

Added changelog entry for the MemoryProfiler class that was
added to libprocess.


Diffs
-

  CHANGELOG 09fff1d50504de158b6c77f51bd7968746e31371 


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


Testing
---


Thanks,

Benno Evers

Re: Review Request 63370: Added new --memory_profiling flag to agent and master binaries.

2018-04-23 Thread Benno Evers


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

(Updated April 23, 2018, 9:08 p.m.)


Review request for mesos and Alexander Rukletsov.


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


Repository: mesos


Description
---

This flag allows explicit disabling of the memory profiler
endpoint in the master and agent binaries.


Diffs
-

  src/master/flags.hpp 505786e5631c891a52d9d7db403eff312f461d3d 
  src/master/flags.cpp dbb35befd612f4be1019293c1889d19296118d07 
  src/master/main.cpp 3def0f29e4d580306d2ecef033a37aad5333c8ea 
  src/slave/flags.hpp beae47f0f8f2178b93a3484d168ce4d71c961841 
  src/slave/flags.cpp 4ea2286a6a874e6e7f473f97fb0431e17b19c1f8 
  src/slave/main.cpp dcc944840d4e4e611e8ed1b0110b8f1bb189bde9 


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


Testing
---


Thanks,

Benno Evers

Re: Review Request 66644: WIP:Remove unknown unreachable tasks when agent re-registers.

2018-04-23 Thread Mesos Reviewbot Windows


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



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

Reviews applied: `['66644']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66644

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66644/logs/mesos-tests-stdout.log):

```
[   OK ] OperationStatusUpdateManagerTest.RecoverNotCheckpointedStream (7 
ms)
[ RUN  ] OperationStatusUpdateManagerTest.RecoverEmptyFile
[   OK ] OperationStatusUpdateManagerTest.RecoverEmptyFile (14 ms)
[ RUN  ] OperationStatusUpdateManagerTest.RecoverEmptyDirectory
[   OK ] OperationStatusUpdateManagerTest.RecoverEmptyDirectory (14 ms)
[ RUN  ] OperationStatusUpdateManagerTest.RecoverTerminatedStream
[   OK ] OperationStatusUpdateManagerTest.RecoverTerminatedStream (19 ms)
[ RUN  ] OperationStatusUpdateManagerTest.IgnoreDuplicateUpdate
[   OK ] OperationStatusUpdateManagerTest.IgnoreDuplicateUpdate (20 ms)
[ RUN  ] OperationStatusUpdateManagerTest.IgnoreDuplicateUpdateAfterRecover
[   OK ] OperationStatusUpdateManagerTest.IgnoreDuplicateUpdateAfterRecover 
(16 ms)
[ RUN  ] OperationStatusUpdateManagerTest.RejectDuplicateAck
[   OK ] OperationStatusUpdateManagerTest.RejectDuplicateAck (15 ms)
[ RUN  ] OperationStatusUpdateManagerTest.RejectDuplicateAckAfterRecover
[   OK ] OperationStatusUpdateManagerTest.RejectDuplicateAckAfterRecover 
(15 ms)
[ RUN  ] OperationStatusUpdateManagerTest.NonStrictRecoveryCorruptedFile
[   OK ] OperationStatusUpdateManagerTest.NonStrictRecoveryCorruptedFile 
(21 ms)
[ RUN  ] OperationStatusUpdateManagerTest.StrictRecoveryCorruptedFile
[   OK ] OperationStatusUpdateManagerTest.StrictRecoveryCorruptedFile (20 
ms)
[ RUN  ] OperationStatusUpdateManagerTest.UpdateLatestWhenResending
[   OK ] OperationStatusUpdateManagerTest.UpdateLatestWhenResending (20 ms)
[--] 16 tests from OperationStatusUpdateManagerTest (277 ms total)

[--] 6 tests from PartitionTest
[ RUN  ] PartitionTest.PartitionedSlave
[   OK ] PartitionTest.PartitionedSlave (286 ms)
[ RUN  ] PartitionTest.PartitionedSlaveExitedExecutor
[   OK ] PartitionTest.PartitionedSlaveExitedExecutor (371 ms)
[ RUN  ] PartitionTest.TaskCompletedOnPartitionedAgent
```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66644/logs/mesos-tests-stderr.log):

```
I0423 20:57:57.007681 18224 master.cpp:8517] Marked agent 
dc3b2518-4a8c-4d3a-bd8e-a36dfba3d82a-S0 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) unreachable: 
health check timed out
I0423 20:57:57.007681 18224 master.cpp:10482] Updating the state of task 1 of 
framework dc3b2518-4a8c-4d3a-bd8e-a36dfba3d82a- (latest state: TASK_LOST, 
status update state: TASK_LOST)
I0423 20:57:57.009660 26088 hierarchical.cpp:609] Removed agent 
dc3b2518-4a8c-4d3a-bd8e-a36dfba3d82a-S0
I0423 20:57:57.010648 18224 master.cpp:10581] Removing task 1 with resources 
cpus(allocated: *):4; mem(allocated: *):2048; disk(allocated: *):1024; 
ports(allocated: *):[31000-32000] of framework 
dc3b2518-4a8c-4d3a-bd8e-a36dfba3d82a- on agent 
dc3b2518-4a8c-4d3a-bd8e-a36dfba3d82a-S0 at slave(87)@10.3.1.8:50409 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0423 20:57:57.010648 18224 master.cpp:8147] Sending status update TASK_LOST 
for task 1 of framework dc3b2518-4a8c-4d3a-bd8e-a36dfba3d82a- 'health check 
timed out'
I0423 20:57:57.011663 18224 master.cpp:10610] Removing executor 'default' with 
resources [] of framework dc3b2518-4a8c-4d3a-bd8e-a36dfba3d82a- on agent 
dc3b2518-4a8c-4d3a-bd8e-a36dfba3d82a-S0 at slave(87)@10.3.1.8:50409 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0423 20:57:57.013650 18224 master.cpp:2045] Notifying framework 
dc3b2518-4a8c-4d3a-bd8e-a36dfba3d82a- (default) at 
scheduler-8199cd61-e811-4eac-a4b4-fcba52ad00ad@10.3.1.8:50409 of lost agent 
dc3b2518-4a8c-4d3a-bd8e-a36dfba3d82a-S0 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0423 20:57:57.014663 30016 slave.cpp:5243] Handling status update 
TASK_FINISHED (Status UUID: 632972b0-c915-4622-a421-7a0c7e536d4b) for task 1 of 
framework dc3b2518-4a8c-4d3a-bd8e-a36dfba3d82a- from 
executor(31)@10.3.1.8:50409
I0423 20:57:57.015681 30016 slave.cpp:1253] Lost leading master
I0423 20:57:57.015681  4088 task_status_update_manager.cpp:181] Pausing sending 
task status updates
I0423 20:57:57.016660 30016 slave.cpp:1315] Detecting new master
I0423 20:57:57.017660 30016 slave.cpp:1260] New master detected at 
master@10.3.1.8:50409
I0423 20:57:57.017660 29096

Re: Review Request 66460: Added a `call()` method to the v1 scheduler library.

2018-04-23 Thread Greg Mann


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


Ship it!




Ship It!

- Greg Mann


On April 23, 2018, 8:23 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66460/
> ---
> 
> (Updated April 23, 2018, 8:23 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-8192
> https://issues.apache.org/jira/browse/MESOS-8192
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a `call()` method to the scheduler library that allows
> clients to send a `v1::scheduler::Call` to the master and receive a
> `v1::scheduler::Response`.
> 
> It is going to be used to test operation state reconciliation.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp d56e08861d5190ef66992d383dc5710d8f6ce661 
>   include/mesos/v1/scheduler/scheduler.proto 
> 25ebcfcb07cb4fb928dcfdc242bb6509c9080491 
>   src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 
> 60b17b9be74132c81532d22eba681feb899b22a3 
>   src/scheduler/scheduler.cpp ecef916ffd0797a13552525ff2cda0f99ca57e74 
> 
> 
> Diff: https://reviews.apache.org/r/66460/diff/5/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66464: Implemented operation status reconciliation.

2018-04-23 Thread Greg Mann


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


Ship it!




Ship It!

- Greg Mann


On April 20, 2018, 10:53 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66464/
> ---
> 
> (Updated April 20, 2018, 10:53 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8191
> https://issues.apache.org/jira/browse/MESOS-8191
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented operation status reconciliation.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler/scheduler.proto 
> 25ebcfcb07cb4fb928dcfdc242bb6509c9080491 
>   src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
>   src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
>   src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 
>   src/messages/messages.proto 556801d9ece3ada6d8e3cec51882ad50c27e24b2 
> 
> 
> Diff: https://reviews.apache.org/r/66464/diff/6/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests` on GNU/Linux
> 
> https://reviews.apache.org/r/66468/ adds new tests.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66227: Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operator API.

2018-04-23 Thread Zhitao Li


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

(Updated April 23, 2018, 1:24 p.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


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


Repository: mesos


Description
---

Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operator API.


Diffs (updated)
-

  src/tests/api_tests.cpp dd8e221d8fd1b2a241505345337897e4ee4a6347 


Diff: https://reviews.apache.org/r/66227/diff/6/

Changes: https://reviews.apache.org/r/66227/diff/5-6/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66460: Added a `call()` method to the v1 scheduler library.

2018-04-23 Thread Gaston Kleiman


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

(Updated April 23, 2018, 1:23 p.m.)


Review request for mesos, Greg Mann and Vinod Kone.


Changes
---

Addressed feedback.


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


Repository: mesos


Description
---

This patch adds a `call()` method to the scheduler library that allows
clients to send a `v1::scheduler::Call` to the master and receive a
`v1::scheduler::Response`.

It is going to be used to test operation state reconciliation.


Diffs (updated)
-

  include/mesos/v1/scheduler.hpp d56e08861d5190ef66992d383dc5710d8f6ce661 
  include/mesos/v1/scheduler/scheduler.proto 
25ebcfcb07cb4fb928dcfdc242bb6509c9080491 
  src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 
60b17b9be74132c81532d22eba681feb899b22a3 
  src/scheduler/scheduler.cpp ecef916ffd0797a13552525ff2cda0f99ca57e74 


Diff: https://reviews.apache.org/r/66460/diff/5/

Changes: https://reviews.apache.org/r/66460/diff/4-5/


Testing
---

`sudo bin/mesos-tests` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 66227: Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operator API.

2018-04-23 Thread Zhitao Li


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

(Updated April 23, 2018, 1:10 p.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


Changes
---

Split into two tests, only use API, and other review comments.


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


Repository: mesos


Description
---

Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operator API.


Diffs (updated)
-

  src/tests/api_tests.cpp dd8e221d8fd1b2a241505345337897e4ee4a6347 


Diff: https://reviews.apache.org/r/66227/diff/5/

Changes: https://reviews.apache.org/r/66227/diff/4-5/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66568: Dropped combined operations with GROW and SHRINK volumes.

2018-04-23 Thread Zhitao Li


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

(Updated April 23, 2018, 1:07 p.m.)


Review request for mesos, Chun-Hung Hsiao and Greg Mann.


Changes
---

Make sure dropReason is set.


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


Repository: mesos


Description
---

These two operations are intended to be non-speculative eventually but
are implemented as speculative right now. To avoid frameworks opt-in to
dangerous behavior, we require that accept can only contain one
`GROW_VOLUME` or `SHRINK_VOLUME` and no other operations.

This is implemented by reuse existing code which already drops `LAUNCH`
or `LAUNCH_GROUP` with proper reason and message.


Diffs (updated)
-

  src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 


Diff: https://reviews.apache.org/r/66568/diff/5/

Changes: https://reviews.apache.org/r/66568/diff/4-5/


Testing
---

Behavior tested in https://reviews.apache.org/r/66569/.


Thanks,

Zhitao Li

Re: Review Request 66696: Updated documentation for operation feedback.

2018-04-23 Thread Mesos Reviewbot Windows


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



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

Reviews applied: `['66458', '66459', '66461', '66489', '66462', '66463', 
'66464', '66465', '66466', '66467', '66460', '66468', '66696']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66696

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66696/logs/mesos-tests-stdout.log):

```
[ RUN  ] 
ContentType/SchedulerTest.OperationFeedbackValidationNoResourceProviderCapability/1
[   OK ] 
ContentType/SchedulerTest.OperationFeedbackValidationNoResourceProviderCapability/1
 (14747 ms)
[ RUN  ] 
ContentType/SchedulerTest.OperationFeedbackValidationSchedulerDriverFramework/0
[   OK ] 
ContentType/SchedulerTest.OperationFeedbackValidationSchedulerDriverFramework/0 
(14378 ms)
[ RUN  ] 
ContentType/SchedulerTest.OperationFeedbackValidationSchedulerDriverFramework/1
[   OK ] 
ContentType/SchedulerTest.OperationFeedbackValidationSchedulerDriverFramework/1 
(14275 ms)
[ RUN  ] ContentType/SchedulerTest.ShutdownExecutor/0
[   OK ] ContentType/SchedulerTest.ShutdownExecutor/0 (14074 ms)
[ RUN  ] ContentType/SchedulerTest.ShutdownExecutor/1
[   OK ] ContentType/SchedulerTest.ShutdownExecutor/1 (14592 ms)
[ RUN  ] ContentType/SchedulerTest.Decline/0
[   OK ] ContentType/SchedulerTest.Decline/0 (14480 ms)
[ RUN  ] ContentType/SchedulerTest.Decline/1
[   OK ] ContentType/SchedulerTest.Decline/1 (14472 ms)
[ RUN  ] ContentType/SchedulerTest.Revive/0
[   OK ] ContentType/SchedulerTest.Revive/0 (14246 ms)
[ RUN  ] ContentType/SchedulerTest.Revive/1
[   OK ] ContentType/SchedulerTest.Revive/1 (14757 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/0
[   OK ] ContentType/SchedulerTest.Suppress/0 (14413 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/1
[   OK ] ContentType/SchedulerTest.Suppress/1 (14235 ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0 (19359 
ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1 (19403 
ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0
[   OK ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0 
(19080 ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/1
```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66696/logs/mesos-tests-stderr.log):

```
I0423 19:57:13.374662  7008 task_status_update_manager.cpp:181] Pausing sending 
task status updates
I0423 19:57:13.374662 12876 slave.cpp:1260] New master detected at 
master@172.27.128.1:53004
I0423 19:57:13.375618 12876 slave.cpp:1315] Detecting new master
I0423 19:57:13.387629 2 slave.cpp:1342] Authenticating with master 
master@172.27.128.1:53004
I0423 19:57:13.387629 2 slave.cpp:1351] Using default CRAM-MD5 authenticatee
I0423 19:57:13.388635 12216 authenticatee.cpp:121] Creating new client SASL 
connection
I0423 19:57:13.389649 11864 master.cpp:9316] Authenticating 
slave(420)@172.27.128.1:53004
I0423 19:57:13.390655 12580 authenticator.cpp:98] Creating new server SASL 
connection
I0423 19:57:13.391664  4832 authenticatee.cpp:213] Received SASL authentication 
mechanisms: CRAM-MD5
I0423 19:57:13.391664  4832 authenticatee.cpp:239] Attempting to authenticate 
with mechanism 'CRAM-MD5'
I0423 19:57:13.392630  7872 authenticator.cpp:204] Received SASL authentication 
start
I0423 19:57:13.392630  7872 authenticator.cpp:326] Authentication requires more 
steps
I0423 19:57:13.392630  7092 authenticatee.cpp:259] Received SASL authentication 
step
I0423 19:57:13.393618 16140 authenticator.cpp:232] Received SASL authentication 
step
I0423 19:57:13.393618 16140 authenticator.cpp:318] Authentication success
I0423 19:57:13.394618 10900 authenticatee.cpp:299] Authentication success
I0423 19:57:13.394618  7488 master.cpp:9346] Successfully authenticated 
principal 'test-principal' at slave(420)@172.27.128.1:53004
I0423 19:57:13.395653  7292 slave.cpp:1434] Successfully authenticated with 
master master@172.27.128.1:53004
I0423 19:57:13.396649 2 master.cpp:6330] Received register agent message 
from slave(420)@172.27.128.1:53004 (winbldsrv-02)
I0423 19:57:13.397634 2 master.cpp:3806] Authorizing agent providing 
resources 'cpus:4; mem:2048; disk:1024; ports:[31000-32000]' with principal 
'test-principal'
I0423 19:57:13.399667 16168 master.cpp:6516] Registering agent at

Re: Review Request 66460: Added a `call()` method to the v1 scheduler library.

2018-04-23 Thread Greg Mann


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




include/mesos/v1/scheduler/scheduler.proto
Lines 500 (patched)


s/ty/by/

I'll fix this while committing.



include/mesos/v1/scheduler/scheduler.proto
Lines 516 (patched)


s/not part/is not part/

I'll fix this while committing.


- Greg Mann


On April 21, 2018, 12:03 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66460/
> ---
> 
> (Updated April 21, 2018, 12:03 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-8192
> https://issues.apache.org/jira/browse/MESOS-8192
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a `call()` method to the scheduler library that allows
> clients to send a `v1::scheduler::Call` to the master and receive a
> `v1::scheduler::Response`.
> 
> It is going to be used to test operation state reconciliation.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp d56e08861d5190ef66992d383dc5710d8f6ce661 
>   include/mesos/v1/scheduler/scheduler.proto 
> 25ebcfcb07cb4fb928dcfdc242bb6509c9080491 
>   src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 
> 60b17b9be74132c81532d22eba681feb899b22a3 
>   src/scheduler/scheduler.cpp ecef916ffd0797a13552525ff2cda0f99ca57e74 
> 
> 
> Diff: https://reviews.apache.org/r/66460/diff/4/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66544: Remembered recovered and subscribed providers in ephemeral state.

2018-04-23 Thread Chun-Hung Hsiao


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


Ship it!




Ship It!

- Chun-Hung Hsiao


On April 11, 2018, 9:23 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66544/
> ---
> 
> (Updated April 11, 2018, 9:23 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8402
> https://issues.apache.org/jira/browse/MESOS-8402
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a data structure to bookkeep subscribed and recovered
> resource providers in the ephemeral state of the resource provider
> manager.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
> 
> 
> Diff: https://reviews.apache.org/r/66544/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66311: Set up recovery code paths of resource provider manager.

2018-04-23 Thread Chun-Hung Hsiao

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

src/resource_provider/manager.cpp
Lines 244-249 (patched)

Nit: Please fix the indentation.

src/resource_provider/manager.cpp
Line 222 (original), 253-259 (patched)

If this is not called elsewhere, how about not exposing this as a function 
but put in it `initialize()`?

src/resource_provider/manager.cpp
Lines 266-270 (patched)

```
// ... actor's indox, which could become large if a big ...
   ^^^  ^^^^^
```
I'm not a native speaker though, so please drop this if I'm incorrect ;)

src/resource_provider/registrar.cpp
Lines 199-205 (original), 204-208 (patched)

I just realized that I missed one thing here: if `recover()` is called 
before `recovered` is assigned, it will return a future that is forever 
pending. We can do one of the followings to resolve this:

1. Revert `recovered` back to an `Option`, move this 
assignment back to `recover()` and guard it with `if (recovered.isNone())`, 
then call `recover()` here.

2. Make `recovered` a `Promise` as we do in 
`ResourceProviderManagerProcess` above, the do
```
recovered.associate(state.fetch(NAME).then(...));
```

3. Don't do anything. Since this is an internal class and the exposed 
`GenericRegistrar` always dispatch `recover()` to this actor, `initialize()` is 
guaranteed to always called before `recover()`. This relies on a convention 
between this actor implementation and the `GenericRegistrar` class.

I think either of the 3 is okay. Personally I would prefer adjusting either 
this class or the `ResourceProviderManagerProcess` class to make the two 
consistent though.

src/resource_provider/registrar.cpp
Line 215 (original), 226 (patched)

`undiscardable(recovered).then(...)` or `recover().then(...)`.

src/resource_provider/registrar.cpp
Line 284 (original), 292 (patched)

This parameter is no longer needed.

- Chun-Hung Hsiao

On April 23, 2018, 11:19 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66311/
> ---
> 
> (Updated April 23, 2018, 11:19 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8735
> https://issues.apache.org/jira/browse/MESOS-8735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adjusts the control flow of the resource provider manager
> so that we can in the future make use of persisted resource provider
> information. While this patch sets up the needed flow, it does not
> implement recovery logic, yet.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
>   src/resource_provider/registrar.hpp 
> 39f45b0a2a7c35bfe72a9305f5248409e4a3ef45 
>   src/resource_provider/registrar.cpp 
> 92ef9aecb1e93d10f46e53984391558f9901a60b 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66311/diff/7/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66310: Passed on registrar when constructing resource provider manager.

2018-04-23 Thread Chun-Hung Hsiao


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


Ship it!




Ship It!

- Chun-Hung Hsiao


On April 23, 2018, 11:18 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66310/
> ---
> 
> (Updated April 23, 2018, 11:18 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8735
> https://issues.apache.org/jira/browse/MESOS-8735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order to support recovering resource provider manager information
> in the future, we need to adjust the construction of the manager to be
> able to consume a registrar.
> 
> This patch lays the groundwork by adjusting interfaces and their
> usage; we will make use of the passed on information in a following
> patch.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.hpp bc017fa3998b780cec82718cabe2a8b470a66db2 
>   src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
>   src/resource_provider/registrar.cpp 
> 92ef9aecb1e93d10f46e53984391558f9901a60b 
>   src/slave/slave.cpp 2b8c6e0771a58efacf26628abae6cbe46380a369 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66310/diff/7/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66644: Remove unknown unreachable tasks when agent re-registers.

2018-04-23 Thread Gaston Kleiman


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




src/master/master.cpp
Lines 7130 (patched)


I'd add a comment here explaining what we are doing.

Maybe something like:

"All tasks from this agent are now reachable. Update framework bookkeeping, 
removing tasks that were dropped en-route to the agent and marked as 
unreachable."



src/tests/partition_tests.cpp
Lines 167-168 (patched)


I'd write something like:

```
This is a regression test for MESOS-8750. It verifies that a master won't 
crash when a `RunTaskMessage` associated to a partition aware framework is 
dropped en route to the agent and then the agent fails over.
```



src/tests/partition_tests.cpp
Lines 169 (patched)


s/cleanUpUnreachableTasks/CleanUpUnreachableTasks/



src/tests/partition_tests.cpp
Lines 182 (patched)


The indentation here looks off.



src/tests/partition_tests.cpp
Lines 200 (patched)


Ditto indentation.



src/tests/partition_tests.cpp
Lines 206-207 (patched)


Ditto indentation.



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


s/Offer/const Offer&/



src/tests/partition_tests.cpp
Lines 220 (patched)


s/Capture/Drop/ ?

Where is the pid unset?

I think I'd write something like: "Drop the `RunTaskMessage`", so that the 
agent doesn't learn about the new task.



src/tests/partition_tests.cpp
Lines 222 (patched)


Ditto indentation.



src/tests/partition_tests.cpp
Lines 228 (patched)


Remove this blank line.



src/tests/partition_tests.cpp
Lines 235 (patched)


Ditto indentation.



src/tests/partition_tests.cpp
Lines 241 (patched)


Ditto indentation.



src/tests/partition_tests.cpp
Lines 279-282 (patched)


Ditto indentation.



src/tests/partition_tests.cpp
Lines 303 (patched)


Ditto indentation.



src/tests/partition_tests.cpp
Lines 317 (patched)


Ditto indentation.



src/tests/partition_tests.cpp
Lines 319-323 (patched)


Ditto indentation.



src/tests/partition_tests.cpp
Lines 328-332 (patched)


Ditto indentation.



src/tests/partition_tests.cpp
Lines 346-350 (patched)


Ditto indentation.



src/tests/partition_tests.cpp
Lines 355-359 (patched)


Ditto indentation.



src/tests/partition_tests.cpp
Lines 379 (patched)


s/reregister/re-register/



src/tests/partition_tests.cpp
Lines 384 (patched)


Ditto indentation.



src/tests/partition_tests.cpp
Lines 433-437 (patched)


Ditto indentation.



src/tests/partition_tests.cpp
Lines 452-456 (patched)


Ditto indentation.


- Gaston Kleiman


On April 23, 2018, 11:11 a.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66644/
> ---
> 
> (Updated April 23, 2018, 11:11 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: 8750
> https://issues.apache.org/jira/browse/8750
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A RunTask messsage could get dropped for an agent while it's
> disconnected from the master and when such an agent goes unreachable
> then this dropped task message gets added to the unreachable tasks.
> When the agent re-registers, the master sends status updates for the
> tasks that the agent reported when re-registering and these tasks are
> also removed from the unreachableTasks on the framework but since the
> agent doesn't know about the dropped task so it doesn't get removed
> from the unreachableTasks leading to a check failure when
> this

Re: Review Request 66696: Updated documentation for operation feedback.

2018-04-23 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [66458, 66459, 66461, 66489, 66462, 66463, 66464, 66465, 
66466, 66467, 66460, 66468, 66696]

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

- Mesos Reviewbot


On April 20, 2018, 10:54 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66696/
> ---
> 
> (Updated April 20, 2018, 10:54 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated documentation for operation feedback.
> 
> 
> Diffs
> -
> 
>   docs/csi.md 2d96ee754b3fdcfb18a7516f30abf0086fa75137 
>   docs/scheduler-http-api.md 3929c33781a152428338c4cdaf7dbc47da7c875e 
> 
> 
> Diff: https://reviews.apache.org/r/66696/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Review Request 66644: Remove unknown unreachable tasks when agent re-registers.

2018-04-23 Thread Megha Sharma


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

Review request for mesos and Jiang Yan Xu.


Bugs: 8750
https://issues.apache.org/jira/browse/8750


Repository: mesos


Description
---

A RunTask messsage could get dropped for an agent while it's
disconnected from the master and when such an agent goes unreachable
then this dropped task message gets added to the unreachable tasks.
When the agent re-registers, the master sends status updates for the
tasks that the agent reported when re-registering and these tasks are
also removed from the unreachableTasks on the framework but since the
agent doesn't know about the dropped task so it doesn't get removed
from the unreachableTasks leading to a check failure when
this inconsistency is detected during framework removal.


Diffs
-

  src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
  src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 
  src/tests/partition_tests.cpp 9138e5c745cf354a3573e1ab0b251d46702833cc 


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


Testing
---


Thanks,

Megha Sharma

Re: Review Request 66762: Removed `developers` from Java binding mesos.pom file.

2018-04-23 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [66762]

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

- Mesos Reviewbot


On April 23, 2018, 1:27 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66762/
> ---
> 
> (Updated April 23, 2018, 1:27 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-8819
> https://issues.apache.org/jira/browse/MESOS-8819
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This field should list information about whom to contact for feedback
> on specific pieces of the released Java package, which largely
> overlaps with the Apache committer concept.
> 
> The information in this file in general duplicates information already
> in `docs/committers.md` which is rendered on the Mesos project
> website.  We also publish a link to the Mesos developers mailing list
> with the bindings which should be sufficient to reach Mesos
> committers.
> 
> 
> Diffs
> -
> 
>   src/java/mesos.pom.in 418326bdad5f06fac9197b0d42b17fdbeb586773 
> 
> 
> Diff: https://reviews.apache.org/r/66762/diff/1/
> 
> 
> Testing
> ---
> 
> Build Mesos JAR file (cmake build).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66759: Fixed a regression in isolator ordering.

2018-04-23 Thread James Peach


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


Ship it!




Good catch @nfnt!

- James Peach


On April 23, 2018, 9:55 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66759/
> ---
> 
> (Updated April 23, 2018, 9:55 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Bugs: MESOS-8818
> https://issues.apache.org/jira/browse/MESOS-8818
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The 'volume/sandbox_path' isolator works on POSIX but was only added
> to the Mesos containerizer isolators for Linux.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> def09f1104213bf73d9f95cb5ad2fb80e3bdb04a 
> 
> 
> Diff: https://reviews.apache.org/r/66759/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 66762: Removed `developers` from Java binding mesos.pom file.

2018-04-23 Thread Alexander Rukletsov


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


Ship it!




Ship It!

- Alexander Rukletsov


On April 23, 2018, 1:27 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66762/
> ---
> 
> (Updated April 23, 2018, 1:27 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-8819
> https://issues.apache.org/jira/browse/MESOS-8819
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This field should list information about whom to contact for feedback
> on specific pieces of the released Java package, which largely
> overlaps with the Apache committer concept.
> 
> The information in this file in general duplicates information already
> in `docs/committers.md` which is rendered on the Mesos project
> website.  We also publish a link to the Mesos developers mailing list
> with the bindings which should be sufficient to reach Mesos
> committers.
> 
> 
> Diffs
> -
> 
>   src/java/mesos.pom.in 418326bdad5f06fac9197b0d42b17fdbeb586773 
> 
> 
> Diff: https://reviews.apache.org/r/66762/diff/1/
> 
> 
> Testing
> ---
> 
> Build Mesos JAR file (cmake build).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66762: Removed `developers` from Java binding mesos.pom file.

2018-04-23 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 66762 was successfully built and tested.

Reviews applied: `['66762']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66762

- Mesos Reviewbot Windows


On April 23, 2018, 3:27 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66762/
> ---
> 
> (Updated April 23, 2018, 3:27 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-8819
> https://issues.apache.org/jira/browse/MESOS-8819
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This field should list information about whom to contact for feedback
> on specific pieces of the released Java package, which largely
> overlaps with the Apache committer concept.
> 
> The information in this file in general duplicates information already
> in `docs/committers.md` which is rendered on the Mesos project
> website.  We also publish a link to the Mesos developers mailing list
> with the bindings which should be sufficient to reach Mesos
> committers.
> 
> 
> Diffs
> -
> 
>   src/java/mesos.pom.in 418326bdad5f06fac9197b0d42b17fdbeb586773 
> 
> 
> Diff: https://reviews.apache.org/r/66762/diff/1/
> 
> 
> Testing
> ---
> 
> Build Mesos JAR file (cmake build).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66546: Prevent resubscription of resource providers with unknown IDs.

2018-04-23 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [66508, 66308, 66309, 66526, 66310, 66311, 66544, 66545, 66546]

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

- Mesos Reviewbot


On April 12, 2018, 9:48 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66546/
> ---
> 
> (Updated April 12, 2018, 9:48 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-7558
> https://issues.apache.org/jira/browse/MESOS-7558
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a check to the resource provider manager's subscribe
> functionality making sure that any the ID sent by a resubscribing
> resource provider corresponds to some previously known resource
> provider.
> 
> This not only serves as convenient validation of user-provided data,
> but also makes sure that the internal state of the resource provider
> manager remains consistent.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66546/diff/5/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 66762: Removed `developers` from Java binding mesos.pom file.

2018-04-23 Thread Benjamin Bannier


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

Review request for mesos and Alexander Rukletsov.


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


Repository: mesos


Description
---

This field should list information about whom to contact for feedback
on specific pieces of the released Java package, which largely
overlaps with the Apache committer concept.

The information in this file in general duplicates information already
in `docs/committers.md` which is rendered on the Mesos project
website.  We also publish a link to the Mesos developers mailing list
with the bindings which should be sufficient to reach Mesos
committers.


Diffs
-

  src/java/mesos.pom.in 418326bdad5f06fac9197b0d42b17fdbeb586773 


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


Testing
---

Build Mesos JAR file (cmake build).


Thanks,

Benjamin Bannier

Re: Review Request 66546: Prevent resubscription of resource providers with unknown IDs.

2018-04-23 Thread Mesos Reviewbot Windows


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



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

Reviews applied: `['66308', '66309', '66526', '66310', '66311', '66544', 
'66545', '66546']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66546

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66546/logs/mesos-tests-stdout.log):

```
[--] 9 tests from Endpoint/SlaveEndpointTest (1123 ms total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (34 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (40 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (76 ms 
total)

[--] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN  ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[   OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (776 ms)
[--] 1 test from IsolationFlag/CpuIsolatorTest (805 ms total)

[--] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN  ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (826 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (850 ms total)

[--] Global test environment tear-down
[==] 958 tests from 94 test cases ran. (462916 ms total)
[  PASSED  ] 956 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] 
ContentType/ResourceProviderManagerHttpApiTest.ResubscribeResourceProvider/0, 
where GetParam() = application/x-protobuf
[  FAILED  ] 
ContentType/ResourceProviderManagerHttpApiTest.ResubscribeResourceProvider/1, 
where GetParam() = application/json

 2 FAILED TESTS
  YOU HAVE 214 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66546/logs/mesos-tests-stderr.log):

```
I0423 12:15:16.187870 23916 master.cpp:10453] Updating the state of task 
e0caaee7-d05a-4955-b18c-f517ab1d9fcc of framework 
844a1eb6-8781-4a83-bba3-b382cf383738- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0423 12:15:16.187870 12784 slave.cpp:3931] Shutting down framework 
844a1eb6-8781-4a83-bba3-b382cf383738-
I0423 12:15:16.187870 12784 slave.cpp:6640] Shutting down executor 
'e0caaee7-d05a-4955-b18c-f517abI0423 12:15:16.026840 29384 exec.cpp:162] 
Version: 1.6.0
I0423 12:15:16.054847 29712 exec.cpp:236] Executor registered on agent 
844a1eb6-8781-4a83-bba3-b382cf383738-S0
I0423 12:15:16.058858 28484 executor.cpp:177] Received SUBSCRIBED event
I0423 12:15:16.063844 28484 executor.cpp:181] Subscribed executor on 
winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0423 12:15:16.063844 28484 executor.cpp:177] Received LAUNCH event
I0423 12:15:16.067847 28484 executor.cpp:649] Starting task 
e0caaee7-d05a-4955-b18c-f517ab1d9fcc
I0423 12:15:16.146878 28484 executor.cpp:484] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0423 12:15:16.158898 28484 executor.cpp:662] Forked command at 27808
I0423 12:15:16.190886 30656 exec.cpp:445] Executor asked to shutdown
I0423 12:15:16.190886 29256 executor.cpp:177] Received SHUTDOWN event
I0423 12:15:16.190886 29256 executor.cpp:759] Shutting down
I0423 12:15:16.190886 29256 executor.cpp:869] Sending SIGTERM to process tree 
at pid 1d9fcc' of framework 844a1eb6-8781-4a83-bba3-b382cf383738- at 
executor(1)@10.3.1.8:61555
I0423 12:15:16.189867 12784 slave.cpp:929] Agent terminating
W0423 12:15:16.189867 12784 slave.cpp:3927] Ignoring shutdown framework 
844a1eb6-8781-4a83-bba3-b382cf383738- because it is terminating
I0423 12:15:16.190886 23916 master.cpp:10552] Removing task 
e0caaee7-d05a-4955-b18c-f517ab1d9fcc with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework 844a1eb6-8781-4a83-bba3-b382cf383738- on 
agent 844a1eb6-8781-4a83-bba3-b382cf383738-S0 at slave(428)@10.3.1.8:61534 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0423 12:15:16.193858 32028 containerizer.cpp:2375] Destroying container 
630750e1-d788-4a8a-a9bf-71c8b408fb17 in RUNNING state
I0423 12:15:16.193858 32028 containerizer.cpp:2989] Transitioning the state of 
container 630750e1-d788-4a8a-a9bf-71c8b408fb17 from RUNNING to DESTROYING
I0423 12:15:16.193858 23916 master.cpp:1296] Agent 
844a1eb6-8781-4a83-bba3-b382cf383738-S0 at slave(428)@10.3.1.8:61534

Re: Review Request 66759: Fixed a regression in isolator ordering.

2018-04-23 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [66759]

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

- Mesos Reviewbot


On April 23, 2018, 9:55 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66759/
> ---
> 
> (Updated April 23, 2018, 9:55 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Bugs: MESOS-8818
> https://issues.apache.org/jira/browse/MESOS-8818
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The 'volume/sandbox_path' isolator works on POSIX but was only added
> to the Mesos containerizer isolators for Linux.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> def09f1104213bf73d9f95cb5ad2fb80e3bdb04a 
> 
> 
> Diff: https://reviews.apache.org/r/66759/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 66759: Fixed a regression in isolator ordering.

2018-04-23 Thread Benjamin Bannier


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


Ship it!




LGTM. There appears to be no dependency between `volume/sandbox_path` and 
`docker/volume`.

- Benjamin Bannier


On April 23, 2018, 11:55 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66759/
> ---
> 
> (Updated April 23, 2018, 11:55 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Bugs: MESOS-8818
> https://issues.apache.org/jira/browse/MESOS-8818
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The 'volume/sandbox_path' isolator works on POSIX but was only added
> to the Mesos containerizer isolators for Linux.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> def09f1104213bf73d9f95cb5ad2fb80e3bdb04a 
> 
> 
> Diff: https://reviews.apache.org/r/66759/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 66544: Remembered recovered and subscribed providers in ephemeral state.

2018-04-23 Thread Benjamin Bannier



> On April 23, 2018, 2:53 a.m., Chun-Hung Hsiao wrote:
> > src/resource_provider/manager.cpp
> > Lines 215 (patched)
> > 
> >
> > In the master, we use `recovered` and `registered` to keep track of 
> > agents. Do you think it is better to keep track of `known` that is a 
> > superset of `subscribed`, or make it into two disjoint `recovered` and 
> > `subscribed` sets?

I believe that would only make sense for the sake of conformity with little 
benefit since otherwise the code here is pretty different from the master code.

In the master this split makes sense since we might be dealing with a 
potentially huge number of agents and optimizing the size of the sets we need 
to examine can be benficial. Here expect a smaller number of resource 
providers, so the performance benefit would be outweight by simpler access to 
the data.

Dropping for now, feel free to reopen.


- Benjamin


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


On April 11, 2018, 11:23 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66544/
> ---
> 
> (Updated April 11, 2018, 11:23 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8402
> https://issues.apache.org/jira/browse/MESOS-8402
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a data structure to bookkeep subscribed and recovered
> resource providers in the ephemeral state of the resource provider
> manager.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
> 
> 
> Diff: https://reviews.apache.org/r/66544/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66759: Fixed a regression in isolator ordering.

2018-04-23 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 66759 was successfully built and tested.

Reviews applied: `['66759']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66759

- Mesos Reviewbot Windows


On April 23, 2018, 11:55 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66759/
> ---
> 
> (Updated April 23, 2018, 11:55 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Bugs: MESOS-8818
> https://issues.apache.org/jira/browse/MESOS-8818
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The 'volume/sandbox_path' isolator works on POSIX but was only added
> to the Mesos containerizer isolators for Linux.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> def09f1104213bf73d9f95cb5ad2fb80e3bdb04a 
> 
> 
> Diff: https://reviews.apache.org/r/66759/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 66310: Passed on registrar when constructing resource provider manager.

2018-04-23 Thread Benjamin Bannier



> On April 23, 2018, 3:05 a.m., Chun-Hung Hsiao wrote:
> > src/tests/resource_provider_manager_tests.cpp
> > Line 132 (original), 132 (patched)
> > 
> >
> > Do you think it is reasonable to always require a registrar, i.e., 
> > disallow `nullptr` and use `InMemoryStore` here? Please drop this if you 
> > think we should allow `nullptr`.

There's a trade-off between simpler testing code and internal complexity. In 
this case I agree with you that we can simplify the implementation of the 
manager a lot by requiring users to always pass in a registry. Adjusted the 
code for that, also added some assertions.


- Benjamin


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


On April 23, 2018, 1:18 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66310/
> ---
> 
> (Updated April 23, 2018, 1:18 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8735
> https://issues.apache.org/jira/browse/MESOS-8735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order to support recovering resource provider manager information
> in the future, we need to adjust the construction of the manager to be
> able to consume a registrar.
> 
> This patch lays the groundwork by adjusting interfaces and their
> usage; we will make use of the passed on information in a following
> patch.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.hpp bc017fa3998b780cec82718cabe2a8b470a66db2 
>   src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
>   src/resource_provider/registrar.cpp 
> 92ef9aecb1e93d10f46e53984391558f9901a60b 
>   src/slave/slave.cpp 2b8c6e0771a58efacf26628abae6cbe46380a369 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66310/diff/7/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66311: Set up recovery code paths of resource provider manager.

2018-04-23 Thread Benjamin Bannier



> On April 23, 2018, 2:44 a.m., Chun-Hung Hsiao wrote:
> > src/resource_provider/manager.cpp
> > Lines 61 (patched)
> > 
> >
> > This is not used here but I see it being used in r/66545 so just leave 
> > a comment.

Moved it there.


> On April 23, 2018, 2:44 a.m., Chun-Hung Hsiao wrote:
> > src/resource_provider/registrar.cpp
> > Line 174 (original), 179 (patched)
> > 
> >
> > Actually, do we really need this? It seems that we can always get 
> > `registry` from `variable`?

I just adopted it from the master registrar, and you are right in that it is 
not needed. On top of that it also introduces a lot of complicated state 
coupling between `variable` and `registry`, so removing it is a great 
suggestion. Code adjusted like suggested.


- Benjamin


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


On April 23, 2018, 1:19 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66311/
> ---
> 
> (Updated April 23, 2018, 1:19 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8735
> https://issues.apache.org/jira/browse/MESOS-8735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adjusts the control flow of the resource provider manager
> so that we can in the future make use of persisted resource provider
> information. While this patch sets up the needed flow, it does not
> implement recovery logic, yet.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
>   src/resource_provider/registrar.hpp 
> 39f45b0a2a7c35bfe72a9305f5248409e4a3ef45 
>   src/resource_provider/registrar.cpp 
> 92ef9aecb1e93d10f46e53984391558f9901a60b 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66311/diff/7/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66311: Set up recovery code paths of resource provider manager.

2018-04-23 Thread Benjamin Bannier


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

(Updated April 23, 2018, 1:19 p.m.)


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


Changes
---

Rebased code since `registrar` is now guaranteed to be not null.


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


Repository: mesos


Description
---

This patch adjusts the control flow of the resource provider manager
so that we can in the future make use of persisted resource provider
information. While this patch sets up the needed flow, it does not
implement recovery logic, yet.


Diffs (updated)
-

  src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
  src/resource_provider/registrar.hpp 39f45b0a2a7c35bfe72a9305f5248409e4a3ef45 
  src/resource_provider/registrar.cpp 92ef9aecb1e93d10f46e53984391558f9901a60b 
  src/tests/resource_provider_manager_tests.cpp 
c52541bf130ccf4795b989b53331176a64a097ea 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 66545: Added admitted resource providers to the manager's registry.

2018-04-23 Thread Benjamin Bannier


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

(Updated April 23, 2018, 1:19 p.m.)


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


Changes
---

Rebased code since `registrar` is now guaranteed to be not null.


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


Repository: mesos


Description
---

Added admitted resource providers to the manager's registry.


Diffs (updated)
-

  src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
  src/resource_provider/registrar.cpp 92ef9aecb1e93d10f46e53984391558f9901a60b 
  src/resource_provider/registry.proto 14bd433ef056f8891e9a38153fdb06c39cee8f62 
  src/tests/resource_provider_manager_tests.cpp 
c52541bf130ccf4795b989b53331176a64a097ea 


Diff: https://reviews.apache.org/r/66545/diff/6/

Changes: https://reviews.apache.org/r/66545/diff/5-6/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 66310: Passed on registrar when constructing resource provider manager.

2018-04-23 Thread Benjamin Bannier


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

(Updated April 23, 2018, 1:18 p.m.)


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


Changes
---

Addressed comment from Chun.


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


Repository: mesos


Description
---

In order to support recovering resource provider manager information
in the future, we need to adjust the construction of the manager to be
able to consume a registrar.

This patch lays the groundwork by adjusting interfaces and their
usage; we will make use of the passed on information in a following
patch.


Diffs (updated)
-

  src/resource_provider/manager.hpp bc017fa3998b780cec82718cabe2a8b470a66db2 
  src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
  src/resource_provider/registrar.cpp 92ef9aecb1e93d10f46e53984391558f9901a60b 
  src/slave/slave.cpp 2b8c6e0771a58efacf26628abae6cbe46380a369 
  src/tests/resource_provider_manager_tests.cpp 
c52541bf130ccf4795b989b53331176a64a097ea 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 66759: Fixed a regression in isolator ordering.

2018-04-23 Thread Jan Schlicht


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

Review request for mesos, Benjamin Bannier and James Peach.


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


Repository: mesos


Description
---

The 'volume/sandbox_path' isolator works on POSIX but was only added
to the Mesos containerizer isolators for Linux.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
def09f1104213bf73d9f95cb5ad2fb80e3bdb04a 


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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 66725: Improved error message in the fetcher.

2018-04-23 Thread Gilbert Song


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


Ship it!




Ship It!

- Gilbert Song


On April 19, 2018, 4:08 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66725/
> ---
> 
> (Updated April 19, 2018, 4:08 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Included error message from `mkdir` when
> returning error.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 8a8d7c3311c6d282648940bc4440f4c6d9d5e918 
> 
> 
> Diff: https://reviews.apache.org/r/66725/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

78 matches

Mail list logo