You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by da...@apache.org on 2018/12/07 09:57:51 UTC

[36/37] lucene-solr:jira/http2: LUCENE-8595: Fix interleaved DV update and reset

LUCENE-8595: Fix interleaved DV update and reset

This change fixes a bug where interleaved update and reset value
to the same doc in the same updates package looses an update
if the reset comes before the update as well as loosing the reset
if the update comes frist.

Co-authored-by: Adrien Grand <jp...@gmail.com>


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/f0db05b3
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/f0db05b3
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/f0db05b3

Branch: refs/heads/jira/http2
Commit: f0db05b37ec5bda5edd35fcb0fb94c4b4a0d9089
Parents: aaa64d7
Author: Simon Willnauer <si...@apache.org>
Authored: Thu Dec 6 21:27:13 2018 +0100
Committer: Simon Willnauer <si...@apache.org>
Committed: Fri Dec 7 08:24:52 2018 +0100

----------------------------------------------------------------------
 lucene/CHANGES.txt                              |  4 ++
 .../lucene/index/DocValuesFieldUpdates.java     | 10 +++-
 .../lucene/index/TestDocValuesFieldUpdates.java | 63 ++++++++++++++++++++
 3 files changed, 74 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f0db05b3/lucene/CHANGES.txt
----------------------------------------------------------------------
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 51b831d..14b4961 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -312,6 +312,10 @@ Bug fixes
 * LUCENE-8586: Intervals.or() could get stuck in an infinite loop on certain indexes
   (Alan Woodward)
 
+* LUCENE-8595: Fix interleaved DV update and reset. Interleaved update and reset value
+  to the same doc in the same updates package looses an update if the reset comes before
+  the update as well as loosing the reset if the update comes frist. (Simon Willnauer, Adrien Grant)
+
 New Features
 
 * LUCENE-8496: Selective indexing - modify BKDReader/BKDWriter to allow users

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f0db05b3/lucene/core/src/java/org/apache/lucene/index/DocValuesFieldUpdates.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/index/DocValuesFieldUpdates.java b/lucene/core/src/java/org/apache/lucene/index/DocValuesFieldUpdates.java
index 9bf9179..7c2c5b1 100644
--- a/lucene/core/src/java/org/apache/lucene/index/DocValuesFieldUpdates.java
+++ b/lucene/core/src/java/org/apache/lucene/index/DocValuesFieldUpdates.java
@@ -298,7 +298,7 @@ abstract class DocValuesFieldUpdates implements Accountable {
         // increasing docID order:
         // NOTE: we can have ties here, when the same docID was updated in the same segment, in which case we rely on sort being
         // stable and preserving original order so the last update to that docID wins
-        return Long.compare(docs.get(i), docs.get(j));
+        return Long.compare(docs.get(i)>>>1, docs.get(j)>>>1);
       }
     }.sort(0, size);
   }
@@ -392,9 +392,13 @@ abstract class DocValuesFieldUpdates implements Accountable {
       }
       long longDoc = docs.get(idx);
       ++idx;
-      while (idx < size && docs.get(idx) == longDoc) {
+      for (; idx < size; idx++) {
         // scan forward to last update to this doc
-        ++idx;
+        final long nextLongDoc = docs.get(idx);
+        if ((longDoc >>> 1) != (nextLongDoc >>> 1)) {
+          break;
+        }
+        longDoc = nextLongDoc;
       }
       hasValue = (longDoc & HAS_VALUE_MASK) >  0;
       if (hasValue) {

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f0db05b3/lucene/core/src/test/org/apache/lucene/index/TestDocValuesFieldUpdates.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/test/org/apache/lucene/index/TestDocValuesFieldUpdates.java b/lucene/core/src/test/org/apache/lucene/index/TestDocValuesFieldUpdates.java
index 2066dcd..6c60df3 100644
--- a/lucene/core/src/test/org/apache/lucene/index/TestDocValuesFieldUpdates.java
+++ b/lucene/core/src/test/org/apache/lucene/index/TestDocValuesFieldUpdates.java
@@ -69,4 +69,67 @@ public class TestDocValuesFieldUpdates extends LuceneTestCase {
     assertEquals(24, iterator.longValue());
     assertEquals(DocIdSetIterator.NO_MORE_DOCS, iterator.nextDoc());
   }
+
+  public void testUpdateAndResetSameDoc() {
+    NumericDocValuesFieldUpdates updates = new NumericDocValuesFieldUpdates(0, "test", 2);
+    updates.add(0, 1);
+    updates.reset(0);
+    updates.finish();
+    NumericDocValuesFieldUpdates.Iterator iterator = updates.iterator();
+    assertEquals(0, iterator.nextDoc());
+    assertFalse(iterator.hasValue());
+    assertEquals(DocIdSetIterator.NO_MORE_DOCS, iterator.nextDoc());
+  }
+
+  public void testUpdateAndResetUpdateSameDoc() {
+    NumericDocValuesFieldUpdates updates = new NumericDocValuesFieldUpdates(0, "test", 3);
+    updates.add(0, 1);
+    updates.add(0);
+    updates.add(0, 2);
+    updates.finish();
+    NumericDocValuesFieldUpdates.Iterator iterator = updates.iterator();
+    assertEquals(0, iterator.nextDoc());
+    assertTrue(iterator.hasValue());
+    assertEquals(2, iterator.longValue());
+    assertEquals(DocIdSetIterator.NO_MORE_DOCS, iterator.nextDoc());
+  }
+
+  public void testUpdatesAndResetRandom() {
+    NumericDocValuesFieldUpdates updates = new NumericDocValuesFieldUpdates(0, "test", 10);
+    int numUpdates = 10 + random().nextInt(100);
+    Integer[] values = new Integer[5];
+    for (int i = 0; i < 5; i++) {
+      values[i] = random().nextBoolean() ? null : random().nextInt(100);
+      if (values[i] == null) {
+        updates.reset(i);
+      } else {
+        updates.add(i, values[i]);
+      }
+    }
+    for (int i = 0; i < numUpdates; i++) {
+      int docId = random().nextInt(5);
+      values[docId] = random().nextBoolean() ? null : random().nextInt(100);
+      if (values[docId] == null) {
+        updates.reset(docId);
+      } else {
+        updates.add(docId, values[docId]);
+      }
+    }
+
+    updates.finish();
+    NumericDocValuesFieldUpdates.Iterator iterator = updates.iterator();
+    int idx = 0;
+    while (iterator.nextDoc() != DocIdSetIterator.NO_MORE_DOCS) {
+      assertEquals(idx, iterator.docID());
+      if (values[idx] == null) {
+        assertFalse(iterator.hasValue());
+      } else {
+        assertTrue(iterator.hasValue());
+        assertEquals(values[idx].longValue(), iterator.longValue());
+      }
+      idx++;
+    }
+
+
+  }
 }