Lucene - Core
  1. Lucene - Core
  2. LUCENE-820

SegmentReader.setNorm can fail to remove separate norms file, on Windows

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1
    • Fix Version/s: 2.2
    • Component/s: core/index
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      While working through LUCENE-710 I hit this bug: on Windows
      only, when SegmentReader.setNorm is called, but separate norms
      (_X_N.sY) had already been previously saved, then, on closing the
      reader, we will write the next gen separate norm file correctly
      (_X_N+1.sY) but fail to delete the current one.

      It's quite minor because the next writer to touch the index will
      remove the stale file.

      This is because the Norm class still holds the IndexInput open when
      the reader commits.

      1. LUCENE-820.patch
        11 kB
        Michael McCandless

        Activity

        Hide
        Michael McCandless added a comment -

        I've attached a patch that fixes this.

        The fix is to close the IndexInput after we've cached the norms in the
        in-memory byte array. Once we've cached the norms, we never use that
        IndexInput again (as far as I can tell?). This is also nice because
        it frees up file handles once norms are cached.

        Changes:

        • Fixed TestMultiSearcher: it was closing a searcher and then
          continuing to use it.
        • Added unit test case showing this bug before the patch.
        • Improved MockRAMDirectory to "act like Windows" by refusing to
          delete or overwrite an open file. I removed "final" from 2
          methods (deleteFile, openInput) of RAMDirectory for this.
        • Changed SegmentReader.getNorms to close the Norms after caching.
        Show
        Michael McCandless added a comment - I've attached a patch that fixes this. The fix is to close the IndexInput after we've cached the norms in the in-memory byte array. Once we've cached the norms, we never use that IndexInput again (as far as I can tell?). This is also nice because it frees up file handles once norms are cached. Changes: Fixed TestMultiSearcher: it was closing a searcher and then continuing to use it. Added unit test case showing this bug before the patch. Improved MockRAMDirectory to "act like Windows" by refusing to delete or overwrite an open file. I removed "final" from 2 methods (deleteFile, openInput) of RAMDirectory for this. Changed SegmentReader.getNorms to close the Norms after caching.
        Hide
        Yonik Seeley added a comment -

        > This is also nice because it frees up file handles once norms are cached.

        Ouch... the file handles should not have been open in the first place
        LUCENE-821 fixes that.

        Show
        Yonik Seeley added a comment - > This is also nice because it frees up file handles once norms are cached. Ouch... the file handles should not have been open in the first place LUCENE-821 fixes that.
        Hide
        Michael McCandless added a comment -

        > Ouch... the file handles should not have been open in the first
        > place LUCENE-821 fixes that.

        Excellent!

        How about you commit LUCENE-821, then I'll update & merge, and commit
        my unit test & improvements to MockRamDirectory to mimic Windows?

        Show
        Michael McCandless added a comment - > Ouch... the file handles should not have been open in the first > place LUCENE-821 fixes that. Excellent! How about you commit LUCENE-821 , then I'll update & merge, and commit my unit test & improvements to MockRamDirectory to mimic Windows?
        Hide
        Michael McCandless added a comment -

        This root cause of this bug was already fixed by LUCENE-821 (thanks Yonik!).

        I committed a test case that shows this bug (and now passes), plus improvements to MockRAMDirectory to make it behave like Windows ("can't delete open files").

        Show
        Michael McCandless added a comment - This root cause of this bug was already fixed by LUCENE-821 (thanks Yonik!). I committed a test case that shows this bug (and now passes), plus improvements to MockRAMDirectory to make it behave like Windows ("can't delete open files").

          People

          • Assignee:
            Michael McCandless
            Reporter:
            Michael McCandless
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development