I am ok with moving latching out of the lockmanager, if it really is
necessary to get the performance you are looking for. I had hoped that
latching would remain a "service" provided by another module so that if
there are multiple store implementation they could share a single
latching implementation. I assume changes to remove latching out of
lock manage will break existing in progress work to do in memory store -
but I don't know for sure.
I do want to say that I am uneasy about the change (not because of any
specific problem with the current patch - just that a change in this
area has such a chance to cause undetected corruption so easily), in
that I do not think existing tests are going to catch problems with the
new implementation (note the double latching requirement below). It would
not surprise me if derbyall probably passes if you make latching a noop
- it all would depend on timing. But a bug in latching is going to
cause the worst kind of intermittent, unreproducible, data corruption
bugs imagineable. I agree it is bad that existing tests don't test
latching very well, but as time went by that code did not change so was
assumed to function correctly and thus not much was pushing to add tests.
Given the numbers presented on performance I also think it is ok to give
up the lock manager deadlock detection of latches. It also means that
the existing lock table VTI's will not show latch information. Again
this information isn't that useful to users, but does make the system
slightly harder to understand for developers. Maybe the appropriate
information now will be available from the JVM, but I have found this
hard to use, and very inconsistent from one JVM version to the next.
As a future dirction it might be nice to be provide a setting which
would allow someone to monitor the store latch info in some sort of
consistent way, but I would not hold up the current work on this
requirement.
Daniel John Debrunner (JIRA) wrote:
[ http://issues.apache.org/jira/browse/DERBY-2107?page=comments#action_12456463 ]
Daniel John Debrunner commented on DERBY-2107:
----------------------------------------------
Couple of comments jump out from an initial look at the patch.
The first may have been an existing issue:
1) The field nestedLatch is used under synchronization when gettting the latch,
but not when releasing the latch.
2) In setExclusive() if an attempt is made to double latch the page it will
suceed, even when the transaction is not in abort. This seems to conflict the
comments in the method. This will not be handled correctly on the unlatch as
the first unlatch will completely clear the latch leaving the caller thinking
it has the latch when it doesn't.
Not sure what the old code did in this situation.
Move page latching out of the lock manager
------------------------------------------
Key: DERBY-2107
URL: http://issues.apache.org/jira/browse/DERBY-2107
Project: Derby
Issue Type: Improvement
Components: Store, Services, Performance
Affects Versions: 10.3.0.0
Reporter: Knut Anders Hatlen
Assigned To: Knut Anders Hatlen
Priority: Minor
Attachments: derby-2107-1a.diff, derby-2107-1a.stat
Latching of pages could be done more efficiently locally in store than in the
lock manager. See the discussion here:
http://thread.gmane.org/gmane.comp.apache.db.derby.devel/33135
