Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.0, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      I hit this while working on on LUCENE-6005: when cutting over TestNumericDocValuesUpdates to the new Document2 API, I accidentally enabled additional docValues in the test, and this this:

      There was 1 failure:
      1) testUpdateSegmentWithNoDocValues(org.apache.lucene.index.TestNumericDocValuesUpdates)
      java.io.FileNotFoundException: _1_Asserting_0.dvm in dir=RAMDirectory@259847e5 lockFactory=org.apache.lucene.store.SingleInstanceLockFactory@30981eab
      	at __randomizedtesting.SeedInfo.seed([0:7C88A439A551C47D]:0)
      	at org.apache.lucene.store.MockDirectoryWrapper.openInput(MockDirectoryWrapper.java:645)
      	at org.apache.lucene.store.Directory.openChecksumInput(Directory.java:110)
      	at org.apache.lucene.codecs.lucene50.Lucene50DocValuesProducer.<init>(Lucene50DocValuesProducer.java:130)
      	at org.apache.lucene.codecs.lucene50.Lucene50DocValuesFormat.fieldsProducer(Lucene50DocValuesFormat.java:182)
      	at org.apache.lucene.codecs.asserting.AssertingDocValuesFormat.fieldsProducer(AssertingDocValuesFormat.java:66)
      	at org.apache.lucene.codecs.perfield.PerFieldDocValuesFormat$FieldsReader.<init>(PerFieldDocValuesFormat.java:267)
      	at org.apache.lucene.codecs.perfield.PerFieldDocValuesFormat.fieldsProducer(PerFieldDocValuesFormat.java:357)
      	at org.apache.lucene.index.SegmentDocValues.newDocValuesProducer(SegmentDocValues.java:51)
      	at org.apache.lucene.index.SegmentDocValues.getDocValuesProducer(SegmentDocValues.java:68)
      	at org.apache.lucene.index.SegmentDocValuesProducer.<init>(SegmentDocValuesProducer.java:63)
      	at org.apache.lucene.index.SegmentReader.initDocValuesProducer(SegmentReader.java:167)
      	at org.apache.lucene.index.SegmentReader.<init>(SegmentReader.java:109)
      	at org.apache.lucene.index.StandardDirectoryReader$1.doBody(StandardDirectoryReader.java:58)
      	at org.apache.lucene.index.StandardDirectoryReader$1.doBody(StandardDirectoryReader.java:50)
      	at org.apache.lucene.index.SegmentInfos$FindSegmentsFile.run(SegmentInfos.java:556)
      	at org.apache.lucene.index.StandardDirectoryReader.open(StandardDirectoryReader.java:50)
      	at org.apache.lucene.index.DirectoryReader.open(DirectoryReader.java:63)
      	at org.apache.lucene.index.TestNumericDocValuesUpdates.testUpdateSegmentWithNoDocValues(TestNumericDocValuesUpdates.java:769)
      

      A one-line change to the existing test (on trunk) causes this corruption:

      Index: lucene/core/src/test/org/apache/lucene/index/TestNumericDocValuesUpdates.java
      ===================================================================
      --- lucene/core/src/test/org/apache/lucene/index/TestNumericDocValuesUpdates.java	(revision 1639580)
      +++ lucene/core/src/test/org/apache/lucene/index/TestNumericDocValuesUpdates.java	(working copy)
      @@ -750,6 +750,7 @@
           // second segment with no NDV
           doc = new Document();
           doc.add(new StringField("id", "doc1", Store.NO));
      +    doc.add(new NumericDocValuesField("foo", 3));
           writer.addDocument(doc);
           doc = new Document();
           doc.add(new StringField("id", "doc2", Store.NO)); // document that isn't updated
      

      For some reason, the base doc values for the 2nd segment is not being written, but clearly should have (to hold field "foo")... I'm not sure why.

      1. LUCENE-6062.patch
        11 kB
        Robert Muir
      2. LUCENE-6062.patch
        3 kB
        Robert Muir

        Activity

        Hide
        Michael McCandless added a comment -

        I haven't tried but I think it's likely this affects binary DV updates as well.

        Show
        Michael McCandless added a comment - I haven't tried but I think it's likely this affects binary DV updates as well.
        Hide
        Shai Erera added a comment -

        I found the problem. With your change to the test, you created the following scenario: update a non-existing NDV field in a segment with other NDV fields (note that without this change, the test ensures that you can update a non-existing NDV fields in a segment without any other NDV fields).

        What happens is that in this code of SegmentDocValuesProducer:

                  if (baseProducer == null) {
                    // the base producer gets all the fields, so the Codec can validate properly
                    baseProducer = segDocValues.getDocValuesProducer(docValuesGen, si, IOContext.READ, dir, dvFormat, fieldInfos);
                    dvGens.add(docValuesGen);
                    dvProducers.add(baseProducer);
                  }
        

        We pass all the fieldInfos, which now also contain an FI for 'ndv'. But that field was never written to the base segment file (the .cfs), and so it cannot be found there...

        Not yet sure how to resolve it. We pass all the FIS because e.g. Lucene50DVP verifies that every field it encounters in the metadata file has a matching entry in the given FieldInfos (to check for index corruption). So we cannot just pass only the FIs with dvGen=-1. On the other hand, we do have a case here where the base .cfs never had an instance of that field ... it's like we need to know in which 'gen' a DV field was introduced. Then we can pass to baseProducer all the FIs whose startGen==-1...

        Show
        Shai Erera added a comment - I found the problem. With your change to the test, you created the following scenario: update a non-existing NDV field in a segment with other NDV fields (note that without this change, the test ensures that you can update a non-existing NDV fields in a segment without any other NDV fields). What happens is that in this code of SegmentDocValuesProducer: if (baseProducer == null ) { // the base producer gets all the fields, so the Codec can validate properly baseProducer = segDocValues.getDocValuesProducer(docValuesGen, si, IOContext.READ, dir, dvFormat, fieldInfos); dvGens.add(docValuesGen); dvProducers.add(baseProducer); } We pass all the fieldInfos, which now also contain an FI for 'ndv'. But that field was never written to the base segment file (the .cfs), and so it cannot be found there... Not yet sure how to resolve it. We pass all the FIS because e.g. Lucene50DVP verifies that every field it encounters in the metadata file has a matching entry in the given FieldInfos (to check for index corruption). So we cannot just pass only the FIs with dvGen=-1. On the other hand, we do have a case here where the base .cfs never had an instance of that field ... it's like we need to know in which 'gen' a DV field was introduced. Then we can pass to baseProducer all the FIs whose startGen==-1...
        Hide
        Shai Erera added a comment -

        To prove this is the problem, I added this to PerFieldDVF.FieldsReader ctor (line 256):

                    if (readState.segmentInfo.name.equals("_1") && fieldName.equals("ndv") && fi.getDocValuesGen() == 1 && readState.fieldInfos.size() > 1) {
                      continue;
                    }
        

        And the test passes. So e.g. if we tracked startDVGen for each field, we'd know in segment _1 that 'ndv' only appeared in gen=1, and therefore not pass it at all to baseProducer. But that causes a format change, which I hope to avoid if possible (especially as it doesn't solve the issue for existing indexes, though I think this is an extreme case for somebody to have run into).

        Show
        Shai Erera added a comment - To prove this is the problem, I added this to PerFieldDVF.FieldsReader ctor (line 256): if (readState.segmentInfo.name.equals( "_1" ) && fieldName.equals( "ndv" ) && fi.getDocValuesGen() == 1 && readState.fieldInfos.size() > 1) { continue ; } And the test passes. So e.g. if we tracked startDVGen for each field, we'd know in segment _1 that 'ndv' only appeared in gen=1, and therefore not pass it at all to baseProducer. But that causes a format change, which I hope to avoid if possible (especially as it doesn't solve the issue for existing indexes, though I think this is an extreme case for somebody to have run into).
        Hide
        Robert Muir added a comment -

        I think its a little simpler... here is a patch.

        We should always pass these producers the same metadata at read that their corresponding consumer saw at write.

        Show
        Robert Muir added a comment - I think its a little simpler... here is a patch. We should always pass these producers the same metadata at read that their corresponding consumer saw at write.
        Hide
        Robert Muir added a comment -

        Updated patch with a dedicated test (keeping the old one as it was). The logic is the same as the first patch, but i tried to clarify better what is going on. Additionally, i removed some extraneous parameters in some of the related methods.

        Show
        Robert Muir added a comment - Updated patch with a dedicated test (keeping the old one as it was). The logic is the same as the first patch, but i tried to clarify better what is going on. Additionally, i removed some extraneous parameters in some of the related methods.
        Hide
        Michael McCandless added a comment -

        +1

        Show
        Michael McCandless added a comment - +1
        Hide
        ASF subversion and git services added a comment -

        Commit 1640464 from Robert Muir in branch 'dev/trunk'
        [ https://svn.apache.org/r1640464 ]

        LUCENE-6062: throw exception instead of doing nothing, when sorting/grouping etc on misconfigured field

        Show
        ASF subversion and git services added a comment - Commit 1640464 from Robert Muir in branch 'dev/trunk' [ https://svn.apache.org/r1640464 ] LUCENE-6062 : throw exception instead of doing nothing, when sorting/grouping etc on misconfigured field
        Hide
        Robert Muir added a comment -

        I will first go back to 5.x, then see if the test fails in 4.x, and how feasible it is to backport.

        The code differs significantly here so the problem may have been recently introduced.

        Show
        Robert Muir added a comment - I will first go back to 5.x, then see if the test fails in 4.x, and how feasible it is to backport. The code differs significantly here so the problem may have been recently introduced.
        Hide
        ASF subversion and git services added a comment -

        Commit 1640471 from Robert Muir in branch 'dev/trunk'
        [ https://svn.apache.org/r1640471 ]

        LUCENE-6062: pass correct fieldinfos to dv producer when the segment has updates

        Show
        ASF subversion and git services added a comment - Commit 1640471 from Robert Muir in branch 'dev/trunk' [ https://svn.apache.org/r1640471 ] LUCENE-6062 : pass correct fieldinfos to dv producer when the segment has updates
        Hide
        ASF subversion and git services added a comment -

        Commit 1640472 from Robert Muir in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1640472 ]

        LUCENE-6062: pass correct fieldinfos to dv producer when the segment has updates

        Show
        ASF subversion and git services added a comment - Commit 1640472 from Robert Muir in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1640472 ] LUCENE-6062 : pass correct fieldinfos to dv producer when the segment has updates
        Hide
        Robert Muir added a comment -

        The bug affects 4.10.x, but the fix would not be easy. On 5.0 fieldinfos handling has been simplified considerably around here, making it easy to pass the correct ones to producers.

        I think this is too much risk to backport.

        Show
        Robert Muir added a comment - The bug affects 4.10.x, but the fix would not be easy. On 5.0 fieldinfos handling has been simplified considerably around here, making it easy to pass the correct ones to producers. I think this is too much risk to backport.
        Hide
        ASF subversion and git services added a comment -

        Commit 1640473 from Robert Muir in branch 'dev/trunk'
        [ https://svn.apache.org/r1640473 ]

        LUCENE-6062: add CHANGES for bugfix

        Show
        ASF subversion and git services added a comment - Commit 1640473 from Robert Muir in branch 'dev/trunk' [ https://svn.apache.org/r1640473 ] LUCENE-6062 : add CHANGES for bugfix
        Hide
        ASF subversion and git services added a comment -

        Commit 1640474 from Robert Muir in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1640474 ]

        LUCENE-6062: add CHANGES for bugfix

        Show
        ASF subversion and git services added a comment - Commit 1640474 from Robert Muir in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1640474 ] LUCENE-6062 : add CHANGES for bugfix
        Hide
        Shai Erera added a comment -

        Thanks Rob for committing this. Patch looks good. Sorry for the late response.

        Show
        Shai Erera added a comment - Thanks Rob for committing this. Patch looks good. Sorry for the late response.
        Hide
        Anshum Gupta added a comment -

        Bulk close after 5.0 release.

        Show
        Anshum Gupta added a comment - Bulk close after 5.0 release.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development