LUCENE-6060: remove IndexWriter.unlock

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

lucene/CHANGES.txt

@@ -246,6 +246,9 @@ Bug Fixes
 
 * LUCENE-6055: PayloadAttribute.clone() now does a deep clone of the underlying
   bytes. (Shai Erera)
+
+* LUCENE-6060: Remove dangerous IndexWriter.unlock method (Simon
+  Willnauer, Mike McCandless)
   
 Documentation
 

lucene/core/src/java/org/apache/lucene/index/IndexWriter.java

@@ -32,8 +32,8 @@ import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
-import java.util.Map.Entry;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Queue;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -4352,17 +4352,6 @@ public class IndexWriter implements Closeable, TwoPhaseCommit, Accountable {
     return directory.makeLock(WRITE_LOCK_NAME).isLocked();
   }
 
-  /**
-   * Forcibly unlocks the index in the named directory.
-   * <P>
-   * Caution: this should only be used by failure recovery code,
-   * when it is known that no other process nor thread is in fact
-   * currently accessing this index.
-   */
-  public static void unlock(Directory directory) throws IOException {
-    directory.makeLock(IndexWriter.WRITE_LOCK_NAME).close();
-  }
-
   /** If {@link DirectoryReader#open(IndexWriter,boolean)} has
    *  been called (ie, this writer is in near real-time
    *  mode), then after a merge completes, this class can be

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

@@ -37,12 +37,10 @@ import java.nio.file.Path;
 
  * <p>When this happens, a {@link LockObtainFailedException}
  * is hit when trying to create a writer, in which case you
- * need to explicitly clear the lock file first.  You can
+ * need to explicitly clear the lock file first by
- * either manually remove the file, or use the {@link
+ * manually removing the file.  But, first be certain that
- * org.apache.lucene.index.IndexWriter#unlock(Directory)}
+ * no writer is in fact writing to the index otherwise you
- * API.  But, first be certain that no writer is in fact
+ * can easily corrupt your index.</p>
- * writing to the index otherwise you can easily corrupt
- * your index.</p>
  *
  * <p>Special care needs to be taken if you change the locking
  * implementation: First be certain that no writer is in fact

lucene/core/src/test/org/apache/lucene/index/TestDirectoryReaderReopen.java

@@ -16,6 +16,7 @@ package org.apache.lucene.index;
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -28,7 +29,6 @@ import java.util.Random;
 import java.util.Set;
 
 import org.apache.lucene.analysis.MockAnalyzer;
-import org.apache.lucene.codecs.Codec;
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.Field;
 import org.apache.lucene.document.FieldType;
@@ -39,12 +39,14 @@ import org.apache.lucene.search.IndexSearcher;
 import org.apache.lucene.search.ScoreDoc;
 import org.apache.lucene.search.TermQuery;
 import org.apache.lucene.store.Directory;
-import org.apache.lucene.store.MockDirectoryWrapper.FakeIOException;
 import org.apache.lucene.store.MockDirectoryWrapper;
+import org.apache.lucene.store.MockDirectoryWrapper.FakeIOException;
 import org.apache.lucene.store.RAMDirectory;
 import org.apache.lucene.util.LuceneTestCase;
 import org.apache.lucene.util.TestUtil;
 
+
+
 public class TestDirectoryReaderReopen extends LuceneTestCase {
   
   public void testReopen() throws Exception {
@@ -432,7 +434,6 @@ public class TestDirectoryReaderReopen extends LuceneTestCase {
   }
   
   public static void createIndex(Random random, Directory dir, boolean multiSegment) throws IOException {
-    IndexWriter.unlock(dir);
     IndexWriter w = new IndexWriter(dir, LuceneTestCase.newIndexWriterConfig(random, new MockAnalyzer(random))
         .setMergePolicy(new LogDocMergePolicy()));
     

lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyWriter.java

@@ -23,18 +23,18 @@ import org.apache.lucene.facet.FacetsConfig;
 import org.apache.lucene.facet.taxonomy.FacetLabel;
 import org.apache.lucene.facet.taxonomy.TaxonomyReader;
 import org.apache.lucene.facet.taxonomy.TaxonomyWriter;
-import org.apache.lucene.facet.taxonomy.writercache.TaxonomyWriterCache;
 import org.apache.lucene.facet.taxonomy.writercache.Cl2oTaxonomyWriterCache;
 import org.apache.lucene.facet.taxonomy.writercache.LruTaxonomyWriterCache;
-import org.apache.lucene.index.LeafReader;
+import org.apache.lucene.facet.taxonomy.writercache.TaxonomyWriterCache;
-import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.CorruptIndexException; // javadocs
 import org.apache.lucene.index.DirectoryReader;
 import org.apache.lucene.index.DocsEnum;
 import org.apache.lucene.index.IndexReader;
 import org.apache.lucene.index.IndexWriter;
-import org.apache.lucene.index.IndexWriterConfig.OpenMode;
 import org.apache.lucene.index.IndexWriterConfig;
+import org.apache.lucene.index.IndexWriterConfig.OpenMode;
+import org.apache.lucene.index.LeafReader;
+import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.LogByteSizeMergePolicy;
 import org.apache.lucene.index.ReaderManager;
 import org.apache.lucene.index.SegmentInfos;
@@ -44,8 +44,6 @@ import org.apache.lucene.index.TieredMergePolicy;
 import org.apache.lucene.store.AlreadyClosedException;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.LockObtainFailedException; // javadocs
-import org.apache.lucene.store.NativeFSLockFactory;
-import org.apache.lucene.store.SimpleFSLockFactory;
 import org.apache.lucene.util.BytesRef;
 
 /*
@@ -135,22 +133,6 @@ public class DirectoryTaxonomyWriter implements TaxonomyWriter {
     return infos.getUserData();
   }
   
-  /**
-   * Forcibly unlocks the taxonomy in the named directory.
-   * <P>
-   * Caution: this should only be used by failure recovery code, when it is
-   * known that no other process nor thread is in fact currently accessing
-   * this taxonomy.
-   * <P>
-   * This method is unnecessary if your {@link Directory} uses a
-   * {@link NativeFSLockFactory} instead of the default
-   * {@link SimpleFSLockFactory}. When the "native" lock is used, a lock
-   * does not stay behind forever when the process using it dies. 
-   */
-  public static void unlock(Directory directory) throws IOException {
-    IndexWriter.unlock(directory);
-  }
-
   /**
    * Construct a Taxonomy writer.
    * 
@@ -173,10 +155,7 @@ public class DirectoryTaxonomyWriter implements TaxonomyWriter {
    * @throws CorruptIndexException
    *     if the taxonomy is corrupted.
    * @throws LockObtainFailedException
-   *     if the taxonomy is locked by another writer. If it is known
+   *     if the taxonomy is locked by another writer.
-   *     that no other concurrent writer is active, the lock might
-   *     have been left around by an old dead process, and should be
-   *     removed using {@link #unlock(Directory)}.
    * @throws IOException
    *     if another error occurred.
    */

