You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mi...@apache.org on 2013/01/03 14:42:31 UTC

svn commit: r1428343 - in /lucene/dev/branches/branch_4x: ./ lucene/ lucene/codecs/ lucene/codecs/src/java/org/apache/lucene/codecs/simpletext/ lucene/core/ lucene/core/src/java/org/apache/lucene/codecs/lucene40/ lucene/core/src/java/org/apache/lucene/...

Author: mikemccand
Date: Thu Jan  3 13:42:30 2013
New Revision: 1428343

URL: http://svn.apache.org/viewvc?rev=1428343&view=rev
Log:
LUCENE-4653: add deletes to TIW.testThreadInterruptDeadlock; fix a few places that didn't handle InterruptedException properly

Modified:
    lucene/dev/branches/branch_4x/   (props changed)
    lucene/dev/branches/branch_4x/lucene/   (props changed)
    lucene/dev/branches/branch_4x/lucene/codecs/   (props changed)
    lucene/dev/branches/branch_4x/lucene/codecs/src/java/org/apache/lucene/codecs/simpletext/SimpleTextFieldsReader.java
    lucene/dev/branches/branch_4x/lucene/core/   (props changed)
    lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/codecs/lucene40/BitVector.java
    lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/ReadersAndLiveDocs.java
    lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/SegmentInfoPerCommit.java
    lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java

Modified: lucene/dev/branches/branch_4x/lucene/codecs/src/java/org/apache/lucene/codecs/simpletext/SimpleTextFieldsReader.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/codecs/src/java/org/apache/lucene/codecs/simpletext/SimpleTextFieldsReader.java?rev=1428343&r1=1428342&r2=1428343&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/codecs/src/java/org/apache/lucene/codecs/simpletext/SimpleTextFieldsReader.java (original)
+++ lucene/dev/branches/branch_4x/lucene/codecs/src/java/org/apache/lucene/codecs/simpletext/SimpleTextFieldsReader.java Thu Jan  3 13:42:30 2013
@@ -24,13 +24,12 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.TreeMap;
-import java.util.TreeSet;
 
 import org.apache.lucene.codecs.FieldsProducer;
 import org.apache.lucene.index.DocsAndPositionsEnum;
 import org.apache.lucene.index.DocsEnum;
-import org.apache.lucene.index.FieldInfo;
 import org.apache.lucene.index.FieldInfo.IndexOptions;
+import org.apache.lucene.index.FieldInfo;
 import org.apache.lucene.index.FieldInfos;
 import org.apache.lucene.index.SegmentReadState;
 import org.apache.lucene.index.Terms;
@@ -40,6 +39,7 @@ import org.apache.lucene.util.ArrayUtil;
 import org.apache.lucene.util.Bits;
 import org.apache.lucene.util.BytesRef;
 import org.apache.lucene.util.CharsRef;
+import org.apache.lucene.util.IOUtils;
 import org.apache.lucene.util.IntsRef;
 import org.apache.lucene.util.OpenBitSet;
 import org.apache.lucene.util.StringHelper;
