lucene-java-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mikemcc...@apache.org
Subject svn commit: r749009 - in /lucene/java/trunk: ./ src/java/org/apache/lucene/index/ src/java/org/apache/lucene/util/ src/test/org/apache/lucene/index/
Date Sun, 01 Mar 2009 12:12:37 GMT
Author: mikemccand
Date: Sun Mar  1 12:12:36 2009
New Revision: 749009

URL: http://svn.apache.org/viewvc?rev=749009&view=rev
Log:
LUCENE-1314: change reference counting to properly track deleted docs & shared core readers
inside SegmentReader

Modified:
    lucene/java/trunk/common-build.xml
    lucene/java/trunk/src/java/org/apache/lucene/index/SegmentReader.java
    lucene/java/trunk/src/java/org/apache/lucene/util/BitVector.java
    lucene/java/trunk/src/test/org/apache/lucene/index/TestIndexReaderClone.java
    lucene/java/trunk/src/test/org/apache/lucene/index/TestIndexReaderReopen.java

Modified: lucene/java/trunk/common-build.xml
URL: http://svn.apache.org/viewvc/lucene/java/trunk/common-build.xml?rev=749009&r1=749008&r2=749009&view=diff
==============================================================================
--- lucene/java/trunk/common-build.xml (original)
+++ lucene/java/trunk/common-build.xml Sun Mar  1 12:12:36 2009
@@ -41,7 +41,7 @@
   <property name="name" value="${ant.project.name}"/>
   <property name="Name" value="Lucene"/>
   <property name="version" value="2.9-dev"/>
-  <property name="compatibility.tag" value="lucene_2_4_back_compat_tests_20090127"/>
+  <property name="compatibility.tag" value="lucene_2_4_back_compat_tests_20090301a"/>
   <property name="spec.version" value="${version}"/>	
   <property name="year" value="2000-${current.year}"/>
   <property name="final.name" value="lucene-${name}-${version}"/>

Modified: lucene/java/trunk/src/java/org/apache/lucene/index/SegmentReader.java
URL: http://svn.apache.org/viewvc/lucene/java/trunk/src/java/org/apache/lucene/index/SegmentReader.java?rev=749009&r1=749008&r2=749009&view=diff
==============================================================================
--- lucene/java/trunk/src/java/org/apache/lucene/index/SegmentReader.java (original)
+++ lucene/java/trunk/src/java/org/apache/lucene/index/SegmentReader.java Sun Mar  1 12:12:36
2009
@@ -71,14 +71,18 @@
   private IndexInput singleNormStream;
   private Ref singleNormRef;
 
+  // Counts how many other reader share the core objects
+  // (freqStream, proxStream, tis, etc.) of this reader;
+  // when coreRef drops to 0, these core objects may be
+  // closed.  A given insance of SegmentReader may be
+  // closed, even those it shares core objects with other
+  // SegmentReaders:
+  private Ref coreRef = new Ref();
+
   // Compound File Reader when based on a compound file segment
   CompoundFileReader cfsReader = null;
   CompoundFileReader storeCFSReader = null;
   
-  // indicates the SegmentReader with which the resources are being shared,
-  // in case this is a re-opened reader
-  private SegmentReader referencedSegmentReader = null;
-  
   /**
    * Sets the initial value 
    */
@@ -331,37 +335,6 @@
     }
   }
 
-  public synchronized void incRef() {
-    super.incRef();
-    Iterator it = norms.values().iterator();
-    while (it.hasNext()) {
-      ((Norm) it.next()).incRef();
-    }
-    if (deletedDocsRef != null) {
-      deletedDocsRef.incRef();
-    }
-  }
-  
-  private synchronized void incRefReaderNotNorms() {
-    super.incRef();
-  }
-  
-  public synchronized void decRef() throws IOException {
-    super.decRef();
-    Iterator it = norms.values().iterator();
-    while (it.hasNext()) {
-      ((Norm) it.next()).decRef();
-    }
-
-    if (deletedDocsRef != null) {
-      deletedDocsRef.decRef();
-    }
-  }
-  
-  private synchronized void decRefReaderNotNorms() throws IOException {
-    super.decRef();
-  }
-
   Map norms = new HashMap();
   
   /** The class which implements SegmentReader. */
@@ -688,6 +661,8 @@
 
     boolean success = false;
     try {
+      coreRef.incRef();
+      clone.coreRef = coreRef;
       clone.readOnly = openReadOnly;
       clone.directory = directory;
       clone.si = si;
@@ -706,19 +681,19 @@
         clone.fieldsReaderOrig = (FieldsReader) fieldsReaderOrig.clone();
       }      
       
