Arik Hadas has uploaded a new change for review. Change subject: core: change asyncRunningVms to ConcurrentSkipListSet ......................................................................
core: change asyncRunningVms to ConcurrentSkipListSet ResourceManager#_asyncRunningVms used to be of type ConcurrentHashMap but provided functionality of Set. Since Java 1.6 there is an easy way to create thread-safe Set which is based on ConcurrentHashMap. By setting the asyncRunningVms collection to such Set, our code becomes more expressive and easier to read. This patch also contains minor refactoring in ResourceManager. Change-Id: I807936d76ee7298a126e7dd61d75cbb345e06829 Signed-off-by: Arik Hadas <aha...@redhat.com> --- M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/ResourceManager.java 1 file changed, 24 insertions(+), 27 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/52/24552/1 diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/ResourceManager.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/ResourceManager.java index 5f2f7df..d9da428 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/ResourceManager.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/ResourceManager.java @@ -2,10 +2,12 @@ import java.lang.reflect.Constructor; import java.util.ArrayList; +import java.util.Collections; import java.util.Date; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import org.apache.commons.lang.StringUtils; @@ -44,11 +46,11 @@ public class ResourceManager { - private static ResourceManager _Instance = new ResourceManager(); - private final Map<Guid, HashSet<Guid>> _vdsAndVmsList = new ConcurrentHashMap<Guid, HashSet<Guid>>(); - private final Map<Guid, VdsManager> _vdsManagersDict = new ConcurrentHashMap<Guid, VdsManager>(); - private final ConcurrentHashMap<Guid, Boolean> _asyncRunningVms = - new ConcurrentHashMap<Guid, Boolean>(); + private static ResourceManager instance = new ResourceManager(); + private final Map<Guid, HashSet<Guid>> vdsAndVmsList = new ConcurrentHashMap<>(); + private final Map<Guid, VdsManager> vdsManagersDict = new ConcurrentHashMap<>(); + private final Set<Guid> asyncRunningVms = + Collections.newSetFromMap(new ConcurrentHashMap<Guid, Boolean>()); private static final String VDSCommandPrefix = "VDSCommand"; @@ -59,7 +61,7 @@ } public static ResourceManager getInstance() { - return _Instance; + return instance; } public void init() { @@ -82,7 +84,7 @@ if (vm.getRunOnVds() != null) { MultiValueMapUtils.addToMap(vm.getRunOnVds(), vm.getId(), - _vdsAndVmsList, + vdsAndVmsList, new MultiValueMapUtils.HashSetCreator<Guid>()); } if (vm.getRunOnVds() != null && nonResponsiveVdss.contains(vm.getRunOnVds())) { @@ -127,20 +129,16 @@ } public boolean AddAsyncRunningVm(Guid vmId) { - boolean returnValue = false; - if (_asyncRunningVms.putIfAbsent(vmId, Boolean.TRUE) == null) { - returnValue = true; - } - return returnValue; + return asyncRunningVms.add(vmId); } public void RemoveAsyncRunningVm(Guid vmId) { - _asyncRunningVms.remove(vmId); + asyncRunningVms.remove(vmId); getEventListener().removeAsyncRunningCommand(vmId); } public void succededToRunVm(Guid vmId, Guid vdsId) { - if (_asyncRunningVms.containsKey(vmId)) { + if (asyncRunningVms.contains(vmId)) { getEventListener().runningSucceded(vmId); } RemoveAsyncRunningVm(vmId); @@ -151,32 +149,31 @@ * @param vmId */ public void RerunFailedCommand(Guid vmId, Guid vdsId) { - Boolean value = _asyncRunningVms.remove(vmId); - // remove async record from broker only - if (value != null) { + if (asyncRunningVms.remove(vmId)) { + // remove async record from broker only getEventListener().rerun(vmId); } } public boolean IsVmInAsyncRunningList(Guid vmId) { - return (_asyncRunningVms.containsKey(vmId)); + return asyncRunningVms.contains(vmId); } public void RemoveVmFromDownVms(Guid vdsId, Guid vmId) { - HashSet<Guid> vms = null; - if ((vms = _vdsAndVmsList.get(vdsId)) != null) { + HashSet<Guid> vms = vdsAndVmsList.get(vdsId); + if (vms != null) { vms.remove(vmId); } } public void HandleVdsFinishedInit(Guid vdsId) { - HashSet<Guid> vms = null; - if ((vms = _vdsAndVmsList.get(vdsId)) != null) { + HashSet<Guid> vms = vdsAndVmsList.get(vdsId); + if (vms != null) { for (Guid vmId : vms) { getEventListener().processOnVmStop(vmId); log.info("Procceed on vm stop entered: " + vmId.toString()); } - _vdsAndVmsList.remove(vdsId); + vdsAndVmsList.remove(vdsId); } } @@ -209,7 +206,7 @@ vdsManager.updateDynamicData(vds.getDynamicData()); } vdsManager.schedulJobs(); - _vdsManagersDict.put(vds.getId(), vdsManager); + vdsManagersDict.put(vds.getId(), vdsManager); log.infoFormat("VDS {0} was added to the Resource Manager", vds.getId()); } @@ -218,12 +215,12 @@ VdsManager vdsManager = GetVdsManager(vdsId); if (vdsManager != null) { vdsManager.dispose(); - _vdsManagersDict.remove(vdsId); + vdsManagersDict.remove(vdsId); } } public VdsManager GetVdsManager(Guid vdsId) { - VdsManager vdsManger = _vdsManagersDict.get(vdsId); + VdsManager vdsManger = vdsManagersDict.get(vdsId); if (vdsManger == null) { log.errorFormat("Cannot get vdsManager for vdsid={0}", vdsId); } @@ -259,7 +256,7 @@ } public boolean IsVmDuringInitiating(Guid vm_guid) { - return _asyncRunningVms.containsKey(vm_guid); + return asyncRunningVms.contains(vm_guid); } /** -- To view, visit http://gerrit.ovirt.org/24552 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I807936d76ee7298a126e7dd61d75cbb345e06829 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <aha...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches