Roy Golan has posted comments on this change.

Change subject: core: Prevent dead lock on vm device (#852451)
......................................................................


Patch Set 3: (3 inline comments)

....................................................
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/MassOperationsGenericDaoDbFacade.java
Line 17:  */
Line 18: public abstract class MassOperationsGenericDaoDbFacade<T extends 
BusinessEntity<ID>, ID extends Serializable>
Line 19:         extends DefaultGenericDaoDbFacade<T, ID> implements 
MassOperationsDao<T> {
Line 20: 
Line 21:     public MassOperationsGenericDaoDbFacade(String 
entityStoredProcedureName) {
I generally think this MassOperation concept would perform better if we'd 
create a special procedure and let the DB do the work instead of loading it in 
a loop
Line 22:         super(entityStoredProcedureName);
Line 23:     }
Line 24: 
Line 25:     @Override


Line 22:         super(entityStoredProcedureName);
Line 23:     }
Line 24: 
Line 25:     @Override
Line 26:     public void updateAll(Collection<T> entities) {
this should be called massUpdate and the method below should be 
massProcedureCall or something similar.
Line 27:         updateAll(getProcedureNameForUpdate(),entities);
Line 28:         for (T entity : entities) {
Line 29:             
getCallsHandler().executeModification(getProcedureNameForUpdate(),
Line 30:                     createFullParametersMapper(entity));


Line 32:     }
Line 33: 
Line 34:     @Override
Line 35:     public void updateAll(String procedureName, Collection<T> 
entities) {
Line 36:         if (procedureName == null) {
I don't like this API its unclear what is the effect of sending null to the 
external user is.moreover it encourages the user to use it instead of using the 
call above to just updateAll(entities)
Line 37:             procedureName = getProcedureNameForUpdate();
Line 38:         }
Line 39:         for (T entity : entities) {
Line 40:             getCallsHandler().executeModification(procedureName,


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If316259f9760777c5f85db0880a157021c61a6ba
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to