Github user rhtyd commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1729#discussion_r85902494
  
    --- 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) {
    +                        s_logger.error("Unexpected exception caught while 
trying to purge oldest VmwareContext", t);
    +                    }
    --- End diff --
    
    In the `registerContext` method we check if the queue is full; in case it 
is full, we remove the oldest element and close it properly. I'll ping you on 
the lines where it happens.


---
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.
---

Reply via email to