Apache
/
lucene
mirror of https://github.com/apache/lucene.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SOLR-4624: CachingDirectoryFactory does not need to support forceNew any longer and it appears to be causing a missing close directory bug. forceNew is no longer respected and will be removed in 4.3. 

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1459565 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Mark Robert Miller 2013-03-21 22:44:14 +00:00
parent 7584c40c7a
commit 5b3cdaca44
8 changed files with 19 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
solr/CHANGES.txt
View File

@ -96,6 +96,8 @@ Other Changes
* SOLR-4607: Use noggit 0.5 release jar rather than a forked copy. (Yonik Seeley, Robert Muir)
* SOLR-4624: forceNew has been removed from the DirectoryFactory and related java apis.
================== 4.2.1 ==================
Versions of Major Components
@ -213,6 +215,9 @@ Bug Fixes
values and top-level phrase slops on queries produced by nested
sub-parsers. (yonik)
* SOLR-4624: CachingDirectoryFactory does not need to support forceNew any
longer and it appears to be causing a missing close directory bug. forceNew
is no longer respected and will be removed in 4.3. (Mark Miller)
Optimizations
----------------------

45
solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java
View File

@ -63,10 +63,6 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
this.closeEntries.add(this);
}
public int refCnt = 1;
// 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;
@ -218,12 +214,8 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
byDirectoryCache.remove(directory);
// if it's been closed, it's path is now
// owned by another Directory instance
if (!cacheValue.latestForPath) {
byPathCache.remove(cacheValue.path);
cacheValue.latestForPath = true;
}
byPathCache.remove(cacheValue.path);
}
}
}
@ -310,18 +302,6 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
return dirFile.canRead() && dirFile.list().length > 0;
}
/*
* (non-Javadoc)
*
* @see org.apache.solr.core.DirectoryFactory#get(java.lang.String,
* java.lang.String)
*/
@Override
public final Directory get(String path, DirContext dirContext, String rawLockType)
throws IOException {
return get(path, dirContext, rawLockType, false);
}
/*
* (non-Javadoc)
*
@ -329,7 +309,7 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
* java.lang.String, boolean)
*/
@Override
public final Directory get(String path, DirContext dirContext, String rawLockType, boolean forceNew)
public final Directory get(String path, DirContext dirContext, String rawLockType)
throws IOException {
String fullPath = normalize(path);
synchronized (this) {
@ -341,24 +321,9 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
Directory directory = null;
if (cacheValue != null) {
directory = cacheValue.directory;
if (forceNew) {
cacheValue.doneWithDir = true;
// we make a quick close attempt,
// otherwise this should be closed
// when whatever is using it, releases it
if (cacheValue.refCnt == 0) {
closeDirectory(cacheValue);
}
// close the entry, it will be owned by the new dir
// we count on it being released by directory
cacheValue.latestForPath = true;
}
}
if (directory == null || forceNew) {
if (directory == null) {
directory = create(fullPath, dirContext);
directory = rateLimit(directory);
@ -369,7 +334,7 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
byDirectoryCache.put(directory, newCacheValue);
byPathCache.put(fullPath, newCacheValue);
log.info("return new directory for " + fullPath + " forceNew: " + forceNew);
log.info("return new directory for " + fullPath);
} else {
cacheValue.refCnt++;
}

15
solr/core/src/java/org/apache/solr/core/DirectoryFactory.java
View File

@ -140,27 +140,12 @@ public abstract class DirectoryFactory implements NamedListInitializedPlugin,
* Returns the Directory for a given path, using the specified rawLockType.
* Will return the same Directory instance for the same path.
*
* Note: sometimes you might pass null for the rawLockType when
* you know the Directory exists and the rawLockType is already
* in use.
*
* @throws IOException If there is a low-level I/O error.
*/
public abstract Directory get(String path, DirContext dirContext, String rawLockType)
throws IOException;
/**
* Returns the Directory for a given path, using the specified rawLockType.
* Will return the same Directory instance for the same path unless forceNew,
* in which case a new Directory is returned. There is no need to call
* {@link #doneWithDirectory(Directory)} in this case - the old Directory
* will be closed when it's ref count hits 0.
*
* @throws IOException If there is a low-level I/O error.
*/
public abstract Directory get(String path, DirContext dirContext, String rawLockType,
boolean forceNew) throws IOException;
/**
* Increment the number of references to the given Directory. You must call
* release for every call to this method.

2
solr/core/src/java/org/apache/solr/core/SolrCore.java
View File

@ -497,7 +497,7 @@ public final class SolrCore implements SolrInfoMBean {
log.warn(logid+"Solr index directory '" + new File(indexDir) + "' doesn't exist."
+ " Creating new index...");
SolrIndexWriter writer = SolrIndexWriter.create("SolrCore.initIndex", indexDir, getDirectoryFactory(), true, schema, solrConfig.indexConfig, solrDelPolicy, codec, false);
SolrIndexWriter writer = SolrIndexWriter.create("SolrCore.initIndex", indexDir, getDirectoryFactory(), true, schema, solrConfig.indexConfig, solrDelPolicy, codec);
writer.close();
}

8
solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java
View File

@ -104,7 +104,7 @@ public final class DefaultSolrCoreState extends SolrCoreState implements Recover
}
if (indexWriter == null) {
indexWriter = createMainIndexWriter(core, "DirectUpdateHandler2", false);
indexWriter = createMainIndexWriter(core, "DirectUpdateHandler2");
}
initRefCntWriter();
writerFree = false;
@ -172,7 +172,7 @@ public final class DefaultSolrCoreState extends SolrCoreState implements Recover
}
}
}
indexWriter = createMainIndexWriter(core, "DirectUpdateHandler2", forceNewDir);
indexWriter = createMainIndexWriter(core, "DirectUpdateHandler2");
log.info("New IndexWriter is ready to be used.");
// we need to null this so it picks up the new writer next get call
refCntWriter = null;
@ -189,10 +189,10 @@ public final class DefaultSolrCoreState extends SolrCoreState implements Recover
newIndexWriter(core, true, false);
}
protected SolrIndexWriter createMainIndexWriter(SolrCore core, String name, boolean forceNewDirectory) throws IOException {
protected SolrIndexWriter createMainIndexWriter(SolrCore core, String name) throws IOException {
return SolrIndexWriter.create(name, core.getNewIndexDir(),
core.getDirectoryFactory(), false, core.getSchema(),
core.getSolrConfig().indexConfig, core.getDeletionPolicy(), core.getCodec(), forceNewDirectory);
core.getSolrConfig().indexConfig, core.getDeletionPolicy(), core.getCodec());
}
@Override

2
solr/core/src/java/org/apache/solr/update/SolrIndexSplitter.java
View File

@ -102,7 +102,7 @@ public class SolrIndexSplitter {
String path = paths.get(partitionNumber);
iw = SolrIndexWriter.create("SplittingIndexWriter"+partitionNumber + " " + ranges.get(partitionNumber), path,
core.getDirectoryFactory(), true, core.getSchema(),
core.getSolrConfig().indexConfig, core.getDeletionPolicy(), core.getCodec(), true);
core.getSolrConfig().indexConfig, core.getDeletionPolicy(), core.getCodec());
}
try {

4
solr/core/src/java/org/apache/solr/update/SolrIndexWriter.java
View File

@ -56,10 +56,10 @@ public class SolrIndexWriter extends IndexWriter {
String name;
private DirectoryFactory directoryFactory;
public static SolrIndexWriter create(String name, String path, DirectoryFactory directoryFactory, boolean create, IndexSchema schema, SolrIndexConfig config, IndexDeletionPolicy delPolicy, Codec codec, boolean forceNewDirectory) throws IOException {
public static SolrIndexWriter create(String name, String path, DirectoryFactory directoryFactory, boolean create, IndexSchema schema, SolrIndexConfig config, IndexDeletionPolicy delPolicy, Codec codec) throws IOException {
SolrIndexWriter w = null;
final Directory d = directoryFactory.get(path, DirContext.DEFAULT, config.lockType, forceNewDirectory);
final Directory d = directoryFactory.get(path, DirContext.DEFAULT, config.lockType);
try {
w = new SolrIndexWriter(name, path, d, create, schema,
config, delPolicy, codec);

24
solr/core/src/test/org/apache/solr/core/CachingDirectoryFactoryTest.java
View File

@ -32,7 +32,6 @@ import org.junit.Test;
public class CachingDirectoryFactoryTest extends SolrTestCaseJ4 {
private Map<String,Tracker> dirs = new HashMap<String,Tracker>();
private List<Tracker> oldDirs = new ArrayList<Tracker>();
private volatile boolean stop = false;
private class Tracker {
@ -96,16 +95,6 @@ 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);
}
}
}
}
@ -187,18 +176,7 @@ public class CachingDirectoryFactoryTest extends SolrTestCaseJ4 {
tracker.dir = df.get(path, DirContext.DEFAULT, null);
dirs.put(path, tracker);
} else {
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.dir = df.get(path, DirContext.DEFAULT, null);
}
tracker.refCnt.incrementAndGet();
}