Anders Morken (JIRA) wrote:
[ http://issues.apache.org/jira/browse/DERBY-801?page=all ]
Anders Morken updated DERBY-801:
--------------------------------
Attachment: DERBY-801-6.patch
DERBY-801-6.patch remedies the noise from the iosInProgress sanity checking by checking the committed drop state before croaking.
I've run jdbcapi/LobLengthTest.junit 100 times in a row now without seeing any problems,
and I used to see the problem once every four or five runs without this patch.
RAFContainer4.java | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletion(-)
Thanks for continuing to improve the fix for this problem, Anders.
I'm not sure checking for the committed drop state in the readPage method is something
we want to do - flushing the cache into a black hole is one thing, trying to read data that
is supposed to be gone is another - I think I'd want to know if we had some logic that permitted
reading from a dropped container. =)
I doubt reading from a commit dropped container happen through the
readPage() call in parallel to drops. Locks/Latches/flags should
make sure that does not occur. I think only time we may read from the
dropped container may be during recovery, if there is a crash just
before stub was created for the dropped container, if container marked
commit dropped , then I think only header is read , but that info is
read by reading directly few bytes from the first page.
I've sort of got a feeling that maybe we are masking a problem here -
should anyone (even the cache?) write to a dropped container?
Pages are discarded from cache before the container is stubbed. See
(removeContainer.pageCache.discard(identity .
Maybe we should remove
pages destined for a dying container from the cache when the
container is dropped? =)
I agree with removing/closing container does not seem to be the right
thing to do when I/O is in progress. Having said that, I am also
wondering if it is a over-kill to add more synchronization for this
case. May be catching for ClosedChannelException is good enough .
One more thing I alos noticed going through the code again is
DEBUG_PRINT("RAFContainer4"," .. statements. As Knut mentioned , they
of no use. No one looks at them unless a test fails. If those cases
should not happen , then throwing an ASSERT failure is better,
otherwise please remove those PRINT statements.
Thanks
-suresh