lucene/facet/src/test/org/apache/lucene/facet/taxonomy/TestTaxonomyCombined.java

@@ -12,8 +12,6 @@ import org.apache.lucene.facet.SlowRAMDirectory;
 import org.apache.lucene.facet.taxonomy.directory.DirectoryTaxonomyReader;
 import org.apache.lucene.facet.taxonomy.directory.DirectoryTaxonomyWriter;
 import org.apache.lucene.store.Directory;
-import org.apache.lucene.store.LockObtainFailedException;
-import org.apache.lucene.store.RAMDirectory;
 import org.apache.lucene.util.LuceneTestCase.SuppressCodecs;
 import org.junit.Test;
 
@@ -915,47 +913,6 @@ public class TestTaxonomyCombined extends FacetTestCase {
     indexDir.close();
   }
   
-  /**
-   * Test what happens if we try to write to a locked taxonomy writer,
-   * and see that we can unlock it and continue.
-   */
-  @Test
-  public void testWriterLock() throws Exception {
-    // native fslock impl gets angry if we use it, so use RAMDirectory explicitly.
-    Directory indexDir = new RAMDirectory();
-    TaxonomyWriter tw = new DirectoryTaxonomyWriter(indexDir);
-    tw.addCategory(new FacetLabel("hi", "there"));
-    tw.commit();
-    // we deliberately not close the write now, and keep it open and
-    // locked.
-    // Verify that the writer worked:
-    TaxonomyReader tr = new DirectoryTaxonomyReader(indexDir);
-    assertEquals(2, tr.getOrdinal(new FacetLabel("hi", "there")));
-    // Try to open a second writer, with the first one locking the directory.
-    // We expect to get a LockObtainFailedException.
-    try {
-      assertNull(new DirectoryTaxonomyWriter(indexDir));
-      fail("should have failed to write in locked directory");
-    } catch (LockObtainFailedException e) {
-      // this is what we expect to happen.
-    }
-    // Remove the lock, and now the open should succeed, and we can
-    // write to the new writer.
-    DirectoryTaxonomyWriter.unlock(indexDir);
-    TaxonomyWriter tw2 = new DirectoryTaxonomyWriter(indexDir);
-    tw2.addCategory(new FacetLabel("hey"));
-    tw2.close();
-    // See that the writer indeed wrote:
-    TaxonomyReader newtr = TaxonomyReader.openIfChanged(tr);
-    assertNotNull(newtr);
-    tr.close();
-    tr = newtr;
-    assertEquals(3, tr.getOrdinal(new FacetLabel("hey")));
-    tr.close();
-    tw.close();
-    indexDir.close();
-  }
-  
   /**
    * fillTaxonomyCheckPaths adds the categories in the categories[] array,
    * and asserts that the additions return exactly paths specified in

solr/core/src/java/org/apache/solr/core/SolrCore.java

@@ -17,6 +17,43 @@
 
 package org.apache.solr.core;
 
+import java.io.Closeable;
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.Writer;
+import java.lang.reflect.Constructor;
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.NoSuchFileException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.IdentityHashMap;
+import java.util.LinkedHashMap;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
+import java.util.StringTokenizer;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.locks.ReentrantLock;
+
 import org.apache.commons.io.FileUtils;
 import org.apache.lucene.codecs.Codec;
 import org.apache.lucene.index.DirectoryReader;
@@ -104,43 +141,6 @@ import org.xml.sax.SAXException;
 
 import javax.xml.parsers.ParserConfigurationException;
 
-import java.io.Closeable;
-import java.io.File;
-import java.io.FileNotFoundException;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
-import java.io.Writer;
-import java.lang.reflect.Constructor;
-import java.net.URL;
-import java.nio.charset.StandardCharsets;
-import java.nio.file.NoSuchFileException;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.IdentityHashMap;
-import java.util.LinkedHashMap;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Locale;
-import java.util.Map;
-import java.util.Properties;
-import java.util.Set;
-import java.util.StringTokenizer;
-import java.util.concurrent.Callable;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
-import java.util.concurrent.atomic.AtomicInteger;
-import java.util.concurrent.atomic.AtomicLong;
-import java.util.concurrent.locks.ReentrantLock;
-
 /**
  *
  */
@@ -507,7 +507,7 @@ public final class SolrCore implements SolrInfoMBean, Closeable {
                   logid
                       + "WARNING: Solr index directory '{}' is locked.  Unlocking...",
                   indexDir);
-              IndexWriter.unlock(dir);
+              dir.makeLock(IndexWriter.WRITE_LOCK_NAME).close();              
             } else {
               log.error(logid
                   + "Solr index directory '{}' is locked.  Throwing exception",