Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libpcp_import: support for an archive-append mode #1610

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

libpcp_import: support for an archive-append mode #1610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9db5042
libpcp_import: support for an archive-append mode
natoscott Jun 15, 2022
44cff82
libpcp: add __pmLogAppend so log internals remain in libpcp
natoscott Jun 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions man/man3/pmistart.3
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@
.br
#include <pcp/import.h>
.sp
int pmiStart(const char *\fIarchive\fP, int \fIinherit\fP);
int pmiStart(const char *\fIarchive\fP, int \fIflags\fP);
.sp
cc ... \-lpcp_import \-lpcp
.ft 1
.SH "Perl SYNOPSIS"
.ft 3
use PCP::LogImport;
.sp
pmiStart($\fIarchive\fP, $\fIinherit\fP);
pmiStart($\fIarchive\fP, $\fIflags\fP);
.ft 1
.SH DESCRIPTION
As part of the Performance Co-Pilot Log Import API (see
Expand Down Expand Up @@ -81,8 +81,19 @@ or
.BR pmPutValuehandle (3).
.PP
If
.I inherit
is true, then the new context will inherit any and all
.I flags
contains PMI_FLAG_APPEND, then the new context will be created so as to
append metadata (metrics, instance domains, instances, labels), temporal
index entries and values to the specified
.IR archive ,
if it exists.
If the
.I archive
does not exist then the behaviour is the same as without this flag.
.PP
If
.I flags
contains PMI_FLAG_INHERIT, then the new context will inherit any and all
metadata (metrics, instance domains, instances and handles) from the current
context, otherwise the new context is created with no metadata.
The basename for the output PCP archive, the source hostname, the
Expand All @@ -93,7 +104,7 @@ If this is the first call to
.B pmiStart
the metadata will be empty
independent of the value of
.IR inherit .
.IR flags .
.PP
Since no physical files for the output PCP archive
will be created until the first call to
Expand Down
136 changes: 93 additions & 43 deletions qa/src/check_import.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,52 +19,12 @@ check(int sts, char *name)
}
}

int
main(int argc, char **argv)
void
test_basics(int Vflag, int version)
{
int sts;
int ctx1;
int ctx2;
int hdl1;
int hdl2;
int errflag = 0;
int c;
int Vflag = 0;
int version;
static char *usage = "[-D debugspec] [-V version]";

pmSetProgname(argv[0]);

while ((c = getopt(argc, argv, "D:V:")) != EOF) {
switch (c) {

case 'D': /* debug options */
sts = pmSetDebug(optarg);
if (sts < 0) {
fprintf(stderr, "%s: unrecognized debug options specification (%s)\n",
pmGetProgname(), optarg);
errflag++;
}
break;

case 'V': /* output archive version */
version = atoi(optarg);
Vflag++;
break;

case '?':
default:
errflag++;
break;
}
}

if (errflag) {
printf("Usage: %s %s\n", pmGetProgname(), usage);
exit(1);
}

pmiDump();

ctx1 = pmiStart("myarchive", 0);
check(ctx1, "pmiStart");
Expand Down Expand Up @@ -335,8 +295,16 @@ main(int argc, char **argv)

sts = pmiEnd();
check(sts, "pmiEnd");
}

void
test_inherit(void)
{
int sts;
int ctx2;
int hdl2;

ctx2 = pmiStart("myotherarchive", 1);
ctx2 = pmiStart("myotherarchive", PMI_FLAG_INHERIT);
check(ctx2, "pmiStart");
sts = pmiAddInstance(pmInDom_build(245,1), "other", 2);
check(sts, "pmiAddInstance");
Expand All @@ -349,5 +317,87 @@ main(int argc, char **argv)

pmiDump();

sts = pmiEnd();
check(sts, "pmiEnd");
}

void
test_append(void)
{
int sts;
int ctx3;
int hdl3;

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));
Copy link
Member

@kmcdonell kmcdonell Jun 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

