You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by us...@apache.org on 2015/05/27 14:33:10 UTC
svn commit: r1681998 - in /lucene/dev/trunk/lucene: CHANGES.txt
core/src/java/org/apache/lucene/index/ParallelCompositeReader.java
core/src/test/org/apache/lucene/index/TestParallelCompositeReader.java
Author: uschindler
Date: Wed May 27 12:33:09 2015
New Revision: 1681998
URL: http://svn.apache.org/r1681998
Log:
LUCENE-6501: Flatten subreader structure in ParallelCompositeReader (fixes close listener bug LUCENE-6500)
Modified:
lucene/dev/trunk/lucene/CHANGES.txt
lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/ParallelCompositeReader.java
lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestParallelCompositeReader.java
Modified: lucene/dev/trunk/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/CHANGES.txt?rev=1681998&r1=1681997&r2=1681998&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/CHANGES.txt (original)
+++ lucene/dev/trunk/lucene/CHANGES.txt Wed May 27 12:33:09 2015
@@ -33,7 +33,23 @@ API Changes
implementation returning the empty list. (Robert Muir)
======================= Lucene 5.3.0 =======================
-(No Changes)
+
+Bug fixes
+
+* LUCENE-6500: ParallelCompositeReader did not always call
+ closed listeners. This was fixed by LUCENE-6501.
+ (Adrien Grand, Uwe Schindler)
+
+Changes in Runtime Behavior
+
+* LUCENE-6501: The subreader structure in ParallelCompositeReader
+ was flattened, because the current implementation had too many
+ hidden bugs regarding refounting and close listeners.
+ If you create a new ParallelCompositeReader, it will just take
+ all leaves of the passed readers and form a flat structure of
+ ParallelLeafReaders instead of trying to assemble the original
+ structure of composite and leaf readers. (Adrien Grand,
+ Uwe Schindler)
======================= Lucene 5.2.0 =======================
Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/ParallelCompositeReader.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/ParallelCompositeReader.java?rev=1681998&r1=1681997&r2=1681998&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/ParallelCompositeReader.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/ParallelCompositeReader.java Wed May 27 12:33:09 2015
@@ -23,12 +23,13 @@ import java.util.IdentityHashMap;
import java.util.List;
import java.util.Set;
-/** An {@link CompositeReader} which reads multiple, parallel indexes. Each index added
- * must have the same number of documents, and exactly the same hierarchical subreader structure,
- * but typically each contains different fields. Deletions are taken from the first reader.
- * Each document contains the union of the fields of all
- * documents with the same document number. When searching, matches for a
- * query term are from the first index added that has the field.
+/** An {@link CompositeReader} which reads multiple, parallel indexes. Each
+ * index added must have the same number of documents, and exactly the same
+ * number of leaves (with equal {@code maxDoc}), but typically each contains
+ * different fields. Deletions are taken from the first reader. Each document
+ * contains the union of the fields of all documents with the same document
+ * number. When searching, matches for a query term are from the first index
+ * added that has the field.
*
* <p>This is useful, e.g., with collections that have large fields which
* change rarely and small fields that change more frequently. The smaller
@@ -46,7 +47,7 @@ import java.util.Set;
* by number of documents per segment. If you use different {@link MergePolicy}s
* it might happen that the segment structure of your index is no longer predictable.
*/
-public class ParallelCompositeReader extends BaseCompositeReader<IndexReader> {
+public class ParallelCompositeReader extends BaseCompositeReader<LeafReader> {
private final boolean closeSubReaders;
private final Set<IndexReader> completeReaderSet =
Collections.newSetFromMap(new IdentityHashMap<IndexReader,Boolean>());
@@ -67,7 +68,7 @@ public class ParallelCompositeReader ext
* readers and storedFieldReaders; when a document is
* loaded, only storedFieldsReaders will be used. */
public ParallelCompositeReader(boolean closeSubReaders, CompositeReader[] readers, CompositeReader[] storedFieldReaders) throws IOException {
- super(prepareSubReaders(readers, storedFieldReaders));
+ super(prepareLeafReaders(readers, storedFieldReaders));
this.closeSubReaders = closeSubReaders;
Collections.addAll(completeReaderSet, readers);
Collections.addAll(completeReaderSet, storedFieldReaders);
@@ -81,84 +82,62 @@ public class ParallelCompositeReader ext
completeReaderSet.addAll(getSequentialSubReaders());
}
- private static IndexReader[] prepareSubReaders(CompositeReader[] readers, CompositeReader[] storedFieldsReaders) throws IOException {
+ private static LeafReader[] prepareLeafReaders(CompositeReader[] readers, CompositeReader[] storedFieldsReaders) throws IOException {
if (readers.length == 0) {
if (storedFieldsReaders.length > 0)
throw new IllegalArgumentException("There must be at least one main reader if storedFieldsReaders are used.");
- return new IndexReader[0];
+ return new LeafReader[0];
} else {
- final List<? extends IndexReader> firstSubReaders = readers[0].getSequentialSubReaders();
+ final List<? extends LeafReaderContext> firstLeaves = readers[0].leaves();
// check compatibility:
- final int maxDoc = readers[0].maxDoc(), noSubs = firstSubReaders.size();
- final int[] childMaxDoc = new int[noSubs];
- final boolean[] childAtomic = new boolean[noSubs];
- for (int i = 0; i < noSubs; i++) {
- final IndexReader r = firstSubReaders.get(i);
- childMaxDoc[i] = r.maxDoc();
- childAtomic[i] = r instanceof LeafReader;
- }
- validate(readers, maxDoc, childMaxDoc, childAtomic);
- validate(storedFieldsReaders, maxDoc, childMaxDoc, childAtomic);
-
- // hierarchically build the same subreader structure as the first CompositeReader with Parallel*Readers:
- final IndexReader[] subReaders = new IndexReader[noSubs];
- for (int i = 0; i < subReaders.length; i++) {
- if (firstSubReaders.get(i) instanceof LeafReader) {
- final LeafReader[] atomicSubs = new LeafReader[readers.length];
- for (int j = 0; j < readers.length; j++) {
- atomicSubs[j] = (LeafReader) readers[j].getSequentialSubReaders().get(i);
- }
- final LeafReader[] storedSubs = new LeafReader[storedFieldsReaders.length];
- for (int j = 0; j < storedFieldsReaders.length; j++) {
- storedSubs[j] = (LeafReader) storedFieldsReaders[j].getSequentialSubReaders().get(i);
- }
- // We pass true for closeSubs and we prevent closing of subreaders in doClose():
- // By this the synthetic throw-away readers used here are completely invisible to ref-counting
- subReaders[i] = new ParallelLeafReader(true, atomicSubs, storedSubs) {
- @Override
- protected void doClose() {}
- };
- } else {
- assert firstSubReaders.get(i) instanceof CompositeReader;
- final CompositeReader[] compositeSubs = new CompositeReader[readers.length];
- for (int j = 0; j < readers.length; j++) {
- compositeSubs[j] = (CompositeReader) readers[j].getSequentialSubReaders().get(i);
- }
- final CompositeReader[] storedSubs = new CompositeReader[storedFieldsReaders.length];
- for (int j = 0; j < storedFieldsReaders.length; j++) {
- storedSubs[j] = (CompositeReader) storedFieldsReaders[j].getSequentialSubReaders().get(i);
- }
- // We pass true for closeSubs and we prevent closing of subreaders in doClose():
- // By this the synthetic throw-away readers used here are completely invisible to ref-counting
- subReaders[i] = new ParallelCompositeReader(true, compositeSubs, storedSubs) {
- @Override
- protected void doClose() {}
- };
+ final int maxDoc = readers[0].maxDoc(), noLeaves = firstLeaves.size();
+ final int[] leafMaxDoc = new int[noLeaves];
+ for (int i = 0; i < noLeaves; i++) {
+ final LeafReader r = firstLeaves.get(i).reader();
+ leafMaxDoc[i] = r.maxDoc();
+ }
+ validate(readers, maxDoc, leafMaxDoc);
+ validate(storedFieldsReaders, maxDoc, leafMaxDoc);
+
+ // flatten structure of each Composite to just LeafReader[]
+ // and combine parallel structure with ParallelLeafReaders:
+ final LeafReader[] wrappedLeaves = new LeafReader[noLeaves];
+ for (int i = 0; i < wrappedLeaves.length; i++) {
+ final LeafReader[] subs = new LeafReader[readers.length];
+ for (int j = 0; j < readers.length; j++) {
+ subs[j] = readers[j].leaves().get(i).reader();
}
+ final LeafReader[] storedSubs = new LeafReader[storedFieldsReaders.length];
+ for (int j = 0; j < storedFieldsReaders.length; j++) {
+ storedSubs[j] = storedFieldsReaders[j].leaves().get(i).reader();
+ }
+ // We pass true for closeSubs and we prevent touching of subreaders in doClose():
+ // By this the synthetic throw-away readers used here are completely invisible to ref-counting
+ wrappedLeaves[i] = new ParallelLeafReader(true, subs, storedSubs) {
+ @Override
+ protected void doClose() {}
+ };
}
- return subReaders;
+ return wrappedLeaves;
}
}
- private static void validate(CompositeReader[] readers, int maxDoc, int[] childMaxDoc, boolean[] childAtomic) {
+ private static void validate(CompositeReader[] readers, int maxDoc, int[] leafMaxDoc) {
for (int i = 0; i < readers.length; i++) {
final CompositeReader reader = readers[i];
- final List<? extends IndexReader> subs = reader.getSequentialSubReaders();
+ final List<? extends LeafReaderContext> subs = reader.leaves();
if (reader.maxDoc() != maxDoc) {
throw new IllegalArgumentException("All readers must have same maxDoc: "+maxDoc+"!="+reader.maxDoc());
}
final int noSubs = subs.size();
- if (noSubs != childMaxDoc.length) {
- throw new IllegalArgumentException("All readers must have same number of subReaders");
+ if (noSubs != leafMaxDoc.length) {
+ throw new IllegalArgumentException("All readers must have same number of leaf readers");
}
for (int subIDX = 0; subIDX < noSubs; subIDX++) {
- final IndexReader r = subs.get(subIDX);
- if (r.maxDoc() != childMaxDoc[subIDX]) {
- throw new IllegalArgumentException("All readers must have same corresponding subReader maxDoc");
- }
- if (!(childAtomic[subIDX] ? (r instanceof LeafReader) : (r instanceof CompositeReader))) {
- throw new IllegalArgumentException("All readers must have same corresponding subReader types (atomic or composite)");
+ final LeafReader r = subs.get(subIDX).reader();
+ if (r.maxDoc() != leafMaxDoc[subIDX]) {
+ throw new IllegalArgumentException("All leaf readers must have same corresponding subReader maxDoc");
}
}
}
Modified: lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestParallelCompositeReader.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestParallelCompositeReader.java?rev=1681998&r1=1681997&r2=1681998&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestParallelCompositeReader.java (original)
+++ lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestParallelCompositeReader.java Wed May 27 12:33:09 2015
@@ -23,9 +23,12 @@ import java.util.Random;
import org.apache.lucene.analysis.MockAnalyzer;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.Field;
-import org.apache.lucene.index.IndexReader.ReaderClosedListener;
import org.apache.lucene.search.BooleanClause.Occur;
-import org.apache.lucene.search.*;
+import org.apache.lucene.search.BooleanQuery;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.ScoreDoc;
+import org.apache.lucene.search.TermQuery;
import org.apache.lucene.store.AlreadyClosedException;
import org.apache.lucene.store.Directory;
import org.apache.lucene.util.LuceneTestCase;
@@ -123,59 +126,65 @@ public class TestParallelCompositeReader
dir2.close();
}
- // closeSubreaders=false
- public void testReaderClosedListener1() throws Exception {
- Directory dir1 = getDir1(random());
- CompositeReader ir1 = DirectoryReader.open(dir1);
+ private void testReaderClosedListener(boolean closeSubReaders, int wrapMultiReaderType) throws IOException {
+ final Directory dir1 = getDir1(random());
+ final CompositeReader ir2, ir1 = DirectoryReader.open(dir1);
+ switch (wrapMultiReaderType) {
+ case 0:
+ ir2 = ir1;
+ break;
+ case 1:
+ // default case, does close subreaders:
+ ir2 = new MultiReader(ir1); break;
+ case 2:
+ ir2 = new MultiReader(new CompositeReader[] {ir1}, false); break;
+ default:
+ throw new AssertionError();
+ }
// with overlapping
- ParallelCompositeReader pr = new ParallelCompositeReader(false,
- new CompositeReader[] {ir1},
- new CompositeReader[] {ir1});
+ ParallelCompositeReader pr = new ParallelCompositeReader(closeSubReaders,
+ new CompositeReader[] {ir2},
+ new CompositeReader[] {ir2});
final int[] listenerClosedCount = new int[1];
assertEquals(3, pr.leaves().size());
for(LeafReaderContext cxt : pr.leaves()) {
- cxt.reader().addReaderClosedListener(new ReaderClosedListener() {
- @Override
- public void onClose(IndexReader reader) {
- listenerClosedCount[0]++;
- }
- });
+ cxt.reader().addReaderClosedListener(reader -> listenerClosedCount[0]++);
}
pr.close();
- ir1.close();
+ if (!closeSubReaders) {
+ ir1.close();
+ }
assertEquals(3, listenerClosedCount[0]);
+
+ // We have to close the extra MultiReader, because it will not close its own subreaders:
+ if (wrapMultiReaderType == 2) {
+ ir2.close();
+ }
dir1.close();
}
- // closeSubreaders=true
+ public void testReaderClosedListener1() throws Exception {
+ testReaderClosedListener(false, 0);
+ }
+
public void testReaderClosedListener2() throws Exception {
- Directory dir1 = getDir1(random());
- CompositeReader ir1 = DirectoryReader.open(dir1);
-
- // with overlapping
- ParallelCompositeReader pr = new ParallelCompositeReader(true,
- new CompositeReader[] {ir1},
- new CompositeReader[] {ir1});
+ testReaderClosedListener(true, 0);
+ }
- final int[] listenerClosedCount = new int[1];
+ public void testReaderClosedListener3() throws Exception {
+ testReaderClosedListener(false, 1);
+ }
- assertEquals(3, pr.leaves().size());
+ public void testReaderClosedListener4() throws Exception {
+ testReaderClosedListener(true, 1);
+ }
- for(LeafReaderContext cxt : pr.leaves()) {
- cxt.reader().addReaderClosedListener(new ReaderClosedListener() {
- @Override
- public void onClose(IndexReader reader) {
- listenerClosedCount[0]++;
- }
- });
- }
- pr.close();
- assertEquals(3, listenerClosedCount[0]);
- dir1.close();
+ public void testReaderClosedListener5() throws Exception {
+ testReaderClosedListener(false, 2);
}
public void testCloseInnerReader() throws Exception {
@@ -395,7 +404,7 @@ public class TestParallelCompositeReader
ParallelCompositeReader pr = new ParallelCompositeReader(new CompositeReader[] {new MultiReader(ir1)});
final String s = pr.toString();
- assertTrue("toString incorrect: " + s, s.startsWith("ParallelCompositeReader(ParallelCompositeReader(ParallelLeafReader("));
+ assertTrue("toString incorrect (should be flattened): " + s, s.startsWith("ParallelCompositeReader(ParallelLeafReader("));
pr.close();
dir1.close();