Arik Hadas has posted comments on this change.

Change subject: vdsbroker: reduced scope of synchronized blocks
......................................................................


Patch Set 1:

(1 comment)

"VdsIdVDSCommandBase - there were few threads blocked on it as well so I have 
decided to reduce synchronization scope.
The changes were perf tested and we saw no blocked threads."

I doubts the results then. it doesn't make sense to me that reducing the 
synchronize blocks that way eliminated the blocked threads on the vds lock. I 
would do another test - compute the time the vds lock is taken before this 
change and with this change. I think the difference will be negligible, but I 
don't know, maybe I'm missing something that is not obvious.

https://gerrit.ovirt.org/#/c/37947/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 574:                         AuditLogType.VDS_FAILED_TO_RUN_VMS);
Line 575:                 log.info("Vds '{}' moved to Error mode after {} 
attempts. Time: {}", vds.getName(),
Line 576:                         mFailedToRunVmAttempts, new Date());
Line 577:             }
Line 578:         }
> Please take a look at 557. It was synchronized on vdsManager#lockObj so we 
I see, the answer is that it is called from FailedToRunVmVDSCommand that locked 
it, but it doesn't lock it anymore after changing VdsIdVDSCommandBase..
Line 579:     }
Line 580: 
Line 581:     /**
Line 582:      */


-- 
To view, visit https://gerrit.ovirt.org/37947
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7d1bfd7b1fc7bfcc6465eae62feda6f1a27ff455
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Tim Speetjens <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to