On 10/04/2014 07:38 AM, Frank Ch. Eigler wrote:
[...]
First of all, what do you think of the general flavour?
Amazingly coincidental choice of names, check out :
git://oss.sgi.com/markgw/pcp/pcp-new-open-source.git
src/libpcp_mon/src/fetchgroup.[ch]
which I wrote way back in 1995 (!) as part of the original pmchart - in
response to exactly the same API wordiness issues that you've encountered.
We separated out the fetchGroup API and put it in libpcp_mon. You
can probably reuse some or all of that code, it's really well debugged.
The original concept is similar to your new proposal in that it juggles
multiple contexts for you (multiple live or archive, including multiple
archives for the same host, but not live and archive simultaneously),
and plonks values directly into your application's data structures on
each fetch, using pointers supplied during setup of each fetch group.
Second, do you endorse the type-safe nature of passing float*
etc. destinations for the metric values, tolerating N sibling
foo_TYPE() functions, instead of something icky like the pmAtomValue
unions or void pointers?
yes definitely, though you could take it further and canonicalize
on just double (or even long double for better precision), string
and aggregate (and event?).
Third, I haven't fully sketched out what to do with
PM_TYPE_{STRING,AGGREGATE}; probably have the user pass a destination
buffer/length, or else have libpcp allocate, or (why not) both as
alternatives. Fine?
yep, that would work just fine as well, provided everyone is clear
on where the alloc and free need to occur.
Fourth, I don't know what to do with events. They're fiendishly
complicated in the pmValue structures, and would have a whale of a
time encoding its recursive subdivision as one function call. Maybe
skip it? Maybe some nasty varargs monstrosity?
hmm, we didn't have that problem back in 1995 :) Just treat it in a
similar way - copy the event into a user supplied buffer .. something
like that, basically a special aggregate type.
Fifth, it is entirely possible for values to come back absent from a
pmFetch, and thus a pmFetchGroup. Encoding the absence of data is
probably necessary, but I'm not sure whether better to do it with a
formal flag (another bool* argument at the *Extend* functions), or
maybe just use sentinel values (0, NaN) on the output. Or both?
the original fetchGroupFetch has integrated error handling, with
tri-state error results. See comments in the code. This worked well,
but did require the application to check for errors before dereferencing
the returned values.
Sixth, not sure how/whether to represent indom profiles. Maybe not
needed; just have the user invoke the non-_indom variant of the
*Extend* functions, enumerating the instances and places where to put
each's data. Or maybe have a variant of the _indom_ functions where
the instance names are already provided?
indom profiles are optimized via the optFetch API that Ken wrote,
circa the same time period. Some regex instance name matching in
the API could be handy too though ...
Seventh, several of the above represent optionalities that could lead
to a cartesian-product-style explosion of the number of individual
functions in the API. I don't mind that FWIW, but someone might.
as above, canonicalize on double, string and aggregate; after all,
most stuff we're interested in will be rate converted anyway, and
and so end up as a double result for the application to report.
The exception could be metrics with discrete semantics, maybe.
Eigth, aw heck, why not say that *Extend* with pmfg index #0 means
"do it right now", i.e., the equivalent of
tmp = pmCreateFetchGroup()
pmExtendFetchGroup(tmp, .....)
pmFetchGroup(tmp)
pmDestroyFetchGroup(tmp)
What could be simpler for one-shot inquiries?
wrap that up in a function, e.g.
ncpu = fetchGroupGetValue(source, time, "hinv.ncpu")
where time is NOW (live) or some archive position.
Ninth, this should apply OK to archives too (for consecutive fetches);
should we bother wrap pmSetMode, or just let the client call it
directly before pmFetchGroup()?
pmFetchGroupArchiveMode(int mode, const struct timeval *when, int interval)
in the old implementation seemed to work well - it walks all contexts and
sets the current time (aka position), sampling interval, replay direction
etc. Similarly for archive bounds, so you could have multiple archives open
and set the bounds you're interested in for all of them. The API then
picks which archive context to fetch from based on the current time,
source, metric and instance.
Feedback welcome!
there you go, and half the work's already done :)
Cheers
-- Mark
|