Uploaded image for project: 'Lucene - Core'
  1. Lucene - Core
  2. LUCENE-6169

Recent Java 9 commit breaks fsync on directory

    Details

    • Type: Bug
    • Status: Open
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: core/store
    • Labels:
    • Lucene Fields:
      New

      Description

      I open this issue to keep track of the communication with Oracle and OpenJDK about this:

      Basically, what happens: In LUCENE-5588 we added support to FSDirectory to be able to sync on directory metadata changes (means the contents of the directory itsself). This is very important on Unix system (maybe also on Windows), because fsyncing a single file does not necessarily writes the directory's contents to disk. Lucene uses this for commits. We first do an atomic rename of the segments file (to make the commit public), but we have to be sure that the rename operation is written to disk. Because of that we must fsync the directory.

      To enforce this with plain system calls (libc), you open a directory for read and then call fsync. In java this can be done by opening a FileChannel on the direczory(for read) and call fc.force() on it.

      Unfortunately the commit http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/e5b66323ae45 in OpenJDK 9 break this. The corresponding issue is https://bugs.openjdk.java.net/browse/JDK-8066915. The JDK now explicitly checks if a file is a directory and disallows opening a FileChannel on it. This breaks our commit safety.

      Because this behaviour is undocumented (not even POSIX has explicit semantics for syncing directories), we know that it worked at least on MacOSX and Linux. The code in IOUtils is currently written in a way that it tries to sync the diretory, but swallows any Exception. So this change does not break Liucene, but it breaks our commit safety. During testing we assert that the fsync actually works on Linux and MacOSX, in production code the user will notice nothing.

      We should take action and contact Alan Bateman about his commit and this issue on the mailing list, possibly through Rory O'Donnell.

        Issue Links

          Activity

          Hide
          thetaphi Uwe Schindler added a comment -

          As a quick fix so we can still run the tests on Java 9, I will commit a workaround and disable the assert on Java 9.

          I will keep this issue open to keep track and revert the workaround once Oracle fixed the problem in some way.

          Show
          thetaphi Uwe Schindler added a comment - As a quick fix so we can still run the tests on Java 9, I will commit a workaround and disable the assert on Java 9. I will keep this issue open to keep track and revert the workaround once Oracle fixed the problem in some way.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1650390 from Uwe Schindler in branch 'dev/trunk'
          [ https://svn.apache.org/r1650390 ]

          LUCENE-6169: Disable the fsync on directory assert for Java 9+, because in Java 9 opening a FileChannel on directory no longer works

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1650390 from Uwe Schindler in branch 'dev/trunk' [ https://svn.apache.org/r1650390 ] LUCENE-6169 : Disable the fsync on directory assert for Java 9+, because in Java 9 opening a FileChannel on directory no longer works
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1650391 from Uwe Schindler in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1650391 ]

          Merged revision(s) 1650390 from lucene/dev/trunk:
          LUCENE-6169: Disable the fsync on directory assert for Java 9+, because in Java 9 opening a FileChannel on directory no longer works

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1650391 from Uwe Schindler in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1650391 ] Merged revision(s) 1650390 from lucene/dev/trunk: LUCENE-6169 : Disable the fsync on directory assert for Java 9+, because in Java 9 opening a FileChannel on directory no longer works
          Hide
          thetaphi Uwe Schindler added a comment -

          I started a mail thread on nio-dev mailing list: http://mail.openjdk.java.net/pipermail/nio-dev/2015-January/002979.html

          Show
          thetaphi Uwe Schindler added a comment - I started a mail thread on nio-dev mailing list: http://mail.openjdk.java.net/pipermail/nio-dev/2015-January/002979.html
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1662561 from Uwe Schindler in branch 'dev/branches/lucene_solr_4_10'
          [ https://svn.apache.org/r1662561 ]

          Merged revision(s) 1650391 from lucene/dev/branches/branch_5x:
          Merged revision(s) 1650390 from lucene/dev/trunk:
          LUCENE-6169: Disable the fsync on directory assert for Java 9+, because in Java 9 opening a FileChannel on directory no longer works

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1662561 from Uwe Schindler in branch 'dev/branches/lucene_solr_4_10' [ https://svn.apache.org/r1662561 ] Merged revision(s) 1650391 from lucene/dev/branches/branch_5x: Merged revision(s) 1650390 from lucene/dev/trunk: LUCENE-6169 : Disable the fsync on directory assert for Java 9+, because in Java 9 opening a FileChannel on directory no longer works
          Hide
          thetaphi Uwe Schindler added a comment -

          The commit in the OpenJDK 9 tree is about to be removed:

          We have revised our approach to this (pair or trio of) problem(s). The following sequence of actions is proposed.

          1. Revert the patch which fixed https://bugs.openjdk.java.net/browse/JDK-8066915, (fs) Files.newByteChannel opens directories for cases where subsequent reads may fail, which instigated the present situation. I will file and post the link to a new Issue for this.

          2. Work on a fix for https://bugs.openjdk.java.net/browse/JDK-8080589, (fc) FileChannel.force should use fcntl(F_FULLFSYNC) instead of fsync on OS X.

          3. Work on a zero to minimal impact fix for https://bugs.openjdk.java.net/browse/JDK-8080235, (fs) Provide ability to flush all modified buffered data to a permanent storage device.

          Hopefully the foregoing plan of record shall be to everyone’s satisfaction and will converge to an eventual mutually acceptable, logical solution.

          Thanks,

          Brian

          Show
          thetaphi Uwe Schindler added a comment - The commit in the OpenJDK 9 tree is about to be removed: We have revised our approach to this (pair or trio of) problem(s). The following sequence of actions is proposed. 1. Revert the patch which fixed https://bugs.openjdk.java.net/browse/JDK-8066915 , (fs) Files.newByteChannel opens directories for cases where subsequent reads may fail, which instigated the present situation. I will file and post the link to a new Issue for this. 2. Work on a fix for https://bugs.openjdk.java.net/browse/JDK-8080589 , (fc) FileChannel.force should use fcntl(F_FULLFSYNC) instead of fsync on OS X. 3. Work on a zero to minimal impact fix for https://bugs.openjdk.java.net/browse/JDK-8080235 , (fs) Provide ability to flush all modified buffered data to a permanent storage device. Hopefully the foregoing plan of record shall be to everyone’s satisfaction and will converge to an eventual mutually acceptable, logical solution. Thanks, Brian
          Hide
          thetaphi Uwe Schindler added a comment -

          Issue: https://bugs.openjdk.java.net/browse/JDK-8080629
          Webrev (revert): http://cr.openjdk.java.net/~bpb/8080629/webrev.00/

          I am looking forward to seen an additional way to fsync a directory!

          Show
          thetaphi Uwe Schindler added a comment - Issue: https://bugs.openjdk.java.net/browse/JDK-8080629 Webrev (revert): http://cr.openjdk.java.net/~bpb/8080629/webrev.00/ I am looking forward to seen an additional way to fsync a directory!
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1715619 from Uwe Schindler in branch 'dev/trunk'
          [ https://svn.apache.org/r1715619 ]

          LUCENE-6169: Remove the Java 9 workaround, as change was reverted in OpenJDK source

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1715619 from Uwe Schindler in branch 'dev/trunk' [ https://svn.apache.org/r1715619 ] LUCENE-6169 : Remove the Java 9 workaround, as change was reverted in OpenJDK source
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1715620 from Uwe Schindler in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1715620 ]

          Merged revision(s) 1715619 from lucene/dev/trunk:
          LUCENE-6169: Remove the Java 9 workaround, as change was reverted in OpenJDK source

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1715620 from Uwe Schindler in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1715620 ] Merged revision(s) 1715619 from lucene/dev/trunk: LUCENE-6169 : Remove the Java 9 workaround, as change was reverted in OpenJDK source
          Hide
          thetaphi Uwe Schindler added a comment - - edited

          I reverted the assert statement in our source. I leave this issue open to track changes in OpenJDK: https://bugs.openjdk.java.net/browse/JDK-8080235

          Show
          thetaphi Uwe Schindler added a comment - - edited I reverted the assert statement in our source. I leave this issue open to track changes in OpenJDK: https://bugs.openjdk.java.net/browse/JDK-8080235

            People

            • Assignee:
              Unassigned
              Reporter:
              thetaphi Uwe Schindler
            • Votes:
              1 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:

                Development