Derby
  1. Derby
  2. DERBY-4963

Revert to FileDescriptor#sync from FileChannel#force to improve interrupt resilience

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 10.8.1.2
    • Component/s: Store
    • Labels:
      None
    • Issue & fix info:
      Release Note Needed

      Description

      FileChannel.force is interruptable, and we really don't want to be interrupted when we flush the log file. Happily, on most platforms, we use the "rws"/"rwd" file open mask which makes the writes thjemselves synchronized, so no subsequent explicit file level sync is needed anyway.

      DirFile4#getRandowmAccessFile should use plain DirRandomAccessFile instead of the current DirRandomAccessFile4. This will make StorageRandomAccessFile#sync map to FileDescriptor#sync instead of FileChannel#force (also for NIO supporting platforms).

      Since FileDescriptor#sync does not allow synching file data only (it also synchronizes metadata), those platforms which do not support write synchronization will experience a performance drop, but this is the price we have to pay to survive interrupts without shutting down the database on those platforms.

      Users which experience this as a problem, should update to a newer JVM which does support "rws"/"rwd" in the mode argument to java.io.RandomAccessFile (http://download.oracle.com/javase/6/docs/api/java/io/RandomAccessFile.html#RandomAccessFile(java.io.File,%20java.lang.String).

      Cf. also discussion on https://issues.apache.org/jira/browse/DERBY-4741?focusedCommentId=12977862&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#action_12977862 .

      1. derby-4963-1.diff
        12 kB
        Dag H. Wanvik
      2. derby-4963-1.stat
        0.9 kB
        Dag H. Wanvik
      3. derby-4963-2.diff
        13 kB
        Dag H. Wanvik
      4. derby-4963-2.stat
        0.9 kB
        Dag H. Wanvik
      5. releaseNote.html
        4 kB
        Rick Hillegas
      6. releaseNote.html
        4 kB
        Rick Hillegas

        Issue Links

          Activity

          Hide
          Dag H. Wanvik added a comment -

          Linking to DERBY-4741.

          Show
          Dag H. Wanvik added a comment - Linking to DERBY-4741 .
          Hide
          Dag H. Wanvik added a comment -

          Marking with "release note needed" since we lose performance on some old platforms.

          Show
          Dag H. Wanvik added a comment - Marking with "release note needed" since we lose performance on some old platforms.
          Hide
          Dag H. Wanvik added a comment -

          Uploading patch derby-4961-1, running regressions.

          Since the new StorageRandomAccessFile#sync implementation will now never heed the metaflag, I removed it.

          Show
          Dag H. Wanvik added a comment - Uploading patch derby-4961-1, running regressions. Since the new StorageRandomAccessFile#sync implementation will now never heed the metaflag, I removed it.
          Hide
          Dag H. Wanvik added a comment -

          Regressions ran ok, will commit tomorrow.

          Show
          Dag H. Wanvik added a comment - Regressions ran ok, will commit tomorrow.
          Hide
          Knut Anders Hatlen added a comment -

          I ran the single-record update performance test in derbyTesting.jar on Solaris/JDK6 with the write-cache disabled, and I didn't see any performance loss with the patch. Solaris is one of the platforms on which rws vs rwd, or sync(true) vs sync(false), does make a difference, so it seems you're right that the code path in question is not executed on recent JVMs. If there had been a change from one write operation to two write operations per log flush, the single-record update performance test should have detected it.

          Show
          Knut Anders Hatlen added a comment - I ran the single-record update performance test in derbyTesting.jar on Solaris/JDK6 with the write-cache disabled, and I didn't see any performance loss with the patch. Solaris is one of the platforms on which rws vs rwd, or sync(true) vs sync(false), does make a difference, so it seems you're right that the code path in question is not executed on recent JVMs. If there had been a change from one write operation to two write operations per log flush, the single-record update performance test should have detected it.
          Hide
          Knut Anders Hatlen added a comment -

          java/engine/org/apache/derby/impl/io/build.xml has two references to DirRandomAccessFile4.java. We should probably remove them as well now that we remove the file.

          Show
          Knut Anders Hatlen added a comment - java/engine/org/apache/derby/impl/io/build.xml has two references to DirRandomAccessFile4.java. We should probably remove them as well now that we remove the file.
          Hide
          Dag H. Wanvik added a comment -

          > java/engine/org/apache/derby/impl/io/build.xml has two references to DirRandomAccessFile4

          Thanks for test driving this patch and catching that omission, Knut! I agree those dangling references should be cleaned up.
          Good to hear the performance test didn't show any drop!
          I also verified in the debugger that the file level sync isn't used for flushing the log on platforms that support rws/rwd, so I think we are ok.

          Show
          Dag H. Wanvik added a comment - > java/engine/org/apache/derby/impl/io/build.xml has two references to DirRandomAccessFile4 Thanks for test driving this patch and catching that omission, Knut! I agree those dangling references should be cleaned up. Good to hear the performance test didn't show any drop! I also verified in the debugger that the file level sync isn't used for flushing the log on platforms that support rws/rwd, so I think we are ok.
          Hide
          Dag H. Wanvik added a comment - - edited

          Committed derby-4963-2 as svn 1057702, resolving. WIll keep it open till I have produced a release note.

          Show
          Dag H. Wanvik added a comment - - edited Committed derby-4963-2 as svn 1057702, resolving. WIll keep it open till I have produced a release note.
          Hide
          Rick Hillegas added a comment -

          Attaching the first rev of a release note for this issue. Dag and Knut, you may want to sanity check this note since you are familiar with the patch. Thanks.

          Show
          Rick Hillegas added a comment - Attaching the first rev of a release note for this issue. Dag and Knut, you may want to sanity check this note since you are familiar with the patch. Thanks.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks for writing the release note, Rick. It is clear and looks correct for the most part. The one thing I believe is inaccurate, is the sentence "Solaris/JDK5 is one of these platforms." I don't think Solaris is affected by this change, at least not on JDK 5.

          I think the only platforms affected are those that suffered from DERBY-1, which are some old releases of Java 1.4.2 and Java 5 on Mac OS/X and FreeBSD (and possibly other BSDs). And, looking at the code, it probably also affects a code path taken by all JVMs at version 1.4.0 and 1.4.1, but I'm not sure if we still support those variants of 1.4 or if we require 1.4.2 now.

          Dag, does that sound correct, or did I garble things?

          Show
          Knut Anders Hatlen added a comment - Thanks for writing the release note, Rick. It is clear and looks correct for the most part. The one thing I believe is inaccurate, is the sentence "Solaris/JDK5 is one of these platforms." I don't think Solaris is affected by this change, at least not on JDK 5. I think the only platforms affected are those that suffered from DERBY-1 , which are some old releases of Java 1.4.2 and Java 5 on Mac OS/X and FreeBSD (and possibly other BSDs). And, looking at the code, it probably also affects a code path taken by all JVMs at version 1.4.0 and 1.4.1, but I'm not sure if we still support those variants of 1.4 or if we require 1.4.2 now. Dag, does that sound correct, or did I garble things?
          Hide
          Knut Anders Hatlen added a comment -

          Dag answered on derby-dev:

          Sounds right.

          Quote from Derby-4741:

          Cf. DERBY-1 and check in LogToFile#openLogFileInWriteMode which calls
          checkJvmSyncError to determine this. The platforms mentioned are
          (e.g. early versions of 1.4.2 and 1.5 on Mac OS and FreeBSD

          As for Sun JVM 1.4.0 and 1.4.1, I have not investigated those, but what
          you say may be true.

          Thanks,
          Dag

          Show
          Knut Anders Hatlen added a comment - Dag answered on derby-dev: Sounds right. Quote from Derby-4741: Cf. DERBY-1 and check in LogToFile#openLogFileInWriteMode which calls checkJvmSyncError to determine this. The platforms mentioned are (e.g. early versions of 1.4.2 and 1.5 on Mac OS and FreeBSD As for Sun JVM 1.4.0 and 1.4.1, I have not investigated those, but what you say may be true. Thanks, Dag
          Hide
          Knut Anders Hatlen added a comment -

          My comment about 1.4.0 and 1.4.1 was based on this comment in LogToFile:

          • Note: Why logging system support file sync and write sync ?
          • Note : The reason to support file and write sync of logs is
          • there was no support to do write sync until jdk1.4 and then
          • there was write sync jvm bug in jdk1.4.1, only in jdk1.4.2 write
          • sync(rws and rwd modes) mechanism can be used correctly.
          • Default in JVMS >= jdk1.4.2 is write sync(see the boot method for jvm checks).

          And in DirStorageFactory4:

          private static final boolean rwsOK = JVMInfo.JDK_ID >= JVMInfo.J2SE_142;

          Show
          Knut Anders Hatlen added a comment - My comment about 1.4.0 and 1.4.1 was based on this comment in LogToFile: Note: Why logging system support file sync and write sync ? Note : The reason to support file and write sync of logs is there was no support to do write sync until jdk1.4 and then there was write sync jvm bug in jdk1.4.1, only in jdk1.4.2 write sync(rws and rwd modes) mechanism can be used correctly. Default in JVMS >= jdk1.4.2 is write sync(see the boot method for jvm checks). And in DirStorageFactory4: private static final boolean rwsOK = JVMInfo.JDK_ID >= JVMInfo.J2SE_142;
          Hide
          Rick Hillegas added a comment -

          Attaching a new rev of the release note, incorporating feedback from Knut and Dag.

          Show
          Rick Hillegas added a comment - Attaching a new rev of the release note, incorporating feedback from Knut and Dag.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks, Rick! The release note looks fine to me.

          Show
          Knut Anders Hatlen added a comment - Thanks, Rick! The release note looks fine to me.
          Hide
          Knut Anders Hatlen added a comment -

          [bulk update] Close all resolved issues that haven't been updated for more than one year.

          Show
          Knut Anders Hatlen added a comment - [bulk update] Close all resolved issues that haven't been updated for more than one year.

            People

            • Assignee:
              Dag H. Wanvik
              Reporter:
              Dag H. Wanvik
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development