LUCENE-771:

* Default write lock location to the index directory and use the
    name "write.lock" (without prefix).

  * Make the LockFactory.clearAllLocks() method package protected.

  * Remove LOCK_DIR and no-argument constructors for
    SimpleFSLockFactory and NativeFSLockFactory (no more global lock
    directory).

  * Deprecate FSDirectory.LOCK_DIR


git-svn-id: https://svn.apache.org/repos/asf/lucene/java/trunk@495760 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -46,6 +46,13 @@ Changes in runtime behavior
     that has since been fixed, plus we no longer support pre-1.4.2 JVMs.
     (Otis Gospodnetic)
 
+ 9. LUCENE-771: The default location of the write lock is now the
+    index directory, and is named simply "write.lock" (without a big
+    digest prefix).  The system properties "org.apache.lucene.lockDir"
+    nor "java.io.tmpdir" are no longer used as the global directory
+    for storing lock files, and the LOCK_DIR field of FSDirectory is
+    now deprecated.  (Mike McCandless)
+
 New features
 
  1. LUCENE-503: New ThaiAnalyzer and ThaiWordFilter in contrib/analyzers

src/java/org/apache/lucene/store/FSDirectory.java

@@ -28,6 +28,9 @@ import java.util.Hashtable;
 
 import org.apache.lucene.index.IndexFileNameFilter;
 
+// Used only for WRITE_LOCK_NAME:
+import org.apache.lucene.index.IndexWriter;
+
 /**
  * Straightforward implementation of {@link Directory} as a directory of files.
  * Locking implementation is by default the {@link SimpleFSLockFactory}, but
@@ -73,17 +76,23 @@ public class FSDirectory extends Directory {
     return FSDirectory.disableLocks;
   }
 
-  // TODO: LOCK_DIR really should only appear in the SimpleFSLockFactory
-  // (and any other file-system based locking implementations).  When we
-  // can next break backwards compatibility we should deprecate it and then
-  // move it.
-
   /**
    * Directory specified by <code>org.apache.lucene.lockDir</code>
-   * or <code>java.io.tmpdir</code> system property.  This may be deprecated in the future.  Please use
+   * or <code>java.io.tmpdir</code> system property.
-   * {@link SimpleFSLockFactory#LOCK_DIR} instead.
+
+   * @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
+   * 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, boolean, LockFactory)}).
    */
-  public static final String LOCK_DIR = SimpleFSLockFactory.LOCK_DIR;
+  public static final String LOCK_DIR = System.getProperty("org.apache.lucene.lockDir",
+                                                           System.getProperty("java.io.tmpdir"));
 
   /** The default class which implements filesystem-based directories. */
   private static Class IMPL;
