From: Robert Varga Date: Tue, 23 May 2017 12:56:37 +0000 (+0200) Subject: BUG-8402: correctly propagate read-only bit X-Git-Tag: release/carbon-sr1~64 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=b24517538beb4f44e6a9a96e68e4bf48156b480f BUG-8402: correctly propagate read-only bit 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 --- 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 index 0000000000..fee3b03624 --- /dev/null +++ b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadPathTransactionRequest.java @@ -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()}. + * + *

+ * This class is visible outside of this package for the purpose of allowing common instanceof checks + * and simplified codepaths. + * + * @author Robert Varga + * + * @param Message type + */ +@Beta +public abstract class AbstractReadPathTransactionRequest> + extends AbstractReadTransactionRequest { + 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 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 index 0000000000..bafa3d9083 --- /dev/null +++ b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadPathTransactionRequestProxyV1.java @@ -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 Message type + */ +abstract class AbstractReadPathTransactionRequestProxyV1> + extends AbstractReadTransactionRequestProxyV1 { + 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); +} diff --git a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadTransactionRequest.java b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadTransactionRequest.java index c0d4714570..3fc4821edf 100644 --- a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadTransactionRequest.java +++ b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadTransactionRequest.java @@ -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. * *

* 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> extends TransactionRequest { 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 diff --git a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadTransactionRequestProxyV1.java b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadTransactionRequestProxyV1.java index 51c60e18c6..49742d8341 100644 --- a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadTransactionRequestProxyV1.java +++ b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/AbstractReadTransactionRequestProxyV1.java @@ -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> extends AbstractTransactionRequestProxy { private static final long serialVersionUID = 1L; - private YangInstanceIdentifier path; private boolean snapshotOnly; protected AbstractReadTransactionRequestProxyV1() { @@ -36,31 +32,25 @@ abstract class AbstractReadTransactionRequestProxyV1 { +public final class ExistsTransactionRequest extends AbstractReadPathTransactionRequest { private static final long serialVersionUID = 1L; public ExistsTransactionRequest(@Nonnull final TransactionIdentifier identifier, final long sequence, diff --git a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ExistsTransactionRequestProxyV1.java b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ExistsTransactionRequestProxyV1.java index 6a6ede13d4..2429947155 100644 --- a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ExistsTransactionRequestProxyV1.java +++ b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ExistsTransactionRequestProxyV1.java @@ -17,7 +17,8 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; * * @author Robert Varga */ -final class ExistsTransactionRequestProxyV1 extends AbstractReadTransactionRequestProxyV1 { +final class ExistsTransactionRequestProxyV1 extends + AbstractReadPathTransactionRequestProxyV1 { 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); } diff --git a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/IncrementTransactionSequenceRequest.java b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/IncrementTransactionSequenceRequest.java index 59faae42cc..ffc0a68b89 100644 --- a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/IncrementTransactionSequenceRequest.java +++ b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/IncrementTransactionSequenceRequest.java @@ -19,18 +19,28 @@ import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier * * @author Robert Varga */ -public final class IncrementTransactionSequenceRequest extends TransactionRequest { +public final class IncrementTransactionSequenceRequest extends + AbstractReadTransactionRequest { 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; - } } diff --git a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/IncrementTransactionSequenceRequestProxyV1.java b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/IncrementTransactionSequenceRequestProxyV1.java index f9f8854d3b..da1659e1fe 100644 --- a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/IncrementTransactionSequenceRequestProxyV1.java +++ b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/IncrementTransactionSequenceRequestProxyV1.java @@ -15,7 +15,7 @@ import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier import org.opendaylight.yangtools.concepts.WritableObjects; final class IncrementTransactionSequenceRequestProxyV1 - extends AbstractTransactionRequestProxy { + extends AbstractReadTransactionRequestProxyV1 { 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); } } diff --git a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ReadTransactionRequest.java b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ReadTransactionRequest.java index a6bc701429..4eb5d1adc7 100644 --- a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ReadTransactionRequest.java +++ b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ReadTransactionRequest.java @@ -20,7 +20,7 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; * @author Robert Varga */ @Beta -public final class ReadTransactionRequest extends AbstractReadTransactionRequest { +public final class ReadTransactionRequest extends AbstractReadPathTransactionRequest { private static final long serialVersionUID = 1L; public ReadTransactionRequest(@Nonnull final TransactionIdentifier identifier, final long sequence, diff --git a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ReadTransactionRequestProxyV1.java b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ReadTransactionRequestProxyV1.java index 153343d9ff..a83b6bcaac 100644 --- a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ReadTransactionRequestProxyV1.java +++ b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ReadTransactionRequestProxyV1.java @@ -17,7 +17,7 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; * * @author Robert Varga */ -final class ReadTransactionRequestProxyV1 extends AbstractReadTransactionRequestProxyV1 { +final class ReadTransactionRequestProxyV1 extends AbstractReadPathTransactionRequestProxyV1 { 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); } diff --git a/opendaylight/md-sal/cds-access-api/src/test/java/org/opendaylight/controller/cluster/access/commands/AbstractReadTransactionRequestTest.java b/opendaylight/md-sal/cds-access-api/src/test/java/org/opendaylight/controller/cluster/access/commands/AbstractReadTransactionRequestTest.java index 376e13f4b8..a922787ea4 100644 --- a/opendaylight/md-sal/cds-access-api/src/test/java/org/opendaylight/controller/cluster/access/commands/AbstractReadTransactionRequestTest.java +++ b/opendaylight/md-sal/cds-access-api/src/test/java/org/opendaylight/controller/cluster/access/commands/AbstractReadTransactionRequestTest.java @@ -12,7 +12,7 @@ import org.junit.Assert; import org.junit.Test; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; -public abstract class AbstractReadTransactionRequestTest +public abstract class AbstractReadTransactionRequestTest extends AbstractTransactionRequestTest { protected static final YangInstanceIdentifier PATH = YangInstanceIdentifier.EMPTY; protected static final boolean SNAPSHOT_ONLY = true; diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractProxyTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractProxyTransaction.java index 8e4c757d33..2661ea82e9 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractProxyTransaction.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractProxyTransaction.java @@ -537,7 +537,8 @@ abstract class AbstractProxyTransaction implements Identifiable { }, now); + increment.getSequence(), localActor(), isSnapshotOnly(), increment.getDelta()), resp -> { }, + now); LOG.debug("Incrementing sequence {} to successor {}", obj, successor); } } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/RemoteProxyTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/RemoteProxyTransaction.java index a70e6c5b6c..829be478f0 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/RemoteProxyTransaction.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/RemoteProxyTransaction.java @@ -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);