From 5ecfdad17e395570edb78698b4805992368c61b1 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 12 Mar 2012 16:56:51 +0100 Subject: bumped version number (of temp branch) --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 2e070386..b38a1955 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ # Process this file with autoconf to produce a configure script. AC_PREREQ(2.61) -AC_INIT([rsyslog],[5.8.8-newstats9],[rsyslog@lists.adiscon.com]) +AC_INIT([rsyslog],[5.8.8-newstats10],[rsyslog@lists.adiscon.com]) AM_INIT_AUTOMAKE m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])]) -- cgit From e6aaf19689791c668ea444a21e470e4db3244cb5 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 14 Mar 2012 12:50:21 +0100 Subject: changed statsobj interface and added better doc --- ChangeLog | 3 +++ plugins/imuxsock/imuxsock.c | 5 ++++- runtime/queue.c | 5 ++--- runtime/queue.h | 2 +- runtime/statsobj.c | 36 ++++++++++++++--------------- runtime/statsobj.h | 55 ++++++++++++++++++++++++++++++++------------- 6 files changed, 67 insertions(+), 39 deletions(-) diff --git a/ChangeLog b/ChangeLog index 61ac356c..ef9d3735 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,9 @@ Version 5.8.9 [V5-stable] 2012-03-?? - bugfix: stopped DA queue was never processed after a restart due to a regression from statistics module +- added better doc for statsobj interface + Thanks to Kaiwang Chen for his suggestions and analysis in regard to the + stats subsystem. --------------------------------------------------------------------------- Version 5.8.8 [V5-stable] 2012-03-05 - bugfix: omprog made rsyslog abort on startup if not binary to diff --git a/plugins/imuxsock/imuxsock.c b/plugins/imuxsock/imuxsock.c index 4523798f..5867f1c9 100644 --- a/plugins/imuxsock/imuxsock.c +++ b/plugins/imuxsock/imuxsock.c @@ -784,7 +784,7 @@ CODESTARTwillRun else if(sd_booted()) { struct stat st; if(stat(SYSTEMD_JOURNAL, &st) != -1 && S_ISDIR(st.st_mode)) { - listeners[0].sockName = SYSTEMD_PATH_LOG; + listeners[0].sockName = (uchar*) SYSTEMD_PATH_LOG; } } if(ratelimitIntervalSysSock > 0) { @@ -1010,10 +1010,13 @@ CODEmodInit_QueryRegCFSLineHdlr /* support statistics gathering */ CHKiRet(statsobj.Construct(&modStats)); CHKiRet(statsobj.SetName(modStats, UCHAR_CONSTANT("imuxsock"))); + STATSCOUNTER_INIT(ctrSubmit, mutCtrSubmit); CHKiRet(statsobj.AddCounter(modStats, UCHAR_CONSTANT("submitted"), ctrType_IntCtr, &ctrSubmit)); + STATSCOUNTER_INIT(ctrLostRatelimit, mutCtrLostRatelimit); CHKiRet(statsobj.AddCounter(modStats, UCHAR_CONSTANT("ratelimit.discarded"), ctrType_IntCtr, &ctrLostRatelimit)); + STATSCOUNTER_INIT(ctrNumRatelimiters, mutCtrNumRatelimiters); CHKiRet(statsobj.AddCounter(modStats, UCHAR_CONSTANT("ratelimit.numratelimiters"), ctrType_IntCtr, &ctrNumRatelimiters)); CHKiRet(statsobj.ConstructFinalize(modStats)); diff --git a/runtime/queue.c b/runtime/queue.c index 5b25fcf7..621a4eed 100644 --- a/runtime/queue.c +++ b/runtime/queue.c @@ -1930,10 +1930,9 @@ qqueueStart(qqueue_t *pThis) /* this is the ConstructionFinalizer */ CHKiRet(statsobj.Construct(&pThis->statsobj)); CHKiRet(statsobj.SetName(pThis->statsobj, qName)); /* we need to save the queue size, as the stats module initializes it to 0! */ - iQueueSizeSave = pThis->iQueueSize; + /* iQueueSize is a dual-use counter: no init, no mutex! */ CHKiRet(statsobj.AddCounter(pThis->statsobj, UCHAR_CONSTANT("size"), ctrType_Int, &pThis->iQueueSize)); - pThis->iQueueSize = iQueueSizeSave; STATSCOUNTER_INIT(pThis->ctrEnqueued, pThis->mutCtrEnqueued); CHKiRet(statsobj.AddCounter(pThis->statsobj, UCHAR_CONSTANT("enqueued"), @@ -1943,7 +1942,7 @@ qqueueStart(qqueue_t *pThis) /* this is the ConstructionFinalizer */ CHKiRet(statsobj.AddCounter(pThis->statsobj, UCHAR_CONSTANT("full"), ctrType_IntCtr, &pThis->ctrFull)); - pThis->ctrMaxqsize = 0; + pThis->ctrMaxqsize = 0; /* no mutex needed, thus no init call */ CHKiRet(statsobj.AddCounter(pThis->statsobj, UCHAR_CONSTANT("maxqsize"), ctrType_Int, &pThis->ctrMaxqsize)); diff --git a/runtime/queue.h b/runtime/queue.h index 97057180..7ef5673c 100644 --- a/runtime/queue.h +++ b/runtime/queue.h @@ -169,7 +169,7 @@ struct queue_s { statsobj_t *statsobj; STATSCOUNTER_DEF(ctrEnqueued, mutCtrEnqueued); STATSCOUNTER_DEF(ctrFull, mutCtrFull); - int ctrMaxqsize; + int ctrMaxqsize; /* NOT guarded by a mutex */ }; diff --git a/runtime/statsobj.c b/runtime/statsobj.c index 131605e0..367ddb15 100644 --- a/runtime/statsobj.c +++ b/runtime/statsobj.c @@ -3,25 +3,23 @@ * This object provides a statistics-gathering facility inside rsyslog. This * functionality will be pragmatically implemented and extended. * - * Copyright 2010 Rainer Gerhards and Adiscon GmbH. + * Copyright 2010-2012 Adiscon GmbH. * * This file is part of the rsyslog runtime library. * - * The rsyslog runtime 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 3 of the License, or - * (at your option) any later version. - * - * The rsyslog runtime 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 License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with the rsyslog runtime library. If not, see . - * - * A copy of the GPL can be found in the file "COPYING" in this distribution. - * A copy of the LGPL can be found in the file "COPYING.LESSER" in this distribution. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * -or- + * see COPYING.ASL20 in the source distribution + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. */ #include "config.h" @@ -139,6 +137,10 @@ finalize_it: /* add a counter to an object * ctrName is duplicated, caller must free it if requried + * NOTE: The counter is READ-ONLY and MUST NOT be modified (most + * importantly, it must not be initialized, so the caller must + * ensure the counter is properly initialized before AddCounter() + * is called. */ static rsRetVal addCounter(statsobj_t *pThis, uchar *ctrName, statsCtrType_t ctrType, void *pCtr) @@ -154,11 +156,9 @@ addCounter(statsobj_t *pThis, uchar *ctrName, statsCtrType_t ctrType, void *pCtr switch(ctrType) { case ctrType_IntCtr: ctr->val.pIntCtr = (intctr_t*) pCtr; - *(ctr->val.pIntCtr) = 0; break; case ctrType_Int: ctr->val.pInt = (int*) pCtr; - *(ctr->val.pInt) = 0; break; } addCtrToList(pThis, ctr); diff --git a/runtime/statsobj.h b/runtime/statsobj.h index 44c26bea..90279883 100644 --- a/runtime/statsobj.h +++ b/runtime/statsobj.h @@ -1,24 +1,22 @@ /* The statsobj object. * - * Copyright 2010 Rainer Gerhards and Adiscon GmbH. + * Copyright 2010-2012 Rainer Gerhards and Adiscon GmbH. * * This file is part of the rsyslog runtime library. * - * The rsyslog runtime 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 3 of the License, or - * (at your option) any later version. - * - * The rsyslog runtime 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 License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with the rsyslog runtime library. If not, see . - * - * A copy of the GPL can be found in the file "COPYING" in this distribution. - * A copy of the LGPL can be found in the file "COPYING.LESSER" in this distribution. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * -or- + * see COPYING.ASL20 in the source distribution + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. */ #ifndef INCLUDED_STATSOBJ_H #define INCLUDED_STATSOBJ_H @@ -96,6 +94,31 @@ PROTOTYPEObj(statsobj); * Unfortunately, this does not work if counter is e.g. "pThis->ctr". * So we decided, for clarity, to always insist on specifying the mutex * name (after all, it's just a few more keystrokes...). + * -------------------------------------------------------------------- + * NOTE WELL + * -------------------------------------------------------------------- + * There are actually two types of stats counters: "regular" counters, + * which are only used for stats purposes and "dual" counters, which + * are primarily used for other purposes but can be included in stats + * as well. ALL regular counters MUST be initialized with + * STATSCOUNTER_INIT and only be modified by STATSCOUNTER_* functions. + * They MUST NOT be used for any other purpose (if this seems to make + * sense, consider changing it to a dual counter). + * Dual counters are somewhat dangerous in that a single variable is + * used for two purposes: the actual application need and stats + * counting. However, this is supported for performance reasons, as it + * provides insight into the inner engine workings without need for + * additional counters (and their maintenance code). Dual counters + * MUST NOT be modified by STATSCOUNTER_* functions. Most importantly, + * it is expected that the actua application code provides proper + * (enough) synchronized access to these counters. Most importantly, + * this means they have NO stats-system mutex associated to them. + * + * The interface function AddCounter() is a read-only function. It + * only provides the stats subsystem with a reference to a counter. + * It is irrelevant if the counter is a regular or dual one. For that + * reason, AddCounter() must not modify the counter contents, as in + * the case of a dual counter application code may be broken. */ #define STATSCOUNTER_DEF(ctr, mut) \ intctr_t ctr; \ -- cgit From 7d516b0a6c46177c18aea30b4617f1d2712d48fd Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 14 Mar 2012 13:51:18 +0100 Subject: added missing initialization of stats counters this was necessary due to refactoring of the stats subsystem ... but should have been done in the first place when this code was written. Thanks to Kaiwang Chen for his analysis of the stats subsystem, which ultimately lead to this patch! --- plugins/imptcp/imptcp.c | 1 + plugins/imudp/imudp.c | 1 + tcpsrv.c | 1 + 3 files changed, 3 insertions(+) diff --git a/plugins/imptcp/imptcp.c b/plugins/imptcp/imptcp.c index e191c64f..9c6c64e4 100644 --- a/plugins/imptcp/imptcp.c +++ b/plugins/imptcp/imptcp.c @@ -775,6 +775,7 @@ addLstn(ptcpsrv_t *pSrv, int sock, int isIPv6) isIPv6 ? "IPv6" : "IPv4"); statname[sizeof(statname)-1] = '\0'; /* just to be on the save side... */ CHKiRet(statsobj.SetName(pLstn->stats, statname)); + STATSCOUNTER_INIT(pLstn->ctrSubmit, pLstn->mutCtrSubmit); CHKiRet(statsobj.AddCounter(pLstn->stats, UCHAR_CONSTANT("submitted"), ctrType_IntCtr, &(pLstn->ctrSubmit))); CHKiRet(statsobj.ConstructFinalize(pLstn->stats)); diff --git a/plugins/imudp/imudp.c b/plugins/imudp/imudp.c index 112738b4..46631e97 100644 --- a/plugins/imudp/imudp.c +++ b/plugins/imudp/imudp.c @@ -234,6 +234,7 @@ static rsRetVal addListner(void __attribute__((unused)) *pVal, uchar *pNewVal) snprintf((char*)statname, sizeof(statname), "imudp(%s:%s)", bindName, port); statname[sizeof(statname)-1] = '\0'; /* just to be on the save side... */ CHKiRet(statsobj.SetName(newlcnfinfo->stats, statname)); + STATSCOUNTER_INIT(newlcnfinfo->ctrSubmit, newlcnfinfo->mutCtrSubmit); CHKiRet(statsobj.AddCounter(newlcnfinfo->stats, UCHAR_CONSTANT("submitted"), ctrType_IntCtr, &(newlcnfinfo->ctrSubmit))); CHKiRet(statsobj.ConstructFinalize(newlcnfinfo->stats)); diff --git a/tcpsrv.c b/tcpsrv.c index 86175ff4..dcf407c0 100644 --- a/tcpsrv.c +++ b/tcpsrv.c @@ -128,6 +128,7 @@ addNewLstnPort(tcpsrv_t *pThis, uchar *pszPort) snprintf((char*)statname, sizeof(statname), "%s(%s)", pThis->pszInputName, pszPort); statname[sizeof(statname)-1] = '\0'; /* just to be on the save side... */ CHKiRet(statsobj.SetName(pEntry->stats, statname)); + STATSCOUNTER_INIT(pEntry->ctrSubmit, pEntry->mutCtrSubmit); CHKiRet(statsobj.AddCounter(pEntry->stats, UCHAR_CONSTANT("submitted"), ctrType_IntCtr, &(pEntry->ctrSubmit))); CHKiRet(statsobj.ConstructFinalize(pEntry->stats)); -- cgit