From 2185f116439ee580a04445504e3166f912e1c55f Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Tue, 16 Nov 2021 08:18:44 +0100 Subject: [PATCH] Cleanup PCEPTopologyDeployerImpl General cleanup of the logic flow in the class, documenting locking rules. Change-Id: Ife8eee1eb7190a2f8355c4a909ba2030cf1162ad JIRA: BGPCEP-983 Signed-off-by: Robert Varga --- .../config/PCEPTopologyDeployerImpl.java | 69 +++++++++++-------- .../config/PCEPTopologyProviderUtil.java | 13 ---- 2 files changed, 39 insertions(+), 43 deletions(-) diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyDeployerImpl.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyDeployerImpl.java index 98f4124065..cb3371a7cb 100644 --- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyDeployerImpl.java +++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyDeployerImpl.java @@ -8,28 +8,28 @@ package org.opendaylight.bgpcep.pcep.topology.provider.config; import static java.util.Objects.requireNonNull; -import static org.opendaylight.bgpcep.pcep.topology.provider.config.PCEPTopologyProviderUtil.filterPcepTopologies; -import static org.opendaylight.mdsal.common.api.LogicalDatastoreType.CONFIGURATION; import java.util.Collection; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; import org.checkerframework.checker.lock.qual.GuardedBy; +import org.checkerframework.checker.lock.qual.Holding; +import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.bgpcep.programming.spi.InstructionScheduler; import org.opendaylight.bgpcep.programming.spi.InstructionSchedulerFactory; import org.opendaylight.mdsal.binding.api.ClusteredDataTreeChangeListener; import org.opendaylight.mdsal.binding.api.DataBroker; -import org.opendaylight.mdsal.binding.api.DataObjectModification; import org.opendaylight.mdsal.binding.api.DataTreeIdentifier; import org.opendaylight.mdsal.binding.api.DataTreeModification; +import org.opendaylight.mdsal.common.api.LogicalDatastoreType; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.config.rev200120.pcep.config.SessionConfig; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.config.rev181109.PcepTopologyTypeConfig; +import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.rev200120.TopologyTypes1; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NetworkTopology; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.TopologyId; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.Topology; +import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.TopologyTypes; import org.opendaylight.yangtools.concepts.ListenerRegistration; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.osgi.service.blueprint.container.BlueprintContainer; @@ -39,10 +39,9 @@ import org.slf4j.LoggerFactory; public class PCEPTopologyDeployerImpl implements ClusteredDataTreeChangeListener, AutoCloseable { private static final Logger LOG = LoggerFactory.getLogger(PCEPTopologyDeployerImpl.class); - private static final long TIMEOUT_NS = TimeUnit.SECONDS.toNanos(5); + private final BlueprintContainer container; - private final InstanceIdentifier networTopology; private final DataBroker dataBroker; private final InstructionSchedulerFactory instructionSchedulerFactory; @GuardedBy("this") @@ -54,51 +53,53 @@ public class PCEPTopologyDeployerImpl implements ClusteredDataTreeChangeListener this.container = requireNonNull(container); this.dataBroker = requireNonNull(dataBroker); this.instructionSchedulerFactory = requireNonNull(instructionSchedulerFactory); - this.networTopology = InstanceIdentifier.builder(NetworkTopology.class).build(); } public synchronized void init() { LOG.info("PCEP Topology Deployer initialized"); - this.listenerRegistration = this.dataBroker.registerDataTreeChangeListener( - DataTreeIdentifier.create(CONFIGURATION, this.networTopology.child(Topology.class)), this); + listenerRegistration = dataBroker.registerDataTreeChangeListener( + DataTreeIdentifier.create(LogicalDatastoreType.CONFIGURATION, + InstanceIdentifier.builder(NetworkTopology.class).child(Topology.class).build()), this); } @Override public synchronized void onDataTreeChanged(final Collection> changes) { - final List> topoChanges = changes.stream() - .map(DataTreeModification::getRootNode) - .collect(Collectors.toList()); - topoChanges.stream().iterator().forEachRemaining(topo -> { + for (var change : changes) { + final var topo = change.getRootNode(); switch (topo.getModificationType()) { case SUBTREE_MODIFIED: case WRITE: + // FIXME: BGPCEP-983: propagate all modifications updateTopologyProvider(topo.getDataAfter()); break; case DELETE: removeTopologyProvider(topo.getDataBefore()); break; default: + throw new IllegalStateException("Unhandled modification type " + topo.getModificationType()); } - }); + } } - private synchronized void updateTopologyProvider(final Topology topology) { + @Holding("this") + private void updateTopologyProvider(final Topology topology) { if (!filterPcepTopologies(topology.getTopologyTypes())) { return; } final TopologyId topologyId = topology.getTopologyId(); LOG.info("Updating Topology {}", topologyId); - final PCEPTopologyProviderBean previous = this.pcepTopologyServices.remove(topology.getTopologyId()); + final PCEPTopologyProviderBean previous = pcepTopologyServices.remove(topologyId); closeTopology(previous, topologyId); createTopologyProvider(topology); } - private synchronized void createTopologyProvider(final Topology topology) { + @Holding("this") + private void createTopologyProvider(final Topology topology) { if (!filterPcepTopologies(topology.getTopologyTypes())) { return; } final TopologyId topologyId = topology.getTopologyId(); - if (this.pcepTopologyServices.containsKey(topology.getTopologyId())) { + if (pcepTopologyServices.containsKey(topologyId)) { LOG.warn("Topology Provider {} already exist. New instance won't be created", topologyId); return; } @@ -106,42 +107,42 @@ public class PCEPTopologyDeployerImpl implements ClusteredDataTreeChangeListener LOG.trace("Topology {}.", topology); final SessionConfig config = topology.augmentation(PcepTopologyTypeConfig.class).getSessionConfig(); - final InstructionScheduler instructionScheduler = this.instructionSchedulerFactory + final InstructionScheduler instructionScheduler = instructionSchedulerFactory .createInstructionScheduler(topologyId.getValue()); final PCEPTopologyConfiguration dependencies = new PCEPTopologyConfiguration(config, topology); - final PCEPTopologyProviderBean pcepTopologyProviderBean = (PCEPTopologyProviderBean) this.container + final PCEPTopologyProviderBean pcepTopologyProviderBean = (PCEPTopologyProviderBean) container .getComponentInstance(PCEPTopologyProviderBean.class.getSimpleName()); - this.pcepTopologyServices.put(topologyId, pcepTopologyProviderBean); + pcepTopologyServices.put(topologyId, pcepTopologyProviderBean); pcepTopologyProviderBean.start(dependencies, instructionScheduler); } + @Holding("this") private synchronized void removeTopologyProvider(final Topology topology) { if (!filterPcepTopologies(topology.getTopologyTypes())) { return; } final TopologyId topologyId = topology.getTopologyId(); - closeTopology(this.pcepTopologyServices.remove(topologyId), topologyId); + closeTopology(pcepTopologyServices.remove(topologyId), topologyId); } @Override public synchronized void close() { LOG.info("PCEP Topology Deployer closing"); - if (this.listenerRegistration != null) { - this.listenerRegistration.close(); - this.listenerRegistration = null; + if (listenerRegistration != null) { + listenerRegistration.close(); + listenerRegistration = null; } - for (Map.Entry entry:this.pcepTopologyServices.entrySet()) { + for (Map.Entry entry : pcepTopologyServices.entrySet()) { closeTopology(entry.getValue(), entry.getKey()); } - this.pcepTopologyServices.clear(); + pcepTopologyServices.clear(); LOG.info("PCEP Topology Deployer closed"); } - @SuppressWarnings("checkstyle:IllegalCatch") - private synchronized void closeTopology(final PCEPTopologyProviderBean topology, final TopologyId topologyId) { + private static void closeTopology(final PCEPTopologyProviderBean topology, final TopologyId topologyId) { if (topology == null) { return; } @@ -153,4 +154,12 @@ public class PCEPTopologyDeployerImpl implements ClusteredDataTreeChangeListener LOG.error("Topology {} instance failed to close service instance", topologyId, e); } } + + private static boolean filterPcepTopologies(final @Nullable TopologyTypes topologyTypes) { + if (topologyTypes == null) { + return false; + } + final TopologyTypes1 aug = topologyTypes.augmentation(TopologyTypes1.class); + return aug != null && aug.getTopologyPcep() != null; + } } \ No newline at end of file diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderUtil.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderUtil.java index 4ea817b97b..f29cbfd358 100644 --- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderUtil.java +++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderUtil.java @@ -13,7 +13,6 @@ import java.net.InetSocketAddress; import java.nio.charset.StandardCharsets; import java.util.Objects; import org.eclipse.jdt.annotation.NonNull; -import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.protocol.concepts.KeyMapping; import org.opendaylight.protocol.pcep.SpeakerIdMapping; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IetfInetUtil; @@ -21,10 +20,8 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types. import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.PortNumber; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.rfc2385.cfg.rev160324.Rfc2385Key; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.config.rev181109.PcepNodeConfig; -import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.rev200120.TopologyTypes1; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.sync.optimizations.config.rev181109.PcepNodeSyncConfig; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.Topology; -import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.TopologyTypes; final class PCEPTopologyProviderUtil { private PCEPTopologyProviderUtil() { @@ -59,16 +56,6 @@ final class PCEPTopologyProviderUtil { return new InetSocketAddress(IetfInetUtil.INSTANCE.inetAddressForNoZone(address), port.getValue().toJava()); } - static boolean filterPcepTopologies(final @Nullable TopologyTypes topologyTypes) { - if (topologyTypes == null) { - return false; - } - final TopologyTypes1 aug = topologyTypes.augmentation(TopologyTypes1.class); - - return aug != null && aug.getTopologyPcep() != null; - } - - static SpeakerIdMapping contructSpeakersId(final Topology topology) { final SpeakerIdMapping ret = SpeakerIdMapping.getSpeakerIdMap(); if (topology.getNode() == null) { -- 2.36.6