Return-Path: X-Original-To: apmail-cloudstack-commits-archive@www.apache.org Delivered-To: apmail-cloudstack-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id B1315101BB for ; Thu, 27 Jun 2013 00:16:56 +0000 (UTC) Received: (qmail 13811 invoked by uid 500); 27 Jun 2013 00:16:56 -0000 Delivered-To: apmail-cloudstack-commits-archive@cloudstack.apache.org Received: (qmail 13792 invoked by uid 500); 27 Jun 2013 00:16:56 -0000 Mailing-List: contact commits-help@cloudstack.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cloudstack.apache.org Delivered-To: mailing list commits@cloudstack.apache.org Received: (qmail 13784 invoked by uid 99); 27 Jun 2013 00:16:56 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 27 Jun 2013 00:16:56 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 59C4B1DF03; Thu, 27 Jun 2013 00:16:56 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: mchen@apache.org To: commits@cloudstack.apache.org Message-Id: <2e7250ad883f4451bdcc7e07e7b39df9@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: git commit: updated refs/heads/master to de44a77 Date: Thu, 27 Jun 2013 00:16:56 +0000 (UTC) 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 Authored: Wed Jun 26 17:16:25 2013 -0700 Committer: Min Chen 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 context = new DeleteContext(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());