Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3.1, 2.4
    • Component/s: core/index
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      In both Lucene 2.3 and trunk, the index becomes corrupted when autoCommit=false

      1. indexstress.patch
        12 kB
        Yonik Seeley
      2. indexstress.patch
        11 kB
        Yonik Seeley
      3. LUCENE-1173.patch
        12 kB
        Michael McCandless

        Issue Links

          Activity

          Hide
          Yonik Seeley added a comment -

          Attaching a patch that can reproduce.
          With autoCommit=true, the test passes. Change it to false and it fails.

          The test basic uses multiple threads to update documents. The last document for any id is kept, and then all these docs are indexed again serially. The two indicies are them compared.

          Show
          Yonik Seeley added a comment - Attaching a patch that can reproduce. With autoCommit=true, the test passes. Change it to false and it fails. The test basic uses multiple threads to update documents. The last document for any id is kept, and then all these docs are indexed again serially. The two indicies are them compared.
          Hide
          Yonik Seeley added a comment -

          Note: if I reduce the test to indexing with a single thread, it still fails.
          Map docs = indexRandom(1, 50, 50, dir1);
          The test still does the indexing in a different thread than the close(), so it's not quite a single threaded test.

          Another thing to note: all of the terms are matching up (the test succeeds if I don't test the stored fields).

          Show
          Yonik Seeley added a comment - Note: if I reduce the test to indexing with a single thread, it still fails. Map docs = indexRandom(1, 50, 50, dir1); The test still does the indexing in a different thread than the close(), so it's not quite a single threaded test. Another thing to note: all of the terms are matching up (the test succeeds if I don't test the stored fields).
          Hide
          Michael McCandless added a comment -

          Uh oh ... I'll take this!

          Show
          Michael McCandless added a comment - Uh oh ... I'll take this!
          Hide
          Yonik Seeley added a comment -

          Thanks Mike!

          Attaching new version of test that correctly deals with terms with no docs (because of deletions).
          Other variations were failing before, now it's just those with autoCommit=false

          Note that it's possible to trigger this bug by indexing only 3 documents:
          mergeFactor=2; maxBufferedDocs=2; Map docs = indexRandom(1, 3, 2, dir1);

          I love random testing

          Show
          Yonik Seeley added a comment - Thanks Mike! Attaching new version of test that correctly deals with terms with no docs (because of deletions). Other variations were failing before, now it's just those with autoCommit=false Note that it's possible to trigger this bug by indexing only 3 documents: mergeFactor=2; maxBufferedDocs=2; Map docs = indexRandom(1, 3, 2, dir1); I love random testing
          Hide
          Michael McCandless added a comment -

          Yes this is one awesome test case

          Thanks.

          Show
          Michael McCandless added a comment - Yes this is one awesome test case Thanks.
          Hide
          Michael McCandless added a comment -

          I just sent email to java-user to give a heads up on this.

          Attach patch fixes the issue. All tests pass.

          I think we should spin 2.3.1 for this one?

          Show
          Michael McCandless added a comment - I just sent email to java-user to give a heads up on this. Attach patch fixes the issue. All tests pass. I think we should spin 2.3.1 for this one?
          Hide
          Yonik Seeley added a comment -

          Patch looks good (heh... a one liner!)
          At least it won't break previously working code since autoCommit=true is the default. The only risk is people trying out the new setting and not realizing it can break things.
          2.3.1 might be nice, but I'll leave to others (who have the actual time to do the work) to decide.

          Show
          Yonik Seeley added a comment - Patch looks good (heh... a one liner!) At least it won't break previously working code since autoCommit=true is the default. The only risk is people trying out the new setting and not realizing it can break things. 2.3.1 might be nice, but I'll leave to others (who have the actual time to do the work) to decide.
          Hide
          Yonik Seeley added a comment -

          Hold up a bit... my random testing may have hit another bug....
          testMultiConfig hit an error at some point when I cranked up the iterations... I'm trying to reproduce.

          Show
          Yonik Seeley added a comment - Hold up a bit... my random testing may have hit another bug.... testMultiConfig hit an error at some point when I cranked up the iterations... I'm trying to reproduce.
          Hide
          Michael McCandless added a comment -

          Backported to 2.3.

          Patch looks good (heh... a one liner!)

          Yeah the worst ones always seem to be one-liner fixes. Sigh.

          Hold up a bit... my random testing may have hit another bug....
          testMultiConfig hit an error at some point when I cranked up the iterations... I'm trying to reproduce.

          I'll go dig on that one next.

          Show
          Michael McCandless added a comment - Backported to 2.3. Patch looks good (heh... a one liner!) Yeah the worst ones always seem to be one-liner fixes. Sigh. Hold up a bit... my random testing may have hit another bug.... testMultiConfig hit an error at some point when I cranked up the iterations... I'm trying to reproduce. I'll go dig on that one next.
          Hide
          Michael McCandless added a comment -

          Committed to 2.3 & trunk.

          Show
          Michael McCandless added a comment - Committed to 2.3 & trunk.

            People

            • Assignee:
              Michael McCandless
              Reporter:
              Yonik Seeley
            • Votes:
              3 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development