Suresh Thalamati:
> 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.
OK, so if this actually happened it would be appropriate to throw
exceptions. Good, that's what'll happen now. =)
> >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 .
OK, I'll take a harder look at what's happening when somebody writes to
a dropped container.
> 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.
OK, I changed the two DEBUG_PRINT statements in readPage/writePage to
asserts, ran derbyall, and whaddayaknow, it breaks something. =)
The problem is caused by openContainer checking for "fileData instanceof
java.io.RandomAccessFile" and then calling the getChannel() method to
get hold of a FileChannel object, when a CorruptRandomAccessFile (which
is not a java.io.RandomAccessFile) is used as a stand-in for a regular
file for testing purposes. Again, not something that'll happen during
regular operations, but it could be considered a symptom of the somewhat
hackish assumption that a regular java.io.RandomAccessFile is hiding
under the derby.io.StorageRandomAccessFile interface. =)
So, either the asserts should check for "fileData instanceof
java.io.RandomAccessFile" as well, or we throw them out. Or I rework the
o.a.derby.io interfaces a bit to support channels.
I'll run tests on the first approach tomorrow. Jämtlands bryggeri make
some excellent beer, but I doubt it's good for my coding and debugging
skills. ;)
Thanks for the advice, Suresh. (And Knut Anders as well, of course. =)
--
Anders Morken
My opinions may have changed, but not the fact that I am right!
