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

HBaseAdmin creates new configurations in getCatalogTracker

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.90.1
    • Fix Version/s: 0.90.3
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      HBaseAdmin.getCatalogTracker creates new Configuration every time it's called, instead HBA should reuse the same one and do the copy inside the constructor.

      1. HBASE-3734.patch
        2 kB
        Jean-Daniel Cryans
      2. HBASE-3734-v2.patch
        4 kB
        Jean-Daniel Cryans

        Activity

        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).
        Hide
        hudson Hudson added a comment -

        Integrated in HBase-TRUNK #1846 (See https://hudson.apache.org/hudson/job/HBase-TRUNK/1846/)

        Show
        hudson Hudson added a comment - Integrated in HBase-TRUNK #1846 (See https://hudson.apache.org/hudson/job/HBase-TRUNK/1846/ )
        Hide
        jdcryans Jean-Daniel Cryans added a comment -

        Committed to branch and trunk, thanks for the reviews Stack!

        Show
        jdcryans Jean-Daniel Cryans added a comment - Committed to branch and trunk, thanks for the reviews Stack!
        Hide
        stack stack added a comment -

        +1 Lets go w/ this.

        Show
        stack stack added a comment - +1 Lets go w/ this.
        Hide
        jdcryans Jean-Daniel Cryans added a comment -

        Patch that fixes the places where it wasn't cleaning the CT, also I still do the configuration copy in the constructor to be safer.

        Show
        jdcryans Jean-Daniel Cryans added a comment - Patch that fixes the places where it wasn't cleaning the CT, also I still do the configuration copy in the constructor to be safer.
        Hide
        stack stack added a comment -

        Is this the right way to go? The javadoc on #getCatalogTracker says:

        @return A new CatalogTracker instance; call {@link #cleanupCatalogTracker(CatalogTracker)} to cleanup the returned catalog tracker.

        Is the problem that cleanupCatalogTracker is not being called?

        Show
        stack stack added a comment - Is this the right way to go? The javadoc on #getCatalogTracker says: @return A new CatalogTracker instance; call {@link #cleanupCatalogTracker(CatalogTracker)} to cleanup the returned catalog tracker. Is the problem that cleanupCatalogTracker is not being called?
        Hide
        jdcryans Jean-Daniel Cryans added a comment -

        Patch that adds the copy in HBA's constructor and removes the copy in getCatalogTracker, passes unit tests.

        Show
        jdcryans Jean-Daniel Cryans added a comment - Patch that adds the copy in HBA's constructor and removes the copy in getCatalogTracker, passes unit tests.
        Hide
        jdcryans Jean-Daniel Cryans added a comment -

        The main concern is that different components in the same JVM have different connections, so a closed client connection doesn't also close a region server connection (like in most tests). The HBaseAdmin constructor should just copy the configuration upfront.

        Show
        jdcryans Jean-Daniel Cryans added a comment - The main concern is that different components in the same JVM have different connections, so a closed client connection doesn't also close a region server connection (like in most tests). The HBaseAdmin constructor should just copy the configuration upfront.
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        HBaseAdmin.getCatalogTracker() calls HConnectionManager.getConnection() which uses Configuration as key in HBASE_INSTANCES.
        Since we want to reuse connection to the same cluster, maybe we can do this:

              connection = HBASE_INSTANCES.get(conf.get("hbase.zookeeper.quorum"));
              if (connection == null) {
                connection = new HConnectionImplementation(conf);
                HBASE_INSTANCES.put(conf.get("hbase.zookeeper.quorum"), connection);
              }
        

        Of course, the above would break deleteConnection() which requires reference counting.
        So maybe we can use a wrapper for (refcount, connection) tuple and store the wrapper object as value in HBASE_INSTANCES ?

        Show
        yuzhihong@gmail.com Ted Yu added a comment - HBaseAdmin.getCatalogTracker() calls HConnectionManager.getConnection() which uses Configuration as key in HBASE_INSTANCES. Since we want to reuse connection to the same cluster, maybe we can do this: connection = HBASE_INSTANCES.get(conf.get( "hbase.zookeeper.quorum" )); if (connection == null ) { connection = new HConnectionImplementation(conf); HBASE_INSTANCES.put(conf.get( "hbase.zookeeper.quorum" ), connection); } Of course, the above would break deleteConnection() which requires reference counting. So maybe we can use a wrapper for (refcount, connection) tuple and store the wrapper object as value in HBASE_INSTANCES ?

          People

          • Assignee:
            jdcryans Jean-Daniel Cryans
            Reporter:
            jdcryans Jean-Daniel Cryans
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development