Index: lucene/test-framework/src/java/org/apache/lucene/index/RandomDocumentsWriterPerThreadPool.java =================================================================== --- lucene/test-framework/src/java/org/apache/lucene/index/RandomDocumentsWriterPerThreadPool.java (revision 1344037) +++ lucene/test-framework/src/java/org/apache/lucene/index/RandomDocumentsWriterPerThreadPool.java (working copy) @@ -25,8 +25,7 @@ * @lucene.internal * @lucene.experimental */ -public class RandomDocumentsWriterPerThreadPool extends - DocumentsWriterPerThreadPool { +class RandomDocumentsWriterPerThreadPool extends DocumentsWriterPerThreadPool { private final ThreadState[] states; private final Random random; private final int maxRetry; Index: lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java =================================================================== --- lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java (revision 1344037) +++ lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java (working copy) @@ -22,6 +22,7 @@ import java.io.*; import java.lang.annotation.*; +import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.util.*; import java.util.concurrent.*; @@ -625,8 +626,11 @@ try { if (rarely(r)) { + Class clazz = Class.forName("org.apache.lucene.index.RandomDocumentsWriterPerThreadPool"); + Constructor ctor = clazz.getConstructor(int.class, Random.class); + ctor.setAccessible(true); // random thread pool - setIndexerThreadPoolMethod.invoke(c, new RandomDocumentsWriterPerThreadPool(maxNumThreadStates, r)); + setIndexerThreadPoolMethod.invoke(c, ctor.newInstance(maxNumThreadStates, r)); } else { // random thread pool c.setMaxThreadStates(maxNumThreadStates); Index: lucene/CHANGES.txt =================================================================== --- lucene/CHANGES.txt (revision 1344037) +++ lucene/CHANGES.txt (working copy) @@ -1001,6 +1001,13 @@ CharsRef's CharSequence methods to throw exceptions in boundary cases to properly meet the specification. (Robert Muir) +* LUCENE-4084: Attempting to reuse a single IndexWriterConfig instance + across more than one IndexWriter resulted in cryptic exception. + This is now fixed, but requires that certain members of + IndexWriterConfig (MergePolicy, FlushPolicy, + DocumentsWriterThreadPool) implement clone. (Robert Muir, Simon + Willnauer, Mike McCandless) + Documentation * LUCENE-3958: Javadocs corrections for IndexWriter. Index: lucene/core/src/test/org/apache/lucene/index/TestIndexWriterConfig.java =================================================================== --- lucene/core/src/test/org/apache/lucene/index/TestIndexWriterConfig.java (revision 1344037) +++ lucene/core/src/test/org/apache/lucene/index/TestIndexWriterConfig.java (working copy) @@ -25,10 +25,13 @@ import org.apache.lucene.analysis.MockAnalyzer; import org.apache.lucene.codecs.Codec; +import org.apache.lucene.document.Document; +import org.apache.lucene.document.TextField; import org.apache.lucene.index.DocumentsWriterPerThread.IndexingChain; import org.apache.lucene.index.IndexWriterConfig.OpenMode; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.similarities.DefaultSimilarity; +import org.apache.lucene.store.Directory; import org.apache.lucene.util.InfoStream; import org.apache.lucene.util.LuceneTestCase; import org.junit.Test; @@ -149,6 +152,9 @@ // toString. continue; } + if (f.getName().equals("inUseByIndexWriter")) { + continue; + } assertTrue(f.getName() + " not found in toString", str.indexOf(f.getName()) != -1); } } @@ -269,4 +275,54 @@ conf.setMergePolicy(null); assertEquals(LogByteSizeMergePolicy.class, conf.getMergePolicy().getClass()); } + + public void testReuse() throws Exception { + IndexWriterConfig iwc = newIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(random())); + Directory dir = newDirectory(); + Document doc = new Document(); + doc.add(newField("foo", "bar", TextField.TYPE_STORED)); + RandomIndexWriter riw = new RandomIndexWriter(random(), dir, iwc); + riw.addDocument(doc); + riw.close(); + + // Sharing IWC should be fine: + riw = new RandomIndexWriter(random(), dir, iwc); + riw.addDocument(doc); + riw.close(); + + dir.close(); + } + + public void testIWCClone() throws Exception { + IndexWriterConfig iwc = newIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(random())); + Directory dir = newDirectory(); + RandomIndexWriter riw = new RandomIndexWriter(random(), dir, iwc); + + // Cannot clone IW's private IWC clone: + try { + riw.w.getConfig().clone(); + fail("did not hit expected exception"); + } catch (IllegalStateException ise) { + // expected + } + riw.close(); + dir.close(); + } + + public void testIWCInvalidReuse() throws Exception { + IndexWriterConfig iwc = newIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(random())); + Directory dir = newDirectory(); + RandomIndexWriter riw = new RandomIndexWriter(random(), dir, iwc); + IndexWriterConfig privateIWC = riw.w.getConfig(); + riw.close(); + + // Cannot clone IW's private IWC clone: + try { + new RandomIndexWriter(random(), dir, privateIWC); + fail("did not hit expected exception"); + } catch (IllegalStateException ise) { + // expected + } + dir.close(); + } } Index: lucene/core/src/test/org/apache/lucene/index/TestPerSegmentDeletes.java =================================================================== --- lucene/core/src/test/org/apache/lucene/index/TestPerSegmentDeletes.java (revision 1344037) +++ lucene/core/src/test/org/apache/lucene/index/TestPerSegmentDeletes.java (working copy) @@ -86,6 +86,7 @@ // which should apply the delete id:2 writer.deleteDocuments(new Term("id", "2")); writer.flush(false, false); + fsmp = (RangeMergePolicy) writer.getConfig().getMergePolicy(); fsmp.doMerge = true; fsmp.start = 0; fsmp.length = 2; Index: lucene/core/src/test/org/apache/lucene/index/TestFlushByRamOrCountsPolicy.java =================================================================== --- lucene/core/src/test/org/apache/lucene/index/TestFlushByRamOrCountsPolicy.java (revision 1344037) +++ lucene/core/src/test/org/apache/lucene/index/TestFlushByRamOrCountsPolicy.java (working copy) @@ -78,6 +78,7 @@ iwc.setMaxBufferedDocs(IndexWriterConfig.DISABLE_AUTO_FLUSH); iwc.setMaxBufferedDeleteTerms(IndexWriterConfig.DISABLE_AUTO_FLUSH); IndexWriter writer = new IndexWriter(dir, iwc); + flushPolicy = (MockDefaultFlushPolicy) writer.getConfig().getFlushPolicy(); assertFalse(flushPolicy.flushOnDocCount()); assertFalse(flushPolicy.flushOnDeleteTerms()); assertTrue(flushPolicy.flushOnRAM()); @@ -135,6 +136,7 @@ iwc.setRAMBufferSizeMB(IndexWriterConfig.DISABLE_AUTO_FLUSH); iwc.setMaxBufferedDeleteTerms(IndexWriterConfig.DISABLE_AUTO_FLUSH); IndexWriter writer = new IndexWriter(dir, iwc); + flushPolicy = (MockDefaultFlushPolicy) writer.getConfig().getFlushPolicy(); assertTrue(flushPolicy.flushOnDocCount()); assertFalse(flushPolicy.flushOnDeleteTerms()); assertFalse(flushPolicy.flushOnRAM()); @@ -183,6 +185,7 @@ iwc.setIndexerThreadPool(threadPool); IndexWriter writer = new IndexWriter(dir, iwc); + flushPolicy = (MockDefaultFlushPolicy) writer.getConfig().getFlushPolicy(); DocumentsWriter docsWriter = writer.getDocsWriter(); assertNotNull(docsWriter); DocumentsWriterFlushControl flushControl = docsWriter.flushControl; Index: lucene/core/src/test/org/apache/lucene/index/TestIndexFileDeleter.java =================================================================== --- lucene/core/src/test/org/apache/lucene/index/TestIndexFileDeleter.java (revision 1344037) +++ lucene/core/src/test/org/apache/lucene/index/TestIndexFileDeleter.java (working copy) @@ -61,7 +61,7 @@ for(i=0;i<35;i++) { addDoc(writer, i); } - mergePolicy.setUseCompoundFile(false); + ((LogMergePolicy) writer.getConfig().getMergePolicy()).setUseCompoundFile(false); for(;i<45;i++) { addDoc(writer, i); } Index: lucene/core/src/test/org/apache/lucene/index/TestTieredMergePolicy.java =================================================================== --- lucene/core/src/test/org/apache/lucene/index/TestTieredMergePolicy.java (revision 1344037) +++ lucene/core/src/test/org/apache/lucene/index/TestTieredMergePolicy.java (working copy) @@ -57,7 +57,7 @@ if (VERBOSE) { System.out.println("\nTEST: forceMergeDeletes2"); } - tmp.setForceMergeDeletesPctAllowed(10.0); + ((TieredMergePolicy) w.getConfig().getMergePolicy()).setForceMergeDeletesPctAllowed(10.0); w.forceMergeDeletes(); assertEquals(60, w.maxDoc()); assertEquals(60, w.numDocs()); Index: lucene/core/src/test/org/apache/lucene/index/TestNoMergePolicy.java =================================================================== --- lucene/core/src/test/org/apache/lucene/index/TestNoMergePolicy.java (revision 1344037) +++ lucene/core/src/test/org/apache/lucene/index/TestNoMergePolicy.java (working copy) @@ -62,6 +62,9 @@ // context, including ones from Object. So just filter out Object. If in // the future MergePolicy will extend a different class than Object, this // will need to change. + if (m.getName().equals("clone")) { + continue; + } if (m.getDeclaringClass() != Object.class) { assertTrue(m + " is not overridden !", m.getDeclaringClass() == NoMergePolicy.class); } Index: lucene/core/src/java/org/apache/lucene/index/MergePolicy.java =================================================================== --- lucene/core/src/java/org/apache/lucene/index/MergePolicy.java (revision 1344037) +++ lucene/core/src/java/org/apache/lucene/index/MergePolicy.java (working copy) @@ -57,7 +57,7 @@ * @lucene.experimental */ -public abstract class MergePolicy implements java.io.Closeable { +public abstract class MergePolicy implements java.io.Closeable, Cloneable { /** OneMerge provides the information necessary to perform * an individual primitive merge operation, resulting in @@ -254,8 +254,21 @@ } } - protected final SetOnce writer; + protected SetOnce writer; + @Override + public MergePolicy clone() { + MergePolicy clone; + try { + clone = (MergePolicy) super.clone(); + } catch (CloneNotSupportedException e) { + // should not happen + throw new RuntimeException(e); + } + clone.writer = new SetOnce(); + return clone; + } + /** * Creates a new merge policy instance. Note that if you intend to use it * without passing it to {@link IndexWriter}, you should call Index: lucene/core/src/java/org/apache/lucene/index/TieredMergePolicy.java =================================================================== --- lucene/core/src/java/org/apache/lucene/index/TieredMergePolicy.java (revision 1344037) +++ lucene/core/src/java/org/apache/lucene/index/TieredMergePolicy.java (working copy) @@ -256,8 +256,6 @@ } } - private final Comparator segmentByteSizeDescending = new SegmentByteSizeDescending(); - /** Holds score and explanation for a single candidate * merge. */ protected static abstract class MergeScore { @@ -277,7 +275,7 @@ final Collection toBeMerged = new HashSet(); final List infosSorted = new ArrayList(infos.asList()); - Collections.sort(infosSorted, segmentByteSizeDescending); + Collections.sort(infosSorted, new SegmentByteSizeDescending()); // Compute total index bytes & print details about the index long totIndexBytes = 0; @@ -516,7 +514,7 @@ return null; } - Collections.sort(eligible, segmentByteSizeDescending); + Collections.sort(eligible, new SegmentByteSizeDescending()); if (verbose()) { message("eligible=" + eligible); @@ -573,7 +571,7 @@ return null; } - Collections.sort(eligible, segmentByteSizeDescending); + Collections.sort(eligible, new SegmentByteSizeDescending()); if (verbose()) { message("eligible=" + eligible); Index: lucene/core/src/java/org/apache/lucene/index/FlushPolicy.java =================================================================== --- lucene/core/src/java/org/apache/lucene/index/FlushPolicy.java (revision 1344037) +++ lucene/core/src/java/org/apache/lucene/index/FlushPolicy.java (working copy) @@ -50,8 +50,8 @@ * @see DocumentsWriterPerThread * @see IndexWriterConfig#setFlushPolicy(FlushPolicy) */ -abstract class FlushPolicy { - protected final SetOnce writer = new SetOnce(); +abstract class FlushPolicy implements Cloneable { + protected SetOnce writer = new SetOnce(); protected IndexWriterConfig indexWriterConfig; /** @@ -132,4 +132,17 @@ return true; } + @Override + public FlushPolicy clone() { + FlushPolicy clone; + try { + clone = (FlushPolicy) super.clone(); + } catch (CloneNotSupportedException e) { + // should not happen + throw new RuntimeException(e); + } + clone.writer = new SetOnce(); + clone.indexWriterConfig = null; + return clone; + } } Index: lucene/core/src/java/org/apache/lucene/index/DocumentsWriterPerThreadPool.java =================================================================== --- lucene/core/src/java/org/apache/lucene/index/DocumentsWriterPerThreadPool.java (revision 1344037) +++ lucene/core/src/java/org/apache/lucene/index/DocumentsWriterPerThreadPool.java (working copy) @@ -36,7 +36,7 @@ * new {@link DocumentsWriterPerThread} instance. *

*/ -abstract class DocumentsWriterPerThreadPool { +abstract class DocumentsWriterPerThreadPool implements Cloneable { /** * {@link ThreadState} references and guards a @@ -119,10 +119,10 @@ } } - private final ThreadState[] threadStates; + private ThreadState[] threadStates; private volatile int numThreadStatesActive; - private final SetOnce globalFieldMap = new SetOnce(); - private final SetOnce documentsWriter = new SetOnce(); + private SetOnce globalFieldMap = new SetOnce(); + private SetOnce documentsWriter = new SetOnce(); /** * Creates a new {@link DocumentsWriterPerThreadPool} with a given maximum of {@link ThreadState}s. @@ -143,6 +143,21 @@ threadStates[i] = new ThreadState(new DocumentsWriterPerThread(documentsWriter.directory, documentsWriter, infos, documentsWriter.chain)); } } + + @Override + public DocumentsWriterPerThreadPool clone() { + DocumentsWriterPerThreadPool clone; + try { + clone = (DocumentsWriterPerThreadPool) super.clone(); + } catch (CloneNotSupportedException e) { + // should not happen + throw new RuntimeException(e); + } + clone.documentsWriter = new SetOnce(); + clone.globalFieldMap = new SetOnce(); + clone.threadStates = new ThreadState[threadStates.length]; + return clone; + } /** * Returns the max number of {@link ThreadState} instances available in this Index: lucene/core/src/java/org/apache/lucene/index/IndexWriterConfig.java =================================================================== --- lucene/core/src/java/org/apache/lucene/index/IndexWriterConfig.java (revision 1344037) +++ lucene/core/src/java/org/apache/lucene/index/IndexWriterConfig.java (working copy) @@ -18,6 +18,7 @@ */ import java.io.PrintStream; +import java.util.concurrent.atomic.AtomicBoolean; import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.codecs.Codec; @@ -27,6 +28,7 @@ import org.apache.lucene.search.similarities.Similarity; import org.apache.lucene.util.InfoStream; import org.apache.lucene.util.PrintStreamInfoStream; +import org.apache.lucene.util.SetOnce; import org.apache.lucene.util.Version; /** @@ -153,6 +155,9 @@ private Version matchVersion; + // Used directly by IndexWriter: + AtomicBoolean inUseByIndexWriter = new AtomicBoolean(); + /** * Creates a new config that with defaults that match the specified * {@link Version} as well as the default {@link @@ -196,14 +201,26 @@ @Override public IndexWriterConfig clone() { - // Shallow clone is the only thing that's possible, since parameters like - // analyzer, index commit etc. do not implement Cloneable. + IndexWriterConfig clone; + if (inUseByIndexWriter.get()) { + throw new IllegalStateException("cannot clone: this IndexWriterConfig is private to IndexWriter; make a new one instead"); + } try { - return (IndexWriterConfig)super.clone(); + clone = (IndexWriterConfig) super.clone(); } catch (CloneNotSupportedException e) { // should not happen throw new RuntimeException(e); } + + // Mostly shallow clone, but do a deepish clone of + // certain objects that have state that cannot be shared + // across IW instances: + clone.inUseByIndexWriter = new AtomicBoolean(); + clone.flushPolicy = flushPolicy.clone(); + clone.indexerThreadPool = indexerThreadPool.clone(); + clone.mergePolicy = mergePolicy.clone(); + + return clone; } /** Returns the default analyzer to use for indexing documents. */ Index: lucene/core/src/java/org/apache/lucene/index/ThreadAffinityDocumentsWriterThreadPool.java =================================================================== --- lucene/core/src/java/org/apache/lucene/index/ThreadAffinityDocumentsWriterThreadPool.java (revision 1344037) +++ lucene/core/src/java/org/apache/lucene/index/ThreadAffinityDocumentsWriterThreadPool.java (working copy) @@ -74,4 +74,11 @@ minThreadState.lock(); return minThreadState; } + + @Override + public ThreadAffinityDocumentsWriterThreadPool clone() { + ThreadAffinityDocumentsWriterThreadPool clone = (ThreadAffinityDocumentsWriterThreadPool) super.clone(); + clone.threadBindings = new ConcurrentHashMap(); + return clone; + } } Index: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java =================================================================== --- lucene/core/src/java/org/apache/lucene/index/IndexWriter.java (revision 1344037) +++ lucene/core/src/java/org/apache/lucene/index/IndexWriter.java (working copy) @@ -588,26 +588,30 @@ */ public IndexWriter(Directory d, IndexWriterConfig conf) throws CorruptIndexException, LockObtainFailedException, IOException { + if (conf.inUseByIndexWriter.get()) { + throw new IllegalStateException("the provided IndexWriterConfig was previously used by a different IndexWriter; please make a new one instead"); + } config = conf.clone(); + config.inUseByIndexWriter.set(true); directory = d; - analyzer = conf.getAnalyzer(); - infoStream = conf.getInfoStream(); - mergePolicy = conf.getMergePolicy(); + analyzer = config.getAnalyzer(); + infoStream = config.getInfoStream(); + mergePolicy = config.getMergePolicy(); mergePolicy.setIndexWriter(this); - mergeScheduler = conf.getMergeScheduler(); - codec = conf.getCodec(); + mergeScheduler = config.getMergeScheduler(); + codec = config.getCodec(); bufferedDeletesStream = new BufferedDeletesStream(infoStream); - poolReaders = conf.getReaderPooling(); + poolReaders = config.getReaderPooling(); writeLock = directory.makeLock(WRITE_LOCK_NAME); - if (!writeLock.obtain(conf.getWriteLockTimeout())) // obtain write lock + if (!writeLock.obtain(config.getWriteLockTimeout())) // obtain write lock throw new LockObtainFailedException("Index locked for write: " + writeLock); boolean success = false; try { - OpenMode mode = conf.getOpenMode(); + OpenMode mode = config.getOpenMode(); boolean create; if (mode == OpenMode.CREATE) { create = true; @@ -641,7 +645,7 @@ } else { segmentInfos.read(directory); - IndexCommit commit = conf.getIndexCommit(); + IndexCommit commit = config.getIndexCommit(); if (commit != null) { // Swap out all segments, but, keep metadata in // SegmentInfos, like version & generation, to @@ -671,7 +675,7 @@ // KeepOnlyLastCommitDeleter: synchronized(this) { deleter = new IndexFileDeleter(directory, - conf.getIndexDeletionPolicy(), + config.getIndexDeletionPolicy(), segmentInfos, infoStream, this); }