lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From markrmil...@apache.org
Subject svn commit: r1457494 - in /lucene/dev/trunk/solr: ./ core/src/java/org/apache/solr/core/ core/src/java/org/apache/solr/handler/ core/src/java/org/apache/solr/handler/admin/ core/src/test/org/apache/solr/core/
Date Sun, 17 Mar 2013 17:37:00 GMT
Author: markrmiller
Date: Sun Mar 17 17:36:59 2013
New Revision: 1457494

URL: http://svn.apache.org/r1457494
Log:
SOLR-4597: CachingDirectoryFactory#remove should not attempt to empty/remove the index right
away but flag for removal after close.

Modified:
    lucene/dev/trunk/solr/CHANGES.txt
    lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java
    lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/EphemeralDirectoryFactory.java
    lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/StandardDirectoryFactory.java
    lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/SnapPuller.java
    lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java
    lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/CachingDirectoryFactoryTest.java

Modified: lucene/dev/trunk/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/CHANGES.txt?rev=1457494&r1=1457493&r2=1457494&view=diff
==============================================================================
--- lucene/dev/trunk/solr/CHANGES.txt (original)
+++ lucene/dev/trunk/solr/CHANGES.txt Sun Mar 17 17:36:59 2013
@@ -144,6 +144,9 @@ Bug Fixes
 * SOLR-4594: StandardDirectoryFactory#remove accesses byDirectoryCache 
   without a lock. (Mark Miller)
 
