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

Exception during IndexWriter.close() prevents release of the write.lock

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.9
    • Fix Version/s: 2.1
    • Component/s: None
    • Labels:
      None
    • Environment:

      Lucene 1.4 through 2.1 HEAD (as of 2006-12-14)

    • Lucene Fields:
      New

      Description

      After encountering a case of index corruption - see http://issues.apache.org/jira/browse/LUCENE-140 - when the close() method encounters an exception in the flushRamSegments() method, the index write.lock is not released (ie. it is not really closed).

      The writelock is only released when the IndexWriter is GC'd and finalize() is called.

        Activity

        Hide
        mikemccand Michael McCandless added a comment -

        Closing all issues that were resolved for 2.1.

        Show
        mikemccand Michael McCandless added a comment - Closing all issues that were resolved for 2.1.
        Hide
        mikemccand Michael McCandless added a comment -

        OK I just commited the fix to the javadoc. Thanks Jed!

        Show
        mikemccand Michael McCandless added a comment - OK I just commited the fix to the javadoc. Thanks Jed!
        Hide
        jedws Jed Wesley-Smith added a comment -

        Awesome, thanks!

        Show
        jedws Jed Wesley-Smith added a comment - Awesome, thanks!
        Hide
        mikemccand Michael McCandless added a comment -

        OK I will update the javadoc for IndexWriter.close to make this clear. Thanks!

        Show
        mikemccand Michael McCandless added a comment - OK I will update the javadoc for IndexWriter.close to make this clear. Thanks!
        Hide
        jedws Jed Wesley-Smith added a comment -

        I guess, particularly in light of LUCENE-702 that this behavior is OK - and the IndexReader.unlock(dir) is a good suggestion. My real problem was that the finalize() method does eventually remove the write lock.

        For me then the suggestion would be to document the exceptional behavior of the close() method (ie. it means that changes haven't been written and the write lock is still held) and link to the IndexReader.unlock(Directory) method.

        Show
        jedws Jed Wesley-Smith added a comment - I guess, particularly in light of LUCENE-702 that this behavior is OK - and the IndexReader.unlock(dir) is a good suggestion. My real problem was that the finalize() method does eventually remove the write lock. For me then the suggestion would be to document the exceptional behavior of the close() method (ie. it means that changes haven't been written and the write lock is still held) and link to the IndexReader.unlock(Directory) method.
        Hide
        hossman Hoss Man added a comment -

        given the changes made in LUCENE-702, i concur with your assesment Michael: keeping the lock open so that the caller can attempt to deal with the problem then retry makes sense.

        even if we decided that the consistent state of the IndexWriter isn't an invarient that the user can rely on, asking users to forcably unlock in the event of an exception on close seems like a more reasonable expectation then to forcably unlock for them automatically.

        Show
        hossman Hoss Man added a comment - given the changes made in LUCENE-702 , i concur with your assesment Michael: keeping the lock open so that the caller can attempt to deal with the problem then retry makes sense. even if we decided that the consistent state of the IndexWriter isn't an invarient that the user can rely on, asking users to forcably unlock in the event of an exception on close seems like a more reasonable expectation then to forcably unlock for them automatically.
        Hide
        mikemccand Michael McCandless added a comment -

        I think this (not releasing write lock on hitting an exception) is
        actually by design. It's because the writer still has pending changes
        to commit to disk.

        And, with the fix for LUCENE-702 (just committed), if we hit an
        exception during IndexWriter.close(), the IndexWriter is left in a
        consistent state (this is not quite the case pre-2.1).

        Meaning, if you caught that exception, fixed the root cause (say freed
        up disk space), and called close again (successfully), you would not
        have lost any documents, and the write lock will be released.

        I can also see that if we did release the write lock on exception,
        this could dangerously / easily mask the fact that there was an
        exception. Ie, if the IOException is caught and ignored (or writes a
        message but nobody sees it), and the write lock was released, then you
        could go for quite a while before discovering eg that new docs weren't
        visible in the index. Whereas, keeping the write lock held on
        exception will cause much faster discovery of the problem (eg when the
        next writer tries to instantiate).

        I think this is the right exception semantics to aim for? Ie if the
        close did not succeed we should not release the write lock (because we
        still have pending changes).

        Then, if you want to force releasing of the write lock, you can still
        do something like this:

        try

        { writer.close(); }

        finally {
        if (IndexReader.isLocked(directory))

        { IndexReader.unlock(directory); }

        }

        Show
        mikemccand Michael McCandless added a comment - I think this (not releasing write lock on hitting an exception) is actually by design. It's because the writer still has pending changes to commit to disk. And, with the fix for LUCENE-702 (just committed), if we hit an exception during IndexWriter.close(), the IndexWriter is left in a consistent state (this is not quite the case pre-2.1). Meaning, if you caught that exception, fixed the root cause (say freed up disk space), and called close again (successfully), you would not have lost any documents, and the write lock will be released. I can also see that if we did release the write lock on exception, this could dangerously / easily mask the fact that there was an exception. Ie, if the IOException is caught and ignored (or writes a message but nobody sees it), and the write lock was released, then you could go for quite a while before discovering eg that new docs weren't visible in the index. Whereas, keeping the write lock held on exception will cause much faster discovery of the problem (eg when the next writer tries to instantiate). I think this is the right exception semantics to aim for? Ie if the close did not succeed we should not release the write lock (because we still have pending changes). Then, if you want to force releasing of the write lock, you can still do something like this: try { writer.close(); } finally { if (IndexReader.isLocked(directory)) { IndexReader.unlock(directory); } }

          People

          • Assignee:
            mikemccand Michael McCandless
            Reporter:
            jedws Jed Wesley-Smith
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development