On 12/09/2015 12:16 AM, Frank Ch. Eigler wrote:
...
Here's some more feedback, both on pmFetchGroup API and the py binding.
I've not read the code in detail yet - just exploring the API for now.
- this is slotted for merging in the next release (3.11), as per
http://pcp.io/roadmap
Overall, I really like it, especially the py binding where we're basically
manipulating lists of 3 tuples (inst, name, pmAtomValue) and get all the
type conversion and rate conversion for free.This leads to some
very compact code compared to traditional pmapi.
- sentinal value of 0 as an alternative error indicator for integer metrics,
as explained at the end of the pmfetchgroup(3) man page :
0 is a very valid value for many metrics (especially those with instant
semantics)
so is hardly a good choice for indicating "no value available".
Perhaps just do away with this and rely only on the per-instance error returns?
(and mandate the currently optional error arrays in the fetchgroup_create
calls)?
- Need some API documentation and more examples of the python binding in
pmFetchGroup(3)
- Internal exception handling to pmReconnectContext, especially for
pmFetchGroup()
but also for all other fetchgroup API functions. Otherwise we'll end up with
duplicated code to do this in new clients. It could be transparent (?)
- We can extract inst id, inst name, values and tiemstamps. But how about
helptext and
some of the other descriptor fields? Or just use the existing pmapi functions
off the
contextfor this?
- how come no support for PM_TYPE_AGGREGATE (and events)?
- Do we want a c++ binding (similar to API to py)?
Also, the fg py binding might probably slot into the pmcc classes better than
pmapi,
but then under the hood you've extended libpcp so I guess it belongs in
pmapi.py,
so we might be able to eventually delete some of the pmcc helpers (?)
- overload pmCreateFetchGroup() to take either a context, or a source string,
(defaulting to "local:"). And then provide method to return the context for
use by other pmapi functions.
- consider another function that is a cross between pmExtendFetchGroup_item and
_indom supplying a regex for a subset of instance names (i.e. use an indom
profile)
Would be very handy for tools that want to filter their reports, and more
scalable
and efficient with very large indoms since they'd actually use the profile,
e.g. massive disk farms with multipath scsi. And proc.
- heaps more QA :
qa for multiple fetchgroups from the same context
qa for multiple fetchgroups using multiple contexts with same and different
host
(ditto in archive mode for qa determinism)
qa for pmFetchGroupSetMode in archive mode. Some API code for pmtime would
help too.
qa for multithreaded use (with separate fetch group in each thread). Not
safe to share
a fetchgroup handle between threads (that's fine and made clear), but what
about
separate fg handles bound to the same context?
qa with valgrind to check pmDestroyFetchGroup frees everything for all the
above tests
and also that regular tools using this API don't grow unnecessarily.
qa for rate conversion of counter metrics (and clearer err reporting)
qa coverage on all supported platforms
- The following doesn't work - the error message could perhaps be improved to
report that two fetches are needed for rate conversion of metrics with counter
semantics, rather than just throwing an obscure exception about nested lambdas
and missing metric values :P
#! /usr/bin/pcp python
from pcp import pmapi
import cpmapi as c_api
ctx = pmapi.pmContext(c_api.PM_CONTEXT_HOST, "local:")
f = pmapi.fetchgroup(ctx)
ndisk = f.extend_item("hinv.ndisk")
disks = f.extend_indom("disk.dev.read")
f.fetch()
print ("%d disks:" % ndisk())
for inst, name, rdops in disks():
print ("\t[%02d] \"%s\" %lu" % (inst, name, rdops()))
-- snip --
# ./fetchgrouptest2.py
2 disks:
Traceback (most recent call last):
File "./fetchgrouptest2.py", line 16, in <module>
print ("\t[%02d] \"%s\" %lu" % (inst, name, rdops()))
File "/usr/lib64/python3.4/site-packages/pcp/pmapi.py", line 2139, in <lambda>
(lambda i: (lambda: decode_one(self, i)))(i))) # nested lambda for proper i
capture
File "/usr/lib64/python3.4/site-packages/pcp/pmapi.py", line 2132, in
decode_one
raise pmErr(self.stss[i])
pcp.pmapi.pmErr: PM_ERR_VALUE Missing metric value(s)
out of time today for any more ...
Cheers
-- Mark
|