Bug 5913: Fix ISE in DefaultShardDataChangeListenerPublisher 06/40306/2
authorTom Pantelis <tpanteli@brocade.com>
Tue, 14 Jun 2016 14:18:30 +0000 (10:18 -0400)
committerTom Pantelis <tpanteli@brocade.com>
Tue, 14 Jun 2016 20:54:51 +0000 (20:54 +0000)
The publishChanges method is only called from the
ShardDataTreeNotificationPublisherActor which is single-threaded so
publishChanges can't be called concurrently. However the
DefaultShardDataChangeListenerPublisher instance is passed via
the PublishNotifications message so the Stopwatch isn't thread safe
wrt thread visibility of its internal state. Therefore it's possible
the change in state done on thread 1 isn't immediately visible to
a subsequent thread. To alleviate this, I moved the Stopwatch and the
elapsed time check to the ShardDataTreeNotificationPublisherActor.

Change-Id: I046e7e92aa96eec01d5a355c8431ef797c534ead
Signed-off-by: Tom Pantelis <tpanteli@brocade.com>
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractShardDataTreeNotificationPublisherActorProxy.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DefaultShardDataChangeListenerPublisher.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DefaultShardDataTreeChangeListenerPublisher.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeNotificationPublisherActor.java

index 93081eb..61a22d4 100644 (file)
@@ -55,7 +55,7 @@ abstract class AbstractShardDataTreeNotificationPublisherActorProxy implements S
 
             String dispatcher = new Dispatchers(actorContext.system().dispatchers()).getDispatcherPath(
                     Dispatchers.DispatcherType.Notification);
-            notifierActor = actorContext.actorOf(ShardDataTreeNotificationPublisherActor.props()
+            notifierActor = actorContext.actorOf(ShardDataTreeNotificationPublisherActor.props(actorName)
                     .withDispatcher(dispatcher).withMailbox(
                             org.opendaylight.controller.cluster.datastore.utils.ActorContext.BOUNDED_MAILBOX), actorName);
         }
index cf98f38..bf52aa1 100644 (file)
@@ -7,8 +7,6 @@
  */
 package org.opendaylight.controller.cluster.datastore;
 
-import com.google.common.base.Stopwatch;
-import java.util.concurrent.TimeUnit;
 import javax.annotation.concurrent.NotThreadSafe;
 import org.opendaylight.controller.md.sal.common.api.data.AsyncDataBroker.DataChangeScope;
 import org.opendaylight.controller.md.sal.common.api.data.AsyncDataChangeListener;
@@ -35,7 +33,6 @@ final class DefaultShardDataChangeListenerPublisher implements ShardDataChangeLi
     private static final Logger LOG = LoggerFactory.getLogger(DefaultShardDataChangeListenerPublisher.class);
 
     private final ListenerTree dataChangeListenerTree = ListenerTree.create();
