Yair Zaslavsky has posted comments on this change.

Change subject: [RFE] Add periodic power management health check..
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.ovirt.org/#/c/27367/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/pm/PmHealthCheckManager.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/pm/PmHealthCheckManager.java:

Line 24:  */
Line 25: public class PmHealthCheckManager {
Line 26: 
Line 27:     private static final Log log = 
LogFactory.getLog(PmHealthCheckManager.class);
Line 28:     private static final PmHealthCheckManager instance = new 
PmHealthCheckManager();
please explain the synchronization later on.. why did you do it?
Why not -
a. private static final volatile PMHealthCheckManager instance = null;

And then at getInstance use the double check pattern?
Line 29:     private boolean active = false;
Line 30: 
Line 31:     private PmHealthCheckManager() {
Line 32:         // intentionally empty


Line 51:     }
Line 52: 
Line 53:     @OnTimerMethodAnnotation("pmHealthCheck")
Line 54:     public void pmHealthCheck() {
Line 55:         // skip PM health check if previous operation is not complete 
yet
s/complete/completed
Line 56:         if (!active) {
Line 57:             try {
Line 58:                 synchronized (instance) {
Line 59:                     log.info("Power Management Health Check started.");


Line 54:     public void pmHealthCheck() {
Line 55:         // skip PM health check if previous operation is not complete 
yet
Line 56:         if (!active) {
Line 57:             try {
Line 58:                 synchronized (instance) {
do u fear that two concurrent calls will happen?
Line 59:                     log.info("Power Management Health Check started.");
Line 60:                     active = true;
Line 61:                     List<VDS> hosts = 
DbFacade.getInstance().getVdsDao().getAll();
Line 62:                     for (VDS host : hosts) {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib1dfeab92a35793ebb421db8e3be94587dfe85e2
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Martin Peřina <mper...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
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