HBase
  1. HBase
  2. HBASE-9534

Short-Circuit Coprocessor HTable access when on the same server

    Details

    • Release Note:
      Allow coprocessors accessing an HTable to short-circuit access to the local HRegionServer, rather than requiring the usual RPC path.
    • Tags:
      phoenix

      Description

      Coprocessors currently create a full HTable when they want to write. However, we know that coprocessors must run from within an HBase server (either master or RS). For the master, its rare that we are going to be doing performance sensitive operations, but RS calls could be very time-intensive.

      Therefore, we should be able to tell when a call from a CP attempts to talk to the RS on which it lives and just short-circuit to calling that RS, rather than going the long way around (which does the full marshalling/unmarshalling of data, as well as going over the loopback interface).

      1. hbase-9534-trunk-v2.patch
        24 kB
        Jesse Yates
      2. hbase-9534-trunk-v1.patch
        21 kB
        Jesse Yates
      3. hbase-9534-trunk-v0.patch
        22 kB
        Jesse Yates
      4. hbase-9534-0.94-v2.patch
        11 kB
        Jesse Yates
      5. hbase-9534-0.94-v1.patch
        13 kB
        Jesse Yates
      6. hbase-9534-0.94-v0.patch
        18 kB
        Jesse Yates
      7. 9534-trunk.txt
        20 kB
        Lars Hofhansl

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          FAILURE: Integrated in hbase-0.96-hadoop2 #59 (See https://builds.apache.org/job/hbase-0.96-hadoop2/59/)
          HBASE-9658: Addendum to HBASE-9534 to fix 0.96 (jyates: rev 1526334)

          • /hbase/branches/0.96/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java
          Show
          Hudson added a comment - FAILURE: Integrated in hbase-0.96-hadoop2 #59 (See https://builds.apache.org/job/hbase-0.96-hadoop2/59/ ) HBASE-9658 : Addendum to HBASE-9534 to fix 0.96 (jyates: rev 1526334) /hbase/branches/0.96/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java
          Hide
          Lars Hofhansl added a comment -

          Hmm... Sorry about this. I took the latest trunk patch here and committed it to 0.96. Thanks for checking Himanshu.

          Show
          Lars Hofhansl added a comment - Hmm... Sorry about this. I took the latest trunk patch here and committed it to 0.96. Thanks for checking Himanshu.
          Hide
          Hudson added a comment -

          FAILURE: Integrated in hbase-0.96 #95 (See https://builds.apache.org/job/hbase-0.96/95/)
          HBASE-9658: Addendum to HBASE-9534 to fix 0.96 (jyates: rev 1526334)

          • /hbase/branches/0.96/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java
          Show
          Hudson added a comment - FAILURE: Integrated in hbase-0.96 #95 (See https://builds.apache.org/job/hbase-0.96/95/ ) HBASE-9658 : Addendum to HBASE-9534 to fix 0.96 (jyates: rev 1526334) /hbase/branches/0.96/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java
          Hide
          Jesse Yates added a comment -

          Good catch Himanshu. Looks like the wrong version got committed to 0.96. I'll post an addendum shortly.

          Show
          Jesse Yates added a comment - Good catch Himanshu. Looks like the wrong version got committed to 0.96. I'll post an addendum shortly.
          Hide
          Himanshu Vashishtha added a comment -

          Also, another nit is we are missing a null check on conf in 96 in HTable ctr now.

          -    if (conf == null) {
          -      this.connection = null;
          -      return;
          -    } 
          
          Show
          Himanshu Vashishtha added a comment - Also, another nit is we are missing a null check on conf in 96 in HTable ctr now. - if (conf == null ) { - this .connection = null ; - return ; - }
          Hide
          Himanshu Vashishtha added a comment -

          The point is the code is different in 96 and trunk now. Looks like different patches were committed? See ctr chaining in HTable, for example.

          Show
          Himanshu Vashishtha added a comment - The point is the code is different in 96 and trunk now. Looks like different patches were committed? See ctr chaining in HTable, for example.
          Hide
          Jesse Yates added a comment -

          I forgot to do 0.96 (did 0.95 by accident instead) - Lars committed to 0.96 for me.

          Show
          Jesse Yates added a comment - I forgot to do 0.96 (did 0.95 by accident instead) - Lars committed to 0.96 for me.
          Show
          Himanshu Vashishtha added a comment - Guys, I see two "different" commits for this jira in trunk and 96. trunk: https://github.com/apache/hbase/commit/af66c6b28edebdf3beb23628419d5c40cf3ea449 96: https://github.com/apache/hbase/commit/30d685ce750bc7361159d32933e08ac834adec7c
          Hide
          Hudson added a comment -

          FAILURE: Integrated in hbase-0.96-hadoop2 #38 (See https://builds.apache.org/job/hbase-0.96-hadoop2/38/)
          HBASE-9534: Short-Circuit Coprocessor HTable access when on the same server (Jesse) (larsh: rev 1524620)

          • /hbase/branches/0.96/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java
          • /hbase/branches/0.96/hbase-server/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java
          • /hbase/branches/0.96/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          Show
          Hudson added a comment - FAILURE: Integrated in hbase-0.96-hadoop2 #38 (See https://builds.apache.org/job/hbase-0.96-hadoop2/38/ ) HBASE-9534 : Short-Circuit Coprocessor HTable access when on the same server (Jesse) (larsh: rev 1524620) /hbase/branches/0.96/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java /hbase/branches/0.96/hbase-server/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java /hbase/branches/0.96/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in hbase-0.96 #70 (See https://builds.apache.org/job/hbase-0.96/70/)
          HBASE-9534: Short-Circuit Coprocessor HTable access when on the same server (Jesse) (larsh: rev 1524620)

          • /hbase/branches/0.96/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java
          • /hbase/branches/0.96/hbase-server/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java
          • /hbase/branches/0.96/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          Show
          Hudson added a comment - FAILURE: Integrated in hbase-0.96 #70 (See https://builds.apache.org/job/hbase-0.96/70/ ) HBASE-9534 : Short-Circuit Coprocessor HTable access when on the same server (Jesse) (larsh: rev 1524620) /hbase/branches/0.96/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java /hbase/branches/0.96/hbase-server/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java /hbase/branches/0.96/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          Hide
          Lars Hofhansl added a comment -

          Committed to 0.96 as well.
          (I guess we need to have a general discussion about whether we can have a feature in 0.94 but not in 0.96. Seems weird if we did.)

          Show
          Lars Hofhansl added a comment - Committed to 0.96 as well. (I guess we need to have a general discussion about whether we can have a feature in 0.94 but not in 0.96. Seems weird if we did.)
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in hbase-0.95-on-hadoop2 #285 (See https://builds.apache.org/job/hbase-0.95-on-hadoop2/285/)
          HBASE-9534: Short-Circuit Coprocessor HTable access when on the same server (jyates: rev 1524523)

          • /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          Show
          Hudson added a comment - SUCCESS: Integrated in hbase-0.95-on-hadoop2 #285 (See https://builds.apache.org/job/hbase-0.95-on-hadoop2/285/ ) HBASE-9534 : Short-Circuit Coprocessor HTable access when on the same server (jyates: rev 1524523) /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #739 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/739/)
          HBASE-9534: Short-Circuit Coprocessor HTable access when on the same server (jyates: rev 1524514)

          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          Show
          Hudson added a comment - SUCCESS: Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #739 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/739/ ) HBASE-9534 : Short-Circuit Coprocessor HTable access when on the same server (jyates: rev 1524514) /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in HBase-TRUNK #4529 (See https://builds.apache.org/job/HBase-TRUNK/4529/)
          HBASE-9534: Short-Circuit Coprocessor HTable access when on the same server (jyates: rev 1524514)

          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          Show
          Hudson added a comment - FAILURE: Integrated in HBase-TRUNK #4529 (See https://builds.apache.org/job/HBase-TRUNK/4529/ ) HBASE-9534 : Short-Circuit Coprocessor HTable access when on the same server (jyates: rev 1524514) /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          Hide
          Enis Soztutar added a comment -

          And change fix version from 0.96.1 to 0.96.0 if the RC is not cut yet.

          Show
          Enis Soztutar added a comment - And change fix version from 0.96.1 to 0.96.0 if the RC is not cut yet.
          Hide
          Elliott Clark added a comment -

          Pretty sure you should have checked this into 0.96 branch and not 0.95 (assuming that stack wanted this in 96.0). The 0.95 branch is dead and defunct.

          Show
          Elliott Clark added a comment - Pretty sure you should have checked this into 0.96 branch and not 0.95 (assuming that stack wanted this in 96.0). The 0.95 branch is dead and defunct.
          Hide
          Hudson added a comment -

          FAILURE: Integrated in HBase-0.94 #1152 (See https://builds.apache.org/job/HBase-0.94/1152/)
          HBASE-9534: Short-Circuit Coprocessor HTable access when on the same server (jyates: rev 1524522)

          • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java
          • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/client/HTable.java
          • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          Show
          Hudson added a comment - FAILURE: Integrated in HBase-0.94 #1152 (See https://builds.apache.org/job/HBase-0.94/1152/ ) HBASE-9534 : Short-Circuit Coprocessor HTable access when on the same server (jyates: rev 1524522) /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/client/HTable.java /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in hbase-0.95 #513 (See https://builds.apache.org/job/hbase-0.95/513/)
          HBASE-9534: Short-Circuit Coprocessor HTable access when on the same server (jyates: rev 1524523)

          • /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          Show
          Hudson added a comment - SUCCESS: Integrated in hbase-0.95 #513 (See https://builds.apache.org/job/hbase-0.95/513/ ) HBASE-9534 : Short-Circuit Coprocessor HTable access when on the same server (jyates: rev 1524523) /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in HBase-0.94-security #295 (See https://builds.apache.org/job/HBase-0.94-security/295/)
          HBASE-9534: Short-Circuit Coprocessor HTable access when on the same server (jyates: rev 1524522)

          • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java
          • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/client/HTable.java
          • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          Show
          Hudson added a comment - SUCCESS: Integrated in HBase-0.94-security #295 (See https://builds.apache.org/job/HBase-0.94-security/295/ ) HBASE-9534 : Short-Circuit Coprocessor HTable access when on the same server (jyates: rev 1524522) /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/client/HTable.java /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          Hide
          Lars Hofhansl added a comment -

          Whoops. Removed in order not to confuse anybody.

          Show
          Lars Hofhansl added a comment - Whoops. Removed in order not to confuse anybody.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12603881/hbase-9534-0.94-v2.patch
          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 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/7294//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/12603881/hbase-9534-0.94-v2.patch 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 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/7294//console This message is automatically generated.
          Hide
          Jesse Yates added a comment -

          Attaching updated 0.94-Lars patch which was got committed.

          Show
          Jesse Yates added a comment - Attaching updated 0.94-Lars patch which was got committed.
          Hide
          Jesse Yates added a comment -

          Hmmm, looks like you have a little cruft in your 0.94 patch there Lars. I'll post a new version (that I'm committing) minus the pom changes.

          Show
          Jesse Yates added a comment - Hmmm, looks like you have a little cruft in your 0.94 patch there Lars. I'll post a new version (that I'm committing) minus the pom changes.
          Hide
          Lars Hofhansl added a comment -

          If it's the same to you, let's do the slightly larger one.

          Show
          Lars Hofhansl added a comment - If it's the same to you, let's do the slightly larger one.
          Hide
          Jesse Yates added a comment -

          Smaller patch, but HTable is a little bit less clean with another round of duplicated code. Meh, I'm fine with either.

          Show
          Jesse Yates added a comment - Smaller patch, but HTable is a little bit less clean with another round of duplicated code. Meh, I'm fine with either.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12603762/9534-trunk.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 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

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

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

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

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

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

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

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//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/12603762/9534-trunk.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 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/7288//console This message is automatically generated.
          Hide
          Lars Hofhansl added a comment -

          Should also add a test to TestHCM.testClusterConnection, to make sure the internal pool is closed correctly.

          Show
          Lars Hofhansl added a comment - Should also add a test to TestHCM.testClusterConnection, to make sure the internal pool is closed correctly.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12603757/hbase-9534-trunk-v2.patch
          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 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

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

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

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

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

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

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

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//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/12603757/hbase-9534-trunk-v2.patch 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 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/7287//console This message is automatically generated.
          Hide
          Lars Hofhansl added a comment -

          Let me know what you think about these.

          Show
          Lars Hofhansl added a comment - Let me know what you think about these.
          Hide
          Lars Hofhansl added a comment -

          Let's keep those members final. There's a slight bit more code duplication, but it's cleaner.

          Show
          Lars Hofhansl added a comment - Let's keep those members final. There's a slight bit more code duplication, but it's cleaner.
          Hide
          Jesse Yates added a comment -

          Hey Lars Hofhansl, me too! Attaching...

          Show
          Jesse Yates added a comment - Hey Lars Hofhansl , me too! Attaching...
          Hide
          Lars Hofhansl added a comment -

          I have an updated patch that passes the tests.

          Show
          Lars Hofhansl added a comment - I have an updated patch that passes the tests.
          Hide
          Lars Hofhansl added a comment -

          I can reproduce the failure locally with the patch. It's related.

          Show
          Lars Hofhansl added a comment - I can reproduce the failure locally with the patch. It's related.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12603706/hbase-9534-trunk-v1.patch
          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 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

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

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

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

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

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

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.client.TestScannerTimeout
          org.apache.hadoop.hbase.client.TestFromClientSideWithCoprocessor
          org.apache.hadoop.hbase.client.TestFromClientSide
          org.apache.hadoop.hbase.regionserver.wal.TestHLog

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//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/12603706/hbase-9534-trunk-v1.patch 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 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.client.TestScannerTimeout org.apache.hadoop.hbase.client.TestFromClientSideWithCoprocessor org.apache.hadoop.hbase.client.TestFromClientSide org.apache.hadoop.hbase.regionserver.wal.TestHLog Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/7282//console This message is automatically generated.
          Hide
          Jesse Yates added a comment -

          Attaching updated trunk patch. Mostly just cleanup - able to drop the interface changes for MasterAdmin/Monitor by moving into the o.a.h.h.client package, just like the 0.94 patch. Also, marked CoprocessorHConnection as InterfaceAudience.Private and javadoc discouraging outside use.

          Show
          Jesse Yates added a comment - Attaching updated trunk patch. Mostly just cleanup - able to drop the interface changes for MasterAdmin/Monitor by moving into the o.a.h.h.client package, just like the 0.94 patch. Also, marked CoprocessorHConnection as InterfaceAudience.Private and javadoc discouraging outside use.
          Hide
          Lars Hofhansl added a comment -

          +1 from me. Moving this to client package is not ideal, but it is also not wrong (it is the client as used from the coprocessor).
          Good stuff.

          Show
          Lars Hofhansl added a comment - +1 from me. Moving this to client package is not ideal, but it is also not wrong (it is the client as used from the coprocessor). Good stuff.
          Hide
          Jesse Yates added a comment -

          Updated patch for 0.94. Does the direct comparison Lars' asked for + now correctly works by subclassing HConnectionImplementation to override #getHRegionConnection (but at the cost of having to move it into the client package w/ a big ol' warning about not for actual client use).

          Show
          Jesse Yates added a comment - Updated patch for 0.94. Does the direct comparison Lars' asked for + now correctly works by subclassing HConnectionImplementation to override #getHRegionConnection (but at the cost of having to move it into the client package w/ a big ol' warning about not for actual client use).
          Hide
          Jesse Yates added a comment -

          Looks like trunk is OK - MultiServerCallable just gets the server location from the HConnection, which is the custom CoprocessorHConnection. Probably need to something a little more ugly with 0.94, maybe doing an inheritance-based solution instead.

          Show
          Jesse Yates added a comment - Looks like trunk is OK - MultiServerCallable just gets the server location from the HConnection, which is the custom CoprocessorHConnection. Probably need to something a little more ugly with 0.94, maybe doing an inheritance-based solution instead.
          Hide
          Lars Hofhansl added a comment -

          OK... Maybe not so nice

          Show
          Lars Hofhansl added a comment - OK... Maybe not so nice
          Hide
          Jesse Yates added a comment -

          Hmmm, upon manual inspection, looks like I completely hosed the above... delegate is just gonna keep on doing its delegate thing and not call the #getClient (for 0.96 case) or #getHRegionConnection (0.94).

          Updated patches coming soon.

          Show
          Jesse Yates added a comment - Hmmm, upon manual inspection, looks like I completely hosed the above... delegate is just gonna keep on doing its delegate thing and not call the #getClient (for 0.96 case) or #getHRegionConnection (0.94). Updated patches coming soon.
          Hide
          Lars Hofhansl added a comment - - edited

          Nice.
          Wouldn't we do this a lot:

          if (!Addressing.createHostAndPortStr(hostname, port).equals(this.serverName.getHostAndPort()))
          

          Makes 2 new String objects each time.
          Might be better to compare host/port directly.

          Show
          Lars Hofhansl added a comment - - edited Nice. Wouldn't we do this a lot: if (!Addressing.createHostAndPortStr(hostname, port).equals( this .serverName.getHostAndPort())) Makes 2 new String objects each time. Might be better to compare host/port directly.
          Hide
          Jesse Yates added a comment -

          Patch for 0.94 - even simpler!

          Kept the Master connection stuff just passing onto the delegate since it doesn't require any interface changes (and still goes the serialization route).

          Show
          Jesse Yates added a comment - Patch for 0.94 - even simpler! Kept the Master connection stuff just passing onto the delegate since it doesn't require any interface changes (and still goes the serialization route).
          Hide
          Jesse Yates added a comment -

          Stack yeah, I wasn't a big fan of making those interfaces public. Alternative was throw an UnspportedOperationException if you try to hit the master with that connection. Keeps the interfaces how they are, but given that we will have this connection, we could just as easily use it to talk locally to the master (though, as I mention in the description, its not really worth it).

          I'm ok rolling those back and going with the exception or marking them InterfaceAudience.Private - former seems cleaner for the moment.

          Show
          Jesse Yates added a comment - Stack yeah, I wasn't a big fan of making those interfaces public. Alternative was throw an UnspportedOperationException if you try to hit the master with that connection. Keeps the interfaces how they are, but given that we will have this connection, we could just as easily use it to talk locally to the master (though, as I mention in the description, its not really worth it). I'm ok rolling those back and going with the exception or marking them InterfaceAudience.Private - former seems cleaner for the moment.
          Hide
          Lars Hofhansl added a comment -

          The rest of the patch is nice, though.

          Should tag the interfaces with InterfaceAudience.Private. Coprocessor are internal, so it's fine (IMHO) to give them access to internals.

          Show
          Lars Hofhansl added a comment - The rest of the patch is nice, though. Should tag the interfaces with InterfaceAudience.Private. Coprocessor are internal, so it's fine (IMHO) to give them access to internals.
          Hide
          stack added a comment -

          Ugly

          -interface MasterAdminKeepAliveConnection
          +public interface MasterAdminKeepAliveConnection

          This is smelly internals and here you are giving everyone a whiff.

          In general, its funky... but in a good way. Can look closer later but seems good to me Jesse.

          Show
          stack added a comment - Ugly -interface MasterAdminKeepAliveConnection +public interface MasterAdminKeepAliveConnection This is smelly internals and here you are giving everyone a whiff. In general, its funky... but in a good way. Can look closer later but seems good to me Jesse.
          Hide
          Jesse Yates added a comment -

          First cut for trunk. Surprisingly, not all the much code, but does get down into the guts a little bit.

          Could use a glance over to make sure I'm not messing up the RPC stuff (stack?).

          Show
          Jesse Yates added a comment - First cut for trunk. Surprisingly, not all the much code, but does get down into the guts a little bit. Could use a glance over to make sure I'm not messing up the RPC stuff ( stack ?).

            People

            • Assignee:
              Jesse Yates
              Reporter:
              Jesse Yates
            • Votes:
              0 Vote for this issue
              Watchers:
              12 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development