Re: Review Request 44948: Add XFS disk resource isolator.

[Jiang Yan Xu]

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


Ship it!




Ship It!

- Jiang Yan Xu


On April 6, 2016, 3:36 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44948/
> ---
> 
> (Updated April 6, 2016, 3:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOs-4828
> https://issues.apache.org/jira/browse/MESOs-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -
> 
>   src/Makefile.am ba9cc8b683bba2ae8fe9d9c58642690f5b80afaf 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a5dd22380066aa85de04d485052084e2629681c0 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
>   src/slave/flags.hpp 69e1b01e09d2a15bee5e0745b751f47aaefe3fbe 
>   src/slave/flags.cpp 315cf47d268bce0a0255a061d64e414c736c8125 
> 
> Diff: https://reviews.apache.org/r/44948/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44950: Add XFS disk isolator documentation.

[Jiang Yan Xu]

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


Fix it, then Ship it!




It's not mentioned in the test summary so could you also verify the formatting 
in a markdown viewer?

We should follow this up with https://issues.apache.org/jira/browse/MESOS-5140.


docs/configuration.md (line 1746)


s/range/ranges/



docs/configuration.md (lines 1746 - 1747)


Also mention "Valid project IDs range from 1 to max(uint32)" before 
"(default: ...)"



docs/configuration.md (line 1747)


s/The default is `[5000-1]`/(default: `[5000-1]`)/ so it looks the 
same as the rest of doc.



docs/mesos-containerizer.md (line 94)


"not be terminated by the containerizer."



docs/mesos-containerizer.md (line 106)


Mention:

1. The XFS disk isolator is functionally similar to Posix disk isolator but 
avoids the `du` cost. It's not recommended to use them together, even though 
they shouldn't interfere with each other.
2. The XFS disk isolator doesn't natively currently support accounting-only 
mode like Posix isolator does. (I guess we don't need to mention the 
`gqnoenforce` option just yet) 
3. The slave flags mentioned in the "Posix Disk Isolator" section are not 
applicable to the XFS disk isolator.

Change the wording as you see fit.


- Jiang Yan Xu


On April 6, 2016, 3:36 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44950/
> ---
> 
> (Updated April 6, 2016, 3:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOs-4828
> https://issues.apache.org/jira/browse/MESOs-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add XFS disk isolator documentation.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c 
>   docs/mesos-containerizer.md 2fde74362f35568bb944da80a1db17e6d3ba9d7b 
> 
> Diff: https://reviews.apache.org/r/44950/diff/
> 
> 
> Testing
> ---
> 
> Make check. Source inspection.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44949: Add XFS disk isolator tests.

[Jiang Yan Xu]

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




src/tests/containerizer/xfs_quota_tests.cpp (line 338)


These more complex and integration oriented tests should all have a header 
comment so it makes it easier for 

Something like:
```
// This test verifies that a task fails to write to disk when it 
// exceeds the requested disk quota.
```



src/tests/containerizer/xfs_quota_tests.cpp (line 345)


Two spaces to continue after `=`.

Here and elsewhere in this test.



src/tests/containerizer/xfs_quota_tests.cpp (line 357)


One space between `;` and `//`.

I saw some other test code that does this but it's probably due to copying 
and pasting of bad examples. Unless it's to align with some other comments, use 
one space.

Here and elsewhere.



src/tests/containerizer/xfs_quota_tests.cpp (line 390)


s/a/an.



src/tests/containerizer/xfs_quota_tests.cpp (line 425)


`s/Future >/Future>/` here and elsewhere.



src/tests/containerizer/xfs_quota_tests.cpp (line 461)


`Timeout::in()` and `Timeout::expired()` is more suitable for this.



src/tests/containerizer/xfs_quota_tests.cpp (line 486)


With Timeout you don't need to do this.



src/tests/containerizer/xfs_quota_tests.cpp (lines 526 - 531)


Here we seem to be implying that we are expecting the task to die because 
it exceeds the disk quota but in fact we are just testing slave recovery and 
the task doesn't die because of the `;`.

How about we just request 1MB and use 1MB (or not use anything at all (just 
sleep) since we are mainly verifying if the project ID is cleared afterwards.



src/tests/containerizer/xfs_quota_tests.cpp (line 545)


Two spaces.



src/tests/containerizer/xfs_quota_tests.cpp (line 551)


This is not guaranteed if `dd` is slow to write to the disk and the rest of 
the test proceeds quickly.

I think it's OK if we don't verify this here because the two tests above 
already verified that disk usage is correctly reported and correctly capped at 
the limit.



src/tests/containerizer/xfs_quota_tests.cpp (lines 565 - 566)


This is not necessary if we

```
AWAIT_READY(slaveReregisteredMessage);
```

because reregistration only happens after `Slave::_recover`.



src/tests/containerizer/xfs_quota_tests.cpp (line 590)


Additionally we should assert there is exactly one container. Otherwise the 
foreach below can fall through without triggering any expectations if 
`sandboxees` is empty.



src/tests/containerizer/xfs_quota_tests.cpp (line 593)


`s/foreach(/foreach (/`



src/tests/containerizer/xfs_quota_tests.cpp (lines 680 - 691)


This part is not necessary. Slave considers itself fully recovered when the 
containers are fully recovered, not when the executors have all reregistered. 
Also the exchange of reconnect and reregister messages doesn't go through a 
delay so no need to advance (only the `reregisterExecutorTimeout` goes through 
a delay).

Simply removing these lines should do it.



src/tests/containerizer/xfs_quota_tests.cpp (line 702)


This is again not guaranteed. I think it's good enough if we just verify 
the limits.



src/tests/containerizer/xfs_quota_tests.cpp (line 718)


`s/foreach(/foreach (/`


- Jiang Yan Xu


On April 6, 2016, 3:37 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44949/
> ---
> 
> (Updated April 6, 2016, 3:37 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
> https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add basic XFS disk isolator tests by cloning the POSIX disk isolator
> tests and making minor changes for the differences in semantics.
> 
> 
> Diffs

Re: Review Request 37168: Add an example framework using dynamic reservation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [37168]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On April 7, 2016, 3:23 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated April 7, 2016, 3:23 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 71c4308cccfa5c56b93f6c3928dd2a1cf3ba9741 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 45808: Update metrics for SUBSCRIBE call for both PID/HTTP frameworks.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 7, 2016, 2:25 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45808/
> ---
> 
> (Updated April 7, 2016, 2:25 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-4981
> https://issues.apache.org/jira/browse/MESOS-4981
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change will cover three cases:
> 1. Directly call {re}registerFramework
> 2. Through HTTP call
> 3. Through MesosSchedulerDriver
> 
> Suggested by Benjamin Mahler to introspect framework_info.id.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 2cfa5075991bbb5df8a7e33b14767de558bda4ac 
> 
> Diff: https://reviews.apache.org/r/45808/diff/
> 
> 
> Testing
> ---
> 
> 1. I can use 'docker-no-executor-framework' to reproduce/verify the issue.
> 
> "master/messages_register_framework": 1.0,
> "master/messages_reregister_framework": 0.0,
> 
> 
> Thanks,
> 
> fan du
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

[haosdent huang]

> On April 6, 2016, 4:38 p.m., haosdent huang wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 181
> > 
> >
> > How about add a default for unexpected state? May just log the error 
> > status.
> 
> Klaus Ma wrote:
> @haosdent, I think we should avoid `default` in `switch`; so compiler 
> will check whether we missed any unexpected state.

Got it. Thanks for explanation. :-)


- haosdent


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


On April 7, 2016, 3:23 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated April 7, 2016, 3:23 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 71c4308cccfa5c56b93f6c3928dd2a1cf3ba9741 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

[Klaus Ma]

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

(Updated April 7, 2016, 11:23 a.m.)


Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.


Changes
---

Address haosdent's comments.


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


Repository: mesos


Description
---

Provide example for dynamic reservation features.


Diffs (updated)
-

  src/Makefile.am 71c4308cccfa5c56b93f6c3928dd2a1cf3ba9741 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 

Diff: https://reviews.apache.org/r/37168/diff/


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 37168: Add an example framework using dynamic reservation.

[Klaus Ma]

> On April 7, 2016, 12:38 a.m., haosdent huang wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 181
> > 
> >
> > How about add a default for unexpected state? May just log the error 
> > status.

@haosdent, I think we should avoid `default` in `switch`; so compiler will 
check whether we missed any unexpected state.


- Klaus


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


On April 6, 2016, 4:19 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated April 6, 2016, 4:19 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 45856: Fixed Nvidia GPU test build for namespace change of MasterDetector.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45856]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On April 7, 2016, 1:54 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45856/
> ---
> 
> (Updated April 7, 2016, 1:54 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-5138
> https://issues.apache.org/jira/browse/MESOS-5138
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed Nvidia GPU test build for namespace change of MasterDetector.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
> 5565227bc0b18f1891265e6eff0c5a22a0c4ab86 
> 
> Diff: https://reviews.apache.org/r/45856/diff/
> 
> 
> Testing
> ---
> 
> make check -j
> 
> ```
> sudo GTEST_FILTER="NvidiaGpuTest.ROOT_CGROUPS_NVIDIA_GPU_VerifyDeviceAccess" 
> bin/mesos-tests.sh
> SUCCESS
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 45808: Update metrics for SUBSCRIBE call for both PID/HTTP frameworks.

[fan du]

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

(Updated 四月 7, 2016, 2:25 a.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Fixup coding style by dressing braces for if and else blocks


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


Repository: mesos


Description
---

This change will cover three cases:
1. Directly call {re}registerFramework
2. Through HTTP call
3. Through MesosSchedulerDriver

Suggested by Benjamin Mahler to introspect framework_info.id.


Diffs (updated)
-

  src/master/master.cpp 2cfa5075991bbb5df8a7e33b14767de558bda4ac 

Diff: https://reviews.apache.org/r/45808/diff/


Testing
---

1. I can use 'docker-no-executor-framework' to reproduce/verify the issue.

"master/messages_register_framework": 1.0,
"master/messages_reregister_framework": 0.0,


Thanks,

fan du

Re: Review Request 45808: Update metrics for SUBSCRIBE call for both PID/HTTP frameworks.

[fan du]

> On 四月 6, 2016, 6:15 p.m., Vinod Kone wrote:
> > src/master/master.cpp, lines 2089-2092
> > 
> >
> > we always use braces around our if and else blocks.
> > 
> > if () {
> > 
> > } else {
> > 
> > }

Good to know the rule here.
Thanks!


- fan


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


On 四月 6, 2016, 8:59 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45808/
> ---
> 
> (Updated 四月 6, 2016, 8:59 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-4981
> https://issues.apache.org/jira/browse/MESOS-4981
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change will cover three cases:
> 1. Directly call {re}registerFramework
> 2. Through HTTP call
> 3. Through MesosSchedulerDriver
> 
> Suggested by Benjamin Mahler to introspect framework_info.id.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 2cfa5075991bbb5df8a7e33b14767de558bda4ac 
> 
> Diff: https://reviews.apache.org/r/45808/diff/
> 
> 
> Testing
> ---
> 
> 1. I can use 'docker-no-executor-framework' to reproduce/verify the issue.
> 
> "master/messages_register_framework": 1.0,
> "master/messages_reregister_framework": 0.0,
> 
> 
> Thanks,
> 
> fan du
> 
>

Re: Review Request 45852: Added standard metrics for GPU resources.

[haosdent huang]

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



I suggest to split webui changes from this patch.

- haosdent huang


On April 7, 2016, 1:54 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45852/
> ---
> 
> (Updated April 7, 2016, 1:54 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4624
> https://issues.apache.org/jira/browse/MESOS-4624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit also includes updates to the webui to show these metrics.
> 
> 
> Diffs
> -
> 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
>   src/slave/metrics.cpp 42c66d7d7176232ccc71f1e040bcae99900f49f8 
>   src/tests/master_tests.cpp 8f93fbaf2bfd66bbc144a85c0097f45c55ff3491 
>   src/tests/slave_tests.cpp 03bb6da687a1bf11d81619839e6730835e5c4d82 
>   src/webui/master/static/framework.html 
> ee42d1ab841a4c42d95512ee60d577b1bbb66bc8 
>   src/webui/master/static/frameworks.html 
> 15ff1e9cb6c70df8df47a1b939681abde591e010 
>   src/webui/master/static/home.html a691084f4992cda65734f5fee3b2f38349737b83 
>   src/webui/master/static/js/controllers.js 
> f92affab41f8418cd7e5ea25561a182a1761fd79 
>   src/webui/master/static/offers.html 
> 01213e9582f50072a9c729782271269f72972d28 
>   src/webui/master/static/slave.html 4419f7c166e8768040dab7dbc6fb64e1382ad272 
>   src/webui/master/static/slave_executor.html 
> 5acb676390fe4ed17369143c5a13202c0981 
>   src/webui/master/static/slave_framework.html 
> 4b2b1562f38f002b4659b4a883249f0469307323 
>   src/webui/master/static/slaves.html 
> 0cb125a7d95ccc7770916cbffa052f43e8ea3d2c 
> 
> Diff: https://reviews.apache.org/r/45852/diff/
> 
> 
> Testing
> ---
> 
> Ran:
> ```
> GTEST_FILTER="SlaveTest.MetricsInMetricsEndpoint:SlaveTest.MetricsInMetricsEndpoint"
>  make -j check
> SUCCESS
> ```
> 
> Manually opened the web UI, clicked around to make sure all the GPU metrics 
> now showed up.
> Also looked at "Inspect Element" to verify that there were no javascript 
> errors when loading.
> 
> Specifically:
> ```
> * The Resources section in the left sidebar of the main page
> * The Resources section of the table in the Frameworks tab
> * The Resources section of the table in the Slaves tab
> * The Resources section of the table in the Offers tab
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 45854: Updated the default JSON representation of a Resource to include GPUs.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On April 7, 2016, 1:54 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45854/
> ---
> 
> (Updated April 7, 2016, 1:54 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-5136
> https://issues.apache.org/jira/browse/MESOS-5136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the default JSON representation of a Resource to include GPUs.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 3748c71686c696c47f93a81e3434bbb55448e334 
> 
> Diff: https://reviews.apache.org/r/45854/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 45855: Updated docs to include references to GPUs as a first class resource.

[haosdent huang]

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




docs/attributes-resources.md (line 40)


`whole number values` means `natural number` here?



docs/attributes-resources.md (line 99)


How about sort it by alphabetically?


- haosdent huang


On April 7, 2016, 1:54 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45855/
> ---
> 
> (Updated April 7, 2016, 1:54 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-5135
> https://issues.apache.org/jira/browse/MESOS-5135
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docs to include references to GPUs as a first class resource.
> 
> 
> Diffs
> -
> 
>   docs/attributes-resources.md 26468d962440560a6b4b35f51ca248ab059ec31f 
>   docs/monitoring.md 76be32842e8b437fda40c8565a34ec4f8d8dfbcc 
> 
> Diff: https://reviews.apache.org/r/45855/diff/
> 
> 
> Testing
> ---
> 
> Looked at regenerated website and it seems to include the new documentation 
> properly.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 45856: Fixed Nvidia GPU test build for namespace change of MasterDetector.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On April 7, 2016, 1:54 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45856/
> ---
> 
> (Updated April 7, 2016, 1:54 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-5138
> https://issues.apache.org/jira/browse/MESOS-5138
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed Nvidia GPU test build for namespace change of MasterDetector.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
> 5565227bc0b18f1891265e6eff0c5a22a0c4ab86 
> 
> Diff: https://reviews.apache.org/r/45856/diff/
> 
> 
> Testing
> ---
> 
> make check -j
> 
> ```
> sudo GTEST_FILTER="NvidiaGpuTest.ROOT_CGROUPS_NVIDIA_GPU_VerifyDeviceAccess" 
> bin/mesos-tests.sh
> SUCCESS
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 45853: Removed 'dashboard.js' from the webui.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On April 7, 2016, 1:54 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45853/
> ---
> 
> (Updated April 7, 2016, 1:54 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-5137
> https://issues.apache.org/jira/browse/MESOS-5137
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This file is no longer referenced anywhere in the webui and none of
> the functions it defines are called anywhere.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/js/dashboard.js 
> d6cbb3cc5aa92249c4b17c6d77260b215203d008 
> 
> Diff: https://reviews.apache.org/r/45853/diff/
> 
> 
> Testing
> ---
> 
> The webui still appears to work in all respects without this file.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Review Request 45853: Removed 'dashboard.js' from the webui.

[Kevin Klues]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

This file is no longer referenced anywhere in the webui and none of
the functions it defines are called anywhere.


Diffs
-

  src/webui/master/static/js/dashboard.js 
d6cbb3cc5aa92249c4b17c6d77260b215203d008 

Diff: https://reviews.apache.org/r/45853/diff/


Testing
---

The webui still appears to work in all respects without this file.


Thanks,

Kevin Klues

Review Request 45855: Updated docs to include references to GPUs as a first class resource.

[Kevin Klues]

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

Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.


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


Repository: mesos


Description
---

Updated docs to include references to GPUs as a first class resource.


Diffs
-

  docs/attributes-resources.md 26468d962440560a6b4b35f51ca248ab059ec31f 
  docs/monitoring.md 76be32842e8b437fda40c8565a34ec4f8d8dfbcc 

Diff: https://reviews.apache.org/r/45855/diff/


Testing
---

Looked at regenerated website and it seems to include the new documentation 
properly.


Thanks,

Kevin Klues

Review Request 45856: Fixed Nvidia GPU test build for namespace change of MasterDetector.

[Kevin Klues]

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

Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.


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


Repository: mesos


Description
---

Fixed Nvidia GPU test build for namespace change of MasterDetector.


Diffs
-

  src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
5565227bc0b18f1891265e6eff0c5a22a0c4ab86 

Diff: https://reviews.apache.org/r/45856/diff/


Testing
---

make check -j

```
sudo GTEST_FILTER="NvidiaGpuTest.ROOT_CGROUPS_NVIDIA_GPU_VerifyDeviceAccess" 
bin/mesos-tests.sh
SUCCESS
```


Thanks,

Kevin Klues

Review Request 45854: Updated the default JSON representation of a Resource to include GPUs.

[Kevin Klues]

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

Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.


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


Repository: mesos


Description
---

Updated the default JSON representation of a Resource to include GPUs.


Diffs
-

  src/common/http.cpp 3748c71686c696c47f93a81e3434bbb55448e334 

Diff: https://reviews.apache.org/r/45854/diff/


Testing
---

make check


Thanks,

Kevin Klues

Review Request 45852: Added standard metrics for GPU resources.

[Kevin Klues]

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

Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.


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


Repository: mesos


Description
---

This commit also includes updates to the webui to show these metrics.


Diffs
-

  src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
  src/slave/metrics.cpp 42c66d7d7176232ccc71f1e040bcae99900f49f8 
  src/tests/master_tests.cpp 8f93fbaf2bfd66bbc144a85c0097f45c55ff3491 
  src/tests/slave_tests.cpp 03bb6da687a1bf11d81619839e6730835e5c4d82 
  src/webui/master/static/framework.html 
ee42d1ab841a4c42d95512ee60d577b1bbb66bc8 
  src/webui/master/static/frameworks.html 
15ff1e9cb6c70df8df47a1b939681abde591e010 
  src/webui/master/static/home.html a691084f4992cda65734f5fee3b2f38349737b83 
  src/webui/master/static/js/controllers.js 
f92affab41f8418cd7e5ea25561a182a1761fd79 
  src/webui/master/static/offers.html 01213e9582f50072a9c729782271269f72972d28 
  src/webui/master/static/slave.html 4419f7c166e8768040dab7dbc6fb64e1382ad272 
  src/webui/master/static/slave_executor.html 
5acb676390fe4ed17369143c5a13202c0981 
  src/webui/master/static/slave_framework.html 
4b2b1562f38f002b4659b4a883249f0469307323 
  src/webui/master/static/slaves.html 0cb125a7d95ccc7770916cbffa052f43e8ea3d2c 

Diff: https://reviews.apache.org/r/45852/diff/


Testing
---

Ran:
```
GTEST_FILTER="SlaveTest.MetricsInMetricsEndpoint:SlaveTest.MetricsInMetricsEndpoint"
 make -j check
SUCCESS
```

Manually opened the web UI, clicked around to make sure all the GPU metrics now 
showed up.
Also looked at "Inspect Element" to verify that there were no javascript errors 
when loading.

Specifically:
```
* The Resources section in the left sidebar of the main page
* The Resources section of the table in the Frameworks tab
* The Resources section of the table in the Slaves tab
* The Resources section of the table in the Offers tab
```


Thanks,

Kevin Klues

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

[haosdent huang]

> On April 6, 2016, 5 p.m., haosdent huang wrote:
> > src/launcher/http_command_executor.cpp, line 264
> > 
> >
> > May you add a comment why need add `Second(1)` here?
> 
> Anand Mazumdar wrote:
> It's pretty self-explanatory that it's the backoff retry timeout. No need 
> for an explicit comment.

hmm, let me drop this.


- haosdent


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


On April 5, 2016, 12:36 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44424/
> ---
> 
> (Updated April 5, 2016, 12:36 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3558
> https://issues.apache.org/jira/browse/MESOS-3558
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated http_command_executor.cpp to use v1 API.
> 
> 
> Diffs
> -
> 
>   src/launcher/http_command_executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44424/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 45768: Fixed commit message hook to skip over the commented lines.

[haosdent huang]

> On April 5, 2016, 8:01 p.m., Kevin Klues wrote:
> > support/hooks/commit-msg, line 20
> > 
> >
> > You should probably quote the $LINE variable here. You should also use 
> > a single "=", not "==". The double equals will not work on all versions of 
> > sh (most notably on mac os).
> > I would also just check the first character to equal "#" instead of 
> > comaping the whole line.
> > 
> > Also, when you hit a line with a comment, you probably shouldn't break. 
> > Instead you should continue (in case there is more text to include in the 
> > commit message later on).
> > 
> > The new loop would look like:
> > 
> > ```
> > while read LINE
> > do
> > if [[ "${LINE:0:1}" == "#" ]]; then continue; fi
> > LENGTH=$(echo $LINE | wc -c)
> > if [ "$LENGTH" -gt "73" ]; then
> > echo >&2 "Error: No line in the commit message summary may 
> > exceed 72 characters."
> > exit 1
> > fi
> > ```
> 
> Kevin Klues wrote:
> Whoops, I meant the following (this actually has the single "=" and uses 
> single "[" "]"'s around the if instead of "[[", which is also non-portable.
> 
> ```
> while read LINE
> do
> if [ "${LINE:0:1}" = "#" ]; then continue; fi
> LENGTH=$(echo $LINE | wc -c)
> if [ "$LENGTH" -gt "73" ]; then
> echo >&2 "Error: No line in the commit message summary may exceed 
> 72 characters."
> exit 1
> fi
> done
> ```
> 
> haosdent huang wrote:
> I suggset use `test "x$LINE" != "x#"`. Refer to 
> http://stackoverflow.com/questions/174119/why-do-shell-script-comparisons-often-use-xvar-xyes
> And I think `LENGTH=$(echo $LINE | wc -c)` also could replace by 
> `LENGTH=${#LINE}`.
> 
> Michael Park wrote:
> Decided to go with `"$(echo -n $LINE | head -c 1)" = "#"` to stay 
> consistent with an existing use of `FIRST_CHAR=$(echo -n $FIRST_LINE | head 
> -c 1)` above. I think at least for now it makes sense to keep `LENGTH=$(echo 
> $LINE | wc -c)` for consistency as well.

Got it, thanks for explanation.


- haosdent


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


On April 6, 2016, 9:30 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45768/
> ---
> 
> (Updated April 6, 2016, 9:30 p.m.)
> 
> 
> Review request for mesos, Joerg Schad and Vinod Kone.
> 
> 
> Bugs: MESOS-5126
> https://issues.apache.org/jira/browse/MESOS-5126
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed commit message hook to skip over the commented lines.
> 
> 
> Diffs
> -
> 
>   support/hooks/commit-msg d3d5415bf6a243def05cf79f4af2b136b9866d68 
> 
> Diff: https://reviews.apache.org/r/45768/diff/
> 
> 
> Testing
> ---
> 
> Was able to commit a patch where there were lines longer than 72 characters 
> in the commented section.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 45798: Move long lived executor to use the v1 API.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45795, 45796, 45797, 45798]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On April 6, 2016, 10:37 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45798/
> ---
> 
> (Updated April 6, 2016, 10:37 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4982
> https://issues.apache.org/jira/browse/MESOS-4982
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_executor.cpp 
> f0ba8966e198a496349e685b9e9ea296b8ed807e 
> 
> Diff: https://reviews.apache.org/r/45798/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 44950: Add XFS disk isolator documentation.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [44950, 44949, 44948, 44947, 45689, 44946, 44945]

Failed command: ./support/apply-review.sh -n -r 45689

Error:
2016-04-06 23:32:22 URL:https://reviews.apache.org/r/45689/diff/raw/ 
[1413/1413] -> "45689.patch" [1]
error: patch failed: src/tests/cluster.hpp:185
error: src/tests/cluster.hpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/12366/console

- Mesos ReviewBot


On April 6, 2016, 10:36 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44950/
> ---
> 
> (Updated April 6, 2016, 10:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOs-4828
> https://issues.apache.org/jira/browse/MESOs-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add XFS disk isolator documentation.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c 
>   docs/mesos-containerizer.md 2fde74362f35568bb944da80a1db17e6d3ba9d7b 
> 
> Diff: https://reviews.apache.org/r/44950/diff/
> 
> 
> Testing
> ---
> 
> Make check. Source inspection.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 45818: Fixed flakiness in PersistentVolumeTest.AccessPersistentVolume.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 6, 2016, 4:35 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45818/
> ---
> 
> (Updated April 6, 2016, 4:35 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5128
> https://issues.apache.org/jira/browse/MESOS-5128
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed flakiness in PersistentVolumeTest.AccessPersistentVolume.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 47867505f2f1d7a722b5a0d4ff3a0fca3188a193 
> 
> Diff: https://reviews.apache.org/r/45818/diff/
> 
> 
> Testing
> ---
> 
> `./src/mesos-tests 
> --gtest_filter="DiskResource/PersistentVolumeTest.AccessPersistentVolume/0" 
> --gtest_repeat=1000 --gtest_break_on_failure` on both OSX and Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 45067: Updated the long-lived-framework example.

[Joseph Wu]

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

(Updated April 6, 2016, 4:19 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and Vinod 
Kone.


Changes
---

Update description to fit under 72 characters per line.


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


Repository: mesos


Description (updated)
---

This gives the example `long-lived-framework` enough options to run 
outside of the build environment.

This also updates:

* The style of the framework code.
* Gives the `ExecutorInfo` some resources (needed for some cgroups 
  isolators).
* Restricts the framework to one agent. Otherwise, it would grab a 
  small chunk of every machine in the cluster.
* Adds filters for declined offers.


Diffs
-

  src/examples/long_lived_framework.cpp 
ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 

Diff: https://reviews.apache.org/r/45067/diff/


Testing
---

make check

Ran this on the master node on a Mesos cluster:
```
./long-lived-framework --master=zk://localhost:2181/mesos 
--executor_uri="https://s3.amazonaws.com/url/to/long-lived-executor"; 
--executor_command="LD_LIBRARY_PATH=/path/to/libmesos && ./long-lived-executor"
```


Thanks,

Joseph Wu

Re: Review Request 45817: Cleaned up some comments.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 6, 2016, 4:34 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45817/
> ---
> 
> (Updated April 6, 2016, 4:34 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5128
> https://issues.apache.org/jira/browse/MESOS-5128
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up some comments.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> a6e3a8b43fb7591ff6f23b019a062696c8bfb719 
>   src/tests/persistent_volume_tests.cpp 
> 47867505f2f1d7a722b5a0d4ff3a0fca3188a193 
> 
> Diff: https://reviews.apache.org/r/45817/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 45440: Added some metrics to the long-lived-framework example.

[Joseph Wu]

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

(Updated April 6, 2016, 4:19 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and Vinod 
Kone.


Changes
---

Update description to fit under 72 characters per line.


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


Repository: mesos


Description (updated)
---

Adds metrics to gauge the health of the framework.  This includes:

* uptime_secs = How long the framework has been running.
* registered = If the framework is registered.
* offers_received = A counter used to determine if the framework is 
  starved or not.
* tasks_launched = Number of tasks launched.
* abnormal_terminations = Number of terminal status updates which 
  were not `TASK_FINISHED`.

Also adds an endpoint `/framework/counters` which returns the list of 
metrics which are "counters".


Diffs
-

  src/examples/long_lived_framework.cpp 
ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 

Diff: https://reviews.apache.org/r/45440/diff/


Testing
---

make check

Also deployed this version on a test cluster.  See the previous review.


Thanks,

Joseph Wu

Re: Review Request 44084: Windows: Forked os::rename().

[Michael Park]

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




3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp (lines 211 - 
256)


Same as https://reviews.apache.org/r/45015/, I thought we concluded that 
actions such as `os::mkdir` and `os::touch` should be `ASSERT_*`, and the 
things that are being actually being tested here, i.e. `os::exists` should be 
`EXPECT_*`.


- Michael Park


On March 24, 2016, 2:59 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44084/
> ---
> 
> (Updated March 24, 2016, 2:59 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Forked os::rename().
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 7bd4bfbc2ec5922879dcefddc12137336b11be52 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 79e30ca04c6d23f92e3a2f80fbe38ae63fde3520 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rename.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/rename.hpp 
> 7b2215208bcaa5613426d8ede9cab21d1a084631 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rename.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> 4c30189bb8261ccfc699da0f31b8b1fd3e9b3c83 
> 
> Diff: https://reviews.apache.org/r/44084/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 45800: Moved long running framework to use the v1 API.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [45800, 45799, 45440, 45067]

Failed command: ./support/apply-review.sh -n -r 45067

Error:
2016-04-06 23:10:16 URL:https://reviews.apache.org/r/45067/diff/raw/ 
[13287/13287] -> "45067.patch" [1]
Total errors found: 0
Checking 1 files
Error: No line in the commit message summary may exceed 72 characters.

Full log: https://builds.apache.org/job/mesos-reviewbot/12365/console

- Mesos ReviewBot


On April 6, 2016, 5:45 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45800/
> ---
> 
> (Updated April 6, 2016, 5:45 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4982
> https://issues.apache.org/jira/browse/MESOS-4982
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary. Also removed the AuthN code. Once, the library
> supports AuthN we can bring it back.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 
> 
> Diff: https://reviews.apache.org/r/45800/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45839: Fixed commit message hook to not lint the diff in verbose mode.

[Michael Park]

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

(Updated April 6, 2016, 11:08 p.m.)


Review request for mesos, Joerg Schad, Kevin Klues, and Vinod Kone.


Changes
---

Added a comment as per Kevin's comment.


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


Repository: mesos


Description (updated)
---

Fixed commit message hook to not lint the diff in verbose mode.


Diffs (updated)
-

  support/hooks/commit-msg b32ee92da7ced33aea60ed4a02b2b98d3611fb9a 

Diff: https://reviews.apache.org/r/45839/diff/


Testing
---


Thanks,

Michael Park

Re: Review Request 45839: Fixed commit message hook to not lint the diff in verbose mode.

[Michael Park]

> On April 6, 2016, 10:21 p.m., Kevin Klues wrote:
> > Ship It!
> 
> Kevin Klues wrote:
> My only comment would be to add a comment about what this line is doing.  
> It's not obvious that this is the delimiter used in verbose mode.

Oh, good point! Thanks, will do.


- Michael


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


On April 6, 2016, 9:46 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45839/
> ---
> 
> (Updated April 6, 2016, 9:46 p.m.)
> 
> 
> Review request for mesos, Joerg Schad, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-5132
> https://issues.apache.org/jira/browse/MESOS-5132
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/hooks/commit-msg b32ee92da7ced33aea60ed4a02b2b98d3611fb9a 
> 
> Diff: https://reviews.apache.org/r/45839/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 44288: Changed MasterDetector/Contender namespace.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On April 6, 2016, 12:02 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44288/
> ---
> 
> (Updated April 6, 2016, 12:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also modified users of MasterContender and MasterDetector to use this
> namespace.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler.hpp 5b153d2acb705cc54fcca86914af72d8b2e3fb55 
>   include/mesos/v1/scheduler.hpp 52647325c30ead321da66ab595bcb5a9d133a978 
>   src/cli/resolve.cpp 257e29034abf32491511f9a4e476b6859714829d 
>   src/local/local.cpp 0d980188f933a8d543af696d8addd7ca5855413e 
>   src/master/contender.cpp 846f516edcc63affd05158e9e5235e6d97f67f15 
>   src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
>   src/master/main.cpp 181bbcb1758c0e9b83ef46496e990ce3d8c2195c 
>   src/master/master.hpp e5b16f94fa5650b9db44cd7c975adeb5871e16f6 
>   src/master/master.cpp 2cfa5075991bbb5df8a7e33b14767de558bda4ac 
>   src/sched/sched.cpp d740ef630683b61554db58e628333e8a19e9aa27 
>   src/scheduler/scheduler.cpp 5263a9c549de299434bb835c12c87615b595991a 
>   src/slave/main.cpp dc3a2d2908ec08df60651dec6bf8eac651579cea 
>   src/slave/slave.hpp 3ba335fcd31a92af9609023daae328b7f4bf5e59 
>   src/slave/slave.cpp 60f93ca2c6251190d5dadfd543f0e5c5fbee1214 
>   src/tests/authentication_tests.cpp 8143cd7a22bbdbcd0fc613cb44eae8b55fd458e7 
>   src/tests/cluster.hpp 91d3998176115002ea22a0825b1aaaee3fe0d768 
>   src/tests/cluster.cpp eefc2fa55bca1ad6a53047046fa4f5996d5c3fef 
>   src/tests/command_executor_tests.cpp 
> 970cdc39f4f2b0377d36acf2465d377d2a6e1d05 
>   src/tests/container_logger_tests.cpp 
> eff19c85d32f4cba6bbda0202f06985224ee659b 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a2ac254423449a6e5185fe83177c45ce0d08420f 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 5e2116355418f5a0716cfd1573bab48ba75df596 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> b3bd5a03266150a0cd83c966d646a32c419bf512 
>   src/tests/containerizer/isolator_tests.cpp 
> 7e16846e97e20a7ef058dd8ae4664fa1c554283d 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> be6c3a118b528c39c534da423c15e9dcbb970dbc 
>   src/tests/containerizer/port_mapping_tests.cpp 
> ce985f44cffd74a853154309397b8ee596934f78 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 74bfce40e11c6d4fe7fb70fa044556f3d8d8f6ac 
>   src/tests/containerizer/runtime_isolator_tests.cpp 
> 9f3b0b08da7cebba722062a9932fae1b5f825efb 
>   src/tests/credentials_tests.cpp b61ba2ea5df8957f12659de219f6a57cf30d987a 
>   src/tests/disk_quota_tests.cpp 7f5e32f3239db3adf6e4cec2df15ccf89b4f13f4 
>   src/tests/exception_tests.cpp a50ccf1255dee59fdbc6fb1539bd1f6429458fb4 
>   src/tests/executor_http_api_tests.cpp 
> ff7b672e03185fca8b408b8805223a314fa3e483 
>   src/tests/fault_tolerance_tests.cpp 
> 67238876952bb9a694cb04722113417a4b4eb58d 
>   src/tests/fetcher_cache_tests.cpp 798c0cf3ab517d50f1a2f3194faf0fa37954319c 
>   src/tests/gc_tests.cpp 68a789cc77b5f1a17b8d002900095b18b1794ddd 
>   src/tests/health_check_tests.cpp 35344d7a3b8fd08da8444c6441c220b02bc1eb8a 
>   src/tests/hook_tests.cpp 595991deab38c34e918601e85d250dc995d0f34c 
>   src/tests/http_fault_tolerance_tests.cpp 
> c06e07daf6d6519c10489310cd4275ae94f302c6 
>   src/tests/master_allocator_tests.cpp 
> 9d74b9ed4b6a4c708f3a044a03ce631899acf4aa 
>   src/tests/master_authorization_tests.cpp 
> 8b9b8991fbb8c5a5beb69416a9c4a4ef3525942d 
>   src/tests/master_contender_detector_tests.cpp 
> 616a2c7316adc116071feaad95e3901ff66213ad 
>   src/tests/master_maintenance_tests.cpp 
> b42a81fc2e0982e8fca669bffb798c0acda684fc 
>   src/tests/master_quota_tests.cpp c39e3ea3c4b1b46075985501d09ec1b9eaf8f9bd 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 988f1d46580ab5a707fe801824e24f94d4f50da7 
>   src/tests/master_tests.cpp bb599c952128e80e71beb2686e8665e5554856bc 
>   src/tests/master_validation_tests.cpp 
> 010479cd18643aad88671b8c6b6f53523786f7a5 
>   src/tests/mesos.hpp 3b565b45f45c84aba42aa6fb29b21f8306c49861 
>   src/tests/mesos.cpp cf38dbb05908800b3a771318fa6922548c86c1f2 
>   src/tests/metrics_tests.cpp 51b4515369b5a72cd9613bb87e7b9df9e1118e83 
>   src/tests/monitor_tests.cpp 5dcb2481ff2f1a7caf54036bc3e60c78feb982b1 
>   src/tests/oversubscription_tests.cpp 
> ba036810758d99a6fb0034c5e2bc7829e2343a44 
>   src/tests/partition_tests.cpp 349adbf67686e6044a2e6a4b673043ad74fce44e 
>   src/tests

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On April 6, 2016, 12:02 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44287/
> ---
> 
> (Updated April 6, 2016, 12:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector abstract classes can be subclassed by
> user-provided classes to allow for arbitrary contender/detector 
> implementations.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44287/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44670/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44544: Moved contender and detector definitions into separate directories.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On April 6, 2016, 12:03 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44544/
> ---
> 
> (Updated April 6, 2016, 12:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Makefile.am.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 55d3b341361bed25f3aa966d77060c88be29e5b0 
>   src/master/contender/contender.hpp PRE-CREATION 
>   src/master/contender/contender.cpp PRE-CREATION 
>   src/master/detector/detector.hpp PRE-CREATION 
>   src/master/detector/detector.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44544/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44670/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44543: Removed unnecessary MasterContender and MasterDetector definitions.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On April 6, 2016, 12:03 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44543/
> ---
> 
> (Updated April 6, 2016, 12:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector are now defined in
> include/mesos/master/contender.hpp and detector.hpp.
> 
> 
> Diffs
> -
> 
>   src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
>   src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
> 
> Diff: https://reviews.apache.org/r/44543/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44670/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 45818: Fixed flakiness in PersistentVolumeTest.AccessPersistentVolume.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45817, 45818]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On April 6, 2016, 4:35 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45818/
> ---
> 
> (Updated April 6, 2016, 4:35 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5128
> https://issues.apache.org/jira/browse/MESOS-5128
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed flakiness in PersistentVolumeTest.AccessPersistentVolume.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 47867505f2f1d7a722b5a0d4ff3a0fca3188a193 
> 
> Diff: https://reviews.apache.org/r/45818/diff/
> 
> 
> Testing
> ---
> 
> `./src/mesos-tests 
> --gtest_filter="DiskResource/PersistentVolumeTest.AccessPersistentVolume/0" 
> --gtest_repeat=1000 --gtest_break_on_failure` on both OSX and Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44546: Moved functions in promises to a common header file.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On April 6, 2016, 12:03 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44546/
> ---
> 
> (Updated April 6, 2016, 12:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved functions in promises to a common header file.
> 
> 
> Diffs
> -
> 
>   src/master/detector/standalone.cpp PRE-CREATION 
>   src/master/detector/zookeeper.cpp 9274435802d6292b183be48f42b43999476e016e 
> 
> Diff: https://reviews.apache.org/r/44546/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44670/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44670: Added master_detector and master_contender flags.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On April 6, 2016, 12:03 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44670/
> ---
> 
> (Updated April 6, 2016, 12:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The master_detector and master_contender flags allow modules to be
> used for specifying the MasterContender and MasterDetector
> implementations to use.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp f8d2cc4c6c8dab00e34ca737dbcb5b9ca3870d6d 
>   src/master/flags.cpp 06852c9de68cce5d40f294f6402f7677ee6183d3 
>   src/master/main.cpp 181bbcb1758c0e9b83ef46496e990ce3d8c2195c 
>   src/slave/flags.hpp 69e1b01e09d2a15bee5e0745b751f47aaefe3fbe 
>   src/slave/flags.cpp 315cf47d268bce0a0255a061d64e414c736c8125 
>   src/slave/main.cpp dc3a2d2908ec08df60651dec6bf8eac651579cea 
> 
> Diff: https://reviews.apache.org/r/44670/diff/
> 
> 
> Testing
> ---
> 
> In addition to all unit tests passing, we are currently using this 
> functionality in our environment with a custom consensus stack. In our world, 
> we have a C++ plugin that calls out to an HTTP REST service (implemented in 
> Java/Scala, not that it matters).
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44289: Added support for contender and detector modules.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On April 6, 2016, 12:03 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44289/
> ---
> 
> (Updated April 6, 2016, 12:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for contender and detector modules.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/contender.hpp PRE-CREATION 
>   include/mesos/module/detector.hpp PRE-CREATION 
>   src/Makefile.am 55d3b341361bed25f3aa966d77060c88be29e5b0 
>   src/examples/test_master_contender_module.cpp PRE-CREATION 
>   src/examples/test_master_detector_module.cpp PRE-CREATION 
>   src/local/local.cpp 0d980188f933a8d543af696d8addd7ca5855413e 
>   src/master/contender/zookeeper.cpp 846f516edcc63affd05158e9e5235e6d97f67f15 
>   src/master/detector/zookeeper.cpp PRE-CREATION 
>   src/module/manager.cpp 7850fd3e1e574e1a92289118e2e27c62b68eaf05 
>   src/tests/module.hpp 4b32f29f2ce76100433621a5cb6b8cc87c9b38f8 
>   src/tests/module.cpp 8cc305c0ef606b07eea39d548d3165a2bb2b042a 
> 
> Diff: https://reviews.apache.org/r/44289/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44670/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44547: Added functions in promises to the future header.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On April 6, 2016, 12:03 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44547/
> ---
> 
> (Updated April 6, 2016, 12:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added functions in promises to the future header.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> ae4f2d659f1540c50dc1975f5771c6291c3b0c57 
> 
> Diff: https://reviews.apache.org/r/44547/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44670/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44669: Implemented create methods in MasterContender and MasterDetector.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On April 6, 2016, 12:03 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44669/
> ---
> 
> (Updated April 6, 2016, 12:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The create method can be used to create a MasterContender/Detector from a
> module (specified using the --modules flag on the command line). If the
> contender/detector module names have not been specified, the method will
> fall back to its prior behavior.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
>   src/master/contender/contender.cpp PRE-CREATION 
>   src/master/detector/detector.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44669/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44670/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44545: Separated standalone and zookeeper classes.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On April 6, 2016, 12:03 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44545/
> ---
> 
> (Updated April 6, 2016, 12:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of keeping standalone and zookeper contender/detector class
> definitions and implementations in the same file, separated them. Also
> made the necessary changes in users of class headers to point to the
> new locations.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 55d3b341361bed25f3aa966d77060c88be29e5b0 
>   src/local/local.cpp 0d980188f933a8d543af696d8addd7ca5855413e 
>   src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
>   src/master/contender.cpp 846f516edcc63affd05158e9e5235e6d97f67f15 
>   src/master/contender/contender.cpp PRE-CREATION 
>   src/master/contender/standalone.hpp PRE-CREATION 
>   src/master/contender/standalone.cpp PRE-CREATION 
>   src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
>   src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
>   src/master/detector/detector.cpp PRE-CREATION 
>   src/master/detector/standalone.cpp PRE-CREATION 
>   src/master/detector/zookeeper.hpp PRE-CREATION 
>   src/master/main.cpp 181bbcb1758c0e9b83ef46496e990ce3d8c2195c 
>   src/master/master.hpp e5b16f94fa5650b9db44cd7c975adeb5871e16f6 
>   src/slave/main.cpp dc3a2d2908ec08df60651dec6bf8eac651579cea 
>   src/tests/authentication_tests.cpp 8143cd7a22bbdbcd0fc613cb44eae8b55fd458e7 
>   src/tests/cluster.cpp eefc2fa55bca1ad6a53047046fa4f5996d5c3fef 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 5e2116355418f5a0716cfd1573bab48ba75df596 
>   src/tests/containerizer/isolator_tests.cpp 
> 7e16846e97e20a7ef058dd8ae4664fa1c554283d 
>   src/tests/fault_tolerance_tests.cpp 
> 67238876952bb9a694cb04722113417a4b4eb58d 
>   src/tests/gc_tests.cpp 68a789cc77b5f1a17b8d002900095b18b1794ddd 
>   src/tests/master_allocator_tests.cpp 
> 9d74b9ed4b6a4c708f3a044a03ce631899acf4aa 
>   src/tests/master_authorization_tests.cpp 
> 8b9b8991fbb8c5a5beb69416a9c4a4ef3525942d 
>   src/tests/master_contender_detector_tests.cpp 
> 616a2c7316adc116071feaad95e3901ff66213ad 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 988f1d46580ab5a707fe801824e24f94d4f50da7 
>   src/tests/master_tests.cpp bb599c952128e80e71beb2686e8665e5554856bc 
>   src/tests/mesos.hpp 3b565b45f45c84aba42aa6fb29b21f8306c49861 
>   src/tests/oversubscription_tests.cpp 
> ba036810758d99a6fb0034c5e2bc7829e2343a44 
>   src/tests/partition_tests.cpp 349adbf67686e6044a2e6a4b673043ad74fce44e 
>   src/tests/persistent_volume_tests.cpp 
> 47867505f2f1d7a722b5a0d4ff3a0fca3188a193 
>   src/tests/reconciliation_tests.cpp 5f2a35efe039da41992057c50e0319c6827f8d22 
>   src/tests/reservation_tests.cpp a9261bdf48c0af933e7fc303b7af356a60b49506 
>   src/tests/scheduler_event_call_tests.cpp 
> 00e99777ba0294c9c12ac86594987afbd9388b51 
>   src/tests/scheduler_http_api_tests.cpp 
> b65790a9aad0ca68c6a93dd1d872442b906598fd 
>   src/tests/scheduler_tests.cpp 0c66f65825a1fff01774bf0d94b08b9c22b1fbda 
>   src/tests/slave_recovery_tests.cpp 0d59a06f8e32f3d88f6c3a222bc6756a889a142e 
>   src/tests/slave_tests.cpp 57fc50360eae85819ae6ce714b0c3c4c1867b2b8 
> 
> Diff: https://reviews.apache.org/r/44545/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44670/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 45798: Move long lived executor to use the v1 API.

[Anand Mazumdar]

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

(Updated April 6, 2016, 10:37 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comment from Vinod


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/examples/long_lived_executor.cpp f0ba8966e198a496349e685b9e9ea296b8ed807e 

Diff: https://reviews.apache.org/r/45798/diff/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated April 6, 2016, 10:37 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44949/diff/


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44950: Add XFS disk isolator documentation.

[James Peach]

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

(Updated April 6, 2016, 10:36 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


Bugs: MESOs-4828
https://issues.apache.org/jira/browse/MESOs-4828


Repository: mesos


Description
---

Add XFS disk isolator documentation.


Diffs (updated)
-

  docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c 
  docs/mesos-containerizer.md 2fde74362f35568bb944da80a1db17e6d3ba9d7b 

Diff: https://reviews.apache.org/r/44950/diff/


Testing
---

Make check. Source inspection.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

[James Peach]

> On April 6, 2016, 9:07 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/xfs/disk.cpp, line 407
> > 
> >
> > Should we assume that freeProjectId already doesn't include 
> > `info->projectId`?

No, ``freeProjectIds`` is initialized from ``totalProjectIds``, so we have to 
remove it because it is still considered in use.


- James


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


On April 6, 2016, 6:43 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44948/
> ---
> 
> (Updated April 6, 2016, 6:43 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOs-4828
> https://issues.apache.org/jira/browse/MESOs-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 55d3b341361bed25f3aa966d77060c88be29e5b0 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a5dd22380066aa85de04d485052084e2629681c0 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
>   src/slave/flags.hpp 69e1b01e09d2a15bee5e0745b751f47aaefe3fbe 
>   src/slave/flags.cpp 315cf47d268bce0a0255a061d64e414c736c8125 
> 
> Diff: https://reviews.apache.org/r/44948/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44946: Add utility functions to manipulate XFS project quotas.

[James Peach]

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

(Updated April 6, 2016, 10:36 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Add utility functions to manipulate XFS project quotas.


Diffs (updated)
-

  src/Makefile.am ba9cc8b683bba2ae8fe9d9c58642690f5b80afaf 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44946/diff/


Testing
---

Make check. Manual verification. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

[James Peach]

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

(Updated April 6, 2016, 10:36 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


Bugs: MESOs-4828
https://issues.apache.org/jira/browse/MESOs-4828


Repository: mesos


Description
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am ba9cc8b683bba2ae8fe9d9c58642690f5b80afaf 
  src/slave/containerizer/mesos/containerizer.cpp 
a5dd22380066aa85de04d485052084e2629681c0 
  src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
  src/slave/flags.hpp 69e1b01e09d2a15bee5e0745b751f47aaefe3fbe 
  src/slave/flags.cpp 315cf47d268bce0a0255a061d64e414c736c8125 

Diff: https://reviews.apache.org/r/44948/diff/


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44947: Add tests for XFS project quota utilities.

[James Peach]

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

(Updated April 6, 2016, 10:35 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Add tests for XFS project quota utilities.


Diffs (updated)
-

  src/Makefile.am ba9cc8b683bba2ae8fe9d9c58642690f5b80afaf 
  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 
  src/tests/environment.cpp acadb5bb0a1b1a9b0cee0345035b93147bf7164c 

Diff: https://reviews.apache.org/r/44947/diff/


Testing
---

Make check. Manual testing. These tests.


Thanks,

James Peach

Re: Review Request 44945: Add autoconf tests for XFS project quotas.

[James Peach]

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

(Updated April 6, 2016, 10:35 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Add autoconf tests for XFS project quotas.


Diffs (updated)
-

  configure.ac c693b825294f82ace8a14563cf2229820e159e3c 

Diff: https://reviews.apache.org/r/44945/diff/


Testing
---

Make check. Manual verification.


Thanks,

James Peach

Re: Review Request 45067: Updated the long-lived-framework example.

[Joseph Wu]

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

(Updated April 6, 2016, 3:34 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and Vinod 
Kone.


Changes
---

Accidentally reset my .reviewboardrc.  Restored description.


Summary (updated)
-

Updated the long-lived-framework example.


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


Repository: mesos


Description (updated)
---

This gives the example `long-lived-framework` enough options to run outside of 
the build environment.

This also updates:

* The style of the framework code.
* Gives the `ExecutorInfo` some resources (needed for some cgroups isolators).
* Restricts the framework to one agent. Otherwise, it would grab a small chunk 
of every machine in the cluster.
* Adds filters for declined offers.


Diffs
-

  src/examples/long_lived_framework.cpp 
ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 

Diff: https://reviews.apache.org/r/45067/diff/


Testing
---

make check

Ran this on the master node on a Mesos cluster:
```
./long-lived-framework --master=zk://localhost:2181/mesos 
--executor_uri="https://s3.amazonaws.com/url/to/long-lived-executor"; 
--executor_command="LD_LIBRARY_PATH=/path/to/libmesos && ./long-lived-executor"
```


Thanks,

Joseph Wu

Re: Review Request 45440: Added some metrics to the long-lived-framework example.

[Joseph Wu]

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

(Updated April 6, 2016, 3:34 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and Vinod 
Kone.


Changes
---

Accidentally reset my .reviewboardrc.  Restored description.


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


Repository: mesos


Description (updated)
---

Adds metrics to gauge the health of the framework.  This includes:

* uptime_secs = How long the framework has been running.
* registered = If the framework is registered.
* offers_received = A counter used to determine if the framework is starved or 
not.
* tasks_launched = Number of tasks launched.
* abnormal_terminations = Number of terminal status updates which were not 
`TASK_FINISHED`.

Also adds an endpoint `/framework/counters` which returns the list of metrics 
which are "counters".


Diffs
-

  src/examples/long_lived_framework.cpp 
ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 

Diff: https://reviews.apache.org/r/45440/diff/


Testing
---

make check

Also deployed this version on a test cluster.  See the previous review.


Thanks,

Joseph Wu

Re: Review Request 45839: Fixed commit message hook to not lint the diff in verbose mode.

[Kevin Klues]

> On April 6, 2016, 10:21 p.m., Kevin Klues wrote:
> > Ship It!

My only comment would be to add a comment about what this line is doing.  It's 
not obvious that this is the delimiter used in verbose mode.


- Kevin


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


On April 6, 2016, 9:46 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45839/
> ---
> 
> (Updated April 6, 2016, 9:46 p.m.)
> 
> 
> Review request for mesos, Joerg Schad, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-5132
> https://issues.apache.org/jira/browse/MESOS-5132
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/hooks/commit-msg b32ee92da7ced33aea60ed4a02b2b98d3611fb9a 
> 
> Diff: https://reviews.apache.org/r/45839/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 45839: Fixed commit message hook to not lint the diff in verbose mode.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On April 6, 2016, 9:46 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45839/
> ---
> 
> (Updated April 6, 2016, 9:46 p.m.)
> 
> 
> Review request for mesos, Joerg Schad, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-5132
> https://issues.apache.org/jira/browse/MESOS-5132
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/hooks/commit-msg b32ee92da7ced33aea60ed4a02b2b98d3611fb9a 
> 
> Diff: https://reviews.apache.org/r/45839/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 45067: Update the long-lived-framework example.

[Joseph Wu]

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

(Updated April 6, 2016, 3:16 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and Vinod 
Kone.


Changes
---

Commenting changes.


Summary (updated)
-

Update the long-lived-framework example.


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


Repository: mesos


Description (updated)
---

Update the long-lived-framework example.


Diffs (updated)
-

  src/examples/long_lived_framework.cpp 
ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 

Diff: https://reviews.apache.org/r/45067/diff/


Testing
---

make check

Ran this on the master node on a Mesos cluster:
```
./long-lived-framework --master=zk://localhost:2181/mesos 
--executor_uri="https://s3.amazonaws.com/url/to/long-lived-executor"; 
--executor_command="LD_LIBRARY_PATH=/path/to/libmesos && ./long-lived-executor"
```


Thanks,

Joseph Wu

Re: Review Request 45440: Added some metrics to the long-lived-framework example.

[Joseph Wu]

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

(Updated April 6, 2016, 3:16 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and Vinod 
Kone.


Changes
---

Addressed a subset of the comments.


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


Repository: mesos


Description (updated)
---

Added some metrics to the long-lived-framework example.


Diffs (updated)
-

  src/examples/long_lived_framework.cpp 
ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 

Diff: https://reviews.apache.org/r/45440/diff/


Testing
---

make check

Also deployed this version on a test cluster.  See the previous review.


Thanks,

Joseph Wu

Re: Review Request 45798: Move long lived executor to use the v1 API.

[Anand Mazumdar]

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

(Updated April 6, 2016, 10:09 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Change to log `SHUTDOWN` event. My bad had forgotten to publish.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/examples/long_lived_executor.cpp f0ba8966e198a496349e685b9e9ea296b8ed807e 

Diff: https://reviews.apache.org/r/45798/diff/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 45798: Move long lived executor to use the v1 API.

[Vinod Kone]

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


Fix it, then Ship it!





src/examples/long_lived_executor.cpp (line 98)


LOG the received event.


- Vinod Kone


On April 6, 2016, 3:43 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45798/
> ---
> 
> (Updated April 6, 2016, 3:43 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4982
> https://issues.apache.org/jira/browse/MESOS-4982
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_executor.cpp 
> f0ba8966e198a496349e685b9e9ea296b8ed807e 
> 
> Diff: https://reviews.apache.org/r/45798/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 45839: Fixed commit message hook to not lint the diff in verbose mode.

[Michael Park]

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

Review request for mesos, Joerg Schad, Kevin Klues, and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  support/hooks/commit-msg b32ee92da7ced33aea60ed4a02b2b98d3611fb9a 

Diff: https://reviews.apache.org/r/45839/diff/


Testing
---


Thanks,

Michael Park

Re: Review Request 44946: Add utility functions to manipulate XFS project quotas.

[Jiang Yan Xu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/xfs/utils.cpp (line 58)


Nit: 

`static constexpr Bytes BASIC_BLOCK_SIZE = Bytes(512);` 

for consistency just because this is how constants in Mesos are generally 
constructed (copy constructed).


- Jiang Yan Xu


On April 6, 2016, 11:42 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44946/
> ---
> 
> (Updated April 6, 2016, 11:42 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
> https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add utility functions to manipulate XFS project quotas.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 55d3b341361bed25f3aa966d77060c88be29e5b0 
>   src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44946/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual verification. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 45768: Fixed commit message hook to skip over the commented lines.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On April 6, 2016, 9:30 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45768/
> ---
> 
> (Updated April 6, 2016, 9:30 p.m.)
> 
> 
> Review request for mesos, Joerg Schad and Vinod Kone.
> 
> 
> Bugs: MESOS-5126
> https://issues.apache.org/jira/browse/MESOS-5126
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed commit message hook to skip over the commented lines.
> 
> 
> Diffs
> -
> 
>   support/hooks/commit-msg d3d5415bf6a243def05cf79f4af2b136b9866d68 
> 
> Diff: https://reviews.apache.org/r/45768/diff/
> 
> 
> Testing
> ---
> 
> Was able to commit a patch where there were lines longer than 72 characters 
> in the commented section.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 45768: Fixed commit message hook to skip over the commented lines.

[Michael Park]

> On April 5, 2016, 8:01 p.m., Kevin Klues wrote:
> > support/hooks/commit-msg, line 20
> > 
> >
> > You should probably quote the $LINE variable here. You should also use 
> > a single "=", not "==". The double equals will not work on all versions of 
> > sh (most notably on mac os).
> > I would also just check the first character to equal "#" instead of 
> > comaping the whole line.
> > 
> > Also, when you hit a line with a comment, you probably shouldn't break. 
> > Instead you should continue (in case there is more text to include in the 
> > commit message later on).
> > 
> > The new loop would look like:
> > 
> > ```
> > while read LINE
> > do
> > if [[ "${LINE:0:1}" == "#" ]]; then continue; fi
> > LENGTH=$(echo $LINE | wc -c)
> > if [ "$LENGTH" -gt "73" ]; then
> > echo >&2 "Error: No line in the commit message summary may 
> > exceed 72 characters."
> > exit 1
> > fi
> > ```
> 
> Kevin Klues wrote:
> Whoops, I meant the following (this actually has the single "=" and uses 
> single "[" "]"'s around the if instead of "[[", which is also non-portable.
> 
> ```
> while read LINE
> do
> if [ "${LINE:0:1}" = "#" ]; then continue; fi
> LENGTH=$(echo $LINE | wc -c)
> if [ "$LENGTH" -gt "73" ]; then
> echo >&2 "Error: No line in the commit message summary may exceed 
> 72 characters."
> exit 1
> fi
> done
> ```
> 
> haosdent huang wrote:
> I suggset use `test "x$LINE" != "x#"`. Refer to 
> http://stackoverflow.com/questions/174119/why-do-shell-script-comparisons-often-use-xvar-xyes
> And I think `LENGTH=$(echo $LINE | wc -c)` also could replace by 
> `LENGTH=${#LINE}`.

Decided to go with `"$(echo -n $LINE | head -c 1)" = "#"` to stay consistent 
with an existing use of `FIRST_CHAR=$(echo -n $FIRST_LINE | head -c 1)` above. 
I think at least for now it makes sense to keep `LENGTH=$(echo $LINE | wc -c)` 
for consistency as well.


- Michael


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


On April 6, 2016, 9:30 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45768/
> ---
> 
> (Updated April 6, 2016, 9:30 p.m.)
> 
> 
> Review request for mesos, Joerg Schad and Vinod Kone.
> 
> 
> Bugs: MESOS-5126
> https://issues.apache.org/jira/browse/MESOS-5126
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed commit message hook to skip over the commented lines.
> 
> 
> Diffs
> -
> 
>   support/hooks/commit-msg d3d5415bf6a243def05cf79f4af2b136b9866d68 
> 
> Diff: https://reviews.apache.org/r/45768/diff/
> 
> 
> Testing
> ---
> 
> Was able to commit a patch where there were lines longer than 72 characters 
> in the commented section.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 45768: Fixed commit message hook to skip over the commented lines.

[Michael Park]

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

(Updated April 6, 2016, 9:30 p.m.)


Review request for mesos, Joerg Schad and Vinod Kone.


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


Repository: mesos


Description (updated)
---

Fixed commit message hook to skip over the commented lines.


Diffs (updated)
-

  support/hooks/commit-msg d3d5415bf6a243def05cf79f4af2b136b9866d68 

Diff: https://reviews.apache.org/r/45768/diff/


Testing
---

Was able to commit a patch where there were lines longer than 72 characters in 
the commented section.


Thanks,

Michael Park

Re: Review Request 44948: Add XFS disk resource isolator.

[Jiang Yan Xu]

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



Mostly LGTM. Just a few additional comments.


src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 21 - 22)


Not used anymore.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 26)


This is not used.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 28 - 30)


One blank line above and below as per convention.

And I don't think any of the three are still used?



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 31)


This is not used either.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 38 - 39)


Not used anymore.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 63 - 66)


There is some awkwardness with the -1: we don't use the last element in the 
range so the check needs to be `ranges.range(i).end() - 1` as well.

However, I am thinking if we can just use the inclusive right bounds. It 
seems to me the open right bound is because of the empty interval set check, 
see my comments below.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 69)


s/open/closed/ if we agree on the comment on `nextProjectId()`.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 116)


We should always add a CHECK if we expect it to never be none. In fact 
it'll never be error either because it simply returns `::getuid()`.

It's just being clear that we intentionally don't check other conditions: 
`CHECK_SOME(uid);`, Kinda like "This page is intentionally left blank".

At this point it seems there is not much value to use `os::getuid()` rather 
than `::getuid()` but let's just keep the use of `os::getuid()` with this CHECK 
for the sake of "There should be one-- and preferably only one --obvious way" 
to get uid.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 199)


s/known/alive/

Otherwise it sounds like it encompasses both alive and known orphans and we 
should merge it with `orphans`. Keeping them separate is more explict so we 
should keep `alive` IMO.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 214 - 217)


Unless there is a collision in UUID this should never happen. I trust UUID 
but we can do a CHECK here instead. :)

```
CHECK(!infos.contains(containerId)) << "ContainerIDs should never collide";
```



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 221)


Add a comment here. Somthing along the lines of:

```
// We fail the isolator recovery upon failure in any container because
// failing to get the project ID usually suggests some fatal issue
// on the host.
```



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 234)


s/message/call later/ because it's not a 'message'.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 236)


Add 

```
// Note that we don't wait for the result of the cleanups as we don't 
// want to block agent recovery for unknown orphans.
```



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 406 - 413)


We make a best effort to cleanup but we should still propagate the failure 
to the containerizer so it can do some containerizer-wide logging and metrics 
update.

How about:
```
infos.erase(containerId);

if (quotaStatus.isError() || projectStatus.isError()) {
  return Failure("Failed to cleanup '" + info->directory + "'");
} else {
  returnProjectId(info->projectId);
  return Nothing();
}
```

?



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (line 407)


Should we assume that freeProjectId already doesn't include 
`info->projectId`?



src/slave/containerizer/mesos/isolators/xfs/disk.cpp (lines 419 - 430)


This seems better:

```
if (freeProjectIds.empty()) {
  return None();
}

prid_t projectId = freeProjectIds.begin()->lower();
free

Re: Review Request 45358: Improved overlay backend to make the rootfs writable.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45358]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On April 6, 2016, 12:12 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45358/
> ---
> 
> (Updated April 6, 2016, 12:12 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, and Jie Yu.
> 
> 
> Bugs: MESOS-4944
> https://issues.apache.org/jira/browse/MESOS-4944
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved overlay backend to make the rootfs writable.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md db4b291498309d5c0bdbf38552b985c33f28e9a0 
>   src/slave/containerizer/mesos/provisioner/backend.hpp 
> c6cca81e287bb9a62e0390f96e1773841887a206 
>   src/slave/containerizer/mesos/provisioner/backends/bind.hpp 
> 9eda944e14f2b05f28620c2e40594ed4e7bab69e 
>   src/slave/containerizer/mesos/provisioner/backends/bind.cpp 
> 9b9f0b9e99a0fc0e9bad8fb2dad41acdc0ca1da1 
>   src/slave/containerizer/mesos/provisioner/backends/copy.hpp 
> b62507f9fc757349d39f39a6654ddd69053bf0e7 
>   src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
> f353c89806816e85bb27875ef5fa68d5c0eaf9ca 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.hpp 
> 85cc737438c88b355f37611bfde50dc80efab017 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
> 93892a75fe3ddd5c7b2a023b63f6d0fa83eeb97d 
>   src/slave/containerizer/mesos/provisioner/paths.hpp 
> 2ea38ac3342e1ebb712a7bea8ebef1bdcf96e809 
>   src/slave/containerizer/mesos/provisioner/paths.cpp 
> 07581f6ac714485e9c743984a553bb6fb26756b7 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 8a4938ebe4e00779e88e7c538445e9ffd51202e2 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> d49204f220c6212f83c2adf5544d04b3386c8eb7 
> 
> Diff: https://reviews.apache.org/r/45358/diff/
> 
> 
> Testing
> ---
> 
> - make check
> - tested manually, with slave using overlay backend. Create a task with mesos 
> containerizer, ubuntu docker image, with the command `mkdir -p /abc && touch 
> /abc/def.txt`.
> - also tested manually with alpine image to verify overlay backend could 
> support 1-layer images.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 45806: Replace Master/Slave Terminology Phase I - Add duplicate binaries.

[Kevin Klues]

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



Thanks for working on this!  I've left some comments below.

In addition to these comments, could you pelase update the summary and 
description to better reflect what this change is doing now.  It's not really 
duplicating binaries, but rather renaming the exisiting files from which these 
bianreis are generated from `s/slave/agent` and updating `configure.ac` and 
`src/Makefile.am` to generate duplicate copies of the binaries (one with the 
slave naming, and one with the agent naming).


configure.ac (line 81)


I would probably move this line up to keep this grouping alphabetical.



configure.ac (lines 82 - 84)


You should probably mark this with an explicit TODO. You should probably 
also refer to it as a script rather than a 'shell'.

Also, all of these lines that wrap (including the existing ones) should 
only be using two space indents instead of a tab. if you could fix that 
throughout, that would be great.

```
# TODO(dongdong): Remove this script once the term
# 'slave' is deprecated (MESOS-3782).
AC_CONFIG_FILES([bin/gdb-mesos-slave.sh:bin/gdb-mesos-agent.sh.in],
  [chmod +x bin/gdb-mesos-slave.sh])
```



configure.ac (line 88)


Same comment about keeping the group alphabetical as above.



configure.ac (lines 89 - 90)


Same comment about the TODO as above.



configure.ac (line 94)


Same comment about keeping the group alphabetical as above.



configure.ac (lines 95 - 97)


Same comment about the TODO and the wrapping as above.



configure.ac (line 102)


Same comment about keeping the group alphabetical as above.



configure.ac (lines 103 - 105)


Same comment about the TODO and the wrapping as above.



configure.ac (line 109)


Same comment about keeping the group alphabetical as above. Also, the 
wrapping.



configure.ac (lines 111 - 112)


Same comment about the TODO as above.



configure.ac (line 121)


Same comment about keeping the group alphabetical as above.



configure.ac (lines 122 - 124)


Same comment about the TODO and the wrapping as above.



configure.ac (line 127)


Same comment about keeping the group alphabetical as above.



configure.ac (lines 128 - 130)


Same comment about the TODO and the wrapping as above.



src/Makefile.am (line 1146)


Please wrap this in a TODO as the comments above suggest.



src/Makefile.am (lines 1152 - 1156)


I would probably move this up to keep things alphabetical.



src/Makefile.am (line 1301)


I would probably move this up to keep things alphabetical in the group.



src/Makefile.am (line 1305)


Same comment as above about keeping things alphabetical.



src/Makefile.am (line 1311)


Same comment as above about keeping things alphabetical.



src/Makefile.am (line 1318)


Same comment as above about keeping things alphabetical.



src/Makefile.am (lines 2069 - 2081)


I probably wouldn't overload this make target to copy in the template.  
Instead, I'd create a new target to do the copying and then make the 
`install-data-hook` target depend on it, i.e.:

```
# Copy mesos-agent-env.sh.template to mesos-slave-env.sh.template.
#
# TODO(dongdong): Remove this target once the
# slave->agent rename is complete(MESOS-3782).
copy-agent-env-template:
cp $(pkgsysconfdir)/mesos-agent-env.sh.template \
$(pkgsysconfdir)/mesos-slave-env.sh.template

# Install compatibility symlinks for modules that used to be in $(LIBDIR)
# but are now in $(PKGMODULEDIR). We use install-data-hook because it
# runs late in the install process after the target directories have
# been

Re: Review Request 45500: Fix Mesos-1104, linux/fs.hpp remove mesos::internal:: should be enough.

[Neil Conway]

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




src/tests/containerizer/fs_tests.cpp (line 35)


Not yours, but we should sort these alphabetically.


- Neil Conway


On April 1, 2016, 5:19 a.m., Deshi Xiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45500/
> ---
> 
> (Updated April 1, 2016, 5:19 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Cong Wang.
> 
> 
> Bugs: MESOS-1104
> https://issues.apache.org/jira/browse/MESOS-1104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> follow haosdent's suggest, remove mesos::internal:: should be enough.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp b7420c682970c4838e84973198ac4fe7af5f68f9 
>   src/linux/fs.hpp 4525a5d5566e2bc913894e993ac7350f1bbd9cc0 
>   src/linux/fs.cpp dbf94759b6382bfafd3f3b8b4c2047af36a53ad5 
>   src/slave/containerizer/mesos/mount.cpp 
> bf17b015e45579882fd31248e8609eec6d58a9da 
>   src/tests/containerizer/fs_tests.cpp 
> 020fd8a4bf5911671e038a96f8b50c0f58e22ed5 
>   src/tests/containerizer/port_mapping_tests.cpp 
> e062daa9fcfc776144b48325daa1f1284c5e59a4 
> 
> Diff: https://reviews.apache.org/r/45500/diff/
> 
> 
> Testing
> ---
> 
> make test on localhost(ubuntu)
> 
> 
> Thanks,
> 
> Deshi Xiao
> 
>

Re: Review Request 45440: Added some metrics to the long-lived-framework example.

[Joseph Wu]

> On April 6, 2016, 11:55 a.m., Vinod Kone wrote:
> > src/examples/long_lived_framework.cpp, line 241
> > 
> >
> > Looking at the code below I think it makes more sense to make 
> > LongLivedScheduler an instance of process instead of making Metrics a 
> > process? That is consistent with how we did it elsewhere in the code base 
> > (e.g., Master and Slave).

I added the sub-class because the `LongLivedScheduler` already inherits from 
`Scheduler`, which does not expect a libprocess process.  And multiple 
inheritance seemed like a bad idea.


> On April 6, 2016, 11:55 a.m., Vinod Kone wrote:
> > src/examples/long_lived_framework.cpp, line 244
> > 
> >
> > why is the process based call "framework" here?

I wanted a proper name for the special metadata endpoint `/framework/counters`. 
 If I didn't add this, the endpoint would be `/(1)/counters`.


> On April 6, 2016, 11:55 a.m., Vinod Kone wrote:
> > src/examples/long_lived_framework.cpp, line 272
> > 
> >
> > s/counter/metrics/
> > 
> > I'm assuming you might want to add gauge type metrics to this endpoint 
> > in the future?
> > 
> > More importantly, I didn't understand why you need to add a special 
> > endpoint for these 2 counters. Why can't they just be part of the 
> > "/metrics/snapshot" endpoint like the other metrics?

This endpoint's sole purpose is to specify which metrics are `Counters`.  We 
still get the metric values from `/metrics/snapshot`, but it can also call this 
endpoint to decide which metrics to treat as counters.


> On April 6, 2016, 11:55 a.m., Vinod Kone wrote:
> > src/examples/long_lived_framework.cpp, lines 298-313
> > 
> >
> > seems a bit weird that these are part of the metrics class. In Master 
> > and Slave, we made these gauges part of the Master and Slave class instead.

I can't put these methods in a non-libprocess-process class since they are 
called via `defer` by the gauges.


- Joseph


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


On April 5, 2016, 3:33 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45440/
> ---
> 
> (Updated April 5, 2016, 3:33 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5062
> https://issues.apache.org/jira/browse/MESOS-5062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds metrics to gauge the health of the framework.  This includes:
> 
> * uptime_secs = How long the framework has been running.
> * registered  = If the framework is registered.
> * offers_received = A counter used to determine if the framework is starved 
> or not.
> * tasks_launched  = Number of tasks launched.
> * abnormal_terminations = Number of terminal status updates which were not 
> `TASK_FINISHED`.
> 
> Also adds an endpoint `/framework/counters` which returns the list of metrics 
> which are "counters".
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 
> 
> Diff: https://reviews.apache.org/r/45440/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Also deployed this version on a test cluster.  See the previous review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 44670: Added master_detector and master_contender flags.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [44670, 44669, 44289, 44547, 44546, 44545, 44544, 44543, 
44288, 44287]

Failed command: ./support/apply-review.sh -n -r 44288

Error:
2016-04-06 19:36:50 URL:https://reviews.apache.org/r/44288/diff/raw/ 
[50506/50506] -> "44288.patch" [1]
error: src/tests/monitor_tests.cpp: does not exist in index
error: patch failed: src/tests/oversubscription_tests.cpp:58
error: src/tests/oversubscription_tests.cpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/12362/console

- Mesos ReviewBot


On April 6, 2016, 4:03 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44670/
> ---
> 
> (Updated April 6, 2016, 4:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The master_detector and master_contender flags allow modules to be
> used for specifying the MasterContender and MasterDetector
> implementations to use.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp f8d2cc4c6c8dab00e34ca737dbcb5b9ca3870d6d 
>   src/master/flags.cpp 06852c9de68cce5d40f294f6402f7677ee6183d3 
>   src/master/main.cpp 181bbcb1758c0e9b83ef46496e990ce3d8c2195c 
>   src/slave/flags.hpp 69e1b01e09d2a15bee5e0745b751f47aaefe3fbe 
>   src/slave/flags.cpp 315cf47d268bce0a0255a061d64e414c736c8125 
>   src/slave/main.cpp dc3a2d2908ec08df60651dec6bf8eac651579cea 
> 
> Diff: https://reviews.apache.org/r/44670/diff/
> 
> 
> Testing
> ---
> 
> In addition to all unit tests passing, we are currently using this 
> functionality in our environment with a custom consensus stack. In our world, 
> we have a C++ plugin that calls out to an HTTP REST service (implemented in 
> Java/Scala, not that it matters).
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 45797: Cleaned up the virtual overloads in long lived executor.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 6, 2016, 3:43 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45797/
> ---
> 
> (Updated April 6, 2016, 3:43 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4982
> https://issues.apache.org/jira/browse/MESOS-4982
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change removes the unused methods that won't be needed
> after moving to the v1 API. It also renames `launchTask` to `launch`.
> This review mainly eases reviewing later in the chain and should not
> be committed on its own.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_executor.cpp 
> f0ba8966e198a496349e685b9e9ea296b8ed807e 
> 
> Diff: https://reviews.apache.org/r/45797/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45796: Deleted the `run` method in long lived executor.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 6, 2016, 3:43 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45796/
> ---
> 
> (Updated April 6, 2016, 3:43 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_executor.cpp 
> f0ba8966e198a496349e685b9e9ea296b8ed807e 
> 
> Diff: https://reviews.apache.org/r/45796/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45795: Fixed a memory leak in long lived executor.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 6, 2016, 3:43 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45795/
> ---
> 
> (Updated April 6, 2016, 3:43 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of allocating the thread object on the heap,
> modified code to make it a stack allocated object.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_executor.cpp 
> f0ba8966e198a496349e685b9e9ea296b8ed807e 
> 
> Diff: https://reviews.apache.org/r/45795/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45749: Adjusted lifetimes of member variables.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On April 5, 2016, 4:01 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45749/
> ---
> 
> (Updated April 5, 2016, 4:01 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Michael Park.
> 
> 
> Bugs: MESOS-5000
> https://issues.apache.org/jira/browse/MESOS-5000
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This correctly reflects the lifetime dependencies among
> Master::storage, Master::state and Master::registrar.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.hpp 91d3998176115002ea22a0825b1aaaee3fe0d768 
> 
> Diff: https://reviews.apache.org/r/45749/diff/
> 
> 
> Testing
> ---
> 
> Without this patch e.g., `MasterTest.MasterLost` fails under OS X with clang 
> (trunk, not optimized), and 
> `ResourceOffersTest.ResourceOfferWithMultipleSlaves` fails under Ubuntu14.04 
> with stock gcc (not optimized). With this patch they can be executed without 
> failure a couple thousand times.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 45440: Added some metrics to the long-lived-framework example.

[Vinod Kone]

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




src/examples/long_lived_framework.cpp (line 92)


do a `process::wait()` here as well?



src/examples/long_lived_framework.cpp (line 241)


Looking at the code below I think it makes more sense to make 
LongLivedScheduler an instance of process instead of making Metrics a process? 
That is consistent with how we did it elsewhere in the code base (e.g., Master 
and Slave).



src/examples/long_lived_framework.cpp (line 244)


why is the process based call "framework" here?



src/examples/long_lived_framework.cpp (line 272)


s/counter/metrics/

I'm assuming you might want to add gauge type metrics to this endpoint in 
the future?

More importantly, I didn't understand why you need to add a special 
endpoint for these 2 counters. Why can't they just be part of the 
"/metrics/snapshot" endpoint like the other metrics?



src/examples/long_lived_framework.cpp (line 274)


s/counter-type//



src/examples/long_lived_framework.cpp (line 275)


Why does this return 202 instead of 200? Looks like it is returning a 200 
below btw.



src/examples/long_lived_framework.cpp (lines 298 - 313)


seems a bit weird that these are part of the metrics class. In Master and 
Slave, we made these gauges part of the Master and Slave class instead.



src/examples/long_lived_framework.cpp (line 312)


shouldn't this be synchronized? ideally this would be just deferred to the 
scheduler process.


- Vinod Kone


On April 5, 2016, 10:33 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45440/
> ---
> 
> (Updated April 5, 2016, 10:33 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5062
> https://issues.apache.org/jira/browse/MESOS-5062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds metrics to gauge the health of the framework.  This includes:
> 
> * uptime_secs = How long the framework has been running.
> * registered  = If the framework is registered.
> * offers_received = A counter used to determine if the framework is starved 
> or not.
> * tasks_launched  = Number of tasks launched.
> * abnormal_terminations = Number of terminal status updates which were not 
> `TASK_FINISHED`.
> 
> Also adds an endpoint `/framework/counters` which returns the list of metrics 
> which are "counters".
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 
> 
> Diff: https://reviews.apache.org/r/45440/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Also deployed this version on a test cluster.  See the previous review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated April 6, 2016, 6:43 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44949/diff/


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

[James Peach]

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

(Updated April 6, 2016, 6:43 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


Bugs: MESOs-4828
https://issues.apache.org/jira/browse/MESOs-4828


Repository: mesos


Description
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am 55d3b341361bed25f3aa966d77060c88be29e5b0 
  src/slave/containerizer/mesos/containerizer.cpp 
a5dd22380066aa85de04d485052084e2629681c0 
  src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
  src/slave/flags.hpp 69e1b01e09d2a15bee5e0745b751f47aaefe3fbe 
  src/slave/flags.cpp 315cf47d268bce0a0255a061d64e414c736c8125 

Diff: https://reviews.apache.org/r/44948/diff/


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44950: Add XFS disk isolator documentation.

[James Peach]

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

(Updated April 6, 2016, 6:43 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


Bugs: MESOs-4828
https://issues.apache.org/jira/browse/MESOs-4828


Repository: mesos


Description
---

Add XFS disk isolator documentation.


Diffs (updated)
-

  docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c 
  docs/mesos-containerizer.md 2fde74362f35568bb944da80a1db17e6d3ba9d7b 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44950/diff/


Testing
---

Make check. Source inspection.


Thanks,

James Peach

Re: Review Request 44947: Add tests for XFS project quota utilities.

[James Peach]

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

(Updated April 6, 2016, 6:42 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Add tests for XFS project quota utilities.


Diffs (updated)
-

  src/Makefile.am 55d3b341361bed25f3aa966d77060c88be29e5b0 
  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 
  src/tests/environment.cpp acadb5bb0a1b1a9b0cee0345035b93147bf7164c 

Diff: https://reviews.apache.org/r/44947/diff/


Testing
---

Make check. Manual testing. These tests.


Thanks,

James Peach

Re: Review Request 44946: Add utility functions to manipulate XFS project quotas.

[James Peach]

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

(Updated April 6, 2016, 6:42 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Add utility functions to manipulate XFS project quotas.


Diffs (updated)
-

  src/Makefile.am 55d3b341361bed25f3aa966d77060c88be29e5b0 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44946/diff/


Testing
---

Make check. Manual verification. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44945: Add autoconf tests for XFS project quotas.

[James Peach]

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

(Updated April 6, 2016, 6:42 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Add autoconf tests for XFS project quotas.


Diffs (updated)
-

  3rdparty/libprocess/include/process/limiter.hpp 
b4b6a7bb48aa19fa40c0ea9f8759b8f553f43ca6 
  configure.ac c693b825294f82ace8a14563cf2229820e159e3c 
  docs/docker-containerizer.md 6ec1cecd2e39bc4737688d8c4f6fc05e82c92023 
  src/CMakeLists.txt 9a4cffa952a452d5df70ea42949142e534067cd6 
  src/Makefile.am ba9cc8b683bba2ae8fe9d9c58642690f5b80afaf 
  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/launcher/executor.cpp bec9cba091df2d1b340e68e67966ec322558c315 
  src/master/allocator/mesos/hierarchical.hpp 
be4ccffa6042261f03690eb38b071e5f3b6c1105 
  src/master/allocator/mesos/hierarchical.cpp 
a8d80b4412b324717f5b5d3ff2711eba0f7a2ed5 
  src/slave/http.cpp a684ff504535e2b9b2064f8048f5e03a0efcb059 
  src/slave/monitor.hpp PRE-CREATION 
  src/slave/monitor.cpp PRE-CREATION 
  src/slave/slave.hpp 5b6076afc62cc3978276162aa4ea6e8c0670f1ab 
  src/slave/slave.cpp cd4264e7376edaba079115632a94e710375f2a55 
  src/tests/limiter.hpp 16d5682b0d76d2348a78abd568deb9fd41900ee4 
  src/tests/mesos.hpp 8c8cd1a55bb37bd9bda53b7dd7e9caa178f2a923 
  src/tests/mesos.cpp 803b925d6741e43342419b600756a7e1e906783c 
  src/tests/monitor_tests.cpp PRE-CREATION 
  src/tests/oversubscription_tests.cpp 84e1104ea14c976b4184c8570ee996891fe260a5 
  src/tests/slave_tests.cpp c4d80aa21a8640ef36226458d684bb3d31366c81 

Diff: https://reviews.apache.org/r/44945/diff/


Testing
---

Make check. Manual verification.


Thanks,

James Peach

Re: Review Request 45801: Reset the `LIBPROCESS_IP` environment variable in `network/cni`.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 6, 2016, 4:14 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45801/
> ---
> 
> (Updated April 6, 2016, 4:14 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-5127
> https://issues.apache.org/jira/browse/MESOS-5127
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently the `LIBPROCESS_IP` environment variable was being set to
> the Agent IP if the environment variable has not be defined by the
> `Framework`. For containers having their own IP address (as with
> containers on CNI networks) this becomes a problem since the command
> executor tries to bind to the `LIBPROCESS_IP` that does not exist in
> its network namespace, and fails. Thus, for containers launched on CNI
> networks the `LIBPROCESS_IP` should not be set, or rather is set to
> "0.0.0.0", allowing the container to bind to the IP address provided
> by the CNI network.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f43ab071de42de16d330cfd2a3b76563bcb173c9 
> 
> Diff: https://reviews.apache.org/r/45801/diff/
> 
> 
> Testing
> ---
> 
> Started a slave with the following flags:
> Flags at startup: --appc_simple_discovery_uri_prefix="http://"; 
> --appc_store_dir="/tmp/mesos/store/appc" --authenticate_http="false" 
> --authenticatee="crammd5" --cgroups_cpu_enable_pids_and_tids_count="false" 
> --cgroups_enable_cfs="false" --cgroups_hierarchy="/sys/fs/cgroup" 
> --cgroups_limit_swap="false" --cgroups_root="mesos" 
> --container_disk_watch_interval="15secs" --containerizers="mesos" 
> --default_role="*" --disk_watch_interval="1mins" --docker="docker" 
> --docker_kill_orphans="true" --docker_registry="/home/vagrant/docker/builds/" 
> --docker_remove_delay="6hrs" --docker_socket="/var/run/docker.sock" 
> --docker_stop_timeout="0ns" --docker_store_dir="/tmp/mesos/store/docker" 
> --enforce_container_disk_quota="false" 
> --executor_environment_variables="{"LIBPROCESS_IP":"0.0.0.0"}" 
> --executor_registration_timeout="1mins" 
> --executor_shutdown_grace_period="5secs" 
> --fetcher_cache_dir="/tmp/mesos/fetch" --fetcher_cache_size="2GB" 
> --frameworks_home="" --gc_delay="1weeks" --gc_disk_headroom="0.1" -he 
 -hadoop_home="" --help="false" --hostname_lookup="true" 
--http_authenticators="basic" --image_providers="docker" 
--image_provisioner_backend="copy" --initialize_driver_logging="true" 
--ip="172.28.128.7" --isolation="filesystem/linux,docker/runtime,network/cni" 
--launcher_dir="/home/vagrant/mesosphere/mesos/build/src" --logbufsecs="0" 
--logging_level="INFO" --master="172.28.128.7:5050" 
--network_cni_config_dir="/home/vagrant/cni/config" 
--network_cni_plugins_dir="/home/vagrant/dev/go/cni/bin" 
--oversubscribed_resources_interval="15secs" --perf_duration="10secs" 
--perf_interval="1mins" --port="5051" --qos_correction_interval_min="0ns" 
--quiet="false" --recover="reconnect" --recovery_timeout="15mins" 
--registration_backoff_factor="1secs" --revocable_cpu_low_priority="true" 
--sandbox_directory="/mnt/mesos/sandbox" --strict="true" --switch_user="true" 
--systemd_enable_support="true" 
--systemd_runtime_directory="/run/systemd/system" --version="false" 
--work_dir="/home/vagrant/mesos_data_s
 lave/"
> 
> Ran `mesos-execute` with a docker-image and with a CNI network specified. The 
> docker image was a Ubuntu 14.04 image, and the command specified was 
> "ifconfig". Ensured that the container was launched succesfully and the task 
> FINISHED succesfully, with the correct IP address.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 45067: Updated the long-lived-framework example.

[Vinod Kone]

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


Fix it, then Ship it!





src/examples/long_lived_framework.cpp (line 155)


no need for quotes around slaveId since it is generated by mesos and we 
know it doesn't have spaces.



src/examples/long_lived_framework.cpp (line 163)


// Helper to decline an offer.



src/examples/long_lived_framework.cpp (line 172)


// Helper to launch a task using the offer.


- Vinod Kone


On April 5, 2016, 10:40 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45067/
> ---
> 
> (Updated April 5, 2016, 10:40 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5062
> https://issues.apache.org/jira/browse/MESOS-5062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives the example `long-lived-framework` enough options to run outside 
> of the build environment.
> 
> This also updates:
> 
> * The style of the framework code.
> * Gives the `ExecutorInfo` some resources (needed for some cgroups isolators).
> * Restricts the framework to one agent.  Otherwise, it would grab a small 
> chunk of every machine in the cluster.
> * Adds filters for declined offers.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 
> 
> Diff: https://reviews.apache.org/r/45067/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Ran this on the master node on a Mesos cluster:
> ```
> ./long-lived-framework --master=zk://localhost:2181/mesos 
> --executor_uri="https://s3.amazonaws.com/url/to/long-lived-executor"; 
> --executor_command="LD_LIBRARY_PATH=/path/to/libmesos && 
> ./long-lived-executor"
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 45668: Enable CMake build for Linux as a CONFIGURATOR option.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [45668]

Failed command: ['bash', '-c', "set -o pipefail; export OS='ubuntu:14.04' 
CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker_build.sh 2>&1 | tee build_45668"]

Error:
+ : ubuntu:14.04
./support/docker_build.sh: line 10: CONFIGURATOR: Environment variable 
'CONFIGURATOR' must be set (e.g., CONFIGURATOR=autotools)

Full log: https://builds.apache.org/job/mesos-reviewbot/12361/console

- Mesos ReviewBot


On April 6, 2016, 3:49 p.m., Juan Larriba wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45668/
> ---
> 
> (Updated April 6, 2016, 3:49 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer, Joerg Schad, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable CMake build for Linux as new CONFIGURATOR option.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh e9b1d7219b261475fb29118ee27d11743c2c5e0d 
> 
> Diff: https://reviews.apache.org/r/45668/diff/
> 
> 
> Testing
> ---
> 
> Built using docker_build.sh on both centos:7 and ubuntu:14.04 using both 
> cmake and autotools. In ubuntu:14.04 was built using gcc and clang, in 
> centos:7 only gcc.
> 
> 
> Thanks,
> 
> Juan Larriba
> 
>

Re: Review Request 45596: Updated docker containerizer private registry doc.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 6, 2016, 5:33 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45596/
> ---
> 
> (Updated April 6, 2016, 5:33 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docker containerizer private registry doc.
> 
> 
> Diffs
> -
> 
>   docs/docker-containerizer.md 865b2cf0e158b1aab6c384d26ab7a3d9800610ac 
> 
> Diff: https://reviews.apache.org/r/45596/diff/
> 
> 
> Testing
> ---
> 
> Tested by buidling with support/site-docker, and verified in web browser.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 45381: Migrate /monitor/statistics and /monitor/statistics.json to slave.

[Jay Guo]

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

(Updated April 6, 2016, 6:16 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

These two endpoints and their underlying logics are moved from
ResourceMonitorProcess to slave process. ResourceMonitor is removed.


Diffs
-

  src/CMakeLists.txt ff225c0d51a32b03a1b5f2ba31718ec2305c7ced 
  src/Makefile.am 55d3b341361bed25f3aa966d77060c88be29e5b0 
  src/slave/http.cpp 7a872c6e21b115500c5291b7cb9fb240ec9dc8ed 
  src/slave/monitor.hpp 70a7c88fefd4577f53f85e3eccdd6b69ab6f3acd 
  src/slave/monitor.cpp 5c1dd354595e67e5eb603c5d49850f84b84b 
  src/slave/slave.hpp 3ba335fcd31a92af9609023daae328b7f4bf5e59 
  src/slave/slave.cpp 60f93ca2c6251190d5dadfd543f0e5c5fbee1214 
  src/tests/limiter.hpp baf396c138113abbf984879fbc2e8fade5e9feac 
  src/tests/mesos.hpp 3b565b45f45c84aba42aa6fb29b21f8306c49861 
  src/tests/mesos.cpp cf38dbb05908800b3a771318fa6922548c86c1f2 
  src/tests/monitor_tests.cpp 5dcb2481ff2f1a7caf54036bc3e60c78feb982b1 
  src/tests/oversubscription_tests.cpp ba036810758d99a6fb0034c5e2bc7829e2343a44 
  src/tests/slave_tests.cpp 57fc50360eae85819ae6ce714b0c3c4c1867b2b8 

Diff: https://reviews.apache.org/r/45381/diff/


Testing
---

make check

https://issues.apache.org/jira/browse/MESOS-4891


Thanks,

Jay Guo

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

[Anand Mazumdar]

> On April 6, 2016, 5 p.m., haosdent huang wrote:
> > src/launcher/http_command_executor.cpp, line 264
> > 
> >
> > May you add a comment why need add `Second(1)` here?

It's pretty self-explanatory that it's the backoff retry timeout. No need for 
an explicit comment.


- Anand


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


On April 5, 2016, 12:36 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44424/
> ---
> 
> (Updated April 5, 2016, 12:36 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3558
> https://issues.apache.org/jira/browse/MESOS-3558
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated http_command_executor.cpp to use v1 API.
> 
> 
> Diffs
> -
> 
>   src/launcher/http_command_executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44424/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 45808: Update metrics for SUBSCRIBE call for both PID/HTTP frameworks.

[Vinod Kone]

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




src/master/master.cpp (lines 2085 - 2088)


we always use braces around our if and else blocks.

if () {

} else {

}



src/master/master.cpp (lines 2285 - 2288)


ditto.


- Vinod Kone


On April 6, 2016, 8:59 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45808/
> ---
> 
> (Updated April 6, 2016, 8:59 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-4981
> https://issues.apache.org/jira/browse/MESOS-4981
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change will cover three cases:
> 1. Directly call {re}registerFramework
> 2. Through HTTP call
> 3. Through MesosSchedulerDriver
> 
> Suggested by Benjamin Mahler to introspect framework_info.id.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 2cfa5075991bbb5df8a7e33b14767de558bda4ac 
> 
> Diff: https://reviews.apache.org/r/45808/diff/
> 
> 
> Testing
> ---
> 
> 1. I can use 'docker-no-executor-framework' to reproduce/verify the issue.
> 
> "master/messages_register_framework": 1.0,
> "master/messages_reregister_framework": 0.0,
> 
> 
> Thanks,
> 
> fan du
> 
>

Re: Review Request 45623: Renamed the parameter 'override' to '_override'.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 3, 2016, 8:30 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45623/
> ---
> 
> (Updated April 3, 2016, 8:30 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed the parameter 'override' to '_override'.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 76773914adb021dda2324eb566db0c697a353526 
> 
> Diff: https://reviews.apache.org/r/45623/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 45800: Moved long running framework to use the v1 API.

[Joseph Wu]

> On April 6, 2016, 10:10 a.m., Joseph Wu wrote:
> > src/examples/long_lived_framework.cpp, lines 217-221
> > 
> >
> > Would be nice to log something here.
> 
> Anand Mazumdar wrote:
> Logging an `ERROR` would be useful. I had copied over from the old 
> interface that did not use to log anything. Does this sound reasonable to you?

Sounds good.  (Logging the `RESCIND` event might also be useful.)


> On April 6, 2016, 10:10 a.m., Joseph Wu wrote:
> > src/examples/long_lived_framework.cpp, line 593
> > 
> >
> > Does the HTTP scheduler driver initialize glog (like the old scheduler 
> > driver does)?
> 
> Anand Mazumdar wrote:
> It doesn't. We should ideally initialize logging inside the framework 
> itself.

The only consequence of not initializing glog is that glog will print a warning 
the first time we hit a `LOG(...) << ...` line.


- Joseph


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


On April 6, 2016, 10:45 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45800/
> ---
> 
> (Updated April 6, 2016, 10:45 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4982
> https://issues.apache.org/jira/browse/MESOS-4982
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary. Also removed the AuthN code. Once, the library
> supports AuthN we can bring it back.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 
> 
> Diff: https://reviews.apache.org/r/45800/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45800: Moved long running framework to use the v1 API.

[Anand Mazumdar]

> On April 6, 2016, 5:10 p.m., Joseph Wu wrote:
> > src/examples/long_lived_framework.cpp, line 593
> > 
> >
> > Does the HTTP scheduler driver initialize glog (like the old scheduler 
> > driver does)?

It doesn't. We should ideally initialize logging inside the framework itself.


> On April 6, 2016, 5:10 p.m., Joseph Wu wrote:
> > src/examples/long_lived_framework.cpp, lines 217-221
> > 
> >
> > Would be nice to log something here.

Logging an `ERROR` would be useful. I had copied over from the old interface 
that did not use to log anything. Does this sound reasonable to you?


- Anand


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


On April 6, 2016, 5:45 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45800/
> ---
> 
> (Updated April 6, 2016, 5:45 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4982
> https://issues.apache.org/jira/browse/MESOS-4982
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary. Also removed the AuthN code. Once, the library
> supports AuthN we can bring it back.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 
> 
> Diff: https://reviews.apache.org/r/45800/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45800: Moved long running framework to use the v1 API.

[Anand Mazumdar]

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

(Updated April 6, 2016, 5:45 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments from Joseph


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


Repository: mesos


Description
---

See summary. Also removed the AuthN code. Once, the library
supports AuthN we can bring it back.


Diffs (updated)
-

  src/examples/long_lived_framework.cpp 
ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 

Diff: https://reviews.apache.org/r/45800/diff/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 45596: Updated docker containerizer private registry doc.

[Gilbert Song]

> On April 2, 2016, 8:20 p.m., Guangya Liu wrote:
> > docs/docker-containerizer.md, line 68
> > 
> >
> > What about 
> > 
> > Rename `.docker/config.json` to `~/.docker/config.json`
> > Rename `.dockercfg` to `~/.dockercfg`
> > 
> > Or clarify that it should be in `HOME` dir, such as s/on each slave/on 
> > each slave `HOME` directory

If we are using the flag pointing to a docker config file, the file is not 
necessary to locate at HOME direcoty.


- Gilbert


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


On April 6, 2016, 10:33 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45596/
> ---
> 
> (Updated April 6, 2016, 10:33 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docker containerizer private registry doc.
> 
> 
> Diffs
> -
> 
>   docs/docker-containerizer.md 865b2cf0e158b1aab6c384d26ab7a3d9800610ac 
> 
> Diff: https://reviews.apache.org/r/45596/diff/
> 
> 
> Testing
> ---
> 
> Tested by buidling with support/site-docker, and verified in web browser.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 45596: Updated docker containerizer private registry doc.

[Gilbert Song]

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

(Updated April 6, 2016, 10:33 a.m.)


Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
Chen.


Repository: mesos


Description
---

Updated docker containerizer private registry doc.


Diffs (updated)
-

  docs/docker-containerizer.md 865b2cf0e158b1aab6c384d26ab7a3d9800610ac 

Diff: https://reviews.apache.org/r/45596/diff/


Testing (updated)
---

Tested by buidling with support/site-docker, and verified in web browser.


Thanks,

Gilbert Song

Re: Review Request 45803: Add PowerPC LE CI for support/docker_build.sh.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45803]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On April 6, 2016, 9:25 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45803/
> ---
> 
> (Updated April 6, 2016, 9:25 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4897
> https://issues.apache.org/jira/browse/MESOS-4897
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add PowerPC LE CI for support/docker_build.sh.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh e9b1d7219b261475fb29118ee27d11743c2c5e0d 
> 
> Diff: https://reviews.apache.org/r/45803/diff/
> 
> 
> Testing
> ---
> 
> On a PowerPC machine, run following command:
> 
> ENVIRONMENT='GLOG_v=0' CONFIGURATION='--enable-libevent --enable-ssl' 
> OS='ppc64le/ubuntu:14.04' COMPILER=gcc ./support/docker_build.sh
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 45768: Fixed commit message hook to skip over the commented lines.

[haosdent huang]

> On April 5, 2016, 8:01 p.m., Kevin Klues wrote:
> > support/hooks/commit-msg, line 20
> > 
> >
> > You should probably quote the $LINE variable here. You should also use 
> > a single "=", not "==". The double equals will not work on all versions of 
> > sh (most notably on mac os).
> > I would also just check the first character to equal "#" instead of 
> > comaping the whole line.
> > 
> > Also, when you hit a line with a comment, you probably shouldn't break. 
> > Instead you should continue (in case there is more text to include in the 
> > commit message later on).
> > 
> > The new loop would look like:
> > 
> > ```
> > while read LINE
> > do
> > if [[ "${LINE:0:1}" == "#" ]]; then continue; fi
> > LENGTH=$(echo $LINE | wc -c)
> > if [ "$LENGTH" -gt "73" ]; then
> > echo >&2 "Error: No line in the commit message summary may 
> > exceed 72 characters."
> > exit 1
> > fi
> > ```
> 
> Kevin Klues wrote:
> Whoops, I meant the following (this actually has the single "=" and uses 
> single "[" "]"'s around the if instead of "[[", which is also non-portable.
> 
> ```
> while read LINE
> do
> if [ "${LINE:0:1}" = "#" ]; then continue; fi
> LENGTH=$(echo $LINE | wc -c)
> if [ "$LENGTH" -gt "73" ]; then
> echo >&2 "Error: No line in the commit message summary may exceed 
> 72 characters."
> exit 1
> fi
> done
> ```

I suggset use `test "x$LINE" != "x#"`. Refer to 
http://stackoverflow.com/questions/174119/why-do-shell-script-comparisons-often-use-xvar-xyes
And I think `LENGTH=$(echo $LINE | wc -c)` also could replace by 
`LENGTH=${#LINE}`.


- haosdent


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


On April 5, 2016, 7:50 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45768/
> ---
> 
> (Updated April 5, 2016, 7:50 p.m.)
> 
> 
> Review request for mesos, Joerg Schad and Vinod Kone.
> 
> 
> Bugs: MESOS-5126
> https://issues.apache.org/jira/browse/MESOS-5126
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/hooks/commit-msg d3d5415bf6a243def05cf79f4af2b136b9866d68 
> 
> Diff: https://reviews.apache.org/r/45768/diff/
> 
> 
> Testing
> ---
> 
> Was able to commit a patch where there were lines longer than 72 characters 
> in the commented section.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 45800: Moved long running framework to use the v1 API.

[Joseph Wu]

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



LGTM.


src/examples/long_lived_framework.cpp (lines 216 - 220)


Would be nice to log something here.



src/examples/long_lived_framework.cpp (line 590)


Does the HTTP scheduler driver initialize glog (like the old scheduler 
driver does)?


- Joseph Wu


On April 5, 2016, 8:43 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45800/
> ---
> 
> (Updated April 5, 2016, 8:43 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4982
> https://issues.apache.org/jira/browse/MESOS-4982
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary. Also removed the AuthN code. Once, the library
> supports AuthN we can bring it back.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 
> 
> Diff: https://reviews.apache.org/r/45800/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

[haosdent huang]

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




src/launcher/http_command_executor.cpp (line 256)


May you add a comment why need add `Second(1)` here?


- haosdent huang


On April 5, 2016, 12:36 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44424/
> ---
> 
> (Updated April 5, 2016, 12:36 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3558
> https://issues.apache.org/jira/browse/MESOS-3558
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated http_command_executor.cpp to use v1 API.
> 
> 
> Diffs
> -
> 
>   src/launcher/http_command_executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44424/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 45818: Fixed flakiness in PersistentVolumeTest.AccessPersistentVolume.

[Joris Van Remoortere]

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



lgtm

- Joris Van Remoortere


On April 6, 2016, 4:35 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45818/
> ---
> 
> (Updated April 6, 2016, 4:35 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5128
> https://issues.apache.org/jira/browse/MESOS-5128
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed flakiness in PersistentVolumeTest.AccessPersistentVolume.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 47867505f2f1d7a722b5a0d4ff3a0fca3188a193 
> 
> Diff: https://reviews.apache.org/r/45818/diff/
> 
> 
> Testing
> ---
> 
> `./src/mesos-tests 
> --gtest_filter="DiskResource/PersistentVolumeTest.AccessPersistentVolume/0" 
> --gtest_repeat=1000 --gtest_break_on_failure` on both OSX and Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>