mirror of https://github.com/apache/lucene.git
SOLR-4597: CachingDirectoryFactory#remove should not attempt to empty/remove the index right away but flag for removal after close.
git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1457494 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
parent
0dece6b609
commit
025e30b992
|
@ -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
|
||||
----------------------
|
||||
|
||||
|
|
|
@ -48,9 +48,10 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
|
|||
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 CachingDirectoryFactory extends DirectoryFactory {
|
|||
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 CachingDirectoryFactory extends DirectoryFactory {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
protected abstract Directory create(String path, DirContext dirContext) throws IOException;
|
||||
|
||||
|
@ -367,6 +377,28 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
|
|||
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 CachingDirectoryFactory extends DirectoryFactory {
|
|||
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);
|
||||
}
|
||||
|
|
|
@ -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 EphemeralDirectoryFactory extends CachingDirectoryFactory
|
|||
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;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -25,6 +25,7 @@ import org.apache.lucene.store.FSDirectory;
|
|||
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 extends CachingDirectoryFactory {
|
|||
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 extends CachingDirectoryFactory {
|
|||
}
|
||||
|
||||
@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);
|
||||
}
|
||||
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
|
|
|
@ -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 RequestHandlerBase {
|
|||
+ 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();
|
||||
|
||||
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());
|
||||
}
|
||||
|
||||
@Override
|
||||
public void postClose(SolrCore core) {
|
||||
try {
|
||||
core.getDirectoryFactory().remove(indexDir);
|
||||
} catch (IOException e) {
|
||||
throw new RuntimeException(e);
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
if (params.getBool(CoreAdminParams.DELETE_DATA_DIR, false)) {
|
||||
core.addCloseHook(new CloseHook() {
|
||||
|
|
|
@ -142,6 +142,12 @@ public class CachingDirectoryFactoryTest extends SolrTestCaseJ4 {
|
|||
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);
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue