HBase
  1. HBase
  2. HBASE-5919

Add fixes for Ted's review comments from HBASE-5869

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      I missed addressing a few of Ted's comments on the end of my navigating HBASE-5869 commit. Fix here. Make it a blocker.

      1. 5919-v4.txt
        2 kB
        Ted Yu
      2. 5919-v2.txt
        2 kB
        Ted Yu
      3. 5919.txt
        2 kB
        stack

        Activity

        Hide
        stack added a comment -

        Address issues raised by Ted over in hbase-5869 that I did not want to address there because the criticims' were relatively nits for a patch of > 300k. I'd gotten two clean hadoopqa builds on v13... and was afraid my patch would fail to apply if I did more hadoopqa cycles.

        Show
        stack added a comment - Address issues raised by Ted over in hbase-5869 that I did not want to address there because the criticims' were relatively nits for a patch of > 300k. I'd gotten two clean hadoopqa builds on v13... and was afraid my patch would fail to apply if I did more hadoopqa cycles.
        Hide
        Ted Yu added a comment -
        +    int length = Math.min(len - off, b.length - off);
        

        I think len represents the length to write. So I am not sure why off would come into play above.

        Show
        Ted Yu added a comment - + int length = Math .min(len - off, b.length - off); I think len represents the length to write. So I am not sure why off would come into play above.
        Hide
        stack added a comment -

        Review how this method is called. Look up in the file two methods.

        Show
        stack added a comment - Review how this method is called. Look up in the file two methods.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12525356/5919.txt
        against trunk revision .

        +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 hadoop23. The patch compiles against the hadoop 0.23.x 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 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.master.TestCatalogJanitor
        org.apache.hadoop.hbase.regionserver.TestResettingCounters
        org.apache.hadoop.hbase.regionserver.TestScanWithBloomError
        org.apache.hadoop.hbase.regionserver.TestColumnSeeking
        org.apache.hadoop.hbase.client.TestResult
        org.apache.hadoop.hbase.regionserver.TestStoreFile
        org.apache.hadoop.hbase.filter.TestDependentColumnFilter
        org.apache.hadoop.hbase.regionserver.TestMemStore
        org.apache.hadoop.hbase.io.hfile.TestSeekTo
        org.apache.hadoop.hbase.TestKeyValue
        org.apache.hadoop.hbase.filter.TestFilter
        org.apache.hadoop.hbase.regionserver.TestScanner
        org.apache.hadoop.hbase.TestSerialization
        org.apache.hadoop.hbase.regionserver.TestCompaction

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1730//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1730//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1730//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/12525356/5919.txt against trunk revision . +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 hadoop23. The patch compiles against the hadoop 0.23.x 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 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.master.TestCatalogJanitor org.apache.hadoop.hbase.regionserver.TestResettingCounters org.apache.hadoop.hbase.regionserver.TestScanWithBloomError org.apache.hadoop.hbase.regionserver.TestColumnSeeking org.apache.hadoop.hbase.client.TestResult org.apache.hadoop.hbase.regionserver.TestStoreFile org.apache.hadoop.hbase.filter.TestDependentColumnFilter org.apache.hadoop.hbase.regionserver.TestMemStore org.apache.hadoop.hbase.io.hfile.TestSeekTo org.apache.hadoop.hbase.TestKeyValue org.apache.hadoop.hbase.filter.TestFilter org.apache.hadoop.hbase.regionserver.TestScanner org.apache.hadoop.hbase.TestSerialization org.apache.hadoop.hbase.regionserver.TestCompaction Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1730//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1730//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1730//console This message is automatically generated.
        Hide
        Ted Yu added a comment -

        Patch v2 gets past the following tests:

          789  mt -Dtest=TestScanWithBloomError
          790  mt -Dtest=TestFilter
        
        Show
        Ted Yu added a comment - Patch v2 gets past the following tests: 789 mt -Dtest=TestScanWithBloomError 790 mt -Dtest=TestFilter
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12525361/5919-v2.txt
        against trunk revision .

        +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 hadoop23. The patch compiles against the hadoop 0.23.x 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 core tests. The patch failed these unit tests:

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1732//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1732//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1732//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/12525361/5919-v2.txt against trunk revision . +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 hadoop23. The patch compiles against the hadoop 0.23.x 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 core tests. The patch failed these unit tests: Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1732//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1732//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1732//console This message is automatically generated.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12525382/5919-v4.txt
        against trunk revision .

        +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 hadoop23. The patch compiles against the hadoop 0.23.x 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 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.master.TestAssignmentManager

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1737//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1737//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1737//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/12525382/5919-v4.txt against trunk revision . +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 hadoop23. The patch compiles against the hadoop 0.23.x 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 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.master.TestAssignmentManager Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1737//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1737//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1737//console This message is automatically generated.
        Hide
        Ted Yu added a comment -

        I looped TestAssignmentManager based on patch v4 5 times.
        They passed.

        I think patch v4 should be good to go.

        Show
        Ted Yu added a comment - I looped TestAssignmentManager based on patch v4 5 times. They passed. I think patch v4 should be good to go.
        Hide
        stack added a comment -

        I thought this was my issue but you seem to have taken it over Ted. Assigning you.

        Show
        stack added a comment - I thought this was my issue but you seem to have taken it over Ted. Assigning you.
        Hide
        Ted Yu added a comment -

        @Stack:
        Do you have further comments about patch v4 ?

        Show
        Ted Yu added a comment - @Stack: Do you have further comments about patch v4 ?
        Hide
        stack added a comment -

        None.

        Show
        stack added a comment - None.
        Hide
        Ted Yu added a comment -

        Integrated to trunk.

        Thanks for the review, Stack.

        Show
        Ted Yu added a comment - Integrated to trunk. Thanks for the review, Stack.
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #2841 (See https://builds.apache.org/job/HBase-TRUNK/2841/)
        HBASE-5919 Add fixes for Ted's review comments from HBASE-5869 (Revision 1333304)

        Result = SUCCESS
        tedyu :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/Bytes.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #2841 (See https://builds.apache.org/job/HBase-TRUNK/2841/ ) HBASE-5919 Add fixes for Ted's review comments from HBASE-5869 (Revision 1333304) Result = SUCCESS tedyu : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/Bytes.java
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK-security #190 (See https://builds.apache.org/job/HBase-TRUNK-security/190/)
        HBASE-5919 Add fixes for Ted's review comments from HBASE-5869 (Revision 1333304)

        Result = SUCCESS
        tedyu :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/Bytes.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK-security #190 (See https://builds.apache.org/job/HBase-TRUNK-security/190/ ) HBASE-5919 Add fixes for Ted's review comments from HBASE-5869 (Revision 1333304) Result = SUCCESS tedyu : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/Bytes.java

          People

          • Assignee:
            Ted Yu
            Reporter:
            stack
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development