From 9db504257ae6101266437a83cde9d1566583c55c Mon Sep 17 00:00:00 2001 From: Nathan Scott Date: Wed, 15 Jun 2022 15:08:10 +1000 Subject: [PATCH 1/2] libpcp_import: support for an archive-append mode 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. --- man/man3/pmistart.3 | 21 +++- qa/src/check_import.c | 136 +++++++++++++++++--------- src/include/pcp/import.h | 3 + src/libpcp/src/exports.in | 1 + src/libpcp/src/logutil.c | 8 +- src/libpcp_import/src/archive.c | 164 ++++++++++++++++++++++++++++---- src/libpcp_import/src/import.c | 47 ++------- src/libpcp_import/src/private.h | 2 + 8 files changed, 272 insertions(+), 110 deletions(-) diff --git a/man/man3/pmistart.3 b/man/man3/pmistart.3 index 0de235da0f..b4a0055a05 100644 --- a/man/man3/pmistart.3 +++ b/man/man3/pmistart.3 @@ -23,7 +23,7 @@ .br #include .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 @@ -31,7 +31,7 @@ cc ... \-lpcp_import \-lpcp .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 @@ -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 @@ -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 diff --git a/qa/src/check_import.c b/qa/src/check_import.c index e152751f27..9b9691177e 100644 --- a/qa/src/check_import.c +++ b/qa/src/check_import.c @@ -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"); @@ -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"); @@ -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)); + 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); } diff --git a/src/include/pcp/import.h b/src/include/pcp/import.h index 5506d1192b..bba0e9feec 100644 --- a/src/include/pcp/import.h +++ b/src/include/pcp/import.h @@ -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); diff --git a/src/libpcp/src/exports.in b/src/libpcp/src/exports.in index 259bd53468..6b5024ad8d 100644 --- a/src/libpcp/src/exports.in +++ b/src/libpcp/src/exports.in @@ -832,6 +832,7 @@ PCP_3.37 { __pmGetTimestamp; __pmLogWriteMark; __pmLogFeaturesStr; + __pmLogLoadIndex; __pmLogLoadLabelSet; __pmLogPutResult3; __pmLogSearchInDom; diff --git a/src/libpcp/src/logutil.c b/src/libpcp/src/logutil.c index a14e2f1031..5ac222f971 100644 --- a/src/libpcp/src/logutil.c +++ b/src/libpcp/src/logutil.c @@ -162,7 +162,7 @@ _logpeek(__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 ((f = __pmFopen(fname, "r")) == NULL) { + if ((f = __pmFopen(fname, "r+")) == NULL) { PM_UNLOCK(logutil_lock); return f; } @@ -196,7 +196,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, "r+")) == NULL) { PM_UNLOCK(logutil_lock); return -oserror(); } @@ -717,14 +717,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, "r+")) == 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, "r+")) == NULL) { sts = -oserror(); goto cleanup; } diff --git a/src/libpcp_import/src/archive.c b/src/libpcp_import/src/archive.c index 4d89b328b0..221f021d39 100644 --- a/src/libpcp_import/src/archive.c +++ b/src/libpcp_import/src/archive.c @@ -42,11 +42,42 @@ check_context_start(pmi_context *current) acp = ¤t->archctl; acp->ac_log = ¤t->logctl; + lcp = ¤t->logctl; + + /* open a possibly-existing-possibly-not archive, create it if not */ + if ((current->flags & PMI_FLAG_APPEND)) { + if (__pmLogFindOpen(acp, current->archive) == 0) { + /* file exists so we're going to use it */ + acp->ac_curvol = -1; + if ((sts = __pmLogChangeVol(acp, lcp->maxvol)) < 0) + return PM_ERR_LOGFILE; + if ((sts = __pmLogLoadMeta(acp)) < 0) + return PM_ERR_LOGFILE; + if ((sts = __pmLogLoadIndex(lcp)) < 0) + return PM_ERR_LOGFILE; + 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); + if (lcp->label.zoneinfo) + pmNewZone(lcp->label.zoneinfo); + else + pmNewZone(lcp->label.timezone); + current->state = CONTEXT_ACTIVE; + lcp->state = PM_LOG_STATE_INIT; + return 1; /* ok */ + } + } + + /* clear append flag as archive must be created */ + current->flags &= ~PMI_FLAG_APPEND; + sts = __pmLogCreate(host, current->archive, current->version, acp); if (sts < 0) return sts; - lcp = ¤t->logctl; if (current->timezone != NULL) { free(lcp->label.timezone); lcp->label.timezone = strdup(current->timezone); @@ -75,18 +106,54 @@ check_context_start(pmi_context *current) return 0; /* ok */ } +static int +compare_instances(__pmArchCtl *acp, pmi_indom *inp) +{ + char **namelist; + int *instlist; + int i, j, n, sts; + + /* when appending, see if this indom is on-disk already */ + sts = n = __pmLogGetInDom(acp, inp->indom, &stamp, &instlist, &namelist); + if (sts < 0) + return sts; + if (n != inp->ninstance) + return -ENOENT; + + for (i = 0; i < inp->ninstance; i++) { + for (j = 0; j < n; j++) { + if (instlist[i] != inp->inst[j]) + continue; + if (strcmp(namelist[i], inp->name[j]) == 0) + break; + } + if (j == n) /* no match found */ + return -ENOENT; + } + + return 0; /* completely matched all indom elements */ +} + static int check_indom(pmi_context *current, pmInDom indom, int *needti) { - int i; - int sts = 0; - __pmArchCtl *acp = ¤t->archctl; - int type = current->version == PM_LOG_VERS03 ? TYPE_INDOM : TYPE_INDOM_V2; + int i; + int sts = 0; + int type; + __pmArchCtl *acp = ¤t->archctl; __pmLogInDom lid; + type = current->version == PM_LOG_VERS03 ? TYPE_INDOM : TYPE_INDOM_V2; + for (i = 0; i < current->nindom; i++) { if (indom == current->indom[i].indom) { if (current->indom[i].meta_done == 0) { + if ((current->flags & PMI_FLAG_APPEND) && + (compare_instances(acp, ¤t->indom[i])) == 0) { + current->indom[i].meta_done = 1; + break; + } + lid.stamp = stamp; lid.indom = current->indom[i].indom; lid.numinst = current->indom[i].ninstance; @@ -95,9 +162,9 @@ check_indom(pmi_context *current, pmInDom indom, int *needti) lid.alloc = 0; if ((sts = __pmLogPutInDom(acp, type, &lid)) < 0) return sts; - current->indom[i].meta_done = 1; *needti = 1; + break; } } } @@ -105,6 +172,20 @@ check_indom(pmi_context *current, pmInDom indom, int *needti) return sts; } +int +compare_descs(pmDesc *a, pmDesc *b) +{ + if (a->type != b->type) + return PM_ERR_LOGCHANGETYPE; + if (a->indom != b->indom) + return PM_ERR_LOGCHANGEINDOM; + if (a->sem != b->sem) + return PM_ERR_LOGCHANGESEM; + if (memcmp(&a->units, &b->units, sizeof(pmUnits)) != 0) + return PM_ERR_LOGCHANGEUNITS; + return 0; +} + static int check_metric(pmi_context *current, pmID pmid, int *needti) { @@ -117,12 +198,25 @@ check_metric(pmi_context *current, pmID pmid, int *needti) continue; if (current->metric[m].meta_done == 0) { char **namelist = ¤t->metric[m].name; + pmDesc chk, *desc = ¤t->metric[m].desc; + int desc_done = 0; + + if ((current->flags & PMI_FLAG_APPEND)) { + if (__pmLogLookupDesc(acp, pmid, &chk) == 0) { + if ((sts = compare_descs(desc, &chk)) < 0) + return sts; + desc_done = 1; + } + } - if ((sts = __pmLogPutDesc(acp, ¤t->metric[m].desc, 1, namelist)) < 0) + if (desc_done) + current->metric[m].meta_done = 1; + else if ((sts = __pmLogPutDesc(acp, desc, 1, namelist)) < 0) return sts; - - current->metric[m].meta_done = 1; - *needti = 1; + else { + current->metric[m].meta_done = 1; + *needti = 1; + } } if (current->metric[m].desc.indom != PM_INDOM_NULL) { if ((sts = check_indom(current, current->metric[m].desc.indom, needti)) < 0) @@ -134,7 +228,7 @@ check_metric(pmi_context *current, pmID pmid, int *needti) return sts; } -static void +static int newvolume(pmi_context *current) { __pmFILE *newfp; @@ -143,8 +237,7 @@ newvolume(pmi_context *current) int nextvol = acp->ac_curvol + 1; if ((newfp = __pmLogNewFile(current->archive, nextvol)) == NULL) { - fprintf(stderr, "logimport: Error: volume %d: %s\n", nextvol, pmErrStr(-oserror())); - return; + return PM_ERR_LOGFILE; } if (pmDebugOptions.log) @@ -155,6 +248,7 @@ newvolume(pmi_context *current) lcp->label.vol = acp->ac_curvol = nextvol; __pmLogWriteLabel(acp->ac_mfp, &lcp->label); __pmFflush(acp->ac_mfp); + return 0; } static off_t flushsize = 100000; @@ -187,7 +281,7 @@ _pmi_put_result(pmi_context *current, __pmResult *result) if (sts < 0) return sts; - old_meta_offset = __pmFtell(lcp->mdfp);; + old_meta_offset = __pmFtell(lcp->mdfp); __pmOverrideLastFd(__pmFileno(acp->ac_mfp)); sts = __pmEncodeResult(acp->ac_log, result, &pb); @@ -214,7 +308,11 @@ _pmi_put_result(pmi_context *current, __pmResult *result) off = __pmFtell(acp->ac_mfp) + ((__pmPDUHdr *)pb)->len - sizeof(__pmPDUHdr) + 2*sizeof(int); if (off >= max_logsz) { - newvolume(current); + sts = newvolume(current); + if (sts < 0) { + __pmUnpinPDUBuf(pb); + return sts; + } flushsize = 100000; needti = 1; } @@ -244,14 +342,30 @@ _pmi_put_result(pmi_context *current, __pmResult *result) return 0; } +int +_pmi_put_mark(pmi_context *current, __pmTimestamp *last_stamp) +{ + __pmArchCtl *acp = ¤t->archctl; + __pmTimestamp msec = { 0, 1000000 }; /* 1msec */ + int sts; + + /* One time processing for the start of the context. */ + sts = check_context_start(current); + if (sts < 0) + return sts; + + return __pmLogWriteMark(acp, last_stamp, &msec); +} + int _pmi_put_text(pmi_context *current) { - int sts; __pmArchCtl *acp = ¤t->archctl; pmi_text *tp; - int t; + char *bp; int needti; + int sts; + int t; /* last_stamp has been set by the caller. */ stamp = current->last_stamp; @@ -269,6 +383,10 @@ _pmi_put_text(pmi_context *current) if (tp->meta_done) continue; /* Already written */ + if ((current->flags & PMI_FLAG_APPEND) && + (__pmLogLookupText(acp, tp->id, tp->type, &bp)) == 0) + continue; /* Previously written to archive */ + if ((tp->type & PM_TEXT_PMID)) { /* * This text is for a metric. Make sure that the metric desc @@ -291,10 +409,9 @@ _pmi_put_text(pmi_context *current) /* * Now write out the text record. * libpcp, via __pmLogPutText(), makes a copy of the storage pointed - * to by buffer. + * to by buffer. (final 1 parameter below == 'cached') */ - if ((sts = __pmLogPutText(¤t->archctl, tp->id, tp->type, - tp->content, 1/*cached*/)) < 0) + if ((sts = __pmLogPutText(acp, tp->id, tp->type, tp->content, 1)) < 0) return sts; tp->meta_done = 1; @@ -311,6 +428,7 @@ _pmi_put_label(pmi_context *current) { int sts; __pmArchCtl *acp = ¤t->archctl; + pmLabelSet *lsp; pmi_label *lp; int l; int needti; @@ -329,6 +447,10 @@ _pmi_put_label(pmi_context *current) for (l = 0; l < current->nlabel; l++) { lp = ¤t->label[l]; + if ((current->flags & PMI_FLAG_APPEND) && + (__pmLogLookupLabel(acp, lp->type, lp->id, &lsp, NULL)) > 0) + continue; /* Previously written to archive */ + if (lp->type == PM_LABEL_ITEM) { /* * This label is for a metric. Make sure that the metric desc @@ -353,7 +475,7 @@ _pmi_put_label(pmi_context *current) * libpcp, via __pmLogPutLabels(), assumes control of the * storage pointed to by lp->labelset. */ - if ((sts = __pmLogPutLabels(¤t->archctl, lp->type, lp->id, + if ((sts = __pmLogPutLabels(acp, lp->type, lp->id, 1, lp->labelset, &stamp)) < 0) return sts; diff --git a/src/libpcp_import/src/import.c b/src/libpcp_import/src/import.c index e339f1b0ef..952a64ae4a 100644 --- a/src/libpcp_import/src/import.c +++ b/src/libpcp_import/src/import.c @@ -1,12 +1,12 @@ /* * Copyright (c) 2013-2022 Red Hat. * Copyright (c) 2010 Ken McDonell. All Rights Reserved. - * + * * This library is free software; you can redistribute it and/or modify it * under the terms of the GNU Lesser General Public License as published * by the Free Software Foundation; either version 2.1 of the License, or * (at your option) any later version. - * + * * This library is distributed in the hope that it will be useful, but * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public @@ -299,7 +299,7 @@ pmiErrStr_r(int code, char *buf, int buflen) } int -pmiStart(const char *archive, int inherit) +pmiStart(const char *archive, int flags) { pmi_context *old_current; char *np; @@ -319,19 +319,15 @@ pmiStart(const char *archive, int inherit) old_current = &context_tab[c]; current = &context_tab[ncontext-1]; + memset(current, 0, sizeof(pmi_context)); current->state = CONTEXT_START; current->version = archive_version; current->archive = strdup(archive); if (current->archive == NULL) { pmNoMem("pmiStart", strlen(archive)+1, PM_FATAL_ERR); } - current->hostname = NULL; - current->timezone = NULL; - current->result = NULL; - memset((void *)¤t->logctl, 0, sizeof(current->logctl)); - memset((void *)¤t->archctl, 0, sizeof(current->archctl)); current->archctl.ac_log = ¤t->logctl; - if (inherit && old_current != NULL) { + if ((flags & PMI_FLAG_INHERIT) != 0 && old_current != NULL) { current->nmetric = old_current->nmetric; if (old_current->metric != NULL) { int m; @@ -346,8 +342,6 @@ pmiStart(const char *archive, int inherit) current->metric[m].meta_done = 0; } } - else - current->metric = NULL; current->nindom = old_current->nindom; if (old_current->indom != NULL) { int i; @@ -390,8 +384,6 @@ pmiStart(const char *archive, int inherit) } } } - else - current->indom = NULL; current->nhandle = old_current->nhandle; if (old_current->handle != NULL) { int h; @@ -404,8 +396,6 @@ pmiStart(const char *archive, int inherit) current->handle[h].inst = old_current->handle[h].inst; } } - else - current->handle = NULL; current->ntext = old_current->ntext; if (old_current->text != NULL) { int t; @@ -423,8 +413,6 @@ pmiStart(const char *archive, int inherit) current->text[t].meta_done = 0; } } - else - current->text = NULL; current->nlabel = old_current->nlabel; if (old_current->label != NULL) { int t; @@ -446,23 +434,11 @@ pmiStart(const char *archive, int inherit) current->label[t].labelset = NULL; } } - else - current->label = NULL; current->last_stamp = old_current->last_stamp; + current->flags |= PMI_FLAG_INHERIT; } - else { - current->nmetric = 0; - current->metric = NULL; - current->nindom = 0; - current->indom = NULL; - current->nhandle = 0; - current->handle = NULL; - current->ntext = 0; - current->text = NULL; - current->nlabel = 0; - current->label = NULL; - current->last_stamp.sec = 0; - current->last_stamp.nsec = 0; + else if ((flags & PMI_FLAG_APPEND) != 0) { + current->flags |= PMI_FLAG_APPEND; } return ncontext; } @@ -1209,17 +1185,14 @@ int pmiPutMark(void) { __pmTimestamp *last_stamp; - __pmArchCtl *acp; - __pmTimestamp msec = { 0, 1000000 }; /* 1msec */ if (current == NULL) return PM_ERR_NOCONTEXT; - acp = ¤t->archctl; last_stamp = ¤t->last_stamp; - if (last_stamp->sec == 0 && last_stamp->nsec == 0) /* no earlier result, no point adding a mark record */ return 0; - return __pmLogWriteMark(acp, last_stamp, &msec); + + return _pmi_put_mark(current, last_stamp); } diff --git a/src/libpcp_import/src/private.h b/src/libpcp_import/src/private.h index 2456642880..3e5b8b0d97 100644 --- a/src/libpcp_import/src/private.h +++ b/src/libpcp_import/src/private.h @@ -71,6 +71,7 @@ typedef struct { pmi_label *label; int last_sts; __pmTimestamp last_stamp; + unsigned int flags; } pmi_context; #define CONTEXT_START 1 @@ -85,6 +86,7 @@ typedef struct { extern int _pmi_stuff_value(pmi_context *, pmi_handle *, const char *) _PMI_HIDDEN; extern int _pmi_put_result(pmi_context *, __pmResult *) _PMI_HIDDEN; +extern int _pmi_put_mark(pmi_context *, __pmTimestamp *) _PMI_HIDDEN; extern int _pmi_put_text(pmi_context *) _PMI_HIDDEN; extern int _pmi_put_label(pmi_context *) _PMI_HIDDEN; extern int _pmi_end(pmi_context *) _PMI_HIDDEN; From 44cff826ebe1f05f1c4a90033d15d92f50afe90f Mon Sep 17 00:00:00 2001 From: Nathan Scott Date: Wed, 15 Jun 2022 17:14:57 +1000 Subject: [PATCH 2/2] libpcp: add __pmLogAppend so log internals remain in libpcp 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. --- src/include/pcp/libpcp.h | 2 ++ src/libpcp/src/exports.in | 1 + src/libpcp/src/logutil.c | 37 +++++++++++++++++++++++++++++---- src/libpcp_import/src/archive.c | 36 ++++++++++---------------------- 4 files changed, 47 insertions(+), 29 deletions(-) diff --git a/src/include/pcp/libpcp.h b/src/include/pcp/libpcp.h index 57586d12f5..9e5f7a684f 100644 --- a/src/include/pcp/libpcp.h +++ b/src/include/pcp/libpcp.h @@ -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 **); @@ -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 *); diff --git a/src/libpcp/src/exports.in b/src/libpcp/src/exports.in index 6b5024ad8d..4f0302a2ee 100644 --- a/src/libpcp/src/exports.in +++ b/src/libpcp/src/exports.in @@ -843,4 +843,5 @@ PCP_3.37 { __pmDupLogInDom; __pmFreeLogInDom; __pmLogEncodeLabels; + __pmLogAppend; } PCP_3.36; diff --git a/src/libpcp/src/logutil.c b/src/libpcp/src/logutil.c index 5ac222f971..ac2d773436 100644 --- a/src/libpcp/src/logutil.c +++ b/src/libpcp/src/logutil.c @@ -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; } @@ -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; @@ -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(); } @@ -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) @@ -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; @@ -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; } diff --git a/src/libpcp_import/src/archive.c b/src/libpcp_import/src/archive.c index 221f021d39..e541836805 100644 --- a/src/libpcp_import/src/archive.c +++ b/src/libpcp_import/src/archive.c @@ -32,48 +32,34 @@ check_context_start(pmi_context *current) if (current->state != CONTEXT_START) return 0; /* ok */ - if (current->hostname == NULL) { - (void)gethostname(myname, MAXHOSTNAMELEN); - myname[MAXHOSTNAMELEN-1] = '\0'; - host = myname; - } - else - host = current->hostname; - acp = ¤t->archctl; acp->ac_log = ¤t->logctl; lcp = ¤t->logctl; /* open a possibly-existing-possibly-not archive, create it if not */ if ((current->flags & PMI_FLAG_APPEND)) { - if (__pmLogFindOpen(acp, current->archive) == 0) { + if (__pmLogAppend(current->archive, acp) == 0) { /* file exists so we're going to use it */ - acp->ac_curvol = -1; - if ((sts = __pmLogChangeVol(acp, lcp->maxvol)) < 0) - return PM_ERR_LOGFILE; - if ((sts = __pmLogLoadMeta(acp)) < 0) - return PM_ERR_LOGFILE; - if ((sts = __pmLogLoadIndex(lcp)) < 0) - return PM_ERR_LOGFILE; - 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); + current->state = CONTEXT_ACTIVE; if (lcp->label.zoneinfo) pmNewZone(lcp->label.zoneinfo); else pmNewZone(lcp->label.timezone); - current->state = CONTEXT_ACTIVE; - lcp->state = PM_LOG_STATE_INIT; return 1; /* ok */ } } - /* clear append flag as archive must be created */ + /* clear append flag as archive must now be created */ current->flags &= ~PMI_FLAG_APPEND; + if (current->hostname == NULL) { + (void)gethostname(myname, MAXHOSTNAMELEN); + myname[MAXHOSTNAMELEN-1] = '\0'; + host = myname; + } + else + host = current->hostname; + sts = __pmLogCreate(host, current->archive, current->version, acp); if (sts < 0) return sts;