You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Ian Holsman <ia...@cnet.com> on 2001/08/24 20:48:58 UTC
Re: [proposal] DBM - allow multiple DBM's of differnt types at the
same time. -- V2
Changes since last time:
* NO CHANGE IN APR_DBM.H (so no diffs ;))
* added licence file to dbm_private and put it in
private/apr_dbm_private.h
* GDBM is now there
* patch to apr_dbm.c.patch
* vtables are now static constants. (as per buckets)
* code style-cleanup to make it more apache. (might still be a stray
tab)
This version does NOT do:
* apr_dbm_open(XXX,...)
* apr_dbm_registration
* allow for multiple DBMs.
all it really does at the moment is split the file into 3 parts.
All it requires for existing 'users' of the DBM is a re-link to the new
library.
IF this patch is Ok, then I'll get the multi-dbm's working (which means
changing the configure, and adding the registeration/open(XXX))
XXX_Usednames is not part of the hook, as you don't need a DBM open to
call it. (and changing it to require it would break mod_dav, as it
doesn't seem to have a open DBM when it calls the funciton)
??comments???
Ian
On Wed, 2001-08-22 at 02:54, Greg Stein wrote:
> -1 until you put that GDBM support back in. It isn't right to just drop it.
>
> On Mon, Aug 20, 2001 at 04:50:25PM -0700, Ian Holsman wrote:
> > On Mon, 2001-08-20 at 13:07, Sterling Hughes wrote:
> >...
> > > Not sure I understand what you're saying? There are ways to allow
> > > the registering of module functionality, without modifying the main
> > > package. Just provide a function like:
> > >
> > > apr_dbm_register()
> > >
> > > Which registers a dbm internally, and then have a constant defined
> > > in a seperate file, and all is clear...
> > that would work,
> > but that wouldn't that mean there would be need to some init code
> > somewhere to call the registration function?
>
> Every time we load a module, we call registration functions. That is no big
> deal.
>
> Your solution of apr_dbm_*_open() still requires a recompilation/relink. The
> registration mechanism is completely open-ended.
>
> > > > 2. I may have a dbm which needs extra parameters to open it. (say for
> > > > exampe cache size, or maybe it requires
> > > > a fixed key length defined) I couldn't do this simply
> > > >
> > >
> > > dbm_set_x() and check the flags, once they're all filled out (a
> > > va_args implementation could also be done...)
>
> Not sure what you mean by dbm_set_x() here. You have to open the thing first
> to get an apr_dbm_t. But then you're too late.
>
> The va_arg solution is a semi-reasonable solution, but that kills any chance
> for type checking.
>
> > > Furthermore, the idea of an abstraction layer is too bring all these
> > > down to a lowest common denominator, at the cost of functionality
> > > sometimes; usually functions that take extra arguments, can be
> > > filled in with acceptable defaults. You'll run into the same
> > > problems in that some db's don't support certain features, and
> > > others do, the idea is to concentrate on the functionality that is
> > > necessary and shared between the different db's (wrap this in a
> > > large imho :).
>
> Exactly. apr_dbm exposes DBM-style functionality. Simple key/value pairs.
> That is it. The open functionality does not need flexibility. It has not
> been demonstrated yet that we need more capability there.
>
> > true, thats why they all return the same thing (apr_dbm_t)
> > the open_DBM function was supposed to be called when the devloper wanted
> > to do something specific with this type of dbm. so in the berkley DB
> > case there could be a apr_dbm_db_getRaw() function which returns the DB*
> > so that developers are not constrained the set of functions we chose to
> > abstract.
>
> No... we already have patterns for getting at underlying data types. See
> apr_portable.h. Follow that pattern *IF* we are to expose those. IMO, we
> should not do so.
>
> > (BTW I was planning on implmenting a shared-memory hash table, that I
> > posted ages ago, via this DBM interface, and it required a different set
> > of opening parameters (key/elem size, #elements) these parameters could
> > be passed via a db_set function call but it would look clunky
>
> No... keys and values are arbitrary length, as specified by an apr_datum_t.
> If your hash cannot handle that, then it cannot be used in the APR DBM
> interface.
>
> >...
> > so..
> > shall we put it to a vote?
> >
> > apr_dbm_open_XXX
> > or
> > apr_dbm_open(XXX)
>
> Registration and the second form.
>
>
> But your patch still has lots of work anyways. The GDBM functionality can't
> just disappear. Sorry, but the DAV functionality uses apr_dbm and you'd
> completely break access to the property files.
>
> Second, the vtable should not be part of the runtime apr_dbm_t structure.
> You should have a pointer to a static const table, like we do with buckets.
>
> The formatting may have gone thru "indent" but it still sucks. Just a glance
> over it shows some random gunk. For example, what the heck is up with the
> formatting of apr_datum_t in apr_dbm.h? And all the spacing of the function
> params in apr_dbm.h has been changed. There are extra blank lines at the end
> of some functions (after returns), and some missing blank lines between
> functions.
>
> We should also see a patch against apr_dbm.h, rather than the whole file. I
> can't see if you've changed the public interface or not.
>
> dbm_private.h is missing a license and disclaimer. It also has a
> non-standard #ifndef/#define/#endif pattern around it. We never start the
> symbol with "_", and on all the new .h files you specified: toss the "1"
> from the #define ... we don't do that either.
>
> Why is the "used names" functionality part of the per-db public interface?
> That should be part of the hook stuff just like the rest.
>
> In any case... the per-db headers should go, in favor of the registration
> mechanism. You would register each DB style with a name. You can then open a
> DB style by name, or using the reserved "any". Note that we would define
> some standard names:
>
> #define APR_DBM_USE_SDBM "sdbm"
> #define APR_DBM_USE_GDBM "gdbm"
> #define APR_DBM_USE_DB "db"
> #define APR_DBM_USE_ANY "any"
>
> Also, the whole point of this exercise was to link in all of the variations.
> That means the .c files should not have the #if APU_USE_*/#endif stuff
> around them. The APU_USE_* symbols are only used to determine which DB to
> use for the "any" case.
>
>
> So... -1 for now. The patch still needs work before it can be applied.
>
> Cheers,
> -g
>
--
Ian Holsman IanH@cnet.com
Performance Measurement & Analysis
CNET Networks - (415) 364-8608
Re: [proposal] DBM - allow multiple DBM's of differnt types at
the same time. -- V2
Posted by Ian Holsman <ia...@cnet.com>.
On Sun, 2001-08-26 at 01:52, Greg Stein wrote:
> On Sat, Aug 25, 2001 at 06:27:23PM -0700, Ian Holsman wrote:
> > does anyone have any objections to this?
> > all it is really doing is removing the MACRO definitions
> > and splitting up the apr_dbm into 4 files (a interface, and 3 implementations)
> > the interface has not changed at all.
>
> I'd still like to review it first, but haven't had the chance yet.
>
I think I'll backport the berkeleyDB patches to the exisitng code and
get that going in the mean time.
the code as is doesn't work on berkeley.
> -g
>
--
Ian Holsman IanH@cnet.com
Performance Measurement & Analysis
CNET Networks - (415) 364-8608
Re: [proposal] DBM - allow multiple DBM's of differnt types at the same time. -- V2
Posted by Greg Stein <gs...@lyra.org>.
On Sat, Aug 25, 2001 at 06:27:23PM -0700, Ian Holsman wrote:
> does anyone have any objections to this?
> all it is really doing is removing the MACRO definitions
> and splitting up the apr_dbm into 4 files (a interface, and 3 implementations)
> the interface has not changed at all.
I'd still like to review it first, but haven't had the chance yet.
-g
--
Greg Stein, http://www.lyra.org/
Re: [proposal] DBM - allow multiple DBM's of differnt types at the same time. -- V2
Posted by Ian Holsman <ia...@cnet.com>.
does anyone have any objections to this?
all it is really doing is removing the MACRO definitions
and splitting up the apr_dbm into 4 files (a interface, and 3 implementations)
the interface has not changed at all.
The next part (registering/multiple DBMs) can be done after this stuff goes through.
..Ian
Ian Holsman wrote:
> Changes since last time:
>
> * NO CHANGE IN APR_DBM.H (so no diffs ;))
> * added licence file to dbm_private and put it in
> private/apr_dbm_private.h
> * GDBM is now there
> * patch to apr_dbm.c.patch
> * vtables are now static constants. (as per buckets)
> * code style-cleanup to make it more apache. (might still be a stray
> tab)
>
> This version does NOT do:
> * apr_dbm_open(XXX,...)
> * apr_dbm_registration
> * allow for multiple DBMs.
>
> all it really does at the moment is split the file into 3 parts.
>
> All it requires for existing 'users' of the DBM is a re-link to the new
> library.
>
> IF this patch is Ok, then I'll get the multi-dbm's working (which means
> changing the configure, and adding the registeration/open(XXX))
>
> XXX_Usednames is not part of the hook, as you don't need a DBM open to
> call it. (and changing it to require it would break mod_dav, as it
> doesn't seem to have a open DBM when it calls the funciton)
>
> ??comments???
> Ian
>
>
Re: [proposal] DBM - allow multiple DBM's of differnt types at thesame time. -- V2
Posted by "William A. Rowe, Jr." <wr...@covalent.net>.
It seems Cliff, Jeff and John Sterling have been kicking around the mod_auth_db
issue again, perhaps time to resurrect this patch and conversation?
Bill
----- Original Message -----
From: "Ian Holsman" <ia...@cnet.com>
To: "Greg Stein" <gs...@lyra.org>
Cc: <de...@apr.apache.org>
Sent: Friday, August 24, 2001 12:48 PM
Subject: Re: [proposal] DBM - allow multiple DBM's of differnt types at thesame time. -- V2
> Changes since last time:
>
> * NO CHANGE IN APR_DBM.H (so no diffs ;))
> * added licence file to dbm_private and put it in
> private/apr_dbm_private.h
> * GDBM is now there
> * patch to apr_dbm.c.patch
> * vtables are now static constants. (as per buckets)
> * code style-cleanup to make it more apache. (might still be a stray
> tab)
>
> This version does NOT do:
> * apr_dbm_open(XXX,...)
> * apr_dbm_registration
> * allow for multiple DBMs.
>
> all it really does at the moment is split the file into 3 parts.
>
> All it requires for existing 'users' of the DBM is a re-link to the new
> library.
>
> IF this patch is Ok, then I'll get the multi-dbm's working (which means
> changing the configure, and adding the registeration/open(XXX))
>
> XXX_Usednames is not part of the hook, as you don't need a DBM open to
> call it. (and changing it to require it would break mod_dav, as it
> doesn't seem to have a open DBM when it calls the funciton)
>
> ??comments???
> Ian
>
>
>
>
> On Wed, 2001-08-22 at 02:54, Greg Stein wrote:
> > -1 until you put that GDBM support back in. It isn't right to just drop it.
> >
> > On Mon, Aug 20, 2001 at 04:50:25PM -0700, Ian Holsman wrote:
> > > On Mon, 2001-08-20 at 13:07, Sterling Hughes wrote:
> > >...
> > > > Not sure I understand what you're saying? There are ways to allow
> > > > the registering of module functionality, without modifying the main
> > > > package. Just provide a function like:
> > > >
> > > > apr_dbm_register()
> > > >
> > > > Which registers a dbm internally, and then have a constant defined
> > > > in a seperate file, and all is clear...
> > > that would work,
> > > but that wouldn't that mean there would be need to some init code
> > > somewhere to call the registration function?
> >
> > Every time we load a module, we call registration functions. That is no big
> > deal.
> >
> > Your solution of apr_dbm_*_open() still requires a recompilation/relink. The
> > registration mechanism is completely open-ended.
> >
> > > > > 2. I may have a dbm which needs extra parameters to open it. (say for
> > > > > exampe cache size, or maybe it requires
> > > > > a fixed key length defined) I couldn't do this simply
> > > > >
> > > >
> > > > dbm_set_x() and check the flags, once they're all filled out (a
> > > > va_args implementation could also be done...)
> >
> > Not sure what you mean by dbm_set_x() here. You have to open the thing first
> > to get an apr_dbm_t. But then you're too late.
> >
> > The va_arg solution is a semi-reasonable solution, but that kills any chance
> > for type checking.
> >
> > > > Furthermore, the idea of an abstraction layer is too bring all these
> > > > down to a lowest common denominator, at the cost of functionality
> > > > sometimes; usually functions that take extra arguments, can be
> > > > filled in with acceptable defaults. You'll run into the same
> > > > problems in that some db's don't support certain features, and
> > > > others do, the idea is to concentrate on the functionality that is
> > > > necessary and shared between the different db's (wrap this in a
> > > > large imho :).
> >
> > Exactly. apr_dbm exposes DBM-style functionality. Simple key/value pairs.
> > That is it. The open functionality does not need flexibility. It has not
> > been demonstrated yet that we need more capability there.
> >
> > > true, thats why they all return the same thing (apr_dbm_t)
> > > the open_DBM function was supposed to be called when the devloper wanted
> > > to do something specific with this type of dbm. so in the berkley DB
> > > case there could be a apr_dbm_db_getRaw() function which returns the DB*
> > > so that developers are not constrained the set of functions we chose to
> > > abstract.
> >
> > No... we already have patterns for getting at underlying data types. See
> > apr_portable.h. Follow that pattern *IF* we are to expose those. IMO, we
> > should not do so.
> >
> > > (BTW I was planning on implmenting a shared-memory hash table, that I
> > > posted ages ago, via this DBM interface, and it required a different set
> > > of opening parameters (key/elem size, #elements) these parameters could
> > > be passed via a db_set function call but it would look clunky
> >
> > No... keys and values are arbitrary length, as specified by an apr_datum_t.
> > If your hash cannot handle that, then it cannot be used in the APR DBM
> > interface.
> >
> > >...
> > > so..
> > > shall we put it to a vote?
> > >
> > > apr_dbm_open_XXX
> > > or
> > > apr_dbm_open(XXX)
> >
> > Registration and the second form.
> >
> >
> > But your patch still has lots of work anyways. The GDBM functionality can't
> > just disappear. Sorry, but the DAV functionality uses apr_dbm and you'd
> > completely break access to the property files.
> >
> > Second, the vtable should not be part of the runtime apr_dbm_t structure.
> > You should have a pointer to a static const table, like we do with buckets.
> >
> > The formatting may have gone thru "indent" but it still sucks. Just a glance
> > over it shows some random gunk. For example, what the heck is up with the
> > formatting of apr_datum_t in apr_dbm.h? And all the spacing of the function
> > params in apr_dbm.h has been changed. There are extra blank lines at the end
> > of some functions (after returns), and some missing blank lines between
> > functions.
> >
> > We should also see a patch against apr_dbm.h, rather than the whole file. I
> > can't see if you've changed the public interface or not.
> >
> > dbm_private.h is missing a license and disclaimer. It also has a
> > non-standard #ifndef/#define/#endif pattern around it. We never start the
> > symbol with "_", and on all the new .h files you specified: toss the "1"
> > from the #define ... we don't do that either.
> >
> > Why is the "used names" functionality part of the per-db public interface?
> > That should be part of the hook stuff just like the rest.
> >
> > In any case... the per-db headers should go, in favor of the registration
> > mechanism. You would register each DB style with a name. You can then open a
> > DB style by name, or using the reserved "any". Note that we would define
> > some standard names:
> >
> > #define APR_DBM_USE_SDBM "sdbm"
> > #define APR_DBM_USE_GDBM "gdbm"
> > #define APR_DBM_USE_DB "db"
> > #define APR_DBM_USE_ANY "any"
> >
> > Also, the whole point of this exercise was to link in all of the variations.
> > That means the .c files should not have the #if APU_USE_*/#endif stuff
> > around them. The APU_USE_* symbols are only used to determine which DB to
> > use for the "any" case.
> >
> >
> > So... -1 for now. The patch still needs work before it can be applied.
> >
> > Cheers,
> > -g
> >
> --
> Ian Holsman IanH@cnet.com
> Performance Measurement & Analysis
> CNET Networks - (415) 364-8608
>