HBASE-10452 Fix potential bugs in exception handlers (Ding Yuan)

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1567979 13f79535-47bb-0310-9956-ffa450edef68

hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java

@@ -429,11 +429,13 @@ public class ClientScanner extends AbstractClientScanner {
         callable.setClose();
         try {
           this.caller.callWithRetries(callable);
+        } catch (UnknownScannerException e) {
+           // We used to catch this error, interpret, and rethrow. However, we
+           // have since decided that it's not nice for a scanner's close to
+           // throw exceptions. Chances are it was just due to lease time out.
         } catch (IOException e) {
-          // We used to catch this error, interpret, and rethrow. However, we
+           /* An exception other than UnknownScanner is unexpected. */
-          // have since decided that it's not nice for a scanner's close to
+           LOG.warn("scanner failed to close. Exception follows: " + e);
-          // throw exceptions. Chances are it was just an UnknownScanner
-          // exception due to lease time out.
         }
         callable = null;
       }

hbase-client/src/main/java/org/apache/hadoop/hbase/client/Get.java

@@ -29,6 +29,8 @@ import java.util.Set;
 import java.util.TreeMap;
 import java.util.TreeSet;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
 import org.apache.hadoop.hbase.HConstants;
@@ -63,6 +65,7 @@ import org.apache.hadoop.hbase.util.Bytes;
 @InterfaceStability.Stable
 public class Get extends Query
   implements Row, Comparable<Row> {
+  private static final Log LOG = LogFactory.getLog(Get.class);
 
   private byte [] row = null;
   private int maxVersions = 1;
@@ -156,11 +159,14 @@ public class Get extends Query
    * @param timestamp version timestamp
    * @return this for invocation chaining
    */
-  public Get setTimeStamp(long timestamp) {
+  public Get setTimeStamp(long timestamp)
+  throws IOException {
     try {
       tr = new TimeRange(timestamp, timestamp+1);
     } catch(IOException e) {
-      // Will never happen
+      // This should never happen, unless integer overflow or something extremely wrong...
+      LOG.error("TimeRange failed, likely caused by integer overflow. ", e);
+      throw e;
     }
     return this;
   }

hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java

@@ -19,6 +19,8 @@
 
 package org.apache.hadoop.hbase.client;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
 import org.apache.hadoop.hbase.HConstants;
@@ -82,6 +84,8 @@ import java.util.TreeSet;
 @InterfaceAudience.Public
 @InterfaceStability.Stable
 public class Scan extends Query {
+  private static final Log LOG = LogFactory.getLog(Scan.class);
+
   private static final String RAW_ATTR = "_raw_";
   private static final String ISOLATION_LEVEL = "_isolationlevel_";
 
@@ -297,11 +301,14 @@ public class Scan extends Query {
    * @see #setMaxVersions(int)
    * @return this
    */
-  public Scan setTimeStamp(long timestamp) {
+  public Scan setTimeStamp(long timestamp)
+  throws IOException {
     try {
       tr = new TimeRange(timestamp, timestamp+1);
     } catch(IOException e) {
-      // Will never happen
+      // This should never happen, unless integer overflow or something extremely wrong...
+      LOG.error("TimeRange failed, likely caused by integer overflow. ", e);
+      throw e;
     }
     return this;
   }

hbase-common/src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java

@@ -145,7 +145,12 @@ public class HBaseConfiguration extends Configuration {
       if (Class.forName("org.apache.hadoop.conf.ConfServlet") != null) {
         isShowConf = true;
       }
-    } catch (Exception e) {
+    } catch (LinkageError e) {
+       // should we handle it more aggressively in addition to log the error?
+       LOG.warn("Error thrown: ", e);
+    } catch (ClassNotFoundException ce) {
+      LOG.debug("ClassNotFound: ConfServlet");
+      // ignore
     }
     return isShowConf;
   }

hbase-common/src/main/java/org/apache/hadoop/hbase/util/JVM.java

@@ -207,7 +207,8 @@ public class JVM {
       if (input != null){
         try {
           input.close();
-        } catch (IOException ignored) {
+        } catch (IOException e) {
+          LOG.warn("Not able to close the InputStream", e);
         }
       }
     }

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyPrefixRegionSplitPolicy.java

@@ -62,7 +62,11 @@ public class KeyPrefixRegionSplitPolicy extends IncreasingToUpperBoundRegionSpli
       try {
         prefixLength = Integer.parseInt(prefixLengthString);
       } catch (NumberFormatException nfe) {
-        // ignore
+        /* Differentiate NumberFormatException from an invalid value range reported below. */
+        LOG.error("Number format exception when parsing " + PREFIX_LENGTH_KEY + " for table "
+            + region.getTableDesc().getTableName() + ":"
+            + prefixLengthString + ". " + nfe);
+        return;
       }
       if (prefixLength <= 0) {
         LOG.error("Invalid value for " + PREFIX_LENGTH_KEY + " for table "

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeRequest.java

@@ -131,8 +131,10 @@ class RegionMergeRequest implements Runnable {
       try {
         this.tableLock.release();
       } catch (IOException ex) {
-        LOG.warn("Could not release the table lock", ex);
+        LOG.error("Could not release the table lock (something is really wrong). " 
-        //TODO: if we get here, and not abort RS, this lock will never be released
+           + "Aborting this server to avoid holding the lock forever.");
+        this.server.abort("Abort; we got an error when releasing the table lock "
+                         + "on " + region_a.getRegionNameAsString());
       }
     }
   }

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java

@@ -132,8 +132,10 @@ class SplitRequest implements Runnable {
       try {
         this.tableLock.release();
       } catch (IOException ex) {
-        LOG.warn("Could not release the table lock", ex);
+        LOG.error("Could not release the table lock (something is really wrong). " 
-        //TODO: if we get here, and not abort RS, this lock will never be released
+           + "Aborting this server to avoid holding the lock forever.");
+        this.server.abort("Abort; we got an error when releasing the table lock "
+                         + "on " + parent.getRegionNameAsString());
       }
     }
   }

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogReader.java

@@ -255,8 +255,15 @@ public class SequenceFileLogReader extends ReaderBase {
       Field fEnd = SequenceFile.Reader.class.getDeclaredField("end");
       fEnd.setAccessible(true);
       end = fEnd.getLong(this.reader);
-    } catch(Exception e) { /* reflection fail. keep going */ }
+    } catch(NoSuchFieldException nfe) {
-
+       /* reflection failure, keep going */
+    } catch(IllegalAccessException iae) {
+       /* reflection failure, keep going */
+    } catch(Exception e) {
+       /* All other cases. Should we handle it more aggressively? */
+       LOG.warn("Unexpected exception when accessing the end field", e);
+    }
+ 
     String msg = (this.path == null? "": this.path.toString()) +
       ", entryStart=" + entryStart + ", pos=" + pos +
       ((end == Long.MAX_VALUE) ? "" : ", end=" + end) +
@@ -268,8 +275,14 @@ public class SequenceFileLogReader extends ReaderBase {
         .getConstructor(String.class)
         .newInstance(msg)
         .initCause(ioe);
-    } catch(Exception e) { /* reflection fail. keep going */ }
+    } catch(NoSuchMethodException nfe) {
-
+       /* reflection failure, keep going */
+    } catch(IllegalAccessException iae) {
+       /* reflection failure, keep going */
+    } catch(Exception e) {
+       /* All other cases. Should we handle it more aggressively? */
+       LOG.warn("Unexpected exception when accessing the end field", e);
+    }
     return ioe;
   }
 }