BUG-6437: Fix race condition when closing singleton group I

Fix
- sort when singleton instances are registered.
- remove configuration when App module close

Change-Id: I5547acb36285c56793564451fcb678ed4e047414
Signed-off-by: Claudio D. Gasparini <cgaspari@cisco.com>
(cherry picked from commit d981514fabdac8fd4a056e239d732ea6556e5ed4)

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/controller/config/yang/bgp/rib/impl/BGPApplicationPeerModule.java b/bgp/rib-impl/src/main/java/org/opendaylight/controller/config/yang/bgp/rib/impl/BGPApplicationPeerModule.java
index 13bc8b99d68f1f690f7f827f750d0d14653304f6..4adfbd4acba1f7f7c53d7840124368d4b49f3277 100755 (executable)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/controller/config/yang/bgp/rib/impl/BGPApplicationPeerModule.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/controller/config/yang/bgp/rib/impl/BGPApplicationPeerModule.java
@@ -59,7 +59,10 @@ public class BGPApplicationPeerModule extends org.opendaylight.controller.config
             .child(Neighbors.class).child(Neighbor.class, neighbor.getKey());
         bgpDeployer.onNeighborModified(bgpIID, neighbor, () -> bgpDeployer.writeConfiguration(neighbor, neighborIId));
 
-        return bgpDeployerTracker::close;
+        return () -> {
+            bgpDeployer.onNeighborRemoved(bgpIID, neighbor);
+            bgpDeployerTracker.close();
+        };
     }
 
     public void setBundleContext(final BundleContext bundleContext) {

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/RIBImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/RIBImpl.java
index cb8def72e9aa3dd6ab632ddd45837f8d3c704abb..dab7ea8768c407ff85c75b62011305e13a634b1d 100755 (executable)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/RIBImpl.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/RIBImpl.java
@@ -23,8 +23,6 @@ import java.util.Set;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.ThreadSafe;
-import org.opendaylight.controller.config.yang.bgp.rib.impl.RIBImplRuntimeRegistration;
-import org.opendaylight.controller.config.yang.bgp.rib.impl.RIBImplRuntimeRegistrator;
 import org.opendaylight.controller.md.sal.common.api.data.AsyncTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.controller.md.sal.common.api.data.TransactionChain;
@@ -122,7 +120,8 @@ public final class RIBImpl extends DefaultRibReference implements ClusterSinglet
         final ClusterIdentifier clusterId, final RIBExtensionConsumerContext extensions, final BGPDispatcher dispatcher,
         final BindingCodecTreeFactory codecFactory, final DOMDataBroker domDataBroker, final List<BgpTableType> localTables,
         @Nonnull final Map<TablesKey, PathSelectionMode> bestPathSelectionStrategies, final GeneratedClassLoadingStrategy classStrategy,
-        final BGPConfigModuleTracker moduleTracker, final BGPOpenConfigProvider openConfigProvider, final BgpDeployer.WriteConfiguration configurationWriter) {
+        final BGPConfigModuleTracker moduleTracker, final BGPOpenConfigProvider openConfigProvider,
+        final BgpDeployer.WriteConfiguration configurationWriter) {
 
         super(InstanceIdentifier.create(BgpRib.class).child(Rib.class, new RibKey(Preconditions.checkNotNull(ribId))));
         this.domChain = domDataBroker.createTransactionChain(this);
@@ -150,9 +149,10 @@ public final class RIBImpl extends DefaultRibReference implements ClusterSinglet
         this.serviceGroupIdentifier = ServiceGroupIdentifier.create(this.ribId + "-service-group");
         Preconditions.checkNotNull(provider, "ClusterSingletonServiceProvider is null");
         this.provider = provider;
+        this.configurationWriter = configurationWriter;
         LOG.info("RIB Singleton Service {} registered", this.getIdentifier());
+        //this need to be always the last step
         this.registration = registerClusterSingletonService(this);
-        this.configurationWriter = configurationWriter;
     }
 
     public RIBImpl(final ClusterSingletonServiceProvider provider, final RibId ribId, final AsNumber localAs, final BgpId localBgpId, @Nullable final ClusterIdentifier clusterId,

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/AppPeer.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/AppPeer.java
index 7527a41cdf357bbd6d0dbab1bb2c8066c4155f72..2c12a617dabae47f271ab02304b9a3338e8e7bd1 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/AppPeer.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/AppPeer.java
@@ -88,6 +88,7 @@ public class AppPeer implements PeerBean {
             this.dataTreeChangeService = rib.getService();
             this.serviceGroupIdentifier = rib.getRibIServiceGroupIdentifier();
             LOG.info("Application Peer Singleton Service {} registered", getIdentifier());
+            //this need to be always the last step
             this.singletonServiceRegistration = rib.registerClusterSingletonService(this);
         }
 

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImpl.java
index c3fc9c9c080c9b1a813906820f9b5a40064a0b51..02fdcaa3bedc4d0e72c51a7eae1d5432714b11dd 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImpl.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImpl.java
@@ -24,7 +24,6 @@ import java.util.HashMap;
 import java.util.Hashtable;
 import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
 import javax.annotation.concurrent.GuardedBy;
 import org.opendaylight.controller.md.sal.binding.api.ClusteredDataTreeChangeListener;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
@@ -175,13 +174,11 @@ public final class BgpDeployerImpl implements BgpDeployer, ClusteredDataTreeChan
 
     private List<PeerBean> closeAllBindedPeers(final InstanceIdentifier<Bgp> rootIdentifier) {
         final List<PeerBean> filtered = new ArrayList<>();
-        for (final Entry<InstanceIdentifier<Neighbor>, PeerBean> entry : this.peers.entrySet()) {
-            if (entry.getKey().contains(rootIdentifier)) {
-                final PeerBean peer = entry.getValue();
-                peer.close();
-                filtered.add(peer);
-            }
-        }
+        this.peers.entrySet().stream().filter(entry -> entry.getKey().contains(rootIdentifier)).forEach(entry -> {
+            final PeerBean peer = entry.getValue();
+            peer.close();
+            filtered.add(peer);
+        });
         return filtered;
     }
 

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java
index 66e22b1c4315593d17dbc4eeeb2523305d828369..cc6a802e31836b16228e31440f5dc68f951387dc 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java
@@ -218,10 +218,11 @@ public class BgpPeer implements PeerBean, BGPPeerRuntimeMXBean {
             this.inetAddress = Ipv4Util.toInetSocketAddress(this.neighborAddress, OpenConfigMappingUtil.getPort(neighbor));
             this.retryTimer = OpenConfigMappingUtil.getRetryTimer(neighbor);
             this.key = Optional.fromNullable(key);
+            this.configurationWriter = configurationWriter;
             this.serviceGroupIdentifier = rib.getRibIServiceGroupIdentifier();
             LOG.info("Peer Singleton Service {} registered", this.serviceGroupIdentifier);
+            //this need to be always the last step
             this.registration = rib.registerClusterSingletonService(this);
-            this.configurationWriter = configurationWriter;
         }
 
         @Override
@@ -247,7 +248,7 @@ public class BgpPeer implements PeerBean, BGPPeerRuntimeMXBean {
 
         @Override
         public ListenableFuture<Void> closeServiceInstance() {
-            LOG.info("Close RIB Singleton Service {}", this.getIdentifier());
+            LOG.info("Close Peer Singleton Service {}", this.getIdentifier());
             if (this.connection != null) {
                 this.connection.cancel(true);
                 this.connection = null;