+* SOLR-4597: CachingDirectoryFactory#remove should not attempt to empty/remove 
+  the index right away but flag for removal after close. (Mark Miller)
+
 Optimizations
 ----------------------
 

Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java?rev=1457494&r1=1457493&r2=1457494&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java
(original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java
Sun Mar 17 17:36:59 2013
@@ -48,9 +48,10 @@ public abstract class CachingDirectoryFa
   protected class CacheValue {
     public Directory directory;
     public int refCnt = 1;
-    public boolean closed;
+    public boolean closed = false;
     public String path;
     public boolean doneWithDir = false;
+    public boolean deleteOnClose = false;
     @Override
     public String toString() {
       return "CachedDir<<" + directory.toString() + ";refCount=" + refCnt + ";path="
+ path + ";done=" + doneWithDir + ">>";
@@ -207,6 +208,15 @@ public abstract class CachingDirectoryFa
       SolrException.log(log, "Error closing directory", t);
     }
     
+    if (cacheValue.deleteOnClose) {
+      try {
+        log.info("Removing directory: " + cacheValue.path);
+        removeDirectory(cacheValue);
+      } catch (Throwable t) {
+        SolrException.log(log, "Error closing directory", t);
+      }
+    }
+    
     if (listeners != null) {
       for (CloseListener listener : listeners) {
         try {
@@ -217,7 +227,7 @@ public abstract class CachingDirectoryFa
       }
     }
   }
-  
+
   @Override
   protected abstract Directory create(String path, DirContext dirContext) throws IOException;
   
@@ -367,6 +377,28 @@ public abstract class CachingDirectoryFa
     close(directory);
   }
   
+  @Override
+  public void remove(String path) throws IOException {
+    synchronized (this) {
+      CacheValue val = byPathCache.get(normalize(path));
+      if (val == null) {
+        throw new IllegalArgumentException("Unknown directory " + path);
+      }
+      val.deleteOnClose = true;
+    }
+  }
+  
+  @Override
+  public void remove(Directory dir) throws IOException {
+    synchronized (this) {
+      CacheValue val = byDirectoryCache.get(dir);
+      if (val == null) {
+        throw new IllegalArgumentException("Unknown directory " + dir);
+      }
+      val.deleteOnClose = true;
+    }
+  }
+  
   private static Directory injectLockFactory(Directory dir, String lockPath,
       String rawLockType) throws IOException {
     if (null == rawLockType) {
@@ -395,7 +427,17 @@ public abstract class CachingDirectoryFa
     return dir;
   }
   
-  protected String stripTrailingSlash(String path) {
+  protected void removeDirectory(CacheValue cacheValue) throws IOException {
+    empty(cacheValue.directory);
+  }
+  
+  @Override
+  public String normalize(String path) throws IOException {
+    path = stripTrailingSlash(path);
+    return path;
+  }
+  
+  private String stripTrailingSlash(String path) {
     if (path.endsWith("/")) {
       path = path.substring(0, path.length() - 1);
     }

Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/EphemeralDirectoryFactory.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/EphemeralDirectoryFactory.java?rev=1457494&r1=1457493&r2=1457494&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/EphemeralDirectoryFactory.java
(original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/EphemeralDirectoryFactory.java
Sun Mar 17 17:36:59 2013
@@ -16,9 +16,12 @@ package org.apache.solr.core;
  * limitations under the License.
  */
 
+import java.io.File;
 import java.io.IOException;
 
+import org.apache.commons.io.FileUtils;
 import org.apache.lucene.store.Directory;
+import org.apache.solr.core.CachingDirectoryFactory.CacheValue;
 
 /**
  * Directory provider for implementations that do not persist over reboots.
@@ -51,21 +54,4 @@ public abstract class EphemeralDirectory
   public boolean isAbsolute(String path) {
     return true;
   }
-  
-  
-  @Override
-  public void remove(Directory dir) throws IOException {
-    // ram dir does not persist its dir anywhere
-  }
-  
-  @Override
-  public void remove(String path) throws IOException {
-    // ram dir does not persist its dir anywhere
-  }
-  
-  @Override
-  public String normalize(String path) throws IOException {
-    path = stripTrailingSlash(path);
-    return path;
-  }
 }

Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/StandardDirectoryFactory.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/StandardDirectoryFactory.java?rev=1457494&r1=1457493&r2=1457494&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/StandardDirectoryFactory.java
(original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/StandardDirectoryFactory.java
Sun Mar 17 17:36:59 2013
@@ -25,6 +25,7 @@ import org.apache.lucene.store.FSDirecto
 import org.apache.lucene.store.IOContext;
 import org.apache.lucene.store.NRTCachingDirectory;
 import org.apache.lucene.store.RateLimitedDirectoryWrapper;
+import org.apache.solr.core.CachingDirectoryFactory.CacheValue;
 
 /**
  * Directory provider which mimics original Solr 
@@ -45,7 +46,7 @@ public class StandardDirectoryFactory ex
   public String normalize(String path) throws IOException {
     String cpath = new File(path).getCanonicalPath();
     
-    return stripTrailingSlash(cpath);
+    return super.normalize(cpath);
   }
   
   public boolean isPersistent() {
@@ -59,23 +60,8 @@ public class StandardDirectoryFactory ex
   }
   
   @Override
-  public void remove(Directory dir) throws IOException {
-    synchronized (this) {
-      CacheValue val = byDirectoryCache.get(dir);
-      if (val == null) {
-        throw new IllegalArgumentException("Unknown directory " + dir);
-      }
-      
-      File dirFile = new File(val.path);
-      FileUtils.deleteDirectory(dirFile);
-
-    }
-  }
-
-  @Override
-  public void remove(String path) throws IOException {
-    String fullPath = normalize(path);
-    File dirFile = new File(fullPath);
+  protected void removeDirectory(CacheValue cacheValue) throws IOException {
+    File dirFile = new File(cacheValue.path);
     FileUtils.deleteDirectory(dirFile);
   }
   

Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/SnapPuller.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/SnapPuller.java?rev=1457494&r1=1457493&r2=1457494&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/SnapPuller.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/SnapPuller.java Sun Mar 17
17:36:59 2013
@@ -481,13 +481,6 @@ public class SnapPuller {
         throw new InterruptedException("Index fetch interrupted");
       } catch (Exception e) {
         throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Index fetch failed
: ", e);
-      } finally {
-        if (deleteTmpIdxDir) {
-          LOG.info("removing temporary index download directory files " + tmpIndexDir);
-          if (tmpIndex != null && core.getDirectoryFactory().exists(tmpIndex)) {
-            DirectoryFactory.empty(tmpIndexDir);
-          }
-        } 
       }
     } finally {
       try {
@@ -504,9 +497,6 @@ public class SnapPuller {
         stop = false;
         fsyncException = null;
       } finally {
-        if (tmpIndexDir != null) {
-          core.getDirectoryFactory().release(tmpIndexDir);
-        }
         if (deleteTmpIdxDir && tmpIndexDir != null) {
           try {
             core.getDirectoryFactory().remove(tmpIndexDir);
@@ -514,6 +504,11 @@ public class SnapPuller {
             SolrException.log(LOG, "Error removing directory " + tmpIndexDir, e);
           }
         }
+        
+        if (tmpIndexDir != null) {
+          core.getDirectoryFactory().release(tmpIndexDir);
+        }
+        
         if (indexDir != null) {
           core.getDirectoryFactory().release(indexDir);
         }

Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java?rev=1457494&r1=1457493&r2=1457494&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java
(original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java
Sun Mar 17 17:36:59 2013
@@ -29,8 +29,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 
-import javax.xml.parsers.ParserConfigurationException;
-
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.lucene.index.DirectoryReader;
@@ -73,7 +71,6 @@ import org.apache.solr.util.RefCounted;
 import org.apache.zookeeper.KeeperException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.xml.sax.SAXException;
 
 /**
  *
@@ -568,26 +565,17 @@ public class CoreAdminHandler extends Re
                     + e.getMessage(), e);
           }
         }
-      }
-      if (params.getBool(CoreAdminParams.DELETE_INDEX, false)) {
-        core.addCloseHook(new CloseHook() {
-          private String indexDir;
-          
-          @Override
-          public void preClose(SolrCore core) {
-            indexDir = core.getIndexDir();
-          }
-          
-          @Override
-          public void postClose(SolrCore core) {
-            try {
-              core.getDirectoryFactory().remove(indexDir);
-            } catch (IOException e) {
-              throw new RuntimeException(e);
-            }
+        
+        if (params.getBool(CoreAdminParams.DELETE_INDEX, false)) {
+          try {
+            core.getDirectoryFactory().remove(core.getIndexDir());
+          } catch (Exception e) {
+            SolrException.log(log, "Failed to flag index dir for removal for core:"
+                    + core.getName() + " dir:" + core.getIndexDir());
           }
-        });
+        }
       }
+
       
       if (params.getBool(CoreAdminParams.DELETE_DATA_DIR, false)) {
         core.addCloseHook(new CloseHook() {

Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/CachingDirectoryFactoryTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/CachingDirectoryFactoryTest.java?rev=1457494&r1=1457493&r2=1457494&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/CachingDirectoryFactoryTest.java
(original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/CachingDirectoryFactoryTest.java
Sun Mar 17 17:36:59 2013
@@ -142,6 +142,12 @@ public class CachingDirectoryFactoryTest
                 if (random.nextBoolean()) {
                   df.doneWithDirectory(tracker.dir);
                 }
+                if (random.nextBoolean()) {
+                  df.remove(tracker.dir);
+                }
+                if (random.nextBoolean()) {
+                  df.remove(tracker.path);
+                }
                 tracker.refCnt.decrementAndGet();
                 df.release(tracker.dir);
               }



Mime
View raw message