Hi David,
----- Original Message -----
> First, some background. I'm working on integrating systemtap and pcp,
> and I wrote a python PMDA that will be similar to the mmv pmda in that
> it will need to monitor a directory for new data. After working on it, I
> found out that the pcp python support doesn't really allow adding new
> metrics (or clearing metrics) after PMDA.run() has been called.
>
> So, commit 38983a5 in the pcpfans.git dsmith/dev branch adds a 'refresh
> metrics' callback to the pcp python support. This allows a python pmda
> to add/remove metrics after PMDA.run() has been called.
Looking good.
> Here's how you'd use it. In your PMDA.init() function, you'd set the
> callback, like this:
>
> self.set_refresh_metrics(self.__refresh_metrics)
>
> Then your __refresh_metrics (or whatever you want to call it) function
> will get called when needed. I based this code on the mmv pmda, so it
> could get called during several PDUs. Then your callback function could
> add metrics. New code in the PMDA class will update everything.
diff --git a/src/python/pcp/pmda.py b/src/python/pcp/pmda.py
index 5d4e71b..51263aa 100644
--- a/src/python/pcp/pmda.py
+++ b/src/python/pcp/pmda.py
@@ -287,6 +287,7 @@ class PMDA(MetricDispatch):
pmdaname = 'pmda' + name
helpfile = '%s/%s/help' % (PCP.pmGetConfig('PCP_PMDAS_DIR'), name)
MetricDispatch.__init__(self, domain, pmdaname, logfile, helpfile)
+ self.__refresh_metrics_func = None
(see below for more, but I'm not sure we need to keep this
handle to the callback routine up at this level as well as
the cpmda callback handle? maybe we can simplify this)
##
@@ -345,6 +346,26 @@ class PMDA(MetricDispatch):
metrics[i] = self._metrictable[i]
cpmda.pmda_dispatch(ibuf.raw, numindoms, mbuf.raw, nummetrics)
+ def __refresh_metrics(self):
Hmm, do we need this interface? (there seem to be no callers in
the wrapper - is the intention the PMDA would call this? If so
a double-underscore (internal) routine is probably not right
(for an API we are expecting PMDAs to use). If not explicitly
needed, we should keep it simple and drop it (but, maybe I am
missing something there?)
I was expecting this "refreshing" would happen around the time
PDUs exchanges happen with pmcd, which delays it until the last
moment (and prevents the situation where unnecessary duplication
of "refresh" work from a PMDA is done). Can we set that "refresh
needed" flag in cpmda land from the PMDA and delay this work til
then? That'd also encapsulate a bit more of the implementation
details by pushing that down into the lower layers of the wrapper.
+ # Call the user's function.
+ if self.__refresh_metrics_func:
+ self.__refresh_metrics_func()
+
+ # Now that the user's function has been called, we need to
+ # refresh all our class internal data.
+ #
+ # FIXME: instead of using cpmda.get_need_refresh(), we could
+ # ask the user to return a '1' from the refresh metrics
+ # function if a refresh is needed.
+ if cpmda.get_need_refresh():
+ self.pmns_refresh()
+ cpmda.pmid_oneline_refresh(self._metric_oneline)
+ cpmda.pmid_longtext_refresh(self._metric_helptext)
+ cpmda.indom_oneline_refresh(self._indom_oneline)
+ cpmda.indom_longtext_refresh(self._indom_helptext)
+ cpmda.pmda_rehash(self._indomtable, self._metrictable)
This ended up with lots of calls back down into the cpmda layer
from the higher level python code - kinda feels like we may have
some unnecessary up/down-calls here? (if that makes sense at all).
IOW, can we push most of this down into the cpmda layer, hopefully
making the interface exposed to PMDAs a little simpler/clearer?
diff --git a/src/python/pmda.c b/src/python/pmda.c
index f0c4845..3d6079c 100644
--- a/src/python/pmda.c
+++ b/src/python/pmda.c
[...]
+static PyObject *
+pmda_rehash(PyObject *self, PyObject *args, PyObject *keywords)
+{
+ char *keyword_list[] = {"indoms", "metrics", NULL};
+
+ if (indom_list) {
+ Py_DECREF(indom_list);
+ indom_list = NULL;
+ }
+ if (metric_list) {
+ Py_DECREF(metric_list);
+ metric_list = NULL;
+ }
+
+ if (!PyArg_ParseTupleAndKeywords(args, keywords, "OO", keyword_list,
+ &indom_list, &metric_list))
+ return NULL;
+
+ if (indom_list && metric_list) {
Do we require both of these (indom_list/metric_list) to be passed in?
I guess it may happen that this code is used from PMDAs that do not
have indoms (or the dynamic state is such that currently there are
only metrics with no associated indoms?) - in those cases, we could
be called here with a metric_list only I guess, so the logic in this
branch would need some tweaking.
#ifdef PyBUF_SIMPLE
+ static void
+ pmda_refresh_metrics(void)
... you've got a fair bit of new code there that's only going to be
compiled for the python 2.x series (guarded by the above ifdef) - I
haven't tried, but I'm punting we'll need to refactor a bit here to
make this work for python 3 as well.
cheers.
--
Nathan
|