HBase
  1. HBase
  2. HBASE-5255

Use singletons for OperationStatus to save memory

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.90.5, 0.92.0
    • Fix Version/s: 0.92.1, 0.94.0
    • Component/s: regionserver
    • Labels:
    • Hadoop Flags:
      Reviewed

      Description

      Every single Put causes the allocation of at least one OperationStatus, yet OperationStatus is almost always stateless, so these allocations are unnecessary and could be avoided. Attached patch adds a few singletons and uses them, with no public API change. I didn't test the patches, but you get the idea.

        Activity

        Hide
        Benoit Sigoure added a comment -

        I didn't create a patch for the 0.90 branch, but if there are going to be more releases in that branch, you should consider applying it there too.

        Show
        Benoit Sigoure added a comment - I didn't create a patch for the 0.90 branch, but if there are going to be more releases in that branch, you should consider applying it there too.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12511469/HBASE-5255-trunk-Use-singletons-to-remove-unnecessary-memory-allocati.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/835//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/12511469/HBASE-5255-trunk-Use-singletons-to-remove-unnecessary-memory-allocati.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/835//console This message is automatically generated.
        Hide
        Ted Yu added a comment - - edited

        I like the introduction of singletons.
        Since there is no setter for code or exceptionMsg, I think we should make them final.

        Show
        Ted Yu added a comment - - edited I like the introduction of singletons. Since there is no setter for code or exceptionMsg, I think we should make them final.
        Hide
        Ted Yu added a comment -

        Patch v2 makes exceptionMsg and code fields final.

        Show
        Ted Yu added a comment - Patch v2 makes exceptionMsg and code fields final.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12511518/5255-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 javadoc. The javadoc tool appears to have generated -145 warning messages.

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

        -1 findbugs. The patch appears to introduce 84 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/837//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/837//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/837//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/12511518/5255-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 javadoc. The javadoc tool appears to have generated -145 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 84 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/837//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/837//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/837//console This message is automatically generated.
        Hide
        Ted Yu added a comment -

        Patch for 0.92 branch

        Show
        Ted Yu added a comment - Patch for 0.92 branch
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12511526/5255-92.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 javadoc. The javadoc tool appears to have generated -145 warning messages.

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

        -1 findbugs. The patch appears to introduce 84 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.regionserver.TestSplitTransactionOnCluster

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/839//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/839//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/839//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/12511526/5255-92.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 javadoc. The javadoc tool appears to have generated -145 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 84 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.regionserver.TestSplitTransactionOnCluster Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/839//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/839//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/839//console This message is automatically generated.
        Hide
        Ted Yu added a comment -

        Integrated to 0.92 and TRUNK.

        Show
        Ted Yu added a comment - Integrated to 0.92 and TRUNK.
        Hide
        Hudson added a comment -

        Integrated in HBase-0.92 #259 (See https://builds.apache.org/job/HBase-0.92/259/)
        HBASE-5255 Use singletons for OperationStatus to save memory (Benoit)

        tedyu :
        Files :

        • /hbase/branches/0.92/CHANGES.txt
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/OperationStatus.java
        Show
        Hudson added a comment - Integrated in HBase-0.92 #259 (See https://builds.apache.org/job/HBase-0.92/259/ ) HBASE-5255 Use singletons for OperationStatus to save memory (Benoit) tedyu : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/OperationStatus.java
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #2645 (See https://builds.apache.org/job/HBase-TRUNK/2645/)
        HBASE-5255 Use singletons for OperationStatus to save memory (Benoit)

        tedyu :
        Files :

        • /hbase/trunk/CHANGES.txt
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/OperationStatus.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #2645 (See https://builds.apache.org/job/HBase-TRUNK/2645/ ) HBASE-5255 Use singletons for OperationStatus to save memory (Benoit) tedyu : Files : /hbase/trunk/CHANGES.txt /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/OperationStatus.java
        Hide
        stack added a comment -

        +1 on patch

        Show
        stack added a comment - +1 on patch
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK-security #86 (See https://builds.apache.org/job/HBase-TRUNK-security/86/)
        HBASE-5255 Use singletons for OperationStatus to save memory (Benoit)

        tedyu :
        Files :

        • /hbase/trunk/CHANGES.txt
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/OperationStatus.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK-security #86 (See https://builds.apache.org/job/HBase-TRUNK-security/86/ ) HBASE-5255 Use singletons for OperationStatus to save memory (Benoit) tedyu : Files : /hbase/trunk/CHANGES.txt /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/OperationStatus.java
        Hide
        Hudson added a comment -

        Integrated in HBase-0.92-security #88 (See https://builds.apache.org/job/HBase-0.92-security/88/)
        HBASE-5255 Use singletons for OperationStatus to save memory (Benoit)

        tedyu :
        Files :

        • /hbase/branches/0.92/CHANGES.txt
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/OperationStatus.java
        Show
        Hudson added a comment - Integrated in HBase-0.92-security #88 (See https://builds.apache.org/job/HBase-0.92-security/88/ ) HBASE-5255 Use singletons for OperationStatus to save memory (Benoit) tedyu : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/OperationStatus.java
        Hide
        Benoit Sigoure added a comment -

        A hunk was missed when this patch got merged in the 0.92 branch:

        
        --- a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        +++ b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        @@ -1862,8 +1862,7 @@ public class HRegion implements HeapSize { // , Writable{
                   continue;
                 }
                 addedSize += applyFamilyMapToMemstore(familyMaps[i]);
        -        batchOp.retCodeDetails[i] = new OperationStatus(
        -            OperationStatusCode.SUCCESS);
        +        batchOp.retCodeDetails[i] = OperationStatus.SUCCESS;
               }
         
               // ------------------------------------
        

        Do you want to file another JIRA about it?

        Show
        Benoit Sigoure added a comment - A hunk was missed when this patch got merged in the 0.92 branch: --- a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -1862,8 +1862,7 @@ public class HRegion implements HeapSize { // , Writable{ continue ; } addedSize += applyFamilyMapToMemstore(familyMaps[i]); - batchOp.retCodeDetails[i] = new OperationStatus( - OperationStatusCode.SUCCESS); + batchOp.retCodeDetails[i] = OperationStatus.SUCCESS; } // ------------------------------------ Do you want to file another JIRA about it?
        Hide
        stack added a comment -

        Thanks Benoit. Fixed it w/ HBASE-5432.

        Show
        stack added a comment - Thanks Benoit. Fixed it w/ HBASE-5432 .
        Hide
        Hudson added a comment -

        Integrated in HBase-0.92 #291 (See https://builds.apache.org/job/HBase-0.92/291/)
        HBASE-5432 Hunk missed applying 'hbase-5255 Use singletons for OperationStatus to save memory' (Revision 1290862)

        Result = SUCCESS

        Show
        Hudson added a comment - Integrated in HBase-0.92 #291 (See https://builds.apache.org/job/HBase-0.92/291/ ) HBASE-5432 Hunk missed applying 'hbase-5255 Use singletons for OperationStatus to save memory' (Revision 1290862) Result = SUCCESS
        Hide
        Hudson added a comment -

        Integrated in HBase-0.92-security #91 (See https://builds.apache.org/job/HBase-0.92-security/91/)
        HBASE-5432 Hunk missed applying 'hbase-5255 Use singletons for OperationStatus to save memory' (Revision 1290862)

        Result = FAILURE

        Show
        Hudson added a comment - Integrated in HBase-0.92-security #91 (See https://builds.apache.org/job/HBase-0.92-security/91/ ) HBASE-5432 Hunk missed applying 'hbase-5255 Use singletons for OperationStatus to save memory' (Revision 1290862) Result = FAILURE

          People

          • Assignee:
            Benoit Sigoure
            Reporter:
            Benoit Sigoure
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development