Bug 8248: Don't access DSBE from different threads

With Epoll we may have more than one LispSouthboundHandler thread, and
they may access the same DataStoreBackEnd (DSBE) instance concurrently.
This patch moves DSBE access to the single threaded
LispSouthboundPlugin. Additionally, it implements some suggestions from
the following thread:

https://lists.opendaylight.org/pipermail/release/2017-April/010273.html

Change-Id: I32f48b6dcb58b241a4a679f7145bd36b5f410861
Signed-off-by: Lorand Jakab <lojakab@cisco.com>

diff --git a/mappingservice/dsbackend/src/main/java/org/opendaylight/lispflowmapping/dsbackend/DataStoreBackEnd.java b/mappingservice/dsbackend/src/main/java/org/opendaylight/lispflowmapping/dsbackend/DataStoreBackEnd.java
index 446ef60660080993c92da64d7d4812b414a465af..d36af2346040d0a3f33914140884ba76bfcdaa33 100644 (file)
--- a/mappingservice/dsbackend/src/main/java/org/opendaylight/lispflowmapping/dsbackend/DataStoreBackEnd.java
+++ b/mappingservice/dsbackend/src/main/java/org/opendaylight/lispflowmapping/dsbackend/DataStoreBackEnd.java
@@ -50,12 +50,18 @@ public class DataStoreBackEnd implements TransactionChainListener {
             InstanceIdentifier.create(MappingDatabase.class);
     private static final InstanceIdentifier<LastUpdated> LAST_UPDATED =
             InstanceIdentifier.create(MappingDatabase.class).child(LastUpdated.class);
+    private DataBroker broker;
     private BindingTransactionChain txChain;
 
     public DataStoreBackEnd(DataBroker broker) {
-        this.txChain = broker.createTransactionChain(this);
+        this.broker = broker;
+        createTransactionChain();
     }
 
+    public void createTransactionChain() {
+        LOG.debug("Creating DataStoreBackEnd transaction chain...");
+        txChain = broker.createTransactionChain(this);
+    }
 
     public void addAuthenticationKey(AuthenticationKey authenticationKey) {
         if (LOG.isDebugEnabled()) {
@@ -307,4 +313,9 @@ public class DataStoreBackEnd implements TransactionChainListener {
     public void onTransactionChainSuccessful(TransactionChain<?, ?> chain) {
         LOG.info("DataStoreBackEnd closed successfully, chain {}", chain);
     }
+
+    public void closeTransactionChain() {
+        LOG.debug("Closing DataStoreBackEnd transaction chain...");
+        txChain.close();
+    }
 }

diff --git a/mappingservice/southbound/src/main/java/org/opendaylight/lispflowmapping/southbound/LispSouthboundPlugin.java b/mappingservice/southbound/src/main/java/org/opendaylight/lispflowmapping/southbound/LispSouthboundPlugin.java
index 58286e68c6bb0d02b73c42986e5e0252e7107abf..ff4d673fe61523f06f47cdcd9839b000e6f9d29d 100644 (file)
--- a/mappingservice/southbound/src/main/java/org/opendaylight/lispflowmapping/southbound/LispSouthboundPlugin.java
+++ b/mappingservice/southbound/src/main/java/org/opendaylight/lispflowmapping/southbound/LispSouthboundPlugin.java
@@ -34,12 +34,14 @@ import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.UnknownHostException;
 import java.nio.ByteBuffer;
+import java.util.List;
 import java.util.concurrent.ThreadFactory;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.NotificationPublishService;
 import org.opendaylight.lispflowmapping.dsbackend.DataStoreBackEnd;
 import org.opendaylight.lispflowmapping.inmemorydb.HashMapDb;
 import org.opendaylight.lispflowmapping.lisp.type.LispMessage;
+import org.opendaylight.lispflowmapping.lisp.util.LispAddressStringifier;
 import org.opendaylight.lispflowmapping.mapcache.AuthKeyDb;
 import org.opendaylight.lispflowmapping.southbound.lisp.AuthenticationKeyDataListener;
 import org.opendaylight.lispflowmapping.southbound.lisp.LispSouthboundHandler;
@@ -51,7 +53,10 @@ import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceProvid
 import org.opendaylight.mdsal.singleton.common.api.ServiceGroupIdentifier;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.inet.binary.types.rev160303.IpAddressBinary;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MessageType;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.eid.container.Eid;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.mapping.authkey.container.MappingAuthkey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.transport.address.TransportAddress;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.mappingservice.rev150906.db.instance.AuthenticationKey;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -93,7 +98,6 @@ public class LispSouthboundPlugin implements IConfigLispSouthboundPlugin, AutoCl
         this.dataBroker = dataBroker;
         this.notificationPublishService = notificationPublishService;
         this.clusterSingletonService = clusterSingletonService;
-        this.clusterSingletonService.registerClusterSingletonService(this);
         if (Epoll.isAvailable()) {
             // When lispflowmapping is under heavy load, there are usually two threads nearing 100% CPU core
             // utilization. In order to have some headroom, we reserve 3 cores for "other" tasks, and allow the
@@ -109,20 +113,21 @@ public class LispSouthboundPlugin implements IConfigLispSouthboundPlugin, AutoCl
             this.akdb = new AuthKeyDb(new HashMapDb());
             this.authenticationKeyDataListener = new AuthenticationKeyDataListener(dataBroker, akdb);
             this.dsbe = new DataStoreBackEnd(dataBroker);
+            restoreDaoFromDatastore();
 
-            lispSouthboundHandler = new LispSouthboundHandler(this);
+            LispSouthboundHandler lispSouthboundHandler = new LispSouthboundHandler(this);
             lispSouthboundHandler.setDataBroker(dataBroker);
             lispSouthboundHandler.setNotificationProvider(notificationPublishService);
             lispSouthboundHandler.setAuthKeyDb(akdb);
             lispSouthboundHandler.setMapRegisterCache(mapRegisterCache);
             lispSouthboundHandler.setMapRegisterCacheTimeout(mapRegisterCacheTimeout);
             lispSouthboundHandler.setAuthenticationKeyDataListener(authenticationKeyDataListener);
-            lispSouthboundHandler.setDataStoreBackEnd(dsbe);
             lispSouthboundHandler.setStats(statistics);
-            lispSouthboundHandler.restoreDaoFromDatastore();
+            this.lispSouthboundHandler = lispSouthboundHandler;
 
-            lispXtrSouthboundHandler = new LispXtrSouthboundHandler();
+            LispXtrSouthboundHandler lispXtrSouthboundHandler = new LispXtrSouthboundHandler();
             lispXtrSouthboundHandler.setNotificationProvider(notificationPublishService);
+            this.lispXtrSouthboundHandler = lispXtrSouthboundHandler;
 
             if (Epoll.isAvailable()) {
                 eventLoopGroup = new EpollEventLoopGroup(numChannels, threadFactory);
@@ -147,6 +152,7 @@ public class LispSouthboundPlugin implements IConfigLispSouthboundPlugin, AutoCl
             start();
             startXtr();
 
+            clusterSingletonService.registerClusterSingletonService(this);
             LOG.info("LISP (RFC6830) Southbound Plugin is up!");
         }
     }
@@ -221,6 +227,23 @@ public class LispSouthboundPlugin implements IConfigLispSouthboundPlugin, AutoCl
         LOG.info("LISP (RFC6830) Southbound Plugin is down!");
     }
 
+    /**
+     * Restore all keys from MDSAL datastore.
+     */
+    public void restoreDaoFromDatastore() {
+        final List<AuthenticationKey> authKeys = dsbe.getAllAuthenticationKeys();
+        LOG.info("Restoring {} keys from datastore into southbound DAO", authKeys.size());
+
+        for (AuthenticationKey authKey : authKeys) {
+            final Eid key = authKey.getEid();
+            final MappingAuthkey mappingAuthkey = authKey.getMappingAuthkey();
+            LOG.debug("Adding authentication key '{}' with key-ID {} for {}", mappingAuthkey.getKeyString(),
+                    mappingAuthkey.getKeyType(),
+                    LispAddressStringifier.getString(key));
+            akdb.addAuthenticationKey(key, mappingAuthkey);
+        }
+    }
+
     public void handleSerializedLispBuffer(TransportAddress address, ByteBuffer outBuffer,
                                            final MessageType packetType) {
         InetAddress ip = getInetAddress(address);
@@ -337,13 +360,13 @@ public class LispSouthboundPlugin implements IConfigLispSouthboundPlugin, AutoCl
         lispSouthboundHandler.close();
         unloadActions();
         clusterSingletonService.close();
+        dsbe.closeTransactionChain();
     }
 
     @Override
     public void instantiateServiceInstance() {
         if (lispSouthboundHandler != null) {
             lispSouthboundHandler.setNotificationProvider(notificationPublishService);
-            lispSouthboundHandler.restoreDaoFromDatastore();
             lispSouthboundHandler.setIsMaster(true);
         }
         if (lispXtrSouthboundHandler != null) {

diff --git a/mappingservice/southbound/src/main/java/org/opendaylight/lispflowmapping/southbound/lisp/LispSouthboundHandler.java b/mappingservice/southbound/src/main/java/org/opendaylight/lispflowmapping/southbound/lisp/LispSouthboundHandler.java
index 3a3493a9361609c9663925dc7adc2516af0bfcc7..b1bd8bcf4e173add7541f223d6bb1235ebf44177 100644 (file)
--- a/mappingservice/southbound/src/main/java/org/opendaylight/lispflowmapping/southbound/lisp/LispSouthboundHandler.java
+++ b/mappingservice/southbound/src/main/java/org/opendaylight/lispflowmapping/southbound/lisp/LispSouthboundHandler.java
@@ -23,7 +23,6 @@ import java.util.Map;
 import java.util.Objects;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.NotificationPublishService;
-import org.opendaylight.lispflowmapping.dsbackend.DataStoreBackEnd;
 import org.opendaylight.lispflowmapping.lisp.authentication.ILispAuthentication;
 import org.opendaylight.lispflowmapping.lisp.authentication.LispAuthenticationUtil;
 import org.opendaylight.lispflowmapping.lisp.serializer.MapNotifySerializer;
@@ -66,7 +65,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.ma
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.mapping.record.container.MappingRecord;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.mapping.record.list.MappingRecordItem;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.transport.address.TransportAddressBuilder;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.mappingservice.rev150906.db.instance.AuthenticationKey;
 import org.opendaylight.yangtools.yang.binding.Notification;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -88,7 +86,6 @@ public class LispSouthboundHandler extends SimpleChannelInboundHandler<DatagramP
     private ConcurrentLispSouthboundStats lispSbStats = null;
     private AuthKeyDb akdb;
     private AuthenticationKeyDataListener authenticationKeyDataListener;
-    private DataStoreBackEnd dsbe;
     private boolean isReadFromChannelEnabled = true;
 
     private Channel channel;
@@ -536,31 +533,10 @@ public class LispSouthboundHandler extends SimpleChannelInboundHandler<DatagramP
         this.authenticationKeyDataListener = authenticationKeyDataListener;
     }
 
-    public void setDataStoreBackEnd(DataStoreBackEnd dsbe) {
-        this.dsbe = dsbe;
-    }
-
     public void setStats(ConcurrentLispSouthboundStats lispSbStats) {
         this.lispSbStats = lispSbStats;
     }
 
-    /**
-     * Restore all keys from MDSAL datastore.
-     */
-    public void restoreDaoFromDatastore() {
-        final List<AuthenticationKey> authKeys = dsbe.getAllAuthenticationKeys();
-        LOG.info("Restoring {} keys from datastore into southbound DAO", authKeys.size());
-
-        for (AuthenticationKey authKey : authKeys) {
-            final Eid key = authKey.getEid();
-            final MappingAuthkey mappingAuthkey = authKey.getMappingAuthkey();
-            LOG.debug("Adding authentication key '{}' with key-ID {} for {}", mappingAuthkey.getKeyString(),
-                    mappingAuthkey.getKeyType(),
-                    LispAddressStringifier.getString(key));
-            akdb.addAuthenticationKey(key, mappingAuthkey);
-        }
-    }
-
     public void setIsMaster(boolean isReadFromChannelEnabled) {
         this.isReadFromChannelEnabled = isReadFromChannelEnabled;
     }

diff --git a/mappingservice/southbound/src/test/java/org/opendaylight/lispflowmapping/southbound/LispSouthboundPluginTest.java b/mappingservice/southbound/src/test/java/org/opendaylight/lispflowmapping/southbound/LispSouthboundPluginTest.java
index e77df27fe6b4c0f06a9a448ce66f4056f96caf52..843b6d4e4c9fc7018185d62733b824e4a2e725ee 100644 (file)
--- a/mappingservice/southbound/src/test/java/org/opendaylight/lispflowmapping/southbound/LispSouthboundPluginTest.java
+++ b/mappingservice/southbound/src/test/java/org/opendaylight/lispflowmapping/southbound/LispSouthboundPluginTest.java
@@ -29,6 +29,7 @@ import org.mockito.ArgumentCaptor;
 import org.mockito.Mockito;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.NotificationPublishService;
+import org.opendaylight.lispflowmapping.dsbackend.DataStoreBackEnd;
 import org.opendaylight.lispflowmapping.lisp.type.LispMessage;
 import org.opendaylight.lispflowmapping.southbound.lisp.LispSouthboundHandler;
 import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceProvider;
@@ -192,6 +193,8 @@ public class LispSouthboundPluginTest {
     public void closeTest() throws Exception {
         EventLoopGroup elgMock = Mockito.mock(EventLoopGroup.class);
         LispSouthboundPluginTest.injectField("eventLoopGroup", elgMock);
+        DataStoreBackEnd dsbeMock = Mockito.mock(DataStoreBackEnd.class);
+        LispSouthboundPluginTest.injectField("dsbe", dsbeMock);
 
         LispSouthboundHandler handlerMock = Mockito.mock(LispSouthboundHandler.class);
         LispSouthboundPluginTest.injectField("lispSouthboundHandler", handlerMock);

diff --git a/mappingservice/southbound/src/test/java/org/opendaylight/lispflowmapping/southbound/lisp/LispSouthboundServiceTest.java b/mappingservice/southbound/src/test/java/org/opendaylight/lispflowmapping/southbound/lisp/LispSouthboundServiceTest.java
index ae963150d27f3f24bf0aa8467bacaf7613956d32..71e5a14e34a020666a1068decd7d5163ada10be5 100644 (file)
--- a/mappingservice/southbound/src/test/java/org/opendaylight/lispflowmapping/southbound/lisp/LispSouthboundServiceTest.java
+++ b/mappingservice/southbound/src/test/java/org/opendaylight/lispflowmapping/southbound/lisp/LispSouthboundServiceTest.java
@@ -42,7 +42,6 @@ import org.mockito.Matchers;
 import org.mockito.Mockito;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.NotificationPublishService;
-import org.opendaylight.lispflowmapping.dsbackend.DataStoreBackEnd;
 import org.opendaylight.lispflowmapping.lisp.serializer.MapNotifySerializer;
 import org.opendaylight.lispflowmapping.lisp.serializer.MapReplySerializer;
 import org.opendaylight.lispflowmapping.lisp.type.LispMessage;
@@ -160,7 +159,6 @@ public class LispSouthboundServiceTest extends BaseTestCase {
         testedLispService.setDataBroker(Mockito.mock(DataBroker.class));
         testedLispService.setAuthKeyDb(akdb);
         testedLispService.setAuthenticationKeyDataListener(Mockito.mock(AuthenticationKeyDataListener.class));
-        testedLispService.setDataStoreBackEnd(Mockito.mock(DataStoreBackEnd.class));
         nps = context.mock(NotificationPublishService.class);
         testedLispService.setNotificationProvider(nps);
         lispSouthboundStats = new ConcurrentLispSouthboundStats();