From e467f838bb6906772fb06b409c05b5f86cb23469 Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Fri, 29 Jul 2016 15:39:48 +0900 Subject: [PATCH] HDFS-10691. FileDistribution fails in hdfs oiv command due to ArrayIndexOutOfBoundsException. Contributed by Yiqun Lin. (cherry picked from commit 204a2055b1b9270ae13ea03b7aeac62b65166efd) (cherry picked from commit 0cff416c35528ed33e87f8401a49503a44f992b0) --- .../FileDistributionCalculator.java | 6 +++ .../FileDistributionVisitor.java | 4 ++ .../TestOfflineImageViewer.java | 49 +++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/FileDistributionCalculator.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/FileDistributionCalculator.java index 056ad96a5ba..33ab641e54c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/FileDistributionCalculator.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/FileDistributionCalculator.java @@ -128,6 +128,12 @@ final class FileDistributionCalculator { int bucket = fileSize > maxSize ? distribution.length - 1 : (int) Math .ceil((double)fileSize / steps); + // Compare the bucket value with distribution's length again, + // because sometimes the bucket value will be equal to + // the length when maxSize can't be divided completely by step. + if (bucket >= distribution.length) { + bucket = distribution.length - 1; + } ++distribution[bucket]; } else if (p.getType() == INodeSection.INode.Type.DIRECTORY) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/FileDistributionVisitor.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/FileDistributionVisitor.java index 146d00a85ee..1cef720624d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/FileDistributionVisitor.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/FileDistributionVisitor.java @@ -145,6 +145,10 @@ class FileDistributionVisitor extends TextWriterImageVisitor { high = distribution.length-1; else high = (int)Math.ceil((double)current.fileSize / step); + + if (high >= distribution.length) { + high = distribution.length - 1; + } distribution[high]++; if(totalFiles % 1000000 == 1) System.out.println("Files processed: " + totalFiles diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java index 1cc82a15310..b9aa7f39e93 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java @@ -70,6 +70,7 @@ import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.FileSystemTestHelper; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hdfs.DFSConfigKeys; +import org.apache.hadoop.hdfs.DFSTestUtil; import org.apache.hadoop.hdfs.DistributedFileSystem; import org.apache.hadoop.hdfs.MiniDFSCluster; import org.apache.hadoop.hdfs.protocol.HdfsConstants.SafeModeAction; @@ -552,4 +553,52 @@ public class TestOfflineImageViewer { GenericTestUtils.assertExceptionContains("Layout version mismatch.", t); } } + + @Test + public void testFileDistributionCalculatorForException() throws Exception { + File fsimageFile = null; + Configuration conf = new Configuration(); + HashMap files = Maps.newHashMap(); + + // Create a initial fsimage file + try (MiniDFSCluster cluster = + new MiniDFSCluster.Builder(conf).numDataNodes(1).build()) { + cluster.waitActive(); + DistributedFileSystem hdfs = cluster.getFileSystem(); + + // Create a reasonable namespace + Path dir = new Path("/dir"); + hdfs.mkdirs(dir); + files.put(dir.toString(), pathToFileEntry(hdfs, dir.toString())); + // Create files with byte size that can't be divided by step size, + // the byte size for here are 3, 9, 15, 21. + for (int i = 0; i < FILES_PER_DIR; i++) { + Path file = new Path(dir, "file" + i); + DFSTestUtil.createFile(hdfs, file, 6 * i + 3, (short) 1, 0); + + files.put(file.toString(), + pathToFileEntry(hdfs, file.toString())); + } + + // Write results to the fsimage file + hdfs.setSafeMode(SafeModeAction.SAFEMODE_ENTER, false); + hdfs.saveNamespace(); + // Determine location of fsimage file + fsimageFile = + FSImageTestUtil.findLatestImageFile(FSImageTestUtil + .getFSImage(cluster.getNameNode()).getStorage().getStorageDir(0)); + if (fsimageFile == null) { + throw new RuntimeException("Didn't generate or can't find fsimage"); + } + } + + // Run the test with params -maxSize 23 and -step 4, it will not throw + // ArrayIndexOutOfBoundsException with index 6 when deals with + // 21 byte size file. + int status = + OfflineImageViewerPB.run(new String[] {"-i", + fsimageFile.getAbsolutePath(), "-o", "-", "-p", + "FileDistribution", "-maxSize", "23", "-step", "4"}); + assertEquals(0, status); + } }