Re: Review Request 66644: Remove unknown unreachable tasks when agent reregisters.

[Megha Sharma]

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

(Updated May 2, 2018, 5:18 a.m.)


Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description (updated)
---

A RunTask messsage could get dropped for an agent while its
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 reregisters, 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 (updated)
-

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


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

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


Testing
---

make check


Thanks,

Megha Sharma

Re: Review Request 66644: Remove unknown unreachable tasks when agent reregisters.

[Megha Sharma]

> On May 1, 2018, 8:50 p.m., Jiang Yan Xu wrote:
> > src/tests/partition_tests.cpp
> > Lines 171 (patched)
> > 
> >
> > This seems to be exceeded the 80 character limit.

For some reason the pre-commit hook didn't complain for this.


> On May 1, 2018, 8:50 p.m., Jiang Yan Xu wrote:
> > src/tests/partition_tests.cpp
> > Lines 47 (patched)
> > 
> >
> > It doesn't look like this is used?

Aah yes that's right.


- Megha


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


On April 25, 2018, 3:46 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66644/
> ---
> 
> (Updated April 25, 2018, 3:46 p.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 gets added to the unreachable tasks.
> When the agent reregisters, tasks reported by it are removed from the 
> unreachableTasks bookkeeping on the master but since the
> agent doesn't know about the dropped task so it doesn't get removed
> from the unreachableTasks leading to a master 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/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 66896: Updated `csi.md`.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66893, 66894, 66896]

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 May 1, 2018, 9:45 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66896/
> ---
> 
> (Updated May 1, 2018, 9:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `csi.md`.
> 
> 
> Diffs
> -
> 
>   docs/csi.md 77a5edbfdc69b451f37f2d5ca7f21bafe19d7872 
> 
> 
> Diff: https://reviews.apache.org/r/66896/diff/1/
> 
> 
> Testing
> ---
> 
> This is a documentation change so no tests are needed.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66901: Add the ASF events link to the website.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66901']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

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

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (56 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (37 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (95 ms 
total)

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

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

[--] Global test environment tear-down
[==] 970 tests from 95 test cases ran. (456599 ms total)
[  PASSED  ] 968 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 216 DISABLED TESTS

```

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

```
I0502 04:34:36.625301  7864 executor.cpp:177] Received SUBSCRIBED event
I0502 04:34:36.630300  7864 executor.cpp:181] Subscribed executor on 
winbldsrv-02.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0502 04:34:36.631300  7864 executor.cpp:177] Received LAUNCH event
I0502 04:34:36.636314  7864 executor.cpp:649] Starting task 
e45e388f-fdef-4fbb-bbab-48ef36d98cc5
I0502 04:34:36.72  7864 executor.cpp:484] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0502 04:34:36.737327  7864 executor.cpp:662] Forked command at 5360
I0502 04:34:36.769330  7940 exec.cpp:445] Executor asked to shutdown
I0502 04:34:36.769330  1116 executor.cpp:177] Received SHUTDOWN event
I0502 04:34:36.769330  1116 executor.cpp:759] Shutting down
I0502 04:34:36.769330  1116 executor.cpp:869] Sending SIGTERM to process tree 
at pid 56330  7332 master.cpp:3259] Deactivating framework 
a27eff9d-3abf-43c3-94c0-54e8825eff61- (default) at 
scheduler-4ad44d3a-4fb0-48eb-9842-aed37fd9cc40@10.3.1.5:56605
I0502 04:34:36.767333 10584 hierarchical.cpp:405] Deactivated framework 
a27eff9d-3abf-43c3-94c0-54e8825eff61-
I0502 04:34:36.767333  7332 master.cpp:10547] Updating the state of task 
e45e388f-fdef-4fbb-bbab-48ef36d98cc5 of framework 
a27eff9d-3abf-43c3-94c0-54e8825eff61- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0502 04:34:36.767333  3108 slave.cpp:3935] Shutting down framework 
a27eff9d-3abf-43c3-94c0-54e8825eff61-
I0502 04:34:36.767333  3108 slave.cpp:6644] Shutting down executor 
'e45e388f-fdef-4fbb-bbab-48ef36d98cc5' of framework 
a27eff9d-3abf-43c3-94c0-54e8825eff61- at executor(1)@10.3.1.5:56626
I0502 04:34:36.768326  3108 slave.cpp:929] Agent terminating
W0502 04:34:36.769330  3108 slave.cpp:3931] Ignoring shutdown framework 
a27eff9d-3abf-43c3-94c0-54e8825eff61- because it is terminating
I0502 04:34:36.769330  7332 master.cpp:10646] Removing task 
e45e388f-fdef-4fbb-bbab-48ef36d98cc5 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework a27eff9d-3abf-43c3-94c0-54e8825eff61- on 
agent a27eff9d-3abf-43c3-94c0-54e8825eff61-S0 at slave(438)@10.3.1.5:56605 
(winbldsrv-02.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0502 04:34:36.773326  7332 master.cpp:1296] Agent 
a27eff9d-3abf-43c3-94c0-54e8825eff61-S0 at slave(438)@10.3.1.5:56605 
(winbldsrv-02.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0502 04:34:36.773326  7332 master.cpp:3296] Disconnecting agent 
a27eff9d-3abf-43c3-94c0-54e8825eff61-S0 at slave(438)@10.3.1.5:56605 
(winbldsrv-02.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)

Review Request 66901: Add the ASF events link to the website.

[James Peach]

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

Review request for mesos, Benjamin Mahler, Greg Mann, and Vinod Kone.


Repository: mesos


Description
---

Update the Mesos website the ASF current event banner described
in http://apache.org/events/README.txt. This is a dynamic banner
that will be updated automatically without the need to update the
website every time for every new event.


Diffs
-

  site/source/layouts/basic.erb 30ae845b88e5247b623f861b1f1c2f4681922380 


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


Testing
---

