Github user rhtyd commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1729#discussion_r85903435 --- Diff: vmware-base/src/com/cloud/hypervisor/vmware/util/VmwareContextPool.java --- @@ -45,76 +43,74 @@ public VmwareContextPool() { this(DEFAULT_IDLE_QUEUE_LENGTH, DEFAULT_CHECK_INTERVAL); } - public VmwareContextPool(int maxIdleQueueLength) { - this(maxIdleQueueLength, DEFAULT_CHECK_INTERVAL); - } - public VmwareContextPool(int maxIdleQueueLength, long idleCheckIntervalMs) { - _pool = new HashMap<String, List<VmwareContext>>(); + _pool = new HashMap<String, Queue<VmwareContext>>(); _maxIdleQueueLength = maxIdleQueueLength; _idleCheckIntervalMs = idleCheckIntervalMs; _timer.scheduleAtFixedRate(getTimerTask(), _idleCheckIntervalMs, _idleCheckIntervalMs); } - public void registerOutstandingContext(VmwareContext context) { - assert (context != null); - synchronized (this) { - _outstandingRegistry.add(context); - } - } - - public void unregisterOutstandingContext(VmwareContext context) { - assert (context != null); - synchronized (this) { - _outstandingRegistry.remove(context); + public VmwareContext getContext(final String vCenterAddress, final String vCenterUserName) { + final String poolKey = composePoolKey(vCenterAddress, vCenterUserName); + if (Strings.isNullOrEmpty(poolKey)) { + return null; } - } - - public VmwareContext getContext(String vCenterAddress, String vCenterUserName) { - String poolKey = composePoolKey(vCenterAddress, vCenterUserName); synchronized (this) { - List<VmwareContext> l = _pool.get(poolKey); - if (l == null) - return null; - - if (l.size() > 0) { - VmwareContext context = l.remove(0); - context.setPoolInfo(this, poolKey); - - if (s_logger.isTraceEnabled()) - s_logger.trace("Return a VmwareContext from the idle pool: " + poolKey + ". current pool size: " + l.size() + ", outstanding count: " + - VmwareContext.getOutstandingContextCount()); + final Queue<VmwareContext> ctxList = _pool.get(poolKey); + if (ctxList != null && ctxList.size() > 0) { + final VmwareContext context = ctxList.remove(); + if (context != null) { + context.setPoolInfo(this, poolKey); + } + if (s_logger.isTraceEnabled()) { + s_logger.trace("Return a VmwareContext from the idle pool: " + poolKey + ". current pool size: " + ctxList.size() + ", outstanding count: " + + VmwareContext.getOutstandingContextCount()); + } return context; } - - // TODO, we need to control the maximum number of outstanding VmwareContext object in the future return null; } } - public void returnContext(VmwareContext context) { + public void registerContext(final VmwareContext context) { assert (context.getPool() == this); assert (context.getPoolKey() != null); synchronized (this) { - List<VmwareContext> l = _pool.get(context.getPoolKey()); - if (l == null) { - l = new ArrayList<VmwareContext>(); - _pool.put(context.getPoolKey(), l); + Queue<VmwareContext> ctxList = _pool.get(context.getPoolKey()); + if (ctxList == null) { + ctxList = EvictingQueue.create(_maxIdleQueueLength); + _pool.put(context.getPoolKey(), ctxList); } - if (l.size() < _maxIdleQueueLength) { - context.clearStockObjects(); - l.add(context); + if (ctxList.size() >= _maxIdleQueueLength) { + final VmwareContext oldestContext = ctxList.remove(); + if (oldestContext != null) { + try { + oldestContext.close(); + } catch (Throwable t) { --- End diff -- @abhinandanprateek ^^ here we close oldest context, before adding a new one. The queue provided the FIFO semantics to purge old contexts and keep new ones around, which is why I switched from previously used arraylist.
--- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---