cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mc...@apache.org
Subject git commit: updated refs/heads/master to de44a77
Date Thu, 27 Jun 2013 00:16:56 GMT
Updated Branches:
  refs/heads/master a654e52cb -> de44a7787


Gracefully handle racing condition in updating state of dataobject and
data store mapping table.

Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/de44a778
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/de44a778
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/de44a778

Branch: refs/heads/master
Commit: de44a77878c972f061ab42ec6e41e1cfb3f67adc
Parents: a654e52
Author: Min Chen <min.chen@citrix.com>
Authored: Wed Jun 26 17:16:25 2013 -0700
Committer: Min Chen <min.chen@citrix.com>
Committed: Wed Jun 26 17:16:25 2013 -0700

----------------------------------------------------------------------
 .../storage/image/store/TemplateObject.java     | 51 ++--------------
 .../datastore/DataObjectManagerImpl.java        | 62 ++++++++++++++++++--
 .../datastore/ObjectInDataStoreManager.java     |  3 +-
 .../datastore/ObjectInDataStoreManagerImpl.java | 34 ++++++-----
 .../storage/volume/VolumeServiceImpl.java       |  8 +--
 5 files changed, 88 insertions(+), 70 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/de44a778/engine/storage/image/src/org/apache/cloudstack/storage/image/store/TemplateObject.java
----------------------------------------------------------------------
diff --git a/engine/storage/image/src/org/apache/cloudstack/storage/image/store/TemplateObject.java
b/engine/storage/image/src/org/apache/cloudstack/storage/image/store/TemplateObject.java
index b093cbd..7a8fc60 100644
--- a/engine/storage/image/src/org/apache/cloudstack/storage/image/store/TemplateObject.java
+++ b/engine/storage/image/src/org/apache/cloudstack/storage/image/store/TemplateObject.java
@@ -27,7 +27,6 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataObjectInStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
 import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event;
-import org.apache.cloudstack.engine.subsystem.api.storage.TemplateEvent;
 import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
 import org.apache.cloudstack.storage.command.CopyCmdAnswer;
 import org.apache.cloudstack.storage.datastore.ObjectInDataStoreManager;
@@ -39,6 +38,7 @@ import org.apache.log4j.Logger;
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.to.DataObjectType;
 import com.cloud.agent.api.to.DataTO;
+import com.cloud.exception.ConcurrentOperationException;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.storage.DataStoreRole;
 import com.cloud.storage.Storage.ImageFormat;
@@ -152,41 +152,14 @@ public class TemplateObject implements TemplateInfo {
         return this.imageVO.getFormat();
     }
 
