Michael Kublin has uploaded a new change for review.
Change subject: engine: Making increase of storage pool master domain version
atomic operation
......................................................................
engine: Making increase of storage pool master domain version atomic operation
The following patch should make an increase of master storage domain version
atomic
operation. These is done in order to prevent races and need for cache reload
and also to
make code simple
Change-Id: Iaa2657ddfdbf2400f24341b07ab60743d6430481
Signed-off-by: Michael Kublin <mkub...@redhat.com>
---
M backend/manager/dbscripts/storages_sp.sql
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStoragePoolWithStoragesCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java
M
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/SimpleJdbcCallsHandler.java
M
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/AsyncTaskDAODbFacadeImpl.java
M
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAO.java
M
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAODbFacadeImpl.java
D
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAOHibernateImpl.java
D
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOHibernateImplTest.java
M
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOTest.java
11 files changed, 48 insertions(+), 134 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/00/11500/1
diff --git a/backend/manager/dbscripts/storages_sp.sql
b/backend/manager/dbscripts/storages_sp.sql
index 1fe56ee..a2f1cb1 100644
--- a/backend/manager/dbscripts/storages_sp.sql
+++ b/backend/manager/dbscripts/storages_sp.sql
@@ -83,6 +83,22 @@
END; $procedure$
LANGUAGE plpgsql;
+Create or replace FUNCTION Increase_storage_pool_master_version(
+ v_id UUID)
+RETURNS INTEGER
+ AS $procedure$
+DECLARE v_master_domain_version INTEGER;
+BEGIN
+ UPDATE storage_pool
+ SET
+ master_domain_version = master_domain_version + 1
+ WHERE id = v_id
+ RETURNING master_domain_version into v_master_domain_version;
+
+ RETURN v_master_domain_version;
+END; $procedure$
+LANGUAGE plpgsql;
+
Create or replace FUNCTION Deletestorage_pool(v_id UUID)
RETURNS VOID
AS $procedure$
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStoragePoolWithStoragesCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStoragePoolWithStoragesCommand.java
index ef6d013..438fcaf 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStoragePoolWithStoragesCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStoragePoolWithStoragesCommand.java
@@ -169,15 +169,12 @@
if (!staticDomainChanged) {
getCompensationContext().snapshotEntity(staticDomain);
}
- // increase master domain version - no need to
snapshot, as we would like
- // the master domain version to grow monotonously
even if the wrapping transaction fails
-
getStoragePool().setmaster_domain_version(getStoragePool().getmaster_domain_version()
+ 1);
storageDomain.setstorage_domain_type(StorageDomainType.Master);
staticDomainChanged = true;
masterStorageDomain = storageDomain;
// The update of storage pool should be without
compensation,
// this is why we run it in a different SUPRESS
transaction.
- updateStoragePoolInDiffTransaction();
+
updateStoragePoolMasterDomainVersionInDiffTransaction();
}
if (staticDomainChanged) {
getStorageDomainStaticDAO().update(staticDomain);
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
index 83d0967..136cca7 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
@@ -266,8 +266,6 @@
@Override
public Object runInTransaction() {
StoragePoolIsoMap newMasterMap =
newMaster.getStoragePoolIsoMapData();
- // We do not need to compensate storage pool, it will
be committed if run during reconstruct
-
getStoragePool().setmaster_domain_version(getStoragePool().getmaster_domain_version()
+ 1);
newMaster.getStorageStaticData().setLastTimeUsedAsMaster(System.currentTimeMillis());
getCompensationContext().snapshotEntity(newMaster.getStorageStaticData());
newMaster.setstorage_domain_type(StorageDomainType.Master);
@@ -292,7 +290,7 @@
} else {
_isLastMaster = true;
}
- updateStoragePoolInDiffTransaction();
+ updateStoragePoolMasterDomainVersionInDiffTransaction();
}
}
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java
index 9153355..033f138 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java
@@ -65,11 +65,12 @@
return
getVdsDAO().getAllForStoragePoolAndStatus(getStoragePool().getId(),
VDSStatus.Up);
}
- protected void updateStoragePoolInDiffTransaction() {
- executeInScope(TransactionScopeOption.Suppress, new
TransactionMethod<Object>() {
+ protected void updateStoragePoolMasterDomainVersionInDiffTransaction() {
+ executeInScope(TransactionScopeOption.Suppress, new
TransactionMethod<Void>() {
@Override
- public Object runInTransaction() {
- getStoragePoolDAO().update(getStoragePool());
+ public Void runInTransaction() {
+ int master_domain_version =
getStoragePoolDAO().increaseStoragePoolMasterVersion(getStoragePool().getId());
+
getStoragePool().setmaster_domain_version(master_domain_version);
return null;
}
});
diff --git
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/SimpleJdbcCallsHandler.java
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/SimpleJdbcCallsHandler.java
index 0021328..e9540fc 100644
---
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/SimpleJdbcCallsHandler.java
+++
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/SimpleJdbcCallsHandler.java
@@ -37,19 +37,19 @@
return executeImpl(procedureName, paramSource,
createCallForModification(procedureName));
}
- public int executeModificationRowsAffected(final String procedureName,
final MapSqlParameterSource paramSource) {
- Integer numOfRows = null;
+ public int executeModificationReturnResult(final String procedureName,
final MapSqlParameterSource paramSource) {
+ Integer procedureResult = null;
Map<String, Object> result = executeImpl(procedureName, paramSource,
createCallForModification(procedureName));
if (!result.isEmpty()) {
List<?> resultArray = (List<?>) result.values().iterator().next();
if (resultArray != null && !resultArray.isEmpty()) {
Map<?, ?> resultMap = (Map<?, ?>) resultArray.get(0);
if (!resultMap.isEmpty()) {
- numOfRows = (Integer) resultMap.values().iterator().next();
+ procedureResult = (Integer)
resultMap.values().iterator().next();
}
}
}
- return (numOfRows != null) ? numOfRows : 0;
+ return (procedureResult != null) ? procedureResult : 0;
}
public <T> T executeRead(final String procedureName,
diff --git
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/AsyncTaskDAODbFacadeImpl.java
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/AsyncTaskDAODbFacadeImpl.java
index ae4c88e..d21158c 100644
---
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/AsyncTaskDAODbFacadeImpl.java
+++
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/AsyncTaskDAODbFacadeImpl.java
@@ -175,7 +175,7 @@
MapSqlParameterSource parameterSource =
getCustomMapSqlParameterSource()
.addValue("task_id", id);
- return
getCallsHandler().executeModificationRowsAffected("Deleteasync_tasks",
parameterSource);
+ return
getCallsHandler().executeModificationReturnResult("Deleteasync_tasks",
parameterSource);
}
@Override
diff --git
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAO.java
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAO.java
index c04d0ef..b0df866 100644
---
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAO.java
+++
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAO.java
@@ -16,6 +16,13 @@
public interface StoragePoolDAO extends GenericDao<storage_pool, Guid>,
StatusAwareDao<Guid, StoragePoolStatus>, SearchDAO<storage_pool> {
/**
+ * Increase master_domain_version of storage pool in DB and return a new
value
+ * @param id
+ * @return
+ */
+ int increaseStoragePoolMasterVersion(Guid id);
+
+ /**
* Retrieves the storage pool with the given ID, with optional filtering
* @param ID
* The ID of the storage pool
diff --git
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAODbFacadeImpl.java
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAODbFacadeImpl.java
index cd5ee77..6f4aa17 100644
---
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAODbFacadeImpl.java
+++
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAODbFacadeImpl.java
@@ -3,7 +3,6 @@
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.List;
-
import org.ovirt.engine.core.common.businessentities.ActionGroup;
import org.ovirt.engine.core.common.businessentities.QuotaEnforcementTypeEnum;
import org.ovirt.engine.core.common.businessentities.StorageFormatType;
@@ -381,4 +380,11 @@
mapper, parameterSource);
}
+ @Override
+ public int increaseStoragePoolMasterVersion(Guid id) {
+ MapSqlParameterSource parameterSource =
getCustomMapSqlParameterSource()
+ .addValue("id", id);
+ return
getCallsHandler().executeModificationReturnResult("Increase_storage_pool_master_version",
parameterSource);
+ }
+
}
diff --git
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAOHibernateImpl.java
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAOHibernateImpl.java
deleted file mode 100644
index 5a5cb11..0000000
---
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAOHibernateImpl.java
+++ /dev/null
@@ -1,100 +0,0 @@
-package org.ovirt.engine.core.dao;
-
-import java.util.List;
-
-import org.hibernate.Query;
-import org.hibernate.Session;
-import org.hibernate.criterion.Restrictions;
-import org.ovirt.engine.core.common.businessentities.ActionGroup;
-import org.ovirt.engine.core.common.businessentities.StoragePoolStatus;
-import org.ovirt.engine.core.common.businessentities.StorageType;
-import org.ovirt.engine.core.common.businessentities.storage_pool;
-import org.ovirt.engine.core.compat.Guid;
-import org.ovirt.engine.core.compat.NotImplementedException;
-
-public class StoragePoolDAOHibernateImpl extends
BaseDAOHibernateImpl<storage_pool, Guid> implements StoragePoolDAO {
- public StoragePoolDAOHibernateImpl() {
- super(storage_pool.class);
- }
-
- @Override
- public storage_pool get(Guid ID, Guid userID, boolean isFiltered) {
- throw new NotImplementedException();
- }
-
- @Override
- public storage_pool getForVds(final Guid vds) {
- Session session = getSession();
- Query query = session.getNamedQuery("all_storage_pools_for_vds");
- query.setParameter("vds_id", vds);
- return (storage_pool) query.uniqueResult();
- }
-
- @Override
- public storage_pool getForVdsGroup(final Guid vdsGroup) {
- Session session = getSession();
- Query query = session.getNamedQuery("all_storage_pools_for_vds_group");
-
- query.setParameter("vds_group_id", vdsGroup);
-
- return (storage_pool) query.uniqueResult();
- }
-
- @Override
- public List<storage_pool> getAll(Guid userID, boolean isFiltered) {
- throw new NotImplementedException();
- }
-
- @Override
- public List<storage_pool> getAllOfType(final StorageType type) {
- return findByCriteria(Restrictions.eq("storagePoolType",
type.getValue()));
- }
-
- @SuppressWarnings("unchecked")
- @Override
- public List<storage_pool> getAllForStorageDomain(final Guid storageDomain)
{
- Session session = getSession();
- Query query =
session.getNamedQuery("all_storage_pools_for_storage_domain");
-
- query.setParameter("storage_domain_id", storageDomain);
-
- return query.list();
- }
-
- @SuppressWarnings("unchecked")
- @Override
- public List<storage_pool> getAllWithQuery(final String queryString) {
- Session session = getSession();
- Query query = session.createQuery(queryString);
-
- return query.list();
- }
-
- @Override
- public void updateStatus(Guid id, StoragePoolStatus status) {
- // TODO Auto-generated method stub
-
- }
-
- @Override
- public void updatePartial(storage_pool pool) {
- // TODO Auto-generated method stub
-
- }
-
- @SuppressWarnings("unchecked")
- @Override
- public List<storage_pool> getDataCentersWithPermittedActionOnClusters(Guid
userId, ActionGroup actionGroup) {
- Query query =
getSession().getNamedQuery("fn_perms_get_storage_pools_with_permitted_action_on_vds_groups");
-
- query.setParameter("v_user_id",
userId).setParameter("v_action_group_id", actionGroup.getId());
-
- return (List<storage_pool>) query.uniqueResult();
- }
-
- @Override
- public List<storage_pool> getAllByStatus(StoragePoolStatus status) {
- // TODO Auto-generated method stub
- return null;
- }
-}
diff --git
a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOHibernateImplTest.java
b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOHibernateImplTest.java
deleted file mode 100644
index 93d23be..0000000
---
a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOHibernateImplTest.java
+++ /dev/null
@@ -1,17 +0,0 @@
-package org.ovirt.engine.core.dao;
-
-import org.junit.Test;
-import org.ovirt.engine.core.compat.Guid;
-import org.ovirt.engine.core.compat.NotImplementedException;
-
-/**
- * A sparse test case for {@link StoragePoolHibernateImpl}
- * intended to fail once we start implementing actual methods
- */
-public class StoragePoolDAOHibernateImplTest {
-
- @Test(expected = NotImplementedException.class)
- public void testUnimplementedMethod() {
- new StoragePoolDAOHibernateImpl().get(new Guid(), new Guid(), true);
- }
-}
diff --git
a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOTest.java
b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOTest.java
index f1ba402..1ac53c8 100644
---
a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOTest.java
+++
b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOTest.java
@@ -279,6 +279,12 @@
assertGetResult(result);
}
+ @Test
+ public void testIncreaseStoragePoolMasterVersion() {
+ int result =
dao.increaseStoragePoolMasterVersion(existingPool.getId());
+ assertEquals(result, existingPool.getmaster_domain_version() + 1);
+ }
+
/**
* Ensures that removing a storage pool works as expected.
*/
--
To view, visit http://gerrit.ovirt.org/11500
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaa2657ddfdbf2400f24341b07ab60743d6430481
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Michael Kublin <mkub...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
