Oved Ourfali has posted comments on this change. Change subject: [RFE] Add periodic power management health check.. ......................................................................
Patch Set 3: (4 comments) http://gerrit.ovirt.org/#/c/27367/3/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 PmHealthCheckManager instance = new PmHealthCheckManager(); isn't that final as well? Line 29: private boolean inWork=false; Line 30: Line 31: private PmHealthCheckManager() { Line 32: // intentionally empty Line 25: public class PmHealthCheckManager { Line 26: Line 27: private static final Log log = LogFactory.getLog(PmHealthCheckManager.class); Line 28: private static PmHealthCheckManager instance = new PmHealthCheckManager(); Line 29: private boolean inWork=false; I'd call it active. also, add spacing: active = false Line 30: Line 31: private PmHealthCheckManager() { Line 32: // intentionally empty Line 33: } Line 38: public void initialize() { Line 39: log.info("Start initializing " + getClass().getSimpleName()); Line 40: if(Config.<Boolean>getValue(ConfigValues.PMHealthCheckEnabled)) { Line 41: Integer pmHealthCheckInterval = Config.<Integer> getValue(ConfigValues.PMHealthCheckIntervalInSec); Line 42: SchedulerUtilQuartzImpl.getInstance().scheduleAFixedDelayJob(this, I'd add some logging here as well, to show that something is being initialized. Line 43: "pmHealthCheck", Line 44: new Class[]{}, Line 45: new Object[]{}, Line 46: pmHealthCheckInterval, http://gerrit.ovirt.org/#/c/27367/3/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql File packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql: Line 445: select fn_db_add_config_value('PMHealthCheckEnabled','false','3.0'); Line 446: select fn_db_add_config_value('PMHealthCheckEnabled','false','3.1'); Line 447: select fn_db_add_config_value('PMHealthCheckEnabled','false','3.2'); Line 448: select fn_db_add_config_value('PMHealthCheckEnabled','false','3.3'); Line 449: select fn_db_add_config_value('PMHealthCheckEnabled','false','3.4'); if here it is false by default, why do you put true in ConfigValues? Also, why not use "general"? Line 450: select fn_db_add_config_value('PMHealthCheckIntervalInSec','3600','general'); Line 451: select fn_db_add_config_value('PosixStorageEnabled','false','3.0'); Line 452: select fn_db_add_config_value('PostgresI18NPrefix','','general'); Line 453: select fn_db_add_config_value('PostgresLikeSyntax','ILIKE','general'); -- 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: 3 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