-    // public boolean stateTransit(TemplateEvent e) throws NoTransitionException
-    // {
-    // this.imageVO = imageDao.findById(this.imageVO.getId());
-    // boolean result = imageMgr.getStateMachine().transitTo(this.imageVO, e,
-    // null, imageDao);
-    // this.imageVO = imageDao.findById(this.imageVO.getId());
-    // return result;
-    // }
-
     @Override
     public void processEvent(Event event) {
         try {
-            if (this.getDataStore().getRole() == DataStoreRole.Image
-                    || this.getDataStore().getRole() == DataStoreRole.ImageCache) {
-                TemplateEvent templEvent = null;
-                if (event == ObjectInDataStoreStateMachine.Event.CreateOnlyRequested) {
-                    templEvent = TemplateEvent.CreateRequested;
-                } else if (event == ObjectInDataStoreStateMachine.Event.DestroyRequested)
{
-                    templEvent = TemplateEvent.DestroyRequested;
-                } else if (event == ObjectInDataStoreStateMachine.Event.OperationSuccessed)
{
-                    templEvent = TemplateEvent.OperationSucceeded;
-                } else if (event == ObjectInDataStoreStateMachine.Event.OperationFailed)
{
-                    templEvent = TemplateEvent.OperationFailed;
-                }
-
-                // if (templEvent != null && this.getDataStore().getRole() ==
-                // DataStoreRole.Image) {
-                // this.stateTransit(templEvent);
-                // }
-            }
-
             objectInStoreMgr.update(this, event);
         } catch (NoTransitionException e) {
-            s_logger.debug("failed to update state", e);
-            throw new CloudRuntimeException("Failed to update state" + e.toString());
+            throw new CloudRuntimeException("Failed to update state", e);
+        } catch (ConcurrentOperationException e) {
+            throw new CloudRuntimeException("Failed to update state", e);
         } finally {
             // in case of OperationFailed, expunge the entry
             if (event == ObjectInDataStoreStateMachine.Event.OperationFailed) {
@@ -229,22 +202,6 @@ public class TemplateObject implements TemplateInfo {
                         this.imageDao.update(templateVO.getId(), templateVO);
                     }
                 }
-
-                TemplateEvent templEvent = null;
-                if (event == ObjectInDataStoreStateMachine.Event.CreateOnlyRequested) {
-                    templEvent = TemplateEvent.CreateRequested;
-                } else if (event == ObjectInDataStoreStateMachine.Event.DestroyRequested)
{
-                    templEvent = TemplateEvent.DestroyRequested;
-                } else if (event == ObjectInDataStoreStateMachine.Event.OperationSuccessed)
{
-                    templEvent = TemplateEvent.OperationSucceeded;
-                } else if (event == ObjectInDataStoreStateMachine.Event.OperationFailed)
{
-                    templEvent = TemplateEvent.OperationFailed;
-                }
-
-                // if (templEvent != null && this.getDataStore().getRole() ==
-                // DataStoreRole.Image) {
-                // this.stateTransit(templEvent);
-                // }
             }
             objectInStoreMgr.update(this, event);
         } catch (NoTransitionException e) {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/de44a778/engine/storage/src/org/apache/cloudstack/storage/datastore/DataObjectManagerImpl.java
----------------------------------------------------------------------
diff --git a/engine/storage/src/org/apache/cloudstack/storage/datastore/DataObjectManagerImpl.java
b/engine/storage/src/org/apache/cloudstack/storage/datastore/DataObjectManagerImpl.java
index 6e97e6f..fa9f993 100644
--- a/engine/storage/src/org/apache/cloudstack/storage/datastore/DataObjectManagerImpl.java
+++ b/engine/storage/src/org/apache/cloudstack/storage/datastore/DataObjectManagerImpl.java
@@ -35,6 +35,7 @@ import org.apache.cloudstack.storage.command.CommandResult;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 
+import com.cloud.exception.ConcurrentOperationException;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.fsm.NoTransitionException;
 
@@ -136,7 +137,18 @@ public class DataObjectManagerImpl implements DataObjectManager {
         } catch (NoTransitionException e) {
             try {
                 objectInDataStoreMgr.update(objInStore, ObjectInDataStoreStateMachine.Event.OperationFailed);
-            } catch (NoTransitionException e1) {
+            } catch (Exception e1) {
+                s_logger.debug("state transation failed", e1);
+            }
+            CreateCmdResult result = new CreateCmdResult(null, null);
+            result.setSuccess(false);
+            result.setResult(e.toString());
+            callback.complete(result);
+            return;
+        } catch (ConcurrentOperationException e) {
+            try {
+                objectInDataStoreMgr.update(objInStore, ObjectInDataStoreStateMachine.Event.OperationFailed);
+            } catch (Exception e1) {
                 s_logger.debug("state transation failed", e1);
             }
             CreateCmdResult result = new CreateCmdResult(null, null);
@@ -170,7 +182,17 @@ public class DataObjectManagerImpl implements DataObjectManager {
         } catch (NoTransitionException e) {
             try {
                 objectInDataStoreMgr.update(objInStrore, ObjectInDataStoreStateMachine.Event.OperationFailed);
-            } catch (NoTransitionException e1) {
+            } catch (Exception e1) {
+                s_logger.debug("failed to change state", e1);
+            }
+
+            upResult.setResult(e.toString());
+            context.getParentCallback().complete(upResult);
+            return null;
+        } catch (ConcurrentOperationException e) {
+            try {
+                objectInDataStoreMgr.update(objInStrore, ObjectInDataStoreStateMachine.Event.OperationFailed);
+            } catch (Exception e1) {
                 s_logger.debug("failed to change state", e1);
             }
 
@@ -202,7 +224,17 @@ public class DataObjectManagerImpl implements DataObjectManager {
             s_logger.debug("failed to change state", e);
             try {
                 objectInDataStoreMgr.update(destData, ObjectInDataStoreStateMachine.Event.OperationFailed);
-            } catch (NoTransitionException e1) {
+            } catch (Exception e1) {
+
+            }
+            CreateCmdResult res = new CreateCmdResult(null, null);
+            res.setResult("Failed to change state: " + e.toString());
+            callback.complete(res);
+        } catch (ConcurrentOperationException e) {
+            s_logger.debug("failed to change state", e);
+            try {
+                objectInDataStoreMgr.update(destData, ObjectInDataStoreStateMachine.Event.OperationFailed);
+            } catch (Exception e1) {
 
             }
             CreateCmdResult res = new CreateCmdResult(null, null);
@@ -227,6 +259,8 @@ public class DataObjectManagerImpl implements DataObjectManager {
                 objectInDataStoreMgr.update(destObj, Event.OperationFailed);
             } catch (NoTransitionException e) {
                 s_logger.debug("Failed to update copying state", e);
+            } catch (ConcurrentOperationException e) {
+                s_logger.debug("Failed to update copying state", e);
             }
             CreateCmdResult res = new CreateCmdResult(null, null);
             res.setResult(result.getResult());
@@ -239,7 +273,16 @@ public class DataObjectManagerImpl implements DataObjectManager {
             s_logger.debug("Failed to update copying state: ", e);
             try {
                 objectInDataStoreMgr.update(destObj, ObjectInDataStoreStateMachine.Event.OperationFailed);
-            } catch (NoTransitionException e1) {
+            } catch (Exception e1) {
+            }
+            CreateCmdResult res = new CreateCmdResult(null, null);
+            res.setResult("Failed to update copying state: " + e.toString());
+            context.getParentCallback().complete(res);
+        } catch (ConcurrentOperationException e) {
+            s_logger.debug("Failed to update copying state: ", e);
+            try {
+                objectInDataStoreMgr.update(destObj, ObjectInDataStoreStateMachine.Event.OperationFailed);
+            } catch (Exception e1) {
             }
             CreateCmdResult res = new CreateCmdResult(null, null);
             res.setResult("Failed to update copying state: " + e.toString());
@@ -268,6 +311,10 @@ public class DataObjectManagerImpl implements DataObjectManager {
             s_logger.debug("destroy failed", e);
             CreateCmdResult res = new CreateCmdResult(null, null);
             callback.complete(res);
+        } catch (ConcurrentOperationException e) {
+            s_logger.debug("destroy failed", e);
+            CreateCmdResult res = new CreateCmdResult(null, null);
+            callback.complete(res);
         }
 
         DeleteContext<CommandResult> context = new DeleteContext<CommandResult>(callback,
data);
@@ -288,6 +335,8 @@ public class DataObjectManagerImpl implements DataObjectManager {
                 objectInDataStoreMgr.update(destObj, Event.OperationFailed);
             } catch (NoTransitionException e) {
                 s_logger.debug("delete failed", e);
+            } catch (ConcurrentOperationException e) {
+                s_logger.debug("delete failed", e);
             }
 
         } else {
@@ -295,6 +344,8 @@ public class DataObjectManagerImpl implements DataObjectManager {
                 objectInDataStoreMgr.update(destObj, Event.OperationSuccessed);
             } catch (NoTransitionException e) {
                 s_logger.debug("delete failed", e);
+            } catch (ConcurrentOperationException e) {
+                s_logger.debug("delete failed", e);
             }
         }
 
@@ -318,6 +369,9 @@ public class DataObjectManagerImpl implements DataObjectManager {
         } catch (NoTransitionException e) {
             s_logger.debug("Failed to update state", e);
             throw new CloudRuntimeException("Failed to update state", e);
+        } catch (ConcurrentOperationException e) {
+            s_logger.debug("Failed to update state", e);
+            throw new CloudRuntimeException("Failed to update state", e);
         }
 
         return objInStore;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/de44a778/engine/storage/src/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManager.java
----------------------------------------------------------------------
diff --git a/engine/storage/src/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManager.java
b/engine/storage/src/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManager.java
index b72d07b..fbd315e 100644
--- a/engine/storage/src/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManager.java
+++ b/engine/storage/src/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManager.java
@@ -22,6 +22,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event;
 
 import com.cloud.agent.api.to.DataObjectType;
+import com.cloud.exception.ConcurrentOperationException;
 import com.cloud.storage.DataStoreRole;
 import com.cloud.utils.fsm.NoTransitionException;
 
@@ -32,7 +33,7 @@ public interface ObjectInDataStoreManager {
 
     public DataObject get(DataObject dataObj, DataStore store);
 
-    public boolean update(DataObject vo, Event event) throws NoTransitionException;
+    public boolean update(DataObject vo, Event event) throws NoTransitionException, ConcurrentOperationException;
 
     DataObjectInStore findObject(long objId, DataObjectType type, long dataStoreId, DataStoreRole
role);
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/de44a778/engine/storage/src/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManagerImpl.java
----------------------------------------------------------------------
diff --git a/engine/storage/src/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManagerImpl.java
b/engine/storage/src/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManagerImpl.java
index 4b3d4a6..427609e 100644
--- a/engine/storage/src/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManagerImpl.java
+++ b/engine/storage/src/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManagerImpl.java
@@ -42,6 +42,7 @@ import org.springframework.stereotype.Component;
 
 import com.cloud.agent.api.to.DataObjectType;
 import com.cloud.agent.api.to.S3TO;
+import com.cloud.exception.ConcurrentOperationException;
 import com.cloud.storage.DataStoreRole;
 import com.cloud.storage.VMTemplateStoragePoolVO;
 import com.cloud.storage.dao.SnapshotDao;
@@ -127,14 +128,14 @@ public class ObjectInDataStoreManagerImpl implements ObjectInDataStoreManager
{
                 if (dataStore.getTO() instanceof S3TO) {
                     TemplateInfo tmpl = (TemplateInfo) obj;
                     installPath += "/" + tmpl.getUniqueName(); // for S3, we
-                                                               // append
-                                                               // template name
-                                                               // in the path
-                                                               // for template
-                                                               // sync since we
-                                                               // don't have
-                                                               // template.properties
-                                                               // there
+                    // append
+                    // template name
+                    // in the path
+                    // for template
+                    // sync since we
+                    // don't have
+                    // template.properties
+                    // there
                 }
                 ts.setInstallPath(installPath);
                 ts.setState(ObjectInDataStoreStateMachine.State.Allocated);
@@ -221,35 +222,40 @@ public class ObjectInDataStoreManagerImpl implements ObjectInDataStoreManager
{
     }
 
     @Override
-    public boolean update(DataObject data, Event event) throws NoTransitionException {
+    public boolean update(DataObject data, Event event) throws NoTransitionException, ConcurrentOperationException
{
         DataObjectInStore obj = this.findObject(data, data.getDataStore());
         if (obj == null) {
             throw new CloudRuntimeException("can't find mapping in ObjectInDataStore table
for: " + data);
         }
 
+        boolean result = true;
         if (data.getDataStore().getRole() == DataStoreRole.Image
                 || data.getDataStore().getRole() == DataStoreRole.ImageCache) {
             switch (data.getType()) {
             case TEMPLATE:
-                this.stateMachines.transitTo(obj, event, null, templateDataStoreDao);
+                result = this.stateMachines.transitTo(obj, event, null, templateDataStoreDao);
                 break;
             case SNAPSHOT:
-                this.stateMachines.transitTo(obj, event, null, snapshotDataStoreDao);
+                result = this.stateMachines.transitTo(obj, event, null, snapshotDataStoreDao);
                 break;
             case VOLUME:
-                this.stateMachines.transitTo(obj, event, null, volumeDataStoreDao);
+                result = this.stateMachines.transitTo(obj, event, null, volumeDataStoreDao);
                 break;
             }
         } else if (data.getType() == DataObjectType.TEMPLATE && data.getDataStore().getRole()
== DataStoreRole.Primary) {
 
-            this.stateMachines.transitTo(obj, event, null, templatePoolDao);
+            result = this.stateMachines.transitTo(obj, event, null, templatePoolDao);
 
         } else if (data.getType() == DataObjectType.SNAPSHOT && data.getDataStore().getRole()
== DataStoreRole.Primary) {
-            this.stateMachines.transitTo(obj, event, null, snapshotDataStoreDao);
+            result = this.stateMachines.transitTo(obj, event, null, snapshotDataStoreDao);
         } else {
             throw new CloudRuntimeException("Invalid data or store type: " + data.getType()
+ " "
                     + data.getDataStore().getRole());
         }
+
+        if (!result){
+            throw new ConcurrentOperationException("Multiple threads are trying to update
data object state, racing condition");
+        }
         return true;
     }
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/de44a778/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
----------------------------------------------------------------------
diff --git a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
index 1d36f93..e9316ff 100644
--- a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
+++ b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
@@ -332,7 +332,7 @@ public class VolumeServiceImpl implements VolumeService {
         try {
             templateOnPrimaryStoreObj.processEvent(Event.CreateOnlyRequested);
         } catch (Exception e) {
-            s_logger.info("Got exception in case of multi-thread");
+            s_logger.info("Multiple threads are trying to copy template to primary storage,
current thread should just wait");
             try {
                 templateOnPrimaryStoreObj = waitForTemplateDownloaded(dataStore, template);
             } catch (Exception e1) {
@@ -1050,7 +1050,7 @@ public class VolumeServiceImpl implements VolumeService {
                         try {
                             _resourceLimitMgr.checkResourceLimit(_accountMgr.getAccount(volume.getAccountId()),
                                     com.cloud.configuration.Resource.ResourceType.secondary_storage,
volInfo.getSize()
-                                            - volInfo.getPhysicalSize());
+                                    - volInfo.getPhysicalSize());
                         } catch (ResourceAllocationException e) {
                             s_logger.warn(e.getMessage());
                             _alertMgr.sendAlert(AlertManager.ALERT_TYPE_RESOURCE_LIMIT_EXCEEDED,
@@ -1075,8 +1075,8 @@ public class VolumeServiceImpl implements VolumeService {
         if (toBeDownloaded.size() > 0) {
             for (VolumeDataStoreVO volumeHost : toBeDownloaded) {
                 if (volumeHost.getDownloadUrl() == null) { // If url is null we
-                                                           // can't initiate the
-                                                           // download
+                    // can't initiate the
+                    // download
                     continue;
                 }
                 s_logger.debug("Volume " + volumeHost.getVolumeId() + " needs to be downloaded
to " + store.getName());


Mime
View raw message