Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.90.5, 0.92.0, 0.94.0
    • Fix Version/s: 0.90.6, 0.92.1, 0.94.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The 'transform' feature, where REST can be instructed, via a table attribute, to apply a transformation (e.g. base64 encoding or decoding) to a (sub)set of column values before serving them up to a client or storing them into HBase, was added some time ago at the request of Jack Levin. I have since come to regret it, it was not a well thought out feature:

      • This is really an application concern.
      • It adds significant overhead to request processing: Periodically a HBaseAdmin is used to retrieve the table descriptor, in order to scan through table attributes for transformation directives.

      I think it is best to rip it out, its a real problem area, and REST should be no more concerned about data formats than the Java API.

      I doubt anyone uses this, not even Jack. Will need to follow up with him to confirm.

      1. HBASE-5228-0.92.patch
        23 kB
        Andrew Purtell
      2. HBASE-5228-trunk.patch
        23 kB
        Andrew Purtell

        Activity

        Hide
        hudson Hudson added a comment -

        Integrated in HBase-0.92-security #86 (See https://builds.apache.org/job/HBase-0.92-security/86/)
        HBASE-5228. [REST] Rip out transform feature

        apurtell :
        Files :

        • /hbase/branches/0.92/CHANGES.txt
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/rest/MultiRowResource.java
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/rest/RowResource.java
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/rest/transform
        • /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/rest/TestTransform.java
        Show
        hudson Hudson added a comment - Integrated in HBase-0.92-security #86 (See https://builds.apache.org/job/HBase-0.92-security/86/ ) HBASE-5228 . [REST] Rip out transform feature apurtell : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/rest/MultiRowResource.java /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/rest/RowResource.java /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/rest/transform /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/rest/TestTransform.java
        Hide
        hudson Hudson added a comment -

        Integrated in HBase-TRUNK-security #83 (See https://builds.apache.org/job/HBase-TRUNK-security/83/)
        HBASE-5228. [REST] Rip out transform feature

        apurtell :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/MultiRowResource.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/RowResource.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/transform
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/rest/TestTransform.java
        Show
        hudson Hudson added a comment - Integrated in HBase-TRUNK-security #83 (See https://builds.apache.org/job/HBase-TRUNK-security/83/ ) HBASE-5228 . [REST] Rip out transform feature apurtell : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/MultiRowResource.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/RowResource.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/transform /hbase/trunk/src/test/java/org/apache/hadoop/hbase/rest/TestTransform.java
        Hide
        hudson Hudson added a comment -

        Integrated in HBase-TRUNK #2641 (See https://builds.apache.org/job/HBase-TRUNK/2641/)
        HBASE-5228. [REST] Rip out transform feature

        apurtell :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/MultiRowResource.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/RowResource.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/transform
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/rest/TestTransform.java
        Show
        hudson Hudson added a comment - Integrated in HBase-TRUNK #2641 (See https://builds.apache.org/job/HBase-TRUNK/2641/ ) HBASE-5228 . [REST] Rip out transform feature apurtell : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/MultiRowResource.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/RowResource.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/transform /hbase/trunk/src/test/java/org/apache/hadoop/hbase/rest/TestTransform.java
        Hide
        hudson Hudson added a comment -

        Integrated in HBase-0.92 #253 (See https://builds.apache.org/job/HBase-0.92/253/)
        HBASE-5228. [REST] Rip out transform feature

        apurtell :
        Files :

        • /hbase/branches/0.92/CHANGES.txt
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/rest/MultiRowResource.java
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/rest/RowResource.java
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/rest/transform
        • /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/rest/TestTransform.java
        Show
        hudson Hudson added a comment - Integrated in HBase-0.92 #253 (See https://builds.apache.org/job/HBase-0.92/253/ ) HBASE-5228 . [REST] Rip out transform feature apurtell : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/rest/MultiRowResource.java /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/rest/RowResource.java /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/rest/transform /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/rest/TestTransform.java
        Hide
        apurtell@yahoo.com Andrew Purtell added a comment -

        There could be more going on, but let's get this problem settled and keep digging. I'll commit to trunk, 0.92, and 0.90 tonight after the meetup.

        Show
        apurtell@yahoo.com Andrew Purtell added a comment - There could be more going on, but let's get this problem settled and keep digging. I'll commit to trunk, 0.92, and 0.90 tonight after the meetup.
        Hide
        xodarap Ben West added a comment -

        @Andrew, Zhihong: Confirmed that I'm no longer seeing all those threads blocked on the HTableDescriptor call.

        I'm not seeing a huge performance improvement, but I switched boxes in between tests (and am having trouble reverting to 0.90.4), so if it's faster for you guys I'll believe that's the cause.

        Show
        xodarap Ben West added a comment - @Andrew, Zhihong: Confirmed that I'm no longer seeing all those threads blocked on the HTableDescriptor call. I'm not seeing a huge performance improvement, but I switched boxes in between tests (and am having trouble reverting to 0.90.4), so if it's faster for you guys I'll believe that's the cause.
        Hide
        xodarap Ben West added a comment -

        @Zhihong, Andrew: yes, that stack trace was prior to Andrew's patch. Installing the patch now, will update with results.

        Show
        xodarap Ben West added a comment - @Zhihong, Andrew: yes, that stack trace was prior to Andrew's patch. Installing the patch now, will update with results.
        Hide
        apurtell Andrew Purtell added a comment -

        @Ted: I observed the blocking on this code path eliminated in runs under jprofiler after the patch was applied.

        @Ben: It would be good to confirm this on your side.

        Show
        apurtell Andrew Purtell added a comment - @Ted: I observed the blocking on this code path eliminated in runs under jprofiler after the patch was applied. @Ben: It would be good to confirm this on your side.
        Hide
        zhihyu@ebaysf.com Ted Yu added a comment -

        @Ben:
        The above strace was captured without Andy's patch, right ?

        Do you want to try out his patch and see if REST server gets faster ?

        Thanks

        Show
        zhihyu@ebaysf.com Ted Yu added a comment - @Ben: The above strace was captured without Andy's patch, right ? Do you want to try out his patch and see if REST server gets faster ? Thanks
        Hide
        xodarap Ben West added a comment -

        Thanks Andrew for this patch.

        For others who might run into this: running a jstack on our REST server revealed that virtually every thread was hung waiting on a table descriptor:

           java.lang.Thread.State: WAITING (on object monitor)
        	at java.lang.Object.wait(Native Method)
        	at java.lang.Object.wait(Object.java:485)
        	at org.apache.hadoop.hbase.ipc.HBaseClient.call(HBaseClient.java:885)
        	- locked <0x000000070218e7d0> (a org.apache.hadoop.hbase.ipc.HBaseClient$Call)
        	at org.apache.hadoop.hbase.ipc.WritableRpcEngine$Invoker.invoke(WritableRpcEngine.java:150)
        	at $Proxy61.getHTableDescriptors(Unknown Source)
        	at org.apache.hadoop.hbase.client.HConnectionManager$HConnectionImplementation.getHTableDescriptor(HConnectionManager.java:1871)
        	at org.apache.hadoop.hbase.client.HTable.getTableDescriptor(HTable.java:404)
        	at org.apache.hadoop.hbase.client.HTablePool$PooledHTable.getTableDescriptor(HTablePool.java:344)
        	at org.apache.hadoop.hbase.rest.RowResultGenerator.<init>(RowResultGenerator.java:64)
        	at org.apache.hadoop.hbase.rest.ResultGenerator.fromRowSpec(ResultGenerator.java:35)
        	at org.apache.hadoop.hbase.rest.RowResource.get(RowResource.java:86)
        

        So if your REST server is running slowly on an affected version, I guess try running a jstack and see if you see this behavior.

        Show
        xodarap Ben West added a comment - Thanks Andrew for this patch. For others who might run into this: running a jstack on our REST server revealed that virtually every thread was hung waiting on a table descriptor: java.lang. Thread .State: WAITING (on object monitor) at java.lang. Object .wait(Native Method) at java.lang. Object .wait( Object .java:485) at org.apache.hadoop.hbase.ipc.HBaseClient.call(HBaseClient.java:885) - locked <0x000000070218e7d0> (a org.apache.hadoop.hbase.ipc.HBaseClient$Call) at org.apache.hadoop.hbase.ipc.WritableRpcEngine$Invoker.invoke(WritableRpcEngine.java:150) at $Proxy61.getHTableDescriptors(Unknown Source) at org.apache.hadoop.hbase.client.HConnectionManager$HConnectionImplementation.getHTableDescriptor(HConnectionManager.java:1871) at org.apache.hadoop.hbase.client.HTable.getTableDescriptor(HTable.java:404) at org.apache.hadoop.hbase.client.HTablePool$PooledHTable.getTableDescriptor(HTablePool.java:344) at org.apache.hadoop.hbase. rest .RowResultGenerator.<init>(RowResultGenerator.java:64) at org.apache.hadoop.hbase. rest .ResultGenerator.fromRowSpec(ResultGenerator.java:35) at org.apache.hadoop.hbase. rest .RowResource.get(RowResource.java:86) So if your REST server is running slowly on an affected version, I guess try running a jstack and see if you see this behavior.
        Hide
        stack stack added a comment -

        You can apply to 0.92 branch Andrew if you want given I've tagged the RC.

        Show
        stack stack added a comment - You can apply to 0.92 branch Andrew if you want given I've tagged the RC.
        Hide
        apurtell Andrew Purtell added a comment -

        Test results for 0.92:

        Tests in error: 
          testMasterFailoverWithMockedRIT(org.apache.hadoop.hbase.master.TestMasterFailover): test timed out after 180000 milliseconds
          testRetrying(org.apache.hadoop.hbase.catalog.TestMetaReaderEditor): test timed out after 180000 milliseconds
          testGetRegionsCatalogTables(org.apache.hadoop.hbase.catalog.TestMetaReaderEditor): Failed after attempts=10, exceptions:
          testTableExists(org.apache.hadoop.hbase.catalog.TestMetaReaderEditor): Connection refused
          testGetRegion(org.apache.hadoop.hbase.catalog.TestMetaReaderEditor): Connection refused
          testScanMetaForTable(org.apache.hadoop.hbase.catalog.TestMetaReaderEditor): Failed after attempts=10, exceptions:
        

        Tests run: 1076, Failures: 0, Errors: 1, Skipped: 8

        Test results for trunk:

        Results :
        
        Tests run: 533, Failures: 0, Errors: 0, Skipped: 2
        

        Test failures for 0.92 are worrisome, but not related to this patch.

        Show
        apurtell Andrew Purtell added a comment - Test results for 0.92: Tests in error: testMasterFailoverWithMockedRIT(org.apache.hadoop.hbase.master.TestMasterFailover): test timed out after 180000 milliseconds testRetrying(org.apache.hadoop.hbase.catalog.TestMetaReaderEditor): test timed out after 180000 milliseconds testGetRegionsCatalogTables(org.apache.hadoop.hbase.catalog.TestMetaReaderEditor): Failed after attempts=10, exceptions: testTableExists(org.apache.hadoop.hbase.catalog.TestMetaReaderEditor): Connection refused testGetRegion(org.apache.hadoop.hbase.catalog.TestMetaReaderEditor): Connection refused testScanMetaForTable(org.apache.hadoop.hbase.catalog.TestMetaReaderEditor): Failed after attempts=10, exceptions: Tests run: 1076, Failures: 0, Errors: 1, Skipped: 8 Test results for trunk: Results : Tests run: 533, Failures: 0, Errors: 0, Skipped: 2 Test failures for 0.92 are worrisome, but not related to this patch.
        Hide
        lhofhansl Lars Hofhansl added a comment -

        +1 on idea and patch.

        Show
        lhofhansl Lars Hofhansl added a comment - +1 on idea and patch.
        Hide
        zhihyu@ebaysf.com Ted Yu added a comment -

        +1 if tests pass.

        The current patch wouldn't apply cleanly on TRUNK, FYI

        Show
        zhihyu@ebaysf.com Ted Yu added a comment - +1 if tests pass. The current patch wouldn't apply cleanly on TRUNK, FYI
        Hide
        apurtell Andrew Purtell added a comment -

        @Stack: Ok

        @Ted: Thanks for the patch review, will fix on commit if you think this is good to go in otherwise.

        Show
        apurtell Andrew Purtell added a comment - @Stack: Ok @Ted: Thanks for the patch review, will fix on commit if you think this is good to go in otherwise.
        Hide
        zhihyu@ebaysf.com Ted Yu added a comment -
             HBaseAdmin admin = new HBaseAdmin(servlet.getConfiguration());
        -    return admin.tableExists(table);
        +    boolean result = admin.tableExists(table);
        +    admin.close();
        

        Looks like admin.close() should be enclosed in finally block.

        Show
        zhihyu@ebaysf.com Ted Yu added a comment - HBaseAdmin admin = new HBaseAdmin(servlet.getConfiguration()); - return admin.tableExists(table); + boolean result = admin.tableExists(table); + admin.close(); Looks like admin.close() should be enclosed in finally block.
        Hide
        zhihyu@ebaysf.com Ted Yu added a comment -

        @Andy:
        Do you want Hadoop QA to run the patch through tests ?

        Show
        zhihyu@ebaysf.com Ted Yu added a comment - @Andy: Do you want Hadoop QA to run the patch through tests ?
        Hide
        stack stack added a comment -

        Can this be in 0.92.1 Andrew? I'd start a 0.92.1 candidate immediately after 0.92.0 release?

        Show
        stack stack added a comment - Can this be in 0.92.1 Andrew? I'd start a 0.92.1 candidate immediately after 0.92.0 release?
        Hide
        apurtell Andrew Purtell added a comment -

        Jack confirmed in private correspondence that this can come out. I'd like to do it ASAP as part of 0.92 (and any further 0.90 release) so we don't pick up a user of this misfeature.

        Show
        apurtell Andrew Purtell added a comment - Jack confirmed in private correspondence that this can come out. I'd like to do it ASAP as part of 0.92 (and any further 0.90 release) so we don't pick up a user of this misfeature.

          People

          • Assignee:
            apurtell Andrew Purtell
            Reporter:
            apurtell Andrew Purtell
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development