BUG-8402: correctly propagate read-only bit 92/57692/3
authorRobert Varga <robert.varga@pantheon.tech>
Tue, 23 May 2017 12:56:37 +0000 (14:56 +0200)
committerRobert Varga <nite@hq.sk>
Sun, 28 May 2017 14:52:24 +0000 (14:52 +0000)
During replay we substitute read requests with an IncrementSequence
request, but that does not indicate whether the transaction state
should be read-only.

This leads to transaction chains allocating a full-blown transaction
instead of a snapshot, hence follow-up transactions fail to allocate,
leading to OutOfOrderRequestException.

Fix this by making IncrementTransactionSequenceRequest a subclass
of AbstractReadTransactionRequest so it carries isSnapshotOnly().

Change-Id: Ifdb6214478aa7548d3bc1f06b532e06c93b3dd0b
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
13 files changed:
opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadPathTransactionRequest.java [new file with mode: 0644]
opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadPathTransactionRequestProxyV1.java [new file with mode: 0644]
opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadTransactionRequest.java
opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadTransactionRequestProxyV1.java
opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ExistsTransactionRequest.java
opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ExistsTransactionRequestProxyV1.java
opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/IncrementTransactionSequenceRequest.java
opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/IncrementTransactionSequenceRequestProxyV1.java
opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ReadTransactionRequest.java
opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ReadTransactionRequestProxyV1.java
opendaylight/md-sal/cds-access-api/src/test/java/org/opendaylight/controller/cluster/access/commands/AbstractReadTransactionRequestTest.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractProxyTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/RemoteProxyTransaction.java

