Lucene - Core
  1. Lucene - Core
  2. LUCENE-3876

TestIndexWriterExceptions fails (reproducible)

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.6, 4.0-ALPHA
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      ant test -Dtestcase=TestIndexWriterExceptions -Dtestmethod=testIllegalPositions -Dtests.seed=-228094d3d2f35cf2:-496e33eec9bbd57c:36a1c54f4e1bb32 -Dargs="-Dfile.encoding=UTF-8"
      
          [junit] junit.framework.AssertionFailedError: position=-2 lastPosition=0
          [junit]     at org.apache.lucene.codecs.lucene40.Lucene40PostingsWriter.addPosition(Lucene40PostingsWriter.java:215)
          [junit]     at org.apache.lucene.index.FreqProxTermsWriterPerField.flush(FreqProxTermsWriterPerField.java:519)
          [junit]     at org.apache.lucene.index.FreqProxTermsWriter.flush(FreqProxTermsWriter.java:92)
          [junit]     at org.apache.lucene.index.TermsHash.flush(TermsHash.java:117)
          [junit]     at org.apache.lucene.index.DocInverter.flush(DocInverter.java:53)
          [junit]     at org.apache.lucene.index.DocFieldProcessor.flush(DocFieldProcessor.java:81)
          [junit]     at org.apache.lucene.index.DocumentsWriterPerThread.flush(DocumentsWriterPerThread.java:475)
          [junit]     at org.apache.lucene.index.DocumentsWriter.doFlush(DocumentsWriter.java:422)
          [junit]     at org.apache.lucene.index.DocumentsWriter.flushAllThreads(DocumentsWriter.java:553)
          [junit]     at org.apache.lucene.index.IndexWriter.doFlush(IndexWriter.java:2640)
          [junit]     at org.apache.lucene.index.IndexWriter.flush(IndexWriter.java:2616)
          [junit]     at org.apache.lucene.index.IndexWriter.closeInternal(IndexWriter.java:851)
          [junit]     at org.apache.lucene.index.IndexWriter.close(IndexWriter.java:810)
          [junit]     at org.apache.lucene.index.IndexWriter.close(IndexWriter.java:774)
          [junit]     at org.apache.lucene.index.TestIndexWriterExceptions.testIllegalPositions(TestIndexWriterExceptions.java:1517)
          [junit]     at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          [junit]     at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
          [junit]     at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
          [junit]     at java.lang.reflect.Method.invoke(Method.java:597)
          [junit]     at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:45)
          [junit]     at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
          [junit]     at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:42)
          [junit]     at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
          [junit]     at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
          [junit]     at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:30)
          [junit]     at org.apache.lucene.util.LuceneTestCase$SubclassSetupTeardownRule$1.evaluate(LuceneTestCase.java:729)
          [junit]     at org.apache.lucene.util.LuceneTestCase$InternalSetupTeardownRule$1.evaluate(LuceneTestCase.java:645)
          [junit]     at org.apache.lucene.util.SystemPropertiesInvariantRule$1.evaluate(SystemPropertiesInvariantRule.java:22)
          [junit]     at org.apache.lucene.util.LuceneTestCase$TestResultInterceptorRule$1.evaluate(LuceneTestCase.java:556)
          [junit]     at org.apache.lucene.util.UncaughtExceptionsRule$1.evaluate(UncaughtExceptionsRule.java:51)
          [junit]     at org.apache.lucene.util.LuceneTestCase$RememberThreadRule$1.evaluate(LuceneTestCase.java:618)
          [junit]     at org.junit.rules.RunRules.evaluate(RunRules.java:18)
          [junit]     at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:263)
          [junit]     at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:68)
          [junit]     at org.apache.lucene.util.LuceneTestCaseRunner.runChild(LuceneTestCaseRunner.java:164)
          [junit]     at org.apache.lucene.util.LuceneTestCaseRunner.runChild(LuceneTestCaseRunner.java:57)
          [junit]     at org.junit.runners.ParentRunner$3.run(ParentRunner.java:231)
          [junit]     at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:60)
          [junit]     at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:229)
          [junit]     at org.junit.runners.ParentRunner.access$000(ParentRunner.java:50)
          [junit]     at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:222)
          [junit]     at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
          [junit]     at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:30)
          [junit]     at org.apache.lucene.util.UncaughtExceptionsRule$1.evaluate(UncaughtExceptionsRule.java:51)
          [junit]     at org.apache.lucene.util.StoreClassNameRule$1.evaluate(StoreClassNameRule.java:21)
          [junit]     at org.apache.lucene.util.SystemPropertiesInvariantRule$1.evaluate(SystemPropertiesInvariantRule.java:22)
          [junit]     at org.junit.rules.RunRules.evaluate(RunRules.java:18)
          [junit]     at org.junit.runners.ParentRunner.run(ParentRunner.java:300)
          [junit]     at junit.framework.JUnit4TestAdapter.run(JUnit4TestAdapter.java:39)
          [junit]     at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.run(JUnitTestRunner.java:518)
          [junit]     at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.launch(JUnitTestRunner.java:1052)
          [junit]     at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.main(JUnitTestRunner.java:906)
          [junit] 
      
      1. LUCENE-3876_test.patch
        1.0 kB
        Robert Muir
      2. LUCENE-3876.patch
        2 kB
        Robert Muir

        Activity

        Hide
        Simon Willnauer added a comment -

        this seems to be related to LUCENE-3874. The patch in LUCENE-3874 isn't actually working all the time since on very large positions is > Int.MAX >> 1 we can easily shift a leading 1 in FreqProxTermsWriterPerField#writeProx makeing the position value negative. I think we need to do something like:

        --- a/lucene/core/src/java/org/apache/lucene/index/DocInverterPerField.java
        +++ b/lucene/core/src/java/org/apache/lucene/index/DocInverterPerField.java
        @@ -110,9 +110,9 @@ final class DocInverterPerField extends DocFieldConsumerPerField {
         
                     final int posIncr = posIncrAttribute.getPositionIncrement();
                     int position = fieldState.position + posIncr;
        -            if (position > 0) {
        +            if (position > 0 && position <= (Integer.MAX_VALUE>>1)) {
                       position--;
        -            } else if (position < 0) {
        +            } else if (position != 0) {
                       throw new IllegalArgumentException("position overflow for field '" + field.name() + "'");
                     }
        

        to make sure we have a leading 0 or rather two leading 0's to prevent the overflow.

        Show
        Simon Willnauer added a comment - this seems to be related to LUCENE-3874 . The patch in LUCENE-3874 isn't actually working all the time since on very large positions is > Int.MAX >> 1 we can easily shift a leading 1 in FreqProxTermsWriterPerField#writeProx makeing the position value negative. I think we need to do something like: --- a/lucene/core/src/java/org/apache/lucene/index/DocInverterPerField.java +++ b/lucene/core/src/java/org/apache/lucene/index/DocInverterPerField.java @@ -110,9 +110,9 @@ final class DocInverterPerField extends DocFieldConsumerPerField { final int posIncr = posIncrAttribute.getPositionIncrement(); int position = fieldState.position + posIncr; - if (position > 0) { + if (position > 0 && position <= ( Integer .MAX_VALUE>>1)) { position--; - } else if (position < 0) { + } else if (position != 0) { throw new IllegalArgumentException( "position overflow for field '" + field.name() + "'" ); } to make sure we have a leading 0 or rather two leading 0's to prevent the overflow.
        Hide
        Michael McCandless added a comment -

        Hmm I think we need a separate check in FreqProxTermsWriterPerField?

        Ie, that class is "private" to the indexing chain; it's a like a codec, that's used to buffer postings in RAM until we write them to the "real" codec, and in theory an app could swap in a different indexing chain that didn't steal a bit from the posDelta...

        Show
        Michael McCandless added a comment - Hmm I think we need a separate check in FreqProxTermsWriterPerField? Ie, that class is "private" to the indexing chain; it's a like a codec, that's used to buffer postings in RAM until we write them to the "real" codec, and in theory an app could swap in a different indexing chain that didn't steal a bit from the posDelta...
        Hide
        Simon Willnauer added a comment -

        Hmm I think we need a separate check in FreqProxTermsWriterPerField?

        yeah I agree. I just made this patch up to show the problem though!

        Show
        Simon Willnauer added a comment - Hmm I think we need a separate check in FreqProxTermsWriterPerField? yeah I agree. I just made this patch up to show the problem though!
        Hide
        Robert Muir added a comment -

        it shoudl check exactly at the point before shifts any bits: and the exception should be UOE

        Show
        Robert Muir added a comment - it shoudl check exactly at the point before shifts any bits: and the exception should be UOE
        Hide
        Robert Muir added a comment -

        By the way, the reason this doesnt fail always but only for certain codecs:

        some codecs have assertions that get tripped, so they fail the test.
        other codecs don't have these asserts, so they pass the test, and checkindex happens to pass.

        but this is only because checkindex ignores deleted docs in testPostings, the index really is corrumpt in those cases!

        attached is a new test demonstrating this: for some codecs it triggers an assert, for others it makes a corrumpt index. I havent tested this yet on 3.x but i suspect it fails!

        Show
        Robert Muir added a comment - By the way, the reason this doesnt fail always but only for certain codecs: some codecs have assertions that get tripped, so they fail the test. other codecs don't have these asserts, so they pass the test, and checkindex happens to pass. but this is only because checkindex ignores deleted docs in testPostings, the index really is corrumpt in those cases! attached is a new test demonstrating this: for some codecs it triggers an assert, for others it makes a corrumpt index. I havent tested this yet on 3.x but i suspect it fails!
        Hide
        Robert Muir added a comment -

        See my comment and test (which produces a corrumpt index on 3.x)

        The fact this test doesnt fail on 3.x is a bad thing

        Show
        Robert Muir added a comment - See my comment and test (which produces a corrumpt index on 3.x) The fact this test doesnt fail on 3.x is a bad thing
        Hide
        Robert Muir added a comment -

        attached is a fix. I want to commit soon.

        We just used the wrong shift. Our sign bit is free here to steal for payloads. So we don't need to limit positions to Integer.MAX_VALUE/2

        Show
        Robert Muir added a comment - attached is a fix. I want to commit soon. We just used the wrong shift. Our sign bit is free here to steal for payloads. So we don't need to limit positions to Integer.MAX_VALUE/2
        Hide
        Michael McCandless added a comment -

        +1

        Show
        Michael McCandless added a comment - +1
        Hide
        Robert Muir added a comment -

        thanks everyone!

        Show
        Robert Muir added a comment - thanks everyone!

          People

          • Assignee:
            Unassigned
            Reporter:
            Dawid Weiss
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development