View website. The image looks like [this](https://pasteboard.co/HjfTvkc.png).


Thanks,

James Peach

Re: Review Request 66858: Added tests for validation of `GrowVolume` and `ShrinkVolume`.

[Chun-Hung Hsiao]

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



How about adding tests to validate that resources with providers are rejected?
```
growVolume.mutable_volume()->mutable_provider_id()->set_value("provider");
```
```
shrinkVolume.mutable_volume()->mutable_provider_id()->set_value("provider");
```

- Chun-Hung Hsiao


On April 27, 2018, 10:07 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66858/
> ---
> 
> (Updated April 27, 2018, 10:07 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests for validation of `GrowVolume` and `ShrinkVolume`.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> a5229610a81dfac9d358597135b63ee51de86659 
> 
> 
> Diff: https://reviews.apache.org/r/66858/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66220: Added tests for `GROW_VOLUME` and `SHRINK_VOLUME` operations.

[Chun-Hung Hsiao]

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


Ship it!




Ship It!

- Chun-Hung Hsiao


On May 1, 2018, 10:44 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66220/
> ---
> 
> (Updated May 1, 2018, 10:44 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
> ---
> 
> Added tests for `GROW_VOLUME` and `SHRINK_VOLUME` operations.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
> 
> 
> Diff: https://reviews.apache.org/r/66220/diff/10/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66858: Added tests for validation of `GrowVolume` and `ShrinkVolume`.

[Chun-Hung Hsiao]

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




src/tests/master_validation_tests.cpp
Lines 1497 (patched)


We usually use backticks for field names in comments. Ditto below.



src/tests/master_validation_tests.cpp
Lines 1528 (patched)


s/`has incompatible disk`/`are incompatible`/



src/tests/master_validation_tests.cpp
Lines 1535 (patched)


Suggestion: Make the volume on a PATH disk so it cannot be grown with a 
ROOT disk.



src/tests/master_validation_tests.cpp
Lines 1552 (patched)


s/`for`/`if`/



src/tests/master_validation_tests.cpp
Lines 1566 (patched)


Suggestion: remove "for 'GrowVolume'" for consistency.

Also: if the agent has no RESIZE_VOLUME capability.



src/tests/master_validation_tests.cpp
Lines 1655 (patched)


s/`for`/`if`/



src/tests/master_validation_tests.cpp
Lines 1657 (patched)


Hmm... we don't have this check when validating `GrowVolume` because we are 
counting on the fact that adding anything disk to a MOUNT disk will result in 
two resources. But I guess for the validation test maybe it's a good idea to 
also test this for `GrowVolume`. What do you think?



src/tests/master_validation_tests.cpp
Lines 1677 (patched)


s/`for`/`if`/



src/tests/master_validation_tests.cpp
Lines 1692 (patched)


Suggestion: remove "for 'ShrinkVolume'" for consistency.

Also: if the agent has no RESIZE_VOLUME capability.


- Chun-Hung Hsiao


On April 27, 2018, 10:07 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66858/
> ---
> 
> (Updated April 27, 2018, 10:07 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests for validation of `GrowVolume` and `ShrinkVolume`.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> a5229610a81dfac9d358597135b63ee51de86659 
> 
> 
> Diff: https://reviews.apache.org/r/66858/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

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

[Chun-Hung Hsiao]

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


Fix it, then Ship it!





src/tests/api_tests.cpp
Lines 3684 (patched)


Suggestion: Test growing a persistent volume through the master operator 
API.



src/tests/api_tests.cpp
Lines 3685 (patched)


Suggestion: This test is disabled ...

Also, is it true that persistent volumes are not supported on Windows yet? 
If so, why is the `CreateAndDestroyVolumes` test not disabled on Windows?



src/tests/api_tests.cpp
Lines 3688 (patched)


Since we don't specify any special flag for this test, this is not 
required. The default `StartMaster()` implementation will call the overridden 
`MasterAPITest::CreateMasterFlags()` to use the long allocation interval.



src/tests/api_tests.cpp
Lines 3700 (patched)


s/`Static`/`static`/



src/tests/api_tests.cpp
Lines 3745 (patched)


Missing a period at the end of the comment. See: 
http://mesos.apache.org/documentation/latest/c++-style-guide/#comments



src/tests/api_tests.cpp
Lines 3787-3793 (patched)


Alternatively, we could do:
```
RepeatedPtrField agentResources = devolve(
v1GetAgentsResponse->get_agents().agents(0).total_resources());
upgradeResources();
```



src/tests/api_tests.cpp
Lines 3801 (patched)


Suggestion: Test shrinking a persistent volume through the master operator 
API.



src/tests/api_tests.cpp
Lines 3802 (patched)


Suggestion: This test is disabled ...

Also, is it true that persistent volumes are not supported on Windows yet? 
If so, why is the `CreateAndDestroyVolumes` test not disabled on Windows?



src/tests/api_tests.cpp
Lines 3805 (patched)


This is not required.



src/tests/api_tests.cpp
Lines 3817 (patched)


s/`Static`/`static`/



src/tests/api_tests.cpp
Lines 3902-3908 (patched)


Alternatively, we could do:
```
RepeatedPtrField agentResources = devolve(
v1GetAgentsResponse->get_agents().agents(0).total_resources());
upgradeResources();
```


- Chun-Hung Hsiao


On April 23, 2018, 8:24 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66227/
> ---
> 
> (Updated April 23, 2018, 8: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
> -
> 
>   src/tests/api_tests.cpp dd8e221d8fd1b2a241505345337897e4ee4a6347 
> 
> 
> Diff: https://reviews.apache.org/r/66227/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66900: Avoid copying of re-register framework messages in the master.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66900 was successfully built and tested.

Reviews applied: `['66860', '66900']`

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

- Mesos Reviewbot Windows


On May 1, 2018, 11:46 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66900/
> ---
> 
> (Updated May 1, 2018, 11:46 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8628
> https://issues.apache.org/jira/browse/MESOS-8628
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prior to this patch, the re-register framework message was copied
> into a subscribe call. This updates the handler to perform moves
> instead.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
>   src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 
> 
> 
> Diff: https://reviews.apache.org/r/66900/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Review Request 66900: Avoid copying of re-register framework messages in the master.

[Meng Zhu]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Prior to this patch, the re-register framework message was copied
into a subscribe call. This updates the handler to perform moves
instead.


Diffs
-

  src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
  src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 


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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66860: Avoid copying of register framework messages in the master.

[Meng Zhu]

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

(Updated May 1, 2018, 4:44 p.m.)


Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Prior to this patch, the register framework message was copied
into a subscribe call. This updates the handler to perform moves
instead.


Diffs (updated)
-

  src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
  src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66858: Added tests for validation of `GrowVolume` and `ShrinkVolume`.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 66531.

Failed command: `python.exe .\support\apply-reviews.py -n -r 66531`

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

Relevant logs:

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

```
error: patch failed: src/master/master.cpp:4907
error: src/master/master.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On April 27, 2018, 3:07 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66858/
> ---
> 
> (Updated April 27, 2018, 3:07 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests for validation of `GrowVolume` and `ShrinkVolume`.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> a5229610a81dfac9d358597135b63ee51de86659 
> 
> 
> Diff: https://reviews.apache.org/r/66858/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 65784: Added validation of QuotaRequest.

[Meng Zhu]

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


Fix it, then Ship it!





src/master/quota.cpp
Lines 199-200 (patched)


Add `AllocationInfo` and `SharedInfo` check as well?


- Meng Zhu


On Feb. 23, 2018, 2:43 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65784/
> ---
> 
> (Updated Feb. 23, 2018, 2:43 p.m.)
> 
> 
> Review request for mesos, Michael Park and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This obviates the need for the old-style validation of quotaInfo,
> but leaves the latter in order to keep the v0 code path untouched.
> A TODO has been added to remove the old validation and replace it
> with the one in this patch.
> 
> 
> Diffs
> -
> 
>   src/master/quota.hpp 7f43996d9aa3d3f462a6ed750733a55d4ef770c8 
>   src/master/quota.cpp 58bab6a678bac9e41a7994ba0b7cc1ed069a8a18 
> 
> 
> Diff: https://reviews.apache.org/r/65784/diff/1/
> 
> 
> Testing
> ---
> 
> make check (test added in subsequent patch)
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 66896: Updated `csi.md`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66896 was successfully built and tested.

Reviews applied: `['66893', '66894', '66896']`

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

- Mesos Reviewbot Windows


On May 1, 2018, 2:45 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66896/
> ---
> 
> (Updated May 1, 2018, 2:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `csi.md`.
> 
> 
> Diffs
> -
> 
>   docs/csi.md 77a5edbfdc69b451f37f2d5ca7f21bafe19d7872 
> 
> 
> Diff: https://reviews.apache.org/r/66896/diff/1/
> 
> 
> Testing
> ---
> 
> This is a documentation change so no tests are needed.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66892: Added `SubprocessTest.PipeLargeOutput`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66892 was successfully built and tested.

Reviews applied: `['66423', '66424', '66425', '66426', '66427', '66428', 
'66455', '66429', '66430', '66431', '66432', '66434', '66435', '66436', 
'66437', '66433', '66438', '66439', '66440', '66442', '66443', '66444', 
'66445', '66578', '66641', '66773', '66790', '66834', '66835', '66836', 
'66892']`

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

- Mesos Reviewbot Windows


On May 1, 2018, 9:24 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66892/
> ---
> 
> (Updated May 1, 2018, 9:24 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, John Kordich, Joseph Wu, and Radhika 
> Jandhyala.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This new test checks exercises our `Subprocess::PIPE()` logic more
> thoroughly by specifically piping enough data such that a single
> memory page is insufficient to hold it. This is especially important
> on Windows because the OS-allocated buffer is the size of one memory
> page. On Windows, it also tests that the expected end-of-file
> condition, `ERROR_BROKEN_PIPE`, is set.
> 
> We also fix another use of `os::WindowsFD` in place of `int_fd`, the
> public name of the type; added an important note about why we hold
> onto the process's handle on Windows; pass the file descriptor structs
> by const-ref instead of value; and finally check the return value
> `os::close()` in `createChildProcess`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> 898326360d6b4f0a50d6ef3f7c86141d0aa70438 
>   3rdparty/libprocess/src/subprocess_windows.hpp 
> 4cc5675621f0846229e9358d70412f44138e7904 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> be99bd6a88b2abcabed8d4d16763c2e29f38eb14 
> 
> 
> Diff: https://reviews.apache.org/r/66892/diff/1/
> 
> 
> Testing
> ---
> 
> `ctest -V` on Linux and Windows (also running `make check` with Autotools).
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65785: Added a test for QuotaRequest validation.

[Meng Zhu]

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


Ship it!




Ship It!


src/tests/master_validation_tests.cpp
Lines 268 (patched)


To me it is simpler to think that "guarantee <= limit" is still a resourceS 
wise check. But the default guarantee value is zero while default limit value 
is infinite.


- Meng Zhu


On Feb. 23, 2018, 2:43 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65785/
> ---
> 
> (Updated Feb. 23, 2018, 2:43 p.m.)
> 
> 
> Review request for mesos, Michael Park and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> 77ec7fb37c9b9c168013d1d81ca862617682af6d 
> 
> 
> Diff: https://reviews.apache.org/r/65785/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 66885: Removed unnecessary `get()` accessors.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66885]

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 May 1, 2018, 9:33 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66885/
> ---
> 
> (Updated May 1, 2018, 9:33 a.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In many cases, containers that have a `get()` accessor also have
> a `->` operator that can be used to directly access their help
> object. Replace `.get().` with `->` for improved consistency and
> readability.
> 
> 
> Diffs
> -
> 
>   src/cli/resolve.cpp fe62f9fc569b77f6e5fb101ac7e2515a9f14d08a 
>   src/java/jni/org_apache_mesos_Log.cpp 
> e5e632c0255c372e2ed5622940523bd1381b7b9c 
>   src/linux/routing/diagnosis/diagnosis.cpp 
> 3e80a29427569180039719ded11194a0afc814b5 
>   src/linux/routing/filter/icmp.cpp 68a1c3486e3fceb7110769240fbca97e3325ef31 
>   src/linux/routing/filter/internal.hpp 
> b22a818517e98fecf5ddf96fd0fa2ed95ecde15d 
>   src/linux/routing/filter/ip.cpp dd8cec7de680731ff626bf928c2f85eea2ef2b7a 
>   src/linux/routing/link/internal.hpp 
> 12064076c017cb71f403625f209e463dbd49d82f 
>   src/linux/routing/link/link.cpp 5388a3d4af11083f08b79e25d439cfe33252f1b8 
>   src/linux/routing/link/veth.cpp e12af9fd9d0874dd006b0ce1ade373bba259aa78 
>   src/linux/routing/queueing/internal.hpp 
> 9fe522ee017c86af8c7b2e518cd0957af08750e4 
>   src/linux/routing/route.cpp ba39eac3aa37244c5ca79f50423423e219b81450 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 96e1a2b4133171e7a204e5d234c28ed81913261c 
>   src/slave/containerizer/mesos/isolators/windows/cpu.cpp 
> 104e35d2cba072c08be1e0ec6074c1bd43464c87 
>   src/slave/containerizer/mesos/isolators/windows/mem.cpp 
> eb6c93e5657db3758325de9192e164da5935ec60 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
> db11f9a095dec0edcdded42d1d41ebcbad68db19 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
> cc14254f3711b81a0f93e9bef77bf81d651718b9 
> 
> 
> Diff: https://reviews.apache.org/r/66885/diff/2/
> 
> 
> Testing
> ---
> 
> make check (Fedora 27)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 66834: Windows: Specialized `flags::parse`.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On May 1, 2018, 3:43 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66834/
> ---
> 
> (Updated May 1, 2018, 3:43 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Specialized `flags::parse`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/parse.hpp 
> eb6d5272ffefcfe4fe7b97f9c7c7084893269b64 
> 
> 
> Diff: https://reviews.apache.org/r/66834/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

[Zhitao Li]

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

(Updated May 1, 2018, 3:56 p.m.)


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


Changes
---

Review comments.


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


Repository: mesos


Description
---

These operator APIs is implemented as speculative for now, but
we plan to convert them to non-speculative in the future.


Diffs (updated)
-

  src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad 
  src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
  src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 


Diff: https://reviews.apache.org/r/66051/diff/16/

Changes: https://reviews.apache.org/r/66051/diff/15-16/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66532: Added test for authorization actions for `RESIZE_VOLUME`.

[Zhitao Li]

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




src/tests/persistent_volume_tests.cpp
Line 1212 (original), 1133 (patched)


For my self: consistency.


- Zhitao Li


On May 1, 2018, 3:45 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66532/
> ---
> 
> (Updated May 1, 2018, 3:45 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8748
> https://issues.apache.org/jira/browse/MESOS-8748
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for authorization actions for `RESIZE_VOLUME`.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp a76ad18a54ec232e6d6c92ec6a45b445a83f174c 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
> 
> 
> Diff: https://reviews.apache.org/r/66532/diff/8/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66532: Added test for authorization actions for `RESIZE_VOLUME`.

[Zhitao Li]

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

(Updated May 1, 2018, 3:55 p.m.)


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


Changes
---

Several more review comments.


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


Repository: mesos


Description
---

Added test for authorization actions for `RESIZE_VOLUME`.


Diffs (updated)
-

  src/tests/authorization_tests.cpp a76ad18a54ec232e6d6c92ec6a45b445a83f174c 
  src/tests/persistent_volume_tests.cpp 
4edf781711d9efdb994114aeb6289b6af750b87a 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66220: Added tests for `GROW_VOLUME` and `SHRINK_VOLUME` operations.

[Zhitao Li]

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

(Updated May 1, 2018, 3:44 p.m.)


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


Changes
---

Several more review comments.


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


Repository: mesos


Description
---

Added tests for `GROW_VOLUME` and `SHRINK_VOLUME` operations.


Diffs (updated)
-

  src/tests/persistent_volume_tests.cpp 
4edf781711d9efdb994114aeb6289b6af750b87a 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66790: Windows: Ported the rest of the `SubprocessTest` suite.

[Andrew Schwartzmeyer]

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

(Updated May 1, 2018, 3:42 p.m.)


Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, and Joseph Wu.


Changes
---

Enabled Flags test.


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


Repository: mesos


Description
---

These tests mostly "just worked" on Windows, with only minor changes,
such as converting POSIX shell-script to Batch or PowerShell, and
expecting Windows line-endings and quoting semantics.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
be99bd6a88b2abcabed8d4d16763c2e29f38eb14 


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

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


Testing (updated)
---

~~13~~ ~~14~~ 15 more tests running on Windows :)


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66532: Added test for authorization actions for `RESIZE_VOLUME`.

[Zhitao Li]

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

(Updated May 1, 2018, 3:45 p.m.)


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


Changes
---

Review comments.


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


Repository: mesos


Description
---

Added test for authorization actions for `RESIZE_VOLUME`.


Diffs (updated)
-

  src/tests/authorization_tests.cpp a76ad18a54ec232e6d6c92ec6a45b445a83f174c 
  src/tests/persistent_volume_tests.cpp 
4edf781711d9efdb994114aeb6289b6af750b87a 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66834: Windows: Specialized `flags::parse`.

[Andrew Schwartzmeyer]

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

(Updated May 1, 2018, 3:43 p.m.)


Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, and Joseph Wu.


Changes
---

Return error if string doesn't split into exactly two parts.


Repository: mesos


Description
---

Windows: Specialized `flags::parse`.


Diffs (updated)
-

  3rdparty/stout/include/stout/flags/parse.hpp 
eb6d5272ffefcfe4fe7b97f9c7c7084893269b64 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66220: Added tests for `GROW_VOLUME` and `SHRINK_VOLUME` operations.

[Zhitao Li]

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

(Updated May 1, 2018, 3:37 p.m.)


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


Changes
---

Review comments.


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


Repository: mesos


Description
---

Added tests for `GROW_VOLUME` and `SHRINK_VOLUME` operations.


Diffs (updated)
-

  src/tests/persistent_volume_tests.cpp 
4edf781711d9efdb994114aeb6289b6af750b87a 


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

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


Testing
---


Thanks,

Zhitao Li

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

[Zhitao Li]

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

(Updated May 1, 2018, 3:35 p.m.)


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


Changes
---

Review comments.


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 (updated)
-

  src/common/protobuf_utils.cpp 78bffd8595f0e9f34e981548d8136ff94160573b 
  src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
  src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 
  src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
  src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 


Diff: https://reviews.apache.org/r/66050/diff/15/

Changes: https://reviews.apache.org/r/66050/diff/14-15/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66892: Added `SubprocessTest.PipeLargeOutput`.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On May 1, 2018, 2:24 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66892/
> ---
> 
> (Updated May 1, 2018, 2:24 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, John Kordich, Joseph Wu, and Radhika 
> Jandhyala.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This new test checks exercises our `Subprocess::PIPE()` logic more
> thoroughly by specifically piping enough data such that a single
> memory page is insufficient to hold it. This is especially important
> on Windows because the OS-allocated buffer is the size of one memory
> page. On Windows, it also tests that the expected end-of-file
> condition, `ERROR_BROKEN_PIPE`, is set.
> 
> We also fix another use of `os::WindowsFD` in place of `int_fd`, the
> public name of the type; added an important note about why we hold
> onto the process's handle on Windows; pass the file descriptor structs
> by const-ref instead of value; and finally check the return value
> `os::close()` in `createChildProcess`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> 898326360d6b4f0a50d6ef3f7c86141d0aa70438 
>   3rdparty/libprocess/src/subprocess_windows.hpp 
> 4cc5675621f0846229e9358d70412f44138e7904 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> be99bd6a88b2abcabed8d4d16763c2e29f38eb14 
> 
> 
> Diff: https://reviews.apache.org/r/66892/diff/1/
> 
> 
> Testing
> ---
> 
> `ctest -V` on Linux and Windows (also running `make check` with Autotools).
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66881: Added benchmark test for master metrics.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66881 was successfully built and tested.

Reviews applied: `['66819', '66820', '66821', '66822', '66823', '66845', 
'66824', '66825', '66846', '66847', '66841', '66842', '66843', '66844', 
'66855', '66861', '66856', '66870', '66874', '66880', '66881']`

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

- Mesos Reviewbot Windows


On May 1, 2018, 1:53 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66881/
> ---
> 
> (Updated May 1, 2018, 1:53 p.m.)
> 
> 
> Review request for Benjamin Mahler, Gaston Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added benchmark test for master metrics.
> 
> 
> Diffs
> -
> 
>   src/tests/master_benchmarks.cpp 80de24da86010e1f04026dfd05706a295b74cbd8 
> 
> 
> Diff: https://reviews.apache.org/r/66881/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 66790: Windows: Ported the rest of the `SubprocessTest` suite.

[Andrew Schwartzmeyer]

> On May 1, 2018, 2:39 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/tests/subprocess_tests.cpp
> > Lines 651-652 (original), 697-698 (patched)
> > 
> >
> > What's wrong with this test?  (You might want to expand the TODO a bit)

I don't know what's wrong with it! I've naively tried just using `cmd.exe` and 
`{cmd.exe, cmd.exe, echo}` but some of the assertions fail. I was punting it, 
but am now debugging it.


- Andrew


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


On April 30, 2018, 2:11 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66790/
> ---
> 
> (Updated April 30, 2018, 2:11 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-8833
> https://issues.apache.org/jira/browse/MESOS-8833
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests mostly "just worked" on Windows, with only minor changes,
> such as converting POSIX shell-script to Batch or PowerShell, and
> expecting Windows line-endings and quoting semantics.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> be99bd6a88b2abcabed8d4d16763c2e29f38eb14 
> 
> 
> Diff: https://reviews.apache.org/r/66790/diff/2/
> 
> 
> Testing
> ---
> 
> ~~13~~ 14 more tests running on Windows :)
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66834: Windows: Specialized `flags::parse`.

[Andrew Schwartzmeyer]

> On May 1, 2018, 2:48 p.m., Joseph Wu wrote:
> > 3rdparty/stout/include/stout/flags/parse.hpp
> > Lines 197-200 (patched)
> > 
> >
> > There should be a check for exactly 2 tokens here.  Instead of using 
> > the `maxTokens` parameter of `strings::split`.
> > 
> > The existing code would crash pretty easily.

> The existing code would crash pretty easily.

The `operator>>` on `WindowsFD` would have to be manually changed to cause a 
crash here, but I hear you. I'll split and `CHECK`.


> On May 1, 2018, 2:48 p.m., Joseph Wu wrote:
> > 3rdparty/stout/include/stout/flags/parse.hpp
> > Lines 201 (patched)
> > 
> >
> > I must be missing something... Where are `parse` and 
> > `parse` defined?

It's the generic template above; uses an `istringstream` which handles `void*` 
and `unisgned __int64` in the STL.


- Andrew


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


On April 26, 2018, 9:22 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66834/
> ---
> 
> (Updated April 26, 2018, 9:22 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Specialized `flags::parse`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/parse.hpp 
> eb6d5272ffefcfe4fe7b97f9c7c7084893269b64 
> 
> 
> Diff: https://reviews.apache.org/r/66834/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

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

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


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 66836: Fixed `mesos-tcp-connect` to use `net::socket`.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On April 26, 2018, 9:22 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66836/
> ---
> 
> (Updated April 26, 2018, 9:22 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use the stout wrapper instead of `::socket` so we have built-in error
> checking (and don't have to worry about types).
> 
> 
> Diffs
> -
> 
>   src/checks/tcp_connect.cpp f5df732415b1e3ac02e9624909d822febe60fe8c 
> 
> 
> Diff: https://reviews.apache.org/r/66836/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66835: Replaced `int` and `HANDLE` types with `int_fd`.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On April 26, 2018, 9:22 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66835/
> ---
> 
> (Updated April 26, 2018, 9:22 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced `int` and `HANDLE` types with `int_fd`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.hpp 
> 0f66d6b49cbea9964457e890eff3cc7f1e058118 
> 
> 
> Diff: https://reviews.apache.org/r/66835/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66834: Windows: Specialized `flags::parse`.

[Joseph Wu]

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




3rdparty/stout/include/stout/flags/parse.hpp
Lines 197-200 (patched)


There should be a check for exactly 2 tokens here.  Instead of using the 
`maxTokens` parameter of `strings::split`.

The existing code would crash pretty easily.



3rdparty/stout/include/stout/flags/parse.hpp
Lines 201 (patched)


I must be missing something... Where are `parse` and 
`parse` defined?


- Joseph Wu


On April 26, 2018, 9:22 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66834/
> ---
> 
> (Updated April 26, 2018, 9:22 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Specialized `flags::parse`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/parse.hpp 
> eb6d5272ffefcfe4fe7b97f9c7c7084893269b64 
> 
> 
> Diff: https://reviews.apache.org/r/66834/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66893: Documented the changes in gRPC and CSI supports.

[Chun-Hung Hsiao]

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

(Updated May 1, 2018, 9:47 p.m.)


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


Repository: mesos


Description
---

Documented the changes in gRPC and CSI supports.


Diffs (updated)
-

  docs/upgrades.md 2de8900d4a7d192fad959da8a1b6929067a6bfc9 


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

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


Testing
---

This is a documentation change so no tests needed.


Thanks,

Chun-Hung Hsiao

Review Request 66896: Updated `csi.md`.

[Chun-Hung Hsiao]

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

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


Repository: mesos


Description
---

Updated `csi.md`.


Diffs
-

  docs/csi.md 77a5edbfdc69b451f37f2d5ca7f21bafe19d7872 


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


Testing
---

This is a documentation change so no tests are needed.


Thanks,

Chun-Hung Hsiao

Re: Review Request 66790: Windows: Ported the rest of the `SubprocessTest` suite.

[Joseph Wu]

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


Fix it, then Ship it!





3rdparty/libprocess/src/tests/subprocess_tests.cpp
Lines 651-652 (original), 697-698 (patched)


What's wrong with this test?  (You might want to expand the TODO a bit)


- Joseph Wu


On April 30, 2018, 2:11 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66790/
> ---
> 
> (Updated April 30, 2018, 2:11 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-8833
> https://issues.apache.org/jira/browse/MESOS-8833
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests mostly "just worked" on Windows, with only minor changes,
> such as converting POSIX shell-script to Batch or PowerShell, and
> expecting Windows line-endings and quoting semantics.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> be99bd6a88b2abcabed8d4d16763c2e29f38eb14 
> 
> 
> Diff: https://reviews.apache.org/r/66790/diff/2/
> 
> 
> Testing
> ---
> 
> ~~13~~ 14 more tests running on Windows :)
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66892: Added `SubprocessTest.PipeLargeOutput`.

[radhika jandhyala via Review Board]

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


Ship it!




Ship It!

- radhika jandhyala


On May 1, 2018, 9:24 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66892/
> ---
> 
> (Updated May 1, 2018, 9:24 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, John Kordich, Joseph Wu, and radhika 
> jandhyala.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This new test checks exercises our `Subprocess::PIPE()` logic more
> thoroughly by specifically piping enough data such that a single
> memory page is insufficient to hold it. This is especially important
> on Windows because the OS-allocated buffer is the size of one memory
> page. On Windows, it also tests that the expected end-of-file
> condition, `ERROR_BROKEN_PIPE`, is set.
> 
> We also fix another use of `os::WindowsFD` in place of `int_fd`, the
> public name of the type; added an important note about why we hold
> onto the process's handle on Windows; pass the file descriptor structs
> by const-ref instead of value; and finally check the return value
> `os::close()` in `createChildProcess`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> 898326360d6b4f0a50d6ef3f7c86141d0aa70438 
>   3rdparty/libprocess/src/subprocess_windows.hpp 
> 4cc5675621f0846229e9358d70412f44138e7904 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> be99bd6a88b2abcabed8d4d16763c2e29f38eb14 
> 
> 
> Diff: https://reviews.apache.org/r/66892/diff/1/
> 
> 
> Testing
> ---
> 
> `ctest -V` on Linux and Windows (also running `make check` with Autotools).
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66892: Added `SubprocessTest.PipeLargeOutput`.

[John Kordich via Review Board]

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


Ship it!




Ship It!

- John Kordich


On May 1, 2018, 9:24 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66892/
> ---
> 
> (Updated May 1, 2018, 9:24 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, John Kordich, Joseph Wu, and radhika 
> jandhyala.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This new test checks exercises our `Subprocess::PIPE()` logic more
> thoroughly by specifically piping enough data such that a single
> memory page is insufficient to hold it. This is especially important
> on Windows because the OS-allocated buffer is the size of one memory
> page. On Windows, it also tests that the expected end-of-file
> condition, `ERROR_BROKEN_PIPE`, is set.
> 
> We also fix another use of `os::WindowsFD` in place of `int_fd`, the
> public name of the type; added an important note about why we hold
> onto the process's handle on Windows; pass the file descriptor structs
> by const-ref instead of value; and finally check the return value
> `os::close()` in `createChildProcess`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> 898326360d6b4f0a50d6ef3f7c86141d0aa70438 
>   3rdparty/libprocess/src/subprocess_windows.hpp 
> 4cc5675621f0846229e9358d70412f44138e7904 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> be99bd6a88b2abcabed8d4d16763c2e29f38eb14 
> 
> 
> Diff: https://reviews.apache.org/r/66892/diff/1/
> 
> 
> Testing
> ---
> 
> `ctest -V` on Linux and Windows (also running `make check` with Autotools).
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 66894: Fixed typos in `upgrades.md`.

[Chun-Hung Hsiao]

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

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


Repository: mesos


Description
---

Fixed typos in `upgrades.md`.


Diffs
-

  docs/upgrades.md 2de8900d4a7d192fad959da8a1b6929067a6bfc9 


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


Testing
---

This is a documentation change so no tests are needed.


Thanks,

Chun-Hung Hsiao

Re: Review Request 66425: Windows: Replaced `WindowsFD` with `int_fd` typedef.

[Andrew Schwartzmeyer]

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

(Updated May 1, 2018, 2:32 p.m.)


Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


Changes
---

Pass the `Option` by const ref.


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


Repository: mesos


Description
---

The latter should be used everywhere but in the implementation for
consistency with the POSIX side of the code.

Also meant fixing the included header (and spacing).


Diffs (updated)
-

  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
6da6f8eb1382d226c8b16a8e4cbb454205ef4045 
  3rdparty/stout/include/stout/os/windows/close.hpp 
ff635e44235d63888a210cd68d49f6678a851e31 
  3rdparty/stout/include/stout/os/windows/dup.hpp 
265046cf7ffc14f7326711d295aa7dd4f0a8a1e3 
  3rdparty/stout/include/stout/os/windows/fcntl.hpp 
bf8c38acad60f9b0eb752053dcd53a9fda7b8bfa 
  3rdparty/stout/include/stout/os/windows/fsync.hpp 
8405247280b51e74a172317816096ca77fdfd1e7 
  3rdparty/stout/include/stout/os/windows/ftruncate.hpp 
fc4a8b5040d56fa9766687e44ce17fbe47d9e8f0 
  3rdparty/stout/include/stout/os/windows/pipe.hpp 
365db9480f6258a03ef2e760a19abef8ab177e58 
  3rdparty/stout/include/stout/os/windows/read.hpp 
8047ad590fcc46d3ec46b551472d8c518ae49cc1 
  3rdparty/stout/include/stout/os/windows/sendfile.hpp 
fff5872db6b3f69464e41e6d108a107e4eeabd12 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
aacd746922495f994891aa85d3e4fa95e2bd1c44 
  3rdparty/stout/include/stout/os/windows/socket.hpp 
259b05b8c85e399feaccec698d58b7d540cad368 
  3rdparty/stout/include/stout/os/windows/write.hpp 
71006489918d9495d37d2fdfdca08b40b419481a 
  3rdparty/stout/include/stout/windows/os.hpp 
900baf9be4e3089cb43e444b14b155a80bcd1591 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66893: Documented the changes in gRPC and CSI supports.

[Chun-Hung Hsiao]

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

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


Repository: mesos


Description
---

Documented the changes in gRPC and CSI supports.


Diffs
-

  docs/upgrades.md 2de8900d4a7d192fad959da8a1b6929067a6bfc9 


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


Testing
---

This is a documentation change so no tests needed.


Thanks,

Chun-Hung Hsiao

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

[Andrew Schwartzmeyer]

> 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...
> 
> Joseph Wu wrote:
> 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?

Done, in https://reviews.apache.org/r/66892/


- Andrew


---
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
> 
>

Review Request 66892: Added `SubprocessTest.PipeLargeOutput`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, John Kordich, Joseph Wu, and radhika 
jandhyala.


Repository: mesos


Description
---

This new test checks exercises our `Subprocess::PIPE()` logic more
thoroughly by specifically piping enough data such that a single
memory page is insufficient to hold it. This is especially important
on Windows because the OS-allocated buffer is the size of one memory
page. On Windows, it also tests that the expected end-of-file
condition, `ERROR_BROKEN_PIPE`, is set.

We also fix another use of `os::WindowsFD` in place of `int_fd`, the
public name of the type; added an important note about why we hold
onto the process's handle on Windows; pass the file descriptor structs
by const-ref instead of value; and finally check the return value
`os::close()` in `createChildProcess`.


Diffs
-

  3rdparty/libprocess/src/subprocess.cpp 
898326360d6b4f0a50d6ef3f7c86141d0aa70438 
  3rdparty/libprocess/src/subprocess_windows.hpp 
4cc5675621f0846229e9358d70412f44138e7904 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
be99bd6a88b2abcabed8d4d16763c2e29f38eb14 


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


Testing
---

`ctest -V` on Linux and Windows (also running `make check` with Autotools).


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66773: Windows: Fixed `os::stat::mtime()` to use `GetFileTime()`.

[Andrew Schwartzmeyer]

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

(Updated May 1, 2018, 2:23 p.m.)


Review request for mesos, Akash Gupta, John Kordich, and Joseph Wu.


Changes
---

Increased verbosity level of diagnostic message.


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


Repository: mesos


Description
---

The functions `mode()`, `dev()`, and `inode()` are unused and do not
make sense on Windows, so they were explicitly deleted. The function
`mtime()` is used and has a logical mapping, `GetFileTime()`. However,
Windows reports time differently from POSIX, so a conversion must also
be performed such that the API `os::stat::mtime()` remains consistent
with its POSIX version.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/permissions.hpp 
453e60c7268db516c2c94501e11a92fe8f490498 
  3rdparty/stout/include/stout/os/windows/stat.hpp 
c04953ee42f45dd80b6362fbeeddf4a0a20e7412 
  3rdparty/stout/tests/os_tests.cpp 4221ecdcefb5602ece20cc90b13c3f17057fcb4d 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66773: Windows: Fixed `os::stat::mtime()` to use `GetFileTime()`.

[Andrew Schwartzmeyer]

> On May 1, 2018, 12:16 p.m., Joseph Wu wrote:
> > 3rdparty/stout/include/stout/os/permissions.hpp
> > Lines 64 (patched)
> > 
> >
> > I have a feeling this will pollute the logs unless you demote it to 
> > `VLOG(2)` or similar.  It doesn't seem too useful of a warning at runtime.

Yeahhh fair enough.


- Andrew


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


On April 30, 2018, 1:52 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66773/
> ---
> 
> (Updated April 30, 2018, 1:52 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, John Kordich, and Joseph Wu.
> 
> 
> Bugs: MESOS-8275
> https://issues.apache.org/jira/browse/MESOS-8275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The functions `mode()`, `dev()`, and `inode()` are unused and do not
> make sense on Windows, so they were explicitly deleted. The function
> `mtime()` is used and has a logical mapping, `GetFileTime()`. However,
> Windows reports time differently from POSIX, so a conversion must also
> be performed such that the API `os::stat::mtime()` remains consistent
> with its POSIX version.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/permissions.hpp 
> 453e60c7268db516c2c94501e11a92fe8f490498 
>   3rdparty/stout/include/stout/os/windows/stat.hpp 
> c04953ee42f45dd80b6362fbeeddf4a0a20e7412 
>   3rdparty/stout/tests/os_tests.cpp 4221ecdcefb5602ece20cc90b13c3f17057fcb4d 
> 
> 
> Diff: https://reviews.apache.org/r/66773/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66532: Added test for authorization actions for `RESIZE_VOLUME`.

[Zhitao Li]

> On April 30, 2018, 9:05 p.m., Chun-Hung Hsiao wrote:
> > src/tests/persistent_volume_tests.cpp
> > Lines 1256 (patched)
> > 
> >
> > Suggestion: Start the first framework with `DEFAULT_CREDENTIAL`, to be 
> > consistent with your comment for the second framework.

This is implicit by using `DEFAULT_FRAMEWORK_INFO`. Are you suggesting to call 
set_principal() explicitly?


- Zhitao


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


On April 27, 2018, 1:08 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66532/
> ---
> 
> (Updated April 27, 2018, 1:08 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8748
> https://issues.apache.org/jira/browse/MESOS-8748
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for authorization actions for `RESIZE_VOLUME`.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp a76ad18a54ec232e6d6c92ec6a45b445a83f174c 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
> 
> 
> Diff: https://reviews.apache.org/r/66532/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66881: Added benchmark test for master metrics.

[Gilbert Song]

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

(Updated May 1, 2018, 1:53 p.m.)


Review request for Benjamin Mahler, Gaston Kleiman, Greg Mann, and Vinod Kone.


Repository: mesos


Description
---

Added benchmark test for master metrics.


Diffs (updated)
-

  src/tests/master_benchmarks.cpp 80de24da86010e1f04026dfd05706a295b74cbd8 


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

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


Testing
---


Thanks,

Gilbert Song

Re: Review Request 66644: Remove unknown unreachable tasks when agent reregisters.

[Jiang Yan Xu]

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



LGTM sans the unresolved issue from the previous revision!


src/tests/partition_tests.cpp
Lines 167-169 (original), 167-170 (patched)


This reads like "the `RunTaskMessage` is remove correctly" but I think the 
subject is the `task`?

Perhaps revise a bit so it says 

```
// This test verifies that if a `RunTaskMessage` dropped en route to
// an agent which later becomes unreachable, the task is removed correctly
// from the master's unreachable task records when the agent
// reregisters.
```

?



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


This seems to be exceeded the 80 character limit.



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


It doesn't look like this is used?


- Jiang Yan Xu


On April 25, 2018, 8:46 a.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66644/
> ---
> 
> (Updated April 25, 2018, 8:46 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 gets added to the unreachable tasks.
> When the agent reregisters, tasks reported by it are removed from the 
> unreachableTasks bookkeeping on the master but since the
> agent doesn't know about the dropped task so it doesn't get removed
> from the unreachableTasks leading to a master 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/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 66641: Added `FsTest.Open` to cover `os::open()`.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On April 16, 2018, 1:24 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66641/
> ---
> 
> (Updated April 16, 2018, 1:24 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, John Kordich, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `FsTest.Open` to cover `os::open()`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> c190baa2230298e428d4034b90dccffb59b4e710 
> 
> 
> Diff: https://reviews.apache.org/r/66641/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66644: Remove unknown unreachable tasks when agent reregisters.

[Jiang Yan Xu]

> On April 25, 2018, 3:39 p.m., Jiang Yan Xu wrote:
> > src/master/master.cpp
> > Lines 10577-10584 (patched)
> > 
> >
> > This is the case we don't need to use `at()` as we are modifying the 
> > hashmap, we are not concerned about adding empty entries. We can just use 
> > `[]`: 
> > http://www.cplusplus.com/reference/unordered_map/unordered_map/operator[]/
> > 
> > The following should work for all the cases here.
> > 
> > ```
> > slaves.unreachableTasks[slave->id].put(task->framework_id(), 
> > task->task_id());
> > ```
> 
> Megha Sharma wrote:
> I agree we can just use the `unordered_map::operator[]` without the empty 
> entry adding concern but `unordered_map::at` here has the same behavior. I 
> don't mind changing it but just curious here as to why we prefer the 
> `operator[]` in this case?

Because the 8 lines can be condensed into 1-2? It's not about `[]` vs. `at` per 
se but the whole condition checking. I'd say the `[]` behavior is precesely 
created for this case so we'd better use it. :)


- Jiang Yan


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


On April 25, 2018, 8:46 a.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66644/
> ---
> 
> (Updated April 25, 2018, 8:46 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 gets added to the unreachable tasks.
> When the agent reregisters, tasks reported by it are removed from the 
> unreachableTasks bookkeeping on the master but since the
> agent doesn't know about the dropped task so it doesn't get removed
> from the unreachableTasks leading to a master 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/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

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

[Mesos Reviewbot]

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



Patch looks great!

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

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 May 1, 2018, 4:43 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66293/
> ---
> 
> (Updated May 1, 2018, 4:43 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 default executor support of `max_completion_time`.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> bf849c4b636e81ec267112bff9621579998941f5 
> 
> 
> Diff: https://reviews.apache.org/r/66293/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

[Chun-Hung Hsiao]

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


Fix it, then Ship it!





src/master/http.cpp
Lines 1557 (patched)


The `volume` and `addition` fields contain



src/master/http.cpp
Lines 1625 (patched)


Suggestion: The `volume` field contains the resource required for this 
operation.


- Chun-Hung Hsiao


On April 27, 2018, 8:09 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66051/
> ---
> 
> (Updated April 27, 2018, 8:09 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
> ---
> 
> These operator APIs is implemented as speculative for now, but
> we plan to convert them to non-speculative in the future.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad 
>   src/master/master.hpp a7cadd9c97add92a0276bf64e0da941cae63fd7c 
>   src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
> 
> 
> Diff: https://reviews.apache.org/r/66051/diff/15/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66828: Introduced a push-based gauge metric.

[Benjamin Mahler]

> On May 1, 2018, 4:25 p.m., Zhitao Li wrote:
> > 3rdparty/libprocess/include/process/metrics/push_gauge.hpp
> > Lines 70-75 (patched)
> > 
> >
> > Hmm, is there a possible race condition here?
> > 
> > Consider two threads calling this with `v=2` and `v=3`, and initial 
> > value as `0`, and the order would be:
> > 
> > T1: `fetch_add(2)`, `prev` in T1 == 0;
> > T2: `fetch_add(3)`, `prev` in T2 == 2;
> > T2: `push(2 + 3)`, metric value is 5;
> > T1: `push(0 + 2)`, metric value is 2.
> > 
> > Metric value and cached value in gauge would be out of sync due to the 
> > race condition?
> 
> Zhitao Li wrote:
> My bad, realized that `push()` is only used as history. Actual sampling 
> happens through `value()` function. Please ignore my comment.

Yes it is a race in which the history can be re-ordered, we'll probably remove 
that functionality.


- Benjamin


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


On April 26, 2018, 9:54 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66828/
> ---
> 
> (Updated April 26, 2018, 9:54 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Gilbert Song, Greg Mann, James 
> Peach, and Vinod Kone.
> 
> 
> Bugs: MESOS-8851
> https://issues.apache.org/jira/browse/MESOS-8851
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A push-based gauge differs from a pull-based gauge in that the
> client is responsible for pushing the latest value into the gauge
> whenever it changes. This can be challenging in some cases as it
> requires the client to have a good handle on when the gauge value
> changes (rather than just computing the current value when asked).
> 
> It is highly recommended to use push-based gauges if possible as
> they provide significant performance benefits over pull-based
> gauges. Pull-based gauge suffer from delays getting processed on
> the event queue of a `Process`, as well as incur computation cost
> on the `Process` each time the metrics are collected. Push-based
> gauges, on the other hand, incur no cost to the owning `Process`
> when metrics are collected, and instead incur a trivial cost when
> the `Process` pushes new values in.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> cd2c3bc62df8de5b50ec2fa830b3e2634ba11e28 
>   3rdparty/libprocess/include/process/metrics/push_gauge.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66828/diff/1/
> 
> 
> Testing
> ---
> 
> Test added in the subsequent patch.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 66773: Windows: Fixed `os::stat::mtime()` to use `GetFileTime()`.

[Joseph Wu]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/os/permissions.hpp
Lines 64 (patched)


I have a feeling this will pollute the logs unless you demote it to 
`VLOG(2)` or similar.  It doesn't seem too useful of a warning at runtime.


- Joseph Wu


On April 30, 2018, 1:52 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66773/
> ---
> 
> (Updated April 30, 2018, 1:52 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, John Kordich, and Joseph Wu.
> 
> 
> Bugs: MESOS-8275
> https://issues.apache.org/jira/browse/MESOS-8275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The functions `mode()`, `dev()`, and `inode()` are unused and do not
> make sense on Windows, so they were explicitly deleted. The function
> `mtime()` is used and has a logical mapping, `GetFileTime()`. However,
> Windows reports time differently from POSIX, so a conversion must also
> be performed such that the API `os::stat::mtime()` remains consistent
> with its POSIX version.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/permissions.hpp 
> 453e60c7268db516c2c94501e11a92fe8f490498 
>   3rdparty/stout/include/stout/os/windows/stat.hpp 
> c04953ee42f45dd80b6362fbeeddf4a0a20e7412 
>   3rdparty/stout/tests/os_tests.cpp 4221ecdcefb5602ece20cc90b13c3f17057fcb4d 
> 
> 
> Diff: https://reviews.apache.org/r/66773/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66437: Windows: Removed `FD_CRT` from `WindowsFD` abstraction.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On April 26, 2018, 9:17 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66437/
> ---
> 
> (Updated April 26, 2018, 9:17 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8675 and MESOS-8683
> https://issues.apache.org/jira/browse/MESOS-8675
> https://issues.apache.org/jira/browse/MESOS-8683
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After all the CRT APIs were replaced with Windows APIs, we no longer
> needed to support the semantics of an `int` file descriptor in
> general (in the sense of opening a CRT handle that's associated with
> the actual kernel object for the given `HANDLE`). There are specific
> use cases (usually third-party code) which still require a CRT
> int-like file descriptor, which the `crt()` function explicitly
> allocates (this allocation used to be done in the constructor).
> 
> Thus the entire `FD_CRT` type was removed from the `WindowsFD`
> abstraction. It still acts like an `int` in the sense that it can be
> constructed from one and compared to one. However, construction via
> `int` only supports the standard file descriptors 0, 1, and 2 for
> `stdin`, `stdout`, and `stderr`. Any other construction creates an
> `int_fd` which holds an `INVALID_HANDLE_VALUE`. When being compared to
> an `int`, the abstraction simply returns -1 if it is invalid (based on
> the result of the `is_valid()` method) or 0 if it is valid. This is to
> support the semantics of checking validity by something like
> `if (fd < 0)` or `if (fd == -1)`.
> 
> With the deletion of the `FD_CRT` type from `WindowsFD`, all the Stout
> APIs that switched on the type were simplified, with the last of the
> CRT code deleted.
> 
> Thanks to the introduction of the private `int get_valid()` function,
> and the removal of the `FD_CRT` type, the comparison operators became
> much simpler.
> 
> Several unit tests in the `FsTest` suite became cross-platform, with
> the `Close` test being simplified to test against an `int_fd`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/close.hpp 
> ff635e44235d63888a210cd68d49f6678a851e31 
>   3rdparty/stout/include/stout/os/windows/dup.hpp 
> 265046cf7ffc14f7326711d295aa7dd4f0a8a1e3 
>   3rdparty/stout/include/stout/os/windows/fcntl.hpp 
> bf8c38acad60f9b0eb752053dcd53a9fda7b8bfa 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> d7f8cdf1ad877eb55589bf5a9e75d295f91990a7 
>   3rdparty/stout/include/stout/os/windows/pipe.hpp 
> 365db9480f6258a03ef2e760a19abef8ab177e58 
>   3rdparty/stout/include/stout/os/windows/read.hpp 
> 8047ad590fcc46d3ec46b551472d8c518ae49cc1 
>   3rdparty/stout/include/stout/os/windows/socket.hpp 
> 259b05b8c85e399feaccec698d58b7d540cad368 
>   3rdparty/stout/include/stout/os/windows/write.hpp 
> 71006489918d9495d37d2fdfdca08b40b419481a 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> c190baa2230298e428d4034b90dccffb59b4e710 
>   3rdparty/stout/tests/os/socket_tests.cpp 
> 8ea0f1238e4293a5cd313b4ee38a920b381f4022 
> 
> 
> Diff: https://reviews.apache.org/r/66437/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66885: Removed unnecessary `get()` accessors.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66885 was successfully built and tested.

Reviews applied: `['66885']`

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

- Mesos Reviewbot Windows


On May 1, 2018, 4:33 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66885/
> ---
> 
> (Updated May 1, 2018, 4:33 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In many cases, containers that have a `get()` accessor also have
> a `->` operator that can be used to directly access their help
> object. Replace `.get().` with `->` for improved consistency and
> readability.
> 
> 
> Diffs
> -
> 
>   src/cli/resolve.cpp fe62f9fc569b77f6e5fb101ac7e2515a9f14d08a 
>   src/java/jni/org_apache_mesos_Log.cpp 
> e5e632c0255c372e2ed5622940523bd1381b7b9c 
>   src/linux/routing/diagnosis/diagnosis.cpp 
> 3e80a29427569180039719ded11194a0afc814b5 
>   src/linux/routing/filter/icmp.cpp 68a1c3486e3fceb7110769240fbca97e3325ef31 
>   src/linux/routing/filter/internal.hpp 
> b22a818517e98fecf5ddf96fd0fa2ed95ecde15d 
>   src/linux/routing/filter/ip.cpp dd8cec7de680731ff626bf928c2f85eea2ef2b7a 
>   src/linux/routing/link/internal.hpp 
> 12064076c017cb71f403625f209e463dbd49d82f 
>   src/linux/routing/link/link.cpp 5388a3d4af11083f08b79e25d439cfe33252f1b8 
>   src/linux/routing/link/veth.cpp e12af9fd9d0874dd006b0ce1ade373bba259aa78 
>   src/linux/routing/queueing/internal.hpp 
> 9fe522ee017c86af8c7b2e518cd0957af08750e4 
>   src/linux/routing/route.cpp ba39eac3aa37244c5ca79f50423423e219b81450 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 96e1a2b4133171e7a204e5d234c28ed81913261c 
>   src/slave/containerizer/mesos/isolators/windows/cpu.cpp 
> 104e35d2cba072c08be1e0ec6074c1bd43464c87 
>   src/slave/containerizer/mesos/isolators/windows/mem.cpp 
> eb6c93e5657db3758325de9192e164da5935ec60 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
> db11f9a095dec0edcdded42d1d41ebcbad68db19 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
> cc14254f3711b81a0f93e9bef77bf81d651718b9 
> 
> 
> Diff: https://reviews.apache.org/r/66885/diff/2/
> 
> 
> Testing
> ---
> 
> make check (Fedora 27)
> 
> 
> Thanks,
> 
> James Peach
> 
>

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

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66293 was successfully built and tested.

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

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

- Mesos Reviewbot Windows


On May 1, 2018, 9:43 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66293/
> ---
> 
> (Updated May 1, 2018, 9:43 a.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 default executor support of `max_completion_time`.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> bf849c4b636e81ec267112bff9621579998941f5 
> 
> 
> Diff: https://reviews.apache.org/r/66293/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66875: Added an hourly timer for docker store pull latency.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [53105, 66875]

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 30, 2018, 4:32 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66875/
> ---
> 
> (Updated April 30, 2018, 4:32 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-6451
> https://issues.apache.org/jira/browse/MESOS-6451
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added pull latency tracking for docker store, which can tell
> us both latency distribution of pull as well as number of pulls.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 8b3f07f5027cb90d4b4ed401960494709d3eda5f 
> 
> 
> Diff: https://reviews.apache.org/r/66875/diff/2/
> 
> 
> Testing
> ---
> 
> Ran agent in command line and trigger several launches through 
> `mesos-execute`, observed following metrics from agent endpoint:
> 
> ```
>   "containerizer/mesos/docker_store/pull_ms": 4084.254208,
>   "containerizer/mesos/docker_store/pull_ms/count": 2,
>   "containerizer/mesos/docker_store/pull_ms/max": 4084.254208,
>   "containerizer/mesos/docker_store/pull_ms/min": 3525.044736,
>   "containerizer/mesos/docker_store/pull_ms/p50": 3804.649472,
>   "containerizer/mesos/docker_store/pull_ms/p90": 4028.3332608,
>   "containerizer/mesos/docker_store/pull_ms/p95": 4056.2937344,
>   "containerizer/mesos/docker_store/pull_ms/p99": 4078.66211328,
>   "containerizer/mesos/docker_store/pull_ms/p999": 4083.694998528,
>   "containerizer/mesos/docker_store/pull_ms/p": 4084.1982870528,
> ```
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66532: Added test for authorization actions for `RESIZE_VOLUME`.

[Chun-Hung Hsiao]

> On May 1, 2018, 4:05 a.m., Chun-Hung Hsiao wrote:
> > src/tests/persistent_volume_tests.cpp
> > Lines 1464 (patched)
> > 
> >
> > No need to resume the clock here, as the fixture teardown will resume 
> > it. However, I'm not sure if you need to resume the clock before stopping 
> > and joining `driver2`.
> 
> Zhitao Li wrote:
> I think enough tests explicitly resume the clock, so I'd like to keep it 
> unless you have issue with it.

That's fine but I was wondering if we should move this before `driver2.stop()`, 
as we do in other tests.


- Chun-Hung


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


On April 27, 2018, 8:08 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66532/
> ---
> 
> (Updated April 27, 2018, 8:08 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8748
> https://issues.apache.org/jira/browse/MESOS-8748
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for authorization actions for `RESIZE_VOLUME`.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp a76ad18a54ec232e6d6c92ec6a45b445a83f174c 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
> 
> 
> Diff: https://reviews.apache.org/r/66532/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66532: Added test for authorization actions for `RESIZE_VOLUME`.

[Zhitao Li]

> On April 30, 2018, 9:05 p.m., Chun-Hung Hsiao wrote:
> > src/tests/persistent_volume_tests.cpp
> > Lines 1464 (patched)
> > 
> >
> > No need to resume the clock here, as the fixture teardown will resume 
> > it. However, I'm not sure if you need to resume the clock before stopping 
> > and joining `driver2`.

I think enough tests explicitly resume the clock, so I'd like to keep it unless 
you have issue with it.


- Zhitao


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


On April 27, 2018, 1:08 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66532/
> ---
> 
> (Updated April 27, 2018, 1:08 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8748
> https://issues.apache.org/jira/browse/MESOS-8748
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for authorization actions for `RESIZE_VOLUME`.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp a76ad18a54ec232e6d6c92ec6a45b445a83f174c 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
> 
> 
> Diff: https://reviews.apache.org/r/66532/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66220: Added tests for `GROW_VOLUME` and `SHRINK_VOLUME` operations.

[Zhitao Li]

> On April 30, 2018, 5:20 p.m., Chun-Hung Hsiao wrote:
> > src/tests/persistent_volume_tests.cpp
> > Lines 459 (patched)
> > 
> >
> > Suggestion for the TODO: Make `MOUNT` a meaningful parameter value for 
> > this test, or create a new fixture to avoid testing against it.
> > 
> > BTW, would it be hard to do what you did for `MOUNT` in the 
> > `ShrinkVolume` test?

Yes it is harder here because there is not conceivable way for a framework to 
receive both a `MOUNT` volume and free disk resource on the same `MOUNT`, so 
framework cannot even construct such operation from offered resources.


> On April 30, 2018, 5:20 p.m., Chun-Hung Hsiao wrote:
> > src/tests/persistent_volume_tests.cpp
> > Lines 837 (patched)
> > 
> >
> > To make this test more succinct, how about doing
> > `{CREATE(volume), GROW_VOLUME(volume, addition), LAUNCH(task)}` so we 
> > don't need the subsequent `acceptOffers`?

I like this suggestion. will try it out.


- Zhitao


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


On April 27, 2018, 1:04 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66220/
> ---
> 
> (Updated April 27, 2018, 1:04 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
> ---
> 
> Added tests for `GROW_VOLUME` and `SHRINK_VOLUME` operations.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
> 
> 
> Diff: https://reviews.apache.org/r/66220/diff/8/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66875: Added an hourly timer for docker store pull latency.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66875 was successfully built and tested.

Reviews applied: `['53105', '66875']`

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

- Mesos Reviewbot Windows


On May 1, 2018, 1:32 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66875/
> ---
> 
> (Updated May 1, 2018, 1:32 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-6451
> https://issues.apache.org/jira/browse/MESOS-6451
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added pull latency tracking for docker store, which can tell
> us both latency distribution of pull as well as number of pulls.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 8b3f07f5027cb90d4b4ed401960494709d3eda5f 
> 
> 
> Diff: https://reviews.apache.org/r/66875/diff/2/
> 
> 
> Testing
> ---
> 
> Ran agent in command line and trigger several launches through 
> `mesos-execute`, observed following metrics from agent endpoint:
> 
> ```
>   "containerizer/mesos/docker_store/pull_ms": 4084.254208,
>   "containerizer/mesos/docker_store/pull_ms/count": 2,
>   "containerizer/mesos/docker_store/pull_ms/max": 4084.254208,
>   "containerizer/mesos/docker_store/pull_ms/min": 3525.044736,
>   "containerizer/mesos/docker_store/pull_ms/p50": 3804.649472,
>   "containerizer/mesos/docker_store/pull_ms/p90": 4028.3332608,
>   "containerizer/mesos/docker_store/pull_ms/p95": 4056.2937344,
>   "containerizer/mesos/docker_store/pull_ms/p99": 4078.66211328,
>   "containerizer/mesos/docker_store/pull_ms/p999": 4083.694998528,
>   "containerizer/mesos/docker_store/pull_ms/p": 4084.1982870528,
> ```
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

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

[Zhitao Li]

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

(Updated May 1, 2018, 9:43 a.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
---

Tested default executor support of `max_completion_time`.


Diffs (updated)
-

  src/tests/default_executor_tests.cpp bf849c4b636e81ec267112bff9621579998941f5 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66284: Tested `max_completion_time` support in docker executor.

[Zhitao Li]

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

(Updated May 1, 2018, 9:42 a.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
---

Tested `max_completion_time` support in docker executor.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
5e3dfdb537f52a85a82c6e8195e70a389b76aadf 


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

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


Testing
---


Thanks,

Zhitao Li

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

[Zhitao Li]

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

(Updated May 1, 2018, 9:42 a.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
---

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/4/

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


Testing
---


Thanks,

Zhitao Li

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

[Zhitao Li]

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

(Updated May 1, 2018, 9:42 a.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, 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/5/

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


Testing
---


Thanks,

Zhitao Li

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

[Zhitao Li]

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

(Updated May 1, 2018, 9:41 a.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/6/

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


Testing
---


Thanks,

Zhitao Li

Review Request 66885: Removed unnecessary `get()` accessors.

[James Peach]

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

Review request for mesos and Benjamin Bannier.


Repository: mesos


Description
---

In many cases, containers that have a `get()` accessor also have
a `->` operator that can be used to directly access their help
object. Replace `.get().` with '->' for improved consistency and
readability.


Diffs
-

  src/cli/resolve.cpp fe62f9fc569b77f6e5fb101ac7e2515a9f14d08a 
  src/java/jni/org_apache_mesos_Log.cpp 
e5e632c0255c372e2ed5622940523bd1381b7b9c 
  src/linux/routing/diagnosis/diagnosis.cpp 
3e80a29427569180039719ded11194a0afc814b5 
  src/linux/routing/filter/icmp.cpp 68a1c3486e3fceb7110769240fbca97e3325ef31 
  src/linux/routing/filter/internal.hpp 
b22a818517e98fecf5ddf96fd0fa2ed95ecde15d 
  src/linux/routing/filter/ip.cpp dd8cec7de680731ff626bf928c2f85eea2ef2b7a 
  src/linux/routing/link/internal.hpp 12064076c017cb71f403625f209e463dbd49d82f 
  src/linux/routing/link/link.cpp 5388a3d4af11083f08b79e25d439cfe33252f1b8 
  src/linux/routing/link/veth.cpp e12af9fd9d0874dd006b0ce1ade373bba259aa78 
  src/linux/routing/queueing/internal.hpp 
9fe522ee017c86af8c7b2e518cd0957af08750e4 
  src/linux/routing/route.cpp ba39eac3aa37244c5ca79f50423423e219b81450 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
96e1a2b4133171e7a204e5d234c28ed81913261c 
  src/slave/containerizer/mesos/isolators/windows/cpu.cpp 
104e35d2cba072c08be1e0ec6074c1bd43464c87 
  src/slave/containerizer/mesos/isolators/windows/mem.cpp 
eb6c93e5657db3758325de9192e164da5935ec60 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
db11f9a095dec0edcdded42d1d41ebcbad68db19 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
cc14254f3711b81a0f93e9bef77bf81d651718b9 


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


Testing
---

make check (Fedora 27)


Thanks,

James Peach

Re: Review Request 66828: Introduced a push-based gauge metric.

[Zhitao Li]

> On May 1, 2018, 9:25 a.m., Zhitao Li wrote:
> > 3rdparty/libprocess/include/process/metrics/push_gauge.hpp
> > Lines 70-75 (patched)
> > 
> >
> > Hmm, is there a possible race condition here?
> > 
> > Consider two threads calling this with `v=2` and `v=3`, and initial 
> > value as `0`, and the order would be:
> > 
> > T1: `fetch_add(2)`, `prev` in T1 == 0;
> > T2: `fetch_add(3)`, `prev` in T2 == 2;
> > T2: `push(2 + 3)`, metric value is 5;
> > T1: `push(0 + 2)`, metric value is 2.
> > 
> > Metric value and cached value in gauge would be out of sync due to the 
> > race condition?

My bad, realized that `push()` is only used as history. Actual sampling happens 
through `value()` function. Please ignore my comment.


- Zhitao


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


On April 26, 2018, 2:54 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66828/
> ---
> 
> (Updated April 26, 2018, 2:54 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Gilbert Song, Greg Mann, James 
> Peach, and Vinod Kone.
> 
> 
> Bugs: MESOS-8851
> https://issues.apache.org/jira/browse/MESOS-8851
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A push-based gauge differs from a pull-based gauge in that the
> client is responsible for pushing the latest value into the gauge
> whenever it changes. This can be challenging in some cases as it
> requires the client to have a good handle on when the gauge value
> changes (rather than just computing the current value when asked).
> 
> It is highly recommended to use push-based gauges if possible as
> they provide significant performance benefits over pull-based
> gauges. Pull-based gauge suffer from delays getting processed on
> the event queue of a `Process`, as well as incur computation cost
> on the `Process` each time the metrics are collected. Push-based
> gauges, on the other hand, incur no cost to the owning `Process`
> when metrics are collected, and instead incur a trivial cost when
> the `Process` pushes new values in.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> cd2c3bc62df8de5b50ec2fa830b3e2634ba11e28 
>   3rdparty/libprocess/include/process/metrics/push_gauge.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66828/diff/1/
> 
> 
> Testing
> ---
> 
> Test added in the subsequent patch.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 66828: Introduced a push-based gauge metric.

[Zhitao Li]

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




3rdparty/libprocess/include/process/metrics/push_gauge.hpp
Lines 70-75 (patched)


Hmm, is there a possible race condition here?

Consider two threads calling this with `v=2` and `v=3`, and initial value 
as `0`, and the order would be:

T1: `fetch_add(2)`, `prev` in T1 == 0;
T2: `fetch_add(3)`, `prev` in T2 == 2;
T2: `push(2 + 3)`, metric value is 5;
T1: `push(0 + 2)`, metric value is 2.

Metric value and cached value in gauge would be out of sync due to the race 
condition?


- Zhitao Li


On April 26, 2018, 2:54 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66828/
> ---
> 
> (Updated April 26, 2018, 2:54 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Gilbert Song, Greg Mann, James 
> Peach, and Vinod Kone.
> 
> 
> Bugs: MESOS-8851
> https://issues.apache.org/jira/browse/MESOS-8851
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A push-based gauge differs from a pull-based gauge in that the
> client is responsible for pushing the latest value into the gauge
> whenever it changes. This can be challenging in some cases as it
> requires the client to have a good handle on when the gauge value
> changes (rather than just computing the current value when asked).
> 
> It is highly recommended to use push-based gauges if possible as
> they provide significant performance benefits over pull-based
> gauges. Pull-based gauge suffer from delays getting processed on
> the event queue of a `Process`, as well as incur computation cost
> on the `Process` each time the metrics are collected. Push-based
> gauges, on the other hand, incur no cost to the owning `Process`
> when metrics are collected, and instead incur a trivial cost when
> the `Process` pushes new values in.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> cd2c3bc62df8de5b50ec2fa830b3e2634ba11e28 
>   3rdparty/libprocess/include/process/metrics/push_gauge.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66828/diff/1/
> 
> 
> Testing
> ---
> 
> Test added in the subsequent patch.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 54986: Added pull gauges for slave message queue.

[Zhitao Li]

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

(Updated May 1, 2018, 8:54 a.m.)


Review request for mesos, Eric Chung, Jason Lai, and James Peach.


Changes
---

Rebase and use PullGauge.


Summary (updated)
-

Added pull gauges for slave message queue.


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


Repository: mesos


Description (updated)
---

Added pull gauges for slave message queue.


Diffs (updated)
-

  src/slave/metrics.hpp 80b47222e05997b23041cc51037824a4cef3ec02 
  src/slave/metrics.cpp a1d01edea108ddecb92030fbd7f2e25f0fd143f0 
  src/slave/slave.hpp fb911efe4985c7bf3b340f29a5d884d476307705 
  src/tests/slave_tests.cpp ae82438a4f2f79820fe6d476e25b4915aa5f212c 


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

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


Testing
---

make check.


Thanks,

Zhitao Li

Re: Review Request 53267: Added a PullGauge to track active subscribers.

[Zhitao Li]

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

(Updated May 1, 2018, 8:54 a.m.)


Review request for mesos, Anand Mazumdar, Jason Lai, and James Peach.


Changes
---

Rebase and use PullGauge.


Summary (updated)
-

Added a PullGauge to track active subscribers.


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


Repository: mesos


Description (updated)
---

Added a PullGauge to track active subscribers.


Diffs (updated)
-

  src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
  src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


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

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


Testing
---

Ran this on master with a client keeps subscribing and disconnecting. Observes 
that gauge gets updated and new log line is printed.


Thanks,

Zhitao Li