HBase
  1. HBase
  2. HBASE-10452

Fix potential bugs in exception handlers

    Details

    • Hadoop Flags:
      Reviewed

      Description

      Hi HBase developers,
      We are a group of researchers on software reliability. Recently we did a study and found that majority of the most severe failures in HBase are caused by bugs in exception handling logic – that it is hard to anticipate all the possible real-world error scenarios. Therefore we built a simple checking tool that automatically detects some bug patterns that have caused some very severe real-world failures. I am reporting some of the results here. Any feedback is much appreciated!

      Ding

      =========================
      Case 1:
      Line: 134, File: "org/apache/hadoop/hbase/regionserver/RegionMergeRequest.java"

        protected void releaseTableLock() {
          if (this.tableLock != null) {
            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
            }
          }
      

      The lock is not released if the exception occurs, causing potential deadlock or starvation.

      Similar code pattern can be found at:
      Line: 135, File: "org/apache/hadoop/hbase/regionserver/SplitRequest.java"
      ==========================================

      =========================
      Case 2:
      Line: 252, File: "org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogReader.java"

          try {
            Field fEnd = SequenceFile.Reader.class.getDeclaredField("end");
            fEnd.setAccessible(true);
            end = fEnd.getLong(this.reader);
          } catch(Exception e) { /* reflection fail. keep going */ }
      

      The caught Exception seems to be too general.
      While reflection-related errors might be harmless, the try block can throw
      other exceptions including "SecurityException", "IllegalAccessException", etc. Currently
      all those exceptions are ignored. Maybe
      the safe way is to ignore the specific reflection-related errors while logging and
      handling other types of unexpected exceptions.
      ==========================================
      =========================
      Case 3:
      Line: 148, File: "org/apache/hadoop/hbase/HBaseConfiguration.java"

          try {
            if (Class.forName("org.apache.hadoop.conf.ConfServlet") != null) {
              isShowConf = true;
            }
          } catch (Exception e) {
          }
      

      Similar to the previous case, the exception handling is too general. While ClassNotFound error might be the normal case and ignored, Class.forName can also throw other exceptions (e.g., LinkageError) under some unexpected and rare error cases. If that happens, the error will be lost. So maybe change it to below:

          try {
            if (Class.forName("org.apache.hadoop.conf.ConfServlet") != null) {
              isShowConf = true;
            }
          } catch (LinkageError e) {
            LOG.warn("..");
            // handle linkage error
          } catch (ExceptionInInitializerError e) {
            LOG.warn("..");
            // handle Initializer error
          } catch (ClassNotFoundException e) {
           LOG.debug("..");
           // ignore
          }
      

      ==========================================
      =========================
      Case 4:
      Line: 163, File: "org/apache/hadoop/hbase/client/Get.java"

        public Get setTimeStamp(long timestamp) {
          try {
            tr = new TimeRange(timestamp, timestamp+1);
          } catch(IOException e) {
            // Will never happen
          }
          return this;
        }
      

      Even if the IOException never happens right now, is it possible to happen in the future due to code change?
      At least there should be a log message. The current behavior is dangerous since if the exception ever happens
      in any unexpected scenario, it will be silently swallowed.

      Similar code pattern can be found at:
      Line: 300, File: "org/apache/hadoop/hbase/client/Scan.java"
      ==========================================

      =========================
      Case 5:
      Line: 207, File: "org/apache/hadoop/hbase/util/JVM.java"

         if (input != null){
              try {
                input.close();
              } catch (IOException ignored) {
              }
            }
      

      Any exception encountered in close is completely ignored, not even logged.
      In particular, the same exception scenario was handled differently in other methods in the same file:
      Line: 154, same file

             if (in != null){
               try {
                 in.close();
               } catch (IOException e) {
                 LOG.warn("Not able to close the InputStream", e);
               }
             }
      

      Line: 248, same file

            if (in != null){
              try {
                in.close();
              } catch (IOException e) {
                LOG.warn("Not able to close the InputStream", e);
              }
            }
      

      ==========================================

      =========================
      Case 6: empty handler for exception: java.io.IOException
      Line: 312, File: "org/apache/hadoop/hbase/rest/RowResource.java"

          } finally {
            if (table != null) try {
              table.close();
            } catch (IOException ioe) { }
          }
      

      IOException is completely ignored. This behavior is inconsistent with the same
      code snippet at line 249 in the same file, where the IOExceptions was logged:

         } finally {
            if (table != null) try {
              table.close();
            } catch (IOException ioe) {
              LOG.debug("Exception received while closing the table", ioe);
            }
          }
      

      ==========================================
      =========================
      Case 7:
      Line: 95, File: "org/apache/hadoop/hbase/master/handler/EnableTableHandler.java"

              try {
                this.assignmentManager.getZKTable().removeEnablingTable(tableName, true);
              } catch (KeeperException e) {
                // TODO : Use HBCK to clear such nodes
                LOG.warn("Failed to delete the ENABLING node for the table " + tableName
                    + ".  The table will remain unusable. Run HBCK to manually fix the problem.");
              }
      

      The log message in the exception handler and the comment seem to suggest that such nodes should be cleared using HBCK.
      ==========================================

      =========================
      Case 8:
      Line: 463, File: "org/apache/hadoop/hbase/client/ClientScanner.java"

              try {
                this.caller.callWithRetries(callable);
              } 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.
              }
      

      Currently the handler is empty because it may be caused by "just an UnknownScanner exception".
      But what if it has other causes? Maybe the catch block can differentiate the exception into
      different causes, ignoring the ones caused by UnknownScanner while handle others differently?
      ==========================================

      1. HBase-10452-trunk.patch
        9 kB
        Ding Yuan
      2. HBase-10452-trunk-v2.patch
        9 kB
        Ding Yuan
      3. HBase-10452-trunk-v3.patch
        10 kB
        Ding Yuan
      4. HBase-10452-trunk-v4.patch
        10 kB
        Ding Yuan

        Activity

        Hide
        Enis Soztutar added a comment -

        Closing this issue after 0.99.0 release.

        Show
        Enis Soztutar added a comment - Closing this issue after 0.99.0 release.
        Hide
        Hudson added a comment -

        FAILURE: Integrated in HBase-TRUNK-on-Hadoop-1.1 #89 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-1.1/89/)
        HBASE-10452 Fix potential bugs in exception handlers (Ding Yuan) (tedyu: rev 1567979)

        • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java
        • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Get.java
        • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
        • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java
        • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/util/JVM.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyPrefixRegionSplitPolicy.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeRequest.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogReader.java
        Show
        Hudson added a comment - FAILURE: Integrated in HBase-TRUNK-on-Hadoop-1.1 #89 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-1.1/89/ ) HBASE-10452 Fix potential bugs in exception handlers (Ding Yuan) (tedyu: rev 1567979) /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Get.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/util/JVM.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyPrefixRegionSplitPolicy.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeRequest.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogReader.java
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in HBase-0.98-on-Hadoop-1.1 #144 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/144/)
        HBASE-10452 Fix potential bugs in exception handlers (Ding Yuan) (tedyu: rev 1567980)

        • /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
        Show
        Hudson added a comment - SUCCESS: Integrated in HBase-0.98-on-Hadoop-1.1 #144 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/144/ ) HBASE-10452 Fix potential bugs in exception handlers (Ding Yuan) (tedyu: rev 1567980) /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
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in HBase-TRUNK #4919 (See https://builds.apache.org/job/HBase-TRUNK/4919/)
        HBASE-10452 Fix potential bugs in exception handlers (Ding Yuan) (tedyu: rev 1567979)

        • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java
        • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Get.java
        • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
        • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java
        • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/util/JVM.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyPrefixRegionSplitPolicy.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeRequest.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogReader.java
        Show
        Hudson added a comment - SUCCESS: Integrated in HBase-TRUNK #4919 (See https://builds.apache.org/job/HBase-TRUNK/4919/ ) HBASE-10452 Fix potential bugs in exception handlers (Ding Yuan) (tedyu: rev 1567979) /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Get.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/util/JVM.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyPrefixRegionSplitPolicy.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeRequest.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitRequest.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogReader.java
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in HBase-0.98 #156 (See https://builds.apache.org/job/HBase-0.98/156/)
        HBASE-10452 Fix potential bugs in exception handlers (Ding Yuan) (tedyu: rev 1567980)

        • /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
        Show
        Hudson added a comment - SUCCESS: Integrated in HBase-0.98 #156 (See https://builds.apache.org/job/HBase-0.98/156/ ) HBASE-10452 Fix potential bugs in exception handlers (Ding Yuan) (tedyu: rev 1567980) /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
        Hide
        Ding Yuan added a comment -

        Great! Thank you all!

        Show
        Ding Yuan added a comment - Great! Thank you all!
        Hide
        Ted Yu added a comment -

        Integrated to 0.98 and trunk.

        Thanks for the patch Ding.

        Show
        Ted Yu added a comment - Integrated to 0.98 and trunk. Thanks for the patch Ding.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12628594/HBase-10452-trunk-v4.patch
        against trunk revision .
        ATTACHMENT ID: 12628594

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

        +1 hadoop1.1. The patch compiles against the hadoop 1.1 profile.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 lineLengths. The patch does not introduce lines longer than 100

        -1 site. The patch appears to cause mvn site goal to fail.

        +1 core tests. The patch passed unit tests in .

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12628594/HBase-10452-trunk-v4.patch against trunk revision . ATTACHMENT ID: 12628594 +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop1.1 . The patch compiles against the hadoop 1.1 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 -1 site . The patch appears to cause mvn site goal to fail. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8677//console This message is automatically generated.
        Hide
        Ding Yuan added a comment -

        Attaching v4 to address Andrew's comments.

        Show
        Ding Yuan added a comment - Attaching v4 to address Andrew's comments.
        Hide
        Andrew Purtell added a comment -

        The comment that was moved in ClientScanner needs fix up. Just remove the "Chances are it was just an UnknownScanner exception due to lease time out." part. Can be done on commit.

        For the, change in HBaseConfiguration, we should log the details of the caught exception, not a hardcoded string.

        Show
        Andrew Purtell added a comment - The comment that was moved in ClientScanner needs fix up. Just remove the "Chances are it was just an UnknownScanner exception due to lease time out." part. Can be done on commit. For the, change in HBaseConfiguration, we should log the details of the caught exception, not a hardcoded string.
        Hide
        Ted Yu added a comment -

        Andrew Purtell:
        What do you think of patch v3 ?

        Show
        Ted Yu added a comment - Andrew Purtell : What do you think of patch v3 ?
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12627964/HBase-10452-trunk-v3.patch
        against trunk revision .
        ATTACHMENT ID: 12627964

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

        +1 hadoop1.1. The patch compiles against the hadoop 1.1 profile.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 lineLengths. The patch does not introduce lines longer than 100

        -1 site. The patch appears to cause mvn site goal to fail.

        +1 core tests. The patch passed unit tests in .

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12627964/HBase-10452-trunk-v3.patch against trunk revision . ATTACHMENT ID: 12627964 +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop1.1 . The patch compiles against the hadoop 1.1 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 -1 site . The patch appears to cause mvn site goal to fail. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8650//console This message is automatically generated.
        Hide
        Ding Yuan added a comment -

        Thanks for the comments! Attached a new patch to address them. As for the possible integer overflow error from TimeRange: an IOException instead of RuntimeException is now thrown so the upper levels will deal with it. Let me know if this is fine.

        Show
        Ding Yuan added a comment - Thanks for the comments! Attached a new patch to address them. As for the possible integer overflow error from TimeRange: an IOException instead of RuntimeException is now thrown so the upper levels will deal with it. Let me know if this is fine.
        Hide
        stack added a comment -

        Thanks for looking into this Ding Yuan All looks good to me caveat what Andy says – just abort if you can't release the lock (something is really wrong if you can't release a lock).

        Show
        stack added a comment - Thanks for looking into this Ding Yuan All looks good to me caveat what Andy says – just abort if you can't release the lock (something is really wrong if you can't release a lock).
        Hide
        Andrew Purtell added a comment -

        Adding error logging is fine.

        Throwing RuntimeExceptions is borderline. I would prefer you just add more error logging in those places.

        The changes to RegionMergeRequest and SplitRequest are not good. Don't retry with a hardcoded and arbitrary strategy. If it is important enough to abort, just abort.

        Show
        Andrew Purtell added a comment - Adding error logging is fine. Throwing RuntimeExceptions is borderline. I would prefer you just add more error logging in those places. The changes to RegionMergeRequest and SplitRequest are not good. Don't retry with a hardcoded and arbitrary strategy. If it is important enough to abort, just abort.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12626957/HBase-10452-trunk-v2.patch
        against trunk revision .
        ATTACHMENT ID: 12626957

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

        +1 hadoop1.1. The patch compiles against the hadoop 1.1 profile.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 lineLengths. The patch does not introduce lines longer than 100

        +1 site. The mvn site goal succeeds with this patch.

        +1 core tests. The patch passed unit tests in .

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12626957/HBase-10452-trunk-v2.patch against trunk revision . ATTACHMENT ID: 12626957 +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop1.1 . The patch compiles against the hadoop 1.1 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8595//console This message is automatically generated.
        Hide
        Ding Yuan added a comment -

        Thanks Ted! Fixed both the line wrapping and the compilation error on java 6.

        Show
        Ding Yuan added a comment - Thanks Ted! Fixed both the line wrapping and the compilation error on java 6.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12626898/HBase-10452-trunk.patch
        against trunk revision .
        ATTACHMENT ID: 12626898

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        -1 hadoop1.0. The patch failed to compile against the hadoop 1.0 profile.
        Here is snippet of errors:

        [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:2.5.1:compile (default-compile) on project hbase-server: Compilation failure: Compilation failure:
        [ERROR] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogReader.java:[258,12] cannot find symbol
        [ERROR] symbol  : class ReflectiveOperationException
        [ERROR] location: class org.apache.hadoop.hbase.regionserver.wal.SequenceFileLogReader
        [ERROR] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogReader.java:[277,12] cannot find symbol
        [ERROR] symbol  : class ReflectiveOperationException
        --
        org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:2.5.1:compile (default-compile) on project hbase-server: Compilation failure
        	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:213)
        	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:153)
        	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:145)
        	at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:84)
        	at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:59)
        --
        Caused by: org.apache.maven.plugin.CompilationFailureException: Compilation failure
        	at org.apache.maven.plugin.AbstractCompilerMojo.execute(AbstractCompilerMojo.java:729)
        	at org.apache.maven.plugin.CompilerMojo.execute(CompilerMojo.java:128)
        	at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:101)
        	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:209)
        	... 19 more

        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8590//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12626898/HBase-10452-trunk.patch against trunk revision . ATTACHMENT ID: 12626898 +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 hadoop1.0 . The patch failed to compile against the hadoop 1.0 profile. Here is snippet of errors: [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:2.5.1:compile ( default -compile) on project hbase-server: Compilation failure: Compilation failure: [ERROR] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogReader.java:[258,12] cannot find symbol [ERROR] symbol : class ReflectiveOperationException [ERROR] location: class org.apache.hadoop.hbase.regionserver.wal.SequenceFileLogReader [ERROR] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogReader.java:[277,12] cannot find symbol [ERROR] symbol : class ReflectiveOperationException -- org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:2.5.1:compile ( default -compile) on project hbase-server: Compilation failure at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:213) at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:153) at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:145) at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:84) at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:59) -- Caused by: org.apache.maven.plugin.CompilationFailureException: Compilation failure at org.apache.maven.plugin.AbstractCompilerMojo.execute(AbstractCompilerMojo.java:729) at org.apache.maven.plugin.CompilerMojo.execute(CompilerMojo.java:128) at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:101) at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:209) ... 19 more Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8590//console This message is automatically generated.
        Hide
        Ted Yu added a comment -

        nit:

        +      throw new RuntimeException("TimeRange failed, likely integer overflow. Preventing bad things to propagate.. ", e);
        

        Please wrap long line - 100 characters limit.

        Show
        Ted Yu added a comment - nit: + throw new RuntimeException( "TimeRange failed, likely integer overflow. Preventing bad things to propagate.. " , e); Please wrap long line - 100 characters limit.
        Hide
        Ding Yuan added a comment -

        Thanks Ram! Attaching a patch against trunk. I did not fix "case 7" since it requires using HBCK to clear the node. I have little expertise in HBase code base and any of my attempt in this case is likely to do more harm than good. Again, any comment is much appreciated!

        Show
        Ding Yuan added a comment - Thanks Ram! Attaching a patch against trunk. I did not fix "case 7" since it requires using HBCK to clear the node. I have little expertise in HBase code base and any of my attempt in this case is likely to do more harm than good. Again, any comment is much appreciated!
        Hide
        ramkrishna.s.vasudevan added a comment -

        Thanks for checking this. It would be better if you create a patch with the above mentioned changes.
        http://hbase.apache.org/book.html#submitting.patches
        If you need to know how to start contributing.

        Show
        ramkrishna.s.vasudevan added a comment - Thanks for checking this. It would be better if you create a patch with the above mentioned changes. http://hbase.apache.org/book.html#submitting.patches If you need to know how to start contributing.

          People

          • Assignee:
            Ding Yuan
            Reporter:
            Ding Yuan
          • Votes:
            0 Vote for this issue
            Watchers:
            9 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development