Author: tgraves Date: Wed Feb 6 16:23:54 2013 New Revision: 1443042 URL: http://svn.apache.org/viewvc?rev=1443042&view=rev Log: HADOOP-9278. Fix the file handle leak in HarMetaData.parseMetaData() in HarFileSystem. (Chris Nauroth via tgraves)
Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LineReader.java hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystemBasics.java Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt?rev=1443042&r1=1443041&r2=1443042&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt Wed Feb 6 16:23:54 2013 @@ -67,6 +67,9 @@ Release 0.23.7 - UNRELEASED HADOOP-9124. SortedMapWritable violates contract of Map interface for equals() and hashCode(). (Surenkumar Nihalani via tgraves) + HADOOP-9278. Fix the file handle leak in HarMetaData.parseMetaData() in + HarFileSystem. (Chris Nauroth via tgraves) + Release 0.23.6 - UNRELEASED INCOMPATIBLE CHANGES Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java?rev=1443042&r1=1443041&r2=1443042&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java Wed Feb 6 16:23:54 2013 @@ -30,8 +30,11 @@ import java.util.TreeMap; import java.util.HashMap; import java.util.concurrent.ConcurrentHashMap; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.permission.FsPermission; +import org.apache.hadoop.io.IOUtils; import org.apache.hadoop.io.Text; import org.apache.hadoop.util.LineReader; import org.apache.hadoop.util.Progressable; @@ -50,6 +53,9 @@ import org.apache.hadoop.util.Progressab */ public class HarFileSystem extends FilterFileSystem { + + private static final Log LOG = LogFactory.getLog(HarFileSystem.class); + public static final int VERSION = 3; private static final Map<URI, HarMetaData> harMetaCache = @@ -987,68 +993,69 @@ public class HarFileSystem extends Filte } private void parseMetaData() throws IOException { - FSDataInputStream in = fs.open(masterIndexPath); - FileStatus masterStat = fs.getFileStatus(masterIndexPath); - masterIndexTimestamp = masterStat.getModificationTime(); - LineReader lin = new LineReader(in, getConf()); - Text line = new Text(); - long read = lin.readLine(line); - - // the first line contains the version of the index file - String versionLine = line.toString(); - String[] arr = versionLine.split(" "); - version = Integer.parseInt(arr[0]); - // make it always backwards-compatible - if (this.version > HarFileSystem.VERSION) { - throw new IOException("Invalid version " + - this.version + " expected " + HarFileSystem.VERSION); - } - - // each line contains a hashcode range and the index file name - String[] readStr = null; - while(read < masterStat.getLen()) { - int b = lin.readLine(line); - read += b; - readStr = line.toString().split(" "); - int startHash = Integer.parseInt(readStr[0]); - int endHash = Integer.parseInt(readStr[1]); - stores.add(new Store(Long.parseLong(readStr[2]), - Long.parseLong(readStr[3]), startHash, - endHash)); - line.clear(); - } + Text line; + long read; + FSDataInputStream in = null; + LineReader lin = null; + try { - // close the master index - lin.close(); - } catch(IOException io){ - // do nothing just a read. - } + in = fs.open(masterIndexPath); + FileStatus masterStat = fs.getFileStatus(masterIndexPath); + masterIndexTimestamp = masterStat.getModificationTime(); + lin = new LineReader(in, getConf()); + line = new Text(); + read = lin.readLine(line); + + // the first line contains the version of the index file + String versionLine = line.toString(); + String[] arr = versionLine.split(" "); + version = Integer.parseInt(arr[0]); + // make it always backwards-compatible + if (this.version > HarFileSystem.VERSION) { + throw new IOException("Invalid version " + + this.version + " expected " + HarFileSystem.VERSION); + } - FSDataInputStream aIn = fs.open(archiveIndexPath); - FileStatus archiveStat = fs.getFileStatus(archiveIndexPath); - archiveIndexTimestamp = archiveStat.getModificationTime(); - LineReader aLin; - - // now start reading the real index file - for (Store s: stores) { - read = 0; - aIn.seek(s.begin); - aLin = new LineReader(aIn, getConf()); - while (read + s.begin < s.end) { - int tmp = aLin.readLine(line); - read += tmp; - String lineFeed = line.toString(); - String[] parsed = lineFeed.split(" "); - parsed[0] = decodeFileName(parsed[0]); - archive.put(new Path(parsed[0]), new HarStatus(lineFeed)); + // each line contains a hashcode range and the index file name + String[] readStr = null; + while(read < masterStat.getLen()) { + int b = lin.readLine(line); + read += b; + readStr = line.toString().split(" "); + int startHash = Integer.parseInt(readStr[0]); + int endHash = Integer.parseInt(readStr[1]); + stores.add(new Store(Long.parseLong(readStr[2]), + Long.parseLong(readStr[3]), startHash, + endHash)); line.clear(); } + } finally { + IOUtils.cleanup(LOG, lin, in); } + + FSDataInputStream aIn = fs.open(archiveIndexPath); try { - // close the archive index - aIn.close(); - } catch(IOException io) { - // do nothing just a read. + FileStatus archiveStat = fs.getFileStatus(archiveIndexPath); + archiveIndexTimestamp = archiveStat.getModificationTime(); + LineReader aLin; + + // now start reading the real index file + for (Store s: stores) { + read = 0; + aIn.seek(s.begin); + aLin = new LineReader(aIn, getConf()); + while (read + s.begin < s.end) { + int tmp = aLin.readLine(line); + read += tmp; + String lineFeed = line.toString(); + String[] parsed = lineFeed.split(" "); + parsed[0] = decodeFileName(parsed[0]); + archive.put(new Path(parsed[0]), new HarStatus(lineFeed)); + line.clear(); + } + } + } finally { + IOUtils.cleanup(LOG, aIn); } } } Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LineReader.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LineReader.java?rev=1443042&r1=1443041&r2=1443042&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LineReader.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LineReader.java Wed Feb 6 16:23:54 2013 @@ -18,6 +18,7 @@ package org.apache.hadoop.util; +import java.io.Closeable; import java.io.IOException; import java.io.InputStream; @@ -39,7 +40,7 @@ import org.apache.hadoop.io.Text; */ @InterfaceAudience.LimitedPrivate({"MapReduce"}) @InterfaceStability.Unstable -public class LineReader { +public class LineReader implements Closeable { private static final int DEFAULT_BUFFER_SIZE = 64 * 1024; private int bufferSize = DEFAULT_BUFFER_SIZE; private InputStream in; Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystemBasics.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystemBasics.java?rev=1443042&r1=1443041&r2=1443042&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystemBasics.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystemBasics.java Wed Feb 6 16:23:54 2013 @@ -28,6 +28,7 @@ import java.net.URI; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.permission.FsPermission; +import org.apache.hadoop.util.Shell; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -46,8 +47,18 @@ public class TestHarFileSystemBasics { private static final String ROOT_PATH = System.getProperty("test.build.data", "build/test/data"); - private static final Path rootPath = new Path( - new File(ROOT_PATH).getAbsolutePath() + "/localfs"); + private static final Path rootPath; + static { + String root = new Path(new File(ROOT_PATH).getAbsolutePath(), "localfs") + .toUri().getPath(); + // Strip drive specifier on Windows, which would make the HAR URI invalid and + // cause tests to fail. + if (Shell.WINDOWS) { + root = root.substring(root.indexOf(':') + 1); + } + rootPath = new Path(root); + } + // NB: .har suffix is necessary private static final Path harPath = new Path(rootPath, "path1/path2/my.har");