From: Kevin Wang Date: Mon, 24 Apr 2017 20:31:12 +0000 (-0700) Subject: BUG-8156 Fix PCEP topology registration X-Git-Tag: release/boron-sr4~3 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=2801f2b71ca49dcee50b643ef5ed4f53938609fc;p=bgpcep.git BUG-8156 Fix PCEP topology registration It is possible that when a PCEP session is being dropped and being unregistered from ServerSessionManager, the thread gets switched thus the topology unregistering cannot finish. If such situation happens, PCEP session from the same PCEP peer will not be able to established until after DEADTIMER_EXPIRED, as the topology instance was not removed correctly. Change-Id: I647ecfbbb35ef6805d563753b7ebd87edfc350fe Signed-off-by: Kevin Wang --- diff --git a/pcep/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java b/pcep/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java index b44bdf7748..06b8910e6c 100755 --- a/pcep/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java +++ b/pcep/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java @@ -26,6 +26,7 @@ import java.util.concurrent.TimeUnit; import javax.annotation.concurrent.GuardedBy; import org.opendaylight.controller.config.yang.pcep.topology.provider.ListenerStateRuntimeMXBean; import org.opendaylight.controller.config.yang.pcep.topology.provider.ListenerStateRuntimeRegistration; +import org.opendaylight.controller.config.yang.pcep.topology.provider.PCEPTopologyProviderRuntimeRegistration; import org.opendaylight.controller.config.yang.pcep.topology.provider.PeerCapabilities; import org.opendaylight.controller.config.yang.pcep.topology.provider.ReplyTime; import org.opendaylight.controller.config.yang.pcep.topology.provider.SessionState; @@ -125,6 +126,7 @@ public abstract class AbstractTopologySessionListener implements PCEPSessi private boolean triggeredResyncInProcess; private ListenerStateRuntimeRegistration registration; + @GuardedBy("this") private final SessionListenerState listenerState; protected AbstractTopologySessionListener(final ServerSessionManager serverSessionManager) { @@ -147,11 +149,13 @@ public abstract class AbstractTopologySessionListener implements PCEPSessi final TopologyNodeState state = this.serverSessionManager.takeNodeState(peerAddress, this, isLspDbRetreived()); // takeNodeState(..) may fail when the server session manager is being restarted due to configuration change if (state == null) { + LOG.error("Unable to fetch topology node state for PCEP session. Closing session {}", session); this.onSessionDown(session, new RuntimeException("Unable to fetch topology node state for PCEP session with " + session.getRemoteAddress())); return; } if (this.session != null || this.nodeState != null) { + LOG.error("PCEP session is already up. Closing session {}", session); this.onSessionDown(session, new IllegalStateException("Session is already up with " + session.getRemoteAddress())); return; } @@ -177,12 +181,13 @@ public abstract class AbstractTopologySessionListener implements PCEPSessi pccBuilder.setReportedLsp(initialNodeState.getAugmentation(Node1.class).getPathComputationClient().getReportedLsp()); } writeNode(pccBuilder, state, topologyAugment); - this.listenerState.init(session); - this.registration = this.serverSessionManager.registerRuntimeRootRegistration(this); + register(); if (this.registration == null) { + LOG.error("PCEP session fails to register. Closing session {}", session); this.onSessionDown(session, new RuntimeException("PCEP Session with " + session.getRemoteAddress() + " fails to register.")); return; } + this.listenerState.init(session); LOG.info("Session with {} attached to topology node {}", session.getRemoteAddress(), state.getNodeId()); } @@ -288,10 +293,14 @@ public abstract class AbstractTopologySessionListener implements PCEPSessi @Override public final synchronized void onMessage(final PCEPSession session, final Message message) { + if (this.nodeState == null) { + LOG.warn("Topology node state is null. Unhandled message {} on session {}", message, session); + return; + } final MessageContext ctx = new MessageContext(this.nodeState.beginTransaction()); if (onMessage(ctx, message)) { - LOG.info("Unhandled message {} on session {}", message, session); + LOG.warn("Unhandled message {} on session {}", message, session); //cancel not supported, submit empty transaction ctx.trans.submit(); return; @@ -321,16 +330,25 @@ public abstract class AbstractTopologySessionListener implements PCEPSessi } } - private synchronized void unregister() { + private final synchronized void unregister() { if (this.registration != null) { this.registration.close(); - LOG.trace("PCEP session {} unregistered successfully.", this.session); + LOG.trace("PCEP session {} is unregistered successfully.", this.session); this.registration = null; } else { LOG.trace("PCEP session {} was not registered.", this.session); } } + private final synchronized void register() { + Preconditions.checkState(this.registration == null); + final PCEPTopologyProviderRuntimeRegistration runtimeReg = this.serverSessionManager.getRuntimeRootRegistration(); + if (runtimeReg != null) { + this.registration = runtimeReg.register(this); + LOG.trace("PCEP session {} is successfully registered.", this.session); + } + } + protected final synchronized PCEPRequest removeRequest(final S id) { final PCEPRequest ret = this.requests.remove(id); if (ret != null) { @@ -610,7 +628,7 @@ public abstract class AbstractTopologySessionListener implements PCEPSessi return false; } - protected SessionListenerState getSessionListenerState() { + protected synchronized SessionListenerState getSessionListenerState() { return this.listenerState; } @@ -635,12 +653,12 @@ public abstract class AbstractTopologySessionListener implements PCEPSessi } @Override - public ReplyTime getReplyTime() { + public synchronized ReplyTime getReplyTime() { return this.listenerState.getReplyTime(); } @Override - public PeerCapabilities getPeerCapabilities() { + public synchronized PeerCapabilities getPeerCapabilities() { return this.listenerState.getPeerCapabilities(); } diff --git a/pcep/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java b/pcep/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java index 34d109907c..228e33f9f8 100755 --- a/pcep/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java +++ b/pcep/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java @@ -21,8 +21,6 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import javax.annotation.concurrent.GuardedBy; -import org.opendaylight.controller.config.yang.pcep.topology.provider.ListenerStateRuntimeMXBean; -import org.opendaylight.controller.config.yang.pcep.topology.provider.ListenerStateRuntimeRegistration; import org.opendaylight.controller.config.yang.pcep.topology.provider.PCEPTopologyProviderRuntimeMXBean; import org.opendaylight.controller.config.yang.pcep.topology.provider.PCEPTopologyProviderRuntimeRegistration; import org.opendaylight.controller.config.yang.pcep.topology.provider.PCEPTopologyProviderRuntimeRegistrator; @@ -232,17 +230,10 @@ final class ServerSessionManager implements PCEPSessionListenerFactory, Topology } } - ListenerStateRuntimeRegistration registerRuntimeRootRegistration(final ListenerStateRuntimeMXBean bean) { - final PCEPTopologyProviderRuntimeRegistration runtimeReg = this.runtimeRootRegistration.get(); - if (runtimeReg != null) { - final ListenerStateRuntimeRegistration reg = runtimeReg.register(bean); - LOG.trace("Bean {} is successfully registered.", bean.getPeerId()); - return reg; - } - return null; + PCEPTopologyProviderRuntimeRegistration getRuntimeRootRegistration() { + return this.runtimeRootRegistration.get(); } - @Override public void setPeerSpecificProposal(final InetSocketAddress address, final TlvsBuilder openBuilder) { Preconditions.checkNotNull(address); this.peerProposal.setPeerProposal(createNodeId(address.getAddress()), openBuilder);