Don't call RAFContainer.padFile() from instances of RAFContainer4

• Key: DERBY-3726
• Type: Improvement
• Status: Closed
• Resolution: Fixed
• Priority: Minor
• Assignee: Knut Anders Hatlen
• Reporter: Knut Anders Hatlen
• Component/s: Store
• Affects Version/s: 10.3.3.0, 10.4.1.3
• Fix Version/s: 10.4.2.0, 10.5.1.1

Description

In this thread on derby-dev, http://mail-archives.apache.org/mod_mbox/db-derby-dev/200806.mbox/[e-mail]%3e,
it was mentioned that RAFContainer4 calls padFile() when creating a container. Since padFile() uses old I/O calls and the rest of RAFContainer4 uses NIO, it could possibly cause similar issues as those seen in DERBY-3347. Although we haven't verified that this is a problem, we should try to avoid mixing old I/O and NIO to be on the safe side.

Comments

[Knut Anders Hatlen]

padFile() is called from three locations in RAFContainer. Two of the calls come from writePage(), which is overloaded in RAFContainer4 and therefore not called by RAFContainer4. The last one is in writeRAFHeader() where it is called if the container is being created.

This last call is the one we'll need to get rid of for RAFContainer4, and it is actually quite easy to get rid of it for RAFContainer as well. Here's what writeRAFHeader() does when it's called with create=true:

a) get an empty embryonic page by calling getEmbryonicPage(null), which is the same as calling new byte[AllocPage.MAX_BORROWED_SPACE].

b) pass the empty embryonic page to writeHeader() which initializes it and writes it to the file

c) call padFile() to make sure the file ends on a page boundary (to work around a bug in the EPOC jvm)

Now, if the the byte array returned from getEmbryonicPage(null) had the size of a full page instead of just a fraction of a page, the call to writeHeader() would automatically give us a file that ended on a page boundary. The attached patch (d3726-1a) therefore replaces the call to getEmbryonicPage(null) with new byte[pageSize], and then it can also remove the call to padFile() and the special handling of fileData==null in getEmbryonicPage().

Derbyall and suites.All ran cleanly on Solaris 10/JDK 6.

[Kristian Waagan]

The patch looks good to me.
I'm not sure we really need the EPOC JVM bug workaround, but it doesn't seem to affect Derby negatively to keep it.

+1 to commit.

[Knut Anders Hatlen]

Thanks for looking at the patch, Kristian. Committed revision 670534.

I wasn't sure whether to keep the workaround for the EPOC bug or not, but I found it safest to keep it for now.

I'm planning to port the fix to 10.4 and possibly 10.3 as well.

[Knut Anders Hatlen]

Merged to 10.4 and committed revision 671480.

[Knut Anders Hatlen]

Merged to 10.3 and committed revision 671567.