busbey commented on a change in pull request #2709: URL: https://github.com/apache/hbase/pull/2709#discussion_r530585606
########## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/util/PoolMap.java ########## @@ -270,58 +195,16 @@ public static PoolType fuzzyMatch(String name) { protected Pool<V> createPool() { switch (poolType) { - case Reusable: - return new ReusablePool<>(poolMaxSize); - case RoundRobin: - return new RoundRobinPool<>(poolMaxSize); - case ThreadLocal: - return new ThreadLocalPool<>(); - } - return null; - } - - /** - * The <code>ReusablePool</code> represents a {@link PoolMap.Pool} that builds - * on the {@link java.util.LinkedList} class. It essentially allows resources to be - * checked out, at which point it is removed from this pool. When the resource - * is no longer required, it should be returned to the pool in order to be - * reused. - * - * <p> - * If {@link #maxSize} is set to {@link Integer#MAX_VALUE}, then the size of - * the pool is unbounded. Otherwise, it caps the number of consumers that can - * check out a resource from this pool to the (non-zero positive) value - * specified in {@link #maxSize}. - * </p> - * - * @param <R> - * the type of the resource - */ - @SuppressWarnings("serial") - public static class ReusablePool<R> extends ConcurrentLinkedQueue<R> implements Pool<R> { - private int maxSize; - - public ReusablePool(int maxSize) { - this.maxSize = maxSize; - - } - - @Override - public R get() { - return poll(); - } - - @Override - public R put(R resource) { - if (super.size() < maxSize) { - add(resource); - } - return null; - } - - @Override - public Collection<R> values() { - return this; + case Reusable: + /* Reusable pool is the same as a 1 element round robin pool. It returns the same element + * until it is removed. */ + return new RoundRobinPool<>(1); Review comment: one important property of the Reusable pool is that things came out of the pool while being used, which essentially ensured a given resource was only ever in use by a single thread at a time (presuming whatever checked the resource out did not make it visible to multiple threads). So under contention we'd have a number of copies of the resource on par with the number of accessors (similar to the ThreadLocal pool) but when the contention passed we wouldn't keep them around. I'm trying to see if we made use of this property or not. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org