@@ -250,6 +259,8 @@ public class FSDirectory extends Directory {
     // system property org.apache.lucene.store.FSDirectoryLockFactoryClass is set,
     // instantiate that; else, use SimpleFSLockFactory:
 
+    boolean doClearLockID = false;
+
     if (lockFactory == null) {
 
       if (disableLocks) {
@@ -258,7 +269,7 @@ public class FSDirectory extends Directory {
       } else {
         String lockClassName = System.getProperty("org.apache.lucene.store.FSDirectoryLockFactoryClass");
 
-        if (lockClassName != null) {
+        if (lockClassName != null && !lockClassName.equals("")) {
           Class c;
 
           try {
@@ -277,14 +288,10 @@ public class FSDirectory extends Directory {
             throw new IOException("unable to cast LockClass " + lockClassName + " instance to a LockFactory");
           }
         } else {
-          // Our default lock is SimpleFSLockFactory:
+          // Our default lock is SimpleFSLockFactory;
-          File lockDir;
+          // default lockDir is our index directory:
-          if (LOCK_DIR == null) {
+          lockFactory = new SimpleFSLockFactory(path);
-            lockDir = directory;
+          doClearLockID = true;
-          } else {
-            lockDir = new File(LOCK_DIR);
-          }
-          lockFactory = new SimpleFSLockFactory(lockDir);
         }
       }
     }
@@ -297,6 +304,11 @@ public class FSDirectory extends Directory {
     directory = path;
 
     setLockFactory(lockFactory);
+    if (doClearLockID) {
+      // Clear the prefix because write.lock will be
+      // stored in our directory:
+      lockFactory.setLockPrefix(null);
+    }
 
     init(path, create, doRemoveOldFiles);
   }
@@ -320,7 +332,15 @@ public class FSDirectory extends Directory {
       }
     }
 
+    if (lockFactory.getLockPrefix() != null) {
       lockFactory.clearAllLocks();
+    } else {
+      // Lock file is stored in the index, so we just remove
+      // it ourselves here:
+      File lockFile = new File(directory, IndexWriter.WRITE_LOCK_NAME);
+      if (lockFile.exists() && !lockFile.delete())
+        throw new IOException("Cannot delete " + lockFile);
+    }
   }
 
   /** Returns an array of strings, one for each Lucene index file in the directory. */

src/java/org/apache/lucene/store/LockFactory.java

@@ -60,5 +60,5 @@ public abstract class LockFactory {
    * are certain the lock files are not in use. {@link FSDirectory}
    * calls this when creating a new index.
    */
-  public abstract void clearAllLocks() throws IOException;
+  abstract void clearAllLocks() throws IOException;
 }

src/java/org/apache/lucene/store/NativeFSLockFactory.java

@@ -67,10 +67,6 @@ public class NativeFSLockFactory extends LockFactory {
    * system property is used.
    */
 
-  public static final String LOCK_DIR =
-    System.getProperty("org.apache.lucene.lockDir",
-                       System.getProperty("java.io.tmpdir"));
-
   private File lockDir;
 
   // Simple test to verify locking system is "working".  On
@@ -94,17 +90,6 @@ public class NativeFSLockFactory extends LockFactory {
     l.release();
   }
 
-  /**
-   * Create a NativeFSLockFactory instance, storing lock
-   * files into the default LOCK_DIR:
-   * <code>org.apache.lucene.lockDir</code> system property,
-   * or (if that is null) then the
-   * <code>java.io.tmpdir</code> system property.
-   */
-  public NativeFSLockFactory() throws IOException {
-    this(new File(LOCK_DIR));
-  }
-
   /**
    * Create a NativeFSLockFactory instance, storing lock
    * files into the specified lockDirName:
@@ -139,22 +124,17 @@ public class NativeFSLockFactory extends LockFactory {
   }
 
   public synchronized Lock makeLock(String lockName) {
-    String fullName;
+    if (lockPrefix != null)
-    if (lockPrefix.equals("")) {
+      lockName = lockPrefix + "-n-" + lockName;
-      fullName = lockName;
+    return new NativeFSLock(lockDir, lockName);
-    } else {
-      fullName = lockPrefix + "-n-" + lockName;
   }
 
-    return new NativeFSLock(lockDir, fullName);
+  protected void clearAllLocks() throws IOException {
-  }
-
-  public void clearAllLocks() throws IOException {
     // Note that this isn't strictly required anymore
     // because the existence of these files does not mean
     // they are locked, but, still do this in case people
     // really want to see the files go away:
-    if (lockDir.exists()) {
+    if (lockDir.exists() && lockPrefix != null) {
         String[] files = lockDir.list();
         if (files == null)
           throw new IOException("Cannot read lock directory " +

src/java/org/apache/lucene/store/SimpleFSLockFactory.java

@@ -38,21 +38,8 @@ public class SimpleFSLockFactory extends LockFactory {
    * system property is used.
    */
 
-  public static final String LOCK_DIR =
-    System.getProperty("org.apache.lucene.lockDir",
-                       System.getProperty("java.io.tmpdir"));
-
   private File lockDir;
 
-  /**
-   * Instantiate using default LOCK_DIR: <code>org.apache.lucene.lockDir</code>
-   * system property, or (if that is null) then <code>java.io.tmpdir</code>.
-   */
-  public SimpleFSLockFactory() throws IOException {
-    lockDir = new File(LOCK_DIR);
-    init(lockDir);
-  }
-
   /**
    * Instantiate using the provided directory (as a File instance).
    * @param lockDir where lock files should be created.
@@ -71,17 +58,18 @@ public class SimpleFSLockFactory extends LockFactory {
   }
 
   protected void init(File lockDir) throws IOException {
-
     this.lockDir = lockDir;
-
   }
 
   public Lock makeLock(String lockName) {
-    return new SimpleFSLock(lockDir, lockPrefix + "-" + lockName);
+    if (lockPrefix != null) {
+      lockName = lockPrefix + "-" + lockName;
+    }
+    return new SimpleFSLock(lockDir, lockName);
   }
 
-  public void clearAllLocks() throws IOException {
+  protected void clearAllLocks() throws IOException {
-    if (lockDir.exists()) {
+    if (lockDir.exists() && lockPrefix != null) {
         String[] files = lockDir.list();
         if (files == null)
           throw new IOException("Cannot read lock directory " +

src/test/org/apache/lucene/store/TestLockFactory.java

@@ -210,8 +210,8 @@ public class TestLockFactory extends TestCase {
                    NoLockFactory.class.isInstance(writer.getDirectory().getLockFactory()));
 
         // Put back to the correct default for subsequent tests:
-        System.setProperty("org.apache.lucene.store.FSDirectoryLockFactoryClass",
+        // System.clearProperty("org.apache.lucene.store.FSDirectoryLockFactoryClass");
-                           "org.apache.lucene.store.SimpleFSLockFactory");
+        System.setProperty("org.apache.lucene.store.FSDirectoryLockFactoryClass", "");
 
         writer.close();
         // Cleanup
@@ -292,7 +292,7 @@ public class TestLockFactory extends TestCase {
     // no unexpected exceptions are raised, but use
     // NativeFSLockFactory:
     public void testStressLocksNativeFSLockFactory() throws IOException {
-      _testStressLocks(new NativeFSLockFactory(), "index.TestLockFactory7");
+      _testStressLocks(new NativeFSLockFactory("index.TestLockFactory7"), "index.TestLockFactory7");
     }
 
     public void _testStressLocks(LockFactory lockFactory, String indexDirName) throws IOException {
@@ -325,9 +325,9 @@ public class TestLockFactory extends TestCase {
     // Verify: NativeFSLockFactory works correctly
     public void testNativeFSLockFactory() throws IOException {
 
-      NativeFSLockFactory f = new NativeFSLockFactory();
+      NativeFSLockFactory f = new NativeFSLockFactory(System.getProperty("tempDir"));
 
-      NativeFSLockFactory f2 = new NativeFSLockFactory();
+      NativeFSLockFactory f2 = new NativeFSLockFactory(System.getProperty("tempDir"));
 
       f.setLockPrefix("test");
       Lock l = f.makeLock("commit");
@@ -350,8 +350,8 @@ public class TestLockFactory extends TestCase {
     public void testNativeFSLockFactoryPrefix() throws IOException {
 
       // Make sure we get identical instances:
-      Directory dir1 = FSDirectory.getDirectory("TestLockFactory.8", true, new NativeFSLockFactory());
+      Directory dir1 = FSDirectory.getDirectory("TestLockFactory.8", true, new NativeFSLockFactory("TestLockFactory.8"));
-      Directory dir2 = FSDirectory.getDirectory("TestLockFactory.9", true, new NativeFSLockFactory());
+      Directory dir2 = FSDirectory.getDirectory("TestLockFactory.9", true, new NativeFSLockFactory("TestLockFactory.9"));
 
       String prefix1 = dir1.getLockFactory().getLockPrefix();
       String prefix2 = dir2.getLockFactory().getLockPrefix();
@@ -362,20 +362,18 @@ public class TestLockFactory extends TestCase {
       rmDir("TestLockFactory.9");
     }
 
-    // Verify: default LockFactory assigns different lock prefixes:
+    // Verify: default LockFactory has no prefix (ie
+    // write.lock is stored in index):
     public void testDefaultFSLockFactoryPrefix() throws IOException {
 
       // Make sure we get identical instances:
-      Directory dir1 = FSDirectory.getDirectory("TestLockFactory.10", true);
+      Directory dir = FSDirectory.getDirectory("TestLockFactory.10", true);
-      Directory dir2 = FSDirectory.getDirectory("TestLockFactory.11", true);
 
-      String prefix1 = dir1.getLockFactory().getLockPrefix();
+      String prefix = dir.getLockFactory().getLockPrefix();
-      String prefix2 = dir2.getLockFactory().getLockPrefix();
+
+      assertTrue("Default lock prefix should be null", null == prefix);
 
-      assertTrue("Default Lock Factories are incorrectly shared: dir1 and dir2 have same lock prefix '" + prefix1 + "'; they should be different",
-                 !prefix1.equals(prefix2));
       rmDir("TestLockFactory.10");
-      rmDir("TestLockFactory.11");
     }
 
     private class WriterThread extends Thread {