2012/3/13 Rainer Gerhards <[email protected]>: >> -----Original Message----- >> From: [email protected] [mailto:rsyslog- >> [email protected]] On Behalf Of Kaiwang Chen >> Sent: Tuesday, March 13, 2012 2:31 PM >> To: rsyslog-users >> Subject: Re: [rsyslog] lots of queue files left in working directory >> >> 2012/3/13 Rainer Gerhards <[email protected]>: >> >> So I came to the conclusion that 16cc84 was not my case. In >> addition, >> >> I suppose the enforcement in 8d2f66 is not a good way to fix the >> >> problem about uninitialized counters. Callers of AddCounter() are >> >> supposed to call, if necessary, STATSCOUNTER_INIT precedingly to do >> >> initialization protected by mutex. The problems in new versions are >> >> that plugins/imptcp/imptcp.c, plugins/imudp/imudp.c and tcpsrv.c >> miss >> >> the call of STATSCOUNTER_INIT, while action.c, >> >> plugins/imuxsock/imuxsock.c, >> >> plugins/omelasticsearch/omelasticsearch.c, runtime/queue.c are OK. >> The >> >> enforcement coexisting with STATSCOUNTER_INIT is confusing, and not >> >> documented as well. >> > >> > Thanks again for the analysis. I guess the interface was not well >> done in the >> > first place, as obviously even I misused it after a couple of weeks. >> I think >> > I will change it so that AddCounter() will receive an additional >> parameter >> > which tells if init is required or not. That should prevent further >> > confusion. Any comments on that? >> >> STATSCOUNTER_INIT takes a mutex as well, will you consider that? Then >> it requires two additional parameters. > > Thanks again - I am right now at the exact same question and have done some > review of why I created it the way it was. The mutex is only present if > atomic operations are not used. The macros take care of that and that was > probably the root reason to use them. As this is a quite efficient method, I > have shifted my thoughts to keep the macros. On the other hand, this probably > means things like the iQueueSize do not have the macros associated, and their > use depends on some non-obvious things. Which is bad. On the other hand, it > is also bad to carry around mutexes that are never used.
If I read it correctly, a statsobj is used to get a snapshot, represented as a string like "key=value ...", of the counters registered by AddCounter(). The status string is fetched by getStatsLine(). In this respect, AddCounter() should be a read-only function. Should I found that the statsobj is useless, it could be unplugged simply by comment out AddCounter() instead of twisting additional lines to guarantee initialization. So in my opinion, the side effect of AddCounter() is a bad idea and should be avoided. Then we fall back to STATSCOUNTER_INIT() explicitly. The statsobj may be extended to retrieve internal variables which are not essentially a dedicated counter, iQueueSize be an example? The counter intialization even could be put together with, say, queue initialization code, so that no need of mutex is required. How do you think about it? > > Not sure if better doc inside the code could help with that. What do you > mean? Just add comment to addCounter exclaiming it is read only and has nothing to do with counter or internal status initialization. > > Rainer > _______________________________________________ > rsyslog mailing list > http://lists.adiscon.net/mailman/listinfo/rsyslog > http://www.rsyslog.com/professional-services/ Thanks, Kaiwang _______________________________________________ rsyslog mailing list http://lists.adiscon.net/mailman/listinfo/rsyslog http://www.rsyslog.com/professional-services/
