libpcp_import: support for an archive-append mode #1610
Conversation
In order to implement sysstat/sar semantics, where an archive is appended to by short-lived processes - and not by a long-running process ala pmlogger - we're in need of an append-mode from the LOGIMPORT(3) API. This is a prototype of the sorts of changes I believe we'll need. I've updated just the one QA test utility to exercise it at this stage and the rest will follow once we've settled on appropriate APIs.
Refactor some new log-append code out of libpcp_import and into libpcp; also ensure archive files are opened for read only when we're not appending.
|
@natoscott Basically LGTM. My only questions (that I can't resolve from the diffs) are:
|
|
I see some of my issues above have been addressed in the first commit (I was only looking at the second commit) ... proper review will be done shortly. |
|
|
||
| ctx3 = pmiStart("myarchive", PMI_FLAG_APPEND); | ||
| check(ctx3, "pmiStart"); | ||
| sts = pmiAddMetric("my.metric.bar", pmID_build(245, 0, 2), PM_TYPE_U64, pmInDom_build(245,1), PM_SEM_INSTANT, pmiUnits(1,-1,0,PM_SPACE_MBYTE,PM_TIME_SEC,0)); |
There was a problem hiding this comment.
Why is this pmiAddMetric() needed? my.metric.bar should have been already defined ... there is no change to qa/369 not qa/369.out so I don't know what the expected QA change is here.
Similarly the pmiAddInstance() calls below are suspect ... the first 3 should error because these instances are already defined.
Or am I misunderstanding the semantics ... by "append" are you only dealing with the libpcp part, and libpcp_import is stateless (in the metadata sense) even when "append" is in play? If the latter, then I think there is a real risk of problems because the libpcp_import view of the metadata and the libpcp view of the metadata and the archive's view of the metadata are not aligned.
There was a problem hiding this comment.
The model I'm catering a need for is the sysstat/sadc scenario, where we don't (want to) know whether we are appending or creating - whichever case it is, it should Just Work and do the Right Thing, transparently and correctly. IOW, the application using libpcp_import doesn't want to know about this subtelty, and the correct behavior should just magically happen.
If the archive doesn't exist create it and populate all metric metadata. If it does exist, check the metric metadata still matches (in-memory vs on-disk - fail if not) and append to the existing archive.
Yep, QA definitely needs to be fleshed out further. Maybe in some cases V3 archives could be easier for deltas - we do now know when appending and needing to add instance metadata. Certainly this could be improved on later anyway, I think in the default case for sysstat we wont be logging fast-changing indoms. |
In order to implement sysstat/sar semantics, where an
archive is appended to by short-lived processes - and
not by a long-running process ala pmlogger - we're in
need of an append-mode from the LOGIMPORT(3) API.
This is a prototype of the sorts of changes I believe
we'll need. I've updated just the one QA test utility
to exercise it at this stage and the rest will follow
once we've settled on appropriate APIs.