HBase
  1. HBase
  2. HBASE-5897

prePut coprocessor hook causing substantial CPU usage

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 0.92.0
    • Fix Version/s: 0.92.2, 0.94.0, 0.95.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      I was running an insert workload against trunk under oprofile and saw that a significant portion of CPU usage was going to calling the "prePut" coprocessor hook inside doMiniBatchPut, even though I don't have any coprocessors installed. I ran a million-row insert and collected CPU time spent in the RS after commenting out the preput hook, and found CPU usage reduced by 33%.

      1. 5897-v3-0.92.txt
        3 kB
        Lars Hofhansl
      2. 5897-v3-0.94.txt
        4 kB
        Lars Hofhansl
      3. 5897-v3.txt
        4 kB
        Lars Hofhansl
      4. 5897-v2.txt
        4 kB
        Ted Yu
      5. testRegionServerCoprocessorExceptionWithRemove.stack
        84 kB
        Ted Yu
      6. 5897-simple.txt
        0.8 kB
        Lars Hofhansl
      7. hbase-5897.txt
        4 kB
        Todd Lipcon

        Activity

        stack made changes -
        Fix Version/s 0.95.0 [ 12324094 ]
        stack made changes -
        Fix Version/s 0.92.2 [ 12319888 ]
        Fix Version/s 0.95.0 [ 12324094 ]
        Lars Hofhansl made changes -
        Fix Version/s 0.94.0 [ 12316419 ]
        stack made changes -
        Fix Version/s 0.95.0 [ 12324094 ]
        Fix Version/s 0.94.0 [ 12316419 ]
        Fix Version/s 0.92.2 [ 12319888 ]
        Fix Version/s 0.96.0 [ 12320040 ]
        Lars Hofhansl made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Hudson added a comment -

        Integrated in HBase-0.92-security #106 (See https://builds.apache.org/job/HBase-0.92-security/106/)
        HBASE-5897 prePut coprocessor hook causing substantial CPU usage (Revision 1332816)

        Result = SUCCESS
        larsh :
        Files :

        • /hbase/branches/0.92/CHANGES.txt
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Show
        Hudson added a comment - Integrated in HBase-0.92-security #106 (See https://builds.apache.org/job/HBase-0.92-security/106/ ) HBASE-5897 prePut coprocessor hook causing substantial CPU usage (Revision 1332816) Result = SUCCESS larsh : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK-security #189 (See https://builds.apache.org/job/HBase-TRUNK-security/189/)
        HBASE-5897 prePut coprocessor hook causing substantial CPU usage (Revision 1332811)

        Result = SUCCESS
        larsh :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK-security #189 (See https://builds.apache.org/job/HBase-TRUNK-security/189/ ) HBASE-5897 prePut coprocessor hook causing substantial CPU usage (Revision 1332811) Result = SUCCESS larsh : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Hide
        Hudson added a comment -

        Integrated in HBase-0.92 #395 (See https://builds.apache.org/job/HBase-0.92/395/)
        HBASE-5897 prePut coprocessor hook causing substantial CPU usage (Revision 1332816)

        Result = SUCCESS
        larsh :
        Files :

        • /hbase/branches/0.92/CHANGES.txt
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Show
        Hudson added a comment - Integrated in HBase-0.92 #395 (See https://builds.apache.org/job/HBase-0.92/395/ ) HBASE-5897 prePut coprocessor hook causing substantial CPU usage (Revision 1332816) Result = SUCCESS larsh : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #2834 (See https://builds.apache.org/job/HBase-TRUNK/2834/)
        HBASE-5897 prePut coprocessor hook causing substantial CPU usage (Revision 1332811)

        Result = FAILURE
        larsh :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #2834 (See https://builds.apache.org/job/HBase-TRUNK/2834/ ) HBASE-5897 prePut coprocessor hook causing substantial CPU usage (Revision 1332811) Result = FAILURE larsh : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Hide
        Hudson added a comment -

        Integrated in HBase-0.94 #165 (See https://builds.apache.org/job/HBase-0.94/165/)
        HBASE-5897 prePut coprocessor hook causing substantial CPU usage (Revision 1332810)

        Result = FAILURE
        larsh :
        Files :

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Show
        Hudson added a comment - Integrated in HBase-0.94 #165 (See https://builds.apache.org/job/HBase-0.94/165/ ) HBASE-5897 prePut coprocessor hook causing substantial CPU usage (Revision 1332810) Result = FAILURE larsh : Files : /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Hide
        Hudson added a comment -

        Integrated in HBase-0.94-security #25 (See https://builds.apache.org/job/HBase-0.94-security/25/)
        HBASE-5897 prePut coprocessor hook causing substantial CPU usage (Revision 1332810)

        Result = SUCCESS
        larsh :
        Files :

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Show
        Hudson added a comment - Integrated in HBase-0.94-security #25 (See https://builds.apache.org/job/HBase-0.94-security/25/ ) HBASE-5897 prePut coprocessor hook causing substantial CPU usage (Revision 1332810) Result = SUCCESS larsh : Files : /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Lars Hofhansl made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Hadoop Flags Reviewed [ 10343 ]
        Resolution Fixed [ 1 ]
        Hide
        Lars Hofhansl added a comment -

        Committed to 0.92, 0.94, and 0.96.

        Show
        Lars Hofhansl added a comment - Committed to 0.92, 0.94, and 0.96.
        Lars Hofhansl made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Lars Hofhansl made changes -
        Attachment 5897-v3-0.92.txt [ 12525218 ]
        Hide
        Lars Hofhansl added a comment -

        0.92 patch

        Show
        Lars Hofhansl added a comment - 0.92 patch
        Lars Hofhansl made changes -
        Comment [ -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12525198/5897-v3-0.94.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 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1708//console

        This message is automatically generated. ]
        Lars Hofhansl made changes -
        Attachment 5897-v3-0.94.txt [ 12525198 ]
        Hide
        Lars Hofhansl added a comment -

        Patch for 0.94

        Show
        Lars Hofhansl added a comment - Patch for 0.94
        Hide
        stack added a comment -

        None from me.

        Show
        stack added a comment - None from me.
        Hide
        Lars Hofhansl added a comment -

        I would like to commit v3 and roll another RC of 0.94.0 in the next few hours. Any objections?

        Show
        Lars Hofhansl added a comment - I would like to commit v3 and roll another RC of 0.94.0 in the next few hours. Any objections?
        Hide
        Lars Hofhansl added a comment -

        +1 on either v2 or v3, btw.

        Show
        Lars Hofhansl added a comment - +1 on either v2 or v3, btw.
        Hide
        Hadoop QA added a comment -

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

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 hadoop23. The patch compiles against the hadoop 0.23.x profile.

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

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

        -1 findbugs. The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings.

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

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.master.TestSplitLogManager
        org.apache.hadoop.hbase.client.TestShell

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1701//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1701//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1701//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/12525142/5897-v3.txt against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 hadoop23. The patch compiles against the hadoop 0.23.x profile. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.master.TestSplitLogManager org.apache.hadoop.hbase.client.TestShell Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1701//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1701//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1701//console This message is automatically generated.
        Hide
        Hadoop QA added a comment -

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

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 hadoop23. The patch compiles against the hadoop 0.23.x profile.

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

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

        -1 findbugs. The patch appears to introduce 2 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.client.TestShell

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1700//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1700//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1700//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/12525137/5897-v2.txt against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 hadoop23. The patch compiles against the hadoop 0.23.x profile. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 2 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.client.TestShell Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1700//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1700//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1700//console This message is automatically generated.
        Lars Hofhansl made changes -
        Attachment 5897-v3.txt [ 12525142 ]
        Hide
        Lars Hofhansl added a comment -

        How about this one. Just removes the precondition checks, applies prePut edit edits before the family edits. And removes an unused variable.

        Show
        Lars Hofhansl added a comment - How about this one. Just removes the precondition checks, applies prePut edit edits before the family edits. And removes an unused variable.
        Hide
        Ted Yu added a comment -

        Sorry, Lars.
        Attaching jstack was to show that patch v1 needed a little refinement.
        Then I went to talk to my manager, and ...

        Show
        Ted Yu added a comment - Sorry, Lars. Attaching jstack was to show that patch v1 needed a little refinement. Then I went to talk to my manager, and ...
        Hide
        Lars Hofhansl added a comment -

        You beat me to it Ted

        I was also going to suggest to remove the new precondition checks. In both cases nothing incorrect will happen (if the prePut skips or the Put has writeToWal==false, the WAL is not applied), but we're changing the coprocessor protocol slightly.

        Show
        Lars Hofhansl added a comment - You beat me to it Ted I was also going to suggest to remove the new precondition checks. In both cases nothing incorrect will happen (if the prePut skips or the Put has writeToWal==false, the WAL is not applied), but we're changing the coprocessor protocol slightly.
        Ted Yu made changes -
        Attachment 5897-v2.txt [ 12525137 ]
        Hide
        Ted Yu added a comment -

        Patch v2 passes TestRegionServerCoprocessorExceptionWithRemove

        Show
        Ted Yu added a comment - Patch v2 passes TestRegionServerCoprocessorExceptionWithRemove
        Hide
        Lars Hofhansl added a comment -

        I see. The call to doPrePut should be the try/catch block, so that closeRegionOperation is executed in case it throws an exception.

        Show
        Lars Hofhansl added a comment - I see. The call to doPrePut should be the try/catch block, so that closeRegionOperation is executed in case it throws an exception.
        Hide
        Lars Hofhansl added a comment -

        That's a huge stack dump. What sticks to me are these:

                at org.apache.hadoop.hbase.HBaseTestingUtility.shutdownMiniCluster(HBaseTestingUtility.java:711)
                at org.apache.hadoop.hbase.coprocessor.TestRegionServerCoprocessorExceptionWithRemove.teardownAfterClass(TestRegionServerCoprocessorExceptionWithRemove.java:77)
        

        and

                at java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock.lock(Reen
        trantReadWriteLock.java:807)
                at org.apache.hadoop.hbase.regionserver.HRegion.doClose(HRegion.java:929
        )
                at org.apache.hadoop.hbase.regionserver.HRegion.close(HRegion.java:879)
                - locked <7934dcbd0> (a java.lang.Object)
                at org.apache.hadoop.hbase.regionserver.handler.CloseRegionHandler.process(CloseRegionHandler.java:121)
        

        On the face of it, it does seem unrelated.

        Show
        Lars Hofhansl added a comment - That's a huge stack dump. What sticks to me are these: at org.apache.hadoop.hbase.HBaseTestingUtility.shutdownMiniCluster(HBaseTestingUtility.java:711) at org.apache.hadoop.hbase.coprocessor.TestRegionServerCoprocessorExceptionWithRemove.teardownAfterClass(TestRegionServerCoprocessorExceptionWithRemove.java:77) and at java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock.lock(Reen trantReadWriteLock.java:807) at org.apache.hadoop.hbase.regionserver.HRegion.doClose(HRegion.java:929 ) at org.apache.hadoop.hbase.regionserver.HRegion.close(HRegion.java:879) - locked <7934dcbd0> (a java.lang. Object ) at org.apache.hadoop.hbase.regionserver.handler.CloseRegionHandler.process(CloseRegionHandler.java:121) On the face of it, it does seem unrelated.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12525125/testRegionServerCoprocessorExceptionWithRemove.stack
        against trunk revision .

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

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

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

        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1697//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/12525125/testRegionServerCoprocessorExceptionWithRemove.stack against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1697//console This message is automatically generated.
        Ted Yu made changes -
        Hide
        Ted Yu added a comment -

        Running the patch against hadoop 1.0 and 0.22, I got TestRegionServerCoprocessorExceptionWithRemove hanging on MacBook.

        jstack attached.

        Show
        Ted Yu added a comment - Running the patch against hadoop 1.0 and 0.22, I got TestRegionServerCoprocessorExceptionWithRemove hanging on MacBook. jstack attached.
        Hide
        Lars Hofhansl added a comment -

        Yep. I was just wondering whether this is a rather artificial scenario. Anyway, your patch makes it much better and actually ensures correct behavior.

        Let's commit, unless there're objections.

        Show
        Lars Hofhansl added a comment - Yep. I was just wondering whether this is a rather artificial scenario. Anyway, your patch makes it much better and actually ensures correct behavior. Let's commit, unless there're objections.
        Hide
        Todd Lipcon added a comment -

        So calling doMiniBatchPut multiple times on behalf of the same Put operation should be happening relatively rarely.

        True, unless there's a lot of contention on some rows. In the test I was doing, I was slamming a fairly small number of rows with puts, so there was a lot of this "back-off" behavior.

        I bet we could improve performance by making it continue past a failed lock acquisition and acquire as many as possible, while pushing those it missed back onto the end of the list. Right now, as soon as it fails any lock acquisition, it stops the batch there (so we always do contiguous ranges of ops instead of arbitrary sets)

        Show
        Todd Lipcon added a comment - So calling doMiniBatchPut multiple times on behalf of the same Put operation should be happening relatively rarely. True, unless there's a lot of contention on some rows. In the test I was doing, I was slamming a fairly small number of rows with puts, so there was a lot of this "back-off" behavior. I bet we could improve performance by making it continue past a failed lock acquisition and acquire as many as possible, while pushing those it missed back onto the end of the list. Right now, as soon as it fails any lock acquisition, it stops the batch there (so we always do contiguous ranges of ops instead of arbitrary sets)
        Hide
        Lars Hofhansl added a comment -

        Looking at the code some more... doMiniBactchPut will handle the entire batch unless it cannot lock the involved rows (see STEP 1. in doMiniBatchPut).
        So calling doMiniBatchPut multiple times on behalf of the same Put operation should be happening relatively rarely.

        Show
        Lars Hofhansl added a comment - Looking at the code some more... doMiniBactchPut will handle the entire batch unless it cannot lock the involved rows (see STEP 1. in doMiniBatchPut). So calling doMiniBatchPut multiple times on behalf of the same Put operation should be happening relatively rarely.
        Hide
        Lars Hofhansl added a comment -

        Looked over Todd's patch. The only difference is that before the prePut's edits ended up in WALEdit before the family edits. Now that is reversed. Not sure if that even makes a difference. +1 otherwise

        Show
        Lars Hofhansl added a comment - Looked over Todd's patch. The only difference is that before the prePut's edits ended up in WALEdit before the family edits. Now that is reversed. Not sure if that even makes a difference. +1 otherwise
        Hide
        stack added a comment -

        +1 on the more radical patch.

        Show
        stack added a comment - +1 on the more radical patch.
        Hide
        Lars Hofhansl added a comment -

        Right. That's what I was trying to say when I attached my patch.

        Show
        Lars Hofhansl added a comment - Right. That's what I was trying to say when I attached my patch.
        Hide
        Anoop Sam John added a comment -

        As per the simple patch also, there can be more CP calls happening for one Put

        -      for (int i = 0; i < batchOp.operations.length; i++) {
        +      for (int i = batchOp.nextIndexToProcess; i < batchOp.operations.length; i++) {
        

        Suppose in 2 calls to doMiniBatchPut() a put(List) with 100 puts operation is getting completed. For the 1st run it will call hook for all 100 Puts. Then in the next run previously it was calling again 100 times. Now it will be for all the remaining puts which were not handled in the 1st iteration.

        In Todd's patch this will not happen.[Calling all pre hook just before the 1st call to the doMiniBatchPut()] But that will call the pre hook much before the actual put operation. Is this correct? How some one can for sure get a pre hook call before the actual put() for a Put?

        Show
        Anoop Sam John added a comment - As per the simple patch also, there can be more CP calls happening for one Put - for ( int i = 0; i < batchOp.operations.length; i++) { + for ( int i = batchOp.nextIndexToProcess; i < batchOp.operations.length; i++) { Suppose in 2 calls to doMiniBatchPut() a put(List) with 100 puts operation is getting completed. For the 1st run it will call hook for all 100 Puts. Then in the next run previously it was calling again 100 times. Now it will be for all the remaining puts which were not handled in the 1st iteration. In Todd's patch this will not happen. [Calling all pre hook just before the 1st call to the doMiniBatchPut()] But that will call the pre hook much before the actual put operation. Is this correct? How some one can for sure get a pre hook call before the actual put() for a Put?
        Hide
        Anoop Sam John added a comment -

        We also saw this behaviour issue while doing a code walkthrough!... But was not sure...

        Show
        Anoop Sam John added a comment - We also saw this behaviour issue while doing a code walkthrough!... But was not sure...
        Lars Hofhansl made changes -
        Fix Version/s 0.92.2 [ 12319888 ]
        Hide
        Andrew Purtell added a comment - - edited

        +1 on the -simple patch (at least). This looks like a bug that wasn't caught because we didn't have multiput test coverage.

        Edit: But we do now, with LoadTestTool.

        Show
        Andrew Purtell added a comment - - edited +1 on the -simple patch (at least). This looks like a bug that wasn't caught because we didn't have multiput test coverage. Edit: But we do now, with LoadTestTool.
        Lars Hofhansl made changes -
        Priority Major [ 3 ] Critical [ 2 ]
        Hide
        Lars Hofhansl added a comment -

        Alright then, let's get Todd's patch into 0.94. Marking as critical.

        Show
        Lars Hofhansl added a comment - Alright then, let's get Todd's patch into 0.94. Marking as critical.
        Hide
        Ted Yu added a comment -

        +1 on Todd's patch.

        Show
        Ted Yu added a comment - +1 on Todd's 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/12525005/5897-simple.txt
        against trunk revision .

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 hadoop23. The patch compiles against the hadoop 0.23.x profile.

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

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

        -1 findbugs. The patch appears to introduce 2 new Findbugs (version 1.3.9) warnings.

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

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

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1682//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1682//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1682//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/12525005/5897-simple.txt against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 hadoop23. The patch compiles against the hadoop 0.23.x profile. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 2 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1682//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1682//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1682//console This message is automatically generated.
        Lars Hofhansl made changes -
        Fix Version/s 0.94.0 [ 12316419 ]
        Fix Version/s 0.96.0 [ 12320040 ]
        Lars Hofhansl made changes -
        Attachment 5897-simple.txt [ 12525005 ]
        Hide
        Lars Hofhansl added a comment -

        the prePut hook runs through all the BatchOperationInProgress during every mini-batch

        Interesting observation. I wonder whether we do something simple (like attached patch) in 0.94 and Todd's fix in 0.96.
        The attached -simple patch at least makes sure we start from the current batch index. operations towards the end of the set of batches would still pass through prePut multiple times, which is bad.

        Show
        Lars Hofhansl added a comment - the prePut hook runs through all the BatchOperationInProgress during every mini-batch Interesting observation. I wonder whether we do something simple (like attached patch) in 0.94 and Todd's fix in 0.96. The attached -simple patch at least makes sure we start from the current batch index. operations towards the end of the set of batches would still pass through prePut multiple times, which is bad.
        Hide
        Hadoop QA added a comment -

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

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 hadoop23. The patch compiles against the hadoop 0.23.x profile.

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

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

        -1 findbugs. The patch appears to introduce 2 new Findbugs (version 1.3.9) warnings.

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

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.master.TestSplitLogManager

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1681//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1681//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1681//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/12525000/hbase-5897.txt against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 hadoop23. The patch compiles against the hadoop 0.23.x profile. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 2 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.master.TestSplitLogManager Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1681//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1681//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1681//console This message is automatically generated.
        Todd Lipcon made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Assignee Todd Lipcon [ tlipcon ]
        Todd Lipcon made changes -
        Field Original Value New Value
        Attachment hbase-5897.txt [ 12525000 ]
        Hide
        Todd Lipcon added a comment -

        This patch fixes the CPU usage in my test case.

        Would be good if someone more familiar with coprocessors can look at this – I think this is also fixing a behavioral bug with coprocessors. So if this seems like the correct fix we should also add a regression test for the behavior.

        Show
        Todd Lipcon added a comment - This patch fixes the CPU usage in my test case. Would be good if someone more familiar with coprocessors can look at this – I think this is also fixing a behavioral bug with coprocessors. So if this seems like the correct fix we should also add a regression test for the behavior.
        Hide
        Todd Lipcon added a comment -

        Perhaps this has to do with the fact that the prePut hook runs through all the BatchOperationInProgress during every mini-batch, instead of just that batch...

        Show
        Todd Lipcon added a comment - Perhaps this has to do with the fact that the prePut hook runs through all the BatchOperationInProgress during every mini-batch, instead of just that batch...
        Hide
        Todd Lipcon added a comment -

        Test methodology: I'm running a single standalone HBase on local filesystem. I rm the dir from /tmp, then start the master, create a table with a single column family, then restart the master with "time bin/hbase master". I then insert 1M rows from YCSB, and hit ^C on the master. I ran the test with the preput hook left in, and with it commented out, and flipped between the two setups twice to make sure I could reproduce. I'm reporting the user and system CPU here, since the wall clock time is sensitive to how quickly I ran the commands.

        with hook commented out:
        run 1:
        user 1m57.760s
        sys 0m3.300s
        run 2:
        user 1m54.840s
        sys 0m3.870s

        with cp hook left in:
        run 1:
        user 3m13.890s
        sys 0m3.940s
        run 2:
        user 3m11.960s
        sys 0m3.930s

        I also re-ran the original oprofile analysis and "doMiniBatchPut" basically dropped off the CPU profile results once I made this change.

        I verified for sure that I have no coprocessors installed by adding a println in RegionCoprocessorHost - it didn't print out.

        Show
        Todd Lipcon added a comment - Test methodology: I'm running a single standalone HBase on local filesystem. I rm the dir from /tmp, then start the master, create a table with a single column family, then restart the master with "time bin/hbase master". I then insert 1M rows from YCSB, and hit ^C on the master. I ran the test with the preput hook left in, and with it commented out, and flipped between the two setups twice to make sure I could reproduce. I'm reporting the user and system CPU here, since the wall clock time is sensitive to how quickly I ran the commands. with hook commented out: run 1: user 1m57.760s sys 0m3.300s run 2: user 1m54.840s sys 0m3.870s with cp hook left in: run 1: user 3m13.890s sys 0m3.940s run 2: user 3m11.960s sys 0m3.930s I also re-ran the original oprofile analysis and "doMiniBatchPut" basically dropped off the CPU profile results once I made this change. I verified for sure that I have no coprocessors installed by adding a println in RegionCoprocessorHost - it didn't print out.
        Todd Lipcon created issue -

          People

          • Assignee:
            Todd Lipcon
            Reporter:
            Todd Lipcon
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development