[rrd-developers] rrd cache work
Florian Forster
rrdtool at nospam.verplant.org
Thu Sep 4 12:57:39 CEST 2008
Hi Kevin,
sorry for the late reply - the List-ID and addredd of the rrd-developers
list has changed. Together with the string `rrd-developers' not
appearing in the subject, the mail was filed in my (spam flooded)
INBOX..
On Tue, Sep 02, 2008 at 11:15:14AM -0500, kevin brintnall wrote:
[Make rrdc_flush_if_daemon an internal function]
> We could do it in a few different ways:
>
> * forward declarations in the relevant *.c files
> * internal-only include file
> * change rrd_flush to a no-op if no daemon connection
I'd use the second option. AfaIk `rrd_tool.h' is already such an
internal-only file..
> It doesn't appear that @time is very widely used.. I'd be OK with
> just throwing an error..
I fully agree. Also, that cache code is meant for programs, not people.
And programs like epoch much better than at-style time. It's people that
can't cope ;)
> > Parsing of the `--daemon' argument and connecting to the daemon
> > would then be done by the `rrdtool' binary instead.
>
> I don't think we can move this functionality to the rrdtool program,
> since there are a bunch of other language bindings that need to be
> able to handle --daemon argument also.
I think you're thinking too much in terms of the `rrdtool' utility and
not enough in terms of a real API:
Right now, the `rrdtool' utility and the `librrd' API basically are the
same thing. The `rrdtool' utility strips off the first element from
`argv' and passes the rest to one of the `rrd_*' functions of the
library. This basically makes the library the feel of many programs
bundled together into one shared object.
Don't know of you know `Git', but they do something *very* similar but
in a more obvious way: They have many `git-*' programs or scripts which
are called by a central script called `git', which also strips of the
first argument. So `git clone' actually calls `git-clone',
`git checkout' actually calls `git-checkout' and so on.
Right now, `rrdtool' and `librrd' do the same thing. One could just as
well build several applications and rewrite `rrdtool' to be a simple
script. `rrdtool info' could then call `rrdtool-info', for example.
As far as I know, one of the goals for 1.4 is to imiprove the library to
provide a ``real'' API, something very similar to the existing `*_r'
functions. This means, of course, that some backwards-incompatible
changes have to be made, but it might also be possible to provide a
compatibility layer..
Having each function parse `--daemon' (as it is right now) would equal
passing `const char *daemon_address' to every function that can use it.
Each function would then do:
<Library>
int rrd_foo (..., const char *daemon_address)
{
if (daemon_address != NULL)
{
rrdc_connect (daemon_address);
do_stuff_with_daemon ();
rrdc_disconnect ();
}
return (do_foo ());
} /* int rrd_foo */
</Library>
<UserProgram>
int main ()
{
for (i = 0; i < large_number; i++)
rrd_foo (data[i], daemon_address);
return (0);
} /* int main */
</UserProgram>
Of course, each function connects and disconnects again, which is not
very good. So a much better way would be:
<Library>
int rrd_foo (..., rrd_cache_handle_t *cache_handle)
{
if (cache_handle != NULL)
do_stuff_with_daemon (cache_handle);
return (do_foo ());
} /* int rrd_foo */
</Library>
<UserProgram>
int main ()
{
rrd_cache_handle_t *connection;
connection = rrdc_connect (...);
/* Print warning. Alternative: Exit with error. */
if (connection == NULL)
fprintf (stderr, "Connecting to %s failed.\n"
"Cache will not be used!\n", ...);
for (i = 0; i < large_number; i++)
rrd_foo (data[i], connection);
rrdc_disconnect (connection);
return (0);
} /* int main */
</UserProgram>
With the second schema, the `UserProgram' needs to connect and
disconnect if it wants to use the daemon. Having such a connection
object has big advantages:
- The `UserProgram' controls *if* a cache is to be used. If connecting
fails, it can immediately notice that and act upon it. If passed a
non-NULL connection handle, it's obvious that success can only be
returned if the communication with the daemon was successful and that
local files are *never* touched directly.
- This way, there can be multiple connections open at the same time. For
example, a user program could do:
<LoadBalancing>
int key = 0;
char *ptr;
for (ptr = name; *ptr != 0; ptr++)
key ^= (int) *ptr;
rrd_foo (..., connections[key % connections_num]);
</LoadBalancing>
<Redundancy>
for (i = 0; i < connections_num; i++)
rrd_foo (..., connections[i]);
</Redundancy>
Other languages should provide the exact same interface. So the Perl
interface might look like this:
<Perl>
my $rrdc_connection = RRDs::connect ($addr)
or die ("RRDs::connect ($addr): " . RRDs::error ());
for (my $i = 0; $i < @files; $i++)
{
RRDs::update ($files[$i], ..., $rrdc_connection);
}
RRDs::disconnect ($rrdc_connection);
</Perl>
With such a library as outlined above, the `rrdtool' command line
utility would be ``degraded'' to `just another program using this
library' - as it ought to be. This would mean, of course, that the
`rrdtool' utility would need to call `rrdc_connect' and thus would need
to parse the `--daemon' command line option. In fact, it'd be a command
line frontend for the library - parsing command line options would be
it's main purpose.
> I rather like the idea of introducing this feature without changes to
> the existing API. I'd like to see us continue in the current
> direction. By forcing changes upstream, we make it harder to adopt
> this enhancement.
I think it's time for the old API to die. I see two big drawbacks to
passing `argc' and `argv' to API functions:
- Every function needs to do parse and verify its arguments, although
the `rrdtool' utility is one of the few programs that simply pass some
unchecked array to the functions. Checking the command line options
should be done by the command line program, not the library. Checking
options in a library should be as simple as:
if (file == NULL) ...
or
if (values_num < 1) ...
- All programs that want to use the library have to ``build'' `argv'
first. This is not hard, but it's annoying. The `*_r' functions do a
*much* better job at providing something usable.
The interface as is should imho *only* be used by the `rrdtool' command
line utility. There it makes sense. So if it was up to me, I'd make a
clear cut and leave the argv-API in the past. Imho changing the file
format is a *much* bigger issue than changing the API, because end-users
are effected, not developers..
I know, however, that I'm basically alone with this opinion. So, what do
you think about the following compromise:
- Make all the `*_r' functions the `official' new API.
- Build the `librrd' with only these symbols, i. e. `librrd' contains
`rrd_update_r', `rrd_fetch_r', ... These functions are declared in
<rrd.h>.
- Build a second library, maybe called `librrd_cl' (`cl' as in `command
line') which provides the following functions:
int rrd_update_argv (int argc, char **argv);
int rrd_fetch_argv (int argc, char **argv);
...
These functions are declared in <rrd_cl.h>. They parse the command
line arguments and possibly call the functions in `librrd' with the
parsed arguments.
- The header files look somewhat like this:
<rrd.h>
#ifndef RRD_H
#define RRD_H 1
int rrd_update_r (...);
int rrd_fetch_r (...);
...
#if !RRD_CL_H
# define rrd_update rrd_update_r
# define rrd_fetch rrd_fetch_r
...
#endif /* !RRD_COMPATIBILITY */
#endif /* RRD_H */
</rrd.h>
<rrd_cl.h>
#ifndef RRD_CL_H
#define RRD_CL_H 1
#if !RRD_I_KNOW_ITS_DEPRECATED
# warn "Use of the argv-interface is deprecated! Please update your application!"
#endif
#include <rrd.h>
int rrd_update_argv (int argc, char **argv);
int rrd_fetch_argv (int argc, char **argv);
...
#define rrd_update rrd_update_argv
#define rrd_fetch rrd_fetch_argv
...
#endif /* RRD_CL_H */
</rrd_cl.h>
This way `old' applications need to include <rrd_cl.h> before <rrd.h>
and link against `rrd_cl' (in addition to `rrd') to get the old
interface.
- The `rrdtool' command line utility uses the `rrd_cl', just like `old'
applications.
> I see your point. There are problems with the throttling approach on
> either extreme.
>
> * if the rate is too high, it becomes the same problem that we have now
That's the user's problem of finding a rate his hardware can handle and
still gives him enough headroom for other work. This is actually very
very specific to hardware and user's requirements.
> * if the rate is too low, then client applications may block for a long
> time waiting for flushes, while the hardware is idle (think graph with
> a lot of RRDs).
No, `flush' command don't honor the rate limit. It's a flush, not a case
of ``Would be nice to have on disk again, in case of something goes
wrong. Don't really care when this happens, though. Thanks.''
Sorry for the long mail, hope this clarifies how I'd change the API and
``the way out'' I'd implement for outdated projects.
Regards,
-octo
--
Florian octo Forster
Hacker in training
GnuPG: 0x91523C3D
http://verplant.org/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://lists.oetiker.ch/pipermail/rrd-developers/attachments/20080904/118b985a/attachment.bin
More information about the rrd-developers
mailing list