From: "Greg Stein" <[EMAIL PROTECTED]>
Sent: Monday, April 30, 2001 8:49 PM
> -1. Please back out.
I'd rather push forward and address your other concerns...
> You cannot lock the file only during write operations. See sdbm_private.h --
> copies of the file's data sits in memory. If User B goes and works with the
> file, then the cached block for User A becomes invalid. Now, if User A goes
> and does some work against the invalid block and writes that back out, then
> you've got corruption.
I'm well aware of this. That's why I posited the question to the list about
the cache invalidation semantic. The other option, of course, is to drop cache
entirely in a shared open.
Please note we continue to lock the file for it's lifetime iff the user doesn't
pass the new APR_SHARELOCK flag. See the snippets I grabbed from the commit
log below.
> I am mildly against the concept, in general, for SDBM (since people can turn
> to other DBM implementations), but if you come up with a patch that solves
> the issues, then we could certainly discuss it.
I'm not proposing we provide the most optimized solution in the world. Only a
straightforward functional build. The mechansims I'm building on only affect
shared opens, not traditional opens we do today. We are _cross_ platforms,
while
there are many dbm implementations, this simplifies a baseline. I _hope_ others
can be implemented with the APR_SHARELOCK flag, very simply and with better
performance.
> The previous changes have looked good since there was no real functional or
> semantic change. This one isn't going to work, unfortunately.
Again, it shouldn't change if SDBM_SHARED isn't toggled. If it has, that's my
bad.
> A client app could open/close the SDBM to keep the contention duration
> shorter. Another alternative (ugly) would be to flush the cache after all
> write operations (thus losing its entire benefit).
What benefit? If you want exclusive access, open APR_WRITE without the share
flag,
as you probably do now. And nothing changed
> There are some other issues with this patch, but it's kind of moot...
Bring them on. Again, I prefer to work forward, since I mentioned in my other
comment, the auth_dbm, auth_digest, and soon ssl will need at least a bare-bones
cross-process dbm.
I'd prefer to dump caching entirely for APR_SHARELOCK access, rather than revert
to the prior code, but if we can find a better way I'd like that. Dumping
caching
can't be worse than closing up and reopening an sdbm on every request.
Bill
> On Mon, Apr 30, 2001 at 07:03:50PM -0000, [EMAIL PROTECTED] wrote:
> > wrowe 01/04/30 12:03:49
> >
> > Modified: include apr_file_io.h
> > . CHANGES
> > dbm/sdbm sdbm.c
> > Log:
> > shared sdbm file locking patch, writes/deletes require an excl lock,
> > read/getkeys require a shared lock
>
close...
> - (void) sdbm_unlock(db);
> + if (!(db->flags & SDBM_SHARED))
> + (void) sdbm_unlock(db);
open...
> + /*
> + * adjust user flags so that SHARELOCK isn't used within
> + * APR (if it's ever implemented) since we will perform
> + * that locking here.
> + */
> + if (flags & APR_SHARELOCK) {
> + flags &= ~APR_SHARELOCK;
> + db->flags |= SDBM_SHARED;
> + }
> /*
> + * if we are opened in SHARED mode, unlock ourself
> + */
> + if (db->flags & SDBM_SHARED)
> + if ((status = sdbm_unlock(db)) != APR_SUCCESS)
> + goto error;
op begin
> + if (db->flags & SDBM_SHARED)
> + if ((status = sdbm_lock(db, 0)) != APR_SUCCESS)
> + return ioerr(db, status);
op end
> + if (db->flags & SDBM_SHARED)
> + if ((status = sdbm_unlock(db)) != APR_SUCCESS)
> + goto error;
