cloudstack-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CLOUDSTACK-9564) Fix memory leak in VmwareContextPool
Date Tue, 01 Nov 2016 10:28:59 GMT

    [ https://issues.apache.org/jira/browse/CLOUDSTACK-9564?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15625056#comment-15625056
] 

ASF GitHub Bot commented on CLOUDSTACK-9564:
--------------------------------------------

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

    https://github.com/apache/cloudstack/pull/1729#discussion_r85904768
  
    --- 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);
    --- End diff --
    
    Since all access is in synchronized blocks this is not required. But a synchronised evictingqueue
could have let you avoid some of synchronised blocks.
    
    Again, this is not required as it is.


> Fix memory leak in VmwareContextPool
> ------------------------------------
>
>                 Key: CLOUDSTACK-9564
>                 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9564
>             Project: CloudStack
>          Issue Type: Bug
>      Security Level: Public(Anyone can view this level - this is the default.) 
>            Reporter: Rohit Yadav
>            Assignee: Rohit Yadav
>
> In a recent management server crash, it was found that the largest contributor to memory
leak was in VmwareContextPool where a registry is held (arraylist) that grows indefinitely.
The list itself is not used anywhere or consumed. There exists a hashmap (pool) that returns
a list of contexts for existing poolkey (address/username) that is used instead. The fix would
be to get rid of the registry and limit the hashmap context list length for any poolkey.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message