db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Knut Anders Hatlen <Knut.Hat...@Sun.COM>
Subject Re: [jira] Commented: (DERBY-2107) Move page latching out of the lock manager
Date Fri, 15 Dec 2006 19:34:03 GMT
Daniel John Debrunner <djd@apache.org> 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

View raw message