Ramesh N has posted comments on this change. Change subject: engine: sync job for gluster disk provisioning ......................................................................
Patch Set 6: (4 comments) Added some unit tests. Patch set to follow. http://gerrit.ovirt.org/#/c/36429/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/StorageDeviceSyncJob.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/StorageDeviceSyncJob.java: Line 47: Line 48: private void refreshStorageDevicesInCluster(VDSGroup cluster) { Line 49: List<VDS> allUpServers = getClusterUtils().getAllUpServers(cluster.getId()); Line 50: for (VDS vds : allUpServers) { Line 51: refreshStorageDevicesFromServer(vds); > You could use a ThreadPool to execute simultaneously on multiple hosts Done Line 52: } Line 53: Line 54: } Line 55: Line 59: if (returnValue.getSucceeded()) { Line 60: List<StorageDevice> storageDevices = (List<StorageDevice>) returnValue.getReturnValue(); Line 61: updateStorageDevices(vds, storageDevices); Line 62: } else { Line 63: log.error("VDS error {}", returnValue.getVdsError().getMessage()); > VDS error retrieving storage devices '{}' Done Line 64: log.debug("VDS error", returnValue.getVdsError()); Line 65: } Line 66: } Line 67: Line 76: // Make deviceUuid to Device map and deviceName to device map so that we can find the Line 77: // newly added and updated devices without looping over the same list again and again. Line 78: for (StorageDevice storageDevice : storageDevicesInDb) { Line 79: nameToDeviceMap.put(storageDevice.getName(), storageDevice); Line 80: if (storageDevice.getDevUuid() != null && !storageDevice.getDevUuid().isEmpty()) { > When would the Device UUID be null? Is the nameToDevice map required? Device UUID is empty for some disks(raid volumes). Over all I have the following assumption. 1. If we have Device UUID then its the key to identify 2. If we don't have Device UUID then name is the key. 3. Name can change after reboot but Device UUID wont change. nameToDeviceMap is required to look up devices using names when loop up on device UUID is failed. Line 81: deviceUuidToDeviceMap.put(storageDevice.getDevUuid(), storageDevice); Line 82: } Line 83: } Line 84: http://gerrit.ovirt.org/#/c/36429/6/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql File packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql: Line 149: select fn_db_add_config_value_for_versions_up_to('GlusterRefreshHeavyWeight', 'false', '3.1'); Line 150: select fn_db_add_config_value('GlusterRefreshRateHooks', '7200', 'general'); Line 151: select fn_db_add_config_value('GlusterRefreshRateLight', '5', 'general'); Line 152: select fn_db_add_config_value('GlusterRefreshRateHeavy', '300', 'general'); Line 153: select fn_db_add_config_value('GlusterRefreshRateStorageDevices', '18000', 'general'); > This should reflect the default in the ConfigValues file so 7200? Done Line 154: select fn_db_add_config_value('GlusterSupport', 'false', '3.0'); Line 155: select fn_db_add_config_value_for_versions_up_to('GlusterSupportForceCreateVolume', 'false', '3.3'); Line 156: select fn_db_add_config_value('GlusterVolumeOptionGroupVirtValue','virt','general'); Line 157: select fn_db_add_config_value('GlusterVolumeOptionOwnerUserVirtValue','36','general'); -- To view, visit http://gerrit.ovirt.org/36429 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I651bb51873a96d491c5a5f51147cb72be958985a Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ramesh N <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Kanagaraj M <[email protected]> Gerrit-Reviewer: Ramesh N <[email protected]> Gerrit-Reviewer: Sahina Bose <[email protected]> Gerrit-Reviewer: Shubhendu Tripathi <[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
