LUCENE-846: don't corrupt IndexWriter instance on hitting exception (eg disk full) during addIndexes(*) when writer is opened with autoCommit=false

git-svn-id: https://svn.apache.org/repos/asf/lucene/java/trunk@522262 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -80,6 +80,13 @@ Bug fixes
 11. LUCENE-828: Minor fix for Term's equal().
     (Paul Cowan via Otis Gospodnetic)
 
+12. LUCENE-846: Fixed: if IndexWriter is opened with autoCommit=false,
+    and you call addIndexes, and hit an exception (eg disk full) then
+    when IndexWriter rolls back its internal state this could corrupt
+    the instance of IndexWriter (but, not the index itself) by
+    referencing already deleted segments.  This bug was only present
+    in 2.2 (trunk), ie was never released.  (Mike McCandless)
+
 New features
 
  1. LUCENE-759: Added two n-gram-producing TokenFilters.

src/java/org/apache/lucene/index/IndexFileDeleter.java

@@ -343,7 +343,7 @@ final class IndexFileDeleter {
     }
   }
 
-  private void incRef(SegmentInfos segmentInfos, boolean isCommit) throws IOException {
+  void incRef(SegmentInfos segmentInfos, boolean isCommit) throws IOException {
     int size = segmentInfos.size();
     for(int i=0;i<size;i++) {
       SegmentInfo segmentInfo = segmentInfos.info(i);
@@ -391,6 +391,16 @@ final class IndexFileDeleter {
     }
   }
 
+  void decRef(SegmentInfos segmentInfos) throws IOException {
+    final int size = segmentInfos.size();
+    for(int i=0;i<size;i++) {
+      SegmentInfo segmentInfo = segmentInfos.info(i);
+      if (segmentInfo.dir == directory) {
+        decRef(segmentInfo.files());
+      }
+    }
+  }
+
   private RefCount getRefCount(String fileName) {
     RefCount rc;
     if (!refCounts.containsKey(fileName)) {

src/java/org/apache/lucene/index/IndexWriter.java

@@ -1206,7 +1206,10 @@ public class IndexWriter {
       flushRamSegments();
       // Turn off auto-commit during our local transaction:
       autoCommit = false;
-    }
+    } else
+      // We must "protect" our files at this point from
+      // deletion in case we need to rollback:
+      deleter.incRef(segmentInfos, false);
   }
 
   /*
@@ -1229,6 +1232,11 @@ public class IndexWriter {
     // Ask deleter to locate unreferenced files we had
     // created & remove them:
     deleter.checkpoint(segmentInfos, false);
+
+    if (!autoCommit)
+      // Remove the incRef we did in startTransaction:
+      deleter.decRef(segmentInfos);
+
     deleter.refresh();
   }
 
@@ -1251,6 +1259,11 @@ public class IndexWriter {
         rollbackTransaction();
       }
     }
+
+    if (!autoCommit)
+      // Remove the incRef we did in startTransaction.
+      deleter.decRef(localRollbackSegmentInfos);
+
     localRollbackSegmentInfos = null;
 
     // Give deleter a chance to remove files now:

src/test/org/apache/lucene/index/TestIndexWriter.java

@@ -199,8 +199,6 @@ public class TestIndexWriter extends TestCase
           methodName = "addIndexesNoOptimize(Directory[])";
         }
 
-        String testName = "disk full test for method " + methodName + " with disk full at " + diskFree + " bytes with autoCommit = " + autoCommit;
-
         int cycleCount = 0;
 
         while(!done) {
@@ -222,6 +220,8 @@ public class TestIndexWriter extends TestCase
             double diskRatio = ((double) diskFree)/diskUsage;
             long thisDiskFree;
 
+            String testName = null;
+
             if (0 == x) {
               thisDiskFree = diskFree;
               if (diskRatio >= 2.0) {
@@ -233,17 +233,18 @@ public class TestIndexWriter extends TestCase
               if (diskRatio >= 6.0) {
                 rate = 0.0;
               }
-              if (debug) {
+              if (debug)
-                System.out.println("\ncycle: " + methodName + ": " + diskFree + " bytes");
+                testName = "disk full test " + methodName + " with disk full at " + diskFree + " bytes autoCommit=" + autoCommit;
-              }
             } else {
               thisDiskFree = 0;
               rate = 0.0;
-              if (debug) {
+              if (debug)
-                System.out.println("\ncycle: " + methodName + ", same writer: unlimited disk space");
+                testName = "disk full test " + methodName + " with unlimited disk space autoCommit=" + autoCommit;
-              }
             }
 
+            if (debug)
+              System.out.println("\ncycle: " + testName);
+
             dir.setMaxSizeInBytes(thisDiskFree);
             dir.setRandomIOExceptionRate(rate, diskFree);
 
@@ -281,10 +282,11 @@ public class TestIndexWriter extends TestCase
               err = e;
               if (debug) {
                 System.out.println("  hit IOException: " + e);
+                // e.printStackTrace(System.out);
               }
 
               if (1 == x) {
-                e.printStackTrace();
+                e.printStackTrace(System.out);
                 fail(methodName + " hit IOException after disk space was freed up");
               }
             }
@@ -323,7 +325,7 @@ public class TestIndexWriter extends TestCase
             try {
               reader = IndexReader.open(dir);
             } catch (IOException e) {
-              e.printStackTrace();
+              e.printStackTrace(System.out);
               fail(testName + ": exception when creating IndexReader: " + e);
             }
             int result = reader.docFreq(searchTerm);
@@ -337,7 +339,7 @@ public class TestIndexWriter extends TestCase
               // On hitting exception we still may have added
               // all docs:
               if (result != START_COUNT && result != END_COUNT) {
-                err.printStackTrace();
+                err.printStackTrace(System.out);
                 fail(testName + ": method did throw exception but docFreq('aaa') is " + result + " instead of expected " + START_COUNT + " or " + END_COUNT);
               }
             }
@@ -346,7 +348,7 @@ public class TestIndexWriter extends TestCase
             try {
               hits = searcher.search(new TermQuery(searchTerm));
             } catch (IOException e) {
-              e.printStackTrace();
+              e.printStackTrace(System.out);
               fail(testName + ": exception when searching: " + e);
             }
             int result2 = hits.length();
@@ -358,7 +360,7 @@ public class TestIndexWriter extends TestCase
               // On hitting exception we still may have added
               // all docs:
               if (result2 != result) {
-                err.printStackTrace();
+                err.printStackTrace(System.out);
                 fail(testName + ": method did throw exception but hits.length for search on term 'aaa' is " + result2 + " instead of expected " + result);
               }
             }