Allon Mureinik has posted comments on this change.
Change subject: engine : VdsUpdateRuntimeInfo updates the data of a specific
VDS.
......................................................................
Patch Set 12: I would prefer that you didn't submit this
(5 inline comments)
The implementation of the DAOs is flawed, IMHO - see inline.
Also, in migration from using VdsUpdateRuntimeInfo.updateAllInTransaction to
the current approach, you're losing the Transaction.Required scope.
Is this intended?
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskImageDynamicDAODbFacadeImpl.java
Line 66: return DiskImageDynamicRowMapper.instance;
Line 67: }
Line 68:
Line 69: @Override
Line 70: public void updateAllInTransaction(Map<Guid, DiskImageDynamic>
vmDiskImageDynamicToSave) {
This entire method is redundant.
What you should do is implement updarteAllInBatch(Collection<T>) in
MassOperationGenericDAO in a similar fashion to updateAll(Collection<T>) and
then call it from there.
There's no point in having this type of method in all the DAOs.
Line 71: updateAllInBatch("Updatedisk_image_dynamic",
Line 72: vmDiskImageDynamicToSave.values(),
Line 73: new MapSqlParameterMapper<DiskImageDynamic>() {
Line 74:
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/VmNetworkStatisticsDaoDbFacadeImpl.java
Line 60: };
Line 61: }
Line 62:
Line 63: @Override
Line 64: public void updateAllInTransaction(List<VmNetworkStatistics>
vmInterfaceStatisticsToSave) {
This entire method is redundant.
What you should do is implement updarteAllInBatch(Collection<T>) in
MassOperationGenericDAO in a similar fashion to updateAll(Collection<T>) and
then call it from there.
There's no point in having this type of method in all the DAOs.
Line 65: updateAllInBatch("updatevm_interface_statistics",
Line 66: vmInterfaceStatisticsToSave,
Line 67: new MapSqlParameterMapper<VmNetworkStatistics>() {
Line 68:
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDeviceDAODbFacadeImpl.java
Line 198:
Line 199: }
Line 200:
Line 201: @Override
Line 202: public void updateAllInTransaction(Collection<VmDevice>
vmDeviceToSave) {
This entire method is redundant.
What you should do is implement updarteAllInBatch(Collection<T>) in
MassOperationGenericDAO in a similar fashion to updateAll(Collection<T>) and
then call it from there.
There's no point in having this type of method in all the DAOs.
Line 203: updateAllInBatch("updatevmdynamic", vmDeviceToSave, new
MapSqlParameterMapper<VmDevice>() {
Line 204: @Override
Line 205: public MapSqlParameterSource map(VmDevice entity) {
Line 206: MapSqlParameterSource paramValue = new
MapSqlParameterSource();
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDynamicDAODbFacadeImpl.java
Line 174: };
Line 175: }
Line 176:
Line 177: @Override
Line 178: public void updateAllInTransaction(Map<Guid, VmDynamic>
vmDynamicToSave) {
This entire method is redundant.
What you should do is implement updarteAllInBatch(Collection<T>) in
MassOperationGenericDAO in a similar fashion to updateAll(Collection<T>) and
then call it from there.
There's no point in having this type of method in all the DAOs.
Line 179: updateAllInBatch("updatevmdynamic", vmDynamicToSave.values(),
new MapSqlParameterMapper<VmDynamic>() {
Line 180: @Override
Line 181: public MapSqlParameterSource map(VmDynamic entity) {
Line 182: MapSqlParameterSource paramValue = new
MapSqlParameterSource();
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmStatisticsDaoDbFacadeImpl.java
Line 70: };
Line 71: }
Line 72:
Line 73: @Override
Line 74: public void updateAllInTransaction(Map<Guid, VmStatistics>
vmStatisticsToSave) {
This entire method is redundant.
What you should do is implement updarteAllInBatch(Collection<T>) in
MassOperationGenericDAO in a similar fashion to updateAll(Collection<T>) and
then call it from there.
There's no point in having this type of method in all the DAOs.
Line 75: updateAllInBatch("updatevmstatistics",
Line 76: vmStatisticsToSave.values(),
Line 77: new MapSqlParameterMapper<VmStatistics>() {
Line 78: @Override
--
To view, visit http://gerrit.ovirt.org/16066
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3e9735f4174ebabbe293d397d931aba72bbbd7b
Gerrit-PatchSet: 12
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liran Zelkha <liran.zel...@gmail.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Liran Zelkha <liran.zel...@gmail.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@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
