SOLR-4597: Additional work.

SOLR-4598: The Core Admin unload command's option 'deleteDataDir', should use the DirectoryFactory API to remove the data dir. 

SOLR-4599: CachingDirectoryFactory calls close(Directory) on forceNew if the Directory has a refCnt of 0, but it should call closeDirectory(CacheValueValue).

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1457556 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Mark Robert Miller 2013-03-17 20:41:17 +00:00
parent fe6c8d3ea8
commit 2d406a50b2
5 changed files with 116 additions and 50 deletions

View File

@ -147,6 +147,13 @@ Bug Fixes
* SOLR-4597: CachingDirectoryFactory#remove should not attempt to empty/remove
the index right away but flag for removal after close. (Mark Miller)
* SOLR-4598: The Core Admin unload command's option 'deleteDataDir', should use
the DirectoryFactory API to remove the data dir. (Mark Miller)
* SOLR-4599: CachingDirectoryFactory calls close(Directory) on forceNew if the
Directory has a refCnt of 0, but it should call closeDirectory(CacheValueValue).
(Mark Miller)
Optimizations
----------------------

View File

@ -22,9 +22,11 @@ import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.IOContext.Context;
@ -43,15 +45,40 @@ import org.slf4j.LoggerFactory;
* per path. Most DirectoryFactory implementations will want to extend this
* class and simply implement {@link DirectoryFactory#create(String, DirContext)}.
*
* This is an expert class and these API's are subject to change.
*
*/
public abstract class CachingDirectoryFactory extends DirectoryFactory {
protected class CacheValue {
public Directory directory;
final public String path;
final public Directory directory;
// use the setter!
private boolean deleteOnClose = false;
public CacheValue(String path, Directory directory) {
this.path = path;
this.directory = directory;
this.closeEntries.add(this);
}
public int refCnt = 1;
public boolean closed = false;
public String path;
// if we are latestForPath, I'm currently using my path
// otherwise a new Directory instance is using my path
// and I must be manipulated by Directory
public boolean latestForPath = false;
// has close(Directory) been called on this?
public boolean closeDirectoryCalled = false;
public boolean doneWithDir = false;
public boolean deleteOnClose = false;
public Set<CacheValue> removeEntries = new HashSet<CacheValue>();
public Set<CacheValue> closeEntries = new HashSet<CacheValue>();
public void setDeleteOnClose(boolean deleteOnClose) {
if (deleteOnClose) {
removeEntries.add(this);
}
this.deleteOnClose = deleteOnClose;
}
@Override
public String toString() {
return "CachedDir<<" + directory.toString() + ";refCount=" + refCnt + ";path=" + path + ";done=" + doneWithDir + ">>";
@ -183,8 +210,9 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
// if it's been closed, it's path is now
// owned by another Directory instance
if (!cacheValue.closed) {
if (!cacheValue.latestForPath) {
byPathCache.remove(cacheValue.path);
cacheValue.latestForPath = true;
}
}
}
@ -201,22 +229,50 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
}
}
}
try {
log.info("Closing directory: " + cacheValue.path);
cacheValue.directory.close();
} catch (Throwable t) {
SolrException.log(log, "Error closing directory", t);
}
cacheValue.closeDirectoryCalled = true;
if (cacheValue.deleteOnClose) {
try {
log.info("Removing directory: " + cacheValue.path);
removeDirectory(cacheValue);
} catch (Throwable t) {
SolrException.log(log, "Error closing directory", t);
// see if we are a subpath
Collection<CacheValue> values = byPathCache.values();
Collection<CacheValue> cacheValues = new ArrayList<CacheValue>();
cacheValues.addAll(values);
cacheValues.remove(cacheValue);
for (CacheValue otherCacheValue : cacheValues) {
// if we are a parent path and all our sub children are not already closed,
// get a sub path to close us later
if (otherCacheValue.path.startsWith(cacheValue.path) && !otherCacheValue.closeDirectoryCalled) {
// we let the sub dir remove and close us
otherCacheValue.removeEntries.addAll(cacheValue.removeEntries);
otherCacheValue.closeEntries.addAll(cacheValue.closeEntries);
cacheValue.closeEntries.clear();
break;
}
}
}
for (CacheValue val : cacheValue.removeEntries) {
try {
log.info("Removing directory: " + val.path);
removeDirectory(val);
} catch (Throwable t) {
SolrException.log(log, "Error removing directory", t);
}
}
for (CacheValue val : cacheValue.closeEntries) {
try {
log.info("Closing directory: " + val.path);
val.directory.close();
} catch (Throwable t) {
SolrException.log(log, "Error closing directory", t);
}
}
if (listeners != null) {
for (CloseListener listener : listeners) {
try {
@ -275,21 +331,13 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
// we make a quick close attempt,
// otherwise this should be closed
// when whatever is using it, releases it
if (cacheValue.refCnt == 0) {
try {
// the following will decref, so
// first incref
cacheValue.refCnt++;
close(cacheValue.directory);
} catch (IOException e) {
SolrException.log(log, "Error closing directory", e);
}
closeDirectory(cacheValue);
}
// close the entry, it will be owned by the new dir
// we count on it being released by directory
cacheValue.closed = true;
cacheValue.latestForPath = true;
}
}
@ -299,9 +347,7 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
directory = rateLimit(directory);
CacheValue newCacheValue = new CacheValue();
newCacheValue.directory = directory;
newCacheValue.path = fullPath;
CacheValue newCacheValue = new CacheValue(fullPath, directory);
injectLockFactory(directory, fullPath, rawLockType);
@ -384,7 +430,7 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
if (val == null) {
throw new IllegalArgumentException("Unknown directory " + path);
}
val.deleteOnClose = true;
val.setDeleteOnClose(true);
}
}
@ -395,7 +441,7 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
if (val == null) {
throw new IllegalArgumentException("Unknown directory " + dir);
}
val.deleteOnClose = true;
val.setDeleteOnClose(true);
}
}

