From 6bc43b8bdb3a57210a192b0173891be064170c41 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Fri, 6 Jun 2014 19:20:03 +0200 Subject: [PATCH] Minimize synchronized blocks This is a simple, obviously-correct improvement of scalability: we reduce the sice of the synchronized sections such that they only cover the datastore interaction. Change-Id: I1c78804e6aed22aac73f67e9c5de43ff465c7836 Signed-off-by: Robert Varga --- .../manager/FlowCapableTopologyExporter.java | 112 ++++++++++-------- 1 file changed, 60 insertions(+), 52 deletions(-) diff --git a/opendaylight/md-sal/topology-manager/src/main/java/org/opendaylight/md/controller/topology/manager/FlowCapableTopologyExporter.java b/opendaylight/md-sal/topology-manager/src/main/java/org/opendaylight/md/controller/topology/manager/FlowCapableTopologyExporter.java index 542e972deb..ae173a7117 100644 --- a/opendaylight/md-sal/topology-manager/src/main/java/org/opendaylight/md/controller/topology/manager/FlowCapableTopologyExporter.java +++ b/opendaylight/md-sal/topology-manager/src/main/java/org/opendaylight/md/controller/topology/manager/FlowCapableTopologyExporter.java @@ -57,13 +57,9 @@ import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.JdkFutureAdapters; -class FlowCapableTopologyExporter implements // - FlowTopologyDiscoveryListener, // - OpendaylightInventoryListener // -{ - +class FlowCapableTopologyExporter implements FlowTopologyDiscoveryListener, OpendaylightInventoryListener { protected final static Logger LOG = LoggerFactory.getLogger(FlowCapableTopologyExporter.class); - public static TopologyKey topology = new TopologyKey(new TopologyId("flow:1")); + public static final TopologyKey TOPOLOGY = new TopologyKey(new TopologyId("flow:1")); // FIXME: Flow capable topology exporter should use transaction chaining API private DataProviderService dataService; @@ -80,8 +76,8 @@ class FlowCapableTopologyExporter implements // public void start() { TopologyBuilder tb = new TopologyBuilder(); - tb.setKey(topology); - topologyPath = InstanceIdentifier.builder(NetworkTopology.class).child(Topology.class, topology).build(); + tb.setKey(TOPOLOGY); + topologyPath = InstanceIdentifier.builder(NetworkTopology.class).child(Topology.class, TOPOLOGY).build(); Topology top = tb.build(); DataModificationTransaction tx = dataService.beginTransaction(); tx.putOperationalData(topologyPath, top); @@ -89,42 +85,49 @@ class FlowCapableTopologyExporter implements // } @Override - public synchronized void onNodeRemoved(final NodeRemoved notification) { + public void onNodeRemoved(final NodeRemoved notification) { NodeId nodeId = toTopologyNodeId(getNodeKey(notification.getNodeRef()).getId()); InstanceIdentifier nodeInstance = toNodeIdentifier(notification.getNodeRef()); - DataModificationTransaction tx = dataService.beginTransaction(); - tx.removeOperationalData(nodeInstance); - removeAffectedLinks(tx, nodeId); - listenOnTransactionState(tx.getIdentifier(),tx.commit()); + synchronized (this) { + DataModificationTransaction tx = dataService.beginTransaction(); + tx.removeOperationalData(nodeInstance); + removeAffectedLinks(tx, nodeId); + listenOnTransactionState(tx.getIdentifier(),tx.commit()); + } } @Override - public synchronized void onNodeUpdated(final NodeUpdated notification) { + public void onNodeUpdated(final NodeUpdated notification) { FlowCapableNodeUpdated fcnu = notification.getAugmentation(FlowCapableNodeUpdated.class); if (fcnu != null) { Node node = toTopologyNode(toTopologyNodeId(notification.getId()), notification.getNodeRef()); InstanceIdentifier path = getNodePath(toTopologyNodeId(notification.getId())); - DataModificationTransaction tx = dataService.beginTransaction(); - tx.putOperationalData(path, node); - listenOnTransactionState(tx.getIdentifier(),tx.commit()); + + synchronized (this) { + DataModificationTransaction tx = dataService.beginTransaction(); + tx.putOperationalData(path, node); + listenOnTransactionState(tx.getIdentifier(),tx.commit()); + } } } @Override - public synchronized void onNodeConnectorRemoved(final NodeConnectorRemoved notification) { + public void onNodeConnectorRemoved(final NodeConnectorRemoved notification) { InstanceIdentifier tpInstance = toTerminationPointIdentifier(notification .getNodeConnectorRef()); TpId tpId = toTerminationPointId(getNodeConnectorKey(notification.getNodeConnectorRef()).getId()); - DataModificationTransaction tx = dataService.beginTransaction(); - tx.removeOperationalData(tpInstance); - removeAffectedLinks(tx, tpId); - listenOnTransactionState(tx.getIdentifier(),tx.commit()); + synchronized (this) { + DataModificationTransaction tx = dataService.beginTransaction(); + tx.removeOperationalData(tpInstance); + removeAffectedLinks(tx, tpId); + listenOnTransactionState(tx.getIdentifier(),tx.commit()); + } } @Override - public synchronized void onNodeConnectorUpdated(final NodeConnectorUpdated notification) { + public void onNodeConnectorUpdated(final NodeConnectorUpdated notification) { FlowCapableNodeConnectorUpdated fcncu = notification.getAugmentation(FlowCapableNodeConnectorUpdated.class); if (fcncu != null) { NodeId nodeId = toTopologyNodeId(getNodeKey(notification.getNodeConnectorRef()).getId()); @@ -132,41 +135,48 @@ class FlowCapableTopologyExporter implements // notification.getNodeConnectorRef()); InstanceIdentifier path = tpPath(nodeId, point.getKey().getTpId()); - DataModificationTransaction tx = dataService.beginTransaction(); - tx.putOperationalData(path, point); - if ((fcncu.getState() != null && fcncu.getState().isLinkDown()) - || (fcncu.getConfiguration() != null && fcncu.getConfiguration().isPORTDOWN())) { - removeAffectedLinks(tx, point.getTpId()); + synchronized (this) { + DataModificationTransaction tx = dataService.beginTransaction(); + tx.putOperationalData(path, point); + if ((fcncu.getState() != null && fcncu.getState().isLinkDown()) + || (fcncu.getConfiguration() != null && fcncu.getConfiguration().isPORTDOWN())) { + removeAffectedLinks(tx, point.getTpId()); + } + listenOnTransactionState(tx.getIdentifier(),tx.commit()); } - listenOnTransactionState(tx.getIdentifier(),tx.commit()); } } @Override - public synchronized void onLinkDiscovered(final LinkDiscovered notification) { + public void onLinkDiscovered(final LinkDiscovered notification) { Link link = toTopologyLink(notification); InstanceIdentifier path = linkPath(link); - DataModificationTransaction tx = dataService.beginTransaction(); - tx.putOperationalData(path, link); - listenOnTransactionState(tx.getIdentifier(),tx.commit()); + synchronized (this) { + DataModificationTransaction tx = dataService.beginTransaction(); + tx.putOperationalData(path, link); + listenOnTransactionState(tx.getIdentifier(),tx.commit()); + } } @Override - public synchronized void onLinkOverutilized(final LinkOverutilized notification) { + public void onLinkOverutilized(final LinkOverutilized notification) { // NOOP } @Override - public synchronized void onLinkRemoved(final LinkRemoved notification) { + public void onLinkRemoved(final LinkRemoved notification) { InstanceIdentifier path = linkPath(toTopologyLink(notification)); - DataModificationTransaction tx = dataService.beginTransaction(); - tx.removeOperationalData(path); - listenOnTransactionState(tx.getIdentifier(),tx.commit()); + + synchronized (this) { + DataModificationTransaction tx = dataService.beginTransaction(); + tx.removeOperationalData(path); + listenOnTransactionState(tx.getIdentifier(),tx.commit()); + } } @Override - public synchronized void onLinkUtilizationNormal(final LinkUtilizationNormal notification) { + public void onLinkUtilizationNormal(final LinkUtilizationNormal notification) { // NOOP } @@ -174,11 +184,11 @@ class FlowCapableTopologyExporter implements // org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.NodeKey invNodeKey = getNodeKey(ref); NodeKey nodeKey = new NodeKey(toTopologyNodeId(invNodeKey.getId())); - return InstanceIdentifier.builder(NetworkTopology.class).child(Topology.class, topology) + return InstanceIdentifier.builder(NetworkTopology.class).child(Topology.class, TOPOLOGY) .child(Node.class, nodeKey).build(); } - private InstanceIdentifier toTerminationPointIdentifier(final NodeConnectorRef ref) { + private static InstanceIdentifier toTerminationPointIdentifier(final NodeConnectorRef ref) { org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.NodeKey invNodeKey = getNodeKey(ref); NodeConnectorKey invNodeConnectorKey = getNodeConnectorKey(ref); return tpPath(toTopologyNodeId(invNodeKey.getId()), toTerminationPointId(invNodeConnectorKey.getId())); @@ -213,22 +223,22 @@ class FlowCapableTopologyExporter implements // } } - private InstanceIdentifier getNodePath(final NodeId nodeId) { + private static InstanceIdentifier getNodePath(final NodeId nodeId) { NodeKey nodeKey = new NodeKey(nodeId); - return InstanceIdentifier.builder(NetworkTopology.class).child(Topology.class, topology) + return InstanceIdentifier.builder(NetworkTopology.class).child(Topology.class, TOPOLOGY) .child(Node.class, nodeKey).build(); } - private InstanceIdentifier tpPath(final NodeId nodeId, final TpId tpId) { + private static InstanceIdentifier tpPath(final NodeId nodeId, final TpId tpId) { NodeKey nodeKey = new NodeKey(nodeId); TerminationPointKey tpKey = new TerminationPointKey(tpId); - return InstanceIdentifier.builder(NetworkTopology.class).child(Topology.class, topology) + return InstanceIdentifier.builder(NetworkTopology.class).child(Topology.class, TOPOLOGY) .child(Node.class, nodeKey).child(TerminationPoint.class, tpKey).build(); } - private InstanceIdentifier linkPath(final Link link) { + private static InstanceIdentifier linkPath(final Link link) { InstanceIdentifier linkInstanceId = InstanceIdentifier.builder(NetworkTopology.class) - .child(Topology.class, topology).child(Link.class, link.getKey()).build(); + .child(Topology.class, TOPOLOGY).child(Link.class, link.getKey()).build(); return linkInstanceId; } @@ -236,17 +246,15 @@ class FlowCapableTopologyExporter implements // * @param txId transaction identificator * @param future transaction result */ - private static void listenOnTransactionState(final Object txId, Future> future) { + private static void listenOnTransactionState(final Object txId, final Future> future) { Futures.addCallback(JdkFutureAdapters.listenInPoolThread(future),new FutureCallback>() { - @Override - public void onFailure(Throwable t) { + public void onFailure(final Throwable t) { LOG.error("Topology export failed for Tx:{}", txId, t); - } @Override - public void onSuccess(RpcResult result) { + public void onSuccess(final RpcResult result) { if(!result.isSuccessful()) { LOG.error("Topology export failed for Tx:{}", txId); } -- 2.36.6