From 509e239d7028097e90a6d2907aa11889ff029629 Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Mon, 1 Jun 2009 18:34:10 +0000 Subject: [PATCH] LUCENE-1658: Fix MMapDirectory, add hack for JVM bug that keeps mmapped files open, fix tests, that cannot use other dir impls than SimpleFSDirectory, some API fine tuning. git-svn-id: https://svn.apache.org/repos/asf/lucene/java/trunk@780770 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES.txt | 11 +- common-build.xml | 2 +- .../org/apache/lucene/store/FSDirectory.java | 229 ++++++------------ .../apache/lucene/store/MMapDirectory.java | 199 +++++++++++++-- .../apache/lucene/store/NIOFSDirectory.java | 19 +- .../lucene/store/SimpleFSDirectory.java | 30 ++- .../apache/lucene/index/TestCompoundFile.java | 79 +++--- src/test/org/apache/lucene/index/TestDoc.java | 2 +- .../lucene/index/TestIndexModifier.java | 2 +- .../apache/lucene/index/TestIndexWriter.java | 2 +- .../lucene/index/TestStressIndexing2.java | 2 +- .../lucene/store/TestBufferedIndexInput.java | 2 +- .../apache/lucene/store/TestDirectory.java | 10 +- .../lucene/store/TestMMapDirectory.java | 52 ---- .../org/apache/lucene/store/_TestHelper.java | 26 +- 15 files changed, 355 insertions(+), 312 deletions(-) delete mode 100644 src/test/org/apache/lucene/store/TestMMapDirectory.java diff --git a/CHANGES.txt b/CHANGES.txt index ba93a841242..69c87ccd982 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -146,9 +146,9 @@ API Changes is deprecated in favor of the new TimeLimitingCollector which extends Collector. (Shai Erera via Mike McCandless) -13. LUCENE-1621: MultiTermQuery#getTerm() has been deprecated as it does +13. LUCENE-1621: MultiTermQuery.getTerm() has been deprecated as it does not make sense for all subclasses of MultiTermQuery. Check individual - subclasses to see if they support #getTerm(). (Mark Miller) + subclasses to see if they support getTerm(). (Mark Miller) 14. LUCENE-1636: Make TokenFilter.input final so it's set only once. (Wouter Heijke, Uwe Schindler via Mike McCandless). @@ -156,7 +156,7 @@ API Changes 15. LUCENE-1658: Renamed FSDirectory to SimpleFSDirectory (but left an FSDirectory base class). Added an FSDirectory.open static method to pick a good default FSDirectory implementation given the OS. - (Michael McCandless) + (Michael McCandless, Uwe Schindler) Bug fixes @@ -212,6 +212,11 @@ Bug fixes rely on this behavior by the 3.0 release of Lucene. (Jonathan Mamou, Mark Miller via Mike McCandless) +15. LUCENE-1658: Fixed MMapDirectory to correctly throw IOExceptions + on EOF, removed numeric overflow possibilities and added support + for a hack to unmap the buffers on closing IndexInput. + (Uwe Schindler) + New features 1. LUCENE-1411: Added expert API to open an IndexWriter on a prior diff --git a/common-build.xml b/common-build.xml index 9005e35a8d5..ce604e9a4bf 100644 --- a/common-build.xml +++ b/common-build.xml @@ -42,7 +42,7 @@ - + diff --git a/src/java/org/apache/lucene/store/FSDirectory.java b/src/java/org/apache/lucene/store/FSDirectory.java index 860a85e1fca..dd45812cd87 100644 --- a/src/java/org/apache/lucene/store/FSDirectory.java +++ b/src/java/org/apache/lucene/store/FSDirectory.java @@ -59,12 +59,29 @@ import org.apache.lucene.index.IndexWriter; * choice. * *
  • {@link MMapDirectory} uses memory-mapped IO when - * reading. This is a good choice if you have plenty + * reading. This is a good choice if you have plenty * of virtual memory relative to your index size, eg * if you are running on a 64 bit JRE, or you are * running on a 32 bit JRE but your index sizes are * small enough to fit into the virtual memory space. - * + * Java has currently the limitation of not being able to + * unmap files from user code. The files are unmapped, when GC + * releases the byte buffers. Due to + * + * this bug in Sun's JRE, MMapDirectory's {@link IndexInput#close} + * is unable to close the underlying OS file handle. Only when + * GC finally collects the underlying objects, which could be + * quite some time later, will the file handle be closed. + * This will consume additional transient disk usage: on Windows, + * attempts to delete or overwrite the files will result in an + * exception; on other platforms, which typically have a "delete on + * last close" semantics, while such operations will succeed, the bytes + * are still consuming space on disk. For many applications this + * limitation is not a problem (e.g. if you have plenty of disk space, + * and you don't rely on overwriting files on Windows) but it's still + * an important limitation to be aware of. This class supplies a + * (possibly dangerous) workaround mentioned in the bug report, + * which may fail on non-Sun JVMs. * * * Unfortunately, because of system peculiarities, there is @@ -85,6 +102,8 @@ import org.apache.lucene.index.IndexWriter; * Java system property, or by calling {@link * #setLockFactory} after creating the Directory. * + *

    In 3.0 this class will become abstract. + * * @see Directory */ // TODO: in 3.0 this will become an abstract base class @@ -118,7 +137,8 @@ public class FSDirectory extends Directory { /** * Returns whether Lucene's use of lock files is disabled. * @return true if locks are disabled, false if locks are enabled. - */ + * @see #setDisableLocks + */ public static boolean getDisableLocks() { return FSDirectory.disableLocks; } @@ -147,16 +167,17 @@ public class FSDirectory extends Directory { try { String name = System.getProperty("org.apache.lucene.FSDirectory.class", - FSDirectory.class.getName()); - IMPL = Class.forName(name); + SimpleFSDirectory.class.getName()); + if (FSDirectory.class.getName().equals(name)) { + // FSDirectory will be abstract, so we replace it by the correct class + IMPL = SimpleFSDirectory.class; + } else { + IMPL = Class.forName(name); + } } catch (ClassNotFoundException e) { throw new RuntimeException("cannot load FSDirectory class: " + e.toString(), e); } catch (SecurityException se) { - try { - IMPL = Class.forName(FSDirectory.class.getName()); - } catch (ClassNotFoundException e) { - throw new RuntimeException("cannot load default FSDirectory class: " + e.toString(), e); - } + IMPL = SimpleFSDirectory.class; } } @@ -316,7 +337,9 @@ public class FSDirectory extends Directory { } } - final void initOutput(String name) throws IOException { + /** Initializes the directory to create a new file with the given name. + * This method should be used in {@link #createOutput}. */ + protected final void initOutput(String name) throws IOException { ensureOpen(); createDir(); File file = new File(directory, name); @@ -324,18 +347,21 @@ public class FSDirectory extends Directory { throw new IOException("Cannot overwrite: " + file); } + /** The underlying filesystem directory */ protected File directory = null; - private int refCount; + + /** @deprecated */ + private int refCount = 0; - protected FSDirectory() {}; // permit subclassing + /** @deprecated */ + protected FSDirectory() {}; // permit subclassing - /** Create a new FSDirectory for the named location. - * @deprecated Use {@link SimpleFSDirectory#SimpleFSDirectory}. + /** Create a new FSDirectory for the named location (ctor for subclasses). * @param path the path of the directory * @param lockFactory the lock factory to use, or null for the default. * @throws IOException */ - public FSDirectory(File path, LockFactory lockFactory) throws IOException { + protected FSDirectory(File path, LockFactory lockFactory) throws IOException { path = getCanonicalPath(path); init(path, lockFactory); refCount = 1; @@ -344,17 +370,20 @@ public class FSDirectory extends Directory { /** Creates an FSDirectory instance, trying to pick the * best implementation given the current environment. * - *

    Currently this returns {@link MMapDirectory} when - * running in a 64 bit JRE, {@link NIOFSDirectory} on - * non-Windows 32 bit JRE, and {@link SimpleFSDirectory} - * on Windows 32 bit JRE. + *

    Currently this returns {@link NIOFSDirectory} + * on non-Windows JREs and {@link SimpleFSDirectory} + * on Windows. * *

    NOTE: this method may suddenly change which * implementation is returned from release to release, in * the event that higher performance defaults become * possible; if the precise implementation is important to * your application, please instantiate it directly, - * instead. + * instead. On 64 bit systems, it may also good to + * return {@link MMapDirectory}, but this is disabled + * because of officially missing unmap support in Java. + * For optimal performance you should consider using + * this implementation on 64 bit JVMs. * *

    See above */ public static FSDirectory open(File path) throws IOException { @@ -364,15 +393,19 @@ public class FSDirectory extends Directory { /** Just like {@link #open(File)}, but allows you to * also specify a custom {@link LockFactory}. */ public static FSDirectory open(File path, LockFactory lockFactory) throws IOException { - if (Constants.JRE_IS_64BIT) { - return new MMapDirectory(path, lockFactory); - } else if (Constants.WINDOWS) { + /* For testing: + MMapDirectory dir=new MMapDirectory(path, lockFactory); + dir.setUseUnmap(true); + return dir; + */ + if (Constants.WINDOWS) { return new SimpleFSDirectory(path, lockFactory); } else { return new NIOFSDirectory(path, lockFactory); } } + /* will move to ctor, when reflection is removed in 3.0 */ private void init(File path, LockFactory lockFactory) throws IOException { // Set up lockFactory with cascaded defaults: if an instance was passed in, @@ -587,15 +620,11 @@ public class FSDirectory extends Directory { } } - /** @deprecated In 3.0 this method will become abstract */ + /** Creates an IndexOutput for the file with the given name. + * In 3.0 this method will become abstract. */ public IndexOutput createOutput(String name) throws IOException { - ensureOpen(); - createDir(); - File file = new File(directory, name); - if (file.exists() && !file.delete()) // delete existing, if any - throw new IOException("Cannot overwrite: " + file); - - return new FSIndexOutput(file); + initOutput(name); + return new FSIndexOutput(new File(directory, name)); } public void sync(String name) throws IOException { @@ -641,7 +670,8 @@ public class FSDirectory extends Directory { return openInput(name, BufferedIndexInput.BUFFER_SIZE); } - /** @deprecated In 3.0 this method will become abstract */ + /** Creates an IndexInput for the file with the given name. + * In 3.0 this method will become abstract. */ public IndexInput openInput(String name, int bufferSize) throws IOException { ensureOpen(); return new FSIndexInput(new File(directory, name), bufferSize); @@ -698,144 +728,37 @@ public class FSDirectory extends Directory { return this.getClass().getName() + "@" + directory; } + /** @deprecated Use SimpleFSDirectory.SimpleFSIndexInput instead */ - protected static class FSIndexInput extends BufferedIndexInput { + protected static class FSIndexInput extends SimpleFSDirectory.SimpleFSIndexInput { - protected static class Descriptor extends RandomAccessFile { - // remember if the file is open, so that we don't try to close it - // more than once - protected volatile boolean isOpen; - long position; - final long length; - + /** @deprecated */ + protected static class Descriptor extends SimpleFSDirectory.SimpleFSIndexInput.Descriptor { + /** @deprecated */ public Descriptor(File file, String mode) throws IOException { super(file, mode); - isOpen=true; - length=length(); - } - - public void close() throws IOException { - if (isOpen) { - isOpen=false; - super.close(); - } - } - - protected void finalize() throws Throwable { - try { - close(); - } finally { - super.finalize(); - } } } - protected final Descriptor file; - boolean isClone; - + /** @deprecated */ public FSIndexInput(File path) throws IOException { - this(path, BufferedIndexInput.BUFFER_SIZE); + super(path); } + /** @deprecated */ public FSIndexInput(File path, int bufferSize) throws IOException { - super(bufferSize); - file = new Descriptor(path, "r"); + super(path, bufferSize); } - /** IndexInput methods */ - protected void readInternal(byte[] b, int offset, int len) - throws IOException { - synchronized (file) { - long position = getFilePointer(); - if (position != file.position) { - file.seek(position); - file.position = position; - } - int total = 0; - do { - int i = file.read(b, offset+total, len-total); - if (i == -1) - throw new IOException("read past EOF"); - file.position += i; - total += i; - } while (total < len); - } - } - - public void close() throws IOException { - // only close the file if this is not a clone - if (!isClone) file.close(); - } - - protected void seekInternal(long position) { - } - - public long length() { - return file.length; - } - - public Object clone() { - FSIndexInput clone = (FSIndexInput)super.clone(); - clone.isClone = true; - return clone; - } - - /** Method used for testing. Returns true if the underlying - * file descriptor is valid. - */ - boolean isFDValid() throws IOException { - return file.getFD().valid(); - } } /** @deprecated Use SimpleFSDirectory.SimpleFSIndexOutput instead */ - protected static class FSIndexOutput extends BufferedIndexOutput { - RandomAccessFile file = null; - - // remember if the file is open, so that we don't try to close it - // more than once - private volatile boolean isOpen; + protected static class FSIndexOutput extends SimpleFSDirectory.SimpleFSIndexOutput { + /** @deprecated */ public FSIndexOutput(File path) throws IOException { - file = new RandomAccessFile(path, "rw"); - isOpen = true; - } - - /** output methods: */ - public void flushBuffer(byte[] b, int offset, int size) throws IOException { - file.write(b, offset, size); - } - public void close() throws IOException { - // only close the file if it has not been closed yet - if (isOpen) { - boolean success = false; - try { - super.close(); - success = true; - } finally { - isOpen = false; - if (!success) { - try { - file.close(); - } catch (Throwable t) { - // Suppress so we don't mask original exception - } - } else - file.close(); - } - } - } - - /** Random-access methods */ - public void seek(long pos) throws IOException { - super.seek(pos); - file.seek(pos); - } - public long length() throws IOException { - return file.length(); - } - public void setLength(long length) throws IOException { - file.setLength(length); + super(path); } + } } diff --git a/src/java/org/apache/lucene/store/MMapDirectory.java b/src/java/org/apache/lucene/store/MMapDirectory.java index 9dae2fa025a..53a6886eaae 100644 --- a/src/java/org/apache/lucene/store/MMapDirectory.java +++ b/src/java/org/apache/lucene/store/MMapDirectory.java @@ -21,19 +21,47 @@ import java.io.IOException; import java.io.File; import java.io.RandomAccessFile; import java.nio.ByteBuffer; +import java.nio.BufferUnderflowException; import java.nio.channels.FileChannel; import java.nio.channels.FileChannel.MapMode; +import java.security.AccessController; +import java.security.PrivilegedExceptionAction; +import java.security.PrivilegedActionException; +import java.lang.reflect.Method; + /** File-based {@link Directory} implementation that uses * mmap for reading, and {@link * SimpleFSDirectory.SimpleFSIndexOutput} for writing. * - *

    NOTE: memory mapping uses up a portion of the + *

    NOTE: memory mapping uses up a portion of the * virtual memory address space in your process equal to the * size of the file being mapped. Before using this class, - * be sure your have plenty of virtual memory, eg by using a - * 64 bit JRE, or a 32 bit JRE with indexes that are + * be sure your have plenty of virtual address space, e.g. by + * using a 64 bit JRE, or a 32 bit JRE with indexes that are * guaranteed to fit within the address space. + * + *

    Due to + * this bug in Sun's JRE, MMapDirectory's {@link IndexInput#close} + * is unable to close the underlying OS file handle. Only when GC + * finally collects the underlying objects, which could be quite + * some time later, will the file handle be closed. + * + *

    This will consume additional transient disk usage: on Windows, + * attempts to delete or overwrite the files will result in an + * exception; on other platforms, which typically have a "delete on + * last close" semantics, while such operations will succeed, the bytes + * are still consuming space on disk. For many applications this + * limitation is not a problem (e.g. if you have plenty of disk space, + * and you don't rely on overwriting files on Windows) but it's still + * an important limitation to be aware of. + * + *

    This class supplies the workaround mentioned in the bug report + * (disabled by default, see {@link #setUseUnmap}), which may fail on + * non-Sun JVMs. It forcefully unmaps the buffer on close by using + * an undocumented internal cleanup functionality. + * {@link #UNMAP_SUPPORTED} is true, if the workaround + * can be enabled (with no guarantees). */ public class MMapDirectory extends FSDirectory { @@ -47,16 +75,101 @@ public class MMapDirectory extends FSDirectory { super(path, lockFactory); } - // back compatibility so FSDirectory can instantiate via - // reflection - /* @deprecated */ - protected MMapDirectory() throws IOException { + /** Create a new MMapDirectory for the named location and the default lock factory. + * + * @param path the path of the directory + * @throws IOException + */ + public MMapDirectory(File path) throws IOException { + super(path, null); } - private static class MMapIndexInput extends IndexInput { + // back compatibility so FSDirectory can instantiate via reflection + /** @deprecated */ + MMapDirectory() {} + + static final Class[] NO_PARAM_TYPES = new Class[0]; + static final Object[] NO_PARAMS = new Object[0]; + + private boolean useUnmapHack = false; + + /** + * true, if this platform supports unmapping mmaped files. + */ + public static final boolean UNMAP_SUPPORTED; + static { + boolean v; + try { + Class.forName("sun.misc.Cleaner"); + Class.forName("java.nio.DirectByteBuffer") + .getMethod("cleaner", NO_PARAM_TYPES); + v = true; + } catch (Exception e) { + v = false; + } + UNMAP_SUPPORTED = v; + } + + /** + * This method enables the workaround for unmapping the buffers + * from address space after closing {@link IndexInput}, that is + * mentioned in the bug report. This hack may fail on non-Sun JVMs. + * It forcefully unmaps the buffer on close by using + * an undocumented internal cleanup functionality. + *

    NOTE: Enabling this is completely unsupported + * by Java and may lead to JVM crashs if IndexInput + * is closed while another thread is still accessing it (SIGSEGV). + * @throws IllegalArgumentException if {@link #UNMAP_SUPPORTED} + * is false and the workaround cannot be enabled. + */ + public void setUseUnmap(final boolean useUnmapHack) { + if (useUnmapHack && !UNMAP_SUPPORTED) + throw new IllegalArgumentException("Unmap hack not supported on this platform!"); + this.useUnmapHack=useUnmapHack; + } + + /** + * Returns true, if the unmap workaround is enabled. + * @see #setUseUnmap + */ + public boolean getUseUnmap() { + return useUnmapHack; + } + + /** + * Try to unmap the buffer, this method silently fails if no support + * for that in the JVM. On Windows, this leads to the fact, + * that mmapped files cannot be modified or deleted. + */ + final void cleanMapping(final ByteBuffer buffer) throws IOException { + if (useUnmapHack) { + try { + AccessController.doPrivileged(new PrivilegedExceptionAction() { + public Object run() throws Exception { + final Method getCleanerMethod = buffer.getClass() + .getMethod("cleaner", NO_PARAM_TYPES); + getCleanerMethod.setAccessible(true); + final Object cleaner = getCleanerMethod.invoke(buffer, NO_PARAMS); + if (cleaner != null) { + cleaner.getClass().getMethod("clean", NO_PARAM_TYPES) + .invoke(cleaner, NO_PARAMS); + } + return null; + } + }); + } catch (PrivilegedActionException e) { + final IOException ioe = new IOException("unable to unmap the mapped buffer"); + ioe.initCause(e.getCause()); + throw ioe; + } + } + } + + private class MMapIndexInput extends IndexInput { private ByteBuffer buffer; private final long length; + private boolean isClone = false; private MMapIndexInput(RandomAccessFile raf) throws IOException { this.length = raf.length(); @@ -64,12 +177,19 @@ public class MMapDirectory extends FSDirectory { } public byte readByte() throws IOException { - return buffer.get(); + try { + return buffer.get(); + } catch (BufferUnderflowException e) { + throw new IOException("read past EOF"); + } } - public void readBytes(byte[] b, int offset, int len) - throws IOException { - buffer.get(b, offset, len); + public void readBytes(byte[] b, int offset, int len) throws IOException { + try { + buffer.get(b, offset, len); + } catch (BufferUnderflowException e) { + throw new IOException("read past EOF"); + } } public long getFilePointer() { @@ -86,17 +206,26 @@ public class MMapDirectory extends FSDirectory { public Object clone() { MMapIndexInput clone = (MMapIndexInput)super.clone(); + clone.isClone = true; clone.buffer = buffer.duplicate(); return clone; } - public void close() throws IOException {} + public void close() throws IOException { + if (isClone || buffer == null) return; + // unmap the buffer (if enabled) and at least unset it for GC + try { + cleanMapping(buffer); + } finally { + buffer = null; + } + } } // Because Java's ByteBuffer uses an int to address the // values, it's necessary to access a file > // Integer.MAX_VALUE in size using multiple byte buffers. - private static class MultiMMapIndexInput extends IndexInput { + private class MultiMMapIndexInput extends IndexInput { private ByteBuffer[] buffers; private int[] bufSizes; // keep here, ByteBuffer.size() method is optional @@ -109,6 +238,7 @@ public class MMapDirectory extends FSDirectory { private ByteBuffer curBuf; // redundant for speed: buffers[curBufIndex] private int curAvail; // redundant for speed: (bufSizes[curBufIndex] - curBuf.position()) + private boolean isClone = false; public MultiMMapIndexInput(RandomAccessFile raf, int maxBufSize) throws IOException { @@ -125,7 +255,7 @@ public class MMapDirectory extends FSDirectory { + raf.toString()); int nrBuffers = (int) (length / maxBufSize); - if ((nrBuffers * maxBufSize) < length) nrBuffers++; + if (((long) nrBuffers * maxBufSize) < length) nrBuffers++; this.buffers = new ByteBuffer[nrBuffers]; this.bufSizes = new int[nrBuffers]; @@ -145,10 +275,12 @@ public class MMapDirectory extends FSDirectory { public byte readByte() throws IOException { // Performance might be improved by reading ahead into an array of - // eg. 128 bytes and readByte() from there. + // e.g. 128 bytes and readByte() from there. if (curAvail == 0) { curBufIndex++; - curBuf = buffers[curBufIndex]; // index out of bounds when too many bytes requested + if (curBufIndex >= buffers.length) + throw new IOException("read past EOF"); + curBuf = buffers[curBufIndex]; curBuf.position(0); curAvail = bufSizes[curBufIndex]; } @@ -162,7 +294,9 @@ public class MMapDirectory extends FSDirectory { len -= curAvail; offset += curAvail; curBufIndex++; - curBuf = buffers[curBufIndex]; // index out of bounds when too many bytes requested + if (curBufIndex >= buffers.length) + throw new IOException("read past EOF"); + curBuf = buffers[curBufIndex]; curBuf.position(0); curAvail = bufSizes[curBufIndex]; } @@ -171,13 +305,13 @@ public class MMapDirectory extends FSDirectory { } public long getFilePointer() { - return (curBufIndex * (long) maxBufSize) + curBuf.position(); + return ((long) curBufIndex * maxBufSize) + curBuf.position(); } public void seek(long pos) throws IOException { curBufIndex = (int) (pos / maxBufSize); curBuf = buffers[curBufIndex]; - int bufOffset = (int) (pos - (curBufIndex * maxBufSize)); + int bufOffset = (int) (pos - ((long) curBufIndex * maxBufSize)); curBuf.position(bufOffset); curAvail = bufSizes[curBufIndex] - bufOffset; } @@ -188,10 +322,11 @@ public class MMapDirectory extends FSDirectory { public Object clone() { MultiMMapIndexInput clone = (MultiMMapIndexInput)super.clone(); + clone.isClone = true; clone.buffers = new ByteBuffer[buffers.length]; // No need to clone bufSizes. // Since most clones will use only one buffer, duplicate() could also be - // done lazy in clones, eg. when adapting curBuf. + // done lazy in clones, e.g. when adapting curBuf. for (int bufNr = 0; bufNr < buffers.length; bufNr++) { clone.buffers[bufNr] = buffers[bufNr].duplicate(); } @@ -205,15 +340,26 @@ public class MMapDirectory extends FSDirectory { return clone; } - public void close() throws IOException {} + public void close() throws IOException { + if (isClone || buffers == null) return; + try { + for (int bufNr = 0; bufNr < buffers.length; bufNr++) { + // unmap the buffer (if enabled) and at least unset it for GC + try { + cleanMapping(buffers[bufNr]); + } finally { + buffers[bufNr] = null; + } + } + } finally { + buffers = null; + } + } } private final int MAX_BBUF = Integer.MAX_VALUE; - public IndexInput openInput(String name) throws IOException { - return openInput(name, BufferedIndexInput.BUFFER_SIZE); - } - + /** Creates an IndexInput for the file with the given name. */ public IndexInput openInput(String name, int bufferSize) throws IOException { ensureOpen(); File f = new File(getFile(), name); @@ -227,6 +373,7 @@ public class MMapDirectory extends FSDirectory { } } + /** Creates an IndexOutput for the file with the given name. */ public IndexOutput createOutput(String name) throws IOException { initOutput(name); return new SimpleFSDirectory.SimpleFSIndexOutput(new File(directory, name)); diff --git a/src/java/org/apache/lucene/store/NIOFSDirectory.java b/src/java/org/apache/lucene/store/NIOFSDirectory.java index 85536913047..13bc83d33c4 100644 --- a/src/java/org/apache/lucene/store/NIOFSDirectory.java +++ b/src/java/org/apache/lucene/store/NIOFSDirectory.java @@ -50,23 +50,32 @@ public class NIOFSDirectory extends FSDirectory { super(path, lockFactory); } - // back compatibility so FSDirectory can instantiate via reflection - /* @deprecated */ - protected NIOFSDirectory() throws IOException { + /** Create a new NIOFSDirectory for the named location and the default lock factory. + * + * @param path the path of the directory + * @throws IOException + */ + public NIOFSDirectory(File path) throws IOException { + super(path, null); } - // Inherit javadoc + // back compatibility so FSDirectory can instantiate via reflection + /** @deprecated */ + NIOFSDirectory() {} + + /** Creates an IndexInput for the file with the given name. */ public IndexInput openInput(String name, int bufferSize) throws IOException { ensureOpen(); return new NIOFSIndexInput(new File(getFile(), name), bufferSize); } + /** Creates an IndexOutput for the file with the given name. */ public IndexOutput createOutput(String name) throws IOException { initOutput(name); return new SimpleFSDirectory.SimpleFSIndexOutput(new File(directory, name)); } - private static class NIOFSIndexInput extends FSDirectory.FSIndexInput { + private static class NIOFSIndexInput extends SimpleFSDirectory.SimpleFSIndexInput { private ByteBuffer byteBuf; // wraps the buffer for NIO diff --git a/src/java/org/apache/lucene/store/SimpleFSDirectory.java b/src/java/org/apache/lucene/store/SimpleFSDirectory.java index 16a36ea6ae3..999eede7d35 100644 --- a/src/java/org/apache/lucene/store/SimpleFSDirectory.java +++ b/src/java/org/apache/lucene/store/SimpleFSDirectory.java @@ -38,19 +38,27 @@ public class SimpleFSDirectory extends FSDirectory { public SimpleFSDirectory(File path, LockFactory lockFactory) throws IOException { super(path, lockFactory); } - - // Inherit javadoc - public IndexOutput createOutput(String name) throws IOException { - ensureOpen(); - createDir(); - File file = new File(directory, name); - if (file.exists() && !file.delete()) // delete existing, if any - throw new IOException("Cannot overwrite: " + file); - - return new SimpleFSIndexOutput(file); + + /** Create a new SimpleFSDirectory for the named location and the default lock factory. + * + * @param path the path of the directory + * @throws IOException + */ + public SimpleFSDirectory(File path) throws IOException { + super(path, null); } - // Inherit javadoc + // back compatibility so FSDirectory can instantiate via reflection + /** @deprecated */ + SimpleFSDirectory() {} + + /** Creates an IndexOutput for the file with the given name. */ + public IndexOutput createOutput(String name) throws IOException { + initOutput(name); + return new SimpleFSIndexOutput(new File(directory, name)); + } + + /** Creates an IndexInput for the file with the given name. */ public IndexInput openInput(String name, int bufferSize) throws IOException { ensureOpen(); return new SimpleFSIndexInput(new File(directory, name), bufferSize); diff --git a/src/test/org/apache/lucene/index/TestCompoundFile.java b/src/test/org/apache/lucene/index/TestCompoundFile.java index 34efe5bbc70..32a55e93bdc 100644 --- a/src/test/org/apache/lucene/index/TestCompoundFile.java +++ b/src/test/org/apache/lucene/index/TestCompoundFile.java @@ -26,7 +26,7 @@ import junit.textui.TestRunner; import org.apache.lucene.store.IndexOutput; import org.apache.lucene.store.Directory; import org.apache.lucene.store.IndexInput; -import org.apache.lucene.store.FSDirectory; +import org.apache.lucene.store.SimpleFSDirectory; import org.apache.lucene.store._TestHelper; import org.apache.lucene.util._TestUtil; @@ -62,9 +62,14 @@ public class TestCompoundFile extends LuceneTestCase super.setUp(); File file = new File(System.getProperty("tempDir"), "testIndex"); _TestUtil.rmDir(file); - dir = FSDirectory.open(file); + // use a simple FSDir here, to be sure to have SimpleFSInputs + dir = new SimpleFSDirectory(file,null); } + public void tearDown() throws Exception { + dir.close(); + super.tearDown(); + } /** Creates a file of the specified size with random data. */ private void createRandomFile(Directory dir, String name, int size) @@ -310,10 +315,10 @@ public class TestCompoundFile extends LuceneTestCase public void testReadAfterClose() throws IOException { - demo_FSIndexInputBug((FSDirectory) dir, "test"); + demo_FSIndexInputBug(dir, "test"); } - private void demo_FSIndexInputBug(FSDirectory fsdir, String file) + private void demo_FSIndexInputBug(Directory fsdir, String file) throws IOException { // Setup the test file - we need more than 1024 bytes @@ -358,7 +363,7 @@ public class TestCompoundFile extends LuceneTestCase CompoundFileReader.CSIndexInput cis = (CompoundFileReader.CSIndexInput) is; - return _TestHelper.isFSIndexInputOpen(cis.base); + return _TestHelper.isSimpleFSIndexInputOpen(cis.base); } else { return false; } @@ -373,49 +378,47 @@ public class TestCompoundFile extends LuceneTestCase IndexInput expected = dir.openInput("f11"); // this test only works for FSIndexInput - if (_TestHelper.isFSIndexInput(expected)) { + assertTrue(_TestHelper.isSimpleFSIndexInput(expected)); + assertTrue(_TestHelper.isSimpleFSIndexInputOpen(expected)); - assertTrue(_TestHelper.isFSIndexInputOpen(expected)); + IndexInput one = cr.openInput("f11"); + assertTrue(isCSIndexInputOpen(one)); - IndexInput one = cr.openInput("f11"); - assertTrue(isCSIndexInputOpen(one)); + IndexInput two = (IndexInput) one.clone(); + assertTrue(isCSIndexInputOpen(two)); - IndexInput two = (IndexInput) one.clone(); - assertTrue(isCSIndexInputOpen(two)); + assertSameStreams("basic clone one", expected, one); + expected.seek(0); + assertSameStreams("basic clone two", expected, two); - assertSameStreams("basic clone one", expected, one); - expected.seek(0); - assertSameStreams("basic clone two", expected, two); + // Now close the first stream + one.close(); + assertTrue("Only close when cr is closed", isCSIndexInputOpen(one)); - // Now close the first stream - one.close(); - assertTrue("Only close when cr is closed", isCSIndexInputOpen(one)); - - // The following should really fail since we couldn't expect to - // access a file once close has been called on it (regardless of - // buffering and/or clone magic) - expected.seek(0); - two.seek(0); - assertSameStreams("basic clone two/2", expected, two); + // The following should really fail since we couldn't expect to + // access a file once close has been called on it (regardless of + // buffering and/or clone magic) + expected.seek(0); + two.seek(0); + assertSameStreams("basic clone two/2", expected, two); - // Now close the compound reader - cr.close(); - assertFalse("Now closed one", isCSIndexInputOpen(one)); - assertFalse("Now closed two", isCSIndexInputOpen(two)); + // Now close the compound reader + cr.close(); + assertFalse("Now closed one", isCSIndexInputOpen(one)); + assertFalse("Now closed two", isCSIndexInputOpen(two)); - // The following may also fail since the compound stream is closed - expected.seek(0); - two.seek(0); - //assertSameStreams("basic clone two/3", expected, two); + // The following may also fail since the compound stream is closed + expected.seek(0); + two.seek(0); + //assertSameStreams("basic clone two/3", expected, two); - // Now close the second clone - two.close(); - expected.seek(0); - two.seek(0); - //assertSameStreams("basic clone two/4", expected, two); - } + // Now close the second clone + two.close(); + expected.seek(0); + two.seek(0); + //assertSameStreams("basic clone two/4", expected, two); expected.close(); } diff --git a/src/test/org/apache/lucene/index/TestDoc.java b/src/test/org/apache/lucene/index/TestDoc.java index e5ef5459ae3..a0509e8a2ad 100644 --- a/src/test/org/apache/lucene/index/TestDoc.java +++ b/src/test/org/apache/lucene/index/TestDoc.java @@ -59,7 +59,7 @@ public class TestDoc extends LuceneTestCase { indexDir = new File(workDir, "testIndex"); indexDir.mkdirs(); - Directory directory = FSDirectory.getDirectory(indexDir, true); + Directory directory = FSDirectory.open(indexDir); directory.close(); files = new LinkedList(); diff --git a/src/test/org/apache/lucene/index/TestIndexModifier.java b/src/test/org/apache/lucene/index/TestIndexModifier.java index 35ec747e6c2..8431b9a7a40 100644 --- a/src/test/org/apache/lucene/index/TestIndexModifier.java +++ b/src/test/org/apache/lucene/index/TestIndexModifier.java @@ -144,7 +144,7 @@ public class TestIndexModifier extends LuceneTestCase { if (tempDir == null) throw new IOException("java.io.tmpdir undefined, cannot run test"); File indexDir = new File(tempDir, "lucenetestindex"); - Directory rd = FSDirectory.getDirectory(indexDir); + Directory rd = FSDirectory.open(indexDir); IndexThread.id = 0; IndexThread.idStack.clear(); IndexModifier index = new IndexModifier(rd, new StandardAnalyzer(), create); diff --git a/src/test/org/apache/lucene/index/TestIndexWriter.java b/src/test/org/apache/lucene/index/TestIndexWriter.java index ef6932c866b..f25b300d546 100644 --- a/src/test/org/apache/lucene/index/TestIndexWriter.java +++ b/src/test/org/apache/lucene/index/TestIndexWriter.java @@ -4207,7 +4207,7 @@ public class TestIndexWriter extends LuceneTestCase public void testOtherFiles() throws Throwable { File indexDir = new File(System.getProperty("tempDir"), "otherfiles"); - Directory dir = new FSDirectory(indexDir, null); + Directory dir = FSDirectory.open(indexDir); try { // Create my own random file: diff --git a/src/test/org/apache/lucene/index/TestStressIndexing2.java b/src/test/org/apache/lucene/index/TestStressIndexing2.java index e7a94eade8a..9239cc551d5 100644 --- a/src/test/org/apache/lucene/index/TestStressIndexing2.java +++ b/src/test/org/apache/lucene/index/TestStressIndexing2.java @@ -69,7 +69,7 @@ public class TestStressIndexing2 extends LuceneTestCase { public void testRandom() throws Throwable { r = newRandom(); Directory dir1 = new MockRAMDirectory(); - // dir1 = FSDirectory.getDirectory("foofoofoo"); + // dir1 = FSDirectory.open("foofoofoo"); Directory dir2 = new MockRAMDirectory(); // mergeFactor=2; maxBufferedDocs=2; Map docs = indexRandom(1, 3, 2, dir1); Map docs = indexRandom(10, 100, 100, dir1); diff --git a/src/test/org/apache/lucene/store/TestBufferedIndexInput.java b/src/test/org/apache/lucene/store/TestBufferedIndexInput.java index d229e85e25b..2cb33f0417e 100755 --- a/src/test/org/apache/lucene/store/TestBufferedIndexInput.java +++ b/src/test/org/apache/lucene/store/TestBufferedIndexInput.java @@ -212,7 +212,7 @@ public class TestBufferedIndexInput extends LuceneTestCase { public MockFSDirectory(File path, Random rand) throws IOException { this.rand = rand; lockFactory = new NoLockFactory(); - dir = FSDirectory.open(path); + dir = new SimpleFSDirectory(path, null); } public IndexInput openInput(String name) throws IOException { diff --git a/src/test/org/apache/lucene/store/TestDirectory.java b/src/test/org/apache/lucene/store/TestDirectory.java index 91938e3151a..1e26ca4a272 100644 --- a/src/test/org/apache/lucene/store/TestDirectory.java +++ b/src/test/org/apache/lucene/store/TestDirectory.java @@ -53,7 +53,7 @@ public class TestDirectory extends LuceneTestCase { int sz = 3; Directory[] dirs = new Directory[sz]; - dirs[0] = new FSDirectory(path, null); + dirs[0] = new SimpleFSDirectory(path, null); dirs[1] = new NIOFSDirectory(path, null); dirs[2] = new MMapDirectory(path, null); @@ -123,7 +123,7 @@ public class TestDirectory extends LuceneTestCase { File path = new File(System.getProperty("tempDir"), "doesnotexist"); try { assertTrue(!path.exists()); - Directory dir = new FSDirectory(path, null); + Directory dir = new SimpleFSDirectory(path, null); assertTrue(!path.exists()); dir.close(); } finally { @@ -159,7 +159,7 @@ public class TestDirectory extends LuceneTestCase { try { path.mkdirs(); new File(path, "subdir").mkdirs(); - Directory fsDir = new FSDirectory(path, null); + Directory fsDir = new SimpleFSDirectory(path, null); assertEquals(0, new RAMDirectory(fsDir).listAll().length); } finally { _TestUtil.rmDir(path); @@ -169,13 +169,13 @@ public class TestDirectory extends LuceneTestCase { // LUCENE-1468 public void testNotDirectory() throws Throwable { File path = new File(System.getProperty("tempDir"), "testnotdir"); - Directory fsDir = new FSDirectory(path, null); + Directory fsDir = new SimpleFSDirectory(path, null); try { IndexOutput out = fsDir.createOutput("afile"); out.close(); assertTrue(fsDir.fileExists("afile")); try { - new FSDirectory(new File(path, "afile"), null); + new SimpleFSDirectory(new File(path, "afile"), null); fail("did not hit expected exception"); } catch (NoSuchDirectoryException nsde) { // Expected diff --git a/src/test/org/apache/lucene/store/TestMMapDirectory.java b/src/test/org/apache/lucene/store/TestMMapDirectory.java deleted file mode 100644 index 062ce29e10f..00000000000 --- a/src/test/org/apache/lucene/store/TestMMapDirectory.java +++ /dev/null @@ -1,52 +0,0 @@ -package org.apache.lucene.store; - -/** - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import org.apache.lucene.util.LuceneTestCase; -import java.lang.reflect.Method; - -public class TestMMapDirectory extends LuceneTestCase { - - // Simply verify that if there is a method in FSDirectory - // that returns IndexInput or a subclass, that - // MMapDirectory overrides it. - public void testIndexInputMethods() throws ClassNotFoundException { - - Class FSDirectory = Class.forName("org.apache.lucene.store.FSDirectory"); - Class IndexInput = Class.forName("org.apache.lucene.store.IndexInput"); - Class MMapDirectory = Class.forName("org.apache.lucene.store.MMapDirectory"); - - Method[] methods = FSDirectory.getDeclaredMethods(); - for(int i=0;i