From 691e944fb808d38a0cc860059acac003584419ca Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sun, 18 Nov 2018 16:43:20 +0100 Subject: [PATCH] Use JvmGlobalLocks in neutronmanager JvmGlobalLocks is giving us all we need to replace String.intern(), user that. JIRA: NETVIRT-1510 Change-Id: Ib97d513902f69ab158423dd6cd28bd43ba1e5c8e Signed-off-by: Robert Varga --- .../netvirt/neutronvpn/NeutronvpnManager.java | 394 +++++++++--------- .../netvirt/neutronvpn/NeutronvpnUtils.java | 11 +- 2 files changed, 216 insertions(+), 189 deletions(-) diff --git a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnManager.java b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnManager.java index dd956c40bc..cb943282bd 100644 --- a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnManager.java +++ b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnManager.java @@ -36,6 +36,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -54,6 +55,7 @@ import org.opendaylight.genius.infra.ManagedNewTransactionRunner; import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; import org.opendaylight.genius.infra.TypedWriteTransaction; import org.opendaylight.genius.mdsalutil.NwConstants; +import org.opendaylight.genius.utils.JvmGlobalLocks; import org.opendaylight.infrautils.jobcoordinator.JobCoordinator; import org.opendaylight.infrautils.utils.concurrent.KeyedLocks; import org.opendaylight.infrautils.utils.concurrent.ListenableFutures; @@ -269,7 +271,9 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even @Nullable NetworkAttributes.NetworkType networkType, long segmentationId) { try { InstanceIdentifier subnetMapIdentifier = NeutronvpnUtils.buildSubnetMapIdentifier(subnetId); - synchronized (subnetId.getValue().intern()) { + final ReentrantLock lock = lockForUuid(subnetId); + lock.lock(); + try { LOG.info("createSubnetmapNode: subnet ID {}", subnetId.toString()); Optional sn = SingleTransactionDataBroker.syncReadOptional(dataBroker, LogicalDatastoreType.CONFIGURATION, subnetMapIdentifier); @@ -285,6 +289,8 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even subnetId.getValue()); SingleTransactionDataBroker.syncWrite(dataBroker, LogicalDatastoreType.CONFIGURATION, subnetMapIdentifier, subnetmapBuilder.build()); + } finally { + lock.unlock(); } } catch (TransactionCommitFailedException | ReadFailedException e) { LOG.error("createSubnetmapNode: Creating subnetmap node failed for subnet {}", subnetId.getValue()); @@ -305,34 +311,35 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even InstanceIdentifier id = InstanceIdentifier.builder(Subnetmaps.class) .child(Subnetmap.class, new SubnetmapKey(subnetId)) .build(); + final ReentrantLock lock = lockForUuid(subnetId); + lock.lock(); try { - synchronized (subnetId.getValue().intern()) { - Optional sn = - SingleTransactionDataBroker.syncReadOptional(dataBroker, LogicalDatastoreType.CONFIGURATION, - id); - if (!sn.isPresent()) { - LOG.error("subnetmap node for subnet {} does not exist, returning", subnetId.getValue()); - return null; - } - LOG.debug("updating existing subnetmap node for subnet ID {}", subnetId.getValue()); - SubnetmapBuilder builder = new SubnetmapBuilder(sn.get()); - if (routerId != null) { - builder.setRouterId(routerId); - } - if (vpnId != null) { - builder.setVpnId(vpnId); - } - if (neutronvpnUtils.getIpVersionFromString(sn.get().getSubnetIp()) == IpVersionChoice.IPV6) { - builder.setInternetVpnId(internetvpnId); - } - Subnetmap subnetmap = builder.build(); - LOG.debug("Creating/Updating subnetMap node: {} ", subnetId.getValue()); - SingleTransactionDataBroker.syncWrite(dataBroker, LogicalDatastoreType.CONFIGURATION, id, subnetmap); - return subnetmap; + Optional sn = + SingleTransactionDataBroker.syncReadOptional(dataBroker, LogicalDatastoreType.CONFIGURATION, id); + if (!sn.isPresent()) { + LOG.error("subnetmap node for subnet {} does not exist, returning", subnetId.getValue()); + return null; + } + LOG.debug("updating existing subnetmap node for subnet ID {}", subnetId.getValue()); + SubnetmapBuilder builder = new SubnetmapBuilder(sn.get()); + if (routerId != null) { + builder.setRouterId(routerId); } + if (vpnId != null) { + builder.setVpnId(vpnId); + } + if (NeutronvpnUtils.getIpVersionFromString(sn.get().getSubnetIp()) == IpVersionChoice.IPV6) { + builder.setInternetVpnId(internetvpnId); + } + Subnetmap subnetmap = builder.build(); + LOG.debug("Creating/Updating subnetMap node: {} ", subnetId.getValue()); + SingleTransactionDataBroker.syncWrite(dataBroker, LogicalDatastoreType.CONFIGURATION, id, subnetmap); + return subnetmap; } catch (ReadFailedException | TransactionCommitFailedException e) { LOG.error("Subnet map update failed for node {}", subnetId.getValue(), e); return null; + } finally { + lock.unlock(); } } @@ -340,35 +347,36 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even @Nullable Uuid routerInterfacePortId, @Nullable String fixedIp, @Nullable String routerIntfMacAddress, @Nullable Uuid vpnId) { InstanceIdentifier id = - InstanceIdentifier.builder(Subnetmaps.class).child(Subnetmap.class, new SubnetmapKey(subnetId)).build(); + InstanceIdentifier.builder(Subnetmaps.class).child(Subnetmap.class, new SubnetmapKey(subnetId)).build(); + final ReentrantLock lock = lockForUuid(subnetId); + lock.lock(); try { - synchronized (subnetId.getValue().intern()) { - Optional sn = - SingleTransactionDataBroker.syncReadOptional(dataBroker, LogicalDatastoreType.CONFIGURATION, - id); - if (!sn.isPresent()) { - LOG.error("WithRouterFixedIP: subnetmap node for subnet {} does not exist, returning ", - subnetId.getValue()); - return; - } - LOG.debug("WithRouterFixedIP: Updating existing subnetmap node for subnet ID {}", + Optional sn = + SingleTransactionDataBroker.syncReadOptional(dataBroker, LogicalDatastoreType.CONFIGURATION, id); + if (!sn.isPresent()) { + LOG.error("WithRouterFixedIP: subnetmap node for subnet {} does not exist, returning ", subnetId.getValue()); - SubnetmapBuilder builder = new SubnetmapBuilder(sn.get()); - builder.setRouterId(routerId); - builder.setRouterInterfacePortId(routerInterfacePortId); - builder.setRouterIntfMacAddress(routerIntfMacAddress); - builder.setRouterInterfaceFixedIp(fixedIp); - if (vpnId != null) { - builder.setVpnId(vpnId); - } - Subnetmap subnetmap = builder.build(); - LOG.debug("WithRouterFixedIP Creating/Updating subnetMap node for Router FixedIp: {} ", - subnetId.getValue()); - SingleTransactionDataBroker.syncWrite(dataBroker, LogicalDatastoreType.CONFIGURATION, id, subnetmap); + return; + } + LOG.debug("WithRouterFixedIP: Updating existing subnetmap node for subnet ID {}", + subnetId.getValue()); + SubnetmapBuilder builder = new SubnetmapBuilder(sn.get()); + builder.setRouterId(routerId); + builder.setRouterInterfacePortId(routerInterfacePortId); + builder.setRouterIntfMacAddress(routerIntfMacAddress); + builder.setRouterInterfaceFixedIp(fixedIp); + if (vpnId != null) { + builder.setVpnId(vpnId); } + Subnetmap subnetmap = builder.build(); + LOG.debug("WithRouterFixedIP Creating/Updating subnetMap node for Router FixedIp: {} ", + subnetId.getValue()); + SingleTransactionDataBroker.syncWrite(dataBroker, LogicalDatastoreType.CONFIGURATION, id, subnetmap); } catch (ReadFailedException | TransactionCommitFailedException e) { LOG.error("WithRouterFixedIP: subnet map for Router FixedIp failed for node {}", subnetId.getValue(), e); + } finally { + lock.unlock(); } } @@ -378,44 +386,46 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even InstanceIdentifier id = InstanceIdentifier.builder(Subnetmaps.class).child(Subnetmap.class, new SubnetmapKey(subnetId)).build(); LOG.info("updateSubnetmapNodeWithPorts : subnetId {}, subnetMapId {}", subnetId.toString(), id.toString()); + final ReentrantLock lock = lockForUuid(subnetId); + lock.lock(); try { - synchronized (subnetId.getValue().intern()) { - Optional sn = - SingleTransactionDataBroker.syncReadOptional(dataBroker, LogicalDatastoreType.CONFIGURATION, - id); - if (sn.isPresent()) { - SubnetmapBuilder builder = new SubnetmapBuilder(sn.get()); - if (null != portId) { - List portList = builder.getPortList(); - if (null == portList) { - portList = new ArrayList<>(); - } - portList.add(portId); - builder.setPortList(portList); - LOG.debug("updateSubnetmapNodeWithPorts: Updating existing subnetmap node {} with port {}", - subnetId.getValue(), portId.getValue()); + Optional sn = + SingleTransactionDataBroker.syncReadOptional(dataBroker, LogicalDatastoreType.CONFIGURATION, + id); + if (sn.isPresent()) { + SubnetmapBuilder builder = new SubnetmapBuilder(sn.get()); + if (null != portId) { + List portList = builder.getPortList(); + if (null == portList) { + portList = new ArrayList<>(); } - if (null != directPortId) { - List directPortList = builder.getDirectPortList(); - if (null == directPortList) { - directPortList = new ArrayList<>(); - } - directPortList.add(directPortId); - builder.setDirectPortList(directPortList); - LOG.debug("Updating existing subnetmap node {} with port {}", subnetId.getValue(), - directPortId.getValue()); + portList.add(portId); + builder.setPortList(portList); + LOG.debug("updateSubnetmapNodeWithPorts: Updating existing subnetmap node {} with port {}", + subnetId.getValue(), portId.getValue()); + } + if (null != directPortId) { + List directPortList = builder.getDirectPortList(); + if (null == directPortList) { + directPortList = new ArrayList<>(); } - subnetmap = builder.build(); - SingleTransactionDataBroker.syncWrite(dataBroker, LogicalDatastoreType.CONFIGURATION, id, - subnetmap); - } else { - LOG.info("updateSubnetmapNodeWithPorts: Subnetmap node is not ready {}, put port {} in unprocessed " + directPortList.add(directPortId); + builder.setDirectPortList(directPortList); + LOG.debug("Updating existing subnetmap node {} with port {}", subnetId.getValue(), + directPortId.getValue()); + } + subnetmap = builder.build(); + SingleTransactionDataBroker.syncWrite(dataBroker, LogicalDatastoreType.CONFIGURATION, id, + subnetmap); + } else { + LOG.info("updateSubnetmapNodeWithPorts: Subnetmap node is not ready {}, put port {} in unprocessed " + "cache ", subnetId.getValue(), portId.getValue()); - unprocessedPortsMap.put(portId, subnetId); - } + unprocessedPortsMap.put(portId, subnetId); } } catch (ReadFailedException | TransactionCommitFailedException e) { LOG.error("Updating port list of a given subnetMap failed for node: {}", subnetId.getValue(), e); + } finally { + lock.unlock(); } return subnetmap; } @@ -426,39 +436,41 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even InstanceIdentifier id = InstanceIdentifier.builder(Subnetmaps.class) .child(Subnetmap.class, new SubnetmapKey(subnetId)) .build(); + final ReentrantLock lock = lockForUuid(subnetId); + lock.lock(); try { - synchronized (subnetId.getValue().intern()) { - Optional sn = - SingleTransactionDataBroker.syncReadOptional(dataBroker, LogicalDatastoreType.CONFIGURATION, - id); - if (sn.isPresent()) { - SubnetmapBuilder builder = new SubnetmapBuilder(sn.get()); - if (routerId != null) { - builder.setRouterId(null); - } - if (networkId != null) { - builder.setNetworkId(null); - } - if (vpnId != null) { - builder.setVpnId(null); - } - builder.setInternetVpnId(null); - if (portId != null && builder.getPortList() != null) { - List portList = builder.getPortList(); - portList.remove(portId); - builder.setPortList(portList); - } - - subnetmap = builder.build(); - LOG.debug("Removing from existing subnetmap node: {} ", subnetId.getValue()); - SingleTransactionDataBroker.syncWrite(dataBroker, LogicalDatastoreType.CONFIGURATION, id, - subnetmap); - } else { - LOG.warn("removing from non-existing subnetmap node: {} ", subnetId.getValue()); + Optional sn = + SingleTransactionDataBroker.syncReadOptional(dataBroker, LogicalDatastoreType.CONFIGURATION, + id); + if (sn.isPresent()) { + SubnetmapBuilder builder = new SubnetmapBuilder(sn.get()); + if (routerId != null) { + builder.setRouterId(null); + } + if (networkId != null) { + builder.setNetworkId(null); + } + if (vpnId != null) { + builder.setVpnId(null); + } + builder.setInternetVpnId(null); + if (portId != null && builder.getPortList() != null) { + List portList = builder.getPortList(); + portList.remove(portId); + builder.setPortList(portList); } + + subnetmap = builder.build(); + LOG.debug("Removing from existing subnetmap node: {} ", subnetId.getValue()); + SingleTransactionDataBroker.syncWrite(dataBroker, LogicalDatastoreType.CONFIGURATION, id, + subnetmap); + } else { + LOG.warn("removing from non-existing subnetmap node: {} ", subnetId.getValue()); } } catch (ReadFailedException | TransactionCommitFailedException e) { LOG.error("Removal from subnetmap failed for node: {}", subnetId.getValue()); + } finally { + lock.unlock(); } return subnetmap; } @@ -469,37 +481,39 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even Subnetmap subnetmap = null; InstanceIdentifier id = InstanceIdentifier.builder(Subnetmaps.class).child(Subnetmap.class, new SubnetmapKey(subnetId)).build(); + final ReentrantLock lock = lockForUuid(subnetId); + lock.lock(); try { - synchronized (subnetId.getValue().intern()) { - Optional sn = - SingleTransactionDataBroker.syncReadOptional(dataBroker, LogicalDatastoreType.CONFIGURATION, - id); - if (sn.isPresent()) { - SubnetmapBuilder builder = new SubnetmapBuilder(sn.get()); - if (null != portId && null != builder.getPortList()) { - List portList = builder.getPortList(); - portList.remove(portId); - builder.setPortList(portList); - LOG.debug("Removing port {} from existing subnetmap node: {} ", portId.getValue(), - subnetId.getValue()); - } - if (null != directPortId && null != builder.getDirectPortList()) { - List directPortList = builder.getDirectPortList(); - directPortList.remove(directPortId); - builder.setDirectPortList(directPortList); - LOG.debug("Removing direct port {} from existing subnetmap node: {} ", directPortId - .getValue(), subnetId.getValue()); - } - subnetmap = builder.build(); - SingleTransactionDataBroker.syncWrite(dataBroker, LogicalDatastoreType.CONFIGURATION, id, - subnetmap); - } else { - LOG.info("Trying to remove port from non-existing subnetmap node {}", subnetId.getValue()); + Optional sn = + SingleTransactionDataBroker.syncReadOptional(dataBroker, LogicalDatastoreType.CONFIGURATION, + id); + if (sn.isPresent()) { + SubnetmapBuilder builder = new SubnetmapBuilder(sn.get()); + if (null != portId && null != builder.getPortList()) { + List portList = builder.getPortList(); + portList.remove(portId); + builder.setPortList(portList); + LOG.debug("Removing port {} from existing subnetmap node: {} ", portId.getValue(), + subnetId.getValue()); + } + if (null != directPortId && null != builder.getDirectPortList()) { + List directPortList = builder.getDirectPortList(); + directPortList.remove(directPortId); + builder.setDirectPortList(directPortList); + LOG.debug("Removing direct port {} from existing subnetmap node: {} ", directPortId + .getValue(), subnetId.getValue()); } + subnetmap = builder.build(); + SingleTransactionDataBroker.syncWrite(dataBroker, LogicalDatastoreType.CONFIGURATION, id, + subnetmap); + } else { + LOG.info("Trying to remove port from non-existing subnetmap node {}", subnetId.getValue()); } } catch (ReadFailedException | TransactionCommitFailedException e) { LOG.error("Removing a port from port list of a subnetmap failed for node: {}", subnetId.getValue(), e); + } finally { + lock.unlock(); } return subnetmap; } @@ -510,13 +524,14 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even InstanceIdentifier subnetMapIdentifier = InstanceIdentifier.builder(Subnetmaps.class).child(Subnetmap.class, new SubnetmapKey(subnetId)).build(); LOG.debug("removing subnetMap node: {} ", subnetId.getValue()); + final ReentrantLock lock = lockForUuid(subnetId); + lock.lock(); try { - synchronized (subnetId.getValue().intern()) { - SingleTransactionDataBroker.syncDelete(dataBroker, LogicalDatastoreType.CONFIGURATION, - subnetMapIdentifier); - } + SingleTransactionDataBroker.syncDelete(dataBroker, LogicalDatastoreType.CONFIGURATION, subnetMapIdentifier); } catch (TransactionCommitFailedException e) { LOG.error("Delete subnetMap node failed for subnet : {} ", subnetId.getValue()); + } finally { + lock.unlock(); } } @@ -1855,59 +1870,61 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even } protected void addToNeutronRouterInterfacesMap(Uuid routerId, String interfaceName) { - synchronized (routerId.getValue().intern()) { - InstanceIdentifier routerInterfacesId = getRouterInterfacesId(routerId); - try { - Optional optRouterInterfaces = - SingleTransactionDataBroker.syncReadOptional(dataBroker, LogicalDatastoreType.CONFIGURATION, - routerInterfacesId); - Interfaces routerInterface = new InterfacesBuilder().withKey(new InterfacesKey(interfaceName)) + final InstanceIdentifier routerInterfacesId = getRouterInterfacesId(routerId); + final ReentrantLock lock = lockForUuid(routerId); + lock.lock(); + try { + Optional optRouterInterfaces = + SingleTransactionDataBroker.syncReadOptional(dataBroker, LogicalDatastoreType.CONFIGURATION, + routerInterfacesId); + Interfaces routerInterface = new InterfacesBuilder().withKey(new InterfacesKey(interfaceName)) .setInterfaceId(interfaceName).build(); - if (optRouterInterfaces.isPresent()) { - SingleTransactionDataBroker.syncWrite(dataBroker, LogicalDatastoreType.CONFIGURATION, - routerInterfacesId.child(Interfaces.class, new InterfacesKey(interfaceName)), - routerInterface); - } else { - // TODO Shouldn't we be doing something with builder and interfaces? -// RouterInterfacesBuilder builder = new RouterInterfacesBuilder().setRouterId(routerId); -// List interfaces = new ArrayList<>(); -// interfaces.add(routerInterface); + if (optRouterInterfaces.isPresent()) { + SingleTransactionDataBroker.syncWrite(dataBroker, LogicalDatastoreType.CONFIGURATION, + routerInterfacesId.child(Interfaces.class, new InterfacesKey(interfaceName)), routerInterface); + } else { + // TODO Shouldn't we be doing something with builder and interfaces? + // RouterInterfacesBuilder builder = new RouterInterfacesBuilder().setRouterId(routerId); + // List interfaces = new ArrayList<>(); + // interfaces.add(routerInterface); - SingleTransactionDataBroker.syncUpdate(dataBroker, LogicalDatastoreType.CONFIGURATION, - routerInterfacesId.child(Interfaces.class, new InterfacesKey(interfaceName)), - routerInterface); - } - } catch (ReadFailedException | TransactionCommitFailedException e) { - LOG.error("Error reading router interfaces for {}", routerInterfacesId, e); + SingleTransactionDataBroker.syncUpdate(dataBroker, LogicalDatastoreType.CONFIGURATION, + routerInterfacesId.child(Interfaces.class, new InterfacesKey(interfaceName)), routerInterface); } + } catch (ReadFailedException | TransactionCommitFailedException e) { + LOG.error("Error reading router interfaces for {}", routerInterfacesId, e); + } finally { + lock.unlock(); } } protected void removeFromNeutronRouterInterfacesMap(Uuid routerId, String interfaceName) { - synchronized (routerId.getValue().intern()) { - InstanceIdentifier routerInterfacesId = getRouterInterfacesId(routerId); - try { - Optional optRouterInterfaces = - SingleTransactionDataBroker.syncReadOptional(dataBroker, LogicalDatastoreType.CONFIGURATION, - routerInterfacesId); - Interfaces routerInterface = new InterfacesBuilder().withKey(new InterfacesKey(interfaceName)) + final InstanceIdentifier routerInterfacesId = getRouterInterfacesId(routerId); + final ReentrantLock lock = lockForUuid(routerId); + lock.lock(); + try { + Optional optRouterInterfaces = + SingleTransactionDataBroker.syncReadOptional(dataBroker, LogicalDatastoreType.CONFIGURATION, + routerInterfacesId); + Interfaces routerInterface = new InterfacesBuilder().withKey(new InterfacesKey(interfaceName)) .setInterfaceId(interfaceName).build(); - if (optRouterInterfaces.isPresent()) { - RouterInterfaces routerInterfaces = optRouterInterfaces.get(); - List interfaces = routerInterfaces.getInterfaces(); - if (interfaces != null && interfaces.remove(routerInterface)) { - if (interfaces.isEmpty()) { - SingleTransactionDataBroker.syncDelete(dataBroker, LogicalDatastoreType.CONFIGURATION, - routerInterfacesId); - } else { - SingleTransactionDataBroker.syncDelete(dataBroker, LogicalDatastoreType.CONFIGURATION, - routerInterfacesId.child(Interfaces.class, new InterfacesKey(interfaceName))); - } + if (optRouterInterfaces.isPresent()) { + RouterInterfaces routerInterfaces = optRouterInterfaces.get(); + List interfaces = routerInterfaces.getInterfaces(); + if (interfaces != null && interfaces.remove(routerInterface)) { + if (interfaces.isEmpty()) { + SingleTransactionDataBroker.syncDelete(dataBroker, LogicalDatastoreType.CONFIGURATION, + routerInterfacesId); + } else { + SingleTransactionDataBroker.syncDelete(dataBroker, LogicalDatastoreType.CONFIGURATION, + routerInterfacesId.child(Interfaces.class, new InterfacesKey(interfaceName))); } } - } catch (ReadFailedException | TransactionCommitFailedException e) { - LOG.error("Error reading the router interfaces for {}", routerInterfacesId, e); } + } catch (ReadFailedException | TransactionCommitFailedException e) { + LOG.error("Error reading the router interfaces for {}", routerInterfacesId, e); + } finally { + lock.unlock(); } } @@ -2336,7 +2353,7 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even List subMapList = neutronvpnUtils.getNeutronRouterSubnetMapList(routerId); IpVersionChoice ipVersion = IpVersionChoice.UNDEFINED; for (Subnetmap sn : subMapList) { - IpVersionChoice ipVers = neutronvpnUtils.getIpVersionFromString(sn.getSubnetIp()); + IpVersionChoice ipVers = NeutronvpnUtils.getIpVersionFromString(sn.getSubnetIp()); if (ipVersion.isIpVersionChosen(ipVers)) { ipVersion = ipVersion.addVersion(ipVers); } @@ -2498,7 +2515,7 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even LOG.error("associateExtNetworkToVpn: can not find subnet with Id {} in ConfigDS", snId.getValue()); continue; } - IpVersionChoice ipVers = neutronvpnUtils.getIpVersionFromString(sm.getSubnetIp()); + IpVersionChoice ipVers = NeutronvpnUtils.getIpVersionFromString(sm.getSubnetIp()); if (ipVers.isIpVersionChosen(IpVersionChoice.IPV4)) { continue; } @@ -2576,7 +2593,7 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even IpVersionChoice ipVersion = IpVersionChoice.UNDEFINED; for (Uuid subnet : networkSubnets) { Subnetmap subnetmap = neutronvpnUtils.getSubnetmap(subnet); - IpVersionChoice ipVers = neutronvpnUtils.getIpVersionFromString(subnetmap.getSubnetIp()); + IpVersionChoice ipVers = NeutronvpnUtils.getIpVersionFromString(subnetmap.getSubnetIp()); if (!ipVersion.isIpVersionChosen(ipVers)) { ipVersion = ipVersion.addVersion(ipVers); } @@ -2630,7 +2647,7 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even LOG.error("disassociateExtNetworkFromVpn: can not find subnet with Id {} in ConfigDS", snId.getValue()); continue; } - IpVersionChoice ipVers = neutronvpnUtils.getIpVersionFromString(sm.getSubnetIp()); + IpVersionChoice ipVers = NeutronvpnUtils.getIpVersionFromString(sm.getSubnetIp()); if (ipVers.isIpVersionChosen(IpVersionChoice.IPV4)) { continue; } @@ -3140,7 +3157,7 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even if (networkUuid != null) { Network portNetwork = neutronvpnUtils.getNeutronNetwork(networkUuid); ProviderTypes providerType = NeutronvpnUtils.getProviderNetworkType(portNetwork); - NetworkAttributes.NetworkType networkType = (providerType != null) + NetworkAttributes.NetworkType networkType = providerType != null ? NetworkAttributes.NetworkType.valueOf(providerType.getName()) : null; String segmentationId = NeutronvpnUtils.getSegmentationIdFromNeutronNetwork(portNetwork); vpnb.setNetworkId(networkUuid).setNetworkType(networkType) @@ -3326,19 +3343,19 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even return existingVpnName; } - private String formatAndLog(Consumer logger, String template, Object arg) { + private static String formatAndLog(Consumer logger, String template, Object arg) { return logAndReturnMessage(logger, MessageFormatter.format(template, arg)); } - private String formatAndLog(Consumer logger, String template, Object arg1, Object arg2) { + private static String formatAndLog(Consumer logger, String template, Object arg1, Object arg2) { return logAndReturnMessage(logger, MessageFormatter.format(template, arg1, arg2)); } - private String formatAndLog(Consumer logger, String template, Object... args) { + private static String formatAndLog(Consumer logger, String template, Object... args) { return logAndReturnMessage(logger, MessageFormatter.arrayFormat(template, args)); } - private String logAndReturnMessage(Consumer logger, FormattingTuple tuple) { + private static String logAndReturnMessage(Consumer logger, FormattingTuple tuple) { String message = tuple.getMessage(); logger.accept(message); return message; @@ -3370,4 +3387,9 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even ? true : false); } } + + private static ReentrantLock lockForUuid(Uuid uuid) { + // FIXME: prove that this locks only on Uuids and not some other entity or create a separate lock domain + return JvmGlobalLocks.getLockForString(uuid.getValue()); + } } diff --git a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java index 498a14f57a..3dbf3856a9 100644 --- a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java +++ b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java @@ -38,6 +38,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; +import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -56,6 +57,7 @@ import org.opendaylight.genius.infra.ManagedNewTransactionRunner; import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; import org.opendaylight.genius.infra.TypedWriteTransaction; import org.opendaylight.genius.mdsalutil.MDSALUtil; +import org.opendaylight.genius.utils.JvmGlobalLocks; import org.opendaylight.infrautils.jobcoordinator.JobCoordinator; import org.opendaylight.infrautils.utils.concurrent.ListenableFutures; import org.opendaylight.netvirt.neutronvpn.api.enums.IpVersionChoice; @@ -919,15 +921,18 @@ public class NeutronvpnUtils { @SuppressWarnings("checkstyle:IllegalCatch") protected void removeLearntVpnVipToPort(String vpnName, String fixedIp) { InstanceIdentifier id = NeutronvpnUtils.buildLearntVpnVipToPortIdentifier(vpnName, fixedIp); + // FIXME: can we use 'id' as the lock name? + final ReentrantLock lock = JvmGlobalLocks.getLockForString(vpnName + fixedIp); + lock.lock(); try { - synchronized ((vpnName + fixedIp).intern()) { - MDSALUtil.syncDelete(dataBroker, LogicalDatastoreType.OPERATIONAL, id); - } + MDSALUtil.syncDelete(dataBroker, LogicalDatastoreType.OPERATIONAL, id); LOG.trace("Neutron router port with fixedIp: {}, vpn {} removed from LearntVpnPortipToPort DS", fixedIp, vpnName); } catch (Exception e) { LOG.error("Failure while removing LearntVpnPortFixedIpToPort map for vpn {} - fixedIP {}", vpnName, fixedIp, e); + } finally { + lock.unlock(); } } -- 2.36.6