diff --git a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadPathTransactionRequest.java b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadPathTransactionRequest.java
new file mode 100644 (file)
index 0000000..fee3b03
--- /dev/null
@@ -0,0 +1,60 @@
+/*
+ * Copyright (c) 2017 Pantheon Technologies, s.r.o. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.controller.cluster.access.commands;
+
+import akka.actor.ActorRef;
+import com.google.common.annotations.Beta;
+import com.google.common.base.MoreObjects.ToStringHelper;
+import com.google.common.base.Preconditions;
+import javax.annotation.Nonnull;
+import org.opendaylight.controller.cluster.access.ABIVersion;
+import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
+
+/**
+ * Abstract base class for {@link TransactionRequest}s accessing data as visible in the isolated context of a particular
+ * transaction. The path of the data being accessed is returned via {@link #getPath()}.
+ *
+ * <p>
+ * This class is visible outside of this package for the purpose of allowing common instanceof checks
+ * and simplified codepaths.
+ *
+ * @author Robert Varga
+ *
+ * @param <T> Message type
+ */
+@Beta
+public abstract class AbstractReadPathTransactionRequest<T extends AbstractReadPathTransactionRequest<T>>
+        extends AbstractReadTransactionRequest<T> {
+    private static final long serialVersionUID = 1L;
+    private final YangInstanceIdentifier path;
+
+    AbstractReadPathTransactionRequest(final TransactionIdentifier identifier, final long sequence,
+        final ActorRef replyTo, final YangInstanceIdentifier path, final boolean snapshotOnly) {
+        super(identifier, sequence, replyTo, snapshotOnly);
+        this.path = Preconditions.checkNotNull(path);
+    }
+
+    AbstractReadPathTransactionRequest(final T request, final ABIVersion version) {
+        super(request, version);
+        this.path = request.getPath();
+    }
+
+    @Nonnull
+    public final YangInstanceIdentifier getPath() {
+        return path;
+    }
+
+    @Override
+    protected ToStringHelper addToStringAttributes(final ToStringHelper toStringHelper) {
+        return super.addToStringAttributes(toStringHelper).add("path", path);
+    }
+
+    @Override
+    protected abstract AbstractReadTransactionRequestProxyV1<T> externalizableProxy(ABIVersion version);
+}
diff --git a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadPathTransactionRequestProxyV1.java b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadPathTransactionRequestProxyV1.java
new file mode 100644 (file)
index 0000000..bafa3d9
--- /dev/null
@@ -0,0 +1,63 @@
+/*
+ * Copyright (c) 2017 Pantheon Technologies, s.r.o. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.controller.cluster.access.commands;
+
+import akka.actor.ActorRef;
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier;
+import org.opendaylight.controller.cluster.datastore.node.utils.stream.NormalizedNodeDataOutput;
+import org.opendaylight.controller.cluster.datastore.node.utils.stream.NormalizedNodeInputOutput;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
+
+/**
+ * Abstract base class for serialization proxies associated with {@link AbstractReadTransactionRequest}s. It implements
+ * the initial (Boron) serialization format.
+ *
+ * @author Robert Varga
+ *
+ * @param <T> Message type
+ */
+abstract class AbstractReadPathTransactionRequestProxyV1<T extends AbstractReadPathTransactionRequest<T>>
+        extends AbstractReadTransactionRequestProxyV1<T> {
+    private static final long serialVersionUID = 1L;
+    private YangInstanceIdentifier path;
+
+    protected AbstractReadPathTransactionRequestProxyV1() {
+        // For Externalizable
+    }
+
+    AbstractReadPathTransactionRequestProxyV1(final T request) {
+        super(request);
+        path = request.getPath();
+    }
+
+    @Override
+    public final void writeExternal(final ObjectOutput out) throws IOException {
+        super.writeExternal(out);
+        try (NormalizedNodeDataOutput nnout = NormalizedNodeInputOutput.newDataOutput(out)) {
+            nnout.writeYangInstanceIdentifier(path);
+        }
+    }
+
+    @Override
+    public final void readExternal(final ObjectInput in) throws ClassNotFoundException, IOException {
+        super.readExternal(in);
+        path = NormalizedNodeInputOutput.newDataInput(in).readYangInstanceIdentifier();
+    }
+
+    @Override
+    protected final T createReadRequest(final TransactionIdentifier target, final long sequence,
+            final ActorRef replyTo, final boolean snapshotOnly) {
+        return createReadPathRequest(target, sequence, replyTo, path, snapshotOnly);
+    }
+
+    abstract T createReadPathRequest(TransactionIdentifier target, long sequence, ActorRef replyTo,
+            YangInstanceIdentifier requestPath, boolean snapshotOnly);
+}
index c0d47145708c5e540e4e2a4fdff9ddcc4b48ad4c..3fc4821edf99e64eed0feec7b983b1015fc86329 100644 (file)
@@ -10,15 +10,11 @@ package org.opendaylight.controller.cluster.access.commands;
 import akka.actor.ActorRef;
 import com.google.common.annotations.Beta;
 import com.google.common.base.MoreObjects.ToStringHelper;
-import com.google.common.base.Preconditions;
-import javax.annotation.Nonnull;
 import org.opendaylight.controller.cluster.access.ABIVersion;
 import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier;
