Author: atm
Date: Tue Jul 12 02:18:30 2011
New Revision: 1145428
URL: http://svn.apache.org/viewvc?rev=1145428&view=rev
Log:
HDFS-2132. Potential resource leak in EditLogFileOutputStream.close. (atm)
Modified:
hadoop/common/trunk/hdfs/CHANGES.txt
hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/EditLogFileOutputStream.java
hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestEditLogFileOutputStream.java
Modified: hadoop/common/trunk/hdfs/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hdfs/CHANGES.txt?rev=1145428&r1=1145427&r2=1145428&view=diff
==============================================================================
--- hadoop/common/trunk/hdfs/CHANGES.txt (original)
+++ hadoop/common/trunk/hdfs/CHANGES.txt Tue Jul 12 02:18:30 2011
@@ -818,6 +818,8 @@ Trunk (unreleased changes)
reading only from a currently being written block. (John George via
szetszwo)
+ HDFS-2132. Potential resource leak in EditLogFileOutputStream.close. (atm)
+
Release 0.22.0 - Unreleased
INCOMPATIBLE CHANGES
Modified: hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/EditLogFileOutputStream.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/EditLogFileOutputStream.java?rev=1145428&r1=1145427&r2=1145428&view=diff
==============================================================================
--- hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/EditLogFileOutputStream.java
(original)
+++ hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/EditLogFileOutputStream.java
Tue Jul 12 02:18:30 2011
@@ -28,8 +28,11 @@ import java.util.zip.Checksum;
import org.apache.hadoop.hdfs.protocol.FSConstants;
import org.apache.hadoop.io.DataOutputBuffer;
+import org.apache.hadoop.io.IOUtils;
import org.apache.hadoop.io.Writable;
+import com.google.common.annotations.VisibleForTesting;
+
/**
* An implementation of the abstract class {@link EditLogOutputStream}, which
* stores edits in a local file.
@@ -120,32 +123,41 @@ class EditLogFileOutputStream extends Ed
@Override
public void close() throws IOException {
- // close should have been called after all pending transactions
- // have been flushed & synced.
- // if already closed, just skip
- if(bufCurrent != null)
- {
- int bufSize = bufCurrent.size();
- if (bufSize != 0) {
- throw new IOException("FSEditStream has " + bufSize
- + " bytes still to be flushed and cannot " + "be closed.");
+ try {
+ // close should have been called after all pending transactions
+ // have been flushed & synced.
+ // if already closed, just skip
+ if(bufCurrent != null)
+ {
+ int bufSize = bufCurrent.size();
+ if (bufSize != 0) {
+ throw new IOException("FSEditStream has " + bufSize
+ + " bytes still to be flushed and cannot " + "be closed.");
+ }
+ bufCurrent.close();
+ bufCurrent = null;
}
- bufCurrent.close();
- bufCurrent = null;
- }
-
- if(bufReady != null) {
- bufReady.close();
- bufReady = null;
- }
-
- // remove the last INVALID marker from transaction log.
- if (fc != null && fc.isOpen()) {
- fc.truncate(fc.position());
- fc.close();
- }
- if (fp != null) {
- fp.close();
+
+ if(bufReady != null) {
+ bufReady.close();
+ bufReady = null;
+ }
+
+ // remove the last INVALID marker from transaction log.
+ if (fc != null && fc.isOpen()) {
+ fc.truncate(fc.position());
+ fc.close();
+ fc = null;
+ }
+ if (fp != null) {
+ fp.close();
+ fp = null;
+ }
+ } finally {
+ IOUtils.cleanup(FSNamesystem.LOG, bufCurrent, bufReady, fc, fp);
+ bufCurrent = bufReady = null;
+ fc = null;
+ fp = null;
}
}
@@ -225,4 +237,14 @@ class EditLogFileOutputStream extends Ed
File getFile() {
return file;
}
+
+ @VisibleForTesting
+ public void setFileChannelForTesting(FileChannel fc) {
+ this.fc = fc;
+ }
+
+ @VisibleForTesting
+ public FileChannel getFileChannelForTesting() {
+ return fc;
+ }
}
Modified: hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestEditLogFileOutputStream.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestEditLogFileOutputStream.java?rev=1145428&r1=1145427&r2=1145428&view=diff
==============================================================================
--- hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestEditLogFileOutputStream.java
(original)
+++ hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestEditLogFileOutputStream.java
Tue Jul 12 02:18:30 2011
@@ -20,9 +20,11 @@ package org.apache.hadoop.hdfs.server.na
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
import java.io.File;
import java.io.IOException;
+import java.nio.channels.FileChannel;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.DU;
@@ -31,6 +33,7 @@ import org.apache.hadoop.fs.permission.F
import org.apache.hadoop.hdfs.HdfsConfiguration;
import org.apache.hadoop.hdfs.MiniDFSCluster;
import org.junit.Test;
+import org.mockito.Mockito;
public class TestEditLogFileOutputStream {
@@ -58,5 +61,29 @@ public class TestEditLogFileOutputStream
assertTrue("Edit log disk space used should be at least 257 blocks",
257 * 4096 <= new DU(editLog, conf).getUsed());
}
+
+ @Test
+ public void testClose() throws IOException {
+ String errorMessage = "TESTING: fc.truncate() threw IOE";
+
+ File testDir = new File(System.getProperty("test.build.data", "/tmp"));
+ assertTrue("could not create test directory", testDir.exists() || testDir.mkdirs());
+ File f = new File(testDir, "edits");
+ assertTrue("could not create test file", f.createNewFile());
+ EditLogFileOutputStream elos = new EditLogFileOutputStream(f, 0);
+
+ FileChannel mockFc = Mockito.spy(elos.getFileChannelForTesting());
+ Mockito.doThrow(new IOException(errorMessage)).when(mockFc).truncate(Mockito.anyLong());
+ elos.setFileChannelForTesting(mockFc);
+
+ try {
+ elos.close();
+ fail("elos.close() succeeded, but should have thrown");
+ } catch (IOException e) {
+ assertEquals("wrong IOE thrown from elos.close()", e.getMessage(), errorMessage);
+ }
+
+ assertEquals("fc was not nulled when elos.close() failed", elos.getFileChannelForTesting(),
null);
+ }
}
|