From 80f00226e75823e6ae339bf55bfce057ed6b8bff Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Tue, 15 Apr 2014 14:24:31 +0000 Subject: [PATCH] LUCENE-5067: fix testThreadSafety to actually test the dir in question (not copy into random impl), fix performance issues, remove Nightly git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1587592 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/lucene/store/TestDirectory.java | 6 ++++++ .../lucene/store/TestFileSwitchDirectory.java | 3 +-- .../lucene/store/TestNRTCachingDirectory.java | 8 ++++++-- .../TestRateLimitedDirectoryWrapper.java | 6 ++++++ .../lucene/store/BaseDirectoryTestCase.java | 20 ++++++++++--------- .../apache/lucene/util/LuceneTestCase.java | 4 ++-- 6 files changed, 32 insertions(+), 15 deletions(-) diff --git a/lucene/core/src/test/org/apache/lucene/store/TestDirectory.java b/lucene/core/src/test/org/apache/lucene/store/TestDirectory.java index 92425bf4e3a..cd9362c69cb 100644 --- a/lucene/core/src/test/org/apache/lucene/store/TestDirectory.java +++ b/lucene/core/src/test/org/apache/lucene/store/TestDirectory.java @@ -37,6 +37,12 @@ public class TestDirectory extends BaseDirectoryTestCase { } } + // we wrap the directory in slow stuff, so only run nightly + @Override @Nightly + public void testThreadSafety() throws Exception { + super.testThreadSafety(); + } + // Test that different instances of FSDirectory can coexist on the same // path, can read, write, and lock files. public void testDirectInstantiation() throws Exception { diff --git a/lucene/core/src/test/org/apache/lucene/store/TestFileSwitchDirectory.java b/lucene/core/src/test/org/apache/lucene/store/TestFileSwitchDirectory.java index 8adf91250b2..3b37d26b829 100644 --- a/lucene/core/src/test/org/apache/lucene/store/TestFileSwitchDirectory.java +++ b/lucene/core/src/test/org/apache/lucene/store/TestFileSwitchDirectory.java @@ -97,8 +97,7 @@ public class TestFileSwitchDirectory extends BaseDirectoryTestCase { private Directory newFSSwitchDirectory(File aDir, File bDir, Set primaryExtensions) throws IOException { Directory a = new SimpleFSDirectory(aDir); Directory b = new SimpleFSDirectory(bDir); - FileSwitchDirectory switchDir = new FileSwitchDirectory(primaryExtensions, a, b, true); - return new MockDirectoryWrapper(random(), switchDir); + return new FileSwitchDirectory(primaryExtensions, a, b, true); } // LUCENE-3380 -- make sure we get exception if the directory really does not exist. diff --git a/lucene/core/src/test/org/apache/lucene/store/TestNRTCachingDirectory.java b/lucene/core/src/test/org/apache/lucene/store/TestNRTCachingDirectory.java index 50edf3f9e5e..b56324ab2c6 100644 --- a/lucene/core/src/test/org/apache/lucene/store/TestNRTCachingDirectory.java +++ b/lucene/core/src/test/org/apache/lucene/store/TestNRTCachingDirectory.java @@ -18,6 +18,7 @@ package org.apache.lucene.store; */ import java.io.File; +import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -38,9 +39,12 @@ import org.apache.lucene.util.TestUtil; public class TestNRTCachingDirectory extends BaseDirectoryTestCase { + // TODO: RAMDir used here, because its still too slow to use e.g. SimpleFS + // for the threads tests... maybe because of the synchronization in listAll? + // would be good to investigate further... @Override - protected Directory getDirectory(File path) { - return new NRTCachingDirectory(newFSDirectory(path), + protected Directory getDirectory(File path) throws IOException { + return new NRTCachingDirectory(new RAMDirectory(), .1 + 2.0*random().nextDouble(), .1 + 5.0*random().nextDouble()); } diff --git a/lucene/core/src/test/org/apache/lucene/store/TestRateLimitedDirectoryWrapper.java b/lucene/core/src/test/org/apache/lucene/store/TestRateLimitedDirectoryWrapper.java index d1031537237..238fac673cb 100644 --- a/lucene/core/src/test/org/apache/lucene/store/TestRateLimitedDirectoryWrapper.java +++ b/lucene/core/src/test/org/apache/lucene/store/TestRateLimitedDirectoryWrapper.java @@ -28,4 +28,10 @@ public class TestRateLimitedDirectoryWrapper extends BaseDirectoryTestCase { dir.setRateLimiter(limiter, IOContext.Context.MERGE); return dir; } + + // since we are rate-limiting, this test gets pretty slow + @Override @Nightly + public void testThreadSafety() throws Exception { + super.testThreadSafety(); + } } diff --git a/lucene/test-framework/src/java/org/apache/lucene/store/BaseDirectoryTestCase.java b/lucene/test-framework/src/java/org/apache/lucene/store/BaseDirectoryTestCase.java index 3600a62e23a..81007e29e9f 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/store/BaseDirectoryTestCase.java +++ b/lucene/test-framework/src/java/org/apache/lucene/store/BaseDirectoryTestCase.java @@ -51,13 +51,11 @@ public abstract class BaseDirectoryTestCase extends LuceneTestCase { } } - // test is occasionally very slow, i dont know why - // try this seed: 7D7E036AD12927F5:93333EF9E6DE44DE - @Nightly public void testThreadSafety() throws Exception { - final Directory raw = getDirectory(createTempDir("testThreadSafety")); - final BaseDirectoryWrapper dir = newDirectory(raw); - dir.setCheckIndexOnClose(false); // we arent making an index + final Directory dir = getDirectory(createTempDir("testThreadSafety")); + if (dir instanceof BaseDirectoryWrapper) { + ((BaseDirectoryWrapper)dir).setCheckIndexOnClose(false); // we arent making an index + } if (dir instanceof MockDirectoryWrapper) { ((MockDirectoryWrapper)dir).setThrottling(MockDirectoryWrapper.Throttling.NEVER); // makes this test really slow } @@ -91,6 +89,7 @@ public abstract class BaseDirectoryTestCase extends LuceneTestCase { class TheThread2 extends Thread { private String name; + private volatile boolean stop; public TheThread2(String name) { this.name = name; @@ -98,7 +97,7 @@ public abstract class BaseDirectoryTestCase extends LuceneTestCase { @Override public void run() { - for (int i = 0; i < 10000; i++) { + while (stop == false) { try { String[] files = dir.listAll(); for (String file : files) { @@ -109,7 +108,7 @@ public abstract class BaseDirectoryTestCase extends LuceneTestCase { } catch (FileNotFoundException | NoSuchFileException e) { // ignore } catch (IOException e) { - if (e.getMessage().contains("still open for writing")) { + if (e.getMessage() != null && e.getMessage().contains("still open for writing")) { // ignore } else { throw new RuntimeException(e); @@ -132,10 +131,13 @@ public abstract class BaseDirectoryTestCase extends LuceneTestCase { theThread2.start(); theThread.join(); + + // after first thread is done, no sense in waiting on thread 2 + // to listFiles() and loop over and over + theThread2.stop = true; theThread2.join(); dir.close(); - raw.close(); } /** LUCENE-1464: just creating a Directory should not diff --git a/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java b/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java index e1f237c4b4a..4a6df5e222f 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java +++ b/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java @@ -1099,11 +1099,11 @@ public abstract class LuceneTestCase extends Assert { } private static BaseDirectoryWrapper wrapDirectory(Random random, Directory directory, boolean bare) { - if (rarely(random)) { + if (rarely(random) && !bare) { directory = new NRTCachingDirectory(directory, random.nextDouble(), random.nextDouble()); } - if (rarely(random)) { + if (rarely(random) && !bare) { final double maxMBPerSec = 10 + 5*(random.nextDouble()-0.5); if (LuceneTestCase.VERBOSE) { System.out.println("LuceneTestCase: will rate limit output IndexOutput to " + maxMBPerSec + " MB/sec");