Liran Zelkha has posted comments on this change.

Change subject: core: Eliminate unneeded updates to VdsDynamic
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.ovirt.org/#/c/23852/1/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDynamicDAODbFacadeImpl.java
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDynamicDAODbFacadeImpl.java:

Line 267:         return VdcDynamicRowMapper.instance;
Line 268:     }
Line 269: 
Line 270:     @Override
Line 271:     public void updateIfNeeded(VdsDynamic vdsDynamic) {
> Liran
I think we can create a UpdateIfNeeded interface, and pull all this stuff up 
there.
Line 272:         VdsDynamic dbData = get(vdsDynamic.getId());
Line 273:         if (!dbData.equals(vdsDynamic)) {
Line 274:             update(vdsDynamic);
Line 275:         } else {


Line 272:         VdsDynamic dbData = get(vdsDynamic.getId());
Line 273:         if (!dbData.equals(vdsDynamic)) {
Line 274:             update(vdsDynamic);
Line 275:         } else {
Line 276:             log.info("Ignored an unneeded update of VdsDynamic");
> We won't want to see this in regular log , I think it should be a debug mes
We should. Updating for no reason is a bug, and developers should see it.
Line 277:         }
Line 278:     }


http://gerrit.ovirt.org/#/c/23852/1/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java:

Line 369:      *
Line 370:      * @param dynamicData
Line 371:      */
Line 372:     public void updateDynamicData(VdsDynamic dynamicData) {
Line 373:         
DbFacade.getInstance().getVdsDynamicDao().updateIfNeeded(dynamicData);
> I think the call here should be to update, there are two cases 
OK. I'll do that.
Line 374:     }
Line 375: 
Line 376:     /**
Line 377:      * Save statistics data to cache and DB.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icab3ed7d251573f150d28f966d0ad2363ecc3441
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liran Zelkha <liran.zel...@gmail.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Liran Zelkha <liran.zel...@gmail.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to