Liron Aravot has posted comments on this change.

Change subject: core: WIP: prevent simultaneous reconstructs on the same pool
......................................................................


Patch Set 7: (7 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ForceRemoveStorageDomainCommand.java
Line 36:                 if (getStorageDomain().getstorage_domain_type() == 
StorageDomainType.Master) {
Line 37:                     ReconstructMasterParameters tempVar = new 
ReconstructMasterParameters(getStoragePool().getId(),
Line 38:                             getStorageDomain().getId(), false);
Line 39:                     
tempVar.setTransactionScopeOption(TransactionScopeOption.RequiresNew);
Line 40:                     
tempVar.setParentCommand(VdcActionType.ForceRemoveStorageDomain);
that's for determining in the reconstruct command that the executing command is 
force remove domain command.
Line 41:                     
Backend.getInstance().runInternalAction(VdcActionType.ReconstructMasterDomain, 
tempVar);
Line 42:                 }
Line 43:                 // try to force detach first
Line 44:                 DetachStorageDomainVDSCommandParameters tempVar2 = new 
DetachStorageDomainVDSCommandParameters(


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ReconstructMasterDomainCommand.java
Line 56:     }
Line 57: 
Line 58:     @Override
Line 59:     protected boolean canDoAction() {
Line 60:         if (this.getActionType() == VdcActionType.RecoveryStoragePool 
|| 
VdcActionType.ForceRemoveStorageDomain.equals(getParameters().getParentCommand()))
 {
will change
Line 61:              if 
(!IrsBrokerCommand.acquireReconstructLockWithWait(getStoragePool().getId())) {
Line 62:                  return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_NOT_EXIST);
Line 63:              }
Line 64:         } else {


Line 178:             }
Line 179: 
Line 180:             setSucceeded(reconstructOpSucceeded);
Line 181:             // we should run compensation under the lock in order to 
avoid
Line 182:             // race conditions.
it is, but outside of the lock..so it may cause race conditions, so we need to 
call it within the lock.
Line 183:         } finally {
Line 184:             try {
Line 185:                 if (!reconstructOpSucceeded) {
Line 186:                     executeInNewTransaction(new 
TransactionMethod<Void>() {


Line 183:         } finally {
Line 184:             try {
Line 185:                 if (!reconstructOpSucceeded) {
Line 186:                     executeInNewTransaction(new 
TransactionMethod<Void>() {
Line 187:                         public Void runInTransaction() {
will add
Line 188:                             performCleanup();
Line 189:                             return null;
Line 190:                         }
Line 191:                     });


Line 206:             
IrsBrokerCommand.releaseReconstructLock(getStoragePool().getId());
Line 207:         }
Line 208:     }
Line 209: 
Line 210:     protected void performCleanup() {
because RecoveryStoragePool which extends this command will call it as well.
Line 211:         compensate();
Line 212:     }
Line 213: 
Line 214:     protected boolean stopSpm() {


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RecoveryStoragePoolCommand.java
Line 83: 
Line 84:     @Override
Line 85:     protected void performCleanup() {
Line 86:         getStoragePoolIsoMapDAO().remove(new 
StoragePoolIsoMapId(getRecoveryStoragePoolParametersData()
Line 87:                 .getNewMasterDomainId(), 
getRecoveryStoragePoolParametersData().getStoragePoolId()));
because when executing recovery storage pool we don't want to compensate in 
case of failed engine while the domain is active on vdsm (on engine start), but 
only in case that the reconstruct operation failed and the domain doesn't 
appear on vdsm.
Line 88:         super.performCleanup();
Line 89:     }
Line 90: 
Line 91:     @Override


Line 111:                     
.ConnectStorageToDomainByVdsId(getNewMaster(false), getVds().getId());
Line 112:         } finally {
Line 113:             if (!connectSucceeded) {
Line 114:                 getStoragePoolIsoMapDAO().remove(new 
StoragePoolIsoMapId(getRecoveryStoragePoolParametersData()
Line 115:                         .getNewMasterDomainId(), 
getRecoveryStoragePoolParametersData().getStoragePoolId()));
nop, as performCleanups calls the reconstruct performCleanups as well - in that 
case we didn't got to execute ReconstructMasterDomainCommand (super.execute())
Line 116:                 getReturnValue().setFault(new VdcFault(new 
VdcBLLException(VdcBllErrors.StorageServerConnectionError,
Line 117:                         "Failed to connect storage"),
Line 118:                         VdcBllErrors.StorageServerConnectionError));
Line 119:             }


--
To view, visit http://gerrit.ovirt.org/8911
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I68783e92c6aca5dc047611f8a5dcd68faffb5893
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to