[collectd] patch for email stats from postfix and amavisd-new, dns stats from powerdns

[Florian Forster]

Hi Luke,

On Mon, Dec 03, 2007 at 11:15:15AM -0800, Luke Heberling wrote:
> The mru cache is both a hash and a linked list.  It needs to be
> quickly accessible by key, and the least recently accessed item should
> also be quickly accessible.

why do you need to access the most recent element? The caches, as they
are implemented in some plugins, search the whole AVL tree for ``old''
elements every now or then and removes them. I guess you want to do
something similar?

> I'm rather embarrassed to say that hadn't even occurred to me. :-(

Don't be embarrassed - it's always hard to work with foreign code..

> > I like that idea. The utils_tail code could start a new thread which
> > calls a callback with each new line read and the line reading, ...
> >
> Perhaps the thread creation could be an option, because in some cases 
> such as mine, it's not really necessary.

It makes it much easier though: The thread could leave the file
descriptor open and call select(2) on it and check if the inode has
changed every few seconds. On Linux `inotify' could be used for an
intelligent alternative to polling. If someone feels up to it, maybe
some FAM (file activity/alteration monitor) support could be added at
some point, too.

> The amavis/postfix plugins just need to catch up before each reading,
> they don't need to be kept up to date continually.

I think keeping them up to date may actually be easier than handling the
``file was moved'' case each time it's called.. And using select,
inotify or FAM this shouldn't be much overhead.

> That just adds [...] the need to synchronize access to the variables
> which hold the counters et al.  

That's just a lock/unlock in the callback and the same in the read
function. I don't think that's a problem.

> There's also error handling. If the thread created by the tail utility
> module is unable to open or read the file, can it properly handle
> errors? Does the callback function serve this purpose? These issues
> might be avoided if the tail utility module does not have to create
> its own thread.

I think handling the state over multiple calls is more confusing that
error handling within a thread with the only purpose of reading one
file.. Basically it's:
-- 8< --
 wait_for = interval_g;
 while ((fd = open (file, "r")) < 0)
 {
   ERROR ("open (%s) failed.", file);
   wait_for = (wait_for < 43200) ? (wait_for * 2) : 86400;
   sleep (wait_for);
 }
-- >8 --

Attached is a proposed interface. Any comments are welcome of course :)

Regards,
-octo
-- 
Florian octo Forster
Hacker in training
GnuPG: 0x91523C3D
http://verplant.org/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: utils_tail.h
Type: text/x-chdr
Size: 2289 bytes
Desc: not available
Url : http://mailman.verplant.org/pipermail/collectd/attachments/20071208/b0dc7ea4/attachment.h 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://mailman.verplant.org/pipermail/collectd/attachments/20071208/b0dc7ea4/attachment.pgp