apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@rowe-clan.net>
Subject Re: cvs commit: apr-util/dbm/sdbm sdbm.c
Date Wed, 02 May 2001 03:48:44 GMT
From: "Greg Stein" <gstein@lyra.org>
Sent: Tuesday, May 01, 2001 9:00 PM

> > Greg, now you have me confused (unless I've missed a post on this subject !?!)
> > 
> > I noted your commits (all look good) and have some additional thoughts below,
> > but the question in my mind is ... do you want me to unroll these patches?
> Yes, please back out the lock/unlock patch. The first three (namespace
> protection, re-indenting, and status code returns) all are fine. It was only
> the portion that introduced the per-operation lock/unlock.

Ok, I was confused, sorry.  Be happy to back out and re-propose the lock/unlock
schema we are developing here, to be committed in a more ordered fashion.

I still think it's rather silly to back out, since these are noops (dependent on
a bit flag we don't implement within Apache at this time), but I'm not about to
argue the point.

> The small tweaks that I made should not interfere with backing out the
> lock/unlock.
> If you have any problem, then I'll do it tonite. I just wanted to be a bit
> more courteous and ask you (personally, it feels more polite to /not/ back
> out somebody's change on them).

No problem but lack of sleep - no committing for me tonight ;->  Be happy to
back it out first thing in the morning, unless you care to jump on it this eve.

> > I also realized that not all Win based systems can be trusted to even update the
> > mtime upon every update.  I only see one answer;
> > 
> >   * expose lock/unlock for multiple operations that take advantage of the cache
> Hmm. This would certainly help an app who is doing multiple operations, but
> if an app forgets, it could lead to corruption. Another possibility is to
> "count" the locks. The caller could:
> - Lock
> - Store
>   - lock
>   - operation
>   - unlock
> - Store again
> - Unlock
> The sub-locks in the Store operation would just incr a count and return
> (thus they'd be fast). If the app makes a mistake in their lock/unlock, or
> simply doesn't care to do it, then each operation is protected.

I like this theory.  I was proposing a lock-bit that would implicitly lock on
any op, but allow the user to explicitly lock (and thus no implicit unlock.)
But if we plan to allow the user to issue multiple locks without penalty, than
fine, a count would be good.  Note that the lock is shared _or_ exclusive (for
write) so that adds additional headaches/semantics.

> >   * flush the cache upon granting every lock
> Hmm. Need to think on it. I imagined doing it on the unlock. We don't want a
> read to say, "oh. that page is in my cache, so I don't have to hit the file
> to read it in [thus avoiding acquiring a lock to do the read]."

That's up to us, of course a read always does an implicit lock, so it would
flush.  It should be transparent either way.

> > Good patches, as I said.  I need to vet this code too, and I'll have more
> > comments in a couple of hours, and add the invalidation (not optimized by
> > mtime stamps) at the same time.
> Okay. I'll look for them tonite.

Well, not if we are folding back out the patch (!?!)  I've got a shared-lock and
the normal apr cvs trees checked out, so I'll create a patch after the backout
if you want that to occur, I'll be careful not to loose my work across unwinding
the patch, and we can pick up the discussion from there.

> >...
> > > sdbm is already cross-process. It arbitrates access to the DBM quite fine.
> > > You're simply trying to make the locking more fine-grained. That's okay, but
> > > not at the expense of trust in this code.
> > 
> > Huh?  By closing and reopening the file (exclusively)?  I don't follow.
> If you create/open an SDBM, then it takes out the appropriater reader/writer
> lock. Granted, you don't want to leave it open longer than your request for
> fear of keeping others out.

If you keep it open for the life of your request (as write) your handler is single 
request at a time.  That's not 'cross-process' in my book, it's uniprocess.
But safe from corruption, yes.

View raw message