Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.98.1, 0.99.0
    • Component/s: None
    • Hadoop Flags:
      Reviewed

      Description

      DoubleColumnInterpreter was requested by Royston Sellman.

      1. 5175-v5.txt
        52 kB
        Ted Yu
      2. DoubleColumnInterpreterV4.patch
        52 kB
        Julian Wissmann
      3. DoubleColumnInterpreterV3.patch
        52 kB
        Julian Wissmann
      4. DoubleColumnInterpreterV2.patch
        1.21 MB
        Julian Wissmann
      5. DoubleColumnInterpreter.patch
        3.54 MB
        Julian Wissmann
      6. DoubleColumnInterpreter.java
        3 kB
        Julian Wissmann
      7. TestDoubleColumnInterpreter.java
        23 kB
        Julian Wissmann
      8. HBase.proto
        5 kB
        Julian Wissmann

        Activity

        Ted Yu created issue -
        Julian Wissmann made changes -
        Field Original Value New Value
        Assignee Julian Wissmann [ juwi ]
        Julian Wissmann made changes -
        Labels aggregator features
        Julian Wissmann made changes -
        Fix Version/s 0.99.0 [ 12325675 ]
        Hide
        Julian Wissmann added a comment -

        I started working on implementing this on the trunk as chances are, that I might need this, myself at some point.

        Show
        Julian Wissmann added a comment - I started working on implementing this on the trunk as chances are, that I might need this, myself at some point.
        Hide
        Julian Wissmann added a comment -

        For starters here are the source files I wrote and modified - in case anyone wants to comment.
        I'll get a patch prepared from those some time in the next few days.

        Gegards
        Julian

        Show
        Julian Wissmann added a comment - For starters here are the source files I wrote and modified - in case anyone wants to comment. I'll get a patch prepared from those some time in the next few days. Gegards Julian
        Julian Wissmann made changes -
        Attachment HBase.proto [ 12634261 ]
        Attachment TestDoubleColumnInterpreter.java [ 12634262 ]
        Attachment DoubleColumnInterpreter.java [ 12634263 ]
        Hide
        Ted Yu added a comment -

        For DoubleColumnInterpreter.java :
        please add license header

         * is supposed to instantiate it and passed along as a parameter. See
         * TestAggregateProtocol methods for its sample usage.
        

        You're going to modify TestAggregateProtocol in next patch, right ?

        public class DoubleColumnInterpreter extends ColumnInterpreter<Double, Double, EmptyMsg, DoubleMsg, DoubleMsg>{
        

        Wrap long line.

        Show
        Ted Yu added a comment - For DoubleColumnInterpreter.java : please add license header * is supposed to instantiate it and passed along as a parameter. See * TestAggregateProtocol methods for its sample usage. You're going to modify TestAggregateProtocol in next patch, right ? public class DoubleColumnInterpreter extends ColumnInterpreter< Double , Double , EmptyMsg, DoubleMsg, DoubleMsg>{ Wrap long line.
        Hide
        Julian Wissmann added a comment -

        Patch created

        -Wrapped long lines
        -Fixed Comment in DoubleColumnInterpreter to reference correct test
        -Added License Headers

        Are there any guidelines on formatting Protobuf autogenerated code, correctly? I autoindented that causing the patch to grow unnecessarily big.

        Show
        Julian Wissmann added a comment - Patch created -Wrapped long lines -Fixed Comment in DoubleColumnInterpreter to reference correct test -Added License Headers Are there any guidelines on formatting Protobuf autogenerated code, correctly? I autoindented that causing the patch to grow unnecessarily big.
        Julian Wissmann made changes -
        Attachment DoubleColumnInterpreter.patch [ 12634430 ]
        Hide
        Ted Yu added a comment -

        Diff for HBaseProtos.java seems to be included in the patch more than once:

        From 0b07009f891a73bd9e335bf925d5dfc7054ac8a9 Mon Sep 17 00:00:00 2001
        From: Julian Wissmann <julianwissmann@gmail.com>
        Date: Thu, 13 Mar 2014 13:05:07 +0100
        Subject: [PATCH 2/7] formatted protos
        
        ---
         .../hbase/protobuf/generated/HBaseProtos.java      | 7899 +++++++++++---------
         hbase-protocol/src/main/protobuf/HBase.proto       |    3 +
         2 files changed, 4517 insertions(+), 3385 deletions(-)
        
        diff --git a/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java b/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java
        

        There is no formatting needed, see:

        -// Generated by the protocol buffer compiler.  DO NOT EDIT!
        
        Show
        Ted Yu added a comment - Diff for HBaseProtos.java seems to be included in the patch more than once: From 0b07009f891a73bd9e335bf925d5dfc7054ac8a9 Mon Sep 17 00:00:00 2001 From: Julian Wissmann <julianwissmann@gmail.com> Date: Thu, 13 Mar 2014 13:05:07 +0100 Subject: [PATCH 2/7] formatted protos --- .../hbase/protobuf/generated/HBaseProtos.java | 7899 +++++++++++--------- hbase-protocol/src/main/protobuf/HBase.proto | 3 + 2 files changed, 4517 insertions(+), 3385 deletions(-) diff --git a/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java b/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java There is no formatting needed, see: - // Generated by the protocol buffer compiler. DO NOT EDIT!
        Hide
        Julian Wissmann added a comment -

        Re-Created patch.
        Issues of the previous patch should be fixed.

        Show
        Julian Wissmann added a comment - Re-Created patch. Issues of the previous patch should be fixed.
        Julian Wissmann made changes -
        Attachment DoubleColumnInterpreterV2.patch [ 12634745 ]
        Ted Yu made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Hadoop QA added a comment -

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

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

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

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8998//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/12634745/DoubleColumnInterpreterV2.patch against trunk revision . ATTACHMENT ID: 12634745 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified tests. -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8998//console This message is automatically generated.
        Hide
        Julian Wissmann added a comment -

        Turns out using git to create a patch that svn will swallow isn't a no-brainer.
        My local system accepted it, this time, so I'm confident Jenkins will, too.

        Show
        Julian Wissmann added a comment - Turns out using git to create a patch that svn will swallow isn't a no-brainer. My local system accepted it, this time, so I'm confident Jenkins will, too.
        Julian Wissmann made changes -
        Attachment DoubleColumnInterpreterV3.patch [ 12634930 ]
        Hide
        Hadoop QA added a comment -

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

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

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

        +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 introduces the following lines longer than 100:
        + private DoubleMsg(boolean noInit)

        { this.unknownFields = com.google.protobuf.UnknownFieldSet.getDefaultInstance(); }

        + result = result && (Double.doubleToLongBits(getDoubleMsg()) == Double.doubleToLongBits(other.getDoubleMsg()));

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

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

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//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/12634930/DoubleColumnInterpreterV3.patch against trunk revision . ATTACHMENT ID: 12634930 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified tests. +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 introduces the following lines longer than 100: + private DoubleMsg(boolean noInit) { this.unknownFields = com.google.protobuf.UnknownFieldSet.getDefaultInstance(); } + result = result && (Double.doubleToLongBits(getDoubleMsg()) == Double.doubleToLongBits(other.getDoubleMsg())); +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//console This message is automatically generated.
        Hide
        Ted Yu added a comment - - edited

        Some nits:

        +   @Override
        +  public Double add(Double d1, Double d2) {
        

        @Override should align with the start of 'p' in public.

        +    } else if (d1 == null) // both are null
        +      return null;
        

        'else' is not needed. Please enclose 'return null;' in braces.

        Running org.apache.hadoop.hbase.coprocessor.TestDoubleColumnInterpreter
        Tests run: 38, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 25.34 sec
        

        Good, new test passed.

        +  @Test(timeout = 300000)
        +  public void testMaxWithValidRange2WithNoCQ() throws Throwable {
        

        We don't need such long timeout, right ?

        +    } catch (Throwable e) {
        +    }
        

        Why is Throwable ignored ?
        The long line warnings were for protobuf generated code.

        Show
        Ted Yu added a comment - - edited Some nits: + @Override + public Double add( Double d1, Double d2) { @Override should align with the start of 'p' in public. + } else if (d1 == null ) // both are null + return null ; 'else' is not needed. Please enclose 'return null;' in braces. Running org.apache.hadoop.hbase.coprocessor.TestDoubleColumnInterpreter Tests run: 38, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 25.34 sec Good, new test passed. + @Test(timeout = 300000) + public void testMaxWithValidRange2WithNoCQ() throws Throwable { We don't need such long timeout, right ? + } catch (Throwable e) { + } Why is Throwable ignored ? The long line warnings were for protobuf generated code.
        Hide
        Julian Wissmann added a comment -

        Sorry about the delay!
        New version of patch, fixing most issues.
        However I did not touch the Test timeouts as I assume 300000 to be a sane default value since it is used in TestAggregateProtocol and TestBigDecimalColumnInterpreter.

        Show
        Julian Wissmann added a comment - Sorry about the delay! New version of patch, fixing most issues. However I did not touch the Test timeouts as I assume 300000 to be a sane default value since it is used in TestAggregateProtocol and TestBigDecimalColumnInterpreter.
        Julian Wissmann made changes -
        Attachment DoubleColumnInterpreterV4.patch [ 12636009 ]
        Hide
        Hadoop QA added a comment -

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

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

        +1 tests included. The patch appears to include 2 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 lineLengths. The patch introduces the following lines longer than 100:
        + private DoubleMsg(boolean noInit)

        { this.unknownFields = com.google.protobuf.UnknownFieldSet.getDefaultInstance(); }

        + result = result && (Double.doubleToLongBits(getDoubleMsg()) == Double.doubleToLongBits(other.getDoubleMsg()));

        +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/9063//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//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/12636009/DoubleColumnInterpreterV4.patch against trunk revision . ATTACHMENT ID: 12636009 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 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 lineLengths . The patch introduces the following lines longer than 100: + private DoubleMsg(boolean noInit) { this.unknownFields = com.google.protobuf.UnknownFieldSet.getDefaultInstance(); } + result = result && (Double.doubleToLongBits(getDoubleMsg()) == Double.doubleToLongBits(other.getDoubleMsg())); +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/9063//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//console This message is automatically generated.
        Hide
        Anoop Sam John added a comment -

        Overall looks good.
        Instead of the deprecated method can you implement public T getValue(byte[] colFamily, byte[] colQualifier, Cell c). No diff in impl code needed.

        +   if (d1 == null ^ d2 == null) {
        +      return (d1 == null) ? d2 : d1; // either of one is null.
        +    } 
        +    if (d1 == null) {// both are null
        +      return null;
        +    }
        

        This would be enough I guess.

        +    if (d1 == null || d2 == null) {
        +      return (d1 == null) ? d2 : d1; 
        +    }
        
        Show
        Anoop Sam John added a comment - Overall looks good. Instead of the deprecated method can you implement public T getValue(byte[] colFamily, byte[] colQualifier, Cell c). No diff in impl code needed. + if (d1 == null ^ d2 == null ) { + return (d1 == null ) ? d2 : d1; // either of one is null . + } + if (d1 == null ) { // both are null + return null ; + } This would be enough I guess. + if (d1 == null || d2 == null ) { + return (d1 == null ) ? d2 : d1; + }
        Hide
        Ted Yu added a comment -

        Patch v5 addresses Anoop's comments.

        Show
        Ted Yu added a comment - Patch v5 addresses Anoop's comments.
        Ted Yu made changes -
        Attachment 5175-v5.txt [ 12636453 ]
        Hide
        Julian Wissmann added a comment -

        I was just re-running relevant tests. Looks like you were faster. Thanks for finishing it up.

        Show
        Julian Wissmann added a comment - I was just re-running relevant tests. Looks like you were faster. Thanks for finishing it up.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12636453/5175-v5.txt
        against trunk revision .
        ATTACHMENT ID: 12636453

        +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 lineLengths. The patch introduces the following lines longer than 100:
        + private DoubleMsg(boolean noInit)

        { this.unknownFields = com.google.protobuf.UnknownFieldSet.getDefaultInstance(); }

        + result = result && (Double.doubleToLongBits(getDoubleMsg()) == Double.doubleToLongBits(other.getDoubleMsg()));

        +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/9082//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//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/12636453/5175-v5.txt against trunk revision . ATTACHMENT ID: 12636453 +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 lineLengths . The patch introduces the following lines longer than 100: + private DoubleMsg(boolean noInit) { this.unknownFields = com.google.protobuf.UnknownFieldSet.getDefaultInstance(); } + result = result && (Double.doubleToLongBits(getDoubleMsg()) == Double.doubleToLongBits(other.getDoubleMsg())); +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/9082//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//console This message is automatically generated.
        Hide
        Honghua Feng added a comment -

        +1

        Show
        Honghua Feng added a comment - +1
        Hide
        Anoop Sam John added a comment -

        +1
        Pls fix the line length issue on commit.

        Show
        Anoop Sam John added a comment - +1 Pls fix the line length issue on commit.
        Hide
        Ted Yu added a comment -

        The long lines were from protobuf generated code.

        Show
        Ted Yu added a comment - The long lines were from protobuf generated code.
        Ted Yu made changes -
        Hadoop Flags Reviewed [ 10343 ]
        Fix Version/s 0.98.1 [ 12325664 ]
        Ted Yu made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Anoop Sam John added a comment -

        The long lines were from protobuf generated code.

        !! We have excluded protobuf generated code from this line length checks. This is broken?

        Show
        Anoop Sam John added a comment - The long lines were from protobuf generated code. !! We have excluded protobuf generated code from this line length checks. This is broken?
        Hide
        Ted Yu added a comment -

        Here is the filter for protobuf generated code:

          lines=`cat $PATCH_DIR/patch | grep "^+" | grep -v "^@@" | grep -v "^+++" | grep -v "import" | grep -v "hbase.protobuf.generated" | awk -v len="$MAX_LINE_LENGTH_PATCH"    'length ($0) > len' | head -n 10`
        

        Obviously 'com.google.protobuf.' should be considered as well.
        Let me log another JIRA for enhancement.

        Show
        Ted Yu added a comment - Here is the filter for protobuf generated code: lines=`cat $PATCH_DIR/patch | grep "^+" | grep -v "^@@" | grep -v "^+++" | grep -v " import " | grep -v "hbase.protobuf.generated" | awk -v len= "$MAX_LINE_LENGTH_PATCH" 'length ($0) > len' | head -n 10` Obviously 'com.google.protobuf.' should be considered as well. Let me log another JIRA for enhancement.
        Hide
        Ted Yu added a comment -

        Logged HBASE-10824 for line length check enhancement.

        Show
        Ted Yu added a comment - Logged HBASE-10824 for line length check enhancement.
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in HBase-0.98 #250 (See https://builds.apache.org/job/HBase-0.98/250/)
        HBASE-5175 Add DoubleColumnInterpreter (Julian Wissmann) (tedyu: rev 1581199)

        • /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/coprocessor/DoubleColumnInterpreter.java
        • /hbase/branches/0.98/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java
        • /hbase/branches/0.98/hbase-protocol/src/main/protobuf/HBase.proto
        • /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestDoubleColumnInterpreter.java
        Show
        Hudson added a comment - SUCCESS: Integrated in HBase-0.98 #250 (See https://builds.apache.org/job/HBase-0.98/250/ ) HBASE-5175 Add DoubleColumnInterpreter (Julian Wissmann) (tedyu: rev 1581199) /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/coprocessor/DoubleColumnInterpreter.java /hbase/branches/0.98/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java /hbase/branches/0.98/hbase-protocol/src/main/protobuf/HBase.proto /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestDoubleColumnInterpreter.java
        Hide
        Hudson added a comment -

        ABORTED: Integrated in HBase-TRUNK #5038 (See https://builds.apache.org/job/HBase-TRUNK/5038/)
        HBASE-5175 Add DoubleColumnInterpreter (Julian Wissmann) (tedyu: rev 1581200)

        • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/coprocessor/DoubleColumnInterpreter.java
        • /hbase/trunk/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java
        • /hbase/trunk/hbase-protocol/src/main/protobuf/HBase.proto
        • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestDoubleColumnInterpreter.java
        Show
        Hudson added a comment - ABORTED: Integrated in HBase-TRUNK #5038 (See https://builds.apache.org/job/HBase-TRUNK/5038/ ) HBASE-5175 Add DoubleColumnInterpreter (Julian Wissmann) (tedyu: rev 1581200) /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/coprocessor/DoubleColumnInterpreter.java /hbase/trunk/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java /hbase/trunk/hbase-protocol/src/main/protobuf/HBase.proto /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestDoubleColumnInterpreter.java
        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.
        Enis Soztutar made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Patch Available Patch Available
        793d 19h 52m 1 Ted Yu 14/Mar/14 16:45
        Patch Available Patch Available Resolved Resolved
        10d 10h 28m 1 Ted Yu 25/Mar/14 03:14
        Resolved Resolved Closed Closed
        333d 20h 16m 1 Enis Soztutar 21/Feb/15 23:31

          People

          • Assignee:
            Julian Wissmann
            Reporter:
            Ted Yu
          • Votes:
            1 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development