Uploaded image for project: 'HBase'
  1. HBase
  2. HBASE-19043

Purge TableWrapper and CoprocessorHConnnection

    XMLWordPrintableJSON

Details

    • Task
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • None
    • 2.0.0-alpha-4, 2.0.0
    • Coprocessors
    • None
    • Reviewed
    • Hide
      Removes getTable from the CoprocessorEnvrionment Interface and from the BaseEnvironment implementation. Also removes TableWrapper and CoprocessorHConnection, two classes that were used by BaseEnvironment to keep a tag on Tables created by Coprocessors that BaseEnvironment might close them out on #shutdown.

      Long after these classes and methods were added, in HBase 1.0.0, we moved to a mode where management of Tables was shifted from HBase to the Client; the Client is to manage lifecycle. Table also became a (relatively) lightweight construct so folks are used to getting a Table instance, using it, and then immediately closing it when done.

      Coprocessors should do the same in hbase2.0.0.

      CoprocessorHConnection short-circuited RPC. This feature has since been integrated into Server Connections; when they create a Connection, they get one that will short-circuit if the request is to a localhost so no need of CoprocessorHConnection any more.

      Coprocessors get the Server Connection when they ask for a Connection from their *CoprocessorEnvironment.
      Show
      Removes getTable from the CoprocessorEnvrionment Interface and from the BaseEnvironment implementation. Also removes TableWrapper and CoprocessorHConnection, two classes that were used by BaseEnvironment to keep a tag on Tables created by Coprocessors that BaseEnvironment might close them out on #shutdown. Long after these classes and methods were added, in HBase 1.0.0, we moved to a mode where management of Tables was shifted from HBase to the Client; the Client is to manage lifecycle. Table also became a (relatively) lightweight construct so folks are used to getting a Table instance, using it, and then immediately closing it when done. Coprocessors should do the same in hbase2.0.0. CoprocessorHConnection short-circuited RPC. This feature has since been integrated into Server Connections; when they create a Connection, they get one that will short-circuit if the request is to a localhost so no need of CoprocessorHConnection any more. Coprocessors get the Server Connection when they ask for a Connection from their *CoprocessorEnvironment.

    Description

      Repeating note from dev list today which had assent from apurtell and appy:

      Subject: [DISCUSS] Punt the Coprocessor TableWrapper and CoprocessorHConnection?
      To: HBase Dev List <dev@hbase.apache.org>
      
      tl;dr I believe the original intent for TableWrapper and
      CoprocessorHConnection can now be gotten elsewhere so we should purge these
      classes.
      
      In base CoprocessorEnvironment, there are methods to return a Table
      instance. There are none to return an Admin. In our code base, the only
      user is the AccessController. Phoenix uses it twice in its Indexer
      implementation.
      
      When you call CE#getTable, the default implementation in
      BaseEnvironemnt calls HTableWrapper.createWrapper which takes a
      BaseEnvironment List in which we keep all Table instances. On shutdown of
      the coprocessor, the list is iterated and all tables are closed.
      
      Going via TableWrapper, the class comment says:
      
      * A wrapper for HTable. Can be used to restrict privilege.
      *
      * Currently it just helps to track tables opened by a Coprocessor and
      * facilitate close of them if it is aborted.
      *
      * We also disallow row locking.
      *
      * There is nothing now that will stop a coprocessor from using HTable
      * objects directly instead of this API, but in the future we intend to
      * analyze coprocessor implementations as they are loaded and reject those
      * which attempt to use objects and methods outside the Environment
      * sandbox.
      
      
      TableWrapper by my reading delegates all calls to a Table instance with no
      interception (there is not rowlocking to override anymore). On open, we do
      ensure the Table is up on a CoprocessorHConnection which does the following:
      
      * Connection to an HTable from within a Coprocessor. We can do some
      nice tricks since we know we
      * are on a regionserver, for instance skipping the full
      serialization/deserialization of objects
      * when talking to the server.
      
      
      The above 'trick' is now commonplace in servers as the Master and
      RegionServer always make Connections that will short-circuit if an
      opportunity.
      
      As I read TableWrapper and CoprocessorHConnection, they were written at
      another time when Table construction was heavyweight and Table#close was
      not expected of clients and before the introduction of the general
      Server-side short-circuit Connection facility.
      
      Unless objection, I think we should purge them.
      
      Writing here in case I'm missing some key facility they provide.
      
      

      See http://search-hadoop.com/m/HBase/YGbb8uCtg26VkBy?subj=+DISCUSS+Punt+the+Coprocessor+TableWrapper+and+CoprocessorHConnection+

      This issue is about removing these classes.

      Attachments

        1. HBASE-19043.master.007.patch
          50 kB
          Michael Stack
        2. HBASE-19043.master.007.patch
          50 kB
          Michael Stack
        3. HBASE-19043.master.006.patch
          49 kB
          Michael Stack
        4. HBASE-19043.master.005.patch
          49 kB
          Michael Stack
        5. HBASE-19043.master.004.patch
          49 kB
          Michael Stack
        6. HBASE-19043.master.003.patch
          49 kB
          Michael Stack
        7. HBASE-19043.master.002.patch
          49 kB
          Michael Stack
        8. HBASE-19043.master.001.patch
          49 kB
          Michael Stack

        Issue Links

          Activity

            People

              stack Michael Stack
              stack Michael Stack
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: