lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mikemcc...@apache.org
Subject svn commit: r1570562 - in /lucene/dev/branches/lucene_solr_4_7: ./ lucene/ lucene/core/ lucene/core/src/java/org/apache/lucene/search/ lucene/core/src/test/org/apache/lucene/search/
Date Fri, 21 Feb 2014 12:49:48 GMT
Author: mikemccand
Date: Fri Feb 21 12:49:47 2014
New Revision: 1570562

URL: http://svn.apache.org/r1570562
Log:
LUCENE-5461: fix thread hazard in ControlledRealTimeReopenThread causing a possible too-long
wait time when a thread was waiting for a specific generation

Modified:
    lucene/dev/branches/lucene_solr_4_7/   (props changed)
    lucene/dev/branches/lucene_solr_4_7/lucene/   (props changed)
    lucene/dev/branches/lucene_solr_4_7/lucene/CHANGES.txt   (contents, props changed)
    lucene/dev/branches/lucene_solr_4_7/lucene/core/   (props changed)
    lucene/dev/branches/lucene_solr_4_7/lucene/core/src/java/org/apache/lucene/search/ControlledRealTimeReopenThread.java
    lucene/dev/branches/lucene_solr_4_7/lucene/core/src/test/org/apache/lucene/search/TestControlledRealTimeReopenThread.java

Modified: lucene/dev/branches/lucene_solr_4_7/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene_solr_4_7/lucene/CHANGES.txt?rev=1570562&r1=1570561&r2=1570562&view=diff
==============================================================================
--- lucene/dev/branches/lucene_solr_4_7/lucene/CHANGES.txt (original)
+++ lucene/dev/branches/lucene_solr_4_7/lucene/CHANGES.txt Fri Feb 21 12:49:47 2014
@@ -191,6 +191,11 @@ Bug fixes
 * LUCENE-5462: RamUsageEstimator.sizeOf(Object) is not used anymore to
   estimate memory usage of segments. This used to make
   SegmentReader.ramBytesUsed very CPU-intensive. (Adrien Grand)
+
+* LUCENE-5461: ControlledRealTimeReopenThread would sometimes wait too
+  long (up to targetMaxStaleSec) when a searcher is waiting for a
+  specific generation, when it should have waited for at most
+  targetMinStaleSec. (Hans Lund via Mike McCandless)
   
 API Changes
 

Modified: lucene/dev/branches/lucene_solr_4_7/lucene/core/src/java/org/apache/lucene/search/ControlledRealTimeReopenThread.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene_solr_4_7/lucene/core/src/java/org/apache/lucene/search/ControlledRealTimeReopenThread.java?rev=1570562&r1=1570561&r2=1570562&view=diff
==============================================================================
--- lucene/dev/branches/lucene_solr_4_7/lucene/core/src/java/org/apache/lucene/search/ControlledRealTimeReopenThread.java
(original)
+++ lucene/dev/branches/lucene_solr_4_7/lucene/core/src/java/org/apache/lucene/search/ControlledRealTimeReopenThread.java
Fri Feb 21 12:49:47 2014
@@ -87,11 +87,11 @@ public class ControlledRealTimeReopenThr
 
     @Override
     public void afterRefresh(boolean didRefresh) {
-      refreshDone(didRefresh);
+      refreshDone();
     }
   }
 
