Hello,
Here is a proposed rework of the way dynamic.c delegates
responsibility for handling a dynamic tree. From:
https://github.com/ubccr/pcp/tree/martins_working
commit 50ac6b858e94759123523017330da270c1517122
Author: Martins Innus <minnus@xxxxxxxxxxx>
Date: Fri Dec 5 11:21:54 2014 -0500
Rework dynamic metric support
Force the check that determines if a metric should be handled by a
certain dynamic tree to traverse the pmns instead of just checking the
cluster
This allows 2 things:
-clusters to be shared between dynamic trees
-does not require related trees to have a common prefix
This requires that the dynamic hash be valid at initailization time
All pmdas that use dynamic metrics must now call pmdaTreeRebuildHash
This was previously required only of daemen pmdas. For dso pmdas,
some sort of auto rebuild was occurring
Now this is also required for dso pmdas
Added fixes for the linux interrupts and gfs2 pmdas, although I
have no way to test the gfs2 pmda
src/include/pcp/pmda.h | 2 +
src/libpcp_pmda/src/dynamic.c | 108
++++++++++++++++++++++++++--------------
src/libpcp_pmda/src/tree.c | 2 +-
src/pmdas/gfs2/sbstats.c | 1 +
src/pmdas/gfs2/worst_glock.c | 1 +
src/pmdas/linux/interrupts.c | 1 +
6 files changed, 76 insertions(+), 39 deletions(-)
Previously, the delegation had 2 requirements: The metric must (1) fall
in the tree prefix and (2) be in the cluster list provided to
pmdaDynamicPMNS. This prevented the same logic from handling related
trees that differed in prefix (the hotproc.foo / proc.foo case ). Also,
if a cluster spanned more than one sub-tree, only the first would be called.
In this reworked case, every check for handling a metric is checked
against the generated pmns tree. I believe this should be safe,
because as far as I can tell, the pmns must be setup before the metrics
are created.
This allows the call to pmdaDynamicPMNS to be simplified. No check
against a prefix or cluster is required. I didn't change the function
call so as not to break existing code. After this change, the prefix is
only used to lookup the cluster mask if it is provided. I think the old
cgroup pmda was the only user of the mask. Is this something that is
still work keeping? If not, both the prefix and cluster parameters
could be removed.
To do this, I had to expose an internal function in tree.c. Not sure if
this is appropriate.
Also, it wasn't clear to me if, overall, this would cause any
performance problems.
This change also exposed an inconsistancy I had previously noticed. DSO
pmdas were not required to call pmdaTreeRebuildHash while daemon pmdas
were. Now this call is required for both cases. I assume that whatever
fixup was occurring for the dso case is now not occurring early enough.
I looked through existing pmdas and added the call to linux and gfs2,
though I have no way to test gfs2.
./check -g pmda.linux
./check -g pmda.proc
both Pass. I looked at the sample pmda and I think that it uses a
different dynamic method that should not be affected. I have not fully
understood the new cgroups code yet, but I think that it should not be
affected if I read it properly. Let me know if there would be any issue
there.
Let me know if there are any problems with this approach.
Thanks!
Martins
|