check(sts, "pmiAddMetric");
sts = pmiAddInstance(pmInDom_build(245,1), "eek really", 1);
check(sts, "pmiAddInstance");
sts = pmiAddInstance(pmInDom_build(245,1), "eek", 2);
check(sts, "pmiAddInstance");
sts = pmiAddInstance(pmInDom_build(245,1), "blah", 3);
check(sts, "pmiAddInstance");
sts = pmiAddInstance(pmInDom_build(245,1), "append", 4);
check(sts, "pmiAddInstance");
hdl3 = pmiGetHandle("my.metric.bar", "append");
check(hdl3, "pmiGetHandle");
sts = pmiPutValueHandle(hdl3, "654321098765432");
check(sts, "pmiPutValueHandle");

sts = pmiWrite(-1, -1);
check(sts, "pmiWrite");

pmiDump();

sts = pmiEnd();
check(sts, "pmiEnd");
}

int
main(int argc, char **argv)
{
int sts;
int errflag = 0;
int c;
int Vflag = 0;
int version = 0;
static char *usage = "[-D debugspec] [-V version]";

pmSetProgname(argv[0]);

while ((c = getopt(argc, argv, "D:V:")) != EOF) {
switch (c) {

case 'D': /* debug options */
sts = pmSetDebug(optarg);
if (sts < 0) {
fprintf(stderr, "%s: unrecognized debug options specification (%s)\n",
pmGetProgname(), optarg);
errflag++;
}
break;

case 'V': /* output archive version */
version = atoi(optarg);
Vflag++;
break;

case '?':
default:
errflag++;
break;
}
}

if (errflag) {
printf("Usage: %s %s\n", pmGetProgname(), usage);
exit(1);
}

pmiDump();
test_basics(Vflag, version);
test_inherit();
test_append();
exit(0);
}
3 changes: 3 additions & 0 deletions src/include/pcp/import.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ extern "C" {
# endif
#endif

#define PMI_FLAG_INHERIT 0x1 /* pmiStart inherits an existing context */
#define PMI_FLAG_APPEND 0x2 /* pmiStart appends to existing archive */

/* core libpcp_import API routines */
PMI_CALL extern int pmiStart(const char *, int);
PMI_CALL extern int pmiUseContext(int);
Expand Down
2 changes: 2 additions & 0 deletions src/include/pcp/libpcp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,7 @@ typedef struct {
/* state values */
#define PM_LOG_STATE_NEW 0
#define PM_LOG_STATE_INIT 1
#define PM_LOG_STATE_APPEND 2

PCP_CALL extern int __pmEncodeResult(const __pmLogCtl *, const __pmResult *, __pmPDU **);

Expand Down Expand Up @@ -925,6 +926,7 @@ typedef struct {
PCP_CALL extern int __pmLogVersion(const __pmLogCtl *);
PCP_CALL extern size_t __pmLogLabelSize(const __pmLogCtl *);
PCP_CALL extern int __pmLogChkLabel(__pmArchCtl *, __pmFILE *, __pmLogLabel *, int);
PCP_CALL extern int __pmLogAppend(const char *, __pmArchCtl *);
PCP_CALL extern int __pmLogCreate(const char *, const char *, int, __pmArchCtl *);
PCP_CALL extern __pmFILE *__pmLogNewFile(const char *, int);
PCP_CALL extern void __pmLogClose(__pmArchCtl *);
Expand Down
2 changes: 2 additions & 0 deletions src/libpcp/src/exports.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,7 @@ PCP_3.37 {
__pmGetTimestamp;
__pmLogWriteMark;
__pmLogFeaturesStr;
__pmLogLoadIndex;
__pmLogLoadLabelSet;
__pmLogPutResult3;
__pmLogSearchInDom;
Expand All @@ -842,4 +843,5 @@ PCP_3.37 {
__pmDupLogInDom;
__pmFreeLogInDom;
__pmLogEncodeLabels;
__pmLogAppend;
} PCP_3.36;
37 changes: 33 additions & 4 deletions src/libpcp/src/logutil.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,13 @@ _logpeek(__pmArchCtl *acp, int vol)
int sts;
__pmFILE *f;
__pmLogLabel label = {0};
const char *mode = (lcp->state == PM_LOG_STATE_APPEND)? "r+" : "r";
char fname[MAXPATHLEN];

pmsprintf(fname, sizeof(fname), "%s.%d", lcp->name, vol);
/* need mutual exclusion here to avoid race with a concurrent uncompress */
PM_LOCK(logutil_lock);
if ((f = __pmFopen(fname, "r")) == NULL) {
if ((f = __pmFopen(fname, mode)) == NULL) {
PM_UNLOCK(logutil_lock);
return f;
}
Expand All @@ -183,6 +184,7 @@ int
__pmLogChangeVol(__pmArchCtl *acp, int vol)
{
__pmLogCtl *lcp = acp->ac_log;
const char *mode = (lcp->state == PM_LOG_STATE_APPEND)? "r+" : "r";
char fname[MAXPATHLEN];
int sts;

Expand All @@ -196,7 +198,7 @@ __pmLogChangeVol(__pmArchCtl *acp, int vol)
pmsprintf(fname, sizeof(fname), "%s.%d", lcp->name, vol);
/* need mutual exclusion here to avoid race with a concurrent uncompress */
PM_LOCK(logutil_lock);
if ((acp->ac_mfp = __pmFopen(fname, "r")) == NULL) {
if ((acp->ac_mfp = __pmFopen(fname, mode)) == NULL) {
PM_UNLOCK(logutil_lock);
return -oserror();
}
Expand Down Expand Up @@ -286,6 +288,32 @@ __pmLogNewFile(const char *base, int vol)
return f;
}

int
__pmLogAppend(const char *base, __pmArchCtl *acp)
{
__pmLogCtl *lcp = acp->ac_log;
int sts;

if (__pmLogFindOpen(acp, base) == 0) {
lcp->state = PM_LOG_STATE_APPEND;
acp->ac_curvol = -1;
if ((sts = __pmLogChangeVol(acp, lcp->maxvol)) < 0)
return sts;
if ((sts = __pmLogLoadMeta(acp)) < 0)
return sts;
if ((sts = __pmLogLoadIndex(lcp)) < 0)
return sts;
if (acp->ac_mfp)
__pmFseek(acp->ac_mfp, 0, SEEK_END);
if (lcp->mdfp)
__pmFseek(lcp->mdfp, 0, SEEK_END);
if (lcp->tifp)
__pmFseek(lcp->tifp, 0, SEEK_END);
return 0;
}
return -ENOENT;
}

int
__pmLogCreate(const char *host, const char *base, int log_version,
__pmArchCtl *acp)
Expand Down Expand Up @@ -629,6 +657,7 @@ int
__pmLogFindOpen(__pmArchCtl *acp, const char *name)
{
__pmLogCtl *lcp = acp->ac_log;
const char *mode = (lcp->state == PM_LOG_STATE_APPEND)? "r+" : "r";
int sts;
int blen;
int exists = 0;
Expand Down Expand Up @@ -717,14 +746,14 @@ __pmLogFindOpen(__pmArchCtl *acp, const char *name)
tp = &direntp->d_name[blen+1];
if (strcmp(tp, "index") == 0) {
exists = 1;
if ((lcp->tifp = __pmFopen(filename, "r")) == NULL) {
if ((lcp->tifp = __pmFopen(filename, mode)) == NULL) {
sts = -oserror();
goto cleanup;
}
}
else if (strcmp(tp, "meta") == 0) {
exists = 1;
if ((lcp->mdfp = __pmFopen(filename, "r")) == NULL) {
if ((lcp->mdfp = __pmFopen(filename, mode)) == NULL) {
sts = -oserror();
goto cleanup;
}
Expand Down
Loading