Ayal Baron has posted comments on this change.

Change subject: engine: Removing unneeded read query from SetVdsStatusVDSCommand
......................................................................


Patch Set 1: (3 inline comments)

....................................................
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/SetVdsStatusVDSCommand.java
Line 19:         SetVdsStatusVDSCommandParameters parameters = getParameters();
Line 20: 
Line 21:         if (_vdsManager != null) {
Line 22: 
Line 23:             VDS vds = getVds();
the query you removed loads the vds from the db while here it can come from 
cache, wouldn't it be more reliable to load it from db here?
Line 24:             updateVdsFromParameters(parameters, vds);
Line 25:             _vdsManager.setStatus(parameters.getStatus(), vds);
Line 26:             _vdsManager.UpdateDynamicData(vds.getDynamicData());
Line 27:             _vdsManager.UpdateStatisticsData(vds.getStatisticsData());


Line 24:             updateVdsFromParameters(parameters, vds);
Line 25:             _vdsManager.setStatus(parameters.getStatus(), vds);
Line 26:             _vdsManager.UpdateDynamicData(vds.getDynamicData());
Line 27:             _vdsManager.UpdateStatisticsData(vds.getStatisticsData());
Line 28: 
I agree this code seems redundant, but are you solving an actual issue or is 
this just a cleanup?
Line 29:             if (vds.getspm_status() != VdsSpmStatus.None && 
parameters.getStatus() != VDSStatus.Up) {
Line 30:                 log.infoFormat("SetVdsStatusVDSCommand::VSD {0} is spm 
and moved from up calling ResetIrs.",
Line 31:                         vds.getvds_name());
Line 32:                 // check if this host was spm and reset if do.


....................................................
Commit Message
Line 7: engine: Removing unneeded read query from SetVdsStatusVDSCommand
Line 8: 
Line 9: The following query was used in order to prevent some kind of race,
Line 10: obvious that a probability that such approach will help is too small.
Line 11: Additional query is perfromed all the time
please specify which additional query you're referring to in both cases (the 
one used to minimize race window and the one that is performed all the time).
While at it, please fix perfromed to performed.
Line 12: 
Line 13: Change-Id: If468915f3bde5c5e2599ee50c38515795a555e79


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If468915f3bde5c5e2599ee50c38515795a555e79
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Michael Kublin <mkub...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to