Daniel John Debrunner <[EMAIL PROTECTED]> writes: > Knut Anders Hatlen (JIRA) wrote: >> [ >> http://issues.apache.org/jira/browse/DERBY-2107?page=comments#action_12458736 >> ] Knut Anders Hatlen commented on DERBY-2107: >> ------------------------------------------- >> >> Thanks for your comments, Dan and Mike. It seems to me that all your >> comments are about the variants of LockingPolicy.lockRecordForRead and >> lockRecordForWrite that take a latch parameter. I share your concern >> for the correctness of this code, and that was actually what made me >> start the thread which is archived at >> <URL:http://thread.gmane.org/gmane.comp.apache.db.derby.devel/33135>. >> >> But this brings us back to the original question in that thread: When >> is this code used? I believe it was concluded that those methods were >> only called when the locking policy was NoLocking, in which case they >> are no-ops. (With the exception of some unit tests which invoke the >> methods of the Page interface directly.) > > I see: > LockingPolicy.lockRecordForWrite(Latch ,...) used in one place in > BasePage.
Yes, lockRecordForRead(Latch, ...) is called from BasePage.fetch() and nowhere else. However, Page.fetch() has no callers outside the test code. So if we can trust "Find Usages" in NetBeans, lockRecordForRead(Latch,...) is definitely not in use today. > LockingPolicy.lockRecordForWrite(Latch ,...) used in two places in > BasePage. Yes, it is used in delete() and update(). Page.delete() is only called from BTreeScan.delete(), and Page.update() is only called from GenericScanController.replace(). I cannot say for sure that none of these methods will try to lock a row. Maybe someone who knows the code better could tell? > From a quick look, tracing back from those calls seems there are > places where the code is used (excluding the test code) including > non-btree code. > > If the code is not required then yes we should just remove it, however > I'm pretty sure that we (as closed source project) didn't originally > add these calls for fun. I thought there was a definite requirement > for releasing a latch while waiting for a lock and I don't believe > anything has changed since then. I was surprised when looking > yesterday that they didn't seem to be called (obviously) from the > non-btree case, but I didn't do a complete search. > > The case I'm thinking of is for a heap page from a heap scan: > > get heap page > identify row that needs locking > lock row > process row > release page > > I would have thought that the lock page in this case must release the > latch if it has to wait for the row lock. I think you are right. If you look at HeapScan.fetchNext(), it calls GenericScanController.fetchRows() which again calls OpenConglomerate.lockPositionForRead(). It releases the latch if it has to wait and returns a boolean which tells fetchRows() whether the latch must be reobtained and the scan repositioned. > I'll go back and re-read that thread and also look at the code and try > and understand more about if these calls are required. > > Thanks, > Dan. -- Knut Anders