-    private final Stopwatch timer = Stopwatch.createUnstarted();
 
     @Override
     public void submitNotification(final DataChangeListenerRegistration<?> listener, final DOMImmutableDataChangeEvent notification) {
@@ -56,22 +53,7 @@ final class DefaultShardDataChangeListenerPublisher implements ShardDataChangeLi
 
     @Override
     public void publishChanges(DataTreeCandidate candidate, String logContext) {
-        timer.start();
-
-        try {
-            ResolveDataChangeEventsTask.create(candidate, dataChangeListenerTree).resolve(this);
-        } finally {
-            timer.stop();
-            long elapsedTime = timer.elapsed(TimeUnit.MILLISECONDS);
-            if(elapsedTime >= PUBLISH_DELAY_THRESHOLD_IN_MS) {
-                LOG.warn("{}: Generation of DataChange events took longer than expected. Elapsed time: {}",
-                        logContext, timer);
-            } else {
-                LOG.debug("{}: Elapsed time for generation of DataChange events: {}", logContext, timer);
-            }
-
-            timer.reset();
-        }
+        ResolveDataChangeEventsTask.create(candidate, dataChangeListenerTree).resolve(this);
     }
 
     @Override
index 217ffd3..b539443 100644 (file)
@@ -7,10 +7,8 @@
  */
 package org.opendaylight.controller.cluster.datastore;
 
-import com.google.common.base.Stopwatch;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.concurrent.TimeUnit;
 import javax.annotation.concurrent.NotThreadSafe;
 import org.opendaylight.controller.md.sal.dom.spi.AbstractDOMDataTreeChangeListenerRegistration;
 import org.opendaylight.controller.sal.core.spi.data.AbstractDOMStoreTreeChangePublisher;
@@ -32,26 +30,9 @@ final class DefaultShardDataTreeChangeListenerPublisher extends AbstractDOMStore
         implements ShardDataTreeChangeListenerPublisher {
     private static final Logger LOG = LoggerFactory.getLogger(DefaultShardDataTreeChangeListenerPublisher.class);
 
-    private final Stopwatch timer = Stopwatch.createUnstarted();
-
     @Override
     public void publishChanges(final DataTreeCandidate candidate, String logContext) {
-        timer.start();
-
-        try {
-            processCandidateTree(candidate);
-        } finally {
-            timer.stop();
-            long elapsedTime = timer.elapsed(TimeUnit.MILLISECONDS);
-            if(elapsedTime >= PUBLISH_DELAY_THRESHOLD_IN_MS) {
-                LOG.warn("{}: Generation of DataTreeCandidateNode events took longer than expected. Elapsed time: {}",
-                        logContext, timer);
-            } else {
-                LOG.debug("{}: Elapsed time for generation of DataTreeCandidateNode events: {}", logContext, timer);
-            }
-
-            timer.reset();
-        }
+        processCandidateTree(candidate);
     }
 
     @Override
index e4e7eb3..8102356 100644 (file)
@@ -8,6 +8,8 @@
 package org.opendaylight.controller.cluster.datastore;
 
 import akka.actor.Props;
+import com.google.common.base.Stopwatch;
+import java.util.concurrent.TimeUnit;
 import org.opendaylight.controller.cluster.common.actor.AbstractUntypedActor;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate;
 
@@ -18,16 +20,39 @@ import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate;
  * @author Thomas Pantelis
  */
 public class ShardDataTreeNotificationPublisherActor extends AbstractUntypedActor {
+    private final Stopwatch timer = Stopwatch.createUnstarted();
+    private final String name;
+
+    private ShardDataTreeNotificationPublisherActor(String name) {
+        this.name = name;
+    }
 
     @Override
     protected void handleReceive(Object message) {
         if(message instanceof PublishNotifications) {
-            ((PublishNotifications)message).publish();
+            PublishNotifications publisher = (PublishNotifications)message;
+            timer.start();
+
+            try {
+                publisher.publish();
+            } finally {
+                long elapsedTime = timer.elapsed(TimeUnit.MILLISECONDS);
+
+                if(elapsedTime >= ShardDataTreeNotificationPublisher.PUBLISH_DELAY_THRESHOLD_IN_MS) {
+                    LOG.warn("{}: Generation of change events for {} took longer than expected. Elapsed time: {}",
+                            publisher.logContext, name, timer);
+                } else {
+                    LOG.debug("{}: Elapsed time for generation of change events for {}: {}", publisher.logContext,
+                            name, timer);
+                }
+
+                timer.reset();
+            }
         }
     }
 
-    static Props props() {
-        return Props.create(ShardDataTreeNotificationPublisherActor.class);
+    static Props props(String notificationType) {
+        return Props.create(ShardDataTreeNotificationPublisherActor.class, notificationType);
     }
 
     static class PublishNotifications {

©2013 OpenDaylight, A Linux Foundation Collaborative Project. All Rights Reserved.
OpenDaylight is a registered trademark of The OpenDaylight Project, Inc.
Linux Foundation and OpenDaylight are registered trademarks of the Linux Foundation.
Linux is a registered trademark of Linus Torvalds.