Hi AurÃlien,
----- Original Message -----
> The indom are all created. I'm sure I must be doing something wrong.
>
OK - I think your code is good, but you are tripping a subtle reference
counting issue in the C/Perl wrapper code. If you change the code that
creates local %smb1 and %smb2 hashes to instead use globals, it works -
looks like nobody created indom hashes with non-global scope before!
Anyway, with attached patch things behave as you're after, I believe,
even with the current perl PMDA wrapper.
If you prefer to keep the locals, you'll want the following patch which
addresses the root cause:
diff --git a/src/perl/PMDA/PMDA.xs b/src/perl/PMDA/PMDA.xs
index 3884926..472ca46 100644
--- a/src/perl/PMDA/PMDA.xs
+++ b/src/perl/PMDA/PMDA.xs
@@ -498,7 +498,7 @@ update_hash_indom(SV *insts, pmInDom indom)
hv_iterinit(ihash);
while ((data = hv_iternextsv(ihash, &instance, &instsize)) != NULL)
- pmdaCacheStore(indom, PMDA_CACHE_ADD, instance, data);
+ pmdaCacheStore(indom, PMDA_CACHE_ADD, instance, SvREFCNT_inc(data));
sts = pmdaCacheOp(indom, PMDA_CACHE_SAVE);
if (sts < 0)
Also, if you issue add_indom with a different order to the sequential
indom numbering, IIRC you need to subsequently use the returned value
(its used as an array index) for indom operations like 'replace' later.
So, I tend to just initialise in the right order and not worry about it
(you'll see in pmdasimple though that it assigns back to $now_indom).
cheers.
ps: one initial bit of early feedback on your updates - I'd recommend
a metric naming convention along the lines of the disk.dev metrics (see
"pminfo disk.dev"). The num_* naming is a bit redundant - just use the
operation names directly, like disk.dev.read (disk read operations), so
cifs.smb1.read instead of num_read. And "byte_read" -> "read_bytes" is
closer to names used elsewhere (like disk.dev.read_bytes).
--
Nathan
cifs-workaround.patch
Description: Text Data
|