-import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 
 /**
- * Abstract base class for {@link TransactionRequest}s accessing data as visible in the isolated context of a particular
- * transaction. The path of the data being accessed is returned via {@link #getPath()}.
+ * Abstract base class for {@link TransactionRequest}s accessing transaction state without modifying it.
  *
  * <p>
  * This class is visible outside of this package for the purpose of allowing common instanceof checks
@@ -32,34 +28,27 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 public abstract class AbstractReadTransactionRequest<T extends AbstractReadTransactionRequest<T>>
         extends TransactionRequest<T> {
     private static final long serialVersionUID = 1L;
-    private final YangInstanceIdentifier path;
+
     private final boolean snapshotOnly;
 
     AbstractReadTransactionRequest(final TransactionIdentifier identifier, final long sequence, final ActorRef replyTo,
-        final YangInstanceIdentifier path, final boolean snapshotOnly) {
+        final boolean snapshotOnly) {
         super(identifier, sequence, replyTo);
-        this.path = Preconditions.checkNotNull(path);
         this.snapshotOnly = snapshotOnly;
     }
 
     AbstractReadTransactionRequest(final T request, final ABIVersion version) {
         super(request, version);
-        this.path = request.getPath();
         this.snapshotOnly = request.isSnapshotOnly();
     }
 
-    @Nonnull
-    public final YangInstanceIdentifier getPath() {
-        return path;
-    }
-
     public final boolean isSnapshotOnly() {
         return snapshotOnly;
     }
 
     @Override
     protected ToStringHelper addToStringAttributes(final ToStringHelper toStringHelper) {
-        return super.addToStringAttributes(toStringHelper).add("path", path);
+        return super.addToStringAttributes(toStringHelper).add("snapshotOnly", snapshotOnly);
     }
 
     @Override
index 51c60e18c6ee71195fba51b65388827efe9b2455..49742d83414276d254794c1ddb4aefefcc8b0540 100644 (file)
@@ -12,9 +12,6 @@ import java.io.IOException;
 import java.io.ObjectInput;
 import java.io.ObjectOutput;
 import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier;
-import org.opendaylight.controller.cluster.datastore.node.utils.stream.NormalizedNodeDataOutput;
-import org.opendaylight.controller.cluster.datastore.node.utils.stream.NormalizedNodeInputOutput;
-import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 
 /**
  * Abstract base class for serialization proxies associated with {@link AbstractReadTransactionRequest}s. It implements
@@ -27,7 +24,6 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 abstract class AbstractReadTransactionRequestProxyV1<T extends AbstractReadTransactionRequest<T>>
         extends AbstractTransactionRequestProxy<T> {
     private static final long serialVersionUID = 1L;
-    private YangInstanceIdentifier path;
     private boolean snapshotOnly;
 
     protected AbstractReadTransactionRequestProxyV1() {
@@ -36,31 +32,25 @@ abstract class AbstractReadTransactionRequestProxyV1<T extends AbstractReadTrans
 
     AbstractReadTransactionRequestProxyV1(final T request) {
         super(request);
-        path = request.getPath();
         snapshotOnly = request.isSnapshotOnly();
     }
 
     @Override
-    public final void writeExternal(final ObjectOutput out) throws IOException {
+    public void writeExternal(final ObjectOutput out) throws IOException {
         super.writeExternal(out);
-        try (NormalizedNodeDataOutput nnout = NormalizedNodeInputOutput.newDataOutput(out)) {
-            nnout.writeYangInstanceIdentifier(path);
-        }
         out.writeBoolean(snapshotOnly);
     }
 
     @Override
-    public final void readExternal(final ObjectInput in) throws ClassNotFoundException, IOException {
+    public void readExternal(final ObjectInput in) throws ClassNotFoundException, IOException {
         super.readExternal(in);
-        path = NormalizedNodeInputOutput.newDataInput(in).readYangInstanceIdentifier();
         snapshotOnly = in.readBoolean();
     }
 
     @Override
     protected final T createRequest(final TransactionIdentifier target, final long sequence, final ActorRef replyTo) {
-        return createReadRequest(target, sequence, replyTo, path, snapshotOnly);
+        return createReadRequest(target, sequence, replyTo, snapshotOnly);
     }
 
-    abstract T createReadRequest(TransactionIdentifier target, long sequence, ActorRef replyTo,
-            YangInstanceIdentifier requestPath, boolean snapshotOnly);
+    abstract T createReadRequest(TransactionIdentifier target, long sequence, ActorRef replyTo, boolean snapshotOnly);
 }
index 1702f142b656de2115f2e8794a29e87e5aa65241..f5534a8fbcb9a82e133c10747c800101c135752a 100644 (file)
@@ -20,7 +20,7 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
  * @author Robert Varga
  */
 @Beta
