> On March 22, 2016, 5:39 a.m., Jie Yu wrote:
> > Instead of handling /containers endpoint in Slave, I would suggest we
> > dispatch the request to ResourceMonitor. I will rename ResourceMonitor to
> > ContainerMonitor, and we will be deprecating the /monitor/statistics
> > endpoints in the future.
>
> Jay Guo wrote:
> Thanks for reviewing! If I understand correctly, the /containers endpoint
> itself is still `installed in slave`, although the action is `dispatched to`
> ResourceMonitorProcess (aka ContainerMonitor) `via` ResourceMonitor (aka
> ContainerMonitor). We will make corresponding changes if this is what you
> have in mind.
>
> Jay Guo wrote:
> *typo: ResourceMonitorProcess (aka ContainerMonitorProcess)
>
> Jie Yu wrote:
> Yes, this is what I was suggesting! Thanks!
Another question: Can we refactor current `ResourceMonitorProcess` to use
`containerizer` to collect `ResourceUsage/ContainerStatus`?
At present, collection of `ContainerStatus` would delegated back and forth
between MonitorProcess and Slave (Slave -> ResourceMonitor ->
ResourceMonitorProcess -> Slave), which, IMHO, is neither intuitive nor
efficient. If ResourceMonitorProcess could possess a `containerizer`, slave
could simply dispatch the job to ResourceMonitorProcess with a list of
ContainerID.
I'm thinking to refactor ResourceMonitorProcess constructor to something like:
```cpp
explicit ResourceMonitorProcess(
Containerizer* _containerizer,
const lambda::function<list<ContainerID>()>& _containerIds)
: ProcessBase("monitor"),
containerizer(_containerizer),
containerIds(_containerIds),
limiter(2, Seconds(1)) {} // 2 permits per second.
```
In this case, `ResourceMonitorProcess::statistics` consults slave for
`list<ContainerID>
`ResourceMonitorProcess::containerStatus` is invoked by slave with
`list<ContainerID>
This requires modification of several test cases. If you agree, we will submit
a seperate patch of refactor.
Thanks!!
- Jay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45014/#review124735
-----------------------------------------------------------
On March 18, 2016, 5:27 a.m., Jay Guo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45014/
> -----------------------------------------------------------
>
> (Updated March 18, 2016, 5:27 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add /containers endpoint to return ResourceUsage.
>
>
> Diffs
> -----
>
> src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e
> src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762
> src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697
> src/tests/slave_tests.cpp ea1d776077bf638885db8421194aa4427c772169
>
> Diff: https://reviews.apache.org/r/45014/diff/
>
>
> Testing
> -------
>
> make check
> `curl agent_ip:port/containers` returns same json content as `curl
> agent_ip:port/monitor/statistics.json`
>
> This is a draft patch of adding /containers endpoint to agent
> [MESOS-4891](https://issues.apache.org/jira/browse/MESOS-4891)
>
> ContainerStatus will be added to response based on this patch.
>
>
> Thanks,
>
> Jay Guo
>
>
