Bug 7814: Fix InvalidActorNameException

When a read tx actor is created in the Shard, the actor name is generated
as eg "shard-member-1:datastore-operational@0:19" where 0 is the instance's
static generation id and 19 is the tx id. If the tx was created from a chain,
the chain's history id is appended. So every part of the name is constant for
the controller instance except the tx id, which is generated via a counter in
AbstractTransactionContextFactory, and the history id which is also generated
via a counter. The counters do make the full tx id unique in the cluster.
However if multiple shards are involved in a single front-end transaction,
they all have the same tx id and thus the actor names generated by each shard
would be the same. It seems that's what occurred in Bug 7814. Therefore I
changed the actor name to include the shard name.

Change-Id: Iacb11bb401bd6bded38847690f8009c115ee0637
Signed-off-by: Tom Pantelis <tpanteli@brocade.com>

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java
index 423323187fff7346cd9b13172b17949f08ec7922..7ca79fc2349add0e9c19236a945aaed53676b45d 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java
@@ -209,8 +209,8 @@ public class Shard extends RaftActor {
                 getRaftActorContext().getConfigParams().getIsolatedCheckIntervalInMillis());
 
         transactionActorFactory = new ShardTransactionActorFactory(store, datastoreContext,
                 getRaftActorContext().getConfigParams().getIsolatedCheckIntervalInMillis());
 
         transactionActorFactory = new ShardTransactionActorFactory(store, datastoreContext,

-                new Dispatchers(context().system().dispatchers()).getDispatcherPath(
-                        Dispatchers.DispatcherType.Transaction), self(), getContext(), shardMBean);
+            new Dispatchers(context().system().dispatchers()).getDispatcherPath(Dispatchers.DispatcherType.Transaction),
+                self(), getContext(), shardMBean, builder.getId().getShardName());

 
         snapshotCohort = ShardSnapshotCohort.create(getContext(), builder.getId().getMemberName(), store, LOG,
             this.name);
 
         snapshotCohort = ShardSnapshotCohort.create(getContext(), builder.getId().getMemberName(), store, LOG,
             this.name);

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransactionActorFactory.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransactionActorFactory.java
index 0ba36640a8f6368451478d0f28451a2c7a437db4..8e9c762bf188e60c02485a74b694ebc0fd3880cc 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransactionActorFactory.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransactionActorFactory.java
@@ -29,26 +29,30 @@ class ShardTransactionActorFactory {
     private final ShardStats shardMBean;
     private final UntypedActorContext actorContext;
     private final ActorRef shardActor;
     private final ShardStats shardMBean;
     private final UntypedActorContext actorContext;
     private final ActorRef shardActor;

+    private final String shardName;

 
     ShardTransactionActorFactory(ShardDataTree dataTree, DatastoreContext datastoreContext,
 
     ShardTransactionActorFactory(ShardDataTree dataTree, DatastoreContext datastoreContext,

-            String txnDispatcherPath, ActorRef shardActor, UntypedActorContext actorContext, ShardStats shardMBean) {
+            String txnDispatcherPath, ActorRef shardActor, UntypedActorContext actorContext, ShardStats shardMBean,
+            String shardName) {

         this.dataTree = Preconditions.checkNotNull(dataTree);
         this.dataTree = Preconditions.checkNotNull(dataTree);

-        this.datastoreContext = datastoreContext;
-        this.txnDispatcherPath = txnDispatcherPath;
-        this.shardMBean = shardMBean;
-        this.actorContext = actorContext;
-        this.shardActor = shardActor;
+        this.datastoreContext = Preconditions.checkNotNull(datastoreContext);
+        this.txnDispatcherPath = Preconditions.checkNotNull(txnDispatcherPath);
+        this.shardMBean = Preconditions.checkNotNull(shardMBean);
+        this.actorContext = Preconditions.checkNotNull(actorContext);
+        this.shardActor = Preconditions.checkNotNull(shardActor);
+        this.shardName = Preconditions.checkNotNull(shardName);

     }
 
     }
 

-    private static String actorNameFor(final TransactionIdentifier txId) {
+    private String actorNameFor(final TransactionIdentifier txId) {

         final LocalHistoryIdentifier historyId = txId.getHistoryId();
         final ClientIdentifier clientId = historyId.getClientId();
         final FrontendIdentifier frontendId = clientId.getFrontendId();
 
         final StringBuilder sb = new StringBuilder("shard-");
         final LocalHistoryIdentifier historyId = txId.getHistoryId();
         final ClientIdentifier clientId = historyId.getClientId();
         final FrontendIdentifier frontendId = clientId.getFrontendId();
 
         final StringBuilder sb = new StringBuilder("shard-");

-        sb.append(frontendId.getMemberName().getName()).append(':');
-        sb.append(frontendId.getClientType().getName()).append('@');
-        sb.append(clientId.getGeneration()).append(':');
+        sb.append(shardName).append('-')
+            .append(frontendId.getMemberName().getName()).append(':')
+            .append(frontendId.getClientType().getName()).append('@')
+            .append(clientId.getGeneration()).append(':');

         if (historyId.getHistoryId() != 0) {
             sb.append(historyId.getHistoryId()).append('-');
         }
         if (historyId.getHistoryId() != 0) {
             sb.append(historyId.getHistoryId()).append('-');
         }

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java
index d484f99696a6f89b7528eda2c9e64abc8c212eca..45239ac63fdff808d8bb4ddceaf70d84f7eb7752 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java
@@ -373,8 +373,9 @@ public class ShardTest extends AbstractShardTest {
                         CreateTransactionReply.class);
 
                 final String path = reply.getTransactionPath().toString();
                         CreateTransactionReply.class);
 
                 final String path = reply.getTransactionPath().toString();

-                assertTrue("Unexpected transaction path " + path, path
-                        .startsWith("akka://test/user/testCreateTransaction/shard-member-1:ShardTransactionTest@0:"));
+                assertTrue("Unexpected transaction path " + path, path.startsWith(String.format(
+                        "akka://test/user/testCreateTransaction/shard-%s-%s:ShardTransactionTest@0:",
+                            shardID.getShardName(), shardID.getMemberName().getName())));

             }
         };
     }
             }
         };
     }


@@ -394,8 +395,9 @@ public class ShardTest extends AbstractShardTest {
                         CreateTransactionReply.class);
 
                 final String path = reply.getTransactionPath().toString();
                         CreateTransactionReply.class);
 
                 final String path = reply.getTransactionPath().toString();

-                assertTrue("Unexpected transaction path " + path, path.startsWith(
-                        "akka://test/user/testCreateTransactionOnChain/shard-member-1:ShardTransactionTest@0:"));
+                assertTrue("Unexpected transaction path " + path, path.startsWith(String.format(
+                        "akka://test/user/testCreateTransactionOnChain/shard-%s-%s:ShardTransactionTest@0:",
+                        shardID.getShardName(), shardID.getMemberName().getName())));

             }
         };
     }
             }
         };
     }