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

Rollback doesn't preserve integrity of original index

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.0.2
    • Fix Version/s: 2.9.4, 3.0.3, 3.1, 4.0-ALPHA
    • Component/s: core/index
    • Labels:
      None
    • Environment:

      Windows XP pro

    • Lucene Fields:
      New

      Description

      After several "updateDocuments" calls a rollback call does not return the index to the prior state.
      This seems to occur if the number of updates exceeds the RAM buffer size i.e. when some flushing of updates occurs.

      Test fails in Lucene 2.4, 2.9, 3.0.1 and 3.0.2

      JUnit to follow.

      1. TestRollback.java
        3 kB
        Mark Harwood
      2. TestRollback.java
        3 kB
        Shai Erera
      3. LUCENE-2536.patch
        5 kB
        Michael McCandless
      4. LUCENE-2536.patch
        4 kB
        Michael McCandless

        Activity

        Hide
        mikemccand Michael McCandless added a comment -

        Thanks Mark – what a doozie!

        Show
        mikemccand Michael McCandless added a comment - Thanks Mark – what a doozie!
        Hide
        markh Mark Harwood added a comment -

        Thanks for the rapid response Mike/Shai.

        I'll need to patch a couple of production systems using 2.4.0 and 2.9.1 with this.
        For the record - with this same change applied, the test passes OK on these older versions too.

        Show
        markh Mark Harwood added a comment - Thanks for the rapid response Mike/Shai. I'll need to patch a couple of production systems using 2.4.0 and 2.9.1 with this. For the record - with this same change applied, the test passes OK on these older versions too.
        Hide
        mikemccand Michael McCandless added a comment -

        Thanks Shai – new patch.

        Show
        mikemccand Michael McCandless added a comment - Thanks Shai – new patch.
        Hide
        shaie Shai Erera added a comment -

        Looks good to me. I think now that the test class contains just this method, we can consolidate all the helper ones into 1? It was required before 'cause there were two test cases. And we can remove the first assert in the test, since we're not checking here that the index contains the number of added docs after it was committed - there are plenty of other tests that do that.

        Show
        shaie Shai Erera added a comment - Looks good to me. I think now that the test class contains just this method, we can consolidate all the helper ones into 1? It was required before 'cause there were two test cases. And we can remove the first assert in the test, since we're not checking here that the index contains the number of added docs after it was committed - there are plenty of other tests that do that.
        Hide
        mikemccand Michael McCandless added a comment -

        Attached patch – started from Shai's patch, tweaked it a bit, added the one line fix & CHANGES entry.

        I see the test in fact failing on 3.x.

        Show
        mikemccand Michael McCandless added a comment - Attached patch – started from Shai's patch, tweaked it a bit, added the one line fix & CHANGES entry. I see the test in fact failing on 3.x.
        Hide
        mikemccand Michael McCandless added a comment -

        Duh – in DocumentsWriter.abort, right after deletesInRAM.clear() we must add deletesFlush.clear(). So, indeed, any deletions pending (either due to .updateDoc or .deleteDocs) when segments were flushed, fail to be cleared and are then applied on close.

        How awful!

        Show
        mikemccand Michael McCandless added a comment - Duh – in DocumentsWriter.abort, right after deletesInRAM.clear() we must add deletesFlush.clear(). So, indeed, any deletions pending (either due to .updateDoc or .deleteDocs) when segments were flushed, fail to be cleared and are then applied on close. How awful!
        Hide
        shaie Shai Erera added a comment -

        It did not fail for me on 3.1, however did fail on 3.0.1.

        I'm attaching a version of this test, which creates far less docs (5, instead of 10,000) and uses a RAMDirectory). The test still fails, which is expected. I assume if we want to make that an official unit test, the test case which passes should go away? (or not)

        Show
        shaie Shai Erera added a comment - It did not fail for me on 3.1, however did fail on 3.0.1. I'm attaching a version of this test, which creates far less docs (5, instead of 10,000) and uses a RAMDirectory). The test still fails, which is expected. I assume if we want to make that an official unit test, the test case which passes should go away? (or not)
        Hide
        markh Mark Harwood added a comment -

        Looks like on the "close" call IndexWriter is flushing a superfluous ".del" file of buffered deletes.

        Show
        markh Mark Harwood added a comment - Looks like on the "close" call IndexWriter is flushing a superfluous ".del" file of buffered deletes.
        Hide
        markh Mark Harwood added a comment -

        Example JUnit

        Show
        markh Mark Harwood added a comment - Example JUnit

          People

          • Assignee:
            mikemccand Michael McCandless
            Reporter:
            markh Mark Harwood
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development