----------------------------------------------------------- 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) <https://reviews.apache.org/r/53105/#comment287930> Should be: ``` #include <process/owned.hpp> #include <process/shared.hpp> #include <process/metrics/metrics.hpp> #include <process/metrics/timer.hpp> ``` src/slave/containerizer/docker.hpp Lines 140 (patched) <https://reviews.apache.org/r/53105/#comment287927> 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) <https://reviews.apache.org/r/53105/#comment287929> Can we do it in this way? ``` Future<Docker::Image> 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/p9999": 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 > >
