Hi Martins,
----- Original Message -----
> Nathan,
>
> > Have a look at the src/pmdas/simple/pmdasimple.perl code - this PMDA has
> > both styles. The $color_indom uses the array style and $now_indom uses a
> > hash (under the covers, in PCP::PMDA, the latter translates to pmdaCache
> > use).
> >
> OK. I think I got this working with a hash. I can't figure out how to
> get confirmation that it actually is using pmdaCache though.
A dirty hack^W^Wneat trick would be to attach to the pmdanfsclient perl
process with gdb, set a breakpoint in pmdaCacheLookup then verify that
it is triggered.
Or look inside the src/perl/PMDA/PMDA.xs update_indom code, and convince
yourself that your add_indom call with a hash will always end up calling
update_hash_indom.
>From inspection, your code is correct - don't worry about the above. :)
> I think I addressed most of the issues raised previously, but wanted to
> get some feedback on a couple of items before finishing up.
*nod* - looking good, nice to see alot of the TODO items vanishing.
> 1. Types should now match kernel types, let me know if you think I
> missed any.
Looks fine. This line...
open STATS, "< /proc/self/mountstats"
I would recommend parameterizing the path so you can pass in all sorts
of odd mountstats files (for testing), without requiring the running
kernel to be preparing the file for you. Something like,
open STATS, '<', MOUNTSTATS_PATH or die...
(die() is a fairly drastic measure to take BTW - that could be made
more robust perhaps)
> 2. I dropped the tcp identifier under xprt, and just created a union of
> all the possible types across tcp, udp and rdma. I have only tested
> tcp. I hope to test udp shortly. I don't think I'll be able to test
> rdma in the near future as we don't use it in production currently.
No problem. If you know the format, or if someone else could help us
out by sending through an example mountstats file, the above testing
trick could be used to test this even without hardware support.
A simpler example (than qa/553 I suggested earlier) of an existing test
that does this is qa/972 - see the use of $ZSWAP_STATS_PATH in there,
along with its use of dbpmda.
> 3. Used mountpoints in the instance domain. I think this is the best we
> can do. Its what mountstats itself is using to partition the data and
Sounds good - its what people will expect & matches up with how they'll
want to use the data, I think.
> 4. There didn't seem to be any consensus on nfsclient.ops.getattr.count
> vs. nfsclient.opts.count.getattr so I left it. Let me know if it should
> be changed.
What was the issue there again? ops usually refers to "operations"
and opts to "options" (e.g. mount options) - from that metric name
above, this sounds like an "ops" kind of metric.
> 5. My biggest question is what to do about the nfs3, nfs4 vs nfs4.1
> per-op statistics. See the attached pdf, one column per version,
> matching types aligned. There are 22, 47 and 53 stats for v3, v4 and
> v4.1 respectively. Should it be treated the same as the tcp/udp/rdma
> case and just zero out the ones that dont apply? I haven't implemented
> this yet, but if that's what you think should be done, it should be
> pretty straightforward to do.
If I understand correctly, this is a two-dimensional dataset kind of
problem (operations x mount points), with a twist that the different
nfs versions have different operations. I'd expand the operations
(as listed in the pdf) in the namespace, and keep the instance domain
as the mount points. That will mean different sub-trees for each nfs
version, I think, and yeah - zeroes or PM_ERR_APPVERSION returned in
cases where a mount point isn't using a particular nfs version - that
works just fine from the client tool POV.
> 6. Finally, how much of the opts, caps, and sec fields should be parsed
> out? I grabbed a couple for now that are obviously useful, but can do
> them all if that's the way to go. Again, similar issue to the previous
> point with respect to options that pertain to nfs versions.
If its alot of data, I'd stick with information that is useful (it all
needs to be classified in terms of metrics metadata) - if its not too
much, I'd just go through the lot in case someone needs one particular
field someday. Sounds more like the former case here.
cheers.
--
Nathan
|