Bug 1109539

"Expected results: persistent mapping (especially important for PCP archive logs)"

Why is that?  What breaks if indom values change across reboots?  Can we make
the pcp archive-gluing client side more resilient?

(In reply to Frank Ch. Eigler from comment #1)
> "Expected results: persistent mapping (especially important for PCP archive
> logs)"
> 
> Why is that?  What breaks if indom values change across reboots?  Can we make
> the pcp archive-gluing client side more resilient?

values in pmResults are indexed with instid (not instance name), so it's never 
good if an instance domain update changes the mapping. Archives are affected 
across reboots due to pmlogmerge and friends. This is why the pmdaCache API 
feature was introduced, adding persistance via opaque keys. See PMDACACHE(3) for details.

I wasn't talking about changing mappings during the lifespan of a pmcd.  But ...

"Archives are affected across reboots due to pmlogmerge and friends."

Doesn't this represent a shortcoming of pmlogmerge?  The individual meta files
contain both instance numbers and names, so pmlogmerge could do some rewriting
as it goes and have the output be self-consistent.

(In reply to Frank Ch. Eigler from comment #3)
> I wasn't talking about changing mappings during the lifespan of a pmcd.  But
> ...
> 
> "Archives are affected across reboots due to pmlogmerge and friends."
> 
> Doesn't this represent a shortcoming of pmlogmerge?  The individual meta
> files
> contain both instance numbers and names, so pmlogmerge could do some
> rewriting
> as it goes and have the output be self-consistent.

yes, pmlogrewrite(1) can re-map instance domains prior to log merge. But that's a heck of a hassle requiring manual intervention compared to having the instance domain persistent in the first place!

"yes, pmlogrewrite(1) can re-map instance domains prior to log merge. But that's a heck of a hassle requiring manual intervention"

OK, but does it need to be a manual hassle?  Could pmlogmerge detect
mismatching indoms, and do the rewriting itself (or subcontract it to
pmlogrewrite jobs)?

"compared to having the instance domain persistent in the first place!"

If the tools were robust enough, this would be a great optimization.

This is not simply an archiving problem (so we cannot just fix it there via post-processing, and think we're done here) - it affects all the tools using live mode as well.  In that mode, the protocol simply does not support instance IDs dynamically changing like this.  Think about what happens internally in pmchart, pmie, pmval, etc in live mode, once they've been asked to fetch a named instance.

These requirements are well documented - in the books and man pages, etc.  The pmdaCacheOp(3) man page has extensive discussion (see paragraph #2 - "The semantics of a PCP instance domain require a number of rules to be followed, namely...").

Marks original statement "need to use the pmdaCache API" is spot on, and is the right way to tackle this.

> Think about what happens internally in pmchart, pmie, pmval, etc
> in live mode, once they've been asked to fetch a named instance.

In live mode, during a persistent connection to pmcd, this should
not happen though, should it?


> These requirements are well documented - in the books and man pages, etc. 
> The pmdaCacheOp(3) man page has extensive discussion (see paragraph #2 -
> "The semantics of a PCP instance domain require a number of rules to be
> followed, namely...").

Bullet 4 there says:

   4. It is preferable, although not mandatory, for the association
   between and external instance name  and an  internal instance
   identifier to be persistent.  This rule is supported by the
   pmdaCache family of routines.

Note "preferable".
 

> Marks original statement "need to use the pmdaCache API" is spot on, and is
> the right way to tackle this.

Yes, it would certainly solve this problem today.  I was thinking ahead to
other problems such as imperfect PMDAs, lost caches, mismatched archives, etc.

(In reply to Frank Ch. Eigler from comment #7)
> In live mode, during a persistent connection to pmcd, this should
> not happen though, should it?

It should not (in general), but in this case, yes it can happen.  Have a look at the code.

>    4. It is preferable, although not mandatory, for the association
>    between and external instance name  and an  internal instance
>    identifier to be persistent.  This rule is supported by the
>    pmdaCache family of routines.
> 
> Note "preferable".

Its "preferable" because it depends on how the clients might use the information, and whether or not its possible, or sufficiently important information to ensure this naming persistence - its at the PMDA authors discretion.  Sometimes (as was the case here), a simple implementation might come first, and later be updated to use pmdaCache interfaces when the need for persistence is more widely understood, and more experience gained with the metric(s).

HTH.

(In reply to Nathan Scott from comment #8)
> (In reply to Frank Ch. Eigler from comment #7)
> > In live mode, during a persistent connection to pmcd, this should
> > not happen though, should it?
> 
> It should not (in general), but in this case, yes it can happen.  Have a
> look at the code.

You're right.  If I understand it correctly, it's because in src/pmdas/linux
devmapper.c, refresh* computes a whole new instance domain table every time
it's deemed out-of-date.

An ugly little shell pipeline implicates a few other linux indoms that might
be similarly affected:

% cd src/pmdas/linux
 grep 'case.*_INDOM' *.c | uniq | awk '{print $3}' | cut -f1 -d: | while read dom
do; echo -n $dom; dom2=`echo $dom | cut -f1 -d_`
grep -iq "pmdaCache.*"$dom *.c && echo -n " cached" || echo -n " uncached"
grep -iq "refresh.*"$dom2 *.c && echo -n " refreshed" || echo -n " unrefreshed"; echo
done | grep uncached.refreshed
CPU_INDOM uncached refreshed
LOADAVG_INDOM uncached refreshed
NFS_INDOM uncached refreshed
SCSI_INDOM uncached refreshed
LV_INDOM uncached refreshed
SLAB_INDOM uncached refreshed

but that includes things like SCSI, which through its refresh_proc_scsi()
function maintains some invariance by only appending to its indom list,
not recreating it from scratch.  So that list includes false positives.

I haven't fixed this directly because the functionality of mapping to the internal non-persistent dm-[0-9]* name from the persistent dm name is now provided by hinv.map.dmname (though in the reverse direction to the mapping provided by hinv.map.lvname, i.e. the persistent dm name is the instance domain and the non-persistent dm-[0-9]* name is the value). This mapping by hinv.map.dmname is available for all dm devices, not just lvm devices.

So can we consider deprecating hinv.map.lvname in favour of hinv.map.dmname? And updating any tools that use the former to use the latter? I think that probably only affects pmatop though I don't know for sure.

Regards
-- Mark

> So can we consider deprecating hinv.map.lvname in favour of hinv.map.dmname?

Yep, thats my understanding of how we would proceed from the earlier
discussions - leaving the existing metric and indom as-is (un-fixable
in this way, hence we went with a new metric/indom pairing).

> And updating any tools that use the former to use the latter?

*nod*

> I think that probably only affects pmatop though I don't know for sure.

Pretty sure thats the case, yes.  Stan originally added this metric in for the
python tools he wrote - so pmcollectl and pmatop are the only possible users AFAICT.

cheers.

upstream patch has been posted for the 3.10.5-1 release

commit ab4eb4adbfec797f51becf3298f5b78f350dcaed
Author: Mark Goodwin <mgoodwin>
Date:   Fri Jun 5 14:23:37 2015 +1000

    Deprecate hinv.nlv and hinv.map.lvname and the LV_NAME instance domain.
    
    The instance domain for hinv.map.lvname is the dm names e.g. dm-1,
    which are not persistent. The values are the logical names, which
    are persistent. These two metrics are now deprecated in favor of
    hinv.map.dmname, which instead uses the persistent names for the
    instance domain, and the dm names as the value of the mapping.
    
    Note also - hinv.map.dmname is a superset of the lvm instances -
    the dmname instance domain also includes non-lvm devices such as
    dm-multipath, dm-cache, etc.  as well as all lvm logical devices.
    
    The only known user of the deprecated hinv.map.lvname metric is the
    old pmatop command, which itself has been deprecated.
    
        modified:   src/pmdas/linux/GNUmakefile
        modified:   src/pmdas/linux/clusters.h
        deleted:    src/pmdas/linux/devmapper.c
        deleted:    src/pmdas/linux/devmapper.h
        modified:   src/pmdas/linux/help
        modified:   src/pmdas/linux/indom.h
        modified:   src/pmdas/linux/pmda.c
        modified:   src/pmdas/linux/root_linux

Hello Nathan,
Could you please verify if the fix for this bug made it into the PCP build for RHEL 7.2? If so, please add the bug to errata.
Thanks!
Martha

Yes, this was included in the rebase, updating the Errata now.

Verified for build pcp-3.10.6-2.el7.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHBA-2015-2096.html