On Tue, May 01, 2001 at 05:35:47PM -0500, William A. Rowe, Jr. wrote: > From: "Greg Stein" <[EMAIL PROTECTED]> > Sent: Tuesday, May 01, 2001 12:45 AM > > > On Mon, Apr 30, 2001 at 09:50:25PM -0500, William A. Rowe, Jr. wrote: > > > 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... > > > > Sorry. In this case, that would not be prudent. Please back out. > > > > The situation is that the SDBM is ten *years* old. It doesn't have bugs. It > > simply is not something that I worry about. These kinds of changes are > > destabilizing, in an area where I think we can't afford it. > > 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. The small tweaks that I made should not interfere with backing out the lock/unlock. [ just do a "cvs diff -r1.N -r1.N-1" to generate a reverse patch, then apply to head, then check in ] 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). >... > 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. > * 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]." > * be sure we aren't using buffered writes on either the pagf or dirf Yah. That would suck. :-) > This is certainly sub-optimal, but still significantly faster than closing and > opening the two files for every request. Absolutely! >... > 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. >... > > 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. My point was that SDBM currently arbitrates across processes today. It just doesn't have the granularity you are seeking. Cheers, -g -- Greg Stein, http://www.lyra.org/