-      if (deletedDocsRef != null) {
-        deletedDocsRef.incRef();
-      }
       if (doClone) {
-        clone.deletedDocs = deletedDocs;
-        clone.deletedDocsRef = deletedDocsRef;
+        if (deletedDocs != null) {
+          deletedDocsRef.incRef();
+          clone.deletedDocs = deletedDocs;
+          clone.deletedDocsRef = deletedDocsRef;
+        }
       } else {
         if (!deletionsUpToDate) {
           // load deleted docs
-          clone.deletedDocs = null;
-          clone.deletedDocsRef = null;
+          assert clone.deletedDocs == null;
           clone.loadDeletedDocs();
-        } else {
+        } else if (deletedDocs != null) {
+          deletedDocsRef.incRef();
           clone.deletedDocs = deletedDocs;
           clone.deletedDocsRef = deletedDocsRef;
         }
@@ -744,16 +719,6 @@
 
       success = true;
     } finally {
-      if (this.referencedSegmentReader != null) {
-        // This reader shares resources with another SegmentReader,
-        // so we increment the other reader's refCount.
-        clone.referencedSegmentReader = this.referencedSegmentReader;
-      } else {
-        // We are the original SegmentReader
-        clone.referencedSegmentReader = this;
-      }
-      clone.referencedSegmentReader.incRefReaderNotNorms();
-      
       if (!success) {
         // An exception occured during reopen, we have to decRef the norms
         // that we incRef'ed already and close singleNormsStream and FieldsReader
@@ -801,17 +766,21 @@
   }
   
   protected void doClose() throws IOException {
-    boolean hasReferencedReader = (referencedSegmentReader != null);
 
     termVectorsLocal.close();
     fieldsReaderLocal.close();
     
-    if (hasReferencedReader) {
-      referencedSegmentReader.decRefReaderNotNorms();
-      referencedSegmentReader = null;
+    if (deletedDocs != null) {
+      deletedDocsRef.decRef();
+    }
+
+    Iterator it = norms.values().iterator();
+    while (it.hasNext()) {
+      ((Norm) it.next()).decRef();
     }
 
-    if (!hasReferencedReader) { 
+    if (coreRef.decRef() == 0) {
+
       // close everything, nothing is shared anymore with other readers
       if (tis != null) {
         tis.close();
@@ -869,12 +838,10 @@
     // deletedDocs BitVector so decRef the current deletedDocsRef,
     // clone the BitVector, create a new deletedDocsRef
     if (deletedDocsRef.refCount() > 1) {
-      synchronized (deletedDocsRef) {
-        Ref oldRef = deletedDocsRef;
-        deletedDocs = cloneDeletedDocs(deletedDocs);
-        deletedDocsRef = new Ref();
-        oldRef.decRef();
-      }
+      Ref oldRef = deletedDocsRef;
+      deletedDocs = cloneDeletedDocs(deletedDocs);
+      deletedDocsRef = new Ref();
+      oldRef.decRef();
     }
     deletedDocsDirty = true;
     undeleteAll = false;

Modified: lucene/java/trunk/src/java/org/apache/lucene/util/BitVector.java
URL: http://svn.apache.org/viewvc/lucene/java/trunk/src/java/org/apache/lucene/util/BitVector.java?rev=749009&r1=749008&r2=749009&view=diff
==============================================================================
--- lucene/java/trunk/src/java/org/apache/lucene/util/BitVector.java (original)
+++ lucene/java/trunk/src/java/org/apache/lucene/util/BitVector.java Sun Mar  1 12:12:36 2009
@@ -47,16 +47,15 @@
     bits = new byte[(size >> 3) + 1];
   }
   
-  BitVector(byte[] bits, int size, int count) {
+  BitVector(byte[] bits, int size) {
     this.bits = bits;
     this.size = size;
-    this.count = count;
   }
   
   public Object clone() {
     byte[] copyBits = new byte[bits.length];
     System.arraycopy(bits, 0, copyBits, 0, bits.length);
-    return new BitVector(copyBits, size, count);
+    return new BitVector(copyBits, size);
   }
   
   /** Sets the value of <code>bit</code> to one. */

Modified: lucene/java/trunk/src/test/org/apache/lucene/index/TestIndexReaderClone.java
URL: http://svn.apache.org/viewvc/lucene/java/trunk/src/test/org/apache/lucene/index/TestIndexReaderClone.java?rev=749009&r1=749008&r2=749009&view=diff
==============================================================================
--- lucene/java/trunk/src/test/org/apache/lucene/index/TestIndexReaderClone.java (original)
+++ lucene/java/trunk/src/test/org/apache/lucene/index/TestIndexReaderClone.java Sun Mar 
1 12:12:36 2009
@@ -423,4 +423,21 @@
     }
     dir1.close();
   }
+
+  public void testLucene1516Bug() throws Exception {
+    final Directory dir1 = new MockRAMDirectory();
+    TestIndexReaderReopen.createIndex(dir1, false);
+    IndexReader r1 = IndexReader.open(dir1);
+    r1.incRef();
+    IndexReader r2 = (IndexReader) r1.clone(false);
+    r1.deleteDocument(5);
+    r1.decRef();
+    
+    r1.incRef();
+    
+    r2.close();
+    r1.decRef();
+    r1.close();
+    dir1.close();
+  }
 }

Modified: lucene/java/trunk/src/test/org/apache/lucene/index/TestIndexReaderReopen.java
URL: http://svn.apache.org/viewvc/lucene/java/trunk/src/test/org/apache/lucene/index/TestIndexReaderReopen.java?rev=749009&r1=749008&r2=749009&view=diff
==============================================================================
--- lucene/java/trunk/src/test/org/apache/lucene/index/TestIndexReaderReopen.java (original)
+++ lucene/java/trunk/src/test/org/apache/lucene/index/TestIndexReaderReopen.java Sun Mar
 1 12:12:36 2009
@@ -364,9 +364,11 @@
       assertEquals(subReaders0.length, subReaders1.length);
       
       for (int i = 0; i < subReaders0.length; i++) {
-        assertRefCountEquals(2, subReaders0[i]);
         if (subReaders0[i] != subReaders1[i]) {
+          assertRefCountEquals(1, subReaders0[i]);
           assertRefCountEquals(1, subReaders1[i]);
+        } else {
+          assertRefCountEquals(2, subReaders0[i]);
         }
       }
 
@@ -390,10 +392,10 @@
         } else {
           assertRefCountEquals(1, subReaders2[i]);
           if (subReaders0[i] == subReaders1[i]) {
-            assertRefCountEquals(3, subReaders2[i]);
+            assertRefCountEquals(2, subReaders2[i]);
             assertRefCountEquals(2, subReaders0[i]);
           } else {
-            assertRefCountEquals(3, subReaders0[i]);
+            assertRefCountEquals(1, subReaders0[i]);
             assertRefCountEquals(1, subReaders1[i]);
           }
         }
@@ -463,7 +465,7 @@
       
       modifyIndex(0, dir1);
       IndexReader reader2 = reader1.reopen();
-      assertRefCountEquals(3 + mode, reader1);
+      assertRefCountEquals(2 + mode, reader1);
 
       if (mode == 1) {
         initReader2.close();
@@ -471,31 +473,31 @@
       
       modifyIndex(1, dir1);
       IndexReader reader3 = reader2.reopen();
-      assertRefCountEquals(4 + mode, reader1);
+      assertRefCountEquals(2 + mode, reader1);
       assertRefCountEquals(1, reader2);
       
       multiReader1.close();
-      assertRefCountEquals(3 + mode, reader1);
+      assertRefCountEquals(1 + mode, reader1);
       
       multiReader1.close();
-      assertRefCountEquals(3 + mode, reader1);
+      assertRefCountEquals(1 + mode, reader1);
 
       if (mode == 1) {
         initReader2.close();
       }
       
       reader1.close();
-      assertRefCountEquals(3, reader1);
+      assertRefCountEquals(1, reader1);
       
       multiReader2.close();
-      assertRefCountEquals(2, reader1);
+      assertRefCountEquals(0, reader1);
       
       multiReader2.close();
-      assertRefCountEquals(2, reader1);
+      assertRefCountEquals(0, reader1);
       
       reader3.close();
-      assertRefCountEquals(1, reader1);
-      assertReaderOpen(reader1);
+      assertRefCountEquals(0, reader1);
+      assertReaderClosed(reader1, true, false);
       
       reader2.close();
       assertRefCountEquals(0, reader1);
@@ -537,7 +539,7 @@
       modifyIndex(0, dir1);
       modifyIndex(0, dir2);
       IndexReader reader2 = reader1.reopen();
-      assertRefCountEquals(3 + mode, reader1);
+      assertRefCountEquals(2 + mode, reader1);
 
       if (mode == 1) {
         initReader2.close();
@@ -545,31 +547,31 @@
       
       modifyIndex(4, dir1);
       IndexReader reader3 = reader2.reopen();
-      assertRefCountEquals(4 + mode, reader1);
+      assertRefCountEquals(2 + mode, reader1);
       assertRefCountEquals(1, reader2);
       
       parallelReader1.close();
-      assertRefCountEquals(3 + mode, reader1);
+      assertRefCountEquals(1 + mode, reader1);
       
       parallelReader1.close();
-      assertRefCountEquals(3 + mode, reader1);
+      assertRefCountEquals(1 + mode, reader1);
 
       if (mode == 1) {
         initReader2.close();
       }
       
       reader1.close();
-      assertRefCountEquals(3, reader1);
+      assertRefCountEquals(1, reader1);
       
       parallelReader2.close();
-      assertRefCountEquals(2, reader1);
+      assertRefCountEquals(0, reader1);
       
       parallelReader2.close();
-      assertRefCountEquals(2, reader1);
+      assertRefCountEquals(0, reader1);
       
       reader3.close();
-      assertRefCountEquals(1, reader1);
-      assertReaderOpen(reader1);
+      assertRefCountEquals(0, reader1);
+      assertReaderClosed(reader1, true, false);
       
       reader2.close();
       assertRefCountEquals(0, reader1);
@@ -617,16 +619,16 @@
     
     // Now reader2-reader5 references reader1. reader1 and reader2
     // share the same norms. reader3, reader4, reader5 also share norms.
-    assertRefCountEquals(5, reader1);
+    assertRefCountEquals(1, reader1);
     assertFalse(reader1.normsClosed());
 
     reader1.close();
 
-    assertRefCountEquals(4, reader1);
+    assertRefCountEquals(0, reader1);
     assertFalse(reader1.normsClosed());
 
     reader2.close();
-    assertRefCountEquals(3, reader1);
+    assertRefCountEquals(0, reader1);
 
     // now the norms for field1 and field2 should be closed
     assertTrue(reader1.normsClosed("field1"));
@@ -637,10 +639,10 @@
     assertFalse(reader1.normsClosed("field4"));
     
     reader3.close();
-    assertRefCountEquals(2, reader1);
+    assertRefCountEquals(0, reader1);
     assertFalse(reader3.normsClosed());
     reader5.close();
-    assertRefCountEquals(1, reader1);
+    assertRefCountEquals(0, reader1);
     assertFalse(reader3.normsClosed());
     reader4.close();
     assertRefCountEquals(0, reader1);
@@ -1140,4 +1142,87 @@
       // expected
     }
   }
+
+  public void testCloseOrig() throws Throwable {
+    Directory dir = new MockRAMDirectory();
+    createIndex(dir, false);
+    IndexReader r1 = IndexReader.open(dir);
+    IndexReader r2 = IndexReader.open(dir);
+    r2.deleteDocument(0);
+    r2.close();
+
+    IndexReader r3 = r1.reopen();
+    assertTrue(r1 != r3);
+    r1.close();
+    try {
+      r1.document(2);
+      fail("did not hit exception");
+    } catch (AlreadyClosedException ace) {
+      // expected
+    }
+    r3.close();
+    dir.close();
+  }
+
+  public void testDeletes() throws Throwable {
+    Directory dir = new MockRAMDirectory();
+    createIndex(dir, false);
+    // Get delete bitVector
+    modifyIndex(0, dir);
+    IndexReader r1 = IndexReader.open(dir);
+
+    // Add doc:
+    modifyIndex(5, dir);
+
+    IndexReader r2 = r1.reopen();
+    assertTrue(r1 != r2);
+
+    IndexReader[] rs2 = r2.getSequentialSubReaders();
+
+    SegmentReader sr1 = (SegmentReader) r1;
+    SegmentReader sr2 = (SegmentReader) rs2[0];
+
+    // At this point they share the same BitVector
+    assertTrue(sr1.deletedDocs==sr2.deletedDocs);
+
+    r2.deleteDocument(0);
+
+    // r1 should not see the delete
+    assertFalse(r1.isDeleted(0));
+
+    // Now r2 should have made a private copy of deleted docs:
+    assertTrue(sr1.deletedDocs!=sr2.deletedDocs);
+
+    r1.close();
+    r2.close();
+    dir.close();
+  }
+
+  public void testDeletes2() throws Throwable {
+    Directory dir = new MockRAMDirectory();
+    createIndex(dir, false);
+    // Get delete bitVector
+    modifyIndex(0, dir);
+    IndexReader r1 = IndexReader.open(dir);
+
+    // Add doc:
+    modifyIndex(5, dir);
+
+    IndexReader r2 = r1.reopen();
+    assertTrue(r1 != r2);
+
+    IndexReader[] rs2 = r2.getSequentialSubReaders();
+
+    SegmentReader sr1 = (SegmentReader) r1;
+    SegmentReader sr2 = (SegmentReader) rs2[0];
+
+    // At this point they share the same BitVector
+    assertTrue(sr1.deletedDocs==sr2.deletedDocs);
+    r1.close();
+
+    r2.deleteDocument(0);
+    assertTrue(sr1.deletedDocs==sr2.deletedDocs);
+    r2.close();
+    dir.close();
+  }
 }



Mime
View raw message