BUG-8511: add more explicit messages

This adds more defensive handling of connections and locking,
even if it should not strictly be necessary, as we are using
atomic operations and run on the actor thread. This makes the
transitions work even in fact of actor context leakage.

Change-Id: I26df0f208d63b861a0f3d3dc3c0f1959bbc79e90
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 90a377ec4fe7c1aa4df6d4fde74bfc8189e95b08)

diff --git a/opendaylight/md-sal/cds-access-client/src/main/java/org/opendaylight/controller/cluster/access/client/ClientActorBehavior.java b/opendaylight/md-sal/cds-access-client/src/main/java/org/opendaylight/controller/cluster/access/client/ClientActorBehavior.java
index 5d61885cd55adde9cc40d85228c9dc02c9931497..ee63eddaa7cd26de705b7e5700e1f35dc3b323c6 100644 (file)
--- a/opendaylight/md-sal/cds-access-client/src/main/java/org/opendaylight/controller/cluster/access/client/ClientActorBehavior.java
+++ b/opendaylight/md-sal/cds-access-client/src/main/java/org/opendaylight/controller/cluster/access/client/ClientActorBehavior.java
@@ -296,16 +296,35 @@ public abstract class ClientActorBehavior<T extends BackendInfo> extends
             conn.finishReplay(forwarder);
 
             // Make sure new lookups pick up the new connection
             conn.finishReplay(forwarder);
 
             // Make sure new lookups pick up the new connection

-            connections.replace(shard, conn, newConn);
-            LOG.info("{}: replaced connection {} with {}", persistenceId(), conn, newConn);
+            if (!connections.replace(shard, conn, newConn)) {
+                final AbstractClientConnection<T> existing = connections.get(conn.cookie());
+                LOG.warn("{}: old connection {} does not match existing {}, new connection {} in limbo",
+                    persistenceId(), conn, existing, newConn);
+            } else {
+                LOG.info("{}: replaced connection {} with {}", persistenceId(), conn, newConn);
+            }

         } finally {
             connectionsLock.unlockWrite(stamp);
         }
     }
 
     void removeConnection(final AbstractClientConnection<?> conn) {
         } finally {
             connectionsLock.unlockWrite(stamp);
         }
     }
 
     void removeConnection(final AbstractClientConnection<?> conn) {

-        connections.remove(conn.cookie(), conn);
-        LOG.debug("{}: removed connection {}", persistenceId(), conn);
+        final long stamp = connectionsLock.writeLock();
+        try {
+            if (!connections.remove(conn.cookie(), conn)) {
+                final AbstractClientConnection<T> existing = connections.get(conn.cookie());
+                if (existing != null) {
+                    LOG.warn("{}: failed to remove connection {}, as it was superseded by {}", persistenceId(), conn,
+                        existing);
+                } else {
+                    LOG.warn("{}: failed to remove connection {}, as it was not tracked", persistenceId(), conn);
+                }
+            } else {
+                LOG.info("{}: removed connection {}", persistenceId(), conn);
+            }
+        } finally {
+            connectionsLock.unlockWrite(stamp);
+        }

     }
 
     @SuppressWarnings("unchecked")
     }
 
     @SuppressWarnings("unchecked")


@@ -314,11 +333,20 @@ public abstract class ClientActorBehavior<T extends BackendInfo> extends
         final ReconnectingClientConnection<T> conn = (ReconnectingClientConnection<T>)newConn;
         LOG.info("{}: connection {} reconnecting as {}", persistenceId(), oldConn, newConn);
 
         final ReconnectingClientConnection<T> conn = (ReconnectingClientConnection<T>)newConn;
         LOG.info("{}: connection {} reconnecting as {}", persistenceId(), oldConn, newConn);
 

-        final boolean replaced = connections.replace(oldConn.cookie(), (AbstractClientConnection<T>)oldConn, conn);
-        if (!replaced) {
-            final AbstractClientConnection<T> existing = connections.get(oldConn.cookie());
-            LOG.warn("{}: old connection {} does not match existing {}, new connection {} in limbo", persistenceId(),
-                oldConn, existing, newConn);
+        final long stamp = connectionsLock.writeLock();
+        try {
+            final boolean replaced = connections.replace(oldConn.cookie(), (AbstractClientConnection<T>)oldConn, conn);
+            if (!replaced) {
+                final AbstractClientConnection<T> existing = connections.get(oldConn.cookie());
+                if (existing != null) {
+                    LOG.warn("{}: failed to replace connection {}, as it was superseded by {}", persistenceId(), conn,
+                        existing);
+                } else {
+                    LOG.warn("{}: failed to replace connection {}, as it was not tracked", persistenceId(), conn);
+                }
+            }
+        } finally {
+            connectionsLock.unlockWrite(stamp);

         }
 
         final Long shard = oldConn.cookie();
         }
 
         final Long shard = oldConn.cookie();