LUCENE-1877, LUCENE-1885: Use NativeFSLockFactory as default for new API (direct ctors & FSDir.open). Fix isLocked() bug in NativeFSLock.

git-svn-id: https://svn.apache.org/repos/asf/lucene/java/trunk@811157 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Uwe Schindler 2009-09-03 22:17:31 +00:00
parent fe6c88cebe
commit 894dae9059
11 changed files with 194 additions and 116 deletions

View File

@ -425,6 +425,16 @@ API Changes
38. LUCENE-1847: Similarity#idf for both a Term and Term Collection have
been deprecated. New versions that return an IDFExplanation have been
added. (Yasoja Seneviratne, Mike McCandless, Mark Miller)
39. LUCENE-1877: Made NativeFSLockFactory the default for
the new FSDirectory API (open(), FSDirectory subclass ctors).
All FSDirectory system properties were deprecated and all lock
implementations use no lock prefix if the locks are stored inside
the index directory. Because the deprecated String/File ctors of
IndexWriter and IndexReader (LUCENE-1672) and FSDirectory.getDirectory()
still use the old SimpleFSLockFactory and the new API
NativeFSLockFactory, we strongly recommend not to mix deprecated
and new API. (Uwe Schindler, Mike McCandless)
Bug fixes
@ -534,6 +544,11 @@ Bug fixes
in their Weight#explain methods - these stats should be corpus wide.
(Yasoja Seneviratne, Mike McCandless, Mark Miller)
27. LUCENE-1885: Fix the bug that NativeFSLock.isLocked() did not work,
if the lock was obtained by another NativeFSLock(Factory) instance.
Because of this IndexReader.isLocked() and IndexWriter.isLocked() did
not work correctly. (Uwe Schindler)
New features
1. LUCENE-1411: Added expert API to open an IndexWriter on a prior

View File

@ -42,7 +42,7 @@
<property name="Name" value="Lucene"/>
<property name="dev.version" value="2.9"/>
<property name="version" value="${dev.version}"/>
<property name="compatibility.tag" value="lucene_2_4_back_compat_tests_20090827"/>
<property name="compatibility.tag" value="lucene_2_4_back_compat_tests_20090903"/>
<property name="spec.version" value="${version}"/>
<property name="year" value="2000-${current.year}"/>
<property name="final.name" value="lucene-${name}-${version}"/>

View File

