From fb7e8ab75f96e4f8335d4cbb90c862bc6219596f Mon Sep 17 00:00:00 2001 From: Shai Erera Date: Wed, 25 May 2011 10:46:23 +0000 Subject: [PATCH] LUCENE-3139: add more verbosing to LTC in case it fails to remove temp directories git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1127470 13f79535-47bb-0310-9956-ffa450edef68 --- .../PersistentSnapshotDeletionPolicy.java | 20 +++++++++----- .../lucene/store/MockIndexOutputWrapper.java | 27 ++++++++++--------- .../apache/lucene/util/LuceneTestCase.java | 25 +++++++++++++---- .../org/apache/lucene/util/_TestUtil.java | 6 ++--- .../TestPersistentSnapshotDeletionPolicy.java | 10 ++++++- 5 files changed, 61 insertions(+), 27 deletions(-) diff --git a/lucene/src/java/org/apache/lucene/index/PersistentSnapshotDeletionPolicy.java b/lucene/src/java/org/apache/lucene/index/PersistentSnapshotDeletionPolicy.java index f4869ea926a..d33e7a3f032 100644 --- a/lucene/src/java/org/apache/lucene/index/PersistentSnapshotDeletionPolicy.java +++ b/lucene/src/java/org/apache/lucene/index/PersistentSnapshotDeletionPolicy.java @@ -59,7 +59,7 @@ public class PersistentSnapshotDeletionPolicy extends SnapshotDeletionPolicy { /** * Reads the snapshots information from the given {@link Directory}. This - * method does can be used if the snapshots information is needed, however you + * method can be used if the snapshots information is needed, however you * cannot instantiate the deletion policy (because e.g., some other process * keeps a lock on the snapshots directory). */ @@ -122,11 +122,19 @@ public class PersistentSnapshotDeletionPolicy extends SnapshotDeletionPolicy { writer.commit(); } - // Initializes the snapshots information. This code should basically run - // only if mode != CREATE, but if it is, it's no harm as we only open the - // reader once and immediately close it. - for (Entry e : readSnapshotsInfo(dir).entrySet()) { - registerSnapshotInfo(e.getKey(), e.getValue(), null); + try { + // Initializes the snapshots information. This code should basically run + // only if mode != CREATE, but if it is, it's no harm as we only open the + // reader once and immediately close it. + for (Entry e : readSnapshotsInfo(dir).entrySet()) { + registerSnapshotInfo(e.getKey(), e.getValue(), null); + } + } catch (RuntimeException e) { + writer.close(); // don't leave any open file handles + throw e; + } catch (IOException e) { + writer.close(); // don't leave any open file handles + throw e; } } diff --git a/lucene/src/test-framework/org/apache/lucene/store/MockIndexOutputWrapper.java b/lucene/src/test-framework/org/apache/lucene/store/MockIndexOutputWrapper.java index 7e6e17d630b..3ae5b1dc7b8 100644 --- a/lucene/src/test-framework/org/apache/lucene/store/MockIndexOutputWrapper.java +++ b/lucene/src/test-framework/org/apache/lucene/store/MockIndexOutputWrapper.java @@ -45,19 +45,22 @@ public class MockIndexOutputWrapper extends IndexOutput { @Override public void close() throws IOException { - dir.maybeThrowDeterministicException(); - delegate.close(); - if (dir.trackDiskUsage) { - // Now compute actual disk usage & track the maxUsedSize - // in the MockDirectoryWrapper: - long size = dir.getRecomputedActualSizeInBytes(); - if (size > dir.maxUsedSize) { - dir.maxUsedSize = size; + try { + dir.maybeThrowDeterministicException(); + } finally { + delegate.close(); + if (dir.trackDiskUsage) { + // Now compute actual disk usage & track the maxUsedSize + // in the MockDirectoryWrapper: + long size = dir.getRecomputedActualSizeInBytes(); + if (size > dir.maxUsedSize) { + dir.maxUsedSize = size; + } + } + synchronized(dir) { + dir.openFileHandles.remove(this); + dir.openFilesForWrite.remove(name); } - } - synchronized(dir) { - dir.openFileHandles.remove(this); - dir.openFilesForWrite.remove(name); } } diff --git a/lucene/src/test-framework/org/apache/lucene/util/LuceneTestCase.java b/lucene/src/test-framework/org/apache/lucene/util/LuceneTestCase.java index 47541e56ce1..5f2321a473e 100644 --- a/lucene/src/test-framework/org/apache/lucene/util/LuceneTestCase.java +++ b/lucene/src/test-framework/org/apache/lucene/util/LuceneTestCase.java @@ -28,6 +28,7 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.*; +import java.util.Map.Entry; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; @@ -132,7 +133,7 @@ public abstract class LuceneTestCase extends Assert { } /** set of directories we created, in afterclass we try to clean these up */ - static final Set tempDirs = Collections.synchronizedSet(new HashSet()); + private static final Map tempDirs = Collections.synchronizedMap(new HashMap()); // by default we randomly pick a different codec for // each test case (non-J4 tests) and each test class (J4 @@ -180,7 +181,7 @@ public abstract class LuceneTestCase extends Assert { SETUP, // test has called setUp() RANTEST, // test is running TEARDOWN // test has called tearDown() - }; + } /** * Some tests expect the directory to contain a single segment, and want to do tests on that segment's reader. @@ -454,11 +455,20 @@ public abstract class LuceneTestCase extends Assert { } // clear out any temp directories if we can if (!testsFailed) { - for (String path : tempDirs) { + for (Entry entry : tempDirs.entrySet()) { try { - _TestUtil.rmDir(new File(path)); + _TestUtil.rmDir(entry.getKey()); } catch (IOException e) { e.printStackTrace(); + System.err.println("path " + entry.getKey() + " allocated from"); + // first two STE's are Java's + StackTraceElement[] elements = entry.getValue(); + for (int i = 2; i < elements.length; i++) { + StackTraceElement ste = elements[i]; + // print only our code's stack information + if (ste.getClassName().indexOf("org.apache.lucene") == -1) break; + System.err.println("\t" + ste); + } } } } @@ -1112,6 +1122,11 @@ public abstract class LuceneTestCase extends Assert { return d; } + /** Registers a temp file that will be deleted when tests are done. */ + public static void registerTempFile(File tmpFile) { + tempDirs.put(tmpFile.getAbsoluteFile(), Thread.currentThread().getStackTrace()); + } + static Directory newDirectoryImpl(Random random, String clazzName) { if (clazzName.equals("random")) clazzName = randomDirectory(random); @@ -1124,7 +1139,7 @@ public abstract class LuceneTestCase extends Assert { final File tmpFile = File.createTempFile("test", "tmp", TEMP_DIR); tmpFile.delete(); tmpFile.mkdir(); - tempDirs.add(tmpFile.getAbsolutePath()); + registerTempFile(tmpFile); return newFSDirectoryImpl(clazz.asSubclass(FSDirectory.class), tmpFile, null); } diff --git a/lucene/src/test-framework/org/apache/lucene/util/_TestUtil.java b/lucene/src/test-framework/org/apache/lucene/util/_TestUtil.java index be1a4500f1e..5fab6ce3c52 100644 --- a/lucene/src/test-framework/org/apache/lucene/util/_TestUtil.java +++ b/lucene/src/test-framework/org/apache/lucene/util/_TestUtil.java @@ -54,8 +54,8 @@ public class _TestUtil { /** Returns temp dir, containing String arg in its name; * does not create the directory. */ public static File getTempDir(String desc) { - File f = new File(LuceneTestCase.TEMP_DIR, desc + "." + new Random().nextLong()); - LuceneTestCase.tempDirs.add(f.getAbsolutePath()); + File f = new File(LuceneTestCase.TEMP_DIR, desc + "." + LuceneTestCase.random.nextLong()); + LuceneTestCase.registerTempFile(f); return f; } @@ -91,7 +91,7 @@ public class _TestUtil { rmDir(destDir); destDir.mkdir(); - LuceneTestCase.tempDirs.add(destDir.getAbsolutePath()); + LuceneTestCase.registerTempFile(destDir); while (entries.hasMoreElements()) { ZipEntry entry = entries.nextElement(); diff --git a/lucene/src/test/org/apache/lucene/index/TestPersistentSnapshotDeletionPolicy.java b/lucene/src/test/org/apache/lucene/index/TestPersistentSnapshotDeletionPolicy.java index fb80fdae884..b18acf20efb 100644 --- a/lucene/src/test/org/apache/lucene/index/TestPersistentSnapshotDeletionPolicy.java +++ b/lucene/src/test/org/apache/lucene/index/TestPersistentSnapshotDeletionPolicy.java @@ -30,9 +30,13 @@ import org.junit.Before; import org.junit.Test; public class TestPersistentSnapshotDeletionPolicy extends TestSnapshotDeletionPolicy { + // Keep it a class member so that getDeletionPolicy can use it private Directory snapshotDir; + // so we can close it if called by SDP tests + private PersistentSnapshotDeletionPolicy psdp; + @Before @Override public void setUp() throws Exception { @@ -43,15 +47,17 @@ public class TestPersistentSnapshotDeletionPolicy extends TestSnapshotDeletionPo @After @Override public void tearDown() throws Exception { + if (psdp != null) psdp.close(); snapshotDir.close(); super.tearDown(); } @Override protected SnapshotDeletionPolicy getDeletionPolicy() throws IOException { + if (psdp != null) psdp.close(); snapshotDir.close(); snapshotDir = newDirectory(); - return new PersistentSnapshotDeletionPolicy( + return psdp = new PersistentSnapshotDeletionPolicy( new KeepOnlyLastCommitDeletionPolicy(), snapshotDir, OpenMode.CREATE, TEST_VERSION_CURRENT); } @@ -173,6 +179,8 @@ public class TestPersistentSnapshotDeletionPolicy extends TestSnapshotDeletionPo fail("should not have reached here - the snapshots directory should be locked!"); } catch (LockObtainFailedException e) { // expected + } finally { + psdp.close(); } // Reading the snapshots info should succeed though