You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by rm...@apache.org on 2015/05/30 06:34:18 UTC
svn commit: r1682574 - in /lucene/dev/branches/lucene6508/lucene:
core/src/java/org/apache/lucene/index/ core/src/java/org/apache/lucene/store/
core/src/test/org/apache/lucene/index/
core/src/test/org/apache/lucene/store/ facet/src/java/org/apache/luce...
Author: rmuir
Date: Sat May 30 04:34:17 2015
New Revision: 1682574
URL: http://svn.apache.org/r1682574
Log:
LUCENE-6508: get tests passing
Modified:
lucene/dev/branches/lucene6508/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
lucene/dev/branches/lucene6508/lucene/core/src/java/org/apache/lucene/store/SimpleFSLockFactory.java
lucene/dev/branches/lucene6508/lucene/core/src/java/org/apache/lucene/store/SingleInstanceLockFactory.java
lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/index/TestAddIndexes.java
lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/index/TestCheckIndex.java
lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java
lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/store/TestDirectory.java
lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/store/TestLock.java
lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/store/TestLockFactory.java
lucene/dev/branches/lucene6508/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyWriter.java
lucene/dev/branches/lucene6508/lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java
Modified: lucene/dev/branches/lucene6508/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene6508/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java?rev=1682574&r1=1682573&r2=1682574&view=diff
==============================================================================
--- lucene/dev/branches/lucene6508/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java (original)
+++ lucene/dev/branches/lucene6508/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java Sat May 30 04:34:17 2015
@@ -252,8 +252,8 @@ public class IndexWriter implements Clos
// when unrecoverable disaster strikes, we populate this with the reason that we had to close IndexWriter
volatile Throwable tragedy;
- private final Directory directory; // where this index resides
- private final Directory mergeDirectory; // used for merging
+ private final Directory directory; // where this index resides
+ private final Directory mergeDirectory; // wrapped with throttling: used for merging
private final Analyzer analyzer; // how to analyze text
private final AtomicLong changeCount = new AtomicLong(); // increments every time a change is completed
@@ -754,8 +754,9 @@ public class IndexWriter implements Clos
config = conf;
writeLock = d.obtainLock(WRITE_LOCK_NAME);
-
- directory = new LockValidatingDirectoryWrapper(d, writeLock);
+ directory = d;
+ // nocommit: turn on carefully after we get other stuff going
+ // validatingDirectory = new LockValidatingDirectoryWrapper(d, writeLock);
// Directory we use for merging, so we can abort running merges, and so
// merge schedulers can optionally rate-limit per-merge IO:
@@ -1031,6 +1032,7 @@ public class IndexWriter implements Clos
/** Returns the Directory used by this index. */
public Directory getDirectory() {
+ // return the original directory the user supplied, unwrapped.
return directory;
}
Modified: lucene/dev/branches/lucene6508/lucene/core/src/java/org/apache/lucene/store/SimpleFSLockFactory.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene6508/lucene/core/src/java/org/apache/lucene/store/SimpleFSLockFactory.java?rev=1682574&r1=1682573&r2=1682574&view=diff
==============================================================================
--- lucene/dev/branches/lucene6508/lucene/core/src/java/org/apache/lucene/store/SimpleFSLockFactory.java (original)
+++ lucene/dev/branches/lucene6508/lucene/core/src/java/org/apache/lucene/store/SimpleFSLockFactory.java Sat May 30 04:34:17 2015
@@ -17,7 +17,6 @@ package org.apache.lucene.store;
* limitations under the License.
*/
-import java.io.File;
import java.io.IOException;
import java.nio.file.AccessDeniedException;
import java.nio.file.FileAlreadyExistsException;
@@ -30,16 +29,11 @@ import java.nio.file.attribute.FileTime;
* <p>Implements {@link LockFactory} using {@link
* Files#createFile}.</p>
*
- * <p><b>NOTE:</b> the {@linkplain File#createNewFile() javadocs
- * for <code>File.createNewFile()</code>} contain a vague
- * yet spooky warning about not using the API for file
- * locking. This warning was added due to <a target="_top"
- * href="http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4676183">this
- * bug</a>, and in fact the only known problem with using
- * this API for locking is that the Lucene write lock may
- * not be released when the JVM exits abnormally.</p>
-
- * <p>When this happens, an {@link IOException}
+ * <p>The main downside with using this API for locking is
+ * that the Lucene write lock may not be released when
+ * the JVM exits abnormally.</p>
+ *
+ * <p>When this happens, an {@link LockObtainFailedException}
* is hit when trying to create a writer, in which case you may
* need to explicitly clear the lock file first by
* manually removing the file. But, first be certain that
@@ -137,7 +131,7 @@ public final class SimpleFSLockFactory e
throw new AlreadyClosedException("Lock file cannot be safely removed. Manual intervention is recommended.", exc);
}
// we did a best effort check, now try to remove the file. if something goes wrong,
- // we need to make it clear to the user that the directory my still remain locked.
+ // we need to make it clear to the user that the directory may still remain locked.
try {
Files.delete(path);
} catch (Throwable exc) {
Modified: lucene/dev/branches/lucene6508/lucene/core/src/java/org/apache/lucene/store/SingleInstanceLockFactory.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene6508/lucene/core/src/java/org/apache/lucene/store/SingleInstanceLockFactory.java?rev=1682574&r1=1682573&r2=1682574&view=diff
==============================================================================
--- lucene/dev/branches/lucene6508/lucene/core/src/java/org/apache/lucene/store/SingleInstanceLockFactory.java (original)
+++ lucene/dev/branches/lucene6508/lucene/core/src/java/org/apache/lucene/store/SingleInstanceLockFactory.java Sat May 30 04:34:17 2015
@@ -48,14 +48,14 @@ public final class SingleInstanceLockFac
private class SingleInstanceLock extends Lock {
private final String lockName;
- private boolean closed;
+ private volatile boolean closed;
public SingleInstanceLock(String lockName) {
this.lockName = lockName;
}
@Override
- public synchronized void ensureValid() throws IOException {
+ public void ensureValid() throws IOException {
if (closed) {
throw new AlreadyClosedException("Lock instance already released: " + this);
}
Modified: lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/index/TestAddIndexes.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/index/TestAddIndexes.java?rev=1682574&r1=1682573&r2=1682574&view=diff
==============================================================================
--- lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/index/TestAddIndexes.java (original)
+++ lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/index/TestAddIndexes.java Sat May 30 04:34:17 2015
@@ -40,6 +40,7 @@ import org.apache.lucene.search.PhraseQu
import org.apache.lucene.store.AlreadyClosedException;
import org.apache.lucene.store.BaseDirectoryWrapper;
import org.apache.lucene.store.Directory;
+import org.apache.lucene.store.LockObtainFailedException;
import org.apache.lucene.store.MockDirectoryWrapper;
import org.apache.lucene.store.RAMDirectory;
import org.apache.lucene.util.IOUtils;
@@ -305,7 +306,7 @@ public class TestAddIndexes extends Luce
writer.addIndexes(aux, dir);
assertTrue(false);
}
- catch (IOException e) {
+ catch (IllegalArgumentException e) {
assertEquals(100, writer.maxDoc());
}
writer.close();
@@ -1272,7 +1273,7 @@ public class TestAddIndexes extends Luce
try {
w2.addIndexes(src);
fail("did not hit expected exception");
- } catch (IOException lofe) {
+ } catch (LockObtainFailedException lofe) {
// expected
}
Modified: lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/index/TestCheckIndex.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/index/TestCheckIndex.java?rev=1682574&r1=1682573&r2=1682574&view=diff
==============================================================================
--- lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/index/TestCheckIndex.java (original)
+++ lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/index/TestCheckIndex.java Sat May 30 04:34:17 2015
@@ -27,6 +27,7 @@ import org.apache.lucene.util.IOUtils;
import org.apache.lucene.util.LineFileDocs;
import org.apache.lucene.util.LuceneTestCase;
import org.apache.lucene.store.Directory;
+import org.apache.lucene.store.LockObtainFailedException;
import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.analysis.CannedTokenStream;
import org.apache.lucene.analysis.MockAnalyzer;
@@ -177,7 +178,7 @@ public class TestCheckIndex extends Luce
try {
new CheckIndex(dir);
fail("should not have obtained write lock");
- } catch (IOException expected) {
+ } catch (LockObtainFailedException expected) {
// ok
}
Modified: lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java?rev=1682574&r1=1682573&r2=1682574&view=diff
==============================================================================
--- lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java (original)
+++ lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java Sat May 30 04:34:17 2015
@@ -67,6 +67,7 @@ import org.apache.lucene.store.BaseDirec
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.IOContext;
import org.apache.lucene.store.IndexOutput;
+import org.apache.lucene.store.LockObtainFailedException;
import org.apache.lucene.store.MockDirectoryWrapper;
import org.apache.lucene.store.NoLockFactory;
import org.apache.lucene.store.RAMDirectory;
@@ -1719,7 +1720,7 @@ public class TestIndexWriter extends Luc
try {
new RandomIndexWriter(random(), d, newIndexWriterConfig(null));
fail("should not be able to create another writer");
- } catch (IOException lofe) {
+ } catch (LockObtainFailedException lofe) {
// expected
}
w1.close();
Modified: lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/store/TestDirectory.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/store/TestDirectory.java?rev=1682574&r1=1682573&r2=1682574&view=diff
==============================================================================
--- lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/store/TestDirectory.java (original)
+++ lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/store/TestDirectory.java Sat May 30 04:34:17 2015
@@ -92,7 +92,7 @@ public class TestDirectory extends Lucen
try {
other.obtainLock(lockname);
fail("didnt get exception");
- } catch (IOException e) {
+ } catch (LockObtainFailedException e) {
// OK
}
}
Modified: lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/store/TestLock.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/store/TestLock.java?rev=1682574&r1=1682573&r2=1682574&view=diff
==============================================================================
--- lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/store/TestLock.java (original)
+++ lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/store/TestLock.java Sat May 30 04:34:17 2015
@@ -28,36 +28,6 @@ import org.apache.lucene.util.LuceneTest
public class TestLock extends LuceneTestCase {
- public void testObtain() {
- LockMock lock = new LockMock();
- Lock.LOCK_POLL_INTERVAL = 10;
-
- try {
- lock.obtain(Lock.LOCK_POLL_INTERVAL);
- fail("Should have failed to obtain lock");
- } catch (IOException e) {
- assertEquals("should attempt to lock more than once", lock.lockAttempts, 2);
- }
- }
-
- private class LockMock extends Lock {
- public int lockAttempts;
-
- @Override
- public boolean obtain() {
- lockAttempts++;
- return false;
- }
- @Override
- public void close() {
- // do nothing
- }
- @Override
- public boolean isLocked() {
- return false;
- }
- }
-
public void testObtainConcurrently() throws InterruptedException, IOException {
final Directory directory;
if (random().nextBoolean()) {
@@ -70,7 +40,7 @@ public class TestLock extends LuceneTest
final AtomicInteger atomicCounter = new AtomicInteger(0);
final ReentrantLock assertingLock = new ReentrantLock();
int numThreads = 2 + random().nextInt(10);
- final int runs = 500 + random().nextInt(1000);
+ final int runs = atLeast(10000);
CyclicBarrier barrier = new CyclicBarrier(numThreads);
Thread[] threads = new Thread[numThreads];
for (int i = 0; i < threads.length; i++) {
@@ -83,16 +53,14 @@ public class TestLock extends LuceneTest
throw new RuntimeException(e);
}
while (running.get()) {
- try (Lock lock = directory.makeLock("foo.lock")) {
- if (lock.isLocked() == false && lock.obtain()) {
- assertTrue(lock.isLocked());
- assertFalse(assertingLock.isLocked());
- if (assertingLock.tryLock()) {
- assertingLock.unlock();
- } else {
- fail();
- }
+ try (Lock lock = directory.obtainLock("foo.lock")) {
+ assertFalse(assertingLock.isLocked());
+ if (assertingLock.tryLock()) {
+ assertingLock.unlock();
+ } else {
+ fail();
}
+ assert lock != null; // stupid compiler
} catch (IOException ex) {
//
}
Modified: lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/store/TestLockFactory.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/store/TestLockFactory.java?rev=1682574&r1=1682573&r2=1682574&view=diff
==============================================================================
--- lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/store/TestLockFactory.java (original)
+++ lucene/dev/branches/lucene6508/lucene/core/src/test/org/apache/lucene/store/TestLockFactory.java Sat May 30 04:34:17 2015
@@ -58,15 +58,6 @@ public class TestLockFactory extends Luc
// Both write lock and commit lock should have been created:
assertEquals("# of unique locks created (after instantiating IndexWriter)",
1, lf.locksCreated.size());
- assertTrue("# calls to makeLock is 0 (after instantiating IndexWriter)",
- lf.makeLockCount >= 1);
-
- for(final String lockName : lf.locksCreated.keySet()) {
- MockLockFactory.MockLock lock = (MockLockFactory.MockLock) lf.locksCreated.get(lockName);
- assertTrue("# calls to Lock.obtain is 0 (after instantiating IndexWriter)",
- lock.lockAttempts > 0);
- }
-
writer.close();
}
@@ -75,7 +66,6 @@ public class TestLockFactory extends Luc
// Verify: NoLockFactory allows two IndexWriters
public void testRAMDirectoryNoLocking() throws IOException {
MockDirectoryWrapper dir = new MockDirectoryWrapper(random(), new RAMDirectory(NoLockFactory.INSTANCE));
- dir.setAssertLocks(false); // we are gonna explicitly test we get this back
IndexWriter writer = new IndexWriter(dir, new IndexWriterConfig(new MockAnalyzer(random())));
writer.commit(); // required so the second open succeed
@@ -166,23 +156,18 @@ public class TestLockFactory extends Luc
public void testNativeFSLockFactory() throws IOException {
Directory dir = FSDirectory.open(createTempDir(LuceneTestCase.getTestClass().getSimpleName()), NativeFSLockFactory.INSTANCE);
- Lock l = dir.makeLock("commit");
- Lock l2 = dir.makeLock("commit");
-
- assertTrue("failed to obtain lock", l.obtain());
- assertTrue("succeeded in obtaining lock twice", !l2.obtain());
+ Lock l = dir.obtainLock("commit");
+ l.ensureValid();
+ try {
+ dir.obtainLock("commit");
+ fail("succeeded in obtaining lock twice, didn't get exception");
+ } catch (LockObtainFailedException expected) {}
l.close();
- assertTrue("failed to obtain 2nd lock after first one was freed", l2.obtain());
- l2.close();
-
- // Make sure we can obtain first one again, test isLocked():
- assertTrue("failed to obtain lock", l.obtain());
- assertTrue(l.isLocked());
- assertTrue(l2.isLocked());
+ // Make sure we can obtain first one again:
+ l = dir.obtainLock("commit");
+ l.ensureValid();
l.close();
- assertFalse(l.isLocked());
- assertFalse(l2.isLocked());
}
@@ -193,10 +178,8 @@ public class TestLockFactory extends Luc
Files.createFile(lockFile);
Directory dir = FSDirectory.open(tempDir, NativeFSLockFactory.INSTANCE);
- Lock l = dir.makeLock("test.lock");
- assertTrue("failed to obtain lock", l.obtain());
+ Lock l = dir.obtainLock("test.lock");
l.close();
- assertFalse("failed to release lock", l.isLocked());
Files.deleteIfExists(lockFile);
}
@@ -303,32 +286,26 @@ public class TestLockFactory extends Luc
class MockLockFactory extends LockFactory {
public Map<String,Lock> locksCreated = Collections.synchronizedMap(new HashMap<String,Lock>());
- public int makeLockCount = 0;
@Override
- public synchronized Lock makeLock(Directory dir, String lockName) {
+ public synchronized Lock obtainLock(Directory dir, String lockName) {
Lock lock = new MockLock();
locksCreated.put(lockName, lock);
- makeLockCount++;
return lock;
}
public class MockLock extends Lock {
- public int lockAttempts;
@Override
- public boolean obtain() {
- lockAttempts++;
- return true;
- }
- @Override
public void close() {
// do nothing
}
+
@Override
- public boolean isLocked() {
- return false;
+ public void ensureValid() throws IOException {
+ // do nothing
}
+
}
}
Modified: lucene/dev/branches/lucene6508/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyWriter.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene6508/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyWriter.java?rev=1682574&r1=1682573&r2=1682574&view=diff
==============================================================================
--- lucene/dev/branches/lucene6508/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyWriter.java (original)
+++ lucene/dev/branches/lucene6508/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyWriter.java Sat May 30 04:34:17 2015
@@ -43,6 +43,7 @@ import org.apache.lucene.index.TermsEnum
import org.apache.lucene.index.TieredMergePolicy;
import org.apache.lucene.store.AlreadyClosedException;
import org.apache.lucene.store.Directory;
+import org.apache.lucene.store.LockObtainFailedException;
import org.apache.lucene.util.BytesRef;
/*
@@ -153,6 +154,8 @@ public class DirectoryTaxonomyWriter imp
* If null or missing, {@link #defaultTaxonomyWriterCache()} is used.
* @throws CorruptIndexException
* if the taxonomy is corrupted.
+ * @throws LockObtainFailedException
+ * if the taxonomy is locked by another writer.
* @throws IOException
* if another error occurred.
*/
Modified: lucene/dev/branches/lucene6508/lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene6508/lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java?rev=1682574&r1=1682573&r2=1682574&view=diff
==============================================================================
--- lucene/dev/branches/lucene6508/lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java (original)
+++ lucene/dev/branches/lucene6508/lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java Sat May 30 04:34:17 2015
@@ -76,7 +76,6 @@ public class MockDirectoryWrapper extend
boolean assertNoDeleteOpenFile = false;
boolean preventDoubleWrite = true;
boolean trackDiskUsage = false;
- boolean wrapLocking = true;
boolean useSlowOpenClosers = LuceneTestCase.TEST_NIGHTLY;
boolean enableVirusScanner = true;
boolean allowRandomFileNotFoundException = true;
@@ -701,19 +700,6 @@ public class MockDirectoryWrapper extend
public void setAssertNoUnrefencedFilesOnClose(boolean v) {
assertNoUnreferencedFilesOnClose = v;
}
-
- /**
- * Set to false if you want to return the pure {@link LockFactory} and not
- * wrap all lock with {@code AssertingLock}.
- * <p>
- * Be careful if you turn this off: {@code MockDirectoryWrapper} might
- * no longer be able to detect if you forget to close an {@link IndexWriter},
- * and spit out horribly scary confusing exceptions instead of
- * simply telling you that.
- */
- public void setAssertLocks(boolean v) {
- this.wrapLocking = v;
- }
@Override
public synchronized void close() throws IOException {
@@ -996,54 +982,10 @@ public class MockDirectoryWrapper extend
@Override
public synchronized Lock obtainLock(String name) throws IOException {
maybeYield();
- if (wrapLocking) {
- final Lock delegateLock = super.obtainLock(name);
- return new AssertingLock(delegateLock, name);
- } else {
- return super.obtainLock(name);
- }
+ return super.obtainLock(name);
+ // TODO: consider mocking locks, but not all the time, can hide bugs
}
- private final class AssertingLock extends Lock {
- private final Lock delegateLock;
- private final String name;
- private volatile boolean closed;
-
- AssertingLock(Lock delegate, String name) {
- this.delegateLock = delegate;
- this.name = name;
- final RuntimeException exception = openLocks.putIfAbsent(name, new RuntimeException("lock \"" + name + "\" was not released: " + delegateLock));
- if (exception != null && delegateLock != NoLockFactory.SINGLETON_LOCK) {
- throw exception;
- }
- }
-
- @Override
- public void close() throws IOException {
- if (closed) {
- return;
- }
- try {
- RuntimeException remove = openLocks.remove(name);
- // TODO: fix stupid tests like TestIndexWriter.testNoSegmentFile to not do this!
- assert remove != null || delegateLock == NoLockFactory.SINGLETON_LOCK;
- } finally {
- closed = true;
- }
- delegateLock.close();
- }
-
- @Override
- public String toString() {
- return "AssertingLock(" + delegateLock + ")";
- }
-
- @Override
- public void ensureValid() throws IOException {
- // TODO
- }
- }
-
/** Use this when throwing fake {@code IOException},
* e.g. from {@link MockDirectoryWrapper.Failure}. */
public static class FakeIOException extends IOException {