BUG-5280: introduce cookie in LocalHistoryIdentifier 07/39607/45
authorRobert Varga <rovarga@cisco.com>
Mon, 30 May 2016 14:19:12 +0000 (16:19 +0200)
committerRobert Varga <rovarga@cisco.com>
Wed, 22 Jun 2016 15:09:58 +0000 (17:09 +0200)
Frontend transactions can map onto multiple backend shards,
hence the current form is not sufficient to identify responses.

Introduce anopaque cookie, which will be assigned to frontend
subtransactions and hence provide identification.

Change-Id: I442dcfa1a6f04330c608f3328a7e10c6aeb90bb0
Signed-off-by: Robert Varga <rovarga@cisco.com>
opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/concepts/LocalHistoryIdentifier.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ClientLocalHistory.java

index 501ac5a..a6e81f1 100644 (file)
@@ -19,60 +19,86 @@ import org.opendaylight.yangtools.concepts.WritableIdentifier;
 import org.opendaylight.yangtools.concepts.WritableObjects;
 
 /**
- * Globally-unique identifier of a local history.
+ * Globally-unique identifier of a local history. This identifier is assigned on the frontend and is composed of
+ * - a {@link ClientIdentifier}, which uniquely identifies a single instantiation of a particular frontend
+ * - an unsigned long, which uniquely identifies the history on the backend
+ * - an unsigned long cookie, assigned by the client and meaningless on the backend, which just reflects it back
  *
  * @author Robert Varga
  */
 public final class LocalHistoryIdentifier implements WritableIdentifier {
+    /*
+     * Implementation note: cookie is currently required only for module-based sharding, which is implemented as part
+     *                      of normal DataBroker interfaces. For DOMDataTreeProducer cookie will always be zero, hence
+     *                      we may end up not needing cookie at all.
+     *
+     *                      We use WritableObjects.writeLongs() to output historyId and cookie (in that order). If we
+     *                      end up not needing the cookie at all, we can switch to writeLong() and use zero flags for
+     *                      compatibility.
+     */
     private static final class Proxy implements Externalizable {
         private static final long serialVersionUID = 1L;
         private ClientIdentifier clientId;
         private long historyId;
+        private long cookie;
 
         public Proxy() {
             // For Externalizable
         }
 
-        Proxy(final ClientIdentifier frontendId, final long historyId) {
+        Proxy(final ClientIdentifier frontendId, final long historyId, final long cookie) {
             this.clientId = Preconditions.checkNotNull(frontendId);
             this.historyId = historyId;
+            this.cookie = cookie;
         }
 
         @Override
         public void writeExternal(final ObjectOutput out) throws IOException {
             clientId.writeTo(out);
-            WritableObjects.writeLong(out, historyId);
+            WritableObjects.writeLongs(out, historyId, cookie);
         }
 
         @Override
         public void readExternal(final ObjectInput in) throws IOException, ClassNotFoundException {
             clientId = ClientIdentifier.readFrom(in);
-            historyId = WritableObjects.readLong(in);
+
+            final byte header = WritableObjects.readLongHeader(in);
+            historyId = WritableObjects.readFirstLong(in, header);
+            cookie = WritableObjects.readSecondLong(in, header);
         }
 
         private Object readResolve() {
-            return new LocalHistoryIdentifier(clientId, historyId);
+            return new LocalHistoryIdentifier(clientId, historyId, cookie);
         }
     }
 
     private static final long serialVersionUID = 1L;
     private final ClientIdentifier clientId;
     private final long historyId;
+    private final long cookie;
 
     public LocalHistoryIdentifier(final ClientIdentifier frontendId, final long historyId) {
+        this(frontendId, historyId, 0);
+    }
+
+    public LocalHistoryIdentifier(final ClientIdentifier frontendId, final long historyId, final long cookie) {
         this.clientId = Preconditions.checkNotNull(frontendId);
         this.historyId = historyId;
+        this.cookie = cookie;
     }
 
     public static LocalHistoryIdentifier readFrom(final DataInput in) throws IOException {
         final ClientIdentifier clientId = ClientIdentifier.readFrom(in);
-        return new LocalHistoryIdentifier(clientId, WritableObjects.readLong(in));
+
+        final byte header = WritableObjects.readLongHeader(in);
+        return new LocalHistoryIdentifier(clientId, WritableObjects.readFirstLong(in, header),
+            WritableObjects.readSecondLong(in, header));
     }
 
     @Override
     public void writeTo(final DataOutput out) throws IOException {
         clientId.writeTo(out);
-        WritableObjects.writeLong(out, historyId);
+        WritableObjects.writeLongs(out, historyId, cookie);
     }
 
     public ClientIdentifier getClientId() {
@@ -83,9 +109,16 @@ public final class LocalHistoryIdentifier implements WritableIdentifier {
         return historyId;
     }
 
+    public long getCookie() {
+        return cookie;
+    }
+
     @Override
     public int hashCode() {
-        return clientId.hashCode() * 31 + Long.hashCode(historyId);
+        int ret = clientId.hashCode();
+        ret = 31 * ret + Long.hashCode(historyId);
+        ret = 31 * ret + Long.hashCode(cookie);
+        return ret;
     }
 
     @Override
@@ -98,16 +131,17 @@ public final class LocalHistoryIdentifier implements WritableIdentifier {
         }
 
         final LocalHistoryIdentifier other = (LocalHistoryIdentifier) o;
-        return historyId == other.historyId && clientId.equals(other.clientId);
+        return historyId == other.historyId && cookie == other.cookie && clientId.equals(other.clientId);
     }
 
     @Override
     public String toString() {
         return MoreObjects.toStringHelper(LocalHistoryIdentifier.class).add("client", clientId)
-                .add("history", Long.toUnsignedString(historyId)).toString();
+                .add("history", Long.toUnsignedString(historyId, 16))
+                .add("cookie", Long.toUnsignedString(cookie, 16)).toString();
     }
 
     private Object writeReplace() {
-        return new Proxy(clientId, historyId);
+        return new Proxy(clientId, historyId, cookie);
     }
 }
index b22f2bd..aded49b 100644 (file)
@@ -10,8 +10,9 @@ package org.opendaylight.controller.cluster.databroker.actors.dds;
 import akka.actor.ActorRef;
 import com.google.common.annotations.Beta;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Verify;
 import java.util.concurrent.atomic.AtomicIntegerFieldUpdater;
-import javax.annotation.concurrent.NotThreadSafe;
+import org.opendaylight.controller.cluster.access.concepts.ClientIdentifier;
 import org.opendaylight.controller.cluster.access.concepts.LocalHistoryIdentifier;
 
 /**
@@ -25,14 +26,14 @@ import org.opendaylight.controller.cluster.access.concepts.LocalHistoryIdentifie
  * @author Robert Varga
  */
 @Beta
-@NotThreadSafe
 public final class ClientLocalHistory implements AutoCloseable {
-    private static final AtomicIntegerFieldUpdater<ClientLocalHistory> CLOSED_UPDATER =
+    private static final AtomicIntegerFieldUpdater<ClientLocalHistory> STATE_UPDATER =
             AtomicIntegerFieldUpdater.newUpdater(ClientLocalHistory.class, "state");
     private static final int IDLE_STATE = 0;
     private static final int CLOSED_STATE = 1;
 
-    private final LocalHistoryIdentifier historyId;
+    private final ClientIdentifier clientId;
+    private final long historyId;
     private final ActorRef backendActor;
     private final ActorRef clientActor;
 
@@ -42,16 +43,19 @@ public final class ClientLocalHistory implements AutoCloseable {
             final ActorRef backendActor) {
         this.clientActor = client.self();
         this.backendActor = Preconditions.checkNotNull(backendActor);
-        this.historyId = new LocalHistoryIdentifier(client.getIdentifier(), historyId);
+        this.clientId = Verify.verifyNotNull(client.getIdentifier());
+        this.historyId = historyId;
     }
 
     private void checkNotClosed() {
-        Preconditions.checkState(state != CLOSED_STATE, "Local history %s has been closed", historyId);
+        if (state == CLOSED_STATE) {
+            throw new IllegalStateException("Local history " + new LocalHistoryIdentifier(clientId, historyId) + " is closed");
+        }
     }
 
     @Override
     public void close() {
-        if (CLOSED_UPDATER.compareAndSet(this, IDLE_STATE, CLOSED_STATE)) {
+        if (STATE_UPDATER.compareAndSet(this, IDLE_STATE, CLOSED_STATE)) {
             // FIXME: signal close to both client actor and backend actor
         } else if (state != CLOSED_STATE) {
             throw new IllegalStateException("Cannot close history with an open transaction");