cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jburwell <...@git.apache.org>
Subject [GitHub] cloudstack pull request #1765: Cloudstack 9586: When using local storage wit...
Date Thu, 17 Nov 2016 06:04:12 GMT
Github user jburwell commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1765#discussion_r88391404
  
    --- Diff: plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java
---
    @@ -100,36 +100,25 @@ protected SR createFileSR(final Connection conn, final String path)
{
             PBD pbd = null;
     
             try {
    -            final String srname = hypervisorResource.getHost().getUuid() + path.trim();
    -
    -            final Set<SR> srs = SR.getByNameLabel(conn, srname);
    -
    -            if (srs != null && !srs.isEmpty()) {
    -                return srs.iterator().next();
    +            final String srname = path.trim();
    +            synchronized (srname.intern()) {
    --- End diff --
    
    Should we consider using a `ReentrantLock` instead of `synchronized`?  `ReentrantLock.tryLock(long,
TimeUnit)` allows the specification of a timeout to acquire the lock.  Since a timeout cannot
be specified to `synchronized`, multiple threads may deadlock attempting to acquire the lock
in the case of a stall.  The downside of this approach is that we would need to track locks
in a `ConcurrentMap` -- ideally with WeakReferences to allow them to be garbage collected
when no threads are operating on them.  


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message