lucene-java-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mikemcc...@apache.org
Subject svn commit: r886288 - in /lucene/java/branches/lucene_2_9: ./ contrib/ contrib/highlighter/src/test/ src/java/org/apache/lucene/index/ src/java/org/apache/lucene/search/ src/java/org/apache/lucene/store/ src/test/org/apache/lucene/analysis/ src/test/or...
Date Wed, 02 Dec 2009 19:43:22 GMT
Author: mikemccand
Date: Wed Dec  2 19:43:22 2009
New Revision: 886288

URL: http://svn.apache.org/viewvc?rev=886288&view=rev
Log:
LUCENE-2095 (on 3.0 branch): fix thread safety issue with IW.commit

Modified:
    lucene/java/branches/lucene_2_9/   (props changed)
    lucene/java/branches/lucene_2_9/CHANGES.txt   (contents, props changed)
    lucene/java/branches/lucene_2_9/contrib/   (props changed)
    lucene/java/branches/lucene_2_9/contrib/CHANGES.txt   (props changed)
    lucene/java/branches/lucene_2_9/contrib/highlighter/src/test/   (props changed)
    lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/index/IndexWriter.java
    lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/search/MultiTermQueryWrapperFilter.java
  (props changed)
    lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/store/RAMDirectory.java
    lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/store/RAMFile.java
    lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/analysis/BaseTokenStreamTestCase.java
  (props changed)
    lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/analysis/TestISOLatin1AccentFilter.java
  (props changed)
    lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/document/TestDateTools.java
  (props changed)
    lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/document/TestNumberTools.java
  (props changed)
    lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java
  (props changed)
    lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/index/TestIndexWriter.java
    lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/store/TestRAMDirectory.java
    lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/util/TestAttributeSource.java
  (props changed)

Propchange: lucene/java/branches/lucene_2_9/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Dec  2 19:43:22 2009
@@ -1,2 +1,3 @@
 /lucene/java/branches/lucene_2_4:748824
-/lucene/java/trunk:824125,826029,826385,830871,833095,833297,833886,882672,883554,884870
+/lucene/java/branches/lucene_3_0:886275
+/lucene/java/trunk:824125,826029,826385,830871,833095,833297,833886,882672,883554,884870,886257

Modified: lucene/java/branches/lucene_2_9/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/java/branches/lucene_2_9/CHANGES.txt?rev=886288&r1=886287&r2=886288&view=diff
==============================================================================
--- lucene/java/branches/lucene_2_9/CHANGES.txt (original)
+++ lucene/java/branches/lucene_2_9/CHANGES.txt Wed Dec  2 19:43:22 2009
@@ -16,6 +16,11 @@
  * LUCENE-2088: addAttribute() should only accept interfaces that
    extend Attribute. (Shai Erera, Uwe Schindler)
 
+ * LUCENE-2095: Fixes: when two threads call IndexWriter.commit() at
+   the same time, it's possible for commit to return control back to
+   one of the threads before all changes are actually committed.
+   (Sanne Grinovero via Mike McCandless)
+
 Optimizations
 
  * LUCENE-2086: When resolving deleted terms, do so in term sort order

Propchange: lucene/java/branches/lucene_2_9/CHANGES.txt
------------------------------------------------------------------------------
    svn:mergeinfo = /lucene/java/trunk/CHANGES.txt:886257

Propchange: lucene/java/branches/lucene_2_9/contrib/
------------------------------------------------------------------------------
    svn:mergeinfo = /lucene/java/trunk/contrib:886257

Propchange: lucene/java/branches/lucene_2_9/contrib/CHANGES.txt
------------------------------------------------------------------------------
    svn:mergeinfo = /lucene/java/trunk/contrib/CHANGES.txt:886257

Propchange: lucene/java/branches/lucene_2_9/contrib/highlighter/src/test/
------------------------------------------------------------------------------
    svn:mergeinfo = /lucene/java/trunk/contrib/highlighter/src/test:886257

Modified: lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/index/IndexWriter.java
URL: http://svn.apache.org/viewvc/lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/index/IndexWriter.java?rev=886288&r1=886287&r2=886288&view=diff
==============================================================================
--- lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/index/IndexWriter.java (original)
+++ lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/index/IndexWriter.java Wed
Dec  2 19:43:22 2009
@@ -4075,9 +4075,14 @@
     startCommit(0, commitUserData);
   }
 
+  // Used only by commit, below; lock order is commitLock -> IW
+  private final Object commitLock = new Object();
+
   private void commit(long sizeInBytes) throws IOException {
-    startCommit(sizeInBytes, null);
-    finishCommit();
+    synchronized(commitLock) {
+      startCommit(sizeInBytes, null);
+      finishCommit();
+    }
   }
 
   /**
@@ -4127,17 +4132,24 @@
 
     ensureOpen();
 
-    if (infoStream != null)
+    if (infoStream != null) {
       message("commit: start");
+    }
 
-    if (autoCommit || pendingCommit == null) {
-      if (infoStream != null)
-        message("commit: now prepare");
-      prepareCommit(commitUserData, true);
-    } else if (infoStream != null)
-      message("commit: already prepared");
+    synchronized(commitLock) {
+      if (infoStream != null) {
+        message("commit: enter lock");
+      }
+      if (autoCommit || pendingCommit == null) {
+        if (infoStream != null)
+          message("commit: now prepare");
+        prepareCommit(commitUserData, true);
+      } else if (infoStream != null) {
+        message("commit: already prepared");
+      }
 
-    finishCommit();
+      finishCommit();
+    }
   }
 
   private synchronized final void finishCommit() throws CorruptIndexException, IOException
{
@@ -5366,6 +5378,9 @@
 
     assert testPoint("startStartCommit");
 
+    // TODO: as of LUCENE-2095, we can simplify this method,
+    // since only 1 thread can be in here at once
+
     if (hitOOM) {
       throw new IllegalStateException("this writer hit an OutOfMemoryError; cannot commit");
     }

Propchange: lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/search/MultiTermQueryWrapperFilter.java
------------------------------------------------------------------------------
--- svn:mergeinfo (added)
+++ svn:mergeinfo Wed Dec  2 19:43:22 2009
@@ -0,0 +1 @@
+/lucene/java/trunk/src/java/org/apache/lucene/search/MultiTermQueryWrapperFilter.java:886257

Modified: lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/store/RAMDirectory.java
URL: http://svn.apache.org/viewvc/lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/store/RAMDirectory.java?rev=886288&r1=886287&r2=886288&view=diff
==============================================================================
--- lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/store/RAMDirectory.java (original)
+++ lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/store/RAMDirectory.java Wed
Dec  2 19:43:22 2009
@@ -37,7 +37,7 @@
   private static final long serialVersionUID = 1l;
 
   HashMap fileMap = new HashMap();
-  long sizeInBytes = 0;
+  long sizeInBytes;
   
   // *****
   // Lock acquisition sequence:  RAMDirectory, then RAMFile
@@ -195,7 +195,7 @@
     if (file!=null) {
         fileMap.remove(name);
         file.directory = null;
-        sizeInBytes -= file.sizeInBytes;       // updates to RAMFile.sizeInBytes synchronized
on directory
+        sizeInBytes -= file.sizeInBytes;
     } else
       throw new FileNotFoundException(name);
   }

Modified: lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/store/RAMFile.java
URL: http://svn.apache.org/viewvc/lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/store/RAMFile.java?rev=886288&r1=886287&r2=886288&view=diff
==============================================================================
--- lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/store/RAMFile.java (original)
+++ lucene/java/branches/lucene_2_9/src/java/org/apache/lucene/store/RAMFile.java Wed Dec
 2 19:43:22 2009
@@ -27,7 +27,7 @@
   private ArrayList buffers = new ArrayList();
   long length;
   RAMDirectory directory;
-  long sizeInBytes;                  // Only maintained if in a directory; updates synchronized
on directory
+  long sizeInBytes;
 
   // This is publicly modifiable via Directory.touchFile(), so direct access not supported
   private long lastModified = System.currentTimeMillis();
@@ -57,16 +57,18 @@
     this.lastModified = lastModified;
   }
 
-  final synchronized byte[] addBuffer(int size) {
+  final byte[] addBuffer(int size) {
     byte[] buffer = newBuffer(size);
-    if (directory!=null)
-      synchronized (directory) {             // Ensure addition of buffer and adjustment
to directory size are atomic wrt directory
-        buffers.add(buffer);
+    synchronized(this) {
+      buffers.add(buffer);
+      sizeInBytes += size;
+    }
+
+    if (directory != null) {
+      synchronized(directory) {
         directory.sizeInBytes += size;
-        sizeInBytes += size;
       }
-    else
-      buffers.add(buffer);
+    }
     return buffer;
   }
 
@@ -88,9 +90,8 @@
     return new byte[size];
   }
 
-  // Only valid if in a directory
-  long getSizeInBytes() {
-    synchronized (directory) {
+  synchronized long getSizeInBytes() {
+    synchronized(directory) {
       return sizeInBytes;
     }
   }

Propchange: lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/analysis/BaseTokenStreamTestCase.java
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Dec  2 19:43:22 2009
@@ -1,2 +1,3 @@
 /lucene/java/branches/lucene_2_4/src/test/org/apache/lucene/analysis/BaseTokenStreamTestCase.java:748824
+/lucene/java/branches/lucene_3_0/src/test/org/apache/lucene/analysis/BaseTokenStreamTestCase.java:886275
 /lucene/java/trunk/src/test/org/apache/lucene/analysis/BaseTokenStreamTestCase.java:818920,824125,826029,826385,830871,833095,833297,833886,882672,883554,884870

Propchange: lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/analysis/TestISOLatin1AccentFilter.java
------------------------------------------------------------------------------
--- svn:mergeinfo (added)
+++ svn:mergeinfo Wed Dec  2 19:43:22 2009
@@ -0,0 +1 @@
+/lucene/java/trunk/src/test/org/apache/lucene/analysis/TestISOLatin1AccentFilter.java:886257

Propchange: lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/document/TestDateTools.java
------------------------------------------------------------------------------
--- svn:mergeinfo (added)
+++ svn:mergeinfo Wed Dec  2 19:43:22 2009
@@ -0,0 +1 @@
+/lucene/java/trunk/src/test/org/apache/lucene/document/TestDateTools.java:886257

Propchange: lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/document/TestNumberTools.java
------------------------------------------------------------------------------
--- svn:mergeinfo (added)
+++ svn:mergeinfo Wed Dec  2 19:43:22 2009
@@ -0,0 +1 @@
+/lucene/java/trunk/src/test/org/apache/lucene/document/TestNumberTools.java:886257

Propchange: lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java
------------------------------------------------------------------------------
--- svn:mergeinfo (added)
+++ svn:mergeinfo Wed Dec  2 19:43:22 2009
@@ -0,0 +1 @@
+/lucene/java/trunk/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java:886257

Modified: lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/index/TestIndexWriter.java
URL: http://svn.apache.org/viewvc/lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/index/TestIndexWriter.java?rev=886288&r1=886287&r2=886288&view=diff
==============================================================================
--- lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/index/TestIndexWriter.java
(original)
+++ lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/index/TestIndexWriter.java
Wed Dec  2 19:43:22 2009
@@ -31,6 +31,7 @@
 import java.util.Map;
 import java.util.HashSet;
 import java.util.Random;
+import java.util.Collections;
 
 import org.apache.lucene.analysis.BaseTokenStreamTestCase;
 import org.apache.lucene.analysis.Analyzer;
@@ -4684,4 +4685,56 @@
     _TestUtil.checkIndex(dir);
     dir.close();
   }
+
+  // LUCENE-2095: make sure with multiple threads commit
+  // doesn't return until all changes are in fact in the
+  // index
+  public void testCommitThreadSafety() throws Throwable {
+    final int NUM_THREADS = 5;
+    final double RUN_SEC = 0.5;
+    final Directory dir = new MockRAMDirectory();
+    final IndexWriter w = new IndexWriter(dir, new SimpleAnalyzer(), IndexWriter.MaxFieldLength.UNLIMITED);
+    w.commit();
+    final List failed = Collections.synchronizedList(new ArrayList());
+    Thread[] threads = new Thread[NUM_THREADS];
+    final long endTime = System.currentTimeMillis()+((long) (RUN_SEC*1000));
+    for(int i=0;i<NUM_THREADS;i++) {
+      final int finalI = i;
+      threads[i] = new Thread() {
+          public void run() {
+            try {
+              final Document doc = new Document();
+              IndexReader r = IndexReader.open(dir);
+              Field f = new Field("f", "", Field.Store.NO, Field.Index.NOT_ANALYZED);
+              doc.add(f);
+              int count = 0;
+              while(System.currentTimeMillis() < endTime && failed.size() == 0)
{
+                for(int j=0;j<10;j++) {
+                  final String s = finalI + "_" + String.valueOf(count++);
+                  f.setValue(s);
+                  w.addDocument(doc);
+                  w.commit();
+                  IndexReader r2 = r.reopen();
+                  assertTrue(r2 != r);
+                  r.close();
+                  r = r2;
+                  assertEquals("term=f:" + s, 1, r.docFreq(new Term("f", s)));
+                }
+              }
+              r.close();
+            } catch (Throwable t) {
+              failed.add(this);
+              throw new RuntimeException(t);
+            }
+          }
+        };
+      threads[i].start();
+    }
+    for(int i=0;i<NUM_THREADS;i++) {
+      threads[i].join();
+    }
+    w.close();
+    dir.close();
+    assertEquals(0, failed.size());
+  }
 }

Modified: lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/store/TestRAMDirectory.java
URL: http://svn.apache.org/viewvc/lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/store/TestRAMDirectory.java?rev=886288&r1=886287&r2=886288&view=diff
==============================================================================
--- lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/store/TestRAMDirectory.java
(original)
+++ lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/store/TestRAMDirectory.java
Wed Dec  2 19:43:22 2009
@@ -147,7 +147,8 @@
     searcher.close();
   }
   
-  private final int numThreads = 50;
+  private final int numThreads = 10;
+
   private final int docsPerThread = 40;
   
   public void testRAMDirectorySize() throws IOException, InterruptedException {
@@ -171,9 +172,6 @@
             } catch (IOException e) {
               throw new RuntimeException(e);
             }
-            synchronized (ramDir) {
-              assertEquals(ramDir.sizeInBytes(), ramDir.getRecomputedSizeInBytes());
-            }
           }
         }
       };

Propchange: lucene/java/branches/lucene_2_9/src/test/org/apache/lucene/util/TestAttributeSource.java
------------------------------------------------------------------------------
--- svn:mergeinfo (added)
+++ svn:mergeinfo Wed Dec  2 19:43:22 2009
@@ -0,0 +1 @@
+/lucene/java/trunk/src/test/org/apache/lucene/util/TestAttributeSource.java:886257



Mime
View raw message