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 2007/12/04 23:19:09 UTC

svn commit: r601114 - in /lucene/java/trunk/src: java/org/apache/lucene/index/IndexWriter.java test/org/apache/lucene/index/TestConcurrentMergeScheduler.java test/org/apache/lucene/store/MockRAMDirectory.java

Author: mikemccand
Date: Tue Dec  4 14:19:08 2007
New Revision: 601114

URL: http://svn.apache.org/viewvc?rev=601114&view=rev
Log:
LUCENE-1075: fix possible thread hazard with IndexWriter.close(false)

Modified:
    lucene/java/trunk/src/java/org/apache/lucene/index/IndexWriter.java
    lucene/java/trunk/src/test/org/apache/lucene/index/TestConcurrentMergeScheduler.java
    lucene/java/trunk/src/test/org/apache/lucene/store/MockRAMDirectory.java

Modified: lucene/java/trunk/src/java/org/apache/lucene/index/IndexWriter.java
URL: http://svn.apache.org/viewvc/lucene/java/trunk/src/java/org/apache/lucene/index/IndexWriter.java?rev=601114&r1=601113&r2=601114&view=diff
==============================================================================
--- lucene/java/trunk/src/java/org/apache/lucene/index/IndexWriter.java (original)
+++ lucene/java/trunk/src/java/org/apache/lucene/index/IndexWriter.java Tue Dec  4 14:19:08 2007
@@ -2602,11 +2602,8 @@
     // file that current segments does not reference), we
     // abort this merge
     if (merge.isAborted()) {
-
-      if (infoStream != null) {
-        if (merge.isAborted())
-          message("commitMerge: skipping merge " + merge.segString(directory) + ": it was aborted");
-      }
+      if (infoStream != null)
+        message("commitMerge: skipping merge " + merge.segString(directory) + ": it was aborted");
 
       assert merge.increfDone;
       decrefMergeSegments(merge);
@@ -2866,9 +2863,8 @@
    *  the synchronized lock on IndexWriter instance. */
   final synchronized void mergeInit(MergePolicy.OneMerge merge) throws IOException {
 
-    // Bind a new segment name here so even with
-    // ConcurrentMergePolicy we keep deterministic segment
-    // names.
+    if (merge.isAborted())
+      throw new IOException("merge is aborted");
 
     assert merge.registerDone;
 
@@ -2982,6 +2978,10 @@
     merge.increfDone = true;
 
     merge.mergeDocStores = mergeDocStores;
+
+    // Bind a new segment name here so even with
+    // ConcurrentMergePolicy we keep deterministic segment
+    // names.
     merge.info = new SegmentInfo(newSegmentName(), 0,
                                  directory, false, true,
                                  docStoreOffset,
@@ -3033,6 +3033,7 @@
 
     try {
       int totDocCount = 0;
+
       for (int i = 0; i < numSegments; i++) {
         SegmentInfo si = sourceSegmentsClone.info(i);
         IndexReader reader = SegmentReader.get(si, MERGE_READ_BUFFER_SIZE, merge.mergeDocStores); // no need to set deleter (yet)
@@ -3042,6 +3043,9 @@
       if (infoStream != null) {
         message("merge: total "+totDocCount+" docs");
       }
+
+      if (merge.isAborted())
+        throw new IOException("merge is aborted");
 
       mergedDocCount = merge.info.docCount = merger.merge(merge.mergeDocStores);
 

Modified: lucene/java/trunk/src/test/org/apache/lucene/index/TestConcurrentMergeScheduler.java
URL: http://svn.apache.org/viewvc/lucene/java/trunk/src/test/org/apache/lucene/index/TestConcurrentMergeScheduler.java?rev=601114&r1=601113&r2=601114&view=diff
==============================================================================
--- lucene/java/trunk/src/test/org/apache/lucene/index/TestConcurrentMergeScheduler.java (original)
+++ lucene/java/trunk/src/test/org/apache/lucene/index/TestConcurrentMergeScheduler.java Tue Dec  4 14:19:08 2007
@@ -19,19 +19,14 @@
 
 import org.apache.lucene.analysis.SimpleAnalyzer;
 import org.apache.lucene.analysis.Analyzer;
-import org.apache.lucene.store.Directory;
-import org.apache.lucene.store.FSDirectory;
 import org.apache.lucene.store.MockRAMDirectory;
 import org.apache.lucene.store.RAMDirectory;
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.Field;
-import org.apache.lucene.util._TestUtil;
-import org.apache.lucene.util.English;
 
 import org.apache.lucene.util.LuceneTestCase;
 
 import java.io.IOException;
-import java.io.File;
 
 public class TestConcurrentMergeScheduler extends LuceneTestCase {
   
@@ -193,6 +188,7 @@
         ConcurrentMergeScheduler cms = new ConcurrentMergeScheduler();
         writer.setMergeScheduler(cms);
         writer.setMaxBufferedDocs(2);
+        writer.setMergeFactor(100);
 
         for(int j=0;j<201;j++) {
           idField.setValue(Integer.toString(iter*201+j));
@@ -205,10 +201,16 @@
           delID += 5;
         }
 
+        // Force a bunch of merge threads to kick off so we
+        // stress out aborting them on close:
+        writer.setMergeFactor(3);
+        writer.addDocument(doc);
+        writer.flush();
+
         writer.close(false);
 
         IndexReader reader = IndexReader.open(directory);
-        assertEquals((1+iter)*181, reader.numDocs());
+        assertEquals((1+iter)*182, reader.numDocs());
         reader.close();
 
         // Reopen

Modified: lucene/java/trunk/src/test/org/apache/lucene/store/MockRAMDirectory.java
URL: http://svn.apache.org/viewvc/lucene/java/trunk/src/test/org/apache/lucene/store/MockRAMDirectory.java?rev=601114&r1=601113&r2=601114&view=diff
==============================================================================
--- lucene/java/trunk/src/test/org/apache/lucene/store/MockRAMDirectory.java (original)
+++ lucene/java/trunk/src/test/org/apache/lucene/store/MockRAMDirectory.java Tue Dec  4 14:19:08 2007
@@ -146,11 +146,17 @@
     RAMFile file = new RAMFile(this);
     synchronized (this) {
       RAMFile existing = (RAMFile)fileMap.get(name);
-      if (existing!=null) {
-        sizeInBytes -= existing.sizeInBytes;
-        existing.directory = null;
+      // Enforce write once:
+      if (existing!=null && !name.equals("segments.gen"))
+        throw new IOException("file " + name + " already exists");
+      else {
+        if (existing!=null) {
+          sizeInBytes -= existing.sizeInBytes;
+          existing.directory = null;
+        }
+
+        fileMap.put(name, file);
       }
-      fileMap.put(name, file);
     }
 
     return new MockRAMOutputStream(this, file);