lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sim...@apache.org
Subject [3/3] lucene-solr:master: LUCENE-8233: Harden PendingDeletes modification assertions
Date Thu, 12 Apr 2018 10:45:15 GMT
LUCENE-8233: Harden PendingDeletes modification assertions

This change adds a missing call to PendingDeletes#onNewReader and
hardens the assertion when a PendingDelete can actually be modified ie.
receive deletes and updates. Now PendingDeltes are also initialized
when no reader is provided but the SegmentCommitInfo has evidence that there
is are no deletes.


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

Branch: refs/heads/master
Commit: 9cb07c735b2095184041280bf03b3df9bfa010a1
Parents: 017f59b
Author: Simon Willnauer <simonw@apache.org>
Authored: Thu Apr 12 11:35:15 2018 +0200
Committer: Simon Willnauer <simonw@apache.org>
Committed: Thu Apr 12 12:44:34 2018 +0200

----------------------------------------------------------------------
 .../org/apache/lucene/index/PendingDeletes.java |  8 ++++++-
 .../apache/lucene/index/PendingSoftDeletes.java |  3 ++-
 .../apache/lucene/index/ReadersAndUpdates.java  |  1 +
 .../apache/lucene/index/TestPendingDeletes.java |  8 +++----
 .../lucene/index/TestPendingSoftDeletes.java    | 23 +++++++++++++++-----
 5 files changed, 31 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9cb07c73/lucene/core/src/java/org/apache/lucene/index/PendingDeletes.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/index/PendingDeletes.java b/lucene/core/src/java/org/apache/lucene/index/PendingDeletes.java
