Mailing-List: contact mesos-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: mesos-dev@incubator.apache.org
Content-Type: multipart/alternative;
	boundary="===============1064919541839772802=="
MIME-Version: 1.0
Subject: Re: Review Request: Create utilities to collect information from the
 proc
 filesystem
From: "Charles Reiss" <woggling@gmail.com>
To: "Charles Reiss" <woggling@gmail.com>,
	"Alex Degtiar" <alex.degtiar@gmail.com>,
	"mesos" <mesos-dev@incubator.apache.org>
Date: Thu, 08 Dec 2011 00:19:42 -0000
Message-ID: <20111208001942.25014.43361@reviews.apache.org>
In-Reply-To: <20111207211013.27036.32981@reviews.apache.org>
References: <20111207211013.27036.32981@reviews.apache.org>

--===============1064919541839772802==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable


> On 2011-12-07 21:10:13, Charles Reiss wrote:
> > src/monitoring/proc_utils.hpp, line 33
> > <https://reviews.apache.org/r/3050/diff/1/?file=3D62807#file62807line33>
> >
> >     Why strings? (and elsewhere)
> >
> =

> Alex Degtiar wrote:
>     I was deciding between using unsigned integers, something like the pi=
d_t type, and strings. I ultimately decided on strings because they support=
ed special non-integer 'pids' proc lets you query (i.e. "self"). This was c=
onvenient for testing.

I don't think there's any special "PID" other than "self", and that's easy =
replaced with getpid() (which your test already needs).


> On 2011-12-07 21:10:13, Charles Reiss wrote:
> > src/monitoring/proc_utils.hpp, line 37
> > <https://reviews.apache.org/r/3050/diff/1/?file=3D62807#file62807line37>
> >
> >     Why milliseconds? libprocess uses seconds since some time, and I th=
ink USER_HZ usually (often?) isn't 1000.
> =

> Alex Degtiar wrote:
>     Sam and I decided on milliseconds (since epoch when appropriate) for =
all measured times because it was the granularity closest to the times in v=
arious sources, and having one unit used consistently seemed cleaner. Would=
 it make it more consistent with the rest of Mesos if we scale it up to sec=
onds?
>     =

>     For reference, granularity of various times we used:
>     process start time (used for duration of initial read)- jiffies (4ms =
+/- depending on system HZ)
>     boot time (used for start time to make it since epoch)- seconds
>     current time (used for duration) - nanoseconds/milliseconds since epo=
ch (depends on system)
>     cpu time (proc)- clock ticks (10ms +/- depending on system SC_CLK_TCK)
>     cpu time (lxc)- nanoseconds (unit returned in, not sure of actual gra=
nularity)
>     Period of measurement - potential lower bound on a single machine is =
probably in the millisecond range.

Assuming process::Clock::now() can be counted on to be in time since the ep=
och (I don't actually know if this is an API gaurentee), then I think you s=
hould have that replace getCurrentTime() and thus use seconds since the epo=
ch for everything.


- Charles


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


On 2011-12-08 00:00:32, Alex Degtiar wrote:
> =

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

> (Updated 2011-12-08 00:00:32)
> =

> =

> Review request for mesos.
> =

> =

> Summary
> -------
> =

> The first of several patches related to resource usage monitoring. This p=
atch provides a collection of utilities for use on Linux for reading stats =
from proc. It is used by both the lxc and proc resource collectors.
> =

> =

> This addresses bug MESOS-89.
>     https://issues.apache.org/jira/browse/MESOS-89
> =

> =

> Diffs
> -----
> =

>   src/tests/Makefile.in ea943f7 =

>   src/tests/proc_utils_tests.cpp PRE-CREATION =

>   src/monitoring/proc_utils.cpp PRE-CREATION =

>   src/Makefile.in 516f128 =

>   src/monitoring/proc_utils.hpp PRE-CREATION =

> =

> Diff: https://reviews.apache.org/r/3050/diff
> =

> =

> Testing
> -------
> =

> Sanity tests have been written in src/tests/proc_utils_tests.cpp for all =
utility functions, and functions have been tested ad hoc.
> =

> =

> Thanks,
> =

> Alex
> =

>


--===============1064919541839772802==--