HBase
  1. HBase
  2. HBASE-5491

Remove HBaseConfiguration.create() call from coprocessor.Exec class

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.92.0
    • Fix Version/s: 0.92.1
    • Component/s: Coprocessors
    • Labels:
      None
    • Environment:

      all

    • Hadoop Flags:
      Reviewed

      Description

      Exec class has a field: "private Configuration conf = HBaseConfiguration.create()"

      Client side generates an Exec instance of the class, each initiated Statistics request by ExecRPCInvoker
      Is so HBaseConfiguration.create for each request needs to call

      When the server side deserialize the Exec Called once HBaseConfiguration.create in,
      HBaseConfiguration.create is a time consuming operation.

      "private Configuration conf = HBaseConfiguration.create()";
      This code is only useful for testing code (org.apache.hadoop.hbase.coprocessor.TestCoprocessorEndpoint.testExecDeserialization),
      other places with the Exec class, pass a Configuration come,
      so no need to "conf" field a default value.

      1. HBASE-5491.patch
        2 kB
        honghua zhu
      2. HBASE-5491-2.patch
        2 kB
        honghua zhu

        Activity

        Hide
        stack added a comment -

        +1 on patch. Will add comment that setConf is for testing only on commit. Waiting on hadoopqa before committing.

        Show
        stack added a comment - +1 on patch. Will add comment that setConf is for testing only on commit. Waiting on hadoopqa before committing.
        Hide
        stack added a comment -

        Although, one question Honghua, why not remove the setConf and in the test do new Exec(HBaseConfiguration.create())?

        Show
        stack added a comment - Although, one question Honghua, why not remove the setConf and in the test do new Exec(HBaseConfiguration.create())?
        Hide
        honghua zhu added a comment -

        @stack

        I just do not want to repair a lot of code.

        Show
        honghua zhu added a comment - @stack I just do not want to repair a lot of code.
        Hide
        Ted Yu added a comment - - edited

        The default Exec ctor is only used by TestCoprocessorEndpoint
        I think we shouldn't introduce public method(s) just for unit tests.
        Stack's last comment makes sense.

        @Honghua:
        Can you attach a new patch ?

        Show
        Ted Yu added a comment - - edited The default Exec ctor is only used by TestCoprocessorEndpoint I think we shouldn't introduce public method(s) just for unit tests. Stack's last comment makes sense. @Honghua: Can you attach a new patch ?
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12516576/HBASE-5491.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 appears to have generated -131 warning messages.

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

        -1 findbugs. The patch appears to introduce 155 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.replication.TestReplicationPeer
        org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat
        org.apache.hadoop.hbase.io.hfile.TestLruBlockCache
        org.apache.hadoop.hbase.mapred.TestTableMapReduce
        org.apache.hadoop.hbase.mapreduce.TestImportTsv

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1062//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1062//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1062//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/12516576/HBASE-5491.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 appears to have generated -131 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 155 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.replication.TestReplicationPeer org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat org.apache.hadoop.hbase.io.hfile.TestLruBlockCache org.apache.hadoop.hbase.mapred.TestTableMapReduce org.apache.hadoop.hbase.mapreduce.TestImportTsv Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1062//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1062//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1062//console This message is automatically generated.
        Hide
        Andrew Purtell added a comment -

        +1

        Show
        Andrew Purtell added a comment - +1
        Hide
        stack added a comment -

        @Honghua My suggestion would make your patch smaller.

        Show
        stack added a comment - @Honghua My suggestion would make your patch smaller.
        Hide
        honghua zhu added a comment -

        @stack

        Well, you are right.

        I do not understand the original author why introduced the conf field to Exec class,
        Exec extends org.apache.hadoop.hbase.ipc.Invocation
        where has a conf field too.

        I think that the new Exec(HBaseConfiguration.create()) is not necessary.

        Show
        honghua zhu added a comment - @stack Well, you are right. I do not understand the original author why introduced the conf field to Exec class, Exec extends org.apache.hadoop.hbase.ipc.Invocation where has a conf field too. I think that the new Exec(HBaseConfiguration.create()) is not necessary.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12516630/HBASE-5491-2.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 appears to have generated -131 warning messages.

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

        -1 findbugs. The patch appears to introduce 155 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.mapreduce.TestImportTsv
        org.apache.hadoop.hbase.mapred.TestTableMapReduce
        org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1064//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1064//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1064//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/12516630/HBASE-5491-2.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 appears to have generated -131 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 155 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.mapreduce.TestImportTsv org.apache.hadoop.hbase.mapred.TestTableMapReduce org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1064//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1064//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1064//console This message is automatically generated.
        Hide
        Ted Yu added a comment -

        Patch v2 looks good.

        Show
        Ted Yu added a comment - Patch v2 looks good.
        Hide
        stack added a comment -

        +1

        Show
        stack added a comment - +1
        Hide
        stack added a comment -

        Committed to 0.92, 0.94 and trunk. Thanks for the patch Honghua

        Show
        stack added a comment - Committed to 0.92, 0.94 and trunk. Thanks for the patch Honghua
        Hide
        Hudson added a comment -

        Integrated in HBase-0.94 #5 (See https://builds.apache.org/job/HBase-0.94/5/)
        HBASE-5491 Remove HBaseConfiguration.create() call from coprocessor.Exec class (Revision 1295431)

        Result = SUCCESS
        stack :
        Files :

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/client/coprocessor/Exec.java
        • /hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorEndpoint.java
        Show
        Hudson added a comment - Integrated in HBase-0.94 #5 (See https://builds.apache.org/job/HBase-0.94/5/ ) HBASE-5491 Remove HBaseConfiguration.create() call from coprocessor.Exec class (Revision 1295431) Result = SUCCESS stack : Files : /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/client/coprocessor/Exec.java /hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorEndpoint.java
        Hide
        Hudson added a comment -

        Integrated in HBase-0.92 #309 (See https://builds.apache.org/job/HBase-0.92/309/)
        HBASE-5491 Remove HBaseConfiguration.create() call from coprocessor.Exec class (Revision 1295430)

        Result = SUCCESS
        stack :
        Files :

        • /hbase/branches/0.92/CHANGES.txt
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/client/coprocessor/Exec.java
        • /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorEndpoint.java
        Show
        Hudson added a comment - Integrated in HBase-0.92 #309 (See https://builds.apache.org/job/HBase-0.92/309/ ) HBASE-5491 Remove HBaseConfiguration.create() call from coprocessor.Exec class (Revision 1295430) Result = SUCCESS stack : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/client/coprocessor/Exec.java /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorEndpoint.java
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #2671 (See https://builds.apache.org/job/HBase-TRUNK/2671/)
        HBASE-5491 Remove HBaseConfiguration.create() call from coprocessor.Exec class (Revision 1295429)

        Result = FAILURE
        stack :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/coprocessor/Exec.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorEndpoint.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #2671 (See https://builds.apache.org/job/HBase-TRUNK/2671/ ) HBASE-5491 Remove HBaseConfiguration.create() call from coprocessor.Exec class (Revision 1295429) Result = FAILURE stack : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/coprocessor/Exec.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorEndpoint.java
        Hide
        Hudson added a comment -

        Integrated in HBase-0.92-security #96 (See https://builds.apache.org/job/HBase-0.92-security/96/)
        HBASE-5491 Remove HBaseConfiguration.create() call from coprocessor.Exec class (Revision 1295430)

        Result = FAILURE
        stack :
        Files :

        • /hbase/branches/0.92/CHANGES.txt
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/client/coprocessor/Exec.java
        • /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorEndpoint.java
        Show
        Hudson added a comment - Integrated in HBase-0.92-security #96 (See https://builds.apache.org/job/HBase-0.92-security/96/ ) HBASE-5491 Remove HBaseConfiguration.create() call from coprocessor.Exec class (Revision 1295430) Result = FAILURE stack : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/client/coprocessor/Exec.java /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorEndpoint.java

          People

          • Assignee:
            Unassigned
            Reporter:
            honghua zhu
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development