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

Manage Connections Through Reference Counts

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Duplicate
    • Affects Version/s: 0.90.2
    • Fix Version/s: 0.92.0
    • Component/s: Client
    • Labels:
      None

      Description

      As of now, the onus is on the developer to explicitly close connections once they're done using them. Furthermore, since connections are managed based on the identity of the Configuration object, one is forced to clone the configuration object in order to be able to clean it up safely (for a case in point, see HTablePool's constructor). As a matter of fact, this issue has been well-documented in the HConnectionManager class:

      But sharing connections makes clean up of HConnection instances a little awkward. Currently, clients cleanup by calling #deleteConnection(Configuration, boolean). This will shutdown the zookeeper connection the HConnection was using and clean up all HConnection resources as well as stopping proxies to servers out on the cluster. Not running the cleanup will not end the world; it'll just stall the closeup some and spew some zookeeper connection failed messages into the log. Running the cleanup on a HConnection that is subsequently used by another will cause breakage so be careful running cleanup. To create a HConnection that is not shared by others, you can create a new Configuration instance, pass this new instance to #getConnection(Configuration), and then when done, close it up by doing something like the following:

       Configuration newConfig = new Configuration(originalConf);
       HConnection connection = HConnectionManager.getConnection(newConfig);
       // Use the connection to your hearts' delight and then when done...
       HConnectionManager.deleteConnection(newConfig, true);
      

      Here, we propose a reference-count based mechanism for managing connections that will allow HTables to clean up after themselves. In particular, we extend the HConnectionInterface interface so as to facilitate reference counting, where, typically, a reference indicates that it is being used by a HTable, although there could be other sources.

      To elaborate, when a HTable is constructed, it increments the reference count on the connection given to it. Similarly, when it is closed, that reference count is decremented. In the event there are no more references to that connection, HTable#close takes it upon itself to delete the connection, thereby sparing the developer from doing so.

      1. HBASE-3766.patch
        7 kB
        Karthick Sankarachary
      2. HBASE-3766.patch
        4 kB
        Karthick Sankarachary
      3. HBASE-3766.patch
        4 kB
        Karthick Sankarachary

        Issue Links

          Activity

          Hide
          jdcryans Jean-Daniel Cryans added a comment -

          First, thanks for starting this discussion.

          About the patch, I see you increment in HTable's constructor, but if the user is reusing a conf object then the counter becomes wrong right?

          Also the counter itself isn't accessed in a synchronize way (as multiple threads using different HTables would need) so this could pose a problem.

          Show
          jdcryans Jean-Daniel Cryans added a comment - First, thanks for starting this discussion. About the patch, I see you increment in HTable's constructor, but if the user is reusing a conf object then the counter becomes wrong right? Also the counter itself isn't accessed in a synchronize way (as multiple threads using different HTables would need) so this could pose a problem.
          Hide
          karthick Karthick Sankarachary added a comment -

          About the patch, I see you increment in HTable's constructor, but if the user is reusing a conf object then the counter becomes wrong right?

          Let's say that a certain conf object is used to create 2 HTable instances. As a result, the reference count on the underlying connection object will be incremented twice, which is the desired behavior. Then, when you close one of the HTables, it will decrement the reference count on the connection, but not close it, since it is still in-use. Later, when you close the other HTable, the reference count will drop to zero, at which point, the underlying connection gets deleted. On the other hand, if you use different conf objects to create the HTables, they will end up having separate reference counts. Please correct me if I'm misunderstood the question.

          Also the counter itself isn't accessed in a synchronize way (as multiple threads using different HTables would need) so this could pose a problem.

          Good catch. I revised the patch such that the reference count is accessed atomically.

          Show
          karthick Karthick Sankarachary added a comment - About the patch, I see you increment in HTable's constructor, but if the user is reusing a conf object then the counter becomes wrong right? Let's say that a certain conf object is used to create 2 HTable instances. As a result, the reference count on the underlying connection object will be incremented twice, which is the desired behavior. Then, when you close one of the HTables, it will decrement the reference count on the connection, but not close it, since it is still in-use. Later, when you close the other HTable, the reference count will drop to zero, at which point, the underlying connection gets deleted. On the other hand, if you use different conf objects to create the HTables, they will end up having separate reference counts. Please correct me if I'm misunderstood the question. Also the counter itself isn't accessed in a synchronize way (as multiple threads using different HTables would need) so this could pose a problem. Good catch. I revised the patch such that the reference count is accessed atomically.
          Hide
          jdcryans Jean-Daniel Cryans added a comment -

          Ah I see, sorry I thought you were adding the reference counting in HCM, but I see it's in HCI (which does make a lot more sense). That works yeah.

          So two other things:

          This is dangerous because you are closing the connections to all the regions servers and the master, you want to pass false:

          + HConnectionManager.deleteConnection(this.configuration, true);

          Then I'd like to see some unit testing.

          Also I'm not sure we want this in 0.90.3, targeting 0.92 would be better.

          Show
          jdcryans Jean-Daniel Cryans added a comment - Ah I see, sorry I thought you were adding the reference counting in HCM, but I see it's in HCI (which does make a lot more sense). That works yeah. So two other things: This is dangerous because you are closing the connections to all the regions servers and the master, you want to pass false: + HConnectionManager.deleteConnection(this.configuration, true); Then I'd like to see some unit testing. Also I'm not sure we want this in 0.90.3, targeting 0.92 would be better.
          Hide
          karthick Karthick Sankarachary added a comment -

          This is dangerous because you are closing the connections to all the regions servers and the master, you want to pass false:

          + HConnectionManager.deleteConnection(this.configuration, true);

          Fair enough.

          Then I'd like to see some unit testing.

          Added a test case in TestFromClientSide called testConnectionReferenceCount.

          Note that, I had to make an exception in the case of the meta table (viz., HConstants#META_TABLE_NAME). Specifically, I do not increment the reference count in that case, because apparently the client (see MetaScanner) doesn't bother closing that table. Furthermore, in order to keep tables already closed from decrementing the reference count, I had to make the HTable#close method idempotent .

          Also I'm not sure we want this in 0.90.3, targeting 0.92 would be better.

          Done.

          Show
          karthick Karthick Sankarachary added a comment - This is dangerous because you are closing the connections to all the regions servers and the master, you want to pass false: + HConnectionManager.deleteConnection(this.configuration, true); Fair enough. Then I'd like to see some unit testing. Added a test case in TestFromClientSide called testConnectionReferenceCount . Note that, I had to make an exception in the case of the meta table (viz., HConstants#META_TABLE_NAME ). Specifically, I do not increment the reference count in that case, because apparently the client (see MetaScanner ) doesn't bother closing that table. Furthermore, in order to keep tables already closed from decrementing the reference count, I had to make the HTable#close method idempotent . Also I'm not sure we want this in 0.90.3, targeting 0.92 would be better. Done.
          Hide
          stack stack added a comment -

          Is this issue still relevant after the addition of ref counting over in HBASE-3777?

          Show
          stack stack added a comment - Is this issue still relevant after the addition of ref counting over in HBASE-3777 ?
          Hide
          karthick Karthick Sankarachary added a comment -

          No, it's not relevant anymore, given that we ended up doing it as part of HBASE-3777. For the sake of completeness, I'll add a comment here later on how reference counting ended up being implemented.

          Show
          karthick Karthick Sankarachary added a comment - No, it's not relevant anymore, given that we ended up doing it as part of HBASE-3777 . For the sake of completeness, I'll add a comment here later on how reference counting ended up being implemented.
          Hide
          lars_francke Lars Francke added a comment -

          This issue was closed as part of a bulk closing operation on 2015-11-20. All issues that have been resolved and where all fixVersions have been released have been closed (following discussions on the mailing list).

          Show
          lars_francke Lars Francke added a comment - This issue was closed as part of a bulk closing operation on 2015-11-20. All issues that have been resolved and where all fixVersions have been released have been closed (following discussions on the mailing list).

            People

            • Assignee:
              karthick Karthick Sankarachary
              Reporter:
              karthick Karthick Sankarachary
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 1h
                1h
                Remaining:
                Remaining Estimate - 1h
                1h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development