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

    https://github.com/apache/cloudstack/pull/572#discussion_r34861726
  
    --- Diff: 
engine/storage/cache/src/org/apache/cloudstack/storage/cache/manager/StorageCacheManagerImpl.java
 ---
    @@ -216,15 +223,82 @@ public boolean stop() {
     
         @Override
         public DataObject createCacheObject(DataObject data, DataStore store) {
    -        DataObjectInStore obj = objectInStoreMgr.findObject(data, store);
    -        if (obj != null && obj.getState() == 
ObjectInDataStoreStateMachine.State.Ready) {
    -            s_logger.debug("there is already one in the cache store");
    -            DataObject dataObj = objectInStoreMgr.get(data, store);
    -            dataObj.incRefCount();
    -            return dataObj;
    +        DataObject objOnCacheStore;
    +        final Object lock;
    +        final DataObjectType type = data.getType();
    +        final String typeName;
    +        final DataStoreRole role = store.getRole();
    +        final long storeId = store.getId();
    +        final long dataId = data.getId();
    +
    +        /*
    +         * Make sure any thread knows own lock type.
    +         */
    +        if (type == DataObjectType.TEMPLATE) {
    +            lock = templateLock;
    +            typeName = "template";
    +        } else if (type == DataObjectType.VOLUME) {
    +            lock = volumeLock;
    +            typeName = "volume";
    +        } else if (type == DataObjectType.SNAPSHOT) {
    +            lock = snapshotLock;
    +            typeName = "snapshot";
    +        } else {
    +            String msg = "unsupported DataObject comes, then can't acquire 
correct lock object";
    +            throw new CloudRuntimeException(msg);
             }
    +        s_logger.debug("check " + typeName + " cache entry(id: " + dataId 
+ ") on store(id: " + storeId + ")");
    +
    +        synchronized (lock) {
    +            DataObjectInStore obj = objectInStoreMgr.findObject(data, 
store);
    +            if (obj != null) {
    +                State st = obj.getState();
    +
    +                long miliSeconds = 10000;
    +                long timeoutSeconds = 3600;
    +                long timeoutMiliSeconds = timeoutSeconds * 1000;
    +                Date now = new Date();
    +                long expiredEpoch = now.getTime() + timeoutMiliSeconds;
    +                Date expiredDate = new Date(expiredEpoch);
    +
    +                /*
    +                 * Waiting for completion of cache copy.
    +                 */
    +                while (st == ObjectInDataStoreStateMachine.State.Allocated 
||
    +                    st == ObjectInDataStoreStateMachine.State.Creating ||
    +                    st == ObjectInDataStoreStateMachine.State.Copying) {
    +
    +                    /*
    +                     * Threads must release lock within waiting for cache 
copy and
    +                     * must be waken up at completion.
    +                     */
    +                    s_logger.debug("waiting cache copy completion type: " 
+ typeName + ", id: " + obj.getObjectId() + ", lock: " + lock.hashCode());
    +                    try {
    +                        lock.wait(miliSeconds);
    +                    } catch (InterruptedException e) {}
    +                    s_logger.debug("waken up");
    +
    +                    now = new Date();
    +                    if (now.after(expiredDate)) {
    +                        String msg = "Waiting time exceeds timeout limit(" 
+ timeoutSeconds + " s)";
    +                        throw new CloudRuntimeException(msg);
    +                    }
     
    -        DataObject objOnCacheStore = store.create(data);
    +                    obj = objectInStoreMgr.findObject(data, store);
    +                    st = obj.getState();
    +                }
    +
    +                if (st == ObjectInDataStoreStateMachine.State.Ready) {
    +                    s_logger.debug("there is already one in the cache 
store");
    +                    DataObject dataObj = objectInStoreMgr.get(data, store);
    +                    dataObj.incRefCount();
    +                    return dataObj;
    --- End diff --
    
    we need to `lock.notifyAll();` before return.


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