-public final class ExistsTransactionRequest extends AbstractReadTransactionRequest<ExistsTransactionRequest> {
+public final class ExistsTransactionRequest extends AbstractReadPathTransactionRequest<ExistsTransactionRequest> {
     private static final long serialVersionUID = 1L;
 
     public ExistsTransactionRequest(@Nonnull final TransactionIdentifier identifier, final long sequence,
index 6a6ede13d47e1e84a1e8c972f05d59fe65ac787e..24299471558bb3be08144ce12ad3831d992414f3 100644 (file)
@@ -17,7 +17,8 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
  *
  * @author Robert Varga
  */
-final class ExistsTransactionRequestProxyV1 extends AbstractReadTransactionRequestProxyV1<ExistsTransactionRequest> {
+final class ExistsTransactionRequestProxyV1 extends
+        AbstractReadPathTransactionRequestProxyV1<ExistsTransactionRequest> {
     private static final long serialVersionUID = 1L;
 
     // checkstyle flags the public modifier as redundant however it is explicitly needed for Java serialization to
@@ -32,7 +33,7 @@ final class ExistsTransactionRequestProxyV1 extends AbstractReadTransactionReque
     }
 
     @Override
-    ExistsTransactionRequest createReadRequest(final TransactionIdentifier target, final long sequence,
+    ExistsTransactionRequest createReadPathRequest(final TransactionIdentifier target, final long sequence,
             final ActorRef replyTo, final YangInstanceIdentifier path, final boolean snapshotOnly) {
         return new ExistsTransactionRequest(target, sequence, replyTo, path, snapshotOnly);
     }
index 59faae42ccb82dd3042c1b6b6653a8923369168b..ffc0a68b8912481bfeca882545ff3626469025e0 100644 (file)
@@ -19,18 +19,28 @@ import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier
  *
  * @author Robert Varga
  */
-public final class IncrementTransactionSequenceRequest extends TransactionRequest<IncrementTransactionSequenceRequest> {
+public final class IncrementTransactionSequenceRequest extends
+        AbstractReadTransactionRequest<IncrementTransactionSequenceRequest> {
     private static final long serialVersionUID = 1L;
 
     private final long increment;
 
     public IncrementTransactionSequenceRequest(final TransactionIdentifier identifier, final long sequence,
-            final ActorRef replyTo, final long increment) {
-        super(identifier, sequence, replyTo);
+            final ActorRef replyTo, final boolean snapshotOnly, final long increment) {
+        super(identifier, sequence, replyTo, snapshotOnly);
         Preconditions.checkArgument(increment >= 0);
         this.increment = increment;
     }
 
+    /**
+     * Return the sequence increment beyond this request's sequence.
+     *
+     * @return Sequence increment, guaranteed to be non-negative.
+     */
+    public long getIncrement() {
+        return increment;
+    }
+
     @Override
     protected IncrementTransactionSequenceRequestProxyV1 externalizableProxy(final ABIVersion version) {
         return new IncrementTransactionSequenceRequestProxyV1(this);
@@ -40,13 +50,4 @@ public final class IncrementTransactionSequenceRequest extends TransactionReques
     protected IncrementTransactionSequenceRequest cloneAsVersion(final ABIVersion targetVersion) {
         return this;
     }
-
-    /**
-     * Return the sequence increment beyond this request's sequence.
-     *
-     * @return Sequence increment, guaranteed to be non-negative.
-     */
-    public long getIncrement() {
-        return increment;
-    }
 }
index f9f8854d3b0a806e8b0d8d2c7f107a4e881ab745..da1659e1fe586acc98fea742a7fce376d8afdca7 100644 (file)
@@ -15,7 +15,7 @@ import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier
 import org.opendaylight.yangtools.concepts.WritableObjects;
 
 final class IncrementTransactionSequenceRequestProxyV1
-        extends AbstractTransactionRequestProxy<IncrementTransactionSequenceRequest> {
+        extends AbstractReadTransactionRequestProxyV1<IncrementTransactionSequenceRequest> {
     private long increment;
 
     // checkstyle flags the public modifier as redundant however it is explicitly needed for Java serialization to
@@ -43,8 +43,8 @@ final class IncrementTransactionSequenceRequestProxyV1
     }
 
     @Override
-    protected IncrementTransactionSequenceRequest createRequest(final TransactionIdentifier target, final long sequence,
-            final ActorRef replyToActor) {
-        return new IncrementTransactionSequenceRequest(target, sequence, replyToActor, increment);
+    IncrementTransactionSequenceRequest createReadRequest(final TransactionIdentifier target, final long sequence,
+            final ActorRef replyToActor, final boolean snapshotOnly) {
+        return new IncrementTransactionSequenceRequest(target, sequence, replyToActor, snapshotOnly, increment);
     }
 }
index a6bc7014290dc1aa26fdaab49a40819d1f2dff01..4eb5d1adc7e9c0e8aab35a0fd687ca4432793f87 100644 (file)
@@ -20,7 +20,7 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
  * @author Robert Varga
  */
 @Beta
-public final class ReadTransactionRequest extends AbstractReadTransactionRequest<ReadTransactionRequest> {
+public final class ReadTransactionRequest extends AbstractReadPathTransactionRequest<ReadTransactionRequest> {
     private static final long serialVersionUID = 1L;
 
     public ReadTransactionRequest(@Nonnull final TransactionIdentifier identifier, final long sequence,
index 153343d9ff430301a1e231db6f2e16a8a5050e36..a83b6bcaacd2dd4ba7a0eddbcab29a171c9664e4 100644 (file)
@@ -17,7 +17,7 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
  *
  * @author Robert Varga
  */
-final class ReadTransactionRequestProxyV1 extends AbstractReadTransactionRequestProxyV1<ReadTransactionRequest> {
+final class ReadTransactionRequestProxyV1 extends AbstractReadPathTransactionRequestProxyV1<ReadTransactionRequest> {
     private static final long serialVersionUID = 1L;
 
     // checkstyle flags the public modifier as redundant however it is explicitly needed for Java serialization to
@@ -32,7 +32,7 @@ final class ReadTransactionRequestProxyV1 extends AbstractReadTransactionRequest
     }
 
     @Override
-    ReadTransactionRequest createReadRequest(final TransactionIdentifier target, final long sequence,
+    ReadTransactionRequest createReadPathRequest(final TransactionIdentifier target, final long sequence,
             final ActorRef replyTo, final YangInstanceIdentifier path, final boolean snapshotOnly) {
         return new ReadTransactionRequest(target, sequence, replyTo, path, snapshotOnly);
     }
index 376e13f4b867151fc094113296983ef5c1822adf..a922787ea42cc047fb7ed2caf0cfa1441f02b6af 100644 (file)
@@ -12,7 +12,7 @@ import org.junit.Assert;
 import org.junit.Test;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 
-public abstract class AbstractReadTransactionRequestTest<T extends AbstractReadTransactionRequest>
+public abstract class AbstractReadTransactionRequestTest<T extends AbstractReadPathTransactionRequest>
         extends AbstractTransactionRequestTest {
     protected static final YangInstanceIdentifier PATH = YangInstanceIdentifier.EMPTY;
     protected static final boolean SNAPSHOT_ONLY = true;
index 8e4c757d33767877bb2065a3de45ab201d1bfaf7..2661ea82e9fb30a65ea6fb656cc03fa944f96a1b 100644 (file)
@@ -537,7 +537,8 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
                     Verify.verify(obj instanceof IncrementSequence);
                     final IncrementSequence increment = (IncrementSequence) obj;
                     successor.replayRequest(new IncrementTransactionSequenceRequest(getIdentifier(),
-                        increment.getSequence(), localActor(), increment.getDelta()), resp -> { }, now);
+                        increment.getSequence(), localActor(), isSnapshotOnly(), increment.getDelta()), resp -> { },
+                        now);
                     LOG.debug("Incrementing sequence {} to successor {}", obj, successor);
                 }
             }
index a70e6c5b6cb2f238e20239626de37fed6b0c56c9..829be478f00a6210c88a027547d1fe6a76ec135a 100644 (file)
@@ -497,7 +497,7 @@ final class RemoteProxyTransaction extends AbstractProxyTransaction {
             final IncrementTransactionSequenceRequest req = (IncrementTransactionSequenceRequest) request;
             ensureFlushedBuider(optTicks);
             enqueueRequest(new IncrementTransactionSequenceRequest(getIdentifier(), nextSequence(), localActor(),
-                req.getIncrement()), callback, enqueuedTicks);
+                snapshotOnly, req.getIncrement()), callback, enqueuedTicks);
             incrementSequence(req.getIncrement());
         } else {
             throw new IllegalArgumentException("Unhandled request {}" + request);