Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.95.2
    • Fix Version/s: 0.95.0
    • Component/s: scripts
    • Labels:
    • Hadoop Flags:
      Reviewed
    1. Hbase-5654.patch
      13 kB
      Ashutosh Jindal
    2. Hbase_5654_V2.patch
      19 kB
      Ashutosh Jindal
    3. Hbase 5654_v3.patch
      19 kB
      Ashutosh Jindal
    4. hbase-5654-tweak.patch
      20 kB
      Jonathan Hsieh
    5. hbase-5654_v5.patch
      20 kB
      Jonathan Hsieh

      Activity

      Hide
      stack added a comment -

      Marking closed.

      Show
      stack added a comment - Marking closed.
      Hide
      ramkrishna.s.vasudevan added a comment -

      Yes Jon. You can see the latest patch uploaded in that defect? This is how it has been handled in the latest patch

      case RS_ZK_REGION_SPLITTING:
      +        LOG.debug("Processed region in state : " + et);
      +        break;
      +      case RS_ZK_REGION_SPLIT:
      +        LOG.debug("Processed region in state : " + et);
      +        break;
             default:
      -        throw new IllegalStateException("Received event is not valid.");
      +        throw new IllegalStateException("Received region in state :" + et + " is not valid");
      
      Show
      ramkrishna.s.vasudevan added a comment - Yes Jon. You can see the latest patch uploaded in that defect? This is how it has been handled in the latest patch case RS_ZK_REGION_SPLITTING: + LOG.debug( "Processed region in state : " + et); + break ; + case RS_ZK_REGION_SPLIT: + LOG.debug( "Processed region in state : " + et); + break ; default : - throw new IllegalStateException( "Received event is not valid." ); + throw new IllegalStateException( "Received region in state :" + et + " is not valid" );
      Hide
      Jonathan Hsieh added a comment -

      Ram - are you going to handle the fix for this in HBASE-5806?

      Show
      Jonathan Hsieh added a comment - Ram - are you going to handle the fix for this in HBASE-5806 ?
      Hide
      ramkrishna.s.vasudevan added a comment -

      Because RS_ZK_REGION_SPLIT and RS_ZK_REGION_SPLITTING will not be handled in processRIT.
      I think in that case we can just say break in default.

      Show
      ramkrishna.s.vasudevan added a comment - Because RS_ZK_REGION_SPLIT and RS_ZK_REGION_SPLITTING will not be handled in processRIT. I think in that case we can just say break in default.
      Hide
      ramkrishna.s.vasudevan added a comment -

      Default should not throw illegal argument exception in processRIT on master failover in trunk. Found it when a test case was written for HBASE-5806.

      Show
      ramkrishna.s.vasudevan added a comment - Default should not throw illegal argument exception in processRIT on master failover in trunk. Found it when a test case was written for HBASE-5806 .
      Hide
      Hudson added a comment -

      Integrated in HBase-TRUNK-security #181 (See https://builds.apache.org/job/HBase-TRUNK-security/181/)
      HBASE-5654 [findbugs] Address dodgy bugs (Ashutosh Jindal) (Revision 1329132)

      Result = FAILURE
      jmhsieh :
      Files :

      • /hbase/trunk/dev-support/findbugs-exclude.xml
      • /hbase/trunk/dev-support/test-patch.properties
      • /hbase/trunk/src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/HServerLoad.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/filter/FilterList.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/IncreasingToUpperBoundRegionSplitPolicy.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreLAB.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionProgress.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/ByteBloomFilter.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/CompoundBloomFilterWriter.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/FSHDFSUtils.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/PoolMap.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/RegionSplitter.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
      Show
      Hudson added a comment - Integrated in HBase-TRUNK-security #181 (See https://builds.apache.org/job/HBase-TRUNK-security/181/ ) HBASE-5654 [findbugs] Address dodgy bugs (Ashutosh Jindal) (Revision 1329132) Result = FAILURE jmhsieh : Files : /hbase/trunk/dev-support/findbugs-exclude.xml /hbase/trunk/dev-support/test-patch.properties /hbase/trunk/src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon /hbase/trunk/src/main/java/org/apache/hadoop/hbase/HServerLoad.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/filter/FilterList.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/IncreasingToUpperBoundRegionSplitPolicy.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreLAB.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionProgress.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/ByteBloomFilter.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/CompoundBloomFilterWriter.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/FSHDFSUtils.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/PoolMap.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/RegionSplitter.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
      Hide
      Ashutosh Jindal added a comment -

      @Jon
      Thanks for your review and patch you attached for the remaining bugs.

      Show
      Ashutosh Jindal added a comment - @Jon Thanks for your review and patch you attached for the remaining bugs.
      Hide
      Hudson added a comment -

      Integrated in HBase-TRUNK #2797 (See https://builds.apache.org/job/HBase-TRUNK/2797/)
      HBASE-5654 [findbugs] Address dodgy bugs (Ashutosh Jindal) (Revision 1329132)

      Result = FAILURE
      jmhsieh :
      Files :

      • /hbase/trunk/dev-support/findbugs-exclude.xml
      • /hbase/trunk/dev-support/test-patch.properties
      • /hbase/trunk/src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/HServerLoad.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/filter/FilterList.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/IncreasingToUpperBoundRegionSplitPolicy.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreLAB.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionProgress.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/ByteBloomFilter.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/CompoundBloomFilterWriter.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/FSHDFSUtils.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/PoolMap.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/RegionSplitter.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
      Show
      Hudson added a comment - Integrated in HBase-TRUNK #2797 (See https://builds.apache.org/job/HBase-TRUNK/2797/ ) HBASE-5654 [findbugs] Address dodgy bugs (Ashutosh Jindal) (Revision 1329132) Result = FAILURE jmhsieh : Files : /hbase/trunk/dev-support/findbugs-exclude.xml /hbase/trunk/dev-support/test-patch.properties /hbase/trunk/src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon /hbase/trunk/src/main/java/org/apache/hadoop/hbase/HServerLoad.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/filter/FilterList.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/IncreasingToUpperBoundRegionSplitPolicy.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreLAB.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionProgress.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/ByteBloomFilter.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/CompoundBloomFilterWriter.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/FSHDFSUtils.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/PoolMap.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/RegionSplitter.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
      Hide
      Jonathan Hsieh added a comment -

      There are a few new issues in this category so I've filed follow on Jiras (HBASE-5854, HBASE-5855) for them.

      The findbugs count on the previous run was 523 so I've modified the patch to change to that number.

      Committed to trunk/0.96. Thanks Ashutosh!

      Show
      Jonathan Hsieh added a comment - There are a few new issues in this category so I've filed follow on Jiras ( HBASE-5854 , HBASE-5855 ) for them. The findbugs count on the previous run was 523 so I've modified the patch to change to that number. Committed to trunk/0.96. Thanks Ashutosh!
      Hide
      Hadoop QA added a comment -

      +1 overall. Here are the results of testing the latest attachment
      http://issues.apache.org/jira/secure/attachment/12523764/hbase-5654-tweak.patch
      against trunk revision .

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

      +1 tests included. The patch appears to include 3 new or modified tests.

      +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 core tests. The patch passed unit tests in .

      Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1605//testReport/
      Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1605//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
      Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1605//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/12523764/hbase-5654-tweak.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +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 core tests. The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1605//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1605//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1605//console This message is automatically generated.
      Hide
      Jonathan Hsieh added a comment -

      @Ashotosh - sorry for the delay on review. Looks good.

      I believe this isn't needed - looks like it was fixed in code. I'll submit a new version of the patch with this excluded and to get the new findbug numbers, and commit.

      + <Match>
      +       <Class name="org.apache.hadoop.hbase.regionserver.StoreFile$Comparators$1"/>
      +	   <Or>
      +         <Method name="apply" />
      +       </Or>
      +       <Bug pattern="NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE" />
      +     </Match>
      
      Show
      Jonathan Hsieh added a comment - @Ashotosh - sorry for the delay on review. Looks good. I believe this isn't needed - looks like it was fixed in code. I'll submit a new version of the patch with this excluded and to get the new findbug numbers, and commit. + <Match> + < Class name= "org.apache.hadoop.hbase.regionserver.StoreFile$Comparators$1" /> + <Or> + <Method name= "apply" /> + </Or> + <Bug pattern= "NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE" /> + </Match>
      Hide
      Jonathan Hsieh added a comment -

      @stack. Got it. Thanks.

      Show
      Jonathan Hsieh added a comment - @stack. Got it. Thanks.
      Hide
      stack added a comment -

      @Jon Our hadoopqa has been hanging with a while. Its probably not this patch. Maybe compare to previous runs. I'm working on trying to figure out why the hangs meantime.

      Show
      stack added a comment - @Jon Our hadoopqa has been hanging with a while. Its probably not this patch. Maybe compare to previous runs. I'm working on trying to figure out why the hangs meantime.
      Hide
      Jonathan Hsieh added a comment -

      -1 core tests. The patch failed these unit tests:

      This usually indicates that there was a hung test somewhere – there is a script in ./dev-* that should help you find it.

      Show
      Jonathan Hsieh added a comment - -1 core tests. The patch failed these unit tests: This usually indicates that there was a hung test somewhere – there is a script in ./dev-* that should help you find it.
      Hide
      Hadoop QA added a comment -

      -1 overall. Here are the results of testing the latest attachment
      http://issues.apache.org/jira/secure/attachment/12523367/Hbase+5654_v3.patch
      against trunk revision .

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

      +1 tests included. The patch appears to include 3 new or modified tests.

      +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 core tests. The patch failed these unit tests:

      Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1576//testReport/
      Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1576//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
      Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1576//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/12523367/Hbase+5654_v3.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +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 core tests. The patch failed these unit tests: Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1576//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1576//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1576//console This message is automatically generated.
      Hide
      Ashutosh Jindal added a comment -

      Attached an updated patch with all the bugs fixed.Please review and provide your comments/suggestion.

      Show
      Ashutosh Jindal added a comment - Attached an updated patch with all the bugs fixed.Please review and provide your comments/suggestion.
      Hide
      Hadoop QA added a comment -

      -1 overall. Here are the results of testing the latest attachment
      http://issues.apache.org/jira/secure/attachment/12523209/Hbase_5654_V2.patch
      against trunk revision .

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

      +1 tests included. The patch appears to include 3 new or modified tests.

      +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 core tests. The patch failed these unit tests:
      org.apache.hadoop.hbase.regionserver.TestServerCustomProtocol

      Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1567//testReport/
      Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1567//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
      Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1567//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/12523209/Hbase_5654_V2.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +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 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.regionserver.TestServerCustomProtocol Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1567//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1567//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1567//console This message is automatically generated.
      Hide
      Ashutosh Jindal added a comment -

      Submitted the updated patch. Please review and provide your comments/suggestions.

      Show
      Ashutosh Jindal added a comment - Submitted the updated patch. Please review and provide your comments/suggestions.
      Hide
      Jonathan Hsieh added a comment -

      There are a few match related entries:

      Here's how one could likely be fixed.

        public static long calculateOverhead(long maxSize, long blockSize, int concurrency){
          // FindBugs ICAST_INTEGER_MULTIPLY_CAST_TO_LONG
          return CACHE_FIXED_OVERHEAD + ClassSize.CONCURRENT_HASHMAP +
              ((long)Math.ceil(maxSize*1.2/blockSize)
                  * ClassSize.CONCURRENT_HASHMAP_ENTRY) +
            //  (concurrency * ClassSize.CONCURRENT_HASHMAP_SEGMENT);
            ((long)concurrency * (long)ClassSize.CONCURRENT_HASHMAP_SEGMENT);
        }
      

      For bounds checks, I think there was another where a double gets converted to a long (what happens is double is larger than max long?)

      Show
      Jonathan Hsieh added a comment - There are a few match related entries: Here's how one could likely be fixed. public static long calculateOverhead( long maxSize, long blockSize, int concurrency){ // FindBugs ICAST_INTEGER_MULTIPLY_CAST_TO_LONG return CACHE_FIXED_OVERHEAD + ClassSize.CONCURRENT_HASHMAP + (( long ) Math .ceil(maxSize*1.2/blockSize) * ClassSize.CONCURRENT_HASHMAP_ENTRY) + // (concurrency * ClassSize.CONCURRENT_HASHMAP_SEGMENT); (( long )concurrency * ( long )ClassSize.CONCURRENT_HASHMAP_SEGMENT); } For bounds checks, I think there was another where a double gets converted to a long (what happens is double is larger than max long?)
      Hide
      Ashutosh Jindal added a comment -

      @Jon

      add bounds checks? Result of integer multiplication cast to long in org.apache.hadoop.hbase.io.hfile.LruBlockCache.calculateOverhead(long, long, int)

      Result of integer multiplication cast to long in org.apache.hadoop.hbase.regionserver.IncreasingToUpperBoundRegionSplitPolicy.getSizeToCheck(int)

      I am not getting what kind of bound checks can be added here. Can u please take a look.

      Show
      Ashutosh Jindal added a comment - @Jon add bounds checks? Result of integer multiplication cast to long in org.apache.hadoop.hbase.io.hfile.LruBlockCache.calculateOverhead(long, long, int) Result of integer multiplication cast to long in org.apache.hadoop.hbase.regionserver.IncreasingToUpperBoundRegionSplitPolicy.getSizeToCheck(int) I am not getting what kind of bound checks can be added here. Can u please take a look.
      Hide
      Ashutosh Jindal added a comment -

      @Jonathan Hsieh
      Thanks for reviewing the patch. I am going through the comments and will submit an updated patch.

      Show
      Ashutosh Jindal added a comment - @Jonathan Hsieh Thanks for reviewing the patch. I am going through the comments and will submit an updated patch.
      Hide
      Jonathan Hsieh added a comment -

      I'm being pretty strict with excludes – for an excludes it should be explained (because they will be ignored forever more.. )

      +	 <Match>
      +       <Class name="org.apache.hadoop.hbase.regionserver.HRegion"/>
      +	   <Or>
      +         <Method name="exec" />
      +       </Or>
      +       <Bug pattern="NP_LOAD_OF_KNOWN_NULL_VALUE" />
      +     </Match>
      

      I think this one may have been handled in anther patch – maybe this exclude isn't needed.

      +	 <Match>
      +       <Class name="org.apache.hadoop.hbase.regionserver.StoreFile$Reader"/>
      +	   <Or>
      +         <Method name="passesGeneralBloomFilter" />
      +       </Or>
      +       <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE" />
      +     </Match>
      

      This one is definitely fixable in code instead of exclude.

      
      +	 <Match>
      +       <Class name="org.apache.hadoop.hbase.filter.FilterList"/>
      +	   <Or>
      +         <Method name="filterKeyValue" />
      +       </Or>
      +       <Bug pattern="SF_SWITCH_NO_DEFAULT" />
      +     </Match>
      +
      ....
      
      +
      +	 <Match>
      +       <Class name="org.apache.hadoop.hbase.master.AssignmentManager"/>
      +	   <Or>
      +         <Method name="processRegionsInTransition" />
      +		 <Method name="handleRegion" />
      +       </Or>
      +       <Bug pattern="SF_SWITCH_NO_DEFAULT" />
      +     </Match>
      +
      ...
      +
      +	 <Match>
      +       <Class name="org.apache.hadoop.hbase.master.AssignmentManager$TimeoutMonitor"/>
      +	   <Or>
      +         <Method name="actOnTimeOut" />
      +       </Or>
      +       <Bug pattern="SF_SWITCH_NO_DEFAULT" />
      +     </Match>
      +
      ...
      +
      +	 <Match>
      +       <Class name="org.apache.hadoop.hbase.master.AssignmentManager$TimeoutMonitor"/>
      +	   <Or>
      +         <Method name="actOnTimeOut" />
      +       </Or>
      +       <Bug pattern="SF_SWITCH_NO_DEFAULT" />
      +     </Match>
      +
      ...
      +     <Match>
      +       <Class name="org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher"/>
      +	   <Or>
      +         <Method name="connectionEvent" />
      +       </Or>
      +       <Bug pattern="SF_SWITCH_NO_DEFAULT" />
      +     </Match>
       
      

      Definitely fixable in code without exclude – (add default case that does nothing or enumrate all cases and have default throws some sort of runtime exception such as IllegalStateException to catch cases where new enums created).

           <Match>
      +       <Class name="org.apache.hadoop.hbase.util.HBaseFsck"/>
      +	   <Or>
      +         <Method name="setSummary" />
      +         <Method name="setDisplayFullReport" />
      +       </Or>
      +       <Bug pattern="ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD" />
      +     </Match>
      +
      

      File a separate jira for this one and I'll take care of it. Please to do not exclude this warning.

      	 <Match>
      +       <Class name=" org.apache.hadoop.hbase.regionserver.metrics"/>
      +	   <Or>
      +         <Method name="SchemaConfigured" />
      +       </Or>
      +       <Bug pattern="NP_LOAD_OF_KNOWN_NULL_VALUE" />
      +     </Match>
      +
      +	 <Match>
              <Class name="org.apache.hadoop.hbase.regionserver.HRegion"/>
      

      Does this do anything? metrics is a package, SchemaConfigured is a class...

      +
      +	 <Match>
      +       <Class name="org.apache.hadoop.hbase.util.ByteBloomFilter"/>
      +	   <Or>
      +         <Method name="optimalFunctionCount" />
      +       </Or>
      +       <Bug pattern="ICAST_IDIV_CAST_TO_DOUBLE" />
      +     </Match>
      +
      

      Probably wants bitSize and maxKeys to be cased to doubles before div.

      Show
      Jonathan Hsieh added a comment - I'm being pretty strict with excludes – for an excludes it should be explained (because they will be ignored forever more.. ) + <Match> + < Class name= "org.apache.hadoop.hbase.regionserver.HRegion" /> + <Or> + <Method name= "exec" /> + </Or> + <Bug pattern= "NP_LOAD_OF_KNOWN_NULL_VALUE" /> + </Match> I think this one may have been handled in anther patch – maybe this exclude isn't needed. + <Match> + < Class name= "org.apache.hadoop.hbase.regionserver.StoreFile$Reader" /> + <Or> + <Method name= "passesGeneralBloomFilter" /> + </Or> + <Bug pattern= "RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE" /> + </Match> This one is definitely fixable in code instead of exclude. + <Match> + < Class name= "org.apache.hadoop.hbase.filter.FilterList" /> + <Or> + <Method name= "filterKeyValue" /> + </Or> + <Bug pattern= "SF_SWITCH_NO_DEFAULT" /> + </Match> + .... + + <Match> + < Class name= "org.apache.hadoop.hbase.master.AssignmentManager" /> + <Or> + <Method name= "processRegionsInTransition" /> + <Method name= "handleRegion" /> + </Or> + <Bug pattern= "SF_SWITCH_NO_DEFAULT" /> + </Match> + ... + + <Match> + < Class name= "org.apache.hadoop.hbase.master.AssignmentManager$TimeoutMonitor" /> + <Or> + <Method name= "actOnTimeOut" /> + </Or> + <Bug pattern= "SF_SWITCH_NO_DEFAULT" /> + </Match> + ... + + <Match> + < Class name= "org.apache.hadoop.hbase.master.AssignmentManager$TimeoutMonitor" /> + <Or> + <Method name= "actOnTimeOut" /> + </Or> + <Bug pattern= "SF_SWITCH_NO_DEFAULT" /> + </Match> + ... + <Match> + < Class name= "org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher" /> + <Or> + <Method name= "connectionEvent" /> + </Or> + <Bug pattern= "SF_SWITCH_NO_DEFAULT" /> + </Match> Definitely fixable in code without exclude – (add default case that does nothing or enumrate all cases and have default throws some sort of runtime exception such as IllegalStateException to catch cases where new enums created). <Match> + < Class name= "org.apache.hadoop.hbase.util.HBaseFsck" /> + <Or> + <Method name= "setSummary" /> + <Method name= "setDisplayFullReport" /> + </Or> + <Bug pattern= "ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD" /> + </Match> + File a separate jira for this one and I'll take care of it. Please to do not exclude this warning. <Match> + < Class name= " org.apache.hadoop.hbase.regionserver.metrics" /> + <Or> + <Method name= "SchemaConfigured" /> + </Or> + <Bug pattern= "NP_LOAD_OF_KNOWN_NULL_VALUE" /> + </Match> + + <Match> < Class name= "org.apache.hadoop.hbase.regionserver.HRegion" /> Does this do anything? metrics is a package, SchemaConfigured is a class... + + <Match> + < Class name= "org.apache.hadoop.hbase.util.ByteBloomFilter" /> + <Or> + <Method name= "optimalFunctionCount" /> + </Or> + <Bug pattern= "ICAST_IDIV_CAST_TO_DOUBLE" /> + </Match> + Probably wants bitSize and maxKeys to be cased to doubles before div.
      Hide
      Jonathan Hsieh added a comment -

      Ashutosh: Thanks for taking this on.

      I'm about half way through the excludes. Just starting the INT category.

      Questions:

      • why no fix?: WritableRcpEngine#getProxy
      • why no fix?: Ambiguous invocations in RounRobinPool/ReusablePool (specify scope of get() by changing to this.get() or ReusablePool.this.get())
      • add bounds checks? Result of integer multiplication cast to long in org.apache.hadoop.hbase.io.hfile.LruBlockCache.calculateOverhead(long, long, int)
        Result of integer multiplication cast to long in org.apache.hadoop.hbase.regionserver.IncreasingToUpperBoundRegionSplitPolicy.getSizeToCheck(int)
      Show
      Jonathan Hsieh added a comment - Ashutosh: Thanks for taking this on. I'm about half way through the excludes. Just starting the INT category. Questions: why no fix?: WritableRcpEngine#getProxy why no fix?: Ambiguous invocations in RounRobinPool/ReusablePool (specify scope of get() by changing to this.get() or ReusablePool.this.get()) add bounds checks? Result of integer multiplication cast to long in org.apache.hadoop.hbase.io.hfile.LruBlockCache.calculateOverhead(long, long, int) Result of integer multiplication cast to long in org.apache.hadoop.hbase.regionserver.IncreasingToUpperBoundRegionSplitPolicy.getSizeToCheck(int)
      Hide
      Ashutosh Jindal added a comment -

      @Uma
      Thank you for your review. I will look into the remaining bugs and submit the updated patch .

      Show
      Ashutosh Jindal added a comment - @Uma Thank you for your review. I will look into the remaining bugs and submit the updated patch .
      Hide
      Uma Maheswara Rao G added a comment -

      Hey Ashutosh,

      Looks there are some more(3) dodgy category bugs in report.
      Also current bugs count in the report is lesser than the count updated in test-patch,properties.

      For the comment in RSStatusTmplImpl, you may have to change in RSStatusTmpl.jamon file.

      Show
      Uma Maheswara Rao G added a comment - Hey Ashutosh, Looks there are some more(3) dodgy category bugs in report. Also current bugs count in the report is lesser than the count updated in test-patch,properties. For the comment in RSStatusTmplImpl, you may have to change in RSStatusTmpl.jamon file.
      Hide
      Hadoop QA added a comment -

      -1 overall. Here are the results of testing the latest attachment
      http://issues.apache.org/jira/secure/attachment/12522672/Hbase-5654.patch
      against trunk revision .

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

      +1 tests included. The patch appears to include 3 new or modified tests.

      +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 core tests. The patch failed these unit tests:
      org.apache.hadoop.hbase.regionserver.TestServerCustomProtocol

      Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1524//testReport/
      Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1524//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
      Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1524//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/12522672/Hbase-5654.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +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 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.regionserver.TestServerCustomProtocol Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1524//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1524//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1524//console This message is automatically generated.
      Hide
      Ashutosh Jindal added a comment -

      Uploaded the patch.

      Show
      Ashutosh Jindal added a comment - Uploaded the patch.
      Hide
      Jonathan Hsieh added a comment -

      Hi Ashutosh, I don't see an attachement – could you attach one that we could take a look at? Thanks!

      Show
      Jonathan Hsieh added a comment - Hi Ashutosh, I don't see an attachement – could you attach one that we could take a look at? Thanks!
      Hide
      Ashutosh Jindal added a comment -

      Patch submitted for findbugs . Please review and provide comments and suggestions

      Show
      Ashutosh Jindal added a comment - Patch submitted for findbugs . Please review and provide comments and suggestions

        People

        • Assignee:
          Ashutosh Jindal
          Reporter:
          Jonathan Hsieh
        • Votes:
          0 Vote for this issue
          Watchers:
          6 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development