index bce704c..fca42b4 100644
--- a/lucene/core/src/java/org/apache/lucene/index/PendingDeletes.java
+++ b/lucene/core/src/java/org/apache/lucene/index/PendingDeletes.java
@@ -52,7 +52,10 @@ class PendingDeletes {
   }
 
   PendingDeletes(SegmentCommitInfo info) {
-    this(info, null, false);
+    this(info, null, info.hasDeletions() == false);
+    // if we don't have deletions we can mark it as initialized since we might receive deletes
on a segment
+    // without having a reader opened on it ie. after a merge when we apply the deletes that
IW received while merging.
+    // For segments that were published we enforce a reader in the BufferedUpdatesStream.SegmentState
ctor
   }
 
   private PendingDeletes(SegmentCommitInfo info, Bits liveDocs, boolean liveDocsInitialized)
{
@@ -65,6 +68,9 @@ class PendingDeletes {
 
 
   protected MutableBits getMutableBits() throws IOException {
+    // if we pull mutable bits but we haven't been initialized something is completely off.
+    // this means we receive deletes without having the bitset that is on-disk ready to be
cloned
+    assert liveDocsInitialized : "can't delete if liveDocs are not initialized";
     if (liveDocsShared) {
       // Copy on write: this means we've cloned a
       // SegmentReader sharing the current liveDocs

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9cb07c73/lucene/core/src/java/org/apache/lucene/index/PendingSoftDeletes.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/index/PendingSoftDeletes.java b/lucene/core/src/java/org/apache/lucene/index/PendingSoftDeletes.java
index b73ac83..0d7852b 100644
--- a/lucene/core/src/java/org/apache/lucene/index/PendingSoftDeletes.java
+++ b/lucene/core/src/java/org/apache/lucene/index/PendingSoftDeletes.java
@@ -67,7 +67,7 @@ final class PendingSoftDeletes extends PendingDeletes {
   void onNewReader(SegmentReader reader, SegmentCommitInfo info) throws IOException {
     super.onNewReader(reader, info);
     hardDeletes.onNewReader(reader, info);
-    if (dvGeneration != info.getDocValuesGen()) { // only re-calculate this if we haven't
seen this generation
+    if (dvGeneration < info.getDocValuesGen()) { // only re-calculate this if we haven't
seen this generation
       final DocIdSetIterator iterator = DocValuesFieldExistsQuery.getDocValuesDocIdSetIterator(field,
reader);
       if (iterator == null) { // nothing is deleted we don't have a soft deletes field in
this segment
         this.pendingDeleteCount = 0;
@@ -120,6 +120,7 @@ final class PendingSoftDeletes extends PendingDeletes {
   void onDocValuesUpdate(FieldInfo info, List<DocValuesFieldUpdates> updatesToApply)
throws IOException {
     if (field.equals(info.name)) {
       assert dvGeneration < info.getDocValuesGen() : "we have seen this generation update
already: " + dvGeneration + " vs. " + info.getDocValuesGen();
+      assert dvGeneration != -2 : "docValues generation is still uninitialized";
       DocValuesFieldUpdates.Iterator[] subs = new DocValuesFieldUpdates.Iterator[updatesToApply.size()];
       for(int i=0; i<subs.length; i++) {
         subs[i] = updatesToApply.get(i).iterator();

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9cb07c73/lucene/core/src/java/org/apache/lucene/index/ReadersAndUpdates.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/index/ReadersAndUpdates.java b/lucene/core/src/java/org/apache/lucene/index/ReadersAndUpdates.java
index 4851d45..6dc8864 100644
--- a/lucene/core/src/java/org/apache/lucene/index/ReadersAndUpdates.java
+++ b/lucene/core/src/java/org/apache/lucene/index/ReadersAndUpdates.java
@@ -619,6 +619,7 @@ final class ReadersAndUpdates {
       final SegmentReader reader;
       if (this.reader == null) {
         reader = new SegmentReader(info, indexCreatedVersionMajor, IOContext.READONCE);
+        pendingDeletes.onNewReader(reader, info);
       } else {
         reader = this.reader;
       }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9cb07c73/lucene/core/src/test/org/apache/lucene/index/TestPendingDeletes.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/test/org/apache/lucene/index/TestPendingDeletes.java b/lucene/core/src/test/org/apache/lucene/index/TestPendingDeletes.java
index e150e06..bbe309a 100644
--- a/lucene/core/src/test/org/apache/lucene/index/TestPendingDeletes.java
+++ b/lucene/core/src/test/org/apache/lucene/index/TestPendingDeletes.java
@@ -40,7 +40,7 @@ public class TestPendingDeletes extends LuceneTestCase {
     RAMDirectory dir = new RAMDirectory();
     SegmentInfo si = new SegmentInfo(dir, Version.LATEST, Version.LATEST, "test", 10, false,
Codec.getDefault(),
         Collections.emptyMap(), StringHelper.randomId(), new HashMap<>(), null);
-    SegmentCommitInfo commitInfo = new SegmentCommitInfo(si, 0, 0, 0, 0);
+    SegmentCommitInfo commitInfo = new SegmentCommitInfo(si, 0, -1, -1, -1);
     PendingDeletes deletes = newPendingDeletes(commitInfo);
     assertNull(deletes.getLiveDocs());
     int docToDelete = TestUtil.nextInt(random(), 0, 7);
@@ -76,7 +76,7 @@ public class TestPendingDeletes extends LuceneTestCase {
     RAMDirectory dir = new RAMDirectory();
     SegmentInfo si = new SegmentInfo(dir, Version.LATEST, Version.LATEST, "test", 6, false,
Codec.getDefault(),
         Collections.emptyMap(), StringHelper.randomId(), new HashMap<>(), null);
-    SegmentCommitInfo commitInfo = new SegmentCommitInfo(si, 0, 0, 0, 0);
+    SegmentCommitInfo commitInfo = new SegmentCommitInfo(si, 0, -1, -1, -1);
     PendingDeletes deletes = newPendingDeletes(commitInfo);
     assertFalse(deletes.writeLiveDocs(dir));
     assertEquals(0, dir.listAll().length);
@@ -86,7 +86,7 @@ public class TestPendingDeletes extends LuceneTestCase {
       deletes.liveDocsShared();
       deletes.delete(2);
     }
-    assertEquals(0, commitInfo.getDelGen());
+    assertEquals(-1, commitInfo.getDelGen());
     assertEquals(0, commitInfo.getDelCount());
 
     assertEquals(secondDocDeletes ? 2 : 1, deletes.numPendingDeletes());
@@ -133,7 +133,7 @@ public class TestPendingDeletes extends LuceneTestCase {
     RAMDirectory dir = new RAMDirectory();
     SegmentInfo si = new SegmentInfo(dir, Version.LATEST, Version.LATEST, "test", 3, false,
Codec.getDefault(),
         Collections.emptyMap(), StringHelper.randomId(), new HashMap<>(), null);
-    SegmentCommitInfo commitInfo = new SegmentCommitInfo(si, 0, 0, 0, 0);
+    SegmentCommitInfo commitInfo = new SegmentCommitInfo(si, 0, -1, -1, -1);
     PendingDeletes deletes = newPendingDeletes(commitInfo);
     for (int i = 0; i < 3; i++) {
       assertTrue(deletes.delete(i));

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9cb07c73/lucene/core/src/test/org/apache/lucene/index/TestPendingSoftDeletes.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/test/org/apache/lucene/index/TestPendingSoftDeletes.java b/lucene/core/src/test/org/apache/lucene/index/TestPendingSoftDeletes.java
index c428a4b..255ff9e 100644
--- a/lucene/core/src/test/org/apache/lucene/index/TestPendingSoftDeletes.java
+++ b/lucene/core/src/test/org/apache/lucene/index/TestPendingSoftDeletes.java
@@ -45,7 +45,7 @@ public class TestPendingSoftDeletes extends TestPendingDeletes {
 
   public void testDeleteSoft() throws IOException {
     Directory dir = newDirectory();
-    IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig()); // no soft delete
field hier
+    IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig()); // no soft delete
field here
     Document doc = new Document();
     doc.add(new StringField("id", "1", Field.Store.YES));
     writer.softUpdateDocument(new Term("id", "1"), doc,
@@ -97,8 +97,20 @@ public class TestPendingSoftDeletes extends TestPendingDeletes {
     RAMDirectory dir = new RAMDirectory();
     SegmentInfo si = new SegmentInfo(dir, Version.LATEST, Version.LATEST, "test", 10, false,
Codec.getDefault(),
         Collections.emptyMap(), StringHelper.randomId(), new HashMap<>(), null);
-    SegmentCommitInfo commitInfo = new SegmentCommitInfo(si, 0, 0, 0, 0);
+    SegmentCommitInfo commitInfo = new SegmentCommitInfo(si, 0, -1, -1, -1);
+    IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig());
+    for (int i = 0; i < si.maxDoc(); i++) {
+      writer.addDocument(new Document());
+    }
+    writer.forceMerge(1);
+    writer.commit();
+    DirectoryReader reader = writer.getReader();
+    assertEquals(1, reader.leaves().size());
+    SegmentReader segmentReader = (SegmentReader) reader.leaves().get(0).reader();
     PendingSoftDeletes deletes = newPendingDeletes(commitInfo);
+    deletes.onNewReader(segmentReader, commitInfo);
+    reader.close();
+    writer.close();
     FieldInfo fieldInfo = new FieldInfo("_soft_deletes", 1, false, false, false, IndexOptions.NONE,
DocValuesType.NUMERIC, 0, Collections.emptyMap(), 0, 0);
     List<Integer> docsDeleted = Arrays.asList(1, 3, 7, 8, DocIdSetIterator.NO_MORE_DOCS);
     List<DocValuesFieldUpdates> updates = Arrays.asList(singleUpdate(docsDeleted, 10));
@@ -152,10 +164,9 @@ public class TestPendingSoftDeletes extends TestPendingDeletes {
     assertEquals(1, reader.leaves().size());
     SegmentReader segmentReader = (SegmentReader) reader.leaves().get(0).reader();
     SegmentCommitInfo segmentInfo = segmentReader.getSegmentInfo();
-    SegmentInfo si = new SegmentInfo(dir, Version.LATEST, Version.LATEST, "test", 3, false,
Codec.getDefault(),
-        Collections.emptyMap(), StringHelper.randomId(), new HashMap<>(), null);
-    PendingSoftDeletes deletes = newPendingDeletes(segmentInfo);
-    FieldInfo fieldInfo = new FieldInfo("_soft_deletes", 1, false, false, false, IndexOptions.NONE,
DocValuesType.NUMERIC, segmentInfo.getDocValuesGen(), Collections.emptyMap(), 0, 0);
+    PendingDeletes deletes = newPendingDeletes(segmentInfo);
+    deletes.onNewReader(segmentReader, segmentInfo);
+    FieldInfo fieldInfo = new FieldInfo("_soft_deletes", 1, false, false, false, IndexOptions.NONE,
DocValuesType.NUMERIC, segmentInfo.getNextDocValuesGen(), Collections.emptyMap(), 0, 0);
     List<Integer> docsDeleted = Arrays.asList(1, DocIdSetIterator.NO_MORE_DOCS);
     List<DocValuesFieldUpdates> updates = Arrays.asList(singleUpdate(docsDeleted, 3));
     deletes.onDocValuesUpdate(fieldInfo, updates);


Mime
View raw message