[rrd-developers] [PATCH] generalisation of rrd_open API

Bernhard Reutner-Fischer rep.dot.nop at gmail.com
Fri Oct 17 17:52:59 CEST 2008


On Fri, Oct 17, 2008 at 04:03:04PM +0100, Daniel.Pocock at barclayscapital.com wrote:
> 
>> Daniel.Pocock at barclayscapital.com wrote:
>> > The madvise stuff has come out, but it will go back in.  
>> There are a 
>> > couple of options:
>> > - pass the header size (which is already known) to 
>> > rrd_file->vfs->open_impl, and call madvise once for the whole region
>> > - add an madvise function to rrd_vfs_t
>> 
>> Taking it out, even temporarily, doesn't seem like that good 
>> of an idea to me.  You're targeting large installations, 
>> which need madvise/fadvise very much.
>> 
>
>That's why I said (further down) that this is not ready to commit - I
>would welcome your suggestions on how to invoke madvise within this
>generalised API.

You should use C99 initializers for the vfs struct.

This one:
+ * rrd_vfs.h     RRD virtual filesystems
+ *****************************************************************************/
+
+#ifndef _RRD_FS_H

s/filesystems/filesystem/
s/_RRD_FS_H/_RRD_VFS_H/
for consistency.

that "*pvt" is completely undocumented. What does it mean? pointer to
VFS type? nah.


We should first get a clear idea about a proper (internal) API imo.
My little testcase was here, fwiw:
http://www.mail-archive.com/rrd-developers@lists.oetiker.ch/msg01866.html
This is a bit outdated by now since it was against some 1.2.xx, but you
get the idea. Simple program that does _not_ have to fallback to copy
huge chunks of rrdtool itself.

Testcase that even does something useful (i.e. read some values out of
an rrd) was posted here, fwiw:
http://www.mail-archive.com/rrd-users@lists.oetiker.ch/msg14102.html
but any other testcase would do, too.

As you can see in such a simple example, the current 1.3.x doesn't even
allow for the minimum client to use librrd for anything useful since the
cf_conv() is not exported anymore, let alone a decent API to fetch
something.

I suggest to clean this up since this will also help supporting an
additional, new fileformat, however that may look like.



More information about the rrd-developers mailing list