@ -95,12 +95,20 @@ import org.apache.lucene.index.IndexWriter;
* desired implementation directly.
*
* <p>The locking implementation is by default {@link
* SimpleFSLockFactory}, but can be changed either by
* passing in a custom {@link LockFactory} instance, or
* specifying the LockFactory class by setting
* NativeFSLockFactory}, but can be changed by
* passing in a custom {@link LockFactory} instance.
* The deprecated <code>getDirectory</code> methods default to use
* {@link SimpleFSLockFactory} for backwards compatibility.
* The system properties
* <code>org.apache.lucene.store.FSDirectoryLockFactoryClass</code>
* Java system property, or by calling {@link
* #setLockFactory} after creating the Directory.
* and <code>org.apache.lucene.FSDirectory.class</code>
* are deprecated and only used by the deprecated
* <code>getDirectory</code> methods. The system property
* <code>org.apache.lucene.lockDir</code> is ignored completely,
* If you really want to store locks
* elsewhere, you can create your own {@link
* SimpleFSLockFactory} (or {@link NativeFSLockFactory},
* etc.) passing in your preferred lock directory.
*
* <p><em>In 3.0 this class will become abstract.</em>
*
@ -129,6 +137,11 @@ public class FSDirectory extends Directory {
* Set whether Lucene's use of lock files is disabled. By default,
* lock files are enabled. They should only be disabled if the index
* is on a read-only medium like a CD-ROM.
* @deprecated Use a {@link #open(File, LockFactory)} or a constructor
* that takes a {@link LockFactory} and supply
* {@link NoLockFactory#getNoLockFactory}. This setting does not work
* with {@link #open(File)} only the deprecated <code>getDirectory</code>
* respect this setting.
*/
public static void setDisableLocks(boolean doDisableLocks) {
FSDirectory.disableLocks = doDisableLocks;
@ -138,6 +151,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
* @deprecated Use a constructor that takes a {@link LockFactory} and
* supply {@link NoLockFactory#getNoLockFactory}.
*/
public static boolean getDisableLocks() {
return FSDirectory.disableLocks;
@ -150,12 +165,12 @@ public class FSDirectory extends Directory {
* @deprecated As of 2.1, <code>LOCK_DIR</code> is unused
* because the write.lock is now stored by default in the
* index directory. If you really want to store locks
* elsewhere you can create your own {@link
* elsewhere, you can create your own {@link
* SimpleFSLockFactory} (or {@link NativeFSLockFactory},
* etc.) passing in your preferred lock directory. Then,
* pass this <code>LockFactory</code> instance to one of
* the <code>getDirectory</code> methods that take a
* <code>lockFactory</code> (for example, {@link #getDirectory(String, LockFactory)}).
* the <code>open</code> methods that take a
* <code>lockFactory</code> (for example, {@link #open(File, LockFactory)}).
*/
public static final String LOCK_DIR = System.getProperty("org.apache.lucene.lockDir",
System.getProperty("java.io.tmpdir"));
@ -358,17 +373,23 @@ public class FSDirectory extends Directory {
/** 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.
* @param lockFactory the lock factory to use, or null for the default
* ({@link NativeFSLockFactory});
* @throws IOException
*/
protected FSDirectory(File path, LockFactory lockFactory) throws IOException {
path = getCanonicalPath(path);
// new ctors use always NativeFSLockFactory as default:
if (lockFactory == null) {
lockFactory = new NativeFSLockFactory();
}
init(path, lockFactory);
refCount = 1;
}
/** Creates an FSDirectory instance, trying to pick the
* best implementation given the current environment.
* The directory returned uses the {@link NativeFSLockFactory}.
*
* <p>Currently this returns {@link NIOFSDirectory}
* on non-Windows JREs and {@link SimpleFSDirectory}
@ -419,8 +440,6 @@ public class FSDirectory extends Directory {
if (directory.exists() && !directory.isDirectory())
throw new NoSuchDirectoryException("file '" + directory + "' exists but is not a directory");
boolean doClearLockID = false;
if (lockFactory == null) {
if (disableLocks) {
@ -447,27 +466,28 @@ public class FSDirectory extends Directory {
} catch (ClassCastException e) {
throw new IOException("unable to cast LockClass " + lockClassName + " instance to a LockFactory");
}
if (lockFactory instanceof NativeFSLockFactory) {
((NativeFSLockFactory) lockFactory).setLockDir(path);
} else if (lockFactory instanceof SimpleFSLockFactory) {
((SimpleFSLockFactory) lockFactory).setLockDir(path);
}
} else {
// Our default lock is SimpleFSLockFactory;
// default lockDir is our index directory:
lockFactory = new SimpleFSLockFactory(path);
doClearLockID = true;
lockFactory = new SimpleFSLockFactory();
}
}
}
setLockFactory(lockFactory);
if (doClearLockID) {
// Clear the prefix because write.lock will be
// stored in our directory:
lockFactory.setLockPrefix(null);
// for filesystem based LockFactory, delete the lockPrefix, if the locks are placed
// in index dir. If no index dir is given, set ourselves
if (lockFactory instanceof FSLockFactory) {
final FSLockFactory lf = (FSLockFactory) lockFactory;
final File dir = lf.getLockDir();
// if the lock factory has no lockDir set, use the this directory as lockDir
if (dir == null) {
lf.setLockDir(this.directory);
lf.setLockPrefix(null);
} else if (dir.getCanonicalPath().equals(this.directory.getCanonicalPath())) {
lf.setLockPrefix(null);
}
}
}

View File

@ -0,0 +1,53 @@
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 java.io.File;
/**
* Base class for file system based locking implementation.
*/
public abstract class FSLockFactory extends LockFactory {
/**
* Directory for the lock files.
*/
protected File lockDir = null;
/**
* Set the lock directory. This method can be only called
* once to initialize the lock directory. It is used by {@link FSDirectory}
* to set the lock directory to itsself.
* Subclasses can also use this method to set the directory
* in the constructor.
*/
protected void setLockDir(File lockDir) {
if (this.lockDir != null)
throw new IllegalStateException("You can set the lock directory for this factory only once.");
this.lockDir = lockDir;
}
/**
* Retrieve the lock directory.
*/
public File getLockDir() {
return lockDir;
}
}

View File

@ -35,7 +35,7 @@ import java.io.IOException;
public abstract class LockFactory {
protected String lockPrefix = "";
protected String lockPrefix = null;
/**
* Set the prefix in use for all locks created in this

View File

@ -74,14 +74,15 @@ public class MMapDirectory extends FSDirectory {
/** Create a new MMapDirectory for the named location.
*
* @param path the path of the directory
* @param lockFactory the lock factory to use, or null for the default.
* @param lockFactory the lock factory to use, or null for the default
* ({@link NativeFSLockFactory});
* @throws IOException
*/
public MMapDirectory(File path, LockFactory lockFactory) throws IOException {
super(path, lockFactory);
}
/** Create a new MMapDirectory for the named location and the default lock factory.
/** Create a new MMapDirectory for the named location and {@link NativeFSLockFactory}.
*
* @param path the path of the directory
* @throws IOException

View File

@ -43,14 +43,15 @@ public class NIOFSDirectory extends FSDirectory {
/** Create a new NIOFSDirectory for the named location.
*
* @param path the path of the directory
* @param lockFactory the lock factory to use, or null for the default.
* @param lockFactory the lock factory to use, or null for the default
* ({@link NativeFSLockFactory});
* @throws IOException
*/
public NIOFSDirectory(File path, LockFactory lockFactory) throws IOException {
super(path, lockFactory);
}
/** Create a new NIOFSDirectory for the named location and the default lock factory.
/** Create a new NIOFSDirectory for the named location and {@link NativeFSLockFactory}.
*
* @param path the path of the directory
* @throws IOException

View File

@ -55,45 +55,50 @@ import java.util.Random;
* @see LockFactory
*/
public class NativeFSLockFactory extends LockFactory {
public class NativeFSLockFactory extends FSLockFactory {
/**
* Directory specified by <code>org.apache.lucene.lockDir</code>
* system property. If that is not set, then <code>java.io.tmpdir</code>
* system property is used.
*/
private File lockDir;
private volatile boolean tested = false;
// Simple test to verify locking system is "working". On
// NFS, if it's misconfigured, you can hit long (35
// second) timeouts which cause Lock.obtain to take far
// too long (it assumes the obtain() call takes zero
// time). Since it's a configuration problem, we test up
// front once on creating the LockFactory:
private void acquireTestLock() throws IOException {
// time).
private synchronized void acquireTestLock() {
if (tested) return;
tested = true;
// Ensure that lockDir exists and is a directory.
if (!lockDir.exists()) {
if (!lockDir.mkdirs())
throw new RuntimeException("Cannot create directory: " +
lockDir.getAbsolutePath());
} else if (!lockDir.isDirectory()) {
throw new RuntimeException("Found regular file where directory expected: " +
lockDir.getAbsolutePath());
}
String randomLockName = "lucene-" + Long.toString(new Random().nextInt(), Character.MAX_RADIX) + "-test.lock";
Lock l = makeLock(randomLockName);
try {
l.obtain();
l.release();
} catch (IOException e) {
IOException e2 = new IOException("Failed to acquire random test lock; please verify filesystem for lock directory '" + lockDir + "' supports locking");
RuntimeException e2 = new RuntimeException("Failed to acquire random test lock; please verify filesystem for lock directory '" + lockDir + "' supports locking");
e2.initCause(e);
throw e2;
}
l.release();
}
}
/**
* Create a NativeFSLockFactory instance, with null (unset)
* lock directory. This is package-private and is only
* used by FSDirectory when creating this LockFactory via
* the System property
* org.apache.lucene.store.FSDirectoryLockFactoryClass.
* lock directory. When you pass this factory to a {@link FSDirectory}
* subclass, the lock directory is automatically set to the
* directory itsself. Be sure to create one instance for each directory
* your create!
*/
NativeFSLockFactory() throws IOException {
public NativeFSLockFactory() throws IOException {
this((File) null);
}
@ -117,32 +122,10 @@ public class NativeFSLockFactory extends LockFactory {
setLockDir(lockDir);
}
/**
* Set the lock directory. This is package-private and is
* only used externally by FSDirectory when creating this
* LockFactory via the System property
* org.apache.lucene.store.FSDirectoryLockFactoryClass.
*/
void setLockDir(File lockDir) throws IOException {
this.lockDir = lockDir;
if (lockDir != null) {
// Ensure that lockDir exists and is a directory.
if (!lockDir.exists()) {
if (!lockDir.mkdirs())
throw new IOException("Cannot create directory: " +
lockDir.getAbsolutePath());
} else if (!lockDir.isDirectory()) {
throw new IOException("Found regular file where directory expected: " +
lockDir.getAbsolutePath());
}
acquireTestLock();
}
}
public synchronized Lock makeLock(String lockName) {
acquireTestLock();
if (lockPrefix != null)
lockName = lockPrefix + "-n-" + lockName;
lockName = lockPrefix + "-" + lockName;
return new NativeFSLock(lockDir, lockName);
}
@ -153,7 +136,7 @@ public class NativeFSLockFactory extends LockFactory {
// really want to see the files go away:
if (lockDir.exists()) {
if (lockPrefix != null) {
lockName = lockPrefix + "-n-" + lockName;
lockName = lockPrefix + "-" + lockName;
}
File lockFile = new File(lockDir, lockName);
if (lockFile.exists() && !lockFile.delete()) {
@ -188,9 +171,13 @@ class NativeFSLock extends Lock {
path = new File(lockDir, lockFileName);
}
private synchronized boolean lockExists() {
return lock != null;
}
public synchronized boolean obtain() throws IOException {
if (isLocked()) {
if (lockExists()) {
// Our instance is already locked:
return false;
}
@ -276,7 +263,7 @@ class NativeFSLock extends Lock {
}
} finally {
if (markedHeld && !isLocked()) {
if (markedHeld && !lockExists()) {
synchronized(LOCK_HELD) {
if (LOCK_HELD.contains(canonicalPath)) {
LOCK_HELD.remove(canonicalPath);
@ -284,11 +271,11 @@ class NativeFSLock extends Lock {
}
}
}
return isLocked();
return lockExists();
}
public synchronized void release() throws IOException {
if (isLocked()) {
if (lockExists()) {
try {
lock.release();
} finally {
@ -313,7 +300,22 @@ class NativeFSLock extends Lock {
}
public synchronized boolean isLocked() {
return lock != null;
// The test for is isLocked is not directly possible with native file locks:
// First a shortcut, if a lock reference in this instance is available
if (lockExists()) return true;
// Look if lock file is present; if not, there can definitely be no lock!
if (!path.exists()) return false;
// Try to obtain and release (if was locked) the lock
try {
boolean obtained = obtain();
if (obtained) release();
return !obtained;
} catch (IOException ioe) {
return false;
}
}
public String toString() {

View File

@ -32,14 +32,15 @@ public class SimpleFSDirectory extends FSDirectory {
/** Create a new SimpleFSDirectory for the named location.
*
* @param path the path of the directory
* @param lockFactory the lock factory to use, or null for the default.
* @param lockFactory the lock factory to use, or null for the default
* ({@link NativeFSLockFactory});
* @throws IOException
*/
public SimpleFSDirectory(File path, LockFactory lockFactory) throws IOException {
super(path, lockFactory);
}
/** Create a new SimpleFSDirectory for the named location and the default lock factory.
/** Create a new SimpleFSDirectory for the named location and {@link NativeFSLockFactory}.
*
* @param path the path of the directory
* @throws IOException

View File

@ -22,8 +22,7 @@ import java.io.IOException;
/**
* <p>Implements {@link LockFactory} using {@link
* File#createNewFile()}. This is the default LockFactory
* for {@link FSDirectory}.</p>
* File#createNewFile()}.</p>
*
* <p><b>NOTE:</b> the <a target="_top"
* href="http://java.sun.com/j2se/1.4.2/docs/api/java/io/File.html#createNewFile()">javadocs
@ -52,24 +51,16 @@ import java.io.IOException;
* @see LockFactory
*/
public class SimpleFSLockFactory extends LockFactory {
/**
* Directory specified by <code>org.apache.lucene.lockDir</code>
* system property. If that is not set, then <code>java.io.tmpdir</code>
* system property is used.
*/
private File lockDir;
public class SimpleFSLockFactory extends FSLockFactory {
/**
* Create a SimpleFSLockFactory instance, with null (unset)
* lock directory. This is package-private and is only
* used by FSDirectory when creating this LockFactory via
* the System property
* org.apache.lucene.store.FSDirectoryLockFactoryClass.
* lock directory. When you pass this factory to a {@link FSDirectory}
* subclass, the lock directory is automatically set to the
* directory itsself. Be sure to create one instance for each directory
* your create!
*/
SimpleFSLockFactory() throws IOException {
public SimpleFSLockFactory() throws IOException {
this((File) null);
}
@ -90,16 +81,6 @@ public class SimpleFSLockFactory extends LockFactory {
setLockDir(lockDir);
}
/**
* Set the lock directory. This is package-private and is
* only used externally by FSDirectory when creating this
* LockFactory via the System property
* org.apache.lucene.store.FSDirectoryLockFactoryClass.
*/
void setLockDir(File lockDir) throws IOException {
this.lockDir = lockDir;
}
public Lock makeLock(String lockName) {
if (lockPrefix != null) {
lockName = lockPrefix + "-" + lockName;

View File

@ -381,26 +381,30 @@ public class TestLockFactory extends LuceneTestCase {
assertTrue("failed to obtain 2nd lock after first one was freed", l2.obtain());
l2.release();
// Make sure we can obtain first one again:
// Make sure we can obtain first one again, test isLocked():
assertTrue("failed to obtain lock", l.obtain());
assertTrue(l.isLocked());
assertTrue(l2.isLocked());
l.release();
assertFalse(l.isLocked());
assertFalse(l2.isLocked());
}
// Verify: NativeFSLockFactory assigns different lock
// prefixes to different directories:
// Verify: NativeFSLockFactory assigns null as lockPrefix if the lockDir is inside directory
public void testNativeFSLockFactoryPrefix() throws IOException {
// Make sure we get identical instances:
File fdir1 = _TestUtil.getTempDir("TestLockFactory.8");
File fdir2 = _TestUtil.getTempDir("TestLockFactory.8.Lockdir");
Directory dir1 = FSDirectory.open(fdir1, new NativeFSLockFactory(fdir1));
File fdir2 = _TestUtil.getTempDir("TestLockFactory.9");
Directory dir2 = FSDirectory.open(fdir2, new NativeFSLockFactory(fdir2));
// same directory, but locks are stored somewhere else. The prefix of the lock factory should != null
Directory dir2 = FSDirectory.open(fdir1, new NativeFSLockFactory(fdir2));
String prefix1 = dir1.getLockFactory().getLockPrefix();
assertNull("Lock prefix for lockDir same as directory should be null", prefix1);
String prefix2 = dir2.getLockFactory().getLockPrefix();
assertNotNull("Lock prefix for lockDir outside of directory should be not null", prefix2);
assertTrue("Native Lock Factories are incorrectly shared: dir1 and dir2 have same lock prefix '" + prefix1 + "'; they should be different",
!prefix1.equals(prefix2));
_TestUtil.rmDir(fdir1);
_TestUtil.rmDir(fdir2);
}