View File

@ -576,23 +576,13 @@ public class CoreAdminHandler extends RequestHandlerBase {
}
}
if (params.getBool(CoreAdminParams.DELETE_DATA_DIR, false)) {
core.addCloseHook(new CloseHook() {
@Override
public void preClose(SolrCore core) {}
@Override
public void postClose(SolrCore core) {
File dataDir = new File(core.getDataDir());
try {
FileUtils.deleteDirectory(dataDir);
} catch (IOException e) {
SolrException.log(log, "Failed to delete data dir for core:"
+ core.getName() + " dir:" + dataDir.getAbsolutePath());
}
}
});
try {
core.getDirectoryFactory().remove(core.getDataDir());
} catch (Exception e) {
SolrException.log(log, "Failed to flag data dir for removal for core:"
+ core.getName() + " dir:" + core.getDataDir());
}
}
if (params.getBool(CoreAdminParams.DELETE_INSTANCE_DIR, false)) {

View File

@ -349,6 +349,7 @@ public class CoreAdminRequest extends SolrRequest
public static class Unload extends CoreAdminRequest {
protected boolean deleteIndex;
private boolean deleteDataDir;
public Unload(boolean deleteIndex) {
action = CoreAdminAction.UNLOAD;
@ -363,12 +364,18 @@ public class CoreAdminRequest extends SolrRequest
this.deleteIndex = deleteIndex;
}
public void setDeleteDataDir(boolean deleteDataDir) {
this.deleteDataDir = deleteDataDir;
}
@Override
public SolrParams getParams() {
ModifiableSolrParams params = (ModifiableSolrParams) super.getParams();
params.set(CoreAdminParams.DELETE_INDEX, deleteIndex);
params.set(CoreAdminParams.DELETE_DATA_DIR, deleteDataDir);
return params;
}
}
public CoreAdminRequest()

View File

@ -19,8 +19,8 @@ package org.apache.solr.client.solrj;
import java.io.File;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.client.solrj.request.AbstractUpdateRequest.ACTION;
import org.apache.solr.client.solrj.request.CoreAdminRequest.Unload;
import org.apache.solr.client.solrj.request.CoreAdminRequest;
import org.apache.solr.client.solrj.request.QueryRequest;
import org.apache.solr.client.solrj.request.UpdateRequest;
@ -44,6 +44,7 @@ public abstract class MultiCoreExampleTestBase extends SolrExampleTestBase
protected static CoreContainer cores;
private File dataDir2;
private File dataDir1;
@Override public String getSolrHome() { return ExternalPaths.EXAMPLE_MULTICORE_HOME; }
@ -64,11 +65,15 @@ public abstract class MultiCoreExampleTestBase extends SolrExampleTestBase
SolrCore.log.info("CORES=" + cores + " : " + cores.getCoreNames());
cores.setPersistent(false);
dataDir1 = new File(TEMP_DIR, getClass().getName() + "-core0-"
+ System.currentTimeMillis());
dataDir1.mkdirs();
dataDir2 = new File(TEMP_DIR, getClass().getName() + "-core1-"
+ System.currentTimeMillis());
dataDir2.mkdirs();
System.setProperty( "solr.core0.data.dir", SolrTestCaseJ4.dataDir.getCanonicalPath() );
System.setProperty( "solr.core0.data.dir", this.dataDir1.getCanonicalPath() );
System.setProperty( "solr.core1.data.dir", this.dataDir2.getCanonicalPath() );
}
@ -219,13 +224,24 @@ public abstract class MultiCoreExampleTestBase extends SolrExampleTestBase
String indexDir = (String) ((NamedList<Object>) coreInfo.get("directory")).get("index");
response = getSolrCore("core0").query(new SolrQuery().setRequestHandler("/admin/system")).getResponse();
coreInfo = (NamedList<Object>) response.get("core");
String dataDir = (String) ((NamedList<Object>) coreInfo.get("directory")).get("data");
System.out.println( (String) ((NamedList<Object>) coreInfo.get("directory")).get("dirimpl"));
// test delete index on core
CoreAdminRequest.unloadCore("corefoo", true, coreadmin);
File dir = new File(indexDir);
assertFalse("Index directory exists after core unload with deleteIndex=true", dir.exists());
Unload req = new Unload(false);
req.setDeleteDataDir(true);
req.setCoreName("core0");
req.process(coreadmin);
dir = new File(dataDir);
assertFalse("Data directory exists after core unload with deleteDataDir=true : " + dir, dir.exists());
}
}