LOB locks are not released after free().

• Key: DERBY-3098
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Øystein Grøvlen
• Reporter: Øystein Grøvlen
• Component/s: Store
• Affects Version/s: 10.2.2.0, 10.3.1.4
• Fix Version/s: 10.3.2.1, 10.4.1.3

Description

When getBlob/getClob is called on the ResultSet, the current row is
locked if the JDBC driver does not cache the entire LOB value in
memory. This is done to prevent the Blob/Clob object from being
changed. Until now, this lock has been held to the end of the
transaction.

JDBC4 introduced free() methods for the Blob/Clob class. The locking
should be changed so that the locks is releases when the Blob/Clob
object is freed.

Comments

[Øystein Grøvlen]

The attached patch, derby-3098fix.diff, fixes the locking. This patch
is not intended for commit since tests should be added first.

Explanation of the patch: By using READ COMMITTED instead of
REPEATABLE READ the locking will not be associated with the
transaction, but with the BaseContainerHandle. Since we use a handle
that we get from reopening the container, we get control over when the
locks are released. The is closed when the stream is closed, or at
the end of the transaction at the latest. (During termination of the
transaction, close will be called on all container handles).

I have also updated the comments to reflect this new implementation.
I must admit I did not quite understand the discussion in the comment
around using the isolation level of the container, but given my
proposed solution, that discussion does not relevant, and I have
deleted it from the comments.

I plan to add the following tests:

1. Check that lock on row is released at free() if isolation level of
   transaction is READ COMMITTED.
2. Check that lock on row is not released at free() if isolation level
   of transaction is REPEATABLE READ.
3. Check that lock on row is released when transaction commits.

Please, speak up, if you think more tests are necessary.

[Knut Anders Hatlen]

Hi Øystein,

The patch looks correct to me and the test plan also sounds good. I'm just wondering how this works when the transaction isolation level is READ UNCOMMITTED. Will a reader have to wait if another transaction has locked a large lob exclusively, but not if the lob is small?

[Øystein Grøvlen]

My testing so far shows that the patch solves the problem for Blob, but it does not appear to do so for Clob.

When I run a simple test program that does getClob, it does not seem like OverflowInputStream is involved.
I will further investigate where the paths differ, but if anybody has an idea why Blob and Clob are handled differently, I welcome any ideas you may have.

[Øystein Grøvlen]

To Knut Anders question of READ UNCOMMITTED. I have not tested it
yet, but I will add tests for that, too.

I assume we also for READ_UNCOMMITTED want the record of the Blob to
be locked since the Blob object should be stable regardless of
isolation level. (I think this illustrate that locking the record is
not the best solution for guaranteeing stability, but creating a new
mechanism that allow more concurrency will be much more work. More
work than I am willing to put into this.)

[Øystein Grøvlen]

I was wrong about Clob not using OverflowInputStream. It does. (I
had an error in my test program so that it was fetching a Clob that
was not large enough for OverflowInputStream to come into play.)

The problem I have is that locks are not released after free() for
Clob. I think the problem is that it is obtaining two sets of locks,
but only releasing one set. My theory is that in the EmbedClob constructor,
both
     ((Resetable) storeStream).initStream();
and
     this.clob = new StoreStreamClob(storeStream, this);
causes locks to be set.

[Øystein Grøvlen]

Attached is new patch, derby-3098fix+tests.diff, that fixes the double
locking problem for Clobs and add tests.

EmbedClob no longer explicity initializes the stream since it will be
done when the StoreStreamClob object is created. I also added a check
in OverflowInputStream to ensure that should a stream be initialized
several times, locking will only be peformed the first time.

Tests has also been added. (Since free() is a JDBC 4 method, most test
cases had to be added to the JDBC 4 test suite, but I also added a
test case to BlobClob4Blob to check that locks are released at commit
if free() is not called.)

With this patch, jdbc4/ClobTest will fail with client/server since it
turns out that when Clob.free() is called on the client, this is not
propagated to the server, and the server-side Clob is not freed until
commit. I will file a separate issue for that.

[Øystein Grøvlen]

Until DERBY-3107 is fixed, ClobTest will fail in client/server.

[Øystein Grøvlen]

Committed patch, derby-3098fix+tests.diff, as revision 584777.
Junit All and derbyall ran without errors.

M java/engine/org/apache/derby/impl/jdbc/EmbedClob.java
Only initialize storeStream once.

M java/engine/org/apache/derby/impl/store/raw/data/OverflowInputStream.java
Do not lock for entire duration anymore, but until corresponding
BaseContainerHandle has been closed. Also, make sure that locking
only happens once.

M java/testing/org/apache/derbyTesting/functionTests/tests/jdbc4/BlobTest.java
Add tests to test locking of Blob after free() for different isolation
levels. Also, moved code from setUp that is only relevant to one test
case to that specific test case.

M java/testing/org/apache/derbyTesting/functionTests/tests/jdbc4/ClobTest.java
Add tests to test locking of Blob after free() for different isolation
levels. Also, moved code from setUp that is only relevant to one test
case to that specific test case.

M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/BlobClob4BlobTest.java
Test that locks on Blob/Clob are released when transaction commits.

[Øystein Grøvlen]

Merged patch, derby-3098fix+tests.diff, to 10.3 branch as revision 584791.