Re: Review Request 53105: Added an hourly timer for `slave/docker_containerizer/pull`.

2018-06-20 Thread Zhitao Li 


> On June 20, 2018, 12:58 a.m., Qian Zhang wrote:
> > src/slave/containerizer/docker.hpp
> > Lines 140 (patched)
> > 
> >
> > It seems the convention in Mesos is to implement a `struct Metrics` for 
> > all the metrics rather than directly adding the metric here. And the name 
> > of the metric should be in underscore_case instead of camelCase, so I would 
> > suggest to name it `image_pull("containerizer/docker/image_pull", 
> > Hours(1))`.
> > 
> > And is 1 hour too short for this metric?

Ack for name and metrics struct

On window: In our operational experience, if something goes wrong on the docker 
registry side, we typicaly detect that within an hour or so, so I would not 
make this much longer and lose sensitivity. If anyone needs a longer view, they 
can usually pipe the metric into another time series database and perform out 
of bound aggregation from there.


- Zhitao


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


On May 21, 2018, 10:16 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53105/
> ---
> 
> (Updated May 21, 2018, 10:16 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Qian Zhang.
> 
> 
> Bugs: MESOS-6451
> https://issues.apache.org/jira/browse/MESOS-6451
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an hourly timer for `slave/docker_containerizer/pull`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 6bdf4c7f60384473927de437a061fafe94170557 
>   src/slave/containerizer/docker.cpp a4c9c10e91e75f406329cbb2086f39b549cbeed0 
> 
> 
> Diff: https://reviews.apache.org/r/53105/diff/5/
> 
> 
> Testing
> ---
> 
> Ran `mesos-execute` with a docker image and docker containerizer, observed 
> the following metrics:
> ```
> curl -s localhost:5051/metrics/snapshot | jq . | grep pull
>   "containerizer/docker/pull_ms/p999": 26209.36173824,
>   "containerizer/docker/pull_ms/p90": 21036.405248,
>   "containerizer/docker/pull_ms/p": 26256.388615424,
>   "containerizer/docker/pull_ms/p50": 135.570944,
>   "containerizer/docker/pull_ms/max": 26261.613824,
>   "containerizer/docker/pull_ms/p95": 23649.009536,
>   "containerizer/docker/pull_ms/min": 103.215872,
>   "containerizer/docker/pull_ms/p99": 25739.0929664,
>   "containerizer/docker/pull_ms": 26261.613824,
>   "containerizer/docker/pull_ms/count": 3
> ```
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 53105: Added an hourly timer for `slave/docker_containerizer/pull`.

2018-06-20 Thread Qian Zhang 

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




src/slave/containerizer/docker.hpp
Lines 27-28 (original), 27-30 (patched)


Should be:
```
#include 
#include 

#include 
#include 

```



src/slave/containerizer/docker.hpp
Lines 140 (patched)


It seems the convention in Mesos is to implement a `struct Metrics` for all 
the metrics rather than directly adding the metric here. And the name of the 
metric should be in underscore_case instead of camelCase, so I would suggest to 
name it `image_pull("containerizer/docker/image_pull", Hours(1))`.

And is 1 hour too short for this metric?



src/slave/containerizer/docker.cpp
Lines 448-453 (original), 448-454 (patched)


Can we do it in this way?
```
  Future future = pullTimer.time(docker->pull(
container->containerWorkDir,
image,
container->forcePullImage()));
```


- Qian Zhang


On May 22, 2018, 1:16 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53105/
> ---
> 
> (Updated May 22, 2018, 1:16 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Qian Zhang.
> 
> 
> Bugs: MESOS-6451
> https://issues.apache.org/jira/browse/MESOS-6451
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an hourly timer for `slave/docker_containerizer/pull`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 6bdf4c7f60384473927de437a061fafe94170557 
>   src/slave/containerizer/docker.cpp a4c9c10e91e75f406329cbb2086f39b549cbeed0 
> 
> 
> Diff: https://reviews.apache.org/r/53105/diff/5/
> 
> 
> Testing
> ---
> 
> Ran `mesos-execute` with a docker image and docker containerizer, observed 
> the following metrics:
> ```
> curl -s localhost:5051/metrics/snapshot | jq . | grep pull
>   "containerizer/docker/pull_ms/p999": 26209.36173824,
>   "containerizer/docker/pull_ms/p90": 21036.405248,
>   "containerizer/docker/pull_ms/p": 26256.388615424,
>   "containerizer/docker/pull_ms/p50": 135.570944,
>   "containerizer/docker/pull_ms/max": 26261.613824,
>   "containerizer/docker/pull_ms/p95": 23649.009536,
>   "containerizer/docker/pull_ms/min": 103.215872,
>   "containerizer/docker/pull_ms/p99": 25739.0929664,
>   "containerizer/docker/pull_ms": 26261.613824,
>   "containerizer/docker/pull_ms/count": 3
> ```
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 53105: Added an hourly timer for `slave/docker_containerizer/pull`.

2018-06-18 Thread Jason Lai 

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


Ship it!




Ship It!

- Jason Lai


On May 21, 2018, 5:16 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53105/
> ---
> 
> (Updated May 21, 2018, 5:16 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Qian Zhang.
> 
> 
> Bugs: MESOS-6451
> https://issues.apache.org/jira/browse/MESOS-6451
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an hourly timer for `slave/docker_containerizer/pull`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 6bdf4c7f60384473927de437a061fafe94170557 
>   src/slave/containerizer/docker.cpp a4c9c10e91e75f406329cbb2086f39b549cbeed0 
> 
> 
> Diff: https://reviews.apache.org/r/53105/diff/5/
> 
> 
> Testing
> ---
> 
> Ran `mesos-execute` with a docker image and docker containerizer, observed 
> the following metrics:
> ```
> curl -s localhost:5051/metrics/snapshot | jq . | grep pull
>   "containerizer/docker/pull_ms/p999": 26209.36173824,
>   "containerizer/docker/pull_ms/p90": 21036.405248,
>   "containerizer/docker/pull_ms/p": 26256.388615424,
>   "containerizer/docker/pull_ms/p50": 135.570944,
>   "containerizer/docker/pull_ms/max": 26261.613824,
>   "containerizer/docker/pull_ms/p95": 23649.009536,
>   "containerizer/docker/pull_ms/min": 103.215872,
>   "containerizer/docker/pull_ms/p99": 25739.0929664,
>   "containerizer/docker/pull_ms": 26261.613824,
>   "containerizer/docker/pull_ms/count": 3
> ```
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 53105: Added an hourly timer for `slave/docker_containerizer/pull`.

2018-04-30 Thread Zhitao Li 

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

(Updated April 30, 2018, 3:35 p.m.)


Review request for mesos, Xiaojian Huang, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Rebase to revive the patch.


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


Repository: mesos


Description
---

Added an hourly timer for `slave/docker_containerizer/pull`.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 6bdf4c7f60384473927de437a061fafe94170557 
  src/slave/containerizer/docker.cpp a4c9c10e91e75f406329cbb2086f39b549cbeed0 


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

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


Testing
---

Ran `mesos-execute` with a docker image and docker containerizer, observed the 
following metrics:
```
curl -s localhost:5051/metrics/snapshot | jq . | grep pull
  "containerizer/docker/pull_ms/p999": 26209.36173824,
  "containerizer/docker/pull_ms/p90": 21036.405248,
  "containerizer/docker/pull_ms/p": 26256.388615424,
  "containerizer/docker/pull_ms/p50": 135.570944,
  "containerizer/docker/pull_ms/max": 26261.613824,
  "containerizer/docker/pull_ms/p95": 23649.009536,
  "containerizer/docker/pull_ms/min": 103.215872,
  "containerizer/docker/pull_ms/p99": 25739.0929664,
  "containerizer/docker/pull_ms": 26261.613824,
  "containerizer/docker/pull_ms/count": 3
```


Thanks,

Zhitao Li

Re: Review Request 53105: Added an hourly timer for `slave/docker_containerizer/pull`.

2017-05-31 Thread Zhitao Li 

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

(Updated May 31, 2017, 5:58 p.m.)


Review request for mesos, Xiaojian Huang, Gilbert Song, Jason Lai, and Jie Yu.


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


Repository: mesos


Description
---

Added an hourly timer for `slave/docker_containerizer/pull`.


Diffs
-

  src/slave/containerizer/docker.hpp 44efa44586455df416fc9cdc51c92bd9027f24c3 
  src/slave/containerizer/docker.cpp 886528f0f706ef2a3c07246406f3ee1ebc45565d 


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


Testing
---

Ran `mesos-execute` with a docker image and docker containerizer, observed the 
following metrics:
```
curl -s localhost:5051/metrics/snapshot | jq . | grep pull
  "containerizer/docker/pull_ms/p999": 26209.36173824,
  "containerizer/docker/pull_ms/p90": 21036.405248,
  "containerizer/docker/pull_ms/p": 26256.388615424,
  "containerizer/docker/pull_ms/p50": 135.570944,
  "containerizer/docker/pull_ms/max": 26261.613824,
  "containerizer/docker/pull_ms/p95": 23649.009536,
  "containerizer/docker/pull_ms/min": 103.215872,
  "containerizer/docker/pull_ms/p99": 25739.0929664,
  "containerizer/docker/pull_ms": 26261.613824,
  "containerizer/docker/pull_ms/count": 3
```


Thanks,

Zhitao Li

Re: Review Request 53105: Added an hourly timer for `slave/docker_containerizer/pull`.

2017-02-15 Thread Zhitao Li 

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

(Updated Feb. 15, 2017, 9:14 p.m.)


Review request for mesos, Xiaojian Huang, Gilbert Song, Jason Lai, and Jie Yu.


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


Repository: mesos


Description
---

Added an hourly timer for `slave/docker_containerizer/pull`.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 44efa44586455df416fc9cdc51c92bd9027f24c3 
  src/slave/containerizer/docker.cpp 886528f0f706ef2a3c07246406f3ee1ebc45565d 

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


Testing
---

Ran `mesos-execute` with a docker image and docker containerizer, observed the 
following metrics:
```
curl -s localhost:5051/metrics/snapshot | jq . | grep pull
  "containerizer/docker/pull_ms/p999": 26209.36173824,
  "containerizer/docker/pull_ms/p90": 21036.405248,
  "containerizer/docker/pull_ms/p": 26256.388615424,
  "containerizer/docker/pull_ms/p50": 135.570944,
  "containerizer/docker/pull_ms/max": 26261.613824,
  "containerizer/docker/pull_ms/p95": 23649.009536,
  "containerizer/docker/pull_ms/min": 103.215872,
  "containerizer/docker/pull_ms/p99": 25739.0929664,
  "containerizer/docker/pull_ms": 26261.613824,
  "containerizer/docker/pull_ms/count": 3
```


Thanks,

Zhitao Li

Re: Review Request 53105: Added an hourly timer for `slave/docker_containerizer/pull`.

2016-11-06 Thread Zhitao Li 

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

(Updated Nov. 7, 2016, 4:40 a.m.)


Review request for mesos, Xiaojian Huang, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added an hourly timer for `slave/docker_containerizer/pull`.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 006f929eca0e0a6b1de941821ac72869ba393d2d 
  src/slave/containerizer/docker.cpp c2ed5240aab9ea9d1a386c44c94e5ae7e98d313c 

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


Testing
---

Ran `mesos-execute` with a docker image and docker containerizer, observed the 
following metrics:
```
curl -s localhost:5051/metrics/snapshot | jq . | grep pull
  "containerizer/docker/pull_ms/p999": 26209.36173824,
  "containerizer/docker/pull_ms/p90": 21036.405248,
  "containerizer/docker/pull_ms/p": 26256.388615424,
  "containerizer/docker/pull_ms/p50": 135.570944,
  "containerizer/docker/pull_ms/max": 26261.613824,
  "containerizer/docker/pull_ms/p95": 23649.009536,
  "containerizer/docker/pull_ms/min": 103.215872,
  "containerizer/docker/pull_ms/p99": 25739.0929664,
  "containerizer/docker/pull_ms": 26261.613824,
  "containerizer/docker/pull_ms/count": 3
```


Thanks,

Zhitao Li

Re: Review Request 53105: Added an hourly timer for `slave/docker_containerizer/pull`.

2016-10-31 Thread Zhitao Li 

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

(Updated Oct. 31, 2016, 11:43 p.m.)


Review request for mesos, Xiaojian Huang, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added an hourly timer for `slave/docker_containerizer/pull`.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 

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


Testing
---

Ran `mesos-execute` with a docker image and docker containerizer, observed the 
following metrics:
```
curl -s localhost:5051/metrics/snapshot | jq . | grep pull
  "containerizer/docker/pull_ms/p999": 26209.36173824,
  "containerizer/docker/pull_ms/p90": 21036.405248,
  "containerizer/docker/pull_ms/p": 26256.388615424,
  "containerizer/docker/pull_ms/p50": 135.570944,
  "containerizer/docker/pull_ms/max": 26261.613824,
  "containerizer/docker/pull_ms/p95": 23649.009536,
  "containerizer/docker/pull_ms/min": 103.215872,
  "containerizer/docker/pull_ms/p99": 25739.0929664,
  "containerizer/docker/pull_ms": 26261.613824,
  "containerizer/docker/pull_ms/count": 3
```


Thanks,

Zhitao Li

Review Request 53105: Added an hourly timer for `slave/docker_containerizer/pull`.

2016-10-21 Thread Zhitao Li 

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

Review request for mesos, Xiaojian Huang, Gilbert Song, Jason Lai, and Jie Yu.


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


Repository: mesos


Description
---

Added an hourly timer for `slave/docker_containerizer/pull`.


Diffs
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 

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


Testing
---

Ran `mesos-execute` with a docker image and docker containerizer, observed the 
following metrics:
```
curl -s localhost:5051/metrics/snapshot | jq . | grep pull
  "containerizer/docker/pull_ms/p999": 26209.36173824,
  "containerizer/docker/pull_ms/p90": 21036.405248,
  "containerizer/docker/pull_ms/p": 26256.388615424,
  "containerizer/docker/pull_ms/p50": 135.570944,
  "containerizer/docker/pull_ms/max": 26261.613824,
  "containerizer/docker/pull_ms/p95": 23649.009536,
  "containerizer/docker/pull_ms/min": 103.215872,
  "containerizer/docker/pull_ms/p99": 25739.0929664,
  "containerizer/docker/pull_ms": 26261.613824,
  "containerizer/docker/pull_ms/count": 3
```


Thanks,

Zhitao Li