You are viewing a plain text version of this content. The canonical link for it is here.
Posted to java-commits@lucene.apache.org by mi...@apache.org on 2009/03/01 13:12:37 UTC
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/
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();
+ }
}