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

Potentially too many connections may be opened if ThreadLocalPool or RoundRobinPool is used

    Details

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

      Description

      See 'Problem with hbase.client.ipc.pool.type=threadlocal in trunk' discussion started by Lars George.

      From Lars Hofhansl:
      Looking at HBaseClient.getConnection(...) I see this:

           synchronized (connections) {
             connection = connections.get(remoteId);
             if (connection == null) {
               connection = new Connection(remoteId);
               connections.put(remoteId, connection);
             }
           }
      

      At the same time PoolMap.ThreadLocalPool.put is defined like this:

         public R put(R resource) {
           R previousResource = get();
           if (previousResource == null) {
      ...
             if (poolSize.intValue() >= maxSize) {
               return null;
             }
      ...
         }
      

      So... If the ThreadLocalPool reaches its capacity it always returns null and hence all new threads will create a
      new connection every time getConnection is called!

      I have also verified with a test program that works fine as long as the number of client threads (which include
      the threads in HTable's threadpool of course) is < poolsize. Once that is no longer the case the number of
      connections "explodes" and the program dies with OOMEs (mostly because each Connection is associated with
      yet another thread).

      It's not clear what should happen, though. Maybe (1) the ThreadLocalPool should not have a limit, or maybe
      (2) allocations past the pool size should throw an exception (i.e. there's a hard limit), or maybe (3) in that case
      a single connection is returned for all threads while the pool it over its limit or (4) we start round robin with the other
      connection in the other thread locals.

      For #1 means that the number of client threads needs to be more carefully managed by the client app.
      In this case it would also be somewhat pointless that Connection have their own threads, we just pass stuff
      between threads.
      #2 would work, but puts more logic in the client.
      #3 would lead to hard to debug performance issues.
      And #4 is messy

      From Ted Yu:
      For HBaseClient, at least the javadoc doesn't match:

         * @param config configuration
         * @return either a {@link PoolType#Reusable} or {@link PoolType#ThreadLocal}
         */
        private static PoolType getPoolType(Configuration config) {
          return PoolType.valueOf(config.get(HConstants.HBASE_CLIENT_IPC_POOL_TYPE),
              PoolType.RoundRobin, PoolType.ThreadLocal);
      

      I think for RoundRobinPool, we shouldn't allow maxSize to be Integer#MAX_VALUE. Otherwise connection explosion described by Lars may incur.

      1. 4150.txt
        0.6 kB
        Ted Yu
      2. 4150-1.txt
        0.6 kB
        Lars Hofhansl
      3. 5140-2.txt
        2 kB
        Karthick Sankarachary
      4. HBASE-4150_final.patch
        3 kB
        Gary Helmling
      5. HBASE-4150-DOC.patch
        4 kB
        Karthick Sankarachary

        Activity

        Hide
        karthick Karthick Sankarachary added a comment -

        I agree that if we don't somehow cap the pool size, we could potentially run into connection overload issues.

        How about if we enforce a hard limit on the pool size? For instance, we could use "hbase.regionserver.handler.count", for lack of a better variable, as the cap for the pool size.

        Further, in the case of a thread-local pool, we might be better off not enforcing that limit at all. If the number of threads goes over the pool size, then we could log a warning message that asks the user to switch to a round-robin pool. If that doesn't help, then we might want to consider throwing an exception, making sure to skip all of the retry logic as it bubbles up.

        Show
        karthick Karthick Sankarachary added a comment - I agree that if we don't somehow cap the pool size, we could potentially run into connection overload issues. How about if we enforce a hard limit on the pool size? For instance, we could use "hbase.regionserver.handler.count", for lack of a better variable, as the cap for the pool size. Further, in the case of a thread-local pool, we might be better off not enforcing that limit at all. If the number of threads goes over the pool size, then we could log a warning message that asks the user to switch to a round-robin pool. If that doesn't help, then we might want to consider throwing an exception, making sure to skip all of the retry logic as it bubbles up.
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        "hbase.regionserver.handler.count" is server side config. Configuration object on client side may not have it.

        I am in favor of throwing exception in the case of a thread-local pool to inform user that they should honor the limit.

        Show
        yuzhihong@gmail.com Ted Yu added a comment - "hbase.regionserver.handler.count" is server side config. Configuration object on client side may not have it. I am in favor of throwing exception in the case of a thread-local pool to inform user that they should honor the limit.
        Hide
        ghelmling Gary Helmling added a comment -

        Hmm, maybe I don't understand the context, but shouldn't a thread-local pool expand to have one instance per calling thread? I wouldn't expect any hard limit to be imposed. Why would you want a limit in this case? Imposing a pool size limit here is effectively just limiting number of allowed threads, but as a side effect. Am I missing something?

        It seems to me if you hit an OOME due to creating too many threads you shouldn't create so many threads.

        Show
        ghelmling Gary Helmling added a comment - Hmm, maybe I don't understand the context, but shouldn't a thread-local pool expand to have one instance per calling thread? I wouldn't expect any hard limit to be imposed. Why would you want a limit in this case? Imposing a pool size limit here is effectively just limiting number of allowed threads, but as a side effect. Am I missing something? It seems to me if you hit an OOME due to creating too many threads you shouldn't create so many threads.
        Hide
        lhofhansl Lars Hofhansl added a comment -

        The original post was from me actually (not Lars George)...

        I agree that no limit for threadlocal pool is also a good option (my #1 above).
        The actual number of threads is actually a bit hard to predict because of the thread pool in HTable (and even harder when using HTablePool), though.

        Show
        lhofhansl Lars Hofhansl added a comment - The original post was from me actually (not Lars George)... I agree that no limit for threadlocal pool is also a good option (my #1 above). The actual number of threads is actually a bit hard to predict because of the thread pool in HTable (and even harder when using HTablePool), though.
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        Looks like we should describe the above scenario in javadoc and book.

        Show
        yuzhihong@gmail.com Ted Yu added a comment - Looks like we should describe the above scenario in javadoc and book.
        Hide
        karthick Karthick Sankarachary added a comment -

        If we decide to leave things as they are, then as Ted suggested, we should describe the best practices in the javadoc and the book. If the client application comprises a small number of threads that perform a lot of scan- or batch-intensive operations, then you may want to consider using thread-local pools. For applications with a large number of threads, you may want to use a bounded round-robin pool, which will require you to fine-tune its size (a number between 5 and 10 might serve as a good starting point). At any rate, the total number of connections should not exceed the hard limits set on the underlying operating system resources. For instance, on *nix systems, the pool size should be lower than the maximum number of open file descriptors (`ulimit -n`).

        Show
        karthick Karthick Sankarachary added a comment - If we decide to leave things as they are, then as Ted suggested, we should describe the best practices in the javadoc and the book. If the client application comprises a small number of threads that perform a lot of scan- or batch-intensive operations, then you may want to consider using thread-local pools. For applications with a large number of threads, you may want to use a bounded round-robin pool, which will require you to fine-tune its size (a number between 5 and 10 might serve as a good starting point). At any rate, the total number of connections should not exceed the hard limits set on the underlying operating system resources. For instance, on *nix systems, the pool size should be lower than the maximum number of open file descriptors (`ulimit -n`).
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        Doug, our doc master, agrees to take over this task.

        Show
        yuzhihong@gmail.com Ted Yu added a comment - Doug, our doc master, agrees to take over this task.
        Hide
        lhofhansl Lars Hofhansl added a comment -

        I don't think we can leave it the way it is for the threadlocal pool.

        The behavior now is worse than having no limit at all.
        Currently, if the number of threads is greater than than the pool-size a new connection will be created for every request (see the code snippet above). That is clearly not what was intended with this.

        So either threadlocal pool should have no limit (at least then we only have one connection per thread), or the it should fail when it gets past the limit.

        Show
        lhofhansl Lars Hofhansl added a comment - I don't think we can leave it the way it is for the threadlocal pool. The behavior now is worse than having no limit at all. Currently, if the number of threads is greater than than the pool-size a new connection will be created for every request (see the code snippet above). That is clearly not what was intended with this. So either threadlocal pool should have no limit (at least then we only have one connection per thread), or the it should fail when it gets past the limit.
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        Does this look good to you Lars ?

        Show
        yuzhihong@gmail.com Ted Yu added a comment - Does this look good to you Lars ?
        Hide
        lhofhansl Lars Hofhansl added a comment -

        Yep... That stops the unbounded growth of the pool. And if somebody wants a connection per thread they can set the size to Integer.MAX_VALUE.

        Thanks Ted!

        Is this needed:
        if (maxSize != Integer.MAX_VALUE)
        ?
        (I don't think poolSize.intValue() >= Integer.MAX_VALUE is ever true. And hence if maxSize was Integer.MAX_VALUE the 2nd if would never be reached anyway.)

        I guess the documentation should state something the extend that a round-robin pool will always grow to the pool-size (at which point the connections will round-robin between threads),
        and the thread-local pool will at most grow to that size (at which point further threads will fail).
        Hence it makes sense to set the size of a threadlocal pool to Integer.MAX_VALUE and then control the number of threads, but the size of a round-robin pool should never be set to Integer.MAX_VALUE.

        Show
        lhofhansl Lars Hofhansl added a comment - Yep... That stops the unbounded growth of the pool. And if somebody wants a connection per thread they can set the size to Integer.MAX_VALUE. Thanks Ted! Is this needed: if (maxSize != Integer.MAX_VALUE) ? (I don't think poolSize.intValue() >= Integer.MAX_VALUE is ever true. And hence if maxSize was Integer.MAX_VALUE the 2nd if would never be reached anyway.) I guess the documentation should state something the extend that a round-robin pool will always grow to the pool-size (at which point the connections will round-robin between threads), and the thread-local pool will at most grow to that size (at which point further threads will fail). Hence it makes sense to set the size of a threadlocal pool to Integer.MAX_VALUE and then control the number of threads, but the size of a round-robin pool should never be set to Integer.MAX_VALUE.
        Hide
        lhofhansl Lars Hofhansl added a comment -

        I.e. this...

        Show
        lhofhansl Lars Hofhansl added a comment - I.e. this...
        Hide
        lhofhansl Lars Hofhansl added a comment -

        I guess poolSize.intValue() >= Integer.MAX_VALUE is true if you have Integer.MAX_VALUE connections (which means you also have Integer.MAX_VALUE threads, so that won't happen).

        Show
        lhofhansl Lars Hofhansl added a comment - I guess poolSize.intValue() >= Integer.MAX_VALUE is true if you have Integer.MAX_VALUE connections (which means you also have Integer.MAX_VALUE threads, so that won't happen).
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        Please grant license to ASF in the future.
        I will wait for a day before committing this change, wrapping the long line.

        Show
        yuzhihong@gmail.com Ted Yu added a comment - Please grant license to ASF in the future. I will wait for a day before committing this change, wrapping the long line.
        Hide
        lhofhansl Lars Hofhansl added a comment -

        Sorry, missed the checkbox (this is my first submission).

        Show
        lhofhansl Lars Hofhansl added a comment - Sorry, missed the checkbox (this is my first submission).
        Hide
        ghelmling Gary Helmling added a comment -

        The behavior now is worse than having no limit at all.
        Currently, if the number of threads is greater than than the pool-size a new connection will be created for every request (see the code snippet above). That is clearly not what was intended with this.

        That certainly seems broken.

        Personally I think it's a bit odd to have a limit apply to a thread local implementation, since ThreadLocal typically means I want one instance per thread. ThreadLocal usage performs its own natural limiting, so I'd be more inclined to just remove the limit checking from ThreadLocalPool.

        I also don't understand why the poolSizes variable is a static Map<ThreadLocalPool<?>,AtomicInteger>, instead of just an instance AtomicInteger variable.

        Show
        ghelmling Gary Helmling added a comment - The behavior now is worse than having no limit at all. Currently, if the number of threads is greater than than the pool-size a new connection will be created for every request (see the code snippet above). That is clearly not what was intended with this. That certainly seems broken. Personally I think it's a bit odd to have a limit apply to a thread local implementation, since ThreadLocal typically means I want one instance per thread. ThreadLocal usage performs its own natural limiting, so I'd be more inclined to just remove the limit checking from ThreadLocalPool . I also don't understand why the poolSizes variable is a static Map<ThreadLocalPool<?>,AtomicInteger> , instead of just an instance AtomicInteger variable.
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        Considering what Lars said @ 04/Aug/11 04:09, it seems that ThreadLocalPool doesn't need to honor maxSize. Hence maxSize member variable can be removed.

        Let's see what Karthick says.

        Show
        yuzhihong@gmail.com Ted Yu added a comment - Considering what Lars said @ 04/Aug/11 04:09, it seems that ThreadLocalPool doesn't need to honor maxSize. Hence maxSize member variable can be removed. Let's see what Karthick says.
        Hide
        lhofhansl Lars Hofhansl added a comment -

        Having an upper safety limit on the number connections (and the accompanying threads) created is not necessarily bad; that is especially true when the number of threads is somewhat hard to predict, for example when using HTablePool and HTables.

        The size for the threadlocal pool should just be "reasonably large" and for the round-robin pool it should be "reasonably small".

        Also, isn't PoolMap also used in HTablePool now (HBASE-2938)? Do we want the exact same behavior there?

        Show
        lhofhansl Lars Hofhansl added a comment - Having an upper safety limit on the number connections (and the accompanying threads) created is not necessarily bad; that is especially true when the number of threads is somewhat hard to predict, for example when using HTablePool and HTables. The size for the threadlocal pool should just be "reasonably large" and for the round-robin pool it should be "reasonably small". Also, isn't PoolMap also used in HTablePool now ( HBASE-2938 )? Do we want the exact same behavior there?
        Hide
        dmeil Doug Meil added a comment -

        A matter of coordination, this ticket is currently assigned to me but I'm going to wait on any doc-changes until you guys figure out what you want to do.

        Show
        dmeil Doug Meil added a comment - A matter of coordination, this ticket is currently assigned to me but I'm going to wait on any doc-changes until you guys figure out what you want to do.
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        Removing maxSize from ThreadLocalPool is a big change.
        I applied the 4150.txt to TRUNK.

        @Doug:
        Please continue with javadoc changes.

        Thanks Lars.

        Show
        yuzhihong@gmail.com Ted Yu added a comment - Removing maxSize from ThreadLocalPool is a big change. I applied the 4150.txt to TRUNK. @Doug: Please continue with javadoc changes. Thanks Lars.
        Hide
        ghelmling Gary Helmling added a comment -

        Throwing a RuntimeException where it was not thrown before in a way that could be disruptive to clients is IMO an even bigger change. Especially in this case, this is relatively likely to occur if careful attention is not paid to aligning pool size with thread allocations.

        I also don't think it's reasonable to commit a change like this when there is ongoing debate about the correct approach. I'm willing to be convinced of alternatives, but I'm not convinced this is currently the correct approach.

        -1 to this change.

        Show
        ghelmling Gary Helmling added a comment - Throwing a RuntimeException where it was not thrown before in a way that could be disruptive to clients is IMO an even bigger change. Especially in this case, this is relatively likely to occur if careful attention is not paid to aligning pool size with thread allocations. I also don't think it's reasonable to commit a change like this when there is ongoing debate about the correct approach. I'm willing to be convinced of alternatives, but I'm not convinced this is currently the correct approach. -1 to this change.
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        Patch reversed.

        @Lars:
        What do you think of removing the limit checking from ThreadLocalPool ?
        Effectively, maxSize would be Interger.MAX_VALUE

        Show
        yuzhihong@gmail.com Ted Yu added a comment - Patch reversed. @Lars: What do you think of removing the limit checking from ThreadLocalPool ? Effectively, maxSize would be Interger.MAX_VALUE
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        Gary agrees to obtain consensus from related parties and finish code change.

        Show
        yuzhihong@gmail.com Ted Yu added a comment - Gary agrees to obtain consensus from related parties and finish code change.
        Hide
        hudson Hudson added a comment -

        Integrated in HBase-TRUNK #2082 (See https://builds.apache.org/job/HBase-TRUNK/2082/)
        HBASE-4150 reverse patch
        HBASE-4150 Potentially too many connections may be opened if ThreadLocalPool
        or RoundRobinPool is used

        tedyu :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/PoolMap.java
        • /hbase/trunk/CHANGES.txt

        tedyu :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/PoolMap.java
        • /hbase/trunk/CHANGES.txt
        Show
        hudson Hudson added a comment - Integrated in HBase-TRUNK #2082 (See https://builds.apache.org/job/HBase-TRUNK/2082/ ) HBASE-4150 reverse patch HBASE-4150 Potentially too many connections may be opened if ThreadLocalPool or RoundRobinPool is used tedyu : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/PoolMap.java /hbase/trunk/CHANGES.txt tedyu : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/PoolMap.java /hbase/trunk/CHANGES.txt
        Hide
        karthick Karthick Sankarachary added a comment -

        I tend to agree with Gary in that it doesn't really make sense to cap the thread-local pool. Having the PoolMap throw an exception seems a little too disruptive. I've attached a [patch|^4150-2.txt] that removes the limit check from ThreadLocalPool and fixes the javadoc in HBaseClient. This way, we at least ensure that the client will create as many Connection objects as threads, no more no less.

        Note that in the PoolMap, we associate a resource Pool with every key, ergo the need for using a map instead of a number for the type for the poolSizes variable. So, in the case of HBaseClient#connections, we'll essentially create a Pool of Connection}}s for every {{ConnectionId, which denotes a region server. While the ThreadLocal class, which ThreadLocalPool extends, does maintain the size in ThreadLocalMap, it is unfortunately not visible to us.

        Show
        karthick Karthick Sankarachary added a comment - I tend to agree with Gary in that it doesn't really make sense to cap the thread-local pool. Having the PoolMap throw an exception seems a little too disruptive. I've attached a [patch|^4150-2.txt] that removes the limit check from ThreadLocalPool and fixes the javadoc in HBaseClient . This way, we at least ensure that the client will create as many Connection objects as threads, no more no less. Note that in the PoolMap , we associate a resource Pool with every key, ergo the need for using a map instead of a number for the type for the poolSizes variable. So, in the case of HBaseClient#connections , we'll essentially create a Pool of Connection}}s for every {{ConnectionId , which denotes a region server. While the ThreadLocal class, which ThreadLocalPool extends, does maintain the size in ThreadLocalMap , it is unfortunately not visible to us.
        Hide
        ghelmling Gary Helmling added a comment -

        Thanks for the updated patch Karthick.

        One additional question on how this interacts with HTablePool. In HTablePool, when obtaining a table:

          private HTableInterface findOrCreateTable(String tableName) {
            HTableInterface table = tables.get(tableName);
            if (table == null) {
              table = createHTable(tableName);
            }
            return table;
          }
        

        In the case of ThreadLocalPool, it seems like there's an exposure here between when the table is created initially and when ThreadLocalPool.put() is called to set the thread local variable (on PooledHTable.close()). This is independent of the issue here, so I can open a separate JIRA, just wanted to get your thoughts on if this is a potential leak.

        I'll run the updated patch through tests and commit if all passes (relative to any current trunk breakage).

        Show
        ghelmling Gary Helmling added a comment - Thanks for the updated patch Karthick. One additional question on how this interacts with HTablePool. In HTablePool, when obtaining a table: private HTableInterface findOrCreateTable( String tableName) { HTableInterface table = tables.get(tableName); if (table == null ) { table = createHTable(tableName); } return table; } In the case of ThreadLocalPool , it seems like there's an exposure here between when the table is created initially and when ThreadLocalPool.put() is called to set the thread local variable (on PooledHTable.close() ). This is independent of the issue here, so I can open a separate JIRA, just wanted to get your thoughts on if this is a potential leak. I'll run the updated patch through tests and commit if all passes (relative to any current trunk breakage).
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        relative to any current trunk breakage

        That's a bit hard to predict after HBASE-4158 went in - just look at recent TRUNK builds

        Show
        yuzhihong@gmail.com Ted Yu added a comment - relative to any current trunk breakage That's a bit hard to predict after HBASE-4158 went in - just look at recent TRUNK builds
        Hide
        lhofhansl Lars Hofhansl added a comment -

        +1 to removing the limit for thread local pool.

        Show
        lhofhansl Lars Hofhansl added a comment - +1 to removing the limit for thread local pool.
        Hide
        karthick Karthick Sankarachary added a comment -

        Yes, I think there may be a potential leak in HTablePool, and we could open a separate issue for that, or do it as part of this issue.

        For one thing, we might want to clear the tables variable when the HTablePool is closed (as shown below). For another, we should override ThreadLocalPool#get method so that it removes the resource, otherwise it might end up referencing a HTableInterface that's has been released.

              1 diff --git a/src/main/java/org/apache/hadoop/hbase/client/HTablePool.java b/src/main/java/org/apache/hadoop/hbase/client/HTablePool.java
              2 index 952a3aa..c198f15 100755
              3 --- a/src/main/java/org/apache/hadoop/hbase/client/HTablePool.java
              4 +++ b/src/main/java/org/apache/hadoop/hbase/client/HTablePool.java
             13 @@ -309,6 +310,7 @@ public class HTablePool implements Closeable {
             14      for (String tableName : tables.keySet()) {
             15        closeTablePool(tableName);
             16      }
             17 +    this.tables.clear();
             18    }
        
        Show
        karthick Karthick Sankarachary added a comment - Yes, I think there may be a potential leak in HTablePool , and we could open a separate issue for that, or do it as part of this issue. For one thing, we might want to clear the tables variable when the HTablePool is closed (as shown below). For another, we should override ThreadLocalPool#get method so that it removes the resource, otherwise it might end up referencing a HTableInterface that's has been released. 1 diff --git a/src/main/java/org/apache/hadoop/hbase/client/HTablePool.java b/src/main/java/org/apache/hadoop/hbase/client/HTablePool.java 2 index 952a3aa..c198f15 100755 3 --- a/src/main/java/org/apache/hadoop/hbase/client/HTablePool.java 4 +++ b/src/main/java/org/apache/hadoop/hbase/client/HTablePool.java 13 @@ -309,6 +310,7 @@ public class HTablePool implements Closeable { 14 for ( String tableName : tables.keySet()) { 15 closeTablePool(tableName); 16 } 17 + this .tables.clear(); 18 }
        Hide
        ghelmling Gary Helmling added a comment -

        Cool, thanks for the confirmation Karthick. I created HBASE-4167 to continue the discussion on HTablePool.

        Show
        ghelmling Gary Helmling added a comment - Cool, thanks for the confirmation Karthick. I created HBASE-4167 to continue the discussion on HTablePool.
        Hide
        ghelmling Gary Helmling added a comment -

        Committed this patch (HBASE-4150_final.patch) to trunk.

        This is based on 5140-2.patch, plus a fix for TestPoolMap to reflect the non-enforcement of limit with ThreadLocalPool.

        So this removes use of pool max size when using ThreadLocalPool.

        Show
        ghelmling Gary Helmling added a comment - Committed this patch ( HBASE-4150 _final.patch) to trunk. This is based on 5140-2.patch, plus a fix for TestPoolMap to reflect the non-enforcement of limit with ThreadLocalPool. So this removes use of pool max size when using ThreadLocalPool.
        Hide
        ghelmling Gary Helmling added a comment -

        Doug, over to you for any doc updates you wanted to make clarifying that pool max size configuration does not apply to ThreadLocalPool.

        Show
        ghelmling Gary Helmling added a comment - Doug, over to you for any doc updates you wanted to make clarifying that pool max size configuration does not apply to ThreadLocalPool.
        Hide
        hudson Hudson added a comment -

        Integrated in HBase-TRUNK #2084 (See https://builds.apache.org/job/HBase-TRUNK/2084/)
        HBASE-4150 Don't enforce pool size limit with ThreadLocalPool

        garyh :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/PoolMap.java
        • /hbase/trunk/CHANGES.txt
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/util/TestPoolMap.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java
        Show
        hudson Hudson added a comment - Integrated in HBase-TRUNK #2084 (See https://builds.apache.org/job/HBase-TRUNK/2084/ ) HBASE-4150 Don't enforce pool size limit with ThreadLocalPool garyh : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/PoolMap.java /hbase/trunk/CHANGES.txt /hbase/trunk/src/test/java/org/apache/hadoop/hbase/util/TestPoolMap.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -
            public ThreadLocalPool(int maxSize) {
        

        Since maxSize is no longer used and ThreadLocalPool isn't exposed outside PoolMap, I think the ctor shouldn't take such parameter.

        Show
        yuzhihong@gmail.com Ted Yu added a comment - public ThreadLocalPool( int maxSize) { Since maxSize is no longer used and ThreadLocalPool isn't exposed outside PoolMap, I think the ctor shouldn't take such parameter.
        Hide
        karthick Karthick Sankarachary added a comment -

        Updated the javadoc in HBaseClient and PoolMap as per the comments above.

        Show
        karthick Karthick Sankarachary added a comment - Updated the javadoc in HBaseClient and PoolMap as per the comments above.
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        +1 on the javadoc patch.
        Minor comment, please put the following on one line (can be done at time of commit):

        +   * @param config
        +   *          configuration
        
        Show
        yuzhihong@gmail.com Ted Yu added a comment - +1 on the javadoc patch. Minor comment, please put the following on one line (can be done at time of commit): + * @param config + * configuration
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        Integrate to TRUNK.

        Thanks for the continued effort, Karthick.

        Show
        yuzhihong@gmail.com Ted Yu added a comment - Integrate to TRUNK. Thanks for the continued effort, Karthick.
        Hide
        lhofhansl Lars Hofhansl added a comment -

        Thanks for the doc patch Karthick, it explains trade-offs very nicely.

        Show
        lhofhansl Lars Hofhansl added a comment - Thanks for the doc patch Karthick, it explains trade-offs very nicely.
        Hide
        stack stack added a comment -

        +1 on doc patch.

        Show
        stack stack added a comment - +1 on doc patch.
        Hide
        hudson Hudson added a comment -

        Integrated in HBase-TRUNK #2113 (See https://builds.apache.org/job/HBase-TRUNK/2113/)
        HBASE-4150 update to javadoc (Karthick Sankarachary)

        tedyu :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/PoolMap.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java
        Show
        hudson Hudson added a comment - Integrated in HBase-TRUNK #2113 (See https://builds.apache.org/job/HBase-TRUNK/2113/ ) HBASE-4150 update to javadoc (Karthick Sankarachary) tedyu : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/PoolMap.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java
        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:
            yuzhihong@gmail.com Ted Yu
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development