Hi David,
----- Original Message -----
> [...]
> We have 3 distinct classes of dictionaries, each treated differently.
> During PMDA.run(), the PMDA class passes down to cpmda several dictionaries:
>
> (1) self._metric_names (used by cpmda.pmns_refresh)
> (2) self._metric_oneline (used by cpmda.pmid_oneline_refresh)
> (2) self._metric_helptext (used by cpmda.pmid_longtext_refresh)
> (2) self._indom_oneline (used by cpmda.indom_oneline_refresh)
> (2) self._indom_helptext (used by cpmda.indom_longtext_refresh)
> (3) self._indomtable (used by cpmda.pmda_dispatch)
> (3) self._metrictable (used by cpmda.pmda_dispatch)
>
> Dictionaries marked by (1) are passed down, used once when needed, then
> discarded.
>
> Dictionaries marked by (2) are passed down and kept, used when needed,
> never discarded.
>
> Dictionaries marked by (3) aren't passed down and kept. Instead the PMDA
> class creates a string buffer for each dictionary and passes that down
> to cpmda. This string buffer is used once and then discarded.
>
> I was thinking all dictionaries were treated as being in class (1) (used
> once then discarded). That's why I was passing down everything again in
> PMDA.__refresh_metrics(). Since I don't need to pass down items again in
> class (2), that simplifies PMDA.__refresh_metrics() to:
>
> ====
> def __refresh_metrics(self):
> # Call the user's function.
> if self.__refresh_metrics_func:
> self.__refresh_metrics_func()
>
> if cpmda.get_need_refresh():
> self.pmns_refresh()
> cpmda.pmda_rehash(self._indomtable, self._metrictable)
> ====
Yeah, that's much neater - nice!
> We actually could go further here, if we can change the cpmda interface.
> I'm not sure how set in stone it is. I'm not talking about a change to
> the PMDA class interface here, just the cpmda interface. If we change
> cpmda.pmda_dispatch() to take the indomtable and metrictable
> dictionaries directly (instead of using string buffers), we could keep
> them around for use later by pmda_rehash() - basically moving them to
> class (2).
That would be neat indeed. The cpmda interface is very "internal" - the
python API (which sits atop it) used by PMDAs is really the interface we
need to preserve. So, I think it'd be OK to make a change like you are
describing here - I can't see that ever affecting anyone. It may still
be possible to preserve back-compat using the varargs/keyword interfaces
- we could support two calling conventions for this API, one with the
dictionaries, the other with the string buffers. In fact, it might even
be enough to do a type-check on the passed-in arguments inside the cpmda
code, and switch behaviour based on what we get given.
But, that may be going too far for back-compat since I really can't see
how this change would affecting anyone, in reality, and it may just end
up with us keeping dead code around. Maybe write the code and you could
then make a judgement call based on how good/bad it gets when preserving
the existing interface/semantics?
> If we go that far, then all dictionaries are in class (2) (passed down
> and kept) - except for _metric_names in class (1). If we decide to move
> it to class (2), then all dictionaries are treated the same. At that
> point, we don't need a wrapper around the user's refresh_metrics()
> function in the PMDA class at all, since the cpmda already has all the
> information it needs (pointers to all the dictionaries) and knows when
> to do the refreshing.
Sounds good to me.
> > The only other thing I wanted to revisit a bit is this subtlety...
> >
> >>> 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
>
> I'm afraid you'll have decide on/help me figure out pcp subtleties...
>
> Right now the user's refresh_metrics callback function gets called down
> in cpmda at similar places as the mmv pmda calls mmv_reload_maybe().
> I'll need help in figuring out if that is causing unnecessary
> duplication. Assuming the user's refresh_metrics callback is smart
> enough to not change anything when it doesn't need to, then cpmda
> shouldn't really refresh anything either.
Yep, that'll be fine & is optimal as you have it, I believe.
> > Ah, yes, right you are - I was thinking PyBUF_SIMPLE was removed in 3.x
> > but on closer inspection it is still there & yep, looks like dead code.
> > I do vaguely remember some py3 porting issues around buffers - but this
> > looks unrelated.
>
> We should probably just delete the whole '#else' code to avoid confusing
> others in the future.
Yes please.
cheers.
--
Nathan
|