HBase
  1. HBase
  2. HBASE-5228

[REST] Rip out "transform" feature

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major 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 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 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 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 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 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 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 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 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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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 added a comment -

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

        Show
        stack added a comment - You can apply to 0.92 branch Andrew if you want given I've tagged the RC.
        Hide
        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
        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
        Lars Hofhansl added a comment -

        +1 on idea and patch.

        Show
        Lars Hofhansl added a comment - +1 on idea and patch.
        Hide
        Ted Yu added a comment -

        +1 if tests pass.

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

        Show
        Ted Yu added a comment - +1 if tests pass. The current patch wouldn't apply cleanly on TRUNK, FYI
        Hide
        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
        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
        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
        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
        Ted Yu added a comment -

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

        Show
        Ted Yu added a comment - @Andy: Do you want Hadoop QA to run the patch through tests ?
        Hide
        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 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
        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
        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:
            Andrew Purtell
            Reporter:
            Andrew Purtell
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development