-  private synchronized void refreshDone(boolean didRefresh) {
+  private synchronized void refreshDone() {
     searchingGen = refreshStartGen;
     notifyAll();
   }
@@ -160,12 +160,15 @@ public class ControlledRealTimeReopenThr
       throw new IllegalArgumentException("targetGen=" + targetGen + " was never returned
by the ReferenceManager instance (current gen=" + curGen + ")");
     }
     if (targetGen > searchingGen) {
-      waitingGen = Math.max(waitingGen, targetGen);
-
       // Notify the reopen thread that the waitingGen has
       // changed, so it may wake up and realize it should
       // not sleep for much or any longer before reopening:
       reopenLock.lock();
+
+      // Need to find waitingGen inside lock as its used to determine
+      // stale time
+      waitingGen = Math.max(waitingGen, targetGen);
+
       try {
         reopenCond.signal();
       } finally {
@@ -178,7 +181,7 @@ public class ControlledRealTimeReopenThr
         if (maxMS < 0) {
           wait();
         } else {
-          long msLeft = ((startMS + maxMS) - (System.nanoTime())/1000000);
+          long msLeft = (startMS + maxMS) - (System.nanoTime())/1000000;
           if (msLeft <= 0) {
             return false;
           } else {
@@ -207,24 +210,25 @@ public class ControlledRealTimeReopenThr
       // next reopen:
       while (!finish) {
 
-        // True if we have someone waiting for reopened searcher:
-        boolean hasWaiting = waitingGen > searchingGen;
-        final long nextReopenStartNS = lastReopenStartNS + (hasWaiting ? targetMinStaleNS
: targetMaxStaleNS);
-
-        final long sleepNS = nextReopenStartNS - System.nanoTime();
-
-        if (sleepNS > 0) {
-          reopenLock.lock();
-          try {
+        // Need lock before finding out if has waiting
+        reopenLock.lock();
+        try {
+          // True if we have someone waiting for reopened searcher:
+          boolean hasWaiting = waitingGen > searchingGen;
+          final long nextReopenStartNS = lastReopenStartNS + (hasWaiting ? targetMinStaleNS
: targetMaxStaleNS);
+
+          final long sleepNS = nextReopenStartNS - System.nanoTime();
+
+          if (sleepNS > 0) {
             reopenCond.awaitNanos(sleepNS);
-          } catch (InterruptedException ie) {
-            Thread.currentThread().interrupt();
-            return;
-          } finally {
-            reopenLock.unlock();
+          } else {
+            break;
           }
-        } else {
-          break;
+        } catch (InterruptedException ie) {
+          Thread.currentThread().interrupt();
+          return;
+        } finally {
+          reopenLock.unlock();
         }
       }
 

Modified: lucene/dev/branches/lucene_solr_4_7/lucene/core/src/test/org/apache/lucene/search/TestControlledRealTimeReopenThread.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene_solr_4_7/lucene/core/src/test/org/apache/lucene/search/TestControlledRealTimeReopenThread.java?rev=1570562&r1=1570561&r2=1570562&view=diff
==============================================================================
--- lucene/dev/branches/lucene_solr_4_7/lucene/core/src/test/org/apache/lucene/search/TestControlledRealTimeReopenThread.java
(original)
+++ lucene/dev/branches/lucene_solr_4_7/lucene/core/src/test/org/apache/lucene/search/TestControlledRealTimeReopenThread.java
Fri Feb 21 12:49:47 2014
@@ -18,6 +18,7 @@ package org.apache.lucene.search;
  */
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutorService;
@@ -27,13 +28,17 @@ import org.apache.lucene.analysis.Analyz
 import org.apache.lucene.analysis.MockAnalyzer;
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.Field;
+import org.apache.lucene.document.TextField;
 import org.apache.lucene.index.DirectoryReader;
+import org.apache.lucene.index.IndexCommit;
 import org.apache.lucene.index.IndexReader;
 import org.apache.lucene.index.IndexWriter;
 import org.apache.lucene.index.IndexWriterConfig;
 import org.apache.lucene.index.IndexableField;
+import org.apache.lucene.index.KeepOnlyLastCommitDeletionPolicy;
 import org.apache.lucene.index.NoMergePolicy;
 import org.apache.lucene.index.RandomIndexWriter;
+import org.apache.lucene.index.SnapshotDeletionPolicy;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.index.ThreadedIndexingAndSearchingTestCase;
 import org.apache.lucene.index.TrackingIndexWriter;
@@ -42,7 +47,9 @@ import org.apache.lucene.store.NRTCachin
 import org.apache.lucene.util.IOUtils;
 import org.apache.lucene.util.LuceneTestCase.SuppressCodecs;
 import org.apache.lucene.util.LuceneTestCase;
+import org.apache.lucene.util._TestUtil;
 import org.apache.lucene.util.ThreadInterruptedException;
+import org.apache.lucene.util.Version;
 
 @SuppressCodecs({ "SimpleText", "Memory", "Direct" })
 public class TestControlledRealTimeReopenThread extends ThreadedIndexingAndSearchingTestCase
{
@@ -414,6 +421,7 @@ public class TestControlledRealTimeReope
 
     try {
       new SearcherManager(w.w, false, theEvilOne);
+      fail("didn't hit expected exception");
     } catch (IllegalStateException ise) {
       // expected
     }
@@ -447,4 +455,83 @@ public class TestControlledRealTimeReope
     iw.close();
     dir.close();
   }
+
+  // LUCENE-5461
+  public void testCRTReopen() throws Exception {
+    //test behaving badly
+
+    //should be high enough
+    int maxStaleSecs = 20;
+
+    //build crap data just to store it.
+    String s = "        abcdefghijklmnopqrstuvwxyz     ";
+    char[] chars = s.toCharArray();
+    StringBuilder builder = new StringBuilder(2048);
+    for (int i = 0; i < 2048; i++) {
+      builder.append(chars[random().nextInt(chars.length)]);
+    }
+    String content = builder.toString();
+
+    final SnapshotDeletionPolicy sdp = new SnapshotDeletionPolicy(new KeepOnlyLastCommitDeletionPolicy());
+    final Directory dir = new NRTCachingDirectory(newFSDirectory(_TestUtil.getTempDir("nrt")),
5, 128);
+    IndexWriterConfig config = new IndexWriterConfig(Version.LUCENE_46,
+                                                     new MockAnalyzer(random()));
+    config.setIndexDeletionPolicy(sdp);
+    config.setOpenMode(IndexWriterConfig.OpenMode.CREATE_OR_APPEND);
+    final IndexWriter iw = new IndexWriter(dir, config);
+    SearcherManager sm = new SearcherManager(iw, true, new SearcherFactory());
+    final TrackingIndexWriter tiw = new TrackingIndexWriter(iw);
+    ControlledRealTimeReopenThread<IndexSearcher> controlledRealTimeReopenThread =
+      new ControlledRealTimeReopenThread<IndexSearcher>(tiw, sm, maxStaleSecs, 0);
+
+    controlledRealTimeReopenThread.setDaemon(true);
+    controlledRealTimeReopenThread.start();
+
+    List<Thread> commitThreads = new ArrayList<Thread>();
+
+    for (int i = 0; i < 500; i++) {
+      if (i > 0 && i % 50 == 0) {
+        Thread commitThread =  new Thread(new Runnable() {
+            @Override
+            public void run() {
+              try {
+                iw.commit();
+                IndexCommit ic = sdp.snapshot();
+                for (String name : ic.getFileNames()) {
+                  //distribute, and backup
+                  //System.out.println(names);
+                  assertTrue(dir.fileExists(name));
+                }
+              } catch (Exception e) {
+                throw new RuntimeException(e);
+              }
+            }
+          });
+        commitThread.start();
+        commitThreads.add(commitThread);
+      }
+      Document d = new Document();
+      d.add(new TextField("count", i + "", Field.Store.NO));
+      d.add(new TextField("content", content, Field.Store.YES));
+      long start = System.currentTimeMillis();
+      long l = tiw.addDocument(d);
+      controlledRealTimeReopenThread.waitForGeneration(l);
+      long wait = System.currentTimeMillis() - start;
+      assertTrue("waited too long for generation " + wait,
+                 wait < (maxStaleSecs *1000));
+      IndexSearcher searcher = sm.acquire();
+      TopDocs td = searcher.search(new TermQuery(new Term("count", i + "")), 10);
+      sm.release(searcher);
+      assertEquals(1, td.totalHits);
+    }
+
+    for(Thread commitThread : commitThreads) {
+      commitThread.join();
+    }
+
+    controlledRealTimeReopenThread.close();
+    sm.close();
+    iw.close();
+    dir.close();
+  }
 }



Mime
View raw message