@@ -67,10 +67,17 @@ class SimpleTextFieldsReader extends Fie
   final static BytesRef PAYLOAD      = SimpleTextFieldsWriter.PAYLOAD;
 
   public SimpleTextFieldsReader(SegmentReadState state) throws IOException {
-    in = state.dir.openInput(SimpleTextPostingsFormat.getPostingsFileName(state.segmentInfo.name, state.segmentSuffix), state.context);
-   
     fieldInfos = state.fieldInfos;
-    fields = readFields(in.clone());
+    in = state.dir.openInput(SimpleTextPostingsFormat.getPostingsFileName(state.segmentInfo.name, state.segmentSuffix), state.context);
+    boolean success = false;
+    try {
+      fields = readFields(in.clone());
+      success = true;
+    } finally {
+      if (!success) {
+        IOUtils.closeWhileHandlingException(this);
+      }
+    }
   }
   
   private TreeMap<String,Long> readFields(IndexInput in) throws IOException {

Modified: lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/codecs/lucene40/BitVector.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/codecs/lucene40/BitVector.java?rev=1428343&r1=1428342&r2=1428343&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/codecs/lucene40/BitVector.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/codecs/lucene40/BitVector.java Thu Jan  3 13:42:30 2013
@@ -26,6 +26,7 @@ import org.apache.lucene.store.Directory
 import org.apache.lucene.store.IOContext;
 import org.apache.lucene.store.IndexInput;
 import org.apache.lucene.store.IndexOutput;
+import org.apache.lucene.util.IOUtils;
 import org.apache.lucene.util.MutableBits;
 
 /** Optimized implementation of a vector of bits.  This is more-or-less like
@@ -238,7 +239,7 @@ final class BitVector implements Cloneab
       }
       assert verifyCount();
     } finally {
-      output.close();
+      IOUtils.close(output);
     }
   }
 

Modified: lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/ReadersAndLiveDocs.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/ReadersAndLiveDocs.java?rev=1428343&r1=1428342&r2=1428343&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/ReadersAndLiveDocs.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/ReadersAndLiveDocs.java Thu Jan  3 13:42:30 2013
@@ -23,6 +23,7 @@ import java.util.concurrent.atomic.Atomi
 import org.apache.lucene.codecs.LiveDocsFormat;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.IOContext;
+import org.apache.lucene.store.TrackingDirectoryWrapper;
 import org.apache.lucene.util.Bits;
 import org.apache.lucene.util.MutableBits;
 
@@ -182,16 +183,28 @@ class ReadersAndLiveDocs {
 
   // NOTE: removes callers ref
   public synchronized void dropReaders() throws IOException {
-    if (reader != null) {
-      //System.out.println("  pool.drop info=" + info + " rc=" + reader.getRefCount());
-      reader.decRef();
-      reader = null;
-    }
-    if (mergeReader != null) {
-      //System.out.println("  pool.drop info=" + info + " merge rc=" + mergeReader.getRefCount());
-      mergeReader.decRef();
-      mergeReader = null;
+    // TODO: can we somehow use IOUtils here...?  problem is
+    // we are calling .decRef not .close)...
+    try {
+      if (reader != null) {
+        //System.out.println("  pool.drop info=" + info + " rc=" + reader.getRefCount());
+        try {
+          reader.decRef();
+        } finally {
+          reader = null;
+        }
+      }
+    } finally {
+      if (mergeReader != null) {
+        //System.out.println("  pool.drop info=" + info + " merge rc=" + mergeReader.getRefCount());
+        try {
+          mergeReader.decRef();
+        } finally {
+          mergeReader = null;
+        }
+      }
     }
+
     decRef();
   }
 
@@ -272,13 +285,37 @@ class ReadersAndLiveDocs {
       // We have new deletes
       assert liveDocs.length() == info.info.getDocCount();
 
+      // Do this so we can delete any created files on
+      // exception; this saves all codecs from having to do
+      // it:
+      TrackingDirectoryWrapper trackingDir = new TrackingDirectoryWrapper(dir);
+
       // We can write directly to the actual name (vs to a
       // .tmp & renaming it) because the file is not live
       // until segments file is written:
-      info.info.getCodec().liveDocsFormat().writeLiveDocs((MutableBits)liveDocs, dir, info, pendingDeleteCount, IOContext.DEFAULT);
+      boolean success = false;
+      try {
+        info.info.getCodec().liveDocsFormat().writeLiveDocs((MutableBits)liveDocs, trackingDir, info, pendingDeleteCount, IOContext.DEFAULT);
+        success = true;
+      } finally {
+        if (!success) {
+          // Advance only the nextWriteDelGen so that a 2nd
+          // attempt to write will write to a new file
+          info.advanceNextWriteDelGen();
+
+          // Delete any partially created file(s):
+          for(String fileName : trackingDir.getCreatedFiles()) {
+            try {
+              dir.deleteFile(fileName);
+            } catch (Throwable t) {
+              // Ignore so we throw only the first exc
+            }
+          }
+        }
+      }
 
       // If we hit an exc in the line above (eg disk full)
-      // then info remains pointing to the previous
+      // then info's delGen remains pointing to the previous
       // (successfully written) del docs:
       info.advanceDelGen();
       info.setDelCount(info.getDelCount() + pendingDeleteCount);

Modified: lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/SegmentInfoPerCommit.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/SegmentInfoPerCommit.java?rev=1428343&r1=1428342&r2=1428343&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/SegmentInfoPerCommit.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/SegmentInfoPerCommit.java Thu Jan  3 13:42:30 2013
@@ -40,6 +40,10 @@ public class SegmentInfoPerCommit {
   // are no deletes yet):
   private long delGen;
 
+  // Normally 1+delGen, unless an exception was hit on last
+  // attempt to write:
+  private long nextWriteDelGen;
+
   private volatile long sizeInBytes = -1;
 
   /** Sole constructor.
@@ -52,17 +56,27 @@ public class SegmentInfoPerCommit {
     this.info = info;
     this.delCount = delCount;
     this.delGen = delGen;
-  }
-
-  void advanceDelGen() {
     if (delGen == -1) {
-      delGen = 1;
+      nextWriteDelGen = 1;
     } else {
-      delGen++;
+      nextWriteDelGen = delGen+1;
     }
+  }
+
+  /** Called when we succeed in writing deletes */
+  void advanceDelGen() {
+    delGen = nextWriteDelGen;
+    nextWriteDelGen = delGen+1;
     sizeInBytes = -1;
   }
 
+  /** Called if there was an exception while writing
+   *  deletes, so that we don't try to write to the same
+   *  file more than once. */
+  void advanceNextWriteDelGen() {
+    nextWriteDelGen++;
+  }
+
   /** Returns total size in bytes of all files for this
    *  segment. 
    * <p><b>NOTE:</b> This value is not correct for 3.0 segments
@@ -128,11 +142,7 @@ public class SegmentInfoPerCommit {
    * of the live docs file.
    */
   public long getNextDelGen() {
-    if (delGen == -1) {
-      return 1;
-    } else {
-      return delGen + 1;
-    }
+    return nextWriteDelGen;
   }
 
   /**
@@ -171,6 +181,12 @@ public class SegmentInfoPerCommit {
 
   @Override
   public SegmentInfoPerCommit clone() {
-    return new SegmentInfoPerCommit(info, delCount, delGen);
+    SegmentInfoPerCommit other = new SegmentInfoPerCommit(info, delCount, delGen);
+    // Not clear that we need to carry over nextWriteDelGen
+    // (i.e. do we ever clone after a failed write and
+    // before the next successful write?), but just do it to
+    // be safe:
+    other.nextWriteDelGen = nextWriteDelGen;
+    return other;
   }
 }

Modified: lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java?rev=1428343&r1=1428342&r2=1428343&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java Thu Jan  3 13:42:30 2013
@@ -1023,9 +1023,16 @@ public class TestIndexWriter extends Luc
             w = new IndexWriter(dir, conf);
 
             Document doc = new Document();
+            Field idField = newStringField("id", "", Field.Store.NO);
+            doc.add(idField);
             doc.add(newField("field", "some text contents", storedTextType));
             for(int i=0;i<100;i++) {
-              w.addDocument(doc);
+              idField.setStringValue(Integer.toString(i));
+              if (i%2 == 0) {
+                w.updateDocument(new Term("id", idField.stringValue()), doc);
+              } else {
+                w.addDocument(doc);
+              }
               if (i%10 == 0) {
                 w.commit();
               }
@@ -1046,10 +1053,12 @@ public class TestIndexWriter extends Luc
             allowInterrupt = true;
           }
         } catch (ThreadInterruptedException re) {
-          if (true || VERBOSE) {
-            System.out.println("TEST: got interrupt");
-            re.printStackTrace(System.out);
-          }
+          // NOTE: important to leave this verbosity/noise
+          // on!!  This test doesn't repro easily so when
+          // Jenkins hits a fail we need to study where the
+          // interrupts struck!
+          System.out.println("TEST: got interrupt");
+          re.printStackTrace(System.out);
           Throwable e = re.getCause();
           assertTrue(e instanceof InterruptedException);
           if (finish) {
@@ -1114,6 +1123,8 @@ public class TestIndexWriter extends Luc
     final int numInterrupts = atLeast(300);
     int i = 0;
     while(i < numInterrupts) {
+      // TODO: would be nice to also sometimes interrupt the
+      // CMS merge threads too ...
       Thread.sleep(10);
       if (t.allowInterrupt) {
         i++;
@@ -1238,7 +1249,6 @@ public class TestIndexWriter extends Luc
     Directory dir = newDirectory();
     IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig(
         TEST_VERSION_CURRENT, new MockAnalyzer(random())));
-    ByteArrayOutputStream bos = new ByteArrayOutputStream(1024);
     writer.addDocument(new Document());
     writer.close();