pcp icon indicating copy to clipboard operation
pcp copied to clipboard

libpcp_import: support for an archive-append mode

Open natoscott opened this issue 3 years ago • 3 comments

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.

natoscott avatar Jun 15 '22 05:06 natoscott

@natoscott Basically LGTM.

My only questions (that I can't resolve from the diffs) are:

  1. Is the state of the auxiliary data (and in particular the hashed indom data) is the same in the open for append case as it would be in the "just keep writing" case? At a first stab this probably needs QA coverage, especially with a V3 archive, a dynamic indom and delta-indom records in play).
  2. There may need to be a <mark> record added, but the logic for this is complicated (consider what pmlogger has to do when a pmcd connection is re-established, or the tests pmlogextract does on pmcd.seqnum). It is unclear to me if this should be part of the libpcp functionality or something the caller needs to take care of ... even in the sysstat case one needs to decide how kernel counter resets to zero on reboot will be accommodated.

kmcdonell avatar Jun 21 '22 22:06 kmcdonell

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.

kmcdonell avatar Jun 21 '22 22:06 kmcdonell

LGTM. QA will need some work. Q on semantics in comments for qa/src/check_import.c libpcp_import does not (yet) support delta indoms for V3 archives ... this may be now even trickier, or may be we don't do it!

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.

natoscott avatar Jun 28 '22 00:06 natoscott