You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by mc...@apache.org on 2013/06/27 02:16:56 UTC
git commit: updated refs/heads/master to de44a77
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 <mi...@citrix.com>
Authored: Wed Jun 26 17:16:25 2013 -0700
Committer: Min Chen <mi...@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());