Re: Review Request 37382: Introduced provisioner Backend interface.

2015-08-18 Thread Jiang Yan Xu


> On Aug. 14, 2015, 11:15 a.m., Jie Yu wrote:
> > src/slave/containerizer/provisioners/backend.hpp, lines 55-57
> > 
> >
> > Hum, what do yo mean here?
> > 
> > I think the backend should be container aware. I.e., we should pass in 
> > containerid in 'provision'  and destroy will take a containerid as well.
> > 
> > There could be multiple rootfs for a container (because image in 
> > volumes). When the container finishes, the provisioner will try to destroy 
> > all rootfs provisioned for that container.
> > 
> > Thoughts?
> 
> Jiang Yan Xu wrote:
> This is intended for the image in volumes case but I was trying to not be 
> too explicit because it's not implemented yet (we should track it with a 
> separate ticket).
> 
> My comment about 'nested' was incorrect, let me clarify here.
> 
> The 'caller' is the provisioner. What I had in mind was: each Backend 
> call handles one image, if `Backend::provision()`s one image and then it 
> should `destroy()` the rootfs provisioned by one image.
> 
> Suppose we have such a rootfs dir and each container has multiple 
> rootfses, some from ContainerInfo::volumnes and one from 
> ContainerInfo::mesos::image but they are stored side-by-side under 
> /.
> 
> ```
> <--rootfs_dir>
> |--
>|--
>   |--
>   |--
>|--
> ```
> 
> IMO since LinuxFilesystemIsolatorProcess::cleanup() calls each 
> `Provisioner::destory(containerId)` (appc|docker), the provisioner can look 
> at the subdir it manages and scan it and call `Backend::destroy(rootfsPath)` 
> for each image under it.
> 
> So each `Backend::destroy(rootfsPath)` call destroys one rootfs and there 
> is no `nesting` under it.
> Passing `containerId` into Backend isn't as clean as it should be the 
> provisioner who converts the id into a path and Backends are this dumb thing 
> which only knows paths.
> 
> More clear?
> 
> Jie Yu wrote:
> So the directory layout listed above is managed by the provisioner or 
> backend? It cannot be backend because otherwise that means provisioner needs 
> to know about the layout used by the backend.
> 
> So if it's managed by provisioner, the layout should probably be:
> ```
> /var/lib/mesos/provisioners/
> |-- appc/
>   |-- 
>  |-- 
>  |-- 
> |-- docker/
>   |-- 
>  |-- 
>  |-- 
> ```
> 
> So we only allow one backend, or we allow multiple backends (e.g., what 
> if we want to use bind mount backend for appc but overlayfs backend for 
> docker, or we want to gradually roll out overlayfs backend so that two 
> backends co-exists for a while)? If the later, how can we figure out which 
> backend to use when destroying root filesystems?
> 
> I just want to get the full picture sorted out. We can chat next week.
> 
> Jiang Yan Xu wrote:
> Note that the above rootfs I mentioned is the result of the Backend 
> either collapsing its layers or overlay them and in the provisioner's view 
> it's just one entity.
> 
> So the backend know about layout within a rootfs (e.g., its knows that a 
> rootfs is "ImageA(a script) -> ImageB(essential software) ->ImageC(base 
> OS)"); the provisioner knows about / manages the direcotry layout of 
> 'rootfs'es but not inside 'rootfs'es (e.g. This container wants three 
> rootfses, two of which from volumes).
> 
> Yeah I agree the interface design requires a full picture of 
> multi-image-per-contain design. I will jot down my thoughts in a doc and 
> let's chat next week.
> 
> Timothy Chen wrote:
> Hey we should talk about this as it affects all of our work, I think I 
> didn't realize we're ending up doing multiple provisioner images for a single 
> executor.
> I think my assumption was that the backend when given a provision call is 
> for a image as Yan mentioned, and returning the rootfs directory path which 
> the provisioner can hold on to know where the backend has provisioned it to.
> 
> We don't really allow multiple backends with our current design, I don't 
> think there is anything stopping us but it then become a very interesting API 
> choice to see where we allow the user to specify this. My intuition is to 
> just use one single backend and we record the backend that is used to 
> provisioner a rootfs, so at least on recovery we can say we can't recover due 
> to a backend change.

Jie and I chatted offline and I am going to submit another review regarding the 
file system layout that addresses multiple provisioners / mutliple backends. 
The current Backend interface design is compatible with it and not itself 
controversial so I am just going to make some documentation adjustments here.

Regardign a few issues which I'll describe in details

Re: Review Request 37198: Add Docker image provisioner and copy backend.

2015-08-18 Thread Timothy Chen

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



src/slave/containerizer/provisioners/docker.hpp (line 81)


Did we introduce DockerImageName later?
A pair of strings is pretty confusing, how about pulling it earlier to here?



src/slave/containerizer/provisioners/docker.hpp (line 84)


Actually it's valid to have a registry name  with a custom port in the 
image name:

localhost:5050/ubuntu:14.04

You need to first split on "/", then do this check.

And we should actually include a optional registry name in the 
DockerImageName struct too.



src/slave/containerizer/provisioners/docker.cpp (line 96)


I think we usually put all the validation logic inside of the Process so 
it's all one place, rather than checking here, which makes this object very 
simple.



src/slave/containerizer/provisioners/docker.cpp (line 163)


Seems like a useless comment :)



src/slave/containerizer/provisioners/docker.cpp (line 234)


"Unable to join discovery local path: " + error()


- Timothy Chen


On Aug. 17, 2015, 9:11 p.m., Lily Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37198/
> ---
> 
> (Updated Aug. 17, 2015, 9:11 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-2850
> https://issues.apache.org/jira/browse/MESOS-2850
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add Docker image provisioner and copy backend.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 457ad26ee55bd7a2aedf27f45db58a9a4a6a5dc5 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> f36424e94c380870cfde49d55af397fa3dc4a612 
>   src/slave/containerizer/provisioner.hpp 
> 541dd4e0b2f0c92a45c00cab6132a2be69654838 
>   src/slave/containerizer/provisioner.cpp 
> efc7e6996ff6663bebaf61989a7e040bd2ad7a5e 
>   src/slave/containerizer/provisioners/docker.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker.cpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/backend.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/backend.cpp PRE-CREATION 
>   src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 
>   src/slave/flags.cpp e43dd1c13dd4263dc326842233808ddb7a9bb74c 
>   src/tests/containerizer/provisioner.hpp 
> c4ba46794fe5d7875fda11155367f521c34ea339 
> 
> Diff: https://reviews.apache.org/r/37198/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Lily Chen
> 
>



Re: Review Request 37197: Docker image store.

2015-08-18 Thread Timothy Chen

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



src/slave/containerizer/provisioners/docker/store.cpp (line 1)


Btw this is actually renamed to be local store later right?

How about just remove the impl completely in this review so later review we 
introduce the local store?


- Timothy Chen


On Aug. 16, 2015, 8:31 a.m., Lily Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37197/
> ---
> 
> (Updated Aug. 16, 2015, 8:31 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-2849
> https://issues.apache.org/jira/browse/MESOS-2849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stored images currently kept indefinitely.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 457ad26ee55bd7a2aedf27f45db58a9a4a6a5dc5 
>   src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION 
>   src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 
>   src/slave/flags.cpp e43dd1c13dd4263dc326842233808ddb7a9bb74c 
> 
> Diff: https://reviews.apache.org/r/37197/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Lily Chen
> 
>



Re: Review Request 37382: Introduced provisioner Backend interface.

2015-08-18 Thread Timothy Chen


> On Aug. 14, 2015, 6:15 p.m., Jie Yu wrote:
> > src/slave/containerizer/provisioners/backend.hpp, lines 55-57
> > 
> >
> > Hum, what do yo mean here?
> > 
> > I think the backend should be container aware. I.e., we should pass in 
> > containerid in 'provision'  and destroy will take a containerid as well.
> > 
> > There could be multiple rootfs for a container (because image in 
> > volumes). When the container finishes, the provisioner will try to destroy 
> > all rootfs provisioned for that container.
> > 
> > Thoughts?
> 
> Jiang Yan Xu wrote:
> This is intended for the image in volumes case but I was trying to not be 
> too explicit because it's not implemented yet (we should track it with a 
> separate ticket).
> 
> My comment about 'nested' was incorrect, let me clarify here.
> 
> The 'caller' is the provisioner. What I had in mind was: each Backend 
> call handles one image, if `Backend::provision()`s one image and then it 
> should `destroy()` the rootfs provisioned by one image.
> 
> Suppose we have such a rootfs dir and each container has multiple 
> rootfses, some from ContainerInfo::volumnes and one from 
> ContainerInfo::mesos::image but they are stored side-by-side under 
> /.
> 
> ```
> <--rootfs_dir>
> |--
>|--
>   |--
>   |--
>|--
> ```
> 
> IMO since LinuxFilesystemIsolatorProcess::cleanup() calls each 
> `Provisioner::destory(containerId)` (appc|docker), the provisioner can look 
> at the subdir it manages and scan it and call `Backend::destroy(rootfsPath)` 
> for each image under it.
> 
> So each `Backend::destroy(rootfsPath)` call destroys one rootfs and there 
> is no `nesting` under it.
> Passing `containerId` into Backend isn't as clean as it should be the 
> provisioner who converts the id into a path and Backends are this dumb thing 
> which only knows paths.
> 
> More clear?
> 
> Jie Yu wrote:
> So the directory layout listed above is managed by the provisioner or 
> backend? It cannot be backend because otherwise that means provisioner needs 
> to know about the layout used by the backend.
> 
> So if it's managed by provisioner, the layout should probably be:
> ```
> /var/lib/mesos/provisioners/
> |-- appc/
>   |-- 
>  |-- 
>  |-- 
> |-- docker/
>   |-- 
>  |-- 
>  |-- 
> ```
> 
> So we only allow one backend, or we allow multiple backends (e.g., what 
> if we want to use bind mount backend for appc but overlayfs backend for 
> docker, or we want to gradually roll out overlayfs backend so that two 
> backends co-exists for a while)? If the later, how can we figure out which 
> backend to use when destroying root filesystems?
> 
> I just want to get the full picture sorted out. We can chat next week.
> 
> Jiang Yan Xu wrote:
> Note that the above rootfs I mentioned is the result of the Backend 
> either collapsing its layers or overlay them and in the provisioner's view 
> it's just one entity.
> 
> So the backend know about layout within a rootfs (e.g., its knows that a 
> rootfs is "ImageA(a script) -> ImageB(essential software) ->ImageC(base 
> OS)"); the provisioner knows about / manages the direcotry layout of 
> 'rootfs'es but not inside 'rootfs'es (e.g. This container wants three 
> rootfses, two of which from volumes).
> 
> Yeah I agree the interface design requires a full picture of 
> multi-image-per-contain design. I will jot down my thoughts in a doc and 
> let's chat next week.

Hey we should talk about this as it affects all of our work, I think I didn't 
realize we're ending up doing multiple provisioner images for a single executor.
I think my assumption was that the backend when given a provision call is for a 
image as Yan mentioned, and returning the rootfs directory path which the 
provisioner can hold on to know where the backend has provisioned it to.

We don't really allow multiple backends with our current design, I don't think 
there is anything stopping us but it then become a very interesting API choice 
to see where we allow the user to specify this. My intuition is to just use one 
single backend and we record the backend that is used to provisioner a rootfs, 
so at least on recovery we can say we can't recover due to a backend change.


- Timothy


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


On Aug. 14, 2015, 5:51 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://review

Re: Review Request 37284: Maintenance Primitives: Added support for Accept / Decline of InverseOffers in master.

2015-08-18 Thread Mesos ReviewBot

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


Bad patch!

Reviews applied: [36321, 36571, 37314, 37325, 37358, 37362, 37364, 37170]

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

Error:
 2015-08-19 03:19:49 URL:https://reviews.apache.org/r/37170/diff/raw/ 
[3962/3962] -> "37170.patch" [1]
error: patch failed: src/master/master.cpp:1361
error: src/master/master.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Aug. 18, 2015, 6:58 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37284/
> ---
> 
> (Updated Aug. 18, 2015, 6:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
> 
> Diff: https://reviews.apache.org/r/37284/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 37586: Generate usage help information in libprocess.

2015-08-18 Thread haosdent huang


> On Aug. 19, 2015, 1:24 a.m., Guangya Liu wrote:
> > Just curious: If change the 3rd party code directly in mesos source code, 
> > then how to handle the case when mesos want to upgrade the 3rd party 
> > libraries?

because libprocess is maintained in mesos, so we could do it like this here. 
For other libraries, we use patch way.


- haosdent


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


On Aug. 18, 2015, 6:32 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37586/
> ---
> 
> (Updated Aug. 18, 2015, 6:32 p.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-3239
> https://issues.apache.org/jira/browse/MESOS-3239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Generate usage help information in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> 441f6d1c15afebdd98b73bb7430fcd8c8cf1e333 
>   3rdparty/libprocess/include/process/system.hpp 
> 7c8b49e78f76f9e131a4367f411c6dba447ccd90 
>   3rdparty/libprocess/src/help.cpp e4e0bb617956c7f79380069015b7170aa7716231 
>   3rdparty/libprocess/src/logging.cpp 
> 3d855e90e83a54cd344e49f075af0eadef1a1358 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> b9617507a16318b7de25d4875d6bc0b4409fcd29 
>   3rdparty/libprocess/src/profiler.cpp 
> 65a2e05e8f6005a378ae3647698dcba60fb95e9f 
> 
> Diff: https://reviews.apache.org/r/37586/diff/
> 
> 
> Testing
> ---
> 
> manual test
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 37176: Maintenance Primitives: Added a new allocation overload to sorter.

2015-08-18 Thread Guangya Liu

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

Ship it!


Ship It!

- Guangya Liu


On 八月 18, 2015, 6:57 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37176/
> ---
> 
> (Updated 八月 18, 2015, 6:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.hpp 
> f66ade06c6a5b4bf816839477cec2d18036c7b1a 
>   src/master/allocator/sorter/drf/sorter.cpp 
> bfc273493419fe46a4d907f4f7fa282cff71b800 
>   src/master/allocator/sorter/sorter.hpp 
> 536a7ad9a2d661bc8aa352d2e0ae41115b1e8a04 
> 
> Diff: https://reviews.apache.org/r/37176/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 37416: Perf supported() should be based on the version of perf, not the version of the kernel.

2015-08-18 Thread Ben Mahler

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



src/linux/perf.cpp (lines 384 - 385)


Any reason to put this in internal::? It seems nice to have this as 
perf::version, even though it's only in the cpp file.



src/linux/perf.cpp (line 390)


const string&

how about a newline for readability?



src/linux/perf.cpp (line 391)


End comments with a period please :)



src/linux/perf.cpp (lines 474 - 478)


Why `_supported` here that takes a version? Why not just have supported 
compute the version and then perform the necessary check?



src/linux/perf.cpp (lines 480 - 481)


Perf versions are numbered similarly to linux versions..?

Not yours but mind ending it with a period?



src/linux/perf.cpp (lines 483 - 489)


Couple of things:

(1) Let's add a comment as to why we're using await here, since it is an 
anti-pattern. Namely, is it because making supported() asynchronous is a 
difficult change?

(2) Let's discard the future when it doesn't transition in the timeout.

(3) We're going to silently say false if the future fails, can we log the 
failure?

(4) 'if (' :)

(5) Can you add some newlines here to make it less dense?


- Ben Mahler


On Aug. 19, 2015, 12:57 a.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37416/
> ---
> 
> (Updated Aug. 19, 2015, 12:57 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Perf supported() should be based on the version of perf, not the version of 
> the kernel.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 
> 
> Diff: https://reviews.apache.org/r/37416/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>



Re: Review Request 37589: Remove unnecessary usage information.

2015-08-18 Thread Guangya Liu

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

Ship it!


Ship It!

- Guangya Liu


On 八月 18, 2015, 6:36 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37589/
> ---
> 
> (Updated 八月 18, 2015, 6:36 p.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-3239
> https://issues.apache.org/jira/browse/MESOS-3239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Remove unnecessary usage information.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp a94a5eec0d4cf095f84255a66117947a308fdb2f 
>   src/master/http.cpp d016b3c055a06e7aa0649a36fd4b095c2938ba56 
>   src/master/registrar.cpp ca8efb4000a688ea42c84123db2a0d3fbe1227c3 
>   src/slave/http.cpp b0fe5f520dfca156548ba8c436d42fc432223f3d 
>   src/slave/monitor.cpp 82aa659b02f98ad8dc4c4f556be7a332dfba6816 
> 
> Diff: https://reviews.apache.org/r/37589/diff/
> 
> 
> Testing
> ---
> 
> manual test
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 37586: Generate usage help information in libprocess.

2015-08-18 Thread Guangya Liu

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


Just curious: If change the 3rd party code directly in mesos source code, then 
how to handle the case when mesos want to upgrade the 3rd party libraries?

- Guangya Liu


On 八月 18, 2015, 6:32 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37586/
> ---
> 
> (Updated 八月 18, 2015, 6:32 p.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-3239
> https://issues.apache.org/jira/browse/MESOS-3239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Generate usage help information in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> 441f6d1c15afebdd98b73bb7430fcd8c8cf1e333 
>   3rdparty/libprocess/include/process/system.hpp 
> 7c8b49e78f76f9e131a4367f411c6dba447ccd90 
>   3rdparty/libprocess/src/help.cpp e4e0bb617956c7f79380069015b7170aa7716231 
>   3rdparty/libprocess/src/logging.cpp 
> 3d855e90e83a54cd344e49f075af0eadef1a1358 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> b9617507a16318b7de25d4875d6bc0b4409fcd29 
>   3rdparty/libprocess/src/profiler.cpp 
> 65a2e05e8f6005a378ae3647698dcba60fb95e9f 
> 
> Diff: https://reviews.apache.org/r/37586/diff/
> 
> 
> Testing
> ---
> 
> manual test
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 37417: Convert Perf event validator to use new shared object.

2015-08-18 Thread Ben Mahler

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


Did you still want to use 'Perf' here? If not, I can't tell what we're gaining 
in this change, does supported() provide additional validation?

- Ben Mahler


On Aug. 18, 2015, 11:01 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37417/
> ---
> 
> (Updated Aug. 18, 2015, 11:01 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3185
> https://issues.apache.org/jira/browse/MESOS-3185
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Convert Perf event validator to use new shared object.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 
> 
> Diff: https://reviews.apache.org/r/37417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>



Re: Review Request 37177: Maintenance Primitives: Added inverse offers.

2015-08-18 Thread Guangya Liu

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



include/mesos/maintenance/maintenance.hpp (line 38)


Remove this blank space. The commit should not be permitted if there are 
blank space in code.


- Guangya Liu


On 八月 18, 2015, 6:57 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37177/
> ---
> 
> (Updated 八月 18, 2015, 6:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp 
> aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 
> e278139f856888d6c6f538f7c0f664087e97f629 
>   src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9748ca0b3fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/master_allocator_tests.cpp 
> 89331965553505f6b7eebf39ad27d943df816a24 
>   src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 
>   src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b 
>   src/tests/resource_offers_tests.cpp 
> 882a9ff4d09aace486182828bf43b643b0d0c519 
>   src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc 
> 
> Diff: https://reviews.apache.org/r/37177/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

2015-08-18 Thread Guangya Liu

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



src/master/master.cpp (line 4145)


I think the blank line can be removed



src/master/master.cpp (line 4841)


I think the blank line can be removed


- Guangya Liu


On 八月 18, 2015, 6:58 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37180/
> ---
> 
> (Updated 八月 18, 2015, 6:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
> 
> Diff: https://reviews.apache.org/r/37180/diff/
> 
> 
> Testing
> ---
> 
> The tests break as expected.
> With the scheduler API change there are CHECKs that fail.
> Once we update the API these will be resolved.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 37283: Maintenance Primitives: Refactored Master maintenance test to use V1 API.

2015-08-18 Thread Guangya Liu

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

Ship it!


Ship It!

- Guangya Liu


On 八月 18, 2015, 6:58 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37283/
> ---
> 
> (Updated 八月 18, 2015, 6:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/master_maintenance_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37283/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 37417: Convert Perf event validator to use new shared object.

2015-08-18 Thread Paul Brett


> On Aug. 18, 2015, 8:38 p.m., Ben Mahler wrote:
> > Paul can you split this change? I personally don't have context on why we 
> > would want to change from sampling 'true' to running against the init 
> > process so I'd like Jie or Ian to review that change, but I can help you 
> > make the refactorings to use 'Perf'. Sound good?

I will back out that change.


- Paul


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


On Aug. 18, 2015, 11:01 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37417/
> ---
> 
> (Updated Aug. 18, 2015, 11:01 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3185
> https://issues.apache.org/jira/browse/MESOS-3185
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Convert Perf event validator to use new shared object.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 
> 
> Diff: https://reviews.apache.org/r/37417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>



Re: Review Request 37416: Perf supported() should be based on the version of perf, not the version of the kernel.

2015-08-18 Thread Paul Brett

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

(Updated Aug. 19, 2015, 12:57 a.m.)


Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Perf supported() should be based on the version of perf, not the version of the 
kernel.


Diffs (updated)
-

  src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 

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


Testing
---

sudo make check


Thanks,

Paul Brett



Re: Review Request 37493: Detect gflags when present and link when building Python module

2015-08-18 Thread Greg Mann

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

(Updated Aug. 19, 2015, 12:33 a.m.)


Review request for mesos, Joris Van Remoortere, Marco Massenzio, Till 
Toenshoff, and Timothy Chen.


Changes
---

Reordered the changes to configure.ac


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


Repository: mesos


Description
---

Detect gflags when present and link when building Python module


Diffs (updated)
-

  configure.ac db0632d60a6d82ab396931b4d913f34b625a45a7 

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


Testing
---

Did the following:

../configure && make && make check

For each of these cases:
1. using bundled glog, with gflags not installed
2. using bundled glog, with gflags installed
3. using system glog (built without gflags) via --with-glog=..., with gflags 
not installed
4. using system glog (built without gflags) via --with-glog=..., with gflags 
installed
5. using system glog (built with gflags) via --with-glog=..., with gflags 
installed

Test suite passes in all cases.


Thanks,

Greg Mann



Re: Review Request 37493: Detect gflags when present and link when building Python module

2015-08-18 Thread Greg Mann

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

(Updated Aug. 18, 2015, 11:48 p.m.)


Review request for mesos, Joris Van Remoortere, Marco Massenzio, Till 
Toenshoff, and Timothy Chen.


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


Repository: mesos


Description
---

Detect gflags when present and link when building Python module


Diffs (updated)
-

  configure.ac db0632d60a6d82ab396931b4d913f34b625a45a7 

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


Testing
---

Did the following:

../configure && make && make check

For each of these cases:
1. using bundled glog, with gflags not installed
2. using bundled glog, with gflags installed
3. using system glog (built without gflags) via --with-glog=..., with gflags 
not installed
4. using system glog (built without gflags) via --with-glog=..., with gflags 
installed
5. using system glog (built with gflags) via --with-glog=..., with gflags 
installed

Test suite passes in all cases.


Thanks,

Greg Mann



Re: Review Request 37427: Docker registry: adding TokenManager.

2015-08-18 Thread Jojy Varghese

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

(Updated Aug. 18, 2015, 11:24 p.m.)


Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen.


Changes
---

style changes.


Repository: mesos


Description
---

Changes:
  - Added Token implementation (RFC 7519).
  - Added TokenManager implementation. This component keeps a cache of tokens
  requested for any future requests.


Diffs (updated)
-

  src/Makefile.am 457ad26ee55bd7a2aedf27f45db58a9a4a6a5dc5 
  src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION 
  src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 37417: Convert Perf event validator to use new shared object.

2015-08-18 Thread Paul Brett

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

(Updated Aug. 18, 2015, 11:01 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Reduce scope of change per review.


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


Repository: mesos


Description
---

Convert Perf event validator to use new shared object.


Diffs (updated)
-

  src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 

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


Testing
---

sudo make check


Thanks,

Paul Brett



Review Request 37592: Added a defaultWindow to metrics based on LIBPROCESS_STATISTICS_WINDOW

2015-08-18 Thread Greg Mann

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

Review request for mesos.


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


Repository: mesos


Description
---

Added a defaultWindow to metrics based on LIBPROCESS_STATISTICS_WINDOW


Diffs
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
f2e84d8e62df58812b660c858eb3b0366db4 
  3rdparty/libprocess/src/metrics/metrics.cpp 
b9617507a16318b7de25d4875d6bc0b4409fcd29 
  3rdparty/libprocess/src/process.cpp 755187c8761137cb2bf2f7295b29a63f63c68bc6 
  3rdparty/libprocess/src/tests/metrics_tests.cpp 
29ed0330a498579896bbef3349572dd35299a40a 

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


Testing
---

make check


Thanks,

Greg Mann



Re: Review Request 37283: Maintenance Primitives: Refactored Master maintenance test to use V1 API.

2015-08-18 Thread Joseph Wu

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

Ship it!


Ship It!

- Joseph Wu


On Aug. 18, 2015, 11:58 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37283/
> ---
> 
> (Updated Aug. 18, 2015, 11:58 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/master_maintenance_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37283/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

2015-08-18 Thread Joseph Wu

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

Ship it!


LGTM.


src/master/master.cpp (line 5745)


Typo still present.


- Joseph Wu


On Aug. 18, 2015, 11:58 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37180/
> ---
> 
> (Updated Aug. 18, 2015, 11:58 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
> 
> Diff: https://reviews.apache.org/r/37180/diff/
> 
> 
> Testing
> ---
> 
> The tests break as expected.
> With the scheduler API change there are CHECKs that fail.
> Once we update the API these will be resolved.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 37177: Maintenance Primitives: Added inverse offers.

2015-08-18 Thread Joseph Wu

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

Ship it!


Ship It!

- Joseph Wu


On Aug. 18, 2015, 11:57 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37177/
> ---
> 
> (Updated Aug. 18, 2015, 11:57 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp 
> aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 
> e278139f856888d6c6f538f7c0f664087e97f629 
>   src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9748ca0b3fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/master_allocator_tests.cpp 
> 89331965553505f6b7eebf39ad27d943df816a24 
>   src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 
>   src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b 
>   src/tests/resource_offers_tests.cpp 
> 882a9ff4d09aace486182828bf43b643b0d0c519 
>   src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc 
> 
> Diff: https://reviews.apache.org/r/37177/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 37175: Maintenance Primitives: Added updateUnavailability to master.

2015-08-18 Thread Joseph Wu

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

Ship it!


LGTM except for one little thing:


src/master/master.hpp (line 478)


Why is this return type `Nothing`, instead of `void`?


- Joseph Wu


On Aug. 18, 2015, 11:57 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37175/
> ---
> 
> (Updated Aug. 18, 2015, 11:57 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   src/master/allocator/mesos/allocator.hpp 
> aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 
> e278139f856888d6c6f538f7c0f664087e97f629 
>   src/master/http.cpp a73ee17bcef72791b06240a4673f466de582c41b 
>   src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/master_maintenance_tests.cpp PRE-CREATION 
>   src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 
> 
> Diff: https://reviews.apache.org/r/37175/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 37173: Maintenance Primitives: Added unavailability to Allocator's Slave struct.

2015-08-18 Thread Joseph Wu

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

Ship it!


LGTM

- Joseph Wu


On Aug. 18, 2015, 11:57 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37173/
> ---
> 
> (Updated Aug. 18, 2015, 11:57 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
>   include/mesos/master/allocator.proto 
> 10fd9a2d5fcbc18a9ca2d6c9c0ec1c605f21872b 
>   src/master/allocator/mesos/allocator.hpp 
> aa55755a9c3250579e9366bdbc17a2449e95d659 
>   src/master/allocator/mesos/hierarchical.hpp 
> e278139f856888d6c6f538f7c0f664087e97f629 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9748ca0b3fee25dcec51c64d8ba84dbd4aaf 
>   src/tests/master_allocator_tests.cpp 
> 89331965553505f6b7eebf39ad27d943df816a24 
>   src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 
>   src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b 
>   src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc 
> 
> Diff: https://reviews.apache.org/r/37173/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 37172: Maintenance Primitives: Set offer `unavailability` if slave is scheduled for maintenance.

2015-08-18 Thread Joseph Wu

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

Ship it!



include/mesos/maintenance/maintenance.hpp (line 37)


Trailing whitespace.



src/master/master.hpp (lines 288 - 289)


Should we standardize how we refer to the maintenance modes in comments?

I've referred to the modes as pronouns [Normal, Draining, Deactivated].  We 
could change them all to be `screaming`-case.  Or to pronoun-case.


- Joseph Wu


On Aug. 18, 2015, 11:57 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37172/
> ---
> 
> (Updated Aug. 18, 2015, 11:57 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/master_maintenance_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37172/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 37170: Maintenance Primitives: Added `Machine` to Slave struct in Master.

2015-08-18 Thread Joseph Wu

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

Ship it!


LGTM.


src/master/master.cpp (lines 1364 - 1365)


Note: There might be a rebase conflict with 
https://reviews.apache.org/r/37314/diff/4#3 since both have the same text.


- Joseph Wu


On Aug. 18, 2015, 11:57 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37170/
> ---
> 
> (Updated Aug. 18, 2015, 11:57 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
> https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
> 
> Diff: https://reviews.apache.org/r/37170/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 37416: Perf supported() should be based on the version of perf, not the version of the kernel.

2015-08-18 Thread Ben Mahler

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


Just some notes before you rebase.


src/linux/perf.cpp (lines 411 - 413)


How about just saying that this returns the version of perf? :)



src/linux/perf.cpp (line 415)


whoops?



src/linux/perf.cpp (line 420)


output can just be a string since this is a .then continuation, no need to 
wrap into a Future



src/linux/perf.cpp (lines 422 - 426)


strings::remove w/ PREFIX should be easier to use than find / erase here



src/linux/perf.cpp (line 516)


Remember to use a space here


- Ben Mahler


On Aug. 13, 2015, 6:40 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37416/
> ---
> 
> (Updated Aug. 13, 2015, 6:40 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Perf supported() should be based on the version of perf, not the version of 
> the kernel.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 
> 
> Diff: https://reviews.apache.org/r/37416/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>



Re: Review Request 37417: Convert Perf event validator to use new shared object.

2015-08-18 Thread Ben Mahler

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


Paul can you split this change? I personally don't have context on why we would 
want to change from sampling 'true' to running against the init process so I'd 
like Jie or Ian to review that change, but I can help you make the refactorings 
to use 'Perf'. Sound good?


src/linux/perf.cpp (lines 395 - 408)


Looks like change is doing more than just using the Perf object. You've 
also changed this to work off of init rather than the 'true' command? Why the 
latter change? (Not clear from the review description or ticket)

Also, any reason you introduce internal::validate()? Looks like it can just 
be inlined inside valid()?



src/linux/perf.cpp (lines 464 - 467)


Let's try to avoid jaggedness in comments :)



src/linux/perf.cpp (lines 473 - 474)


future.await() is an anti-pattern, so let's explain why it's needed here. I 
assume it's because isolator creation is synchronous currently and making it 
asynchronous is a larger undertaking?



src/linux/perf.cpp (line 476)


Please remember to put spaces between your if and open paren. Ideally this 
should be automated, but it's not right now :)


- Ben Mahler


On Aug. 18, 2015, 6:43 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37417/
> ---
> 
> (Updated Aug. 18, 2015, 6:43 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3185
> https://issues.apache.org/jira/browse/MESOS-3185
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Convert Perf event validator to use new shared object.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 
> 
> Diff: https://reviews.apache.org/r/37417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>



Re: Review Request 37424: Timeout the perf future if the process does not complete.

2015-08-18 Thread Ben Mahler

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

Ship it!


Thanks, the summary here is now stale, but I'll update it for you when I commit.

- Ben Mahler


On Aug. 18, 2015, 6:43 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37424/
> ---
> 
> (Updated Aug. 18, 2015, 6:43 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3185
> https://issues.apache.org/jira/browse/MESOS-3185
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Timeout the perf future if the process does not complete.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 
> 
> Diff: https://reviews.apache.org/r/37424/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>



Re: Review Request 37423: Split out common functions for running "perf" into a common perf class with wrapper functions to allow for reuse between sample, valid and version operations.

2015-08-18 Thread Ben Mahler

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

Ship it!


Will get this committed now, thanks Paul!


src/linux/perf.cpp (line 167)


This seems a little brittle, because the caller may have already put 'perf' 
as the first argument, no? I'll add a note as to why we need this hack.



src/linux/perf.cpp (line 183)


newline for readability?



src/linux/perf.cpp (line 344)


PerfSampler doesn't exist..? We can just remove this comment altogether IMO.



src/linux/perf.cpp (lines 356 - 370)


Recall that with .then your lambda doesn't need to take a Future since it 
only composes when the future succeeds, normally calling .get() on a Future 
would need to be guarded to ensure the future is ready.

Ideally we could compose with Try here:

```
auto stamp = [start, duration] (
const hashmap& statistics) {
  foreachvalue (mesos::PerfStatistics& s, statistics) {
s.set_timestamp(start.secs());
s.set_duration(duration.secs());
  }
  return stats;
}

return future
  .then(perf::parse)
  .then(stamp);
```

But we can't compose like this currently, so I'll just pull out a single 
lambda for parsing / stamping.


- Ben Mahler


On Aug. 18, 2015, 6:10 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37423/
> ---
> 
> (Updated Aug. 18, 2015, 6:10 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3185
> https://issues.apache.org/jira/browse/MESOS-3185
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Split out common functions for running "perf" into a common perf class with 
> wrapper functions to allow for reuse between sample, valid and version 
> operations.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 
> 
> Diff: https://reviews.apache.org/r/37423/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>



Re: Review Request 37584: Fix bug accessing error() when no Error

2015-08-18 Thread Ben Mahler


> On Aug. 18, 2015, 6:34 p.m., Vinod Kone wrote:
> > src/launcher/fetcher.cpp, lines 100-106
> > 
> >
> > just do
> > 
> > return Error("Skipping fetch with Hadoop client: " + 
> >  (available.isError() ? availabe.error() : " client not 
> > found"));
> 
> Marco Massenzio wrote:
> So, that was my first choice, but I then reflected that my finding out 
> the site of the error was made more complicated due to the available log 
> lines being far away from it; so I had to do some digging and investigating.
> 
> In general, I have been taught to prefer to have a LOG(ERROR) near the 
> site where the error actually happens, as that also avoids running wild goose 
> chases :)
> Especially if it so happens that the error message may be 
> (unintentionally) misleading.
> 
> What do you think?
> 
> Vinod Kone wrote:
> I think it's not consistent with how other cases are handled in this 
> file. This particular should've been easy to find because the call site is 
> also in fetcher.cpp. I'll fix this and commit for you.

Well, that leads to double logging unfortunately, since both the callee and 
caller log the same error, no? Typically if we have a library function that 
returns a Try, we'll put enough information in the Try error, and leave it up 
to the caller to decide how to log it. Otherwise, logging gets a bit hard to 
manage, make sense?


- Ben


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


On Aug. 18, 2015, 5:24 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37584/
> ---
> 
> (Updated Aug. 18, 2015, 5:24 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3287
> https://issues.apache.org/jira/browse/MESOS-3287
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When trying to download an artifact with the Hadoop client,
> and the client is not `available()` we correctly return a
> false value, but then in the error message, we tried to
> make a call to `Try::error` which failed and crashed Master.
> 
> This fixes it.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp f8b46b8f957942d0cfdc45d3361f52dae3e514a3 
> 
> Diff: https://reviews.apache.org/r/37584/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>



Re: Review Request 37584: Fix bug accessing error() when no Error

2015-08-18 Thread Vinod Kone


> On Aug. 18, 2015, 6:34 p.m., Vinod Kone wrote:
> > src/launcher/fetcher.cpp, lines 100-106
> > 
> >
> > just do
> > 
> > return Error("Skipping fetch with Hadoop client: " + 
> >  (available.isError() ? availabe.error() : " client not 
> > found"));
> 
> Marco Massenzio wrote:
> So, that was my first choice, but I then reflected that my finding out 
> the site of the error was made more complicated due to the available log 
> lines being far away from it; so I had to do some digging and investigating.
> 
> In general, I have been taught to prefer to have a LOG(ERROR) near the 
> site where the error actually happens, as that also avoids running wild goose 
> chases :)
> Especially if it so happens that the error message may be 
> (unintentionally) misleading.
> 
> What do you think?

I think it's not consistent with how other cases are handled in this file. This 
particular should've been easy to find because the call site is also in 
fetcher.cpp. I'll fix this and commit for you.


- Vinod


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


On Aug. 18, 2015, 5:24 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37584/
> ---
> 
> (Updated Aug. 18, 2015, 5:24 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3287
> https://issues.apache.org/jira/browse/MESOS-3287
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When trying to download an artifact with the Hadoop client,
> and the client is not `available()` we correctly return a
> false value, but then in the error message, we tried to
> make a call to `Try::error` which failed and crashed Master.
> 
> This fixes it.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp f8b46b8f957942d0cfdc45d3361f52dae3e514a3 
> 
> Diff: https://reviews.apache.org/r/37584/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>



Re: Review Request 37588: Fixed master to drop scheduler HTTP calls during recovery.

2015-08-18 Thread Vinod Kone


> On Aug. 18, 2015, 6:49 p.m., Ben Mahler wrote:
> > It seems like Forbidden should be used for authorization issues, can we 
> > just have the non-leaders say they are not available? It seems to make 
> > sense, since they are not elected, they are not available.

changed master to send 503 when it's not elected.


> On Aug. 18, 2015, 6:49 p.m., Ben Mahler wrote:
> > src/master/http.cpp, lines 345-359
> > 
> >
> > Would be nice to have some metrics for visibility into these, given we 
> > have dropped_messages for pid-based schedulers. TODO?

added a TODO.


- Vinod


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


On Aug. 18, 2015, 7:10 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37588/
> ---
> 
> (Updated Aug. 18, 2015, 7:10 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Bugs: MESOS-3290
> https://issues.apache.org/jira/browse/MESOS-3290
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master returns an error when it is not the leader or is still doing recovery. 
> This is same as what happens with PID frameworks.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp d016b3c055a06e7aa0649a36fd4b095c2938ba56 
>   src/scheduler/scheduler.cpp 37f41debc394a773f33465dab1a89d7ef7264f64 
> 
> Diff: https://reviews.apache.org/r/37588/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 37588: Fixed master to drop scheduler HTTP calls during recovery.

2015-08-18 Thread Vinod Kone

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

(Updated Aug. 18, 2015, 7:10 p.m.)


Review request for mesos, Anand Mazumdar and Ben Mahler.


Changes
---

removed the dependency and addressed comments. NNFR.


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


Repository: mesos


Description
---

Master returns an error when it is not the leader or is still doing recovery. 
This is same as what happens with PID frameworks.


Diffs (updated)
-

  src/master/http.cpp d016b3c055a06e7aa0649a36fd4b095c2938ba56 
  src/scheduler/scheduler.cpp 37f41debc394a773f33465dab1a89d7ef7264f64 

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


Testing
---

make check


Thanks,

Vinod Kone



Re: Review Request 37178: Maintenance Primitives: Added InverseOffers to Scheduler Event Offers.

2015-08-18 Thread Joris Van Remoortere

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

(Updated Aug. 18, 2015, 7:02 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  include/mesos/scheduler/scheduler.proto 
89daf8a6b74057ee156b3ad691397e76fcb835b8 
  include/mesos/v1/scheduler/scheduler.proto 
bd5e82a614b1163b29f9b20e562208efa1ba4b55 

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


Testing
---


Thanks,

Joris Van Remoortere



Re: Review Request 37280: Maintenance Primitives: Added updateInverseOffer to Allocator.

2015-08-18 Thread Joris Van Remoortere

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

(Updated Aug. 18, 2015, 6:58 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
  src/master/allocator/mesos/allocator.hpp 
aa55755a9c3250579e9366bdbc17a2449e95d659 
  src/master/allocator/mesos/hierarchical.hpp 
e278139f856888d6c6f538f7c0f664087e97f629 
  src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 

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


Testing
---


Thanks,

Joris Van Remoortere



Re: Review Request 37234: Maintenance Primitives: Added URL field to InverseOffer proto.

2015-08-18 Thread Joris Van Remoortere

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

(Updated Aug. 18, 2015, 6:58 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 

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


Testing
---


Thanks,

Joris Van Remoortere



Re: Review Request 37284: Maintenance Primitives: Added support for Accept / Decline of InverseOffers in master.

2015-08-18 Thread Joris Van Remoortere

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

(Updated Aug. 18, 2015, 6:58 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 

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


Testing
---


Thanks,

Joris Van Remoortere



Re: Review Request 37283: Maintenance Primitives: Refactored Master maintenance test to use V1 API.

2015-08-18 Thread Joris Van Remoortere

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

(Updated Aug. 18, 2015, 6:58 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/master_maintenance_tests.cpp PRE-CREATION 

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


Testing
---


Thanks,

Joris Van Remoortere



Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

2015-08-18 Thread Joris Van Remoortere

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

(Updated Aug. 18, 2015, 6:58 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 

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


Testing
---

The tests break as expected.
With the scheduler API change there are CHECKs that fail.
Once we update the API these will be resolved.


Thanks,

Joris Van Remoortere



Re: Review Request 37177: Maintenance Primitives: Added inverse offers.

2015-08-18 Thread Joris Van Remoortere

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

(Updated Aug. 18, 2015, 6:57 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/maintenance/maintenance.hpp PRE-CREATION 
  include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
  src/master/allocator/mesos/allocator.hpp 
aa55755a9c3250579e9366bdbc17a2449e95d659 
  src/master/allocator/mesos/hierarchical.hpp 
e278139f856888d6c6f538f7c0f664087e97f629 
  src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
  src/tests/hierarchical_allocator_tests.cpp 
9748ca0b3fee25dcec51c64d8ba84dbd4aaf 
  src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24 
  src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 
  src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b 
  src/tests/resource_offers_tests.cpp 882a9ff4d09aace486182828bf43b643b0d0c519 
  src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc 

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


Testing
---


Thanks,

Joris Van Remoortere



Re: Review Request 37282: Maintenance Primitives: Added InverseOffer to V1 API.

2015-08-18 Thread Joris Van Remoortere

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

(Updated Aug. 18, 2015, 6:58 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 
  src/internal/devolve.hpp 6e4306df78b9b8d2054e6550209341fd7b0972d6 
  src/internal/devolve.cpp 0a069e51053b572a8d5dc95380732119504dd0c9 
  src/internal/evolve.hpp 13e9f52da98567038ec717f394f79e526a1521e9 
  src/internal/evolve.cpp 11ce9e77490e93f781ceebc33063d13953a11765 
  src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c 

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


Testing
---


Thanks,

Joris Van Remoortere



Re: Review Request 37175: Maintenance Primitives: Added updateUnavailability to master.

2015-08-18 Thread Joris Van Remoortere

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

(Updated Aug. 18, 2015, 6:57 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
  src/master/allocator/mesos/allocator.hpp 
aa55755a9c3250579e9366bdbc17a2449e95d659 
  src/master/allocator/mesos/hierarchical.hpp 
e278139f856888d6c6f538f7c0f664087e97f629 
  src/master/http.cpp a73ee17bcef72791b06240a4673f466de582c41b 
  src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
  src/tests/master_maintenance_tests.cpp PRE-CREATION 
  src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 

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


Testing
---


Thanks,

Joris Van Remoortere



Re: Review Request 37172: Maintenance Primitives: Set offer `unavailability` if slave is scheduled for maintenance.

2015-08-18 Thread Joris Van Remoortere

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

(Updated Aug. 18, 2015, 6:57 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/maintenance/maintenance.hpp PRE-CREATION 
  src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
  src/tests/master_maintenance_tests.cpp PRE-CREATION 

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


Testing
---


Thanks,

Joris Van Remoortere



Re: Review Request 37176: Maintenance Primitives: Added a new allocation overload to sorter.

2015-08-18 Thread Joris Van Remoortere

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

(Updated Aug. 18, 2015, 6:57 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/allocator/sorter/drf/sorter.hpp 
f66ade06c6a5b4bf816839477cec2d18036c7b1a 
  src/master/allocator/sorter/drf/sorter.cpp 
bfc273493419fe46a4d907f4f7fa282cff71b800 
  src/master/allocator/sorter/sorter.hpp 
536a7ad9a2d661bc8aa352d2e0ae41115b1e8a04 

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


Testing
---


Thanks,

Joris Van Remoortere



Re: Review Request 37173: Maintenance Primitives: Added unavailability to Allocator's Slave struct.

2015-08-18 Thread Joris Van Remoortere

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

(Updated Aug. 18, 2015, 6:57 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
  include/mesos/master/allocator.proto 10fd9a2d5fcbc18a9ca2d6c9c0ec1c605f21872b 
  src/master/allocator/mesos/allocator.hpp 
aa55755a9c3250579e9366bdbc17a2449e95d659 
  src/master/allocator/mesos/hierarchical.hpp 
e278139f856888d6c6f538f7c0f664087e97f629 
  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
  src/tests/hierarchical_allocator_tests.cpp 
9748ca0b3fee25dcec51c64d8ba84dbd4aaf 
  src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24 
  src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 
  src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b 
  src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc 

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


Testing
---


Thanks,

Joris Van Remoortere



Re: Review Request 37178: Maintenance Primitives: Added InverseOffers to Scheduler Event Offers.

2015-08-18 Thread Joris Van Remoortere

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

(Updated Aug. 18, 2015, 6:57 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/scheduler/scheduler.proto 
89daf8a6b74057ee156b3ad691397e76fcb835b8 
  include/mesos/v1/scheduler/scheduler.proto 
bd5e82a614b1163b29f9b20e562208efa1ba4b55 

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


Testing
---


Thanks,

Joris Van Remoortere



Re: Review Request 37170: Maintenance Primitives: Added `Machine` to Slave struct in Master.

2015-08-18 Thread Joris Van Remoortere

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

(Updated Aug. 18, 2015, 6:57 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 

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


Testing
---


Thanks,

Joris Van Remoortere



Re: Review Request 37281: Maintenance Primitives: Added Unavailability to Offer in V1 API.

2015-08-18 Thread Joris Van Remoortere

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

(Updated Aug. 18, 2015, 6:57 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 

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


Testing
---


Thanks,

Joris Van Remoortere



Re: Review Request 37584: Fix bug accessing error() when no Error

2015-08-18 Thread Marco Massenzio


> On Aug. 18, 2015, 6:34 p.m., Vinod Kone wrote:
> > src/launcher/fetcher.cpp, lines 100-106
> > 
> >
> > just do
> > 
> > return Error("Skipping fetch with Hadoop client: " + 
> >  (available.isError() ? availabe.error() : " client not 
> > found"));

So, that was my first choice, but I then reflected that my finding out the site 
of the error was made more complicated due to the available log lines being far 
away from it; so I had to do some digging and investigating.

In general, I have been taught to prefer to have a LOG(ERROR) near the site 
where the error actually happens, as that also avoids running wild goose chases 
:)
Especially if it so happens that the error message may be (unintentionally) 
misleading.

What do you think?


- Marco


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


On Aug. 18, 2015, 5:24 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37584/
> ---
> 
> (Updated Aug. 18, 2015, 5:24 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3287
> https://issues.apache.org/jira/browse/MESOS-3287
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When trying to download an artifact with the Hadoop client,
> and the client is not `available()` we correctly return a
> false value, but then in the error message, we tried to
> make a call to `Try::error` which failed and crashed Master.
> 
> This fixes it.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp f8b46b8f957942d0cfdc45d3361f52dae3e514a3 
> 
> Diff: https://reviews.apache.org/r/37584/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>



Re: Review Request 37588: Fixed master to drop scheduler HTTP calls during recovery.

2015-08-18 Thread Ben Mahler

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

Ship it!


It seems like Forbidden should be used for authorization issues, can we just 
have the non-leaders say they are not available? It seems to make sense, since 
they are not elected, they are not available.


src/master/http.cpp (lines 345 - 359)


Would be nice to have some metrics for visibility into these, given we have 
dropped_messages for pid-based schedulers. TODO?



src/scheduler/scheduler.cpp (line 378)


It seems strange to me that forbidden is coming from non-leaders, forbidden 
seems like an authorization issue, no? Don't we want non-leaders to just say 
they are not available?



src/scheduler/scheduler.cpp (line 379)


Or the leadership changed since we sent this message..?


- Ben Mahler


On Aug. 18, 2015, 6:28 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37588/
> ---
> 
> (Updated Aug. 18, 2015, 6:28 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Bugs: MESOS-3290
> https://issues.apache.org/jira/browse/MESOS-3290
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master returns an error when it is not the leader or is still doing recovery. 
> This is same as what happens with PID frameworks.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp d016b3c055a06e7aa0649a36fd4b095c2938ba56 
>   src/scheduler/scheduler.cpp 37f41debc394a773f33465dab1a89d7ef7264f64 
> 
> Diff: https://reviews.apache.org/r/37588/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 37417: Convert Perf event validator to use new shared object.

2015-08-18 Thread Paul Brett

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

(Updated Aug. 18, 2015, 6:43 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Convert Perf event validator to use new shared object.


Diffs
-

  src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 

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


Testing
---

sudo make check


Thanks,

Paul Brett



Re: Review Request 37424: Timeout the perf future if the process does not complete.

2015-08-18 Thread Paul Brett

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

(Updated Aug. 18, 2015, 6:43 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Timeout the perf future if the process does not complete.


Diffs
-

  src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 

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


Testing
---

sudo make check


Thanks,

Paul Brett



Re: Review Request 37417: Convert Perf event validator to use new shared object.

2015-08-18 Thread Paul Brett

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

(Updated Aug. 18, 2015, 6:42 p.m.)


Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Convert Perf event validator to use new shared object.


Diffs
-

  src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 

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


Testing
---

sudo make check


Thanks,

Paul Brett



Re: Review Request 37587: Add Unavailable HTTP response.

2015-08-18 Thread Ben Mahler

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

Ship it!


Ship It!

- Ben Mahler


On Aug. 18, 2015, 6:26 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37587/
> ---
> 
> (Updated Aug. 18, 2015, 6:26 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Bugs: MESOS-3290
> https://issues.apache.org/jira/browse/MESOS-3290
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Needed this for subsequent review.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/37587/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 37424: Timeout the perf future if the process does not complete.

2015-08-18 Thread Paul Brett

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

(Updated Aug. 18, 2015, 6:40 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Address review comments.


Repository: mesos


Description
---

Timeout the perf future if the process does not complete.


Diffs (updated)
-

  src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 

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


Testing
---

sudo make check


Thanks,

Paul Brett



Re: Review Request 37424: Timeout the perf future if the process does not complete.

2015-08-18 Thread Paul Brett


> On Aug. 17, 2015, 6:22 p.m., Ben Mahler wrote:
> > src/linux/perf.cpp, line 199
> > 
> >
> > We may want to update this to become killtree in a separate patch? Or 
> > are we guaranteed that perf will clean up child processes? If so, that 
> > warrants a comment :)

This is guaranteed to work because of the process group setup by setupChild.  I 
will add an appropriate comment.


> On Aug. 17, 2015, 6:22 p.m., Ben Mahler wrote:
> > src/linux/perf.cpp, lines 304-312
> > 
> >
> > How about we just use the discard semantics that are already set up 
> > inside initialize()? If a discard is requested, we will terminate.
> > 
> > This leaves the composition in the caller, which we usually prefer 
> > (rather than pushing in timeout logic to all libraries, it's easier to 
> > provide discard semantics and have the caller discard when it's no longer 
> > interested).
> > 
> > So the idea here is now your callers use a .after() which does a 
> > discard().

I'll move the timeout to the calling sites where required.


- Paul


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


On Aug. 13, 2015, 12:29 a.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37424/
> ---
> 
> (Updated Aug. 13, 2015, 12:29 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Timeout the perf future if the process does not complete.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 
> 
> Diff: https://reviews.apache.org/r/37424/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>



Re: Review Request 37587: Add Unavailable HTTP response.

2015-08-18 Thread Ben Mahler

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



3rdparty/libprocess/include/process/http.hpp (lines 590 - 601)


Actually, nevermind, this already exists :)


- Ben Mahler


On Aug. 18, 2015, 6:26 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37587/
> ---
> 
> (Updated Aug. 18, 2015, 6:26 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Bugs: MESOS-3290
> https://issues.apache.org/jira/browse/MESOS-3290
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Needed this for subsequent review.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/37587/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Review Request 37586: Generate usage help information in libprocess.

2015-08-18 Thread haosdent huang

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

Review request for mesos.


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


Repository: mesos


Description
---

Generate usage help information in libprocess.


Diffs
-

  3rdparty/libprocess/include/process/help.hpp 
441f6d1c15afebdd98b73bb7430fcd8c8cf1e333 
  3rdparty/libprocess/include/process/system.hpp 
7c8b49e78f76f9e131a4367f411c6dba447ccd90 
  3rdparty/libprocess/src/help.cpp e4e0bb617956c7f79380069015b7170aa7716231 
  3rdparty/libprocess/src/logging.cpp 3d855e90e83a54cd344e49f075af0eadef1a1358 
  3rdparty/libprocess/src/metrics/metrics.cpp 
b9617507a16318b7de25d4875d6bc0b4409fcd29 
  3rdparty/libprocess/src/profiler.cpp 65a2e05e8f6005a378ae3647698dcba60fb95e9f 

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


Testing
---

manual test


Thanks,

haosdent huang



Re: Review Request 37589: Remove unnecessary usage information.

2015-08-18 Thread haosdent huang

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

(Updated Aug. 18, 2015, 6:36 p.m.)


Review request for mesos, Michael Park and Vinod Kone.


Summary (updated)
-

Remove unnecessary usage information.


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


Repository: mesos


Description (updated)
---

Remove unnecessary usage information.


Diffs
-

  src/files/files.cpp a94a5eec0d4cf095f84255a66117947a308fdb2f 
  src/master/http.cpp d016b3c055a06e7aa0649a36fd4b095c2938ba56 
  src/master/registrar.cpp ca8efb4000a688ea42c84123db2a0d3fbe1227c3 
  src/slave/http.cpp b0fe5f520dfca156548ba8c436d42fc432223f3d 
  src/slave/monitor.cpp 82aa659b02f98ad8dc4c4f556be7a332dfba6816 

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


Testing
---

manual test


Thanks,

haosdent huang



Re: Review Request 37584: Fix bug accessing error() when no Error

2015-08-18 Thread Vinod Kone

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



src/launcher/fetcher.cpp (lines 100 - 106)


just do

return Error("Skipping fetch with Hadoop client: " + 
 (available.isError() ? availabe.error() : " client not 
found"));


- Vinod Kone


On Aug. 18, 2015, 5:24 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37584/
> ---
> 
> (Updated Aug. 18, 2015, 5:24 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3287
> https://issues.apache.org/jira/browse/MESOS-3287
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When trying to download an artifact with the Hadoop client,
> and the client is not `available()` we correctly return a
> false value, but then in the error message, we tried to
> make a call to `Try::error` which failed and crashed Master.
> 
> This fixes it.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp f8b46b8f957942d0cfdc45d3361f52dae3e514a3 
> 
> Diff: https://reviews.apache.org/r/37584/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>



Review Request 37589: Generate usage help information in mesos.

2015-08-18 Thread haosdent huang

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

Review request for mesos, Michael Park and Vinod Kone.


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


Repository: mesos


Description
---

Generate usage help information in mesos.


Diffs
-

  src/files/files.cpp a94a5eec0d4cf095f84255a66117947a308fdb2f 
  src/master/http.cpp d016b3c055a06e7aa0649a36fd4b095c2938ba56 
  src/master/registrar.cpp ca8efb4000a688ea42c84123db2a0d3fbe1227c3 
  src/slave/http.cpp b0fe5f520dfca156548ba8c436d42fc432223f3d 
  src/slave/monitor.cpp 82aa659b02f98ad8dc4c4f556be7a332dfba6816 

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


Testing
---

manual test


Thanks,

haosdent huang



Re: Review Request 37586: Generate usage help information in libprocess.

2015-08-18 Thread haosdent huang

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

(Updated Aug. 18, 2015, 6:32 p.m.)


Review request for mesos, Michael Park and Vinod Kone.


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


Repository: mesos


Description
---

Generate usage help information in libprocess.


Diffs (updated)
-

  3rdparty/libprocess/include/process/help.hpp 
441f6d1c15afebdd98b73bb7430fcd8c8cf1e333 
  3rdparty/libprocess/include/process/system.hpp 
7c8b49e78f76f9e131a4367f411c6dba447ccd90 
  3rdparty/libprocess/src/help.cpp e4e0bb617956c7f79380069015b7170aa7716231 
  3rdparty/libprocess/src/logging.cpp 3d855e90e83a54cd344e49f075af0eadef1a1358 
  3rdparty/libprocess/src/metrics/metrics.cpp 
b9617507a16318b7de25d4875d6bc0b4409fcd29 
  3rdparty/libprocess/src/profiler.cpp 65a2e05e8f6005a378ae3647698dcba60fb95e9f 

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


Testing
---

manual test


Thanks,

haosdent huang



Review Request 37588: Fixed master to drop scheduler HTTP calls during recovery.

2015-08-18 Thread Vinod Kone

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

Review request for mesos, Anand Mazumdar and Ben Mahler.


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


Repository: mesos


Description
---

Master returns an error when it is not the leader or is still doing recovery. 
This is same as what happens with PID frameworks.


Diffs
-

  src/master/http.cpp d016b3c055a06e7aa0649a36fd4b095c2938ba56 
  src/scheduler/scheduler.cpp 37f41debc394a773f33465dab1a89d7ef7264f64 

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


Testing
---

make check


Thanks,

Vinod Kone



Review Request 37587: Add Unavailable HTTP response.

2015-08-18 Thread Vinod Kone

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

Review request for mesos, Anand Mazumdar and Ben Mahler.


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


Repository: mesos


Description
---

Needed this for subsequent review.


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
fbd6cf7967173495875a8ea90ed28ade88b982a2 

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


Testing
---

make check


Thanks,

Vinod Kone



Re: Review Request 37417: Convert Perf event validator to use new shared object.

2015-08-18 Thread Paul Brett

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

(Updated Aug. 18, 2015, 6:12 p.m.)


Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Convert Perf event validator to use new shared object.


Diffs
-

  src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 

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


Testing
---

sudo make check


Thanks,

Paul Brett



Re: Review Request 37423: Split out common functions for running "perf" into a common perf class with wrapper functions to allow for reuse between sample, valid and version operations.

2015-08-18 Thread Paul Brett

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

(Updated Aug. 18, 2015, 6:10 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Incorporate review comments.


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


Repository: mesos


Description
---

Split out common functions for running "perf" into a common perf class with 
wrapper functions to allow for reuse between sample, valid and version 
operations.


Diffs (updated)
-

  src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 

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


Testing
---

sudo make check


Thanks,

Paul Brett



Review Request 37584: Fix bug accessing error() when no Error

2015-08-18 Thread Marco Massenzio

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

Review request for mesos and Adam B.


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


Repository: mesos


Description
---

When trying to download an artifact with the Hadoop client,
and the client is not `available()` we correctly return a
false value, but then in the error message, we tried to
make a call to `Try::error` which failed and crashed Master.

This fixes it.


Diffs
-

  src/launcher/fetcher.cpp f8b46b8f957942d0cfdc45d3361f52dae3e514a3 

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


Testing
---

make check


Thanks,

Marco Massenzio



Re: Review Request 37562: The revocable resource information also are missed for slave node in monitoring doc , fix it in this patch.

2015-08-18 Thread Vinod Kone

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

Ship it!


Ship It!

- Vinod Kone


On Aug. 18, 2015, 5:11 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37562/
> ---
> 
> (Updated Aug. 18, 2015, 5:11 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3286
> https://issues.apache.org/jira/browse/MESOS-3286
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The revocable resource information also are missed for slave node in 
> monitoring doc ,fix it in this patch.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 6ddf07d334ece4d4dc83d805ac0c51c9579c47c9 
> 
> Diff: https://reviews.apache.org/r/37562/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>



Re: Review Request 37512: Added user doc for Scheduler HTTP API.

2015-08-18 Thread Guangya Liu

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

Ship it!


Ship It!

- Guangya Liu


On 八月 18, 2015, 3:08 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37512/
> ---
> 
> (Updated 八月 18, 2015, 3:08 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Bugs: MESOS-3281
> https://issues.apache.org/jira/browse/MESOS-3281
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mostly copy pasted from the design doc with some formatting.
> 
> 
> Diffs
> -
> 
>   docs/scheduler_http_api.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37512/diff/
> 
> 
> Testing
> ---
> 
> gist: https://gist.github.com/vinodkone/5ae803ee35274ca46a3e
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 37505: Fix broken health check in docker executor.

2015-08-18 Thread haosdent huang


> On Aug. 17, 2015, 9:59 p.m., Timothy Chen wrote:
> > Hi haosdent, thanks for working on this but I think running the healthcheck 
> > outside of the container doesn't make much sense to me.
> > I think we should try to run it inside of the container (docker exec), but 
> > since docker exec is introduced until few later versions we also need to 
> > check to make sure the right version is running on the slave.

got it. Let me update.


- haosdent


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


On Aug. 16, 2015, 9:12 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37505/
> ---
> 
> (Updated Aug. 16, 2015, 9:12 a.m.)
> 
> 
> Review request for mesos, Adam B and Timothy Chen.
> 
> 
> Bugs: MESOS-3136
> https://issues.apache.org/jira/browse/MESOS-3136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix broken health check in docker executor.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 
>   src/launcher/executor.cpp 9fa7dcfc39a6706f545b3328e468d9cd25d603ae 
>   src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea 
> 
> Diff: https://reviews.apache.org/r/37505/diff/
> 
> 
> Testing
> ---
> 
> # Add a new test, HealthCheckTest.ROOT_DOCKER_DockerHealthyTask
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthyTask" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>