You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by te...@apache.org on 2014/02/13 18:27:10 UTC

svn commit: r1567980 - in /hbase/branches/0.98: hbase-client/src/main/java/org/apache/hadoop/hbase/client/ hbase-common/src/main/java/org/apache/hadoop/hbase/ hbase-common/src/main/java/org/apache/hadoop/hbase/util/ hbase-server/src/main/java/org/apach...

Author: tedyu
Date: Thu Feb 13 17:27:09 2014
New Revision: 1567980

URL: http://svn.apache.org/r1567980
Log:
HBASE-10452 Fix potential bugs in exception handlers (Ding Yuan)


Modified:
    hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java
    hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Get.java
    hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
    hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java
    hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/util/JVM.java
    hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyPrefixRegionSplitPolicy.java
    hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeRequest.java
    hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java
    hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogReader.java

Modified: hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java?rev=1567980&r1=1567979&r2=1567980&view=diff
==============================================================================
--- hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java (original)
+++ hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java Thu Feb 13 17:27:09 2014
@@ -429,11 +429,13 @@ public class ClientScanner extends Abstr
         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
-          // have since decided that it's not nice for a scanner's close to
-          // throw exceptions. Chances are it was just an UnknownScanner
-          // exception due to lease time out.
+           /* An exception other than UnknownScanner is unexpected. */
+           LOG.warn("scanner failed to close. Exception follows: " + e);
         }
         callable = null;
       }

Modified: hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Get.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Get.java?rev=1567980&r1=1567979&r2=1567980&view=diff
==============================================================================
--- hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Get.java (original)
+++ hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Get.java Thu Feb 13 17:27:09 2014
@@ -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.Byte
 @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;
   }

Modified: hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java?rev=1567980&r1=1567979&r2=1567980&view=diff
==============================================================================
--- hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java (original)
+++ hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java Thu Feb 13 17:27:09 2014
@@ -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;
   }

Modified: hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java?rev=1567980&r1=1567979&r2=1567980&view=diff
==============================================================================
--- hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java (original)
+++ hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java Thu Feb 13 17:27:09 2014
@@ -145,7 +145,12 @@ public class HBaseConfiguration extends 
       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;
   }

Modified: hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/util/JVM.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/util/JVM.java?rev=1567980&r1=1567979&r2=1567980&view=diff
==============================================================================
--- hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/util/JVM.java (original)
+++ hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/util/JVM.java Thu Feb 13 17:27:09 2014
@@ -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);
         }
       }
     }

Modified: hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyPrefixRegionSplitPolicy.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyPrefixRegionSplitPolicy.java?rev=1567980&r1=1567979&r2=1567980&view=diff
==============================================================================
--- hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyPrefixRegionSplitPolicy.java (original)
+++ hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyPrefixRegionSplitPolicy.java Thu Feb 13 17:27:09 2014
@@ -62,7 +62,11 @@ public class KeyPrefixRegionSplitPolicy 
       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 "

Modified: hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeRequest.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeRequest.java?rev=1567980&r1=1567979&r2=1567980&view=diff
==============================================================================
--- hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeRequest.java (original)
+++ hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeRequest.java Thu Feb 13 17:27:09 2014
@@ -131,8 +131,10 @@ class RegionMergeRequest implements Runn
       try {
         this.tableLock.release();
       } catch (IOException ex) {
-        LOG.warn("Could not release the table lock", ex);
-        //TODO: if we get here, and not abort RS, this lock will never be released
+        LOG.error("Could not release the table lock (something is really wrong). " 
+           + "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());
       }
     }
   }

Modified: hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java?rev=1567980&r1=1567979&r2=1567980&view=diff
==============================================================================
--- hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java (original)
+++ hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java Thu Feb 13 17:27:09 2014
@@ -132,8 +132,10 @@ class SplitRequest implements Runnable {
       try {
         this.tableLock.release();
       } catch (IOException ex) {
-        LOG.warn("Could not release the table lock", ex);
-        //TODO: if we get here, and not abort RS, this lock will never be released
+        LOG.error("Could not release the table lock (something is really wrong). " 
+           + "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());
       }
     }
   }

Modified: hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogReader.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogReader.java?rev=1567980&r1=1567979&r2=1567980&view=diff
==============================================================================
--- hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogReader.java (original)
+++ hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogReader.java Thu Feb 13 17:27:09 2014
@@ -255,8 +255,15 @@ public class SequenceFileLogReader exten
       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 exten
         .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;
   }
 }