From: Robert Varga Date: Mon, 30 May 2016 14:19:12 +0000 (+0200) Subject: BUG-5280: introduce cookie in LocalHistoryIdentifier X-Git-Tag: release/boron~103 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=97ff7dff8e58531065833736d5788808ca9e0396 BUG-5280: introduce cookie in LocalHistoryIdentifier 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 --- diff --git a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/concepts/LocalHistoryIdentifier.java b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/concepts/LocalHistoryIdentifier.java index 501ac5abee..a6e81f13aa 100644 --- a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/concepts/LocalHistoryIdentifier.java +++ b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/concepts/LocalHistoryIdentifier.java @@ -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); } } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ClientLocalHistory.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ClientLocalHistory.java index b22f2bdc7b..aded49b144 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ClientLocalHistory.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ClientLocalHistory.java @@ -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 CLOSED_UPDATER = + private static final AtomicIntegerFieldUpdater 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");