diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 56caef4a5ba..9093d296788 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -67,6 +67,9 @@ Bug Fixes * SOLR-4551: CachingDirectoryFactory needs to create CacheEntry's with the fullpath not path. (Mark Miller) + +* SOLR-4555: When forceNew is used with CachingDirectoryFactory#get, the old + CachValue should have it's path set to null. (Mark Miller) Other Changes ---------------------- diff --git a/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java b/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java index d427a6197c9..45a9d01f009 100644 --- a/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java +++ b/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java @@ -166,7 +166,9 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory { closeDirectory(cacheValue); byDirectoryCache.remove(directory); - byPathCache.remove(cacheValue.path); + if (cacheValue.path != null) { + byPathCache.remove(cacheValue.path); + } } } } @@ -259,6 +261,10 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory { } } + // kill the path, it will be owned by the new dir + // we count on it being released by directory + cacheValue.path = null; + } } diff --git a/solr/core/src/java/org/apache/solr/core/StandardDirectoryFactory.java b/solr/core/src/java/org/apache/solr/core/StandardDirectoryFactory.java index 300dc354a31..4eb71363bbe 100644 --- a/solr/core/src/java/org/apache/solr/core/StandardDirectoryFactory.java +++ b/solr/core/src/java/org/apache/solr/core/StandardDirectoryFactory.java @@ -64,8 +64,10 @@ public class StandardDirectoryFactory extends CachingDirectoryFactory { if (val == null) { throw new IllegalArgumentException("Unknown directory " + dir); } - File dirFile = new File(val.path); - FileUtils.deleteDirectory(dirFile); + if (val.path != null) { + File dirFile = new File(val.path); + FileUtils.deleteDirectory(dirFile); + } } @Override diff --git a/solr/core/src/test/org/apache/solr/core/CachingDirectoryFactoryTest.java b/solr/core/src/test/org/apache/solr/core/CachingDirectoryFactoryTest.java index a9287e600e2..d09f103b93f 100644 --- a/solr/core/src/test/org/apache/solr/core/CachingDirectoryFactoryTest.java +++ b/solr/core/src/test/org/apache/solr/core/CachingDirectoryFactoryTest.java @@ -32,6 +32,7 @@ import org.junit.Test; public class CachingDirectoryFactoryTest extends SolrTestCaseJ4 { private Map dirs = new HashMap(); + private List oldDirs = new ArrayList(); private volatile boolean stop = false; private class Tracker { @@ -84,6 +85,17 @@ public class CachingDirectoryFactoryTest extends SolrTestCaseJ4 { } } } + sz = oldDirs.size(); + if (sz > 0) { + for (Tracker tracker : oldDirs) { + int cnt = tracker.refCnt.get(); + for (int i = 0; i < cnt; i++) { + tracker.refCnt.decrementAndGet(); + df.release(tracker.dir); + } + } + } + } df.close(); @@ -157,7 +169,18 @@ public class CachingDirectoryFactoryTest extends SolrTestCaseJ4 { tracker.dir = df.get(path, DirContext.DEFAULT, null); dirs.put(path, tracker); } else { - tracker.dir = df.get(path, DirContext.DEFAULT, null); + if (random.nextInt(10) > 6) { + Tracker oldTracker = new Tracker(); + oldTracker.refCnt = new AtomicInteger(tracker.refCnt.get()); + oldTracker.path = tracker.path; + oldTracker.dir = tracker.dir; + oldDirs.add(oldTracker); + + tracker.dir = df.get(path, DirContext.DEFAULT, null, true); + tracker.refCnt = new AtomicInteger(0); + } else { + tracker.dir = df.get(path, DirContext.DEFAULT, null); + } } tracker.refCnt.incrementAndGet(); } diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java b/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java index a9d9b9dcd7d..26a26820b18 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java @@ -1316,7 +1316,7 @@ public abstract class AbstractFullDistribZkTestBase extends AbstractDistribZkTes retry = true; } cnt++; - if (cnt > 10) break; + if (cnt > 20) break; Thread.sleep(2000); } while (retry); }