Sahina Bose has posted comments on this change.
Change subject: engine:Sync gluster hooks
......................................................................
Patch Set 4: (21 inline comments)
Thanks Shireesh for the detailed review and comments. Have modified and tested
code, patchset to follow
....................................................
File backend/manager/dbscripts/upgrade/pre_upgrade/0000_config.sql
Line 142: select
fn_db_add_config_value('GlusterVolumeOptionGroupVirtValue','virt','general');
Line 143: select
fn_db_add_config_value('GlusterVolumeOptionOwnerUserVirtValue','36','general');
Line 144: select
fn_db_add_config_value('GlusterVolumeOptionOwnerGroupVirtValue','36','general');
Line 145: select fn_db_add_config_value('GlusterRefreshRateHooks', '600',
'general');
Line 146: select fn_db_add_config_value('GlusterHooksEnabled', 'true', '3.3');
Done
Line 147: select
fn_db_add_config_value('GuestToolsSetupIsoPrefix','RHEV-toolsSetup_','general');
Line 148: select fn_db_add_config_value('HardwareInfoEnabled','false','3.0');
Line 149: select fn_db_add_config_value('HardwareInfoEnabled','false','3.1');
Line 150: select fn_db_add_config_value('HardwareInfoEnabled','true','3.2');
Line 142: select
fn_db_add_config_value('GlusterVolumeOptionGroupVirtValue','virt','general');
Line 143: select
fn_db_add_config_value('GlusterVolumeOptionOwnerUserVirtValue','36','general');
Line 144: select
fn_db_add_config_value('GlusterVolumeOptionOwnerGroupVirtValue','36','general');
Line 145: select fn_db_add_config_value('GlusterRefreshRateHooks', '600',
'general');
Line 146: select fn_db_add_config_value('GlusterHooksEnabled', 'true', '3.3');
Done.
Line 147: select
fn_db_add_config_value('GuestToolsSetupIsoPrefix','RHEV-toolsSetup_','general');
Line 148: select fn_db_add_config_value('HardwareInfoEnabled','false','3.0');
Line 149: select fn_db_add_config_value('HardwareInfoEnabled','false','3.1');
Line 150: select fn_db_add_config_value('HardwareInfoEnabled','true','3.2');
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterHookSyncJob.java
Line 40: log.debug("Refreshing hooks list");
Line 41: List<VDSGroup> clusters = getClusterDao().getAll();
Line 42:
Line 43: for (VDSGroup cluster : clusters) {
Line 44: if (!cluster.supportsGlusterService() &&
GlusterFeatureSupported.glusterHooks(cluster.getcompatibility_version()))
Sure - will change
Line 45: {
Line 46: break;
Line 47: }
Line 48:
Line 42:
Line 43: for (VDSGroup cluster : clusters) {
Line 44: if (!cluster.supportsGlusterService() &&
GlusterFeatureSupported.glusterHooks(cluster.getcompatibility_version()))
Line 45: {
Line 46: break;
Yes. my bad!
Line 47: }
Line 48:
Line 49: List<VDS> upServers =
getClusterUtils().getAllUpServers(cluster.getId());
Line 50:
Line 44: if (!cluster.supportsGlusterService() &&
GlusterFeatureSupported.glusterHooks(cluster.getcompatibility_version()))
Line 45: {
Line 46: break;
Line 47: }
Line 48:
Done
Line 49: List<VDS> upServers =
getClusterUtils().getAllUpServers(cluster.getId());
Line 50:
Line 51: List<Callable<Pair<VDS, VDSReturnValue>>> taskList = new
ArrayList<Callable<Pair<VDS, VDSReturnValue>>>();
Line 52: for (final VDS upServer : upServers) {
Line 80:
Line 81: for (Pair<VDS, VDSReturnValue> pairResult : pairResults) {
Line 82: VDS server = pairResult.getFirst();
Line 83: if (!pairResult.getSecond().getSucceeded()) {
Line 84: log.infoFormat("Failed to get hooklist from server {0}
with error {1} ", server, pairResult.getSecond().getVdsError());
Done
Line 85: logUtil.logServerMessage(server,
AuditLogType.GLUSTER_HOOK_LIST_FAILED);
Line 86: break;
Line 87: }
Line 88: List<GlusterHookEntity> returnedHooks =
(List<GlusterHookEntity>) pairResult.getSecond().getReturnValue();
Line 82: VDS server = pairResult.getFirst();
Line 83: if (!pairResult.getSecond().getSucceeded()) {
Line 84: log.infoFormat("Failed to get hooklist from server {0}
with error {1} ", server, pairResult.getSecond().getVdsError());
Line 85: logUtil.logServerMessage(server,
AuditLogType.GLUSTER_HOOK_LIST_FAILED);
Line 86: break;
Done
Line 87: }
Line 88: List<GlusterHookEntity> returnedHooks =
(List<GlusterHookEntity>) pairResult.getSecond().getReturnValue();
Line 89:
Line 90: for (GlusterHookEntity retHook: returnedHooks) {
Line 84: log.infoFormat("Failed to get hooklist from server {0}
with error {1} ", server, pairResult.getSecond().getVdsError());
Line 85: logUtil.logServerMessage(server,
AuditLogType.GLUSTER_HOOK_LIST_FAILED);
Line 86: break;
Line 87: }
Line 88: List<GlusterHookEntity> returnedHooks =
(List<GlusterHookEntity>) pairResult.getSecond().getReturnValue();
Done
Line 89:
Line 90: for (GlusterHookEntity retHook: returnedHooks) {
Line 91: HookKey key=
HookKey.valueOf(retHook.getGlusterCommand(),retHook.getStage(),retHook.getName());
Line 92: GlusterHookEntity existingHook =
existingHookMap.get(key);
Line 87: }
Line 88: List<GlusterHookEntity> returnedHooks =
(List<GlusterHookEntity>) pairResult.getSecond().getReturnValue();
Line 89:
Line 90: for (GlusterHookEntity retHook: returnedHooks) {
Line 91: HookKey key=
HookKey.valueOf(retHook.getGlusterCommand(),retHook.getStage(),retHook.getName());
When I initially started I had envisaged using HookKey class differently -
thought I would need to retrieve command/stage from it too. But now what you
are saying makes things much simpler. Thanks!
Line 92: GlusterHookEntity existingHook =
existingHookMap.get(key);
Line 93:
Line 94: addToReturnedHookKeyList(returnedHookKeyList, retHook);
Line 95:
Line 90: for (GlusterHookEntity retHook: returnedHooks) {
Line 91: HookKey key=
HookKey.valueOf(retHook.getGlusterCommand(),retHook.getStage(),retHook.getName());
Line 92: GlusterHookEntity existingHook =
existingHookMap.get(key);
Line 93:
Line 94: addToReturnedHookKeyList(returnedHookKeyList, retHook);
Done
Line 95:
Line 96: if (existingHook != null) {
Line 97: GlusterServerHook serverHook =
getServerHookForServer(existingHook, server);
Line 98: // if null, insert serverhook
Line 97: GlusterServerHook serverHook =
getServerHookForServer(existingHook, server);
Line 98: // if null, insert serverhook
Line 99: if (serverHook == null) {
Line 100: log.infoFormat("Adding new server hook {0} in
server {1} ", key,server);
Line 101:
newServerHooks.add(buildServerHook(server.getId(), existingHook.getId(),
retHook));
Adding ServerHook in DB for each server makes it easier to manage if we have to
show list of server hooks. But as per discussion, we will only show conflicting
hooks. So changing this
Line 102: } else {
Line 103: // if not null, compare to see if update required
Line 104: if
(!(serverHook.getChecksum().equals(retHook.getChecksum()) &&
serverHook.getContentType().equals(retHook.getContentType())
Line 105: &&
serverHook.getStatus().equals(retHook.getStatus()))) {
Line 99: if (serverHook == null) {
Line 100: log.infoFormat("Adding new server hook {0} in
server {1} ", key,server);
Line 101:
newServerHooks.add(buildServerHook(server.getId(), existingHook.getId(),
retHook));
Line 102: } else {
Line 103: // if not null, compare to see if update required
Done
Line 104: if
(!(serverHook.getChecksum().equals(retHook.getChecksum()) &&
serverHook.getContentType().equals(retHook.getContentType())
Line 105: &&
serverHook.getStatus().equals(retHook.getStatus()))) {
Line 106:
serverHook.setChecksum(retHook.getChecksum());
Line 107:
serverHook.setContentType(retHook.getContentType());
Line 100: log.infoFormat("Adding new server hook {0} in
server {1} ", key,server);
Line 101:
newServerHooks.add(buildServerHook(server.getId(), existingHook.getId(),
retHook));
Line 102: } else {
Line 103: // if not null, compare to see if update required
Line 104: if
(!(serverHook.getChecksum().equals(retHook.getChecksum()) &&
serverHook.getContentType().equals(retHook.getContentType())
I will keep this for now. If needed will refactor later.
Line 105: &&
serverHook.getStatus().equals(retHook.getStatus()))) {
Line 106:
serverHook.setChecksum(retHook.getChecksum());
Line 107:
serverHook.setContentType(retHook.getContentType());
Line 108: serverHook.setStatus(retHook.getStatus());
Line 107:
serverHook.setContentType(retHook.getContentType());
Line 108: serverHook.setStatus(retHook.getStatus());
Line 109: updatedServerHooks.add(serverHook);
Line 110: }
Line 111:
Done
Line 112: }
Line 113: Integer oldConflictValue =
existingHook.getConflictStatus();
Line 114: existingHook = setConflict(existingHook, retHook);
Line 115: if
(!oldConflictValue.equals(existingHook.getConflictStatus())) {
Line 120: hooksWithConflictChanges.put(key,
existingHook);
Line 121: }
Line 122: } else {
Line 123: log.infoFormat("Adding new hook {0} in server {1}
", key,server);
Line 124: existingHook = newHookMap.get(key);
Done
Line 125: if (existingHook == null) {
Line 126: existingHook = retHook;
Line 127: existingHook.setId(Guid.NewGuid());
Line 128: }
Line 200: }
Line 201:
Line 202: private GlusterServerHook
getServerHookForServer(GlusterHookEntity hook, VDS server) {
Line 203: if (hook.getServerHooks() == null) {
Line 204:
hook.setServerHooks(getHooksDao().getGlusterServerHooks(hook.getId()));
Yes, removing this method to use the dao call. Initially added this to avoid
multiple db calls, but not sure if there's significant gain with only conflict
hooks being stored.
Line 205: }
Line 206: for (GlusterServerHook serverHook: hook.getServerHooks()) {
Line 207: if (serverHook.getServerId().equals(server.getId())) {
Line 208: return serverHook;
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterJobsManager.java
Line 44: "refreshHooks",
Line 45: new Class[0],
Line 46: new Object[0],
Line 47: getGlusterRefreshRateHooks(),
Line 48: getGlusterRefreshRateHooks(),
Done
Line 49: TimeUnit.SECONDS);
Line 50:
Line 51: }
Line 52:
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GlusterHookSyncJobTest.java
Line 157: return cluster;
Line 158: }
Line 159:
Line 160: @Test
Line 161: public void syncHooks() {
Yes. will add
Line 162: initMocks();
Line 163: hookManager.refreshHooks();
Line 164: Mockito.verify(hooksDao,
times(1)).save(any(GlusterHookEntity.class));
Line 165: //Mockito.verify(hooksDao,
times(1)).saveGlusterServerHook(any(GlusterServerHook.class));
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
Line 1358:
Line 1359:
Line 1360: @TypeConverterAttribute(Integer.class)
Line 1361: @DefaultValueAttribute("600")
Line 1362: GlusterRefreshRateHooks(503),
Done
Line 1363:
Line 1364: @TypeConverterAttribute(Boolean.class)
Line 1365: @DefaultValueAttribute("false")
Line 1366: GlusterHooksEnabled(504),
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java
Line 106: severities.put(AuditLogType.GLUSTER_HOOK_ENABLE_FAILED,
AuditLogSeverity.ERROR);
Line 107: severities.put(AuditLogType.GLUSTER_HOOK_DISABLE,
AuditLogSeverity.NORMAL);
Line 108: severities.put(AuditLogType.GLUSTER_HOOK_DISABLE_PARTIAL,
AuditLogSeverity.WARNING);
Line 109: severities.put(AuditLogType.GLUSTER_HOOK_DISABLE_FAILED,
AuditLogSeverity.ERROR);
Line 110: severities.put(AuditLogType.GLUSTER_HOOK_LIST_FAILED,
AuditLogSeverity.ERROR);
Done
Line 111: }
Line 112:
Line 113: private static void initDefaultSeverities() {
Line 114: severities.put(AuditLogType.UNASSIGNED,
AuditLogSeverity.NORMAL);
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterHooksDaoDbFacadeImpl.java
Line 74: .addValue("includeContent", false));
Line 75: return glusterHook;
Line 76: }
Line 77:
Line 78: public List<GlusterServerHook> getGlusterServerHooks(Guid hookId) {
Removing this method as not required any more
Line 79: List<GlusterServerHook> serverHooks =
getCallsHandler().executeReadList("GetGlusterServerHooksById",
glusterServerHookRowMapper,
Line 80: createIdParameterMapper(hookId));
Line 81: return serverHooks;
Line 82: }
--
To view, visit http://gerrit.ovirt.org/13943
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2766edd4e810677a200cf5c45d334cabef5f2924
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sahina Bose <sab...@redhat.com>
Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com>
Gerrit-Reviewer: Sahina Bose <sab...@redhat.com>
Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
