Re: Review Request 51477: Implemented `RunTaskGroupMessage` handler on the agent.

2016-08-31 Thread Qian Zhang

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




src/slave/slave.cpp (line 1549)


I would suggest to change it to `ignoring running` in order to be 
consistent with the other warning message.



src/slave/slave.cpp (lines 1686 - 1690)


Should it be `CHECK_NE(task.isSome(), taskGroup.isSome())`?



src/slave/slave.cpp (line 1721)


s/run/running/



src/slave/slave.cpp (line 1752)


I think we can `break` from here.



src/slave/slave.cpp (lines 1777 - 1778)


Kill one blank line.



src/slave/slave.cpp (lines 1851 - 1852)


I think we need to `break` between these two `}` if kill == true.



src/slave/slave.cpp (line 1879)


I do not think we need this, because if we get here, `kill` must be still 
`false`.



src/slave/slave.cpp (line 1946)


Not yours, but I think we do not need this blank line.



src/slave/slave.cpp (lines 2091 - 2095)


Should it be `CHECK_NE(tasks.isSome(), taskGroups.isSome())`?



src/slave/slave.cpp (line 2521)


Here we send a `KillTaskMessage` to executor, but the task to be killed may 
be part of a task group, so that means it is executor's responsibility to kill 
the tasks in the whole group, right? What if executor does not do it? Will we 
enforce it in agent?


- Qian Zhang


On Aug. 28, 2016, 1:44 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51477/
> ---
> 
> (Updated Aug. 28, 2016, 1:44 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6076
> https://issues.apache.org/jira/browse/MESOS-6076
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This changes implements the `runTaskGroup()` handler on the
> agent ensuring that task group is sent atomically to the executor
> via the `LAUNCH_GROUP` event. It also refactors `runTask()`/`_runTask()`
> to go through a common handler function. Also, it ensures that all
> tasks in `framework->pending`/`queuedTasks` that are killed before
> running the task group result in all the tasks being killed.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 4add4c0180ea56039e0d5009bad4d9346128bde6 
>   src/slave/slave.cpp 5d162d00a8a9a0e318c39bda93dd584f23c87987 
> 
> Diff: https://reviews.apache.org/r/51477/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 51560: Deprecated using health checks without setting the type.

2016-08-31 Thread haosdent huang


> On Aug. 31, 2016, 7:07 p.m., Silas Snider wrote:
> > src/health-check/health_checker.cpp, line 183
> > 
> >
> > This is incorrect. In 1.0.0, you could specify *either* HTTP *or* 
> > Command healthcheck without specifying the type field, but this change will 
> > make the absence of type only support command health checks.
> 
> haosdent huang wrote:
> Actually we didn't support HTTP/TCP before 1.0.0. So I think should be OK?
> 
> Silas Snider wrote:
> Indeed, but it was in the proto for use by schedulers/executors.
> 
> haosdent huang wrote:
> Sorry, I still could not get the idea here. 
> 
> If there are not used -> Should be OK although schedulers/executors use 
> old protocols without using HTTP/TCP health check. -> It should be safe to 
> set the type to COMMAND if it is empty, right?
> 
> Silas Snider wrote:
> The point is: just because Mesos itself didn't use them doesn't mean that 
> they weren't being set/read by custom executors and frameworks. It's still 
> part of the API that you support either command or http healthchecks *in the 
> api* without type being set.
> 
> Silas Snider wrote:
> Also, it *was* supported by 1.0.0, so it **must** be supported until 2.0.
> 
> haosdent huang wrote:
> >The point is: just because Mesos itself didn't use them doesn't mean 
> that they weren't being set/read by custom executors and frameworks. It's 
> still part of the API that you support either command or http healthchecks in 
> the api without type being set.
> 
> Thank you very much, got it.
> 
> >Also, it was supported by 1.0.0, so it must be supported until 2.0.
> 
> I think we need to discuss about it more.
> 
> Silas Snider wrote:
> Can you clarify? What needs to be discussed in your opinion?

Do you use slack, we could discuss in this channel: 
https://mesos.slack.com/messages/health-check/ . Could join mesos slack via 
https://mesos-slackin.herokuapp.com .

I think we still need add a type to determine current health check type. 
Otherwise, we would confuse when user set command, http and tcp fields in 
`HealthCheck` protobuf message.


- haosdent


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


On Aug. 31, 2016, 6:29 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51560/
> ---
> 
> (Updated Aug. 31, 2016, 6:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Bugs: MESOS-6110
> https://issues.apache.org/jira/browse/MESOS-6110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Deprecated using health checks without setting the type.
> 
> 
> Diffs
> -
> 
>   CHANGELOG eadf546f96e4f8f83107de3e1421197875a46e2b 
>   docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 
>   src/health-check/health_checker.cpp 
> f373df19fc8af8e9650be61e3b101e89362a67cd 
> 
> Diff: https://reviews.apache.org/r/51560/diff/
> 
> 
> Testing
> ---
> 
> Could verfied from 
> https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 51560: Deprecated using health checks without setting the type.

2016-08-31 Thread Silas Snider


> On Aug. 31, 2016, 7:07 p.m., Silas Snider wrote:
> > src/health-check/health_checker.cpp, line 183
> > 
> >
> > This is incorrect. In 1.0.0, you could specify *either* HTTP *or* 
> > Command healthcheck without specifying the type field, but this change will 
> > make the absence of type only support command health checks.
> 
> haosdent huang wrote:
> Actually we didn't support HTTP/TCP before 1.0.0. So I think should be OK?
> 
> Silas Snider wrote:
> Indeed, but it was in the proto for use by schedulers/executors.
> 
> haosdent huang wrote:
> Sorry, I still could not get the idea here. 
> 
> If there are not used -> Should be OK although schedulers/executors use 
> old protocols without using HTTP/TCP health check. -> It should be safe to 
> set the type to COMMAND if it is empty, right?
> 
> Silas Snider wrote:
> The point is: just because Mesos itself didn't use them doesn't mean that 
> they weren't being set/read by custom executors and frameworks. It's still 
> part of the API that you support either command or http healthchecks *in the 
> api* without type being set.
> 
> Silas Snider wrote:
> Also, it *was* supported by 1.0.0, so it **must** be supported until 2.0.
> 
> haosdent huang wrote:
> >The point is: just because Mesos itself didn't use them doesn't mean 
> that they weren't being set/read by custom executors and frameworks. It's 
> still part of the API that you support either command or http healthchecks in 
> the api without type being set.
> 
> Thank you very much, got it.
> 
> >Also, it was supported by 1.0.0, so it must be supported until 2.0.
> 
> I think we need to discuss about it more.

Can you clarify? What needs to be discussed in your opinion?


- Silas


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


On Aug. 31, 2016, 6:29 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51560/
> ---
> 
> (Updated Aug. 31, 2016, 6:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Bugs: MESOS-6110
> https://issues.apache.org/jira/browse/MESOS-6110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Deprecated using health checks without setting the type.
> 
> 
> Diffs
> -
> 
>   CHANGELOG eadf546f96e4f8f83107de3e1421197875a46e2b 
>   docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 
>   src/health-check/health_checker.cpp 
> f373df19fc8af8e9650be61e3b101e89362a67cd 
> 
> Diff: https://reviews.apache.org/r/51560/diff/
> 
> 
> Testing
> ---
> 
> Could verfied from 
> https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 51560: Deprecated using health checks without setting the type.

2016-08-31 Thread haosdent huang


> On Aug. 31, 2016, 7:07 p.m., Silas Snider wrote:
> > src/health-check/health_checker.cpp, line 183
> > 
> >
> > This is incorrect. In 1.0.0, you could specify *either* HTTP *or* 
> > Command healthcheck without specifying the type field, but this change will 
> > make the absence of type only support command health checks.
> 
> haosdent huang wrote:
> Actually we didn't support HTTP/TCP before 1.0.0. So I think should be OK?
> 
> Silas Snider wrote:
> Indeed, but it was in the proto for use by schedulers/executors.
> 
> haosdent huang wrote:
> Sorry, I still could not get the idea here. 
> 
> If there are not used -> Should be OK although schedulers/executors use 
> old protocols without using HTTP/TCP health check. -> It should be safe to 
> set the type to COMMAND if it is empty, right?
> 
> Silas Snider wrote:
> The point is: just because Mesos itself didn't use them doesn't mean that 
> they weren't being set/read by custom executors and frameworks. It's still 
> part of the API that you support either command or http healthchecks *in the 
> api* without type being set.
> 
> Silas Snider wrote:
> Also, it *was* supported by 1.0.0, so it **must** be supported until 2.0.

>The point is: just because Mesos itself didn't use them doesn't mean that they 
>weren't being set/read by custom executors and frameworks. It's still part of 
>the API that you support either command or http healthchecks in the api 
>without type being set.

Thank you very much, got it.

>Also, it was supported by 1.0.0, so it must be supported until 2.0.

I think we need to discuss about it more.


- haosdent


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


On Aug. 31, 2016, 6:29 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51560/
> ---
> 
> (Updated Aug. 31, 2016, 6:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Bugs: MESOS-6110
> https://issues.apache.org/jira/browse/MESOS-6110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Deprecated using health checks without setting the type.
> 
> 
> Diffs
> -
> 
>   CHANGELOG eadf546f96e4f8f83107de3e1421197875a46e2b 
>   docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 
>   src/health-check/health_checker.cpp 
> f373df19fc8af8e9650be61e3b101e89362a67cd 
> 
> Diff: https://reviews.apache.org/r/51560/diff/
> 
> 
> Testing
> ---
> 
> Could verfied from 
> https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 51560: Deprecated using health checks without setting the type.

2016-08-31 Thread Silas Snider


> On Aug. 31, 2016, 7:07 p.m., Silas Snider wrote:
> > src/health-check/health_checker.cpp, line 183
> > 
> >
> > This is incorrect. In 1.0.0, you could specify *either* HTTP *or* 
> > Command healthcheck without specifying the type field, but this change will 
> > make the absence of type only support command health checks.
> 
> haosdent huang wrote:
> Actually we didn't support HTTP/TCP before 1.0.0. So I think should be OK?
> 
> Silas Snider wrote:
> Indeed, but it was in the proto for use by schedulers/executors.
> 
> haosdent huang wrote:
> Sorry, I still could not get the idea here. 
> 
> If there are not used -> Should be OK although schedulers/executors use 
> old protocols without using HTTP/TCP health check. -> It should be safe to 
> set the type to COMMAND if it is empty, right?
> 
> Silas Snider wrote:
> The point is: just because Mesos itself didn't use them doesn't mean that 
> they weren't being set/read by custom executors and frameworks. It's still 
> part of the API that you support either command or http healthchecks *in the 
> api* without type being set.

Also, it *was* supported by 1.0.0, so it **must** be supported until 2.0.


- Silas


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


On Aug. 31, 2016, 6:29 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51560/
> ---
> 
> (Updated Aug. 31, 2016, 6:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Bugs: MESOS-6110
> https://issues.apache.org/jira/browse/MESOS-6110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Deprecated using health checks without setting the type.
> 
> 
> Diffs
> -
> 
>   CHANGELOG eadf546f96e4f8f83107de3e1421197875a46e2b 
>   docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 
>   src/health-check/health_checker.cpp 
> f373df19fc8af8e9650be61e3b101e89362a67cd 
> 
> Diff: https://reviews.apache.org/r/51560/diff/
> 
> 
> Testing
> ---
> 
> Could verfied from 
> https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 51560: Deprecated using health checks without setting the type.

2016-08-31 Thread Silas Snider


> On Aug. 31, 2016, 7:07 p.m., Silas Snider wrote:
> > src/health-check/health_checker.cpp, line 183
> > 
> >
> > This is incorrect. In 1.0.0, you could specify *either* HTTP *or* 
> > Command healthcheck without specifying the type field, but this change will 
> > make the absence of type only support command health checks.
> 
> haosdent huang wrote:
> Actually we didn't support HTTP/TCP before 1.0.0. So I think should be OK?
> 
> Silas Snider wrote:
> Indeed, but it was in the proto for use by schedulers/executors.
> 
> haosdent huang wrote:
> Sorry, I still could not get the idea here. 
> 
> If there are not used -> Should be OK although schedulers/executors use 
> old protocols without using HTTP/TCP health check. -> It should be safe to 
> set the type to COMMAND if it is empty, right?

The point is: just because Mesos itself didn't use them doesn't mean that they 
weren't being set/read by custom executors and frameworks. It's still part of 
the API that you support either command or http healthchecks *in the api* 
without type being set.


- Silas


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


On Aug. 31, 2016, 6:29 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51560/
> ---
> 
> (Updated Aug. 31, 2016, 6:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Bugs: MESOS-6110
> https://issues.apache.org/jira/browse/MESOS-6110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Deprecated using health checks without setting the type.
> 
> 
> Diffs
> -
> 
>   CHANGELOG eadf546f96e4f8f83107de3e1421197875a46e2b 
>   docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 
>   src/health-check/health_checker.cpp 
> f373df19fc8af8e9650be61e3b101e89362a67cd 
> 
> Diff: https://reviews.apache.org/r/51560/diff/
> 
> 
> Testing
> ---
> 
> Could verfied from 
> https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 51560: Deprecated using health checks without setting the type.

2016-08-31 Thread haosdent huang


> On Aug. 31, 2016, 7:07 p.m., Silas Snider wrote:
> > src/health-check/health_checker.cpp, line 183
> > 
> >
> > This is incorrect. In 1.0.0, you could specify *either* HTTP *or* 
> > Command healthcheck without specifying the type field, but this change will 
> > make the absence of type only support command health checks.
> 
> haosdent huang wrote:
> Actually we didn't support HTTP/TCP before 1.0.0. So I think should be OK?
> 
> Silas Snider wrote:
> Indeed, but it was in the proto for use by schedulers/executors.

Sorry, I still could not get the idea here. 

If there are not used -> Should be OK although schedulers/executors use old 
protocols without using HTTP/TCP health check. -> It should be safe to set the 
type to COMMAND if it is empty, right?


- haosdent


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


On Aug. 31, 2016, 6:29 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51560/
> ---
> 
> (Updated Aug. 31, 2016, 6:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Bugs: MESOS-6110
> https://issues.apache.org/jira/browse/MESOS-6110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Deprecated using health checks without setting the type.
> 
> 
> Diffs
> -
> 
>   CHANGELOG eadf546f96e4f8f83107de3e1421197875a46e2b 
>   docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 
>   src/health-check/health_checker.cpp 
> f373df19fc8af8e9650be61e3b101e89362a67cd 
> 
> Diff: https://reviews.apache.org/r/51560/diff/
> 
> 
> Testing
> ---
> 
> Could verfied from 
> https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 51392: Supported provisioner listContainers() to be recursive.

2016-08-31 Thread Jie Yu

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




src/slave/containerizer/mesos/provisioner/paths.cpp (line 148)


s/std/lambda/


- Jie Yu


On Aug. 29, 2016, 9:20 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51392/
> ---
> 
> (Updated Aug. 29, 2016, 9:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
> Wu, and Kevin Klues.
> 
> 
> Bugs: MESOS-6067
> https://issues.apache.org/jira/browse/MESOS-6067
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch supports collecting all containerIds (all containers in the
> nested hierarchy) recursively.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/paths.cpp 
> 86a45f30c22dc1e41f4779c0ce8c11d02dcc46bb 
> 
> Diff: https://reviews.apache.org/r/51392/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>



Re: Review Request 51566: Changed Master::_accept() to not pass invalid tasks to the allocator.

2016-08-31 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [51566]

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

- Mesos ReviewBot


On Aug. 31, 2016, 10:42 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51566/
> ---
> 
> (Updated Aug. 31, 2016, 10:42 p.m.)
> 
> 
> Review request for mesos and Anindya Sinha.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> So in Allocator::updateAllocation() we can assume all tasks are valid.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 192b2ce5eb24deb6e843f2f1a8c915114614b268 
> 
> Diff: https://reviews.apache.org/r/51566/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>



Re: Review Request 51379: Entered the appropriate task's namespaces during health checking.

2016-08-31 Thread Jie Yu

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




src/health-check/health_checker.cpp (line 89)


ns::setns is not async signal safe. You cannot call it directly. Please 
take a look at this review
https://reviews.apache.org/r/51277/diff/1#index_header


- Jie Yu


On Aug. 28, 2016, 1:58 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51379/
> ---
> 
> (Updated Aug. 28, 2016, 1:58 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón 
> Kleiman, Gilbert Song, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5961
> https://issues.apache.org/jira/browse/MESOS-5961
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 8d679cd33b6ddf3a5c11bb8c458a97b8809473ac 
>   src/health-check/health_checker.hpp 
> b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
>   src/health-check/health_checker.cpp 
> 5bdbd2ab45feecfab494bcacfbcc92c396f4f723 
>   src/launcher/executor.cpp 71ede1ea4f4e97fe94bd2bd136f17f231cedbce6 
> 
> Diff: https://reviews.apache.org/r/51379/diff/
> 
> 
> Testing
> ---
> 
> Manual testing of various scenarios for both command and docker executors; 
> host, bridged, and overlay networks; container and non-container tasks. 
> Details see here:
> https://gist.github.com/haosdent/577e4a527e2a80e2aa1b1eb783870f8d
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 51378: Exposed `process::internal::defaultClone` to `process` namespace.

2016-08-31 Thread Jie Yu

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




3rdparty/libprocess/include/process/posix/subprocess.hpp (lines 55 - 68)


Any reason you need to expose this internal method? Why not just write your 
own clone function at the call sites? THis default clone sounds pretty straight 
forward to me. Copy that to the call site is probably more readable.

`process::defaultClone` just sounds like a very weird public interface.

I'd suggest we revert this change.


- Jie Yu


On Aug. 28, 2016, 1:55 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51378/
> ---
> 
> (Updated Aug. 28, 2016, 1:55 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón 
> Kleiman, Gilbert Song, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5961
> https://issues.apache.org/jira/browse/MESOS-5961
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed `process::internal::defaultClone` to `process` namespace.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/posix/subprocess.hpp 
> a871fe484905165eed093124687c4920f3968ccc 
> 
> Diff: https://reviews.apache.org/r/51378/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/51379/
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 51565: Added a way to set logrotate settings per executor.

2016-08-31 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [51565]

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

- Mesos ReviewBot


On Aug. 31, 2016, 10:23 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51565/
> ---
> 
> (Updated Aug. 31, 2016, 10:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Cody Maloney, Artem Harutyunyan, 
> and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The provided `LogrotateContainerLogger` did not have enough granularity
> when setting log rotation settings.  This patch adds a way for each
> executor to set its own log rotation settings, using the global values
> as defaults.
> 
> The executor settings are provided via environment variables in the
> `ExecutorInfo`.
> 
> 
> Diffs
> -
> 
>   docs/logging.md 3b36870c22336440b0d7bf1359e1d0a97986a0f6 
>   src/slave/container_loggers/lib_logrotate.hpp 
> f216548ef37f5c2245ef64d21e84e06100e8e5ae 
>   src/slave/container_loggers/lib_logrotate.cpp 
> 01552752a56ee7377a631a783f2168ba0eea2799 
>   src/tests/container_logger_tests.cpp 
> e8f934106510fe02b8b92be19c918a1e5c0b78fd 
> 
> Diff: https://reviews.apache.org/r/51565/diff/
> 
> 
> Testing
> ---
> 
> Previewed documentation change via the website previewer.
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 45960: Added interfaces to handle and track shareable resources.

2016-08-31 Thread Jiang Yan Xu

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


Ship it!




I moved over the following tests from /r/45964/ which covers changes in this 
review.

```
TEST_F(ResourceValidationTest, UnshareableResource)
TEST_F(ResourceValidationTest, SharedPersistentVolume)
TEST(SharedResourcesTest, Filter)
```

- Jiang Yan Xu


On Aug. 24, 2016, 9:13 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45960/
> ---
> 
> (Updated Aug. 24, 2016, 9:13 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added interfaces to handle and track shareable resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp b132933a499a70a3f032d6910621202840be5576 
>   include/mesos/v1/resources.hpp e7f53292f0b8c7cf7ef04d26ef1f7e42b9a0b687 
>   src/common/resources.cpp a5f5902d8f7f2757e3aee35619bff5cc3a52f29b 
>   src/v1/resources.cpp 172217505d80d66cb7e10b3635dc273229313601 
> 
> Diff: https://reviews.apache.org/r/45960/diff/
> 
> 
> Testing
> ---
> 
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>



Review Request 51566: Changed Master::_accept() to not pass invalid tasks to the allocator.

2016-08-31 Thread Jiang Yan Xu

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

Review request for mesos and Anindya Sinha.


Repository: mesos


Description
---

So in Allocator::updateAllocation() we can assume all tasks are valid.


Diffs
-

  src/master/master.cpp 192b2ce5eb24deb6e843f2f1a8c915114614b268 

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


Testing
---

make check.


Thanks,

Jiang Yan Xu



Review Request 51565: Added a way to set logrotate settings per executor.

2016-08-31 Thread Joseph Wu

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

Review request for mesos, Benjamin Mahler, Cody Maloney, Artem Harutyunyan, and 
Vinod Kone.


Repository: mesos


Description
---

The provided `LogrotateContainerLogger` did not have enough granularity
when setting log rotation settings.  This patch adds a way for each
executor to set its own log rotation settings, using the global values
as defaults.

The executor settings are provided via environment variables in the
`ExecutorInfo`.


Diffs
-

  docs/logging.md 3b36870c22336440b0d7bf1359e1d0a97986a0f6 
  src/slave/container_loggers/lib_logrotate.hpp 
f216548ef37f5c2245ef64d21e84e06100e8e5ae 
  src/slave/container_loggers/lib_logrotate.cpp 
01552752a56ee7377a631a783f2168ba0eea2799 
  src/tests/container_logger_tests.cpp e8f934106510fe02b8b92be19c918a1e5c0b78fd 

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


Testing
---

Previewed documentation change via the website previewer.

make check


Thanks,

Joseph Wu



Re: Review Request 51561: Fixed the overview table style in upgrades.md.

2016-08-31 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [51560, 51561]

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

- Mesos ReviewBot


On Aug. 31, 2016, 6:29 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51561/
> ---
> 
> (Updated Aug. 31, 2016, 6:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the overview table style in upgrades.md.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 
> 
> Diff: https://reviews.apache.org/r/51561/diff/
> 
> 
> Testing
> ---
> 
> Could verfied from 
> https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 51553: Changed the way `HAP::updateAllocation()` calls `Resources.apply()`.

2016-08-31 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [51553]

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

- Mesos ReviewBot


On Aug. 31, 2016, 6:27 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51553/
> ---
> 
> (Updated Aug. 31, 2016, 6:27 p.m.)
> 
> 
> Review request for mesos and Anindya Sinha.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> ```
> This is needed for supporting LAUNCH operations with shared resources
> (More details in https://reviews.apache.org/r/45961/) but this patch
> itself has no functional change.
> 
> Also, this patch together with https://reviews.apache.org/r/51412/
> results in no change in the granularity in which Offer operations are
> applied. Each single operation in the ACCEPT call is still individually
> applied to Resources objects managed by the master, the allocator and
> the sorters, same as before.
> ```
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 9e5db2196c6a541dc1208ba8b9f13ef9a518bcc4 
> 
> Diff: https://reviews.apache.org/r/51553/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>



Re: Review Request 51560: Deprecated using health checks without setting the type.

2016-08-31 Thread Silas Snider


> On Aug. 31, 2016, 7:07 p.m., Silas Snider wrote:
> > src/health-check/health_checker.cpp, line 183
> > 
> >
> > This is incorrect. In 1.0.0, you could specify *either* HTTP *or* 
> > Command healthcheck without specifying the type field, but this change will 
> > make the absence of type only support command health checks.
> 
> haosdent huang wrote:
> Actually we didn't support HTTP/TCP before 1.0.0. So I think should be OK?

Indeed, but it was in the proto for use by schedulers/executors.


- Silas


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


On Aug. 31, 2016, 6:29 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51560/
> ---
> 
> (Updated Aug. 31, 2016, 6:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Bugs: MESOS-6110
> https://issues.apache.org/jira/browse/MESOS-6110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Deprecated using health checks without setting the type.
> 
> 
> Diffs
> -
> 
>   CHANGELOG eadf546f96e4f8f83107de3e1421197875a46e2b 
>   docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 
>   src/health-check/health_checker.cpp 
> f373df19fc8af8e9650be61e3b101e89362a67cd 
> 
> Diff: https://reviews.apache.org/r/51560/diff/
> 
> 
> Testing
> ---
> 
> Could verfied from 
> https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 51560: Deprecated using health checks without setting the type.

2016-08-31 Thread haosdent huang


> On Aug. 31, 2016, 7:07 p.m., Silas Snider wrote:
> > src/health-check/health_checker.cpp, line 183
> > 
> >
> > This is incorrect. In 1.0.0, you could specify *either* HTTP *or* 
> > Command healthcheck without specifying the type field, but this change will 
> > make the absence of type only support command health checks.

Actually we didn't support HTTP/TCP before 1.0.0. So I think should be OK?


- haosdent


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


On Aug. 31, 2016, 6:29 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51560/
> ---
> 
> (Updated Aug. 31, 2016, 6:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Bugs: MESOS-6110
> https://issues.apache.org/jira/browse/MESOS-6110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Deprecated using health checks without setting the type.
> 
> 
> Diffs
> -
> 
>   CHANGELOG eadf546f96e4f8f83107de3e1421197875a46e2b 
>   docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 
>   src/health-check/health_checker.cpp 
> f373df19fc8af8e9650be61e3b101e89362a67cd 
> 
> Diff: https://reviews.apache.org/r/51560/diff/
> 
> 
> Testing
> ---
> 
> Could verfied from 
> https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 51560: Deprecated using health checks without setting the type.

2016-08-31 Thread Silas Snider

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




src/health-check/health_checker.cpp (line 183)


This is incorrect. In 1.0.0, you could specify *either* HTTP *or* Command 
healthcheck without specifying the type field, but this change will make the 
absence of type only support command health checks.


- Silas Snider


On Aug. 31, 2016, 6:29 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51560/
> ---
> 
> (Updated Aug. 31, 2016, 6:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Bugs: MESOS-6110
> https://issues.apache.org/jira/browse/MESOS-6110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Deprecated using health checks without setting the type.
> 
> 
> Diffs
> -
> 
>   CHANGELOG eadf546f96e4f8f83107de3e1421197875a46e2b 
>   docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 
>   src/health-check/health_checker.cpp 
> f373df19fc8af8e9650be61e3b101e89362a67cd 
> 
> Diff: https://reviews.apache.org/r/51560/diff/
> 
> 
> Testing
> ---
> 
> Could verfied from 
> https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 51560: Deprecated using health checks without setting the type.

2016-08-31 Thread Joseph Wu

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




src/health-check/health_checker.cpp (line 178)


The deprecation cycle is 6 months now, so we can only remove this TODO 
around version ~1.4


- Joseph Wu


On Aug. 31, 2016, 11:29 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51560/
> ---
> 
> (Updated Aug. 31, 2016, 11:29 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Bugs: MESOS-6110
> https://issues.apache.org/jira/browse/MESOS-6110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Deprecated using health checks without setting the type.
> 
> 
> Diffs
> -
> 
>   CHANGELOG eadf546f96e4f8f83107de3e1421197875a46e2b 
>   docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 
>   src/health-check/health_checker.cpp 
> f373df19fc8af8e9650be61e3b101e89362a67cd 
> 
> Diff: https://reviews.apache.org/r/51560/diff/
> 
> 
> Testing
> ---
> 
> Could verfied from 
> https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 50994: Added 'HealthCheck' protobuf validation.

2016-08-31 Thread haosdent huang


> On Aug. 30, 2016, 10:34 p.m., Joseph Wu wrote:
> > src/health-check/health_checker.cpp, lines 308-310
> > 
> >
> > Looks like this breaks backwards compatibility:
> > https://issues.apache.org/jira/browse/MESOS-6110

Thx @kaysoky ! Posted a fix at https://reviews.apache.org/r/51560


- haosdent


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


On Aug. 12, 2016, 11:16 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50994/
> ---
> 
> (Updated Aug. 12, 2016, 11:16 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Gastón 
> Kleiman.
> 
> 
> Bugs: MESOS-6025
> https://issues.apache.org/jira/browse/MESOS-6025
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'HealthCheck' protobuf validation.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> 7bc345bbc835f31feb2114cba434ab544451dbb0 
>   src/health-check/health_checker.cpp 
> d43cb0568c120cbcec6a73d232396ccc54cf3e58 
>   src/master/validation.hpp 43d876b00511d4ef652c2685d4950c994b5eef31 
>   src/master/validation.cpp af18e5aef3be59830b0a7b0235bbc739dba1029c 
>   src/tests/health_check_tests.cpp 3b44d80fd079f216896f9386fb22d518061938ab 
> 
> Diff: https://reviews.apache.org/r/50994/diff/
> 
> 
> Testing
> ---
> 
> Test via `sudo GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="*HealthCheckTest*" --verbose`
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 51561: Fixed the overview table style in upgrades.md.

2016-08-31 Thread haosdent huang

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

(Updated Aug. 31, 2016, 6:29 p.m.)


Review request for mesos, Alexander Rukletsov and Joseph Wu.


Repository: mesos


Description
---

Fixed the overview table style in upgrades.md.


Diffs (updated)
-

  docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 

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


Testing (updated)
---

Could verfied from 
https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md


Thanks,

haosdent huang



Re: Review Request 51560: Deprecated using health checks without setting the type.

2016-08-31 Thread haosdent huang

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

(Updated Aug. 31, 2016, 6:29 p.m.)


Review request for mesos, Alexander Rukletsov and Joseph Wu.


Summary (updated)
-

Deprecated using health checks without setting the type.


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


Repository: mesos


Description (updated)
---

Deprecated using health checks without setting the type.


Diffs (updated)
-

  CHANGELOG eadf546f96e4f8f83107de3e1421197875a46e2b 
  docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 
  src/health-check/health_checker.cpp f373df19fc8af8e9650be61e3b101e89362a67cd 

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


Testing (updated)
---

Could verfied from 
https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md


Thanks,

haosdent huang



Review Request 51553: Changed the way `HAP::updateAllocation()` calls `Resources.apply()`.

2016-08-31 Thread Jiang Yan Xu

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

Review request for mesos and Anindya Sinha.


Repository: mesos


Description
---

```
This is needed for supporting LAUNCH operations with shared resources
(More details in https://reviews.apache.org/r/45961/) but this patch
itself has no functional change.

Also, this patch together with https://reviews.apache.org/r/51412/
results in no change in the granularity in which Offer operations are
applied. Each single operation in the ACCEPT call is still individually
applied to Resources objects managed by the master, the allocator and
the sorters, same as before.
```


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
9e5db2196c6a541dc1208ba8b9f13ef9a518bcc4 

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


Testing
---

make check


Thanks,

Jiang Yan Xu



Review Request 51561: Fixed the overview table style in upgrades.md.

2016-08-31 Thread haosdent huang

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

Review request for mesos, Alexander Rukletsov and Joseph Wu.


Repository: mesos


Description
---

Fixed the overview table style in upgrades.md.


Diffs
-

  docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 

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


Testing
---


Thanks,

haosdent huang



Review Request 51560: Deprecated the health check without type specified.

2016-08-31 Thread haosdent huang

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

Review request for mesos, Alexander Rukletsov and Joseph Wu.


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


Repository: mesos


Description
---

Deprecated the health check without type specified.


Diffs
-

  CHANGELOG eadf546f96e4f8f83107de3e1421197875a46e2b 
  docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 
  src/health-check/health_checker.cpp f373df19fc8af8e9650be61e3b101e89362a67cd 

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


Testing
---


Thanks,

haosdent huang



Re: Review Request 51027: Track allocation candidates to bound allocator.

2016-08-31 Thread Klaus Ma


> On Aug. 26, 2016, 2:24 p.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 275
> > 
> >
> > I think we should use `insert(...)` instead of `=`; if the following 
> > event in the queue, are we still going to get the correct result?
> > ```
> > batch -> addFramework(f1) -> addFramework(f2)
> > ```
> 
> Jacob Janco wrote:
> insert in a loop?

No; if multiple frameworks register at almost the same time, will there be 
multiple addFramework events pending in the queue? Did not get a chance to test 
it, but did we consider such kind of race condition?


- Klaus


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


On Aug. 31, 2016, 12:53 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51027/
> ---
> 
> (Updated Aug. 31, 2016, 12:53 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, James Peach, Klaus 
> Ma, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3157
> https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Triggered allocations dispatch allocate() only
>   if there is no pending allocation in the queue.
> - Allocation candidates are accumulated and only
>   cleared when enqueued allocations are processed.
> - Batched allocations are handled synchronously.
> 
>   
> - Carrying over work from 
>   https://reviews.apache.org/r/41658/ and added 
>   the previous reviewers
> - Specifically, this patch introduces the boolean 
>   flag pendingAllocation, which when set on event 
>   triggered allocations, will prevent additional 
>   no-op allocations: the flag is cleared when the 
>   enqueued allocation is processed, subsequent 
>   event triggered allocations will update a set
>   of allocation candidates rather than dispatching 
>   an additional allocate().
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> bdbc6d3b5b959990538f4e3b7b1a3b031d9aea05 
>   src/master/allocator/mesos/hierarchical.cpp 
> 234ef98529964a0b6d3f132426a6c8ccbb1263ee 
> 
> Diff: https://reviews.apache.org/r/51027/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> note: check without filters depends on https://reviews.apache.org/r/51028
> 
> With new benchmark https://reviews.apache.org/r/49617: 
> Sample output without 51027:
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
> Using 1 agents and 3000 frameworks
> Added 3000 frameworks in 57251us
> Added 1 agents in 3.2134535333mins
> allocator settled after  1.6123603833mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
>  (290578 ms)
> 
> Sample output with 51027:
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
> Using 1 agents and 3000 frameworks
> Added 3000 frameworks in 39817us
> Added 1 agents in 3.2286054167mins
> allocator settled after  25.525654secs
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
>  (220137 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>



Re: Review Request 45961: Support sharing of resources through reference counting of resources.

2016-08-31 Thread Anindya Sinha

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

(Updated Aug. 31, 2016, 9:17 a.m.)


Review request for mesos, Benjamin Mahler, Joris Van Remoortere, Michael Park, 
and Jiang Yan Xu.


Changes
---

Addressed review comments and migration to updated updateAllocation() api


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


Repository: mesos


Description
---

o Each shared resource is accouted via its share count. This count is
  updated based on the resource operations (such as add and subtract)
  in various scenarios such as task launch and terminate at multiple
  modules such as master, allocator, sorter, etc.
o Only allow DESTROY if there are no running or pending tasks using
  the volume. However, if the volume is in a pending offer to one or
  more frameworks, rescind that offer before processing the DESTROY.
o To allow multiple tasks to be launched in the same ACCEPT call
  using the same shared resource, we update the allocator and
  sorter with additional copies of shared resources to reflect the
  true shared count of allocated shared resources.


Diffs (updated)
-

  src/common/resources.cpp a5f5902d8f7f2757e3aee35619bff5cc3a52f29b 
  src/common/resources_utils.cpp 8e881a09500b0966b577e514077460f724868a8d 
  src/master/allocator/mesos/hierarchical.hpp 
dd07ed221d2c1755d2478369641ffdc46ecc4471 
  src/master/allocator/mesos/hierarchical.cpp 
9e5db2196c6a541dc1208ba8b9f13ef9a518bcc4 
  src/master/allocator/sorter/drf/sorter.hpp 
09aa685f2bd7197385959d7d70d5411d0fd72f06 
  src/master/allocator/sorter/drf/sorter.cpp 
1f2954c564eea5a3c649a5cc7181cb69329f9e35 
  src/master/http.cpp 525ef6cd7545d25f3ac89a6325ace6e6d768262a 
  src/master/master.hpp c32c7e9d859ef73216354e2c03ecc07d0009b12f 
  src/master/master.cpp 192b2ce5eb24deb6e843f2f1a8c915114614b268 
  src/master/validation.hpp 3205ee1e699d0fb7ca16ced96a07be3a07cf8956 
  src/master/validation.cpp 15e2ecc2de99d6bed522f45f855ba686bf19c008 
  src/tests/master_validation_tests.cpp 
e440ff3f6d04f797e65874b9f610ed63d9f28e0e 
  src/tests/sorter_tests.cpp 821e30d6574b045d25d4de4f7c3b8ac5346d3002 
  src/v1/resources.cpp 172217505d80d66cb7e10b3635dc273229313601 

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


Testing
---

Tests successful.


Thanks,

Anindya Sinha