From eeb9206acee8517b820baf3a0bca3a1b834ea890 Mon Sep 17 00:00:00 2001 From: stack Date: Tue, 15 Oct 2019 15:39:40 -0700 Subject: [PATCH] HBASE-23177 If fail to open reference because FNFE, make it plain it is a Reference --- .../org/apache/hadoop/hbase/io/FileLink.java | 8 ++-- .../org/apache/hadoop/hbase/io/HFileLink.java | 3 ++ .../hbase/regionserver/StoreFileInfo.java | 15 +++++- .../hbase/regionserver/TestStoreFileInfo.java | 46 ++++++++++++++++++- 4 files changed, 66 insertions(+), 6 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/FileLink.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/FileLink.java index 36e086a596..8e3e4ab49a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/FileLink.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/FileLink.java @@ -319,7 +319,7 @@ public class FileLink { if (!(ioe instanceof FileNotFoundException)) throw re; } } - throw new FileNotFoundException("Unable to open link: " + fileLink); + throw new FileNotFoundException(this.fileLink.toString()); } @Override @@ -363,7 +363,7 @@ public class FileLink { @Override public String toString() { - StringBuilder str = new StringBuilder(getClass().getName()); + StringBuilder str = new StringBuilder(getClass().getSimpleName()); str.append(" locations=["); for (int i = 0; i < locations.length; ++i) { if (i > 0) str.append(", "); @@ -394,7 +394,7 @@ public class FileLink { return locations[i]; } } - throw new FileNotFoundException("Unable to open link: " + this); + throw new FileNotFoundException(toString()); } /** @@ -412,7 +412,7 @@ public class FileLink { // Try another file location } } - throw new FileNotFoundException("Unable to open link: " + this); + throw new FileNotFoundException(toString()); } /** diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/HFileLink.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/HFileLink.java index 2aebdf0d04..959a5ab71e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/HFileLink.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/HFileLink.java @@ -70,6 +70,9 @@ public class HFileLink extends FileLink { * Region name is ([a-f0-9]+), so '-' is an invalid character for the region name. * HFile is ([0-9a-f]+(?:_SeqId_[0-9]+_)?) covering the plain hfiles (uuid) * and the bulk loaded (_SeqId_[0-9]+_) hfiles. + * + *

Here is an example name: /hbase/test/0123/cf/testtb=4567-abcd where 'testtb' is table name + * and '4567' is region name and 'abcd' is filename. */ public static final String LINK_NAME_REGEX = String.format("(?:(?:%s=)?)%s=%s-%s", diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java index 8f9e1c8a4c..1e45ef0cfd 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java @@ -285,7 +285,20 @@ public class StoreFileInfo { } else if (this.reference != null) { // HFile Reference Path referencePath = getReferredToFile(this.getPath()); - in = new FSDataInputStreamWrapper(fs, referencePath, doDropBehind, readahead); + /* + try { + */ + in = new FSDataInputStreamWrapper(fs, referencePath, doDropBehind, readahead); + /* + } catch (FileNotFoundException fnfe) { + // Intercept the exception so can insert more info about the Reference; otherwise + // exception just complains about some random file -- operator doesn't realize it + // other end of a Reference + FileNotFoundException newFnfe = new FileNotFoundException(toString()); + newFnfe.initCause(fnfe); + throw newFnfe; + } + */ status = fs.getFileStatus(referencePath); } else { in = new FSDataInputStreamWrapper(fs, this.getPath(), doDropBehind, readahead); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileInfo.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileInfo.java index 24b73a520a..204840eb94 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileInfo.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileInfo.java @@ -1,4 +1,4 @@ -/** +/* * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information @@ -19,11 +19,17 @@ package org.apache.hadoop.hbase.regionserver; import static org.junit.Assert.*; +import java.io.FileNotFoundException; import java.io.IOException; +import java.util.concurrent.atomic.AtomicInteger; + +import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.io.HFileLink; +import org.apache.hadoop.hbase.io.Reference; import org.apache.hadoop.hbase.testclassification.RegionServerTests; import org.apache.hadoop.hbase.testclassification.SmallTests; import org.junit.ClassRule; @@ -88,5 +94,43 @@ public class TestStoreFileInfo { assertEquals(info1, info2); assertEquals(info1.hashCode(), info2.hashCode()); } + + @Test + public void testOpenErrorMessageHFileLink() throws IOException, IllegalStateException { + // Test file link exception + // Try to open nonsense hfilelink. Make sure exception is from HFileLink. + Path p = new Path("/hbase/test/0123/cf/testtb=4567-abcd"); + try (FileSystem fs = FileSystem.get(TEST_UTIL.getConfiguration())) { + StoreFileInfo sfi = new StoreFileInfo(TEST_UTIL.getConfiguration(), fs, p); + try { + sfi.open(fs, null, false, 1000, true, new AtomicInteger(), false); + throw new IllegalStateException(); + } catch (FileNotFoundException fnfe) { + assertTrue(fnfe.getMessage().contains(HFileLink.class.getSimpleName())); + } + } + } + + @Test + public void testOpenErrorMessageReference() throws IOException { + // Test file link exception + // Try to open nonsense hfilelink. Make sure exception is from HFileLink. + Path p = new Path(TEST_UTIL.getDataTestDirOnTestFS(),"4567.abcd"); + FileSystem fs = FileSystem.get(TEST_UTIL.getConfiguration()); + fs.mkdirs(p.getParent()); + Reference r = Reference.createBottomReference(HConstants.EMPTY_START_ROW); + r.write(fs, p); + StoreFileInfo sfi = new StoreFileInfo(TEST_UTIL.getConfiguration(), fs, p); + /* + try { + */ + sfi.open(fs, null, false, 1000, true, new AtomicInteger(), false); + /* + throw new IllegalStateException(); + } catch (FileNotFoundException fnfe) { + assertTrue(fnfe.getMessage().contains("->")); + } + */ + } } -- 2.19.1