From 55c3e2de3d636482ef2c51bdf88e89a34fc58b32 Mon Sep 17 00:00:00 2001 From: Karthik Kambatla Date: Thu, 26 May 2016 14:24:31 -0700 Subject: [PATCH] HDFS-10463. TestRollingFileSystemSinkWithHdfs needs some cleanup. (Daniel Templeton via kasha) --- .../sink/RollingFileSystemSinkTestBase.java | 8 +- .../TestRollingFileSystemSinkWithHdfs.java | 93 ++++++++++++------- 2 files changed, 63 insertions(+), 38 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/sink/RollingFileSystemSinkTestBase.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/sink/RollingFileSystemSinkTestBase.java index 4ac10f2763d..8c366db9e45 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/sink/RollingFileSystemSinkTestBase.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/sink/RollingFileSystemSinkTestBase.java @@ -221,8 +221,12 @@ public class RollingFileSystemSinkTestBase { mm1.testMetric2.incr(2); ms.publishMetricsNow(); // publish the metrics - ms.stop(); - ms.shutdown(); + + try { + ms.stop(); + } finally { + ms.shutdown(); + } return readLogFile(path, then, count); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/metrics2/sink/TestRollingFileSystemSinkWithHdfs.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/metrics2/sink/TestRollingFileSystemSinkWithHdfs.java index fc25ded1804..2a7660486e8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/metrics2/sink/TestRollingFileSystemSinkWithHdfs.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/metrics2/sink/TestRollingFileSystemSinkWithHdfs.java @@ -26,6 +26,7 @@ import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hdfs.MiniDFSCluster; +import org.apache.hadoop.metrics2.MetricsException; import org.apache.hadoop.metrics2.MetricsSystem; import org.apache.hadoop.metrics2.sink.RollingFileSystemSinkTestBase.MyMetrics1; import org.junit.After; @@ -108,7 +109,7 @@ public class TestRollingFileSystemSinkWithHdfs public void testSilentAppend() throws Exception { String path = "hdfs://" + cluster.getNameNode().getHostAndPort() + "/tmp"; - assertExtraContents(doAppendTest(path, false, true, 1)); + assertExtraContents(doAppendTest(path, true, true, 1)); } /** @@ -158,8 +159,11 @@ public class TestRollingFileSystemSinkWithHdfs assertTrue("No exception was generated while writing metrics " + "even though HDFS was unavailable", MockSink.errored); - ms.stop(); - ms.shutdown(); + try { + ms.stop(); + } finally { + ms.shutdown(); + } } /** @@ -180,12 +184,16 @@ public class TestRollingFileSystemSinkWithHdfs shutdownHdfs(); MockSink.errored = false; - ms.stop(); + try { + ms.stop(); - assertTrue("No exception was generated while stopping sink " - + "even though HDFS was unavailable", MockSink.errored); - - ms.shutdown(); + assertTrue("No exception was generated while stopping sink " + + "even though HDFS was unavailable", MockSink.errored); + } catch (MetricsException ex) { + // Expected + } finally { + ms.shutdown(); + } } /** @@ -211,8 +219,11 @@ public class TestRollingFileSystemSinkWithHdfs + "while HDFS was unavailable, even though the sink is set to " + "ignore errors", MockSink.errored); - ms.stop(); - ms.shutdown(); + try { + ms.stop(); + } finally { + ms.shutdown(); + } } /** @@ -233,13 +244,15 @@ public class TestRollingFileSystemSinkWithHdfs shutdownHdfs(); MockSink.errored = false; - ms.stop(); + try { + ms.stop(); - assertFalse("An exception was generated stopping sink " - + "while HDFS was unavailable, even though the sink is set to " - + "ignore errors", MockSink.errored); - - ms.shutdown(); + assertFalse("An exception was generated stopping sink " + + "while HDFS was unavailable, even though the sink is set to " + + "ignore errors", MockSink.errored); + } finally { + ms.shutdown(); + } } /** @@ -268,29 +281,37 @@ public class TestRollingFileSystemSinkWithHdfs int count = 0; - // Sleep until the flusher has run. This should never actually need to - // sleep, but the sleep is here to make sure this test isn't flakey. - while (!RollingFileSystemSink.hasFlushed) { - Thread.sleep(10L); + try { + // Sleep until the flusher has run. This should never actually need to + // sleep, but the sleep is here to make sure this test isn't flakey. + while (!RollingFileSystemSink.hasFlushed) { + Thread.sleep(10L); - if (++count > 1000) { - fail("Flush thread did not run within 10 seconds"); + if (++count > 1000) { + fail("Flush thread did not run within 10 seconds"); + } + } + + Calendar now = Calendar.getInstance(); + Path currentDir = new Path(path, DATE_FORMAT.format(now.getTime()) + "00"); + FileSystem fs = FileSystem.newInstance(new URI(path), new Configuration()); + Path currentFile = + findMostRecentLogFile(fs, new Path(currentDir, getLogFilename())); + FileStatus status = fs.getFileStatus(currentFile); + + // Each metrics record is 118+ bytes, depending on hostname + assertTrue("The flusher thread didn't flush the log contents. Expected " + + "at least 236 bytes in the log file, but got " + status.getLen(), + status.getLen() >= 236); + } finally { + RollingFileSystemSink.forceFlush = false; + + try { + ms.stop(); + } finally { + ms.shutdown(); } } - - Calendar now = Calendar.getInstance(); - Path currentDir = new Path(path, DATE_FORMAT.format(now.getTime()) + "00"); - FileSystem fs = FileSystem.newInstance(new URI(path), new Configuration()); - Path currentFile = - findMostRecentLogFile(fs, new Path(currentDir, getLogFilename())); - FileStatus status = fs.getFileStatus(currentFile); - - // Each metrics record is 118+ bytes, depending on hostname - assertTrue("The flusher thread didn't flush the log contents. Expected " - + "at least 236 bytes in the log file, but got " + status.getLen(), - status.getLen() >= 236); - - ms.stop(); } /**