From 63e6ab3f36e57954baf391855541cf3d42d38a0f Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Wed, 17 May 2017 16:56:57 +0200 Subject: [PATCH] BUG-8402: Record modification failures When a modification fails to apply, we must record the resulting failure, as we have partially applied the state and hence should never attempt to try to do it again even if the client retransmits the request. Furthermore we should stop responding to any subsequent requests including reads, as our responses are not accurate anyway (and the requests may have been enqueued before the client saw the failure). Enqueue the failure and respond to all subsequent requests with it, forcing the transaction to fail the canCommit() phase. Change-Id: I1d25f1b3a688e02f8a69f54f22a5d6d2dd43339c Signed-off-by: Robert Varga --- .../commands/TransactionModification.java | 6 +++ .../FrontendReadOnlyTransaction.java | 2 +- .../FrontendReadWriteTransaction.java | 8 ++-- .../datastore/FrontendTransaction.java | 37 +++++++++++++++++-- 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/TransactionModification.java b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/TransactionModification.java index 413f4fbe53..d71142201c 100644 --- a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/TransactionModification.java +++ b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/TransactionModification.java @@ -8,6 +8,7 @@ package org.opendaylight.controller.cluster.access.commands; import com.google.common.annotations.Beta; +import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; import java.io.IOException; import org.opendaylight.controller.cluster.datastore.node.utils.stream.NormalizedNodeDataInput; @@ -38,6 +39,11 @@ public abstract class TransactionModification { return path; } + @Override + public final String toString() { + return MoreObjects.toStringHelper(this).add("path", path).toString(); + } + abstract byte getType(); void writeTo(final NormalizedNodeDataOutput out) throws IOException { diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadOnlyTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadOnlyTransaction.java index e5680c500c..284acbab0b 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadOnlyTransaction.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadOnlyTransaction.java @@ -51,7 +51,7 @@ final class FrontendReadOnlyTransaction extends FrontendTransaction { // Sequence has already been checked @Override - @Nullable TransactionSuccess handleRequest(final TransactionRequest request, final RequestEnvelope envelope, + @Nullable TransactionSuccess doHandleRequest(final TransactionRequest request, final RequestEnvelope envelope, final long now) throws RequestException { if (request instanceof ExistsTransactionRequest) { return handleExistsTransaction((ExistsTransactionRequest) request); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadWriteTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadWriteTransaction.java index 9f1df4c8ab..45edfc9333 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadWriteTransaction.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadWriteTransaction.java @@ -84,7 +84,7 @@ final class FrontendReadWriteTransaction extends FrontendTransaction { // Sequence has already been checked @Override - @Nullable TransactionSuccess handleRequest(final TransactionRequest request, final RequestEnvelope envelope, + @Nullable TransactionSuccess doHandleRequest(final TransactionRequest request, final RequestEnvelope envelope, final long now) throws RequestException { if (request instanceof ModifyTransactionRequest) { return handleModifyTransaction((ModifyTransactionRequest) request, envelope, now); @@ -297,7 +297,7 @@ final class FrontendReadWriteTransaction extends FrontendTransaction { } else if (m instanceof TransactionMerge) { modification.merge(m.getPath(), ((TransactionMerge) m).getData()); } else { - LOG.warn("{}: ignoring unhandled modification {}", history().persistenceId(), m); + LOG.warn("{}: ignoring unhandled modification {}", persistenceId(), m); } } } @@ -324,7 +324,7 @@ final class FrontendReadWriteTransaction extends FrontendTransaction { coordinatedCommit(envelope, now); return null; default: - LOG.warn("{}: rejecting unsupported protocol {}", history().persistenceId(), maybeProto.get()); + LOG.warn("{}: rejecting unsupported protocol {}", persistenceId(), maybeProto.get()); throw new UnsupportedRequestException(request); } } @@ -334,7 +334,7 @@ final class FrontendReadWriteTransaction extends FrontendTransaction { // only once. if (readyCohort == null) { readyCohort = openTransaction.ready(); - LOG.debug("{}: transitioned {} to ready", history().persistenceId(), openTransaction.getIdentifier()); + LOG.debug("{}: transitioned {} to ready", persistenceId(), openTransaction.getIdentifier()); openTransaction = null; } } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendTransaction.java index 8846467b59..c0249fd000 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendTransaction.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendTransaction.java @@ -23,6 +23,8 @@ import org.opendaylight.controller.cluster.access.concepts.RequestException; import org.opendaylight.controller.cluster.access.concepts.RuntimeRequestException; import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier; import org.opendaylight.yangtools.concepts.Identifiable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Frontend common transaction state as observed by the shard leader. @@ -31,6 +33,8 @@ import org.opendaylight.yangtools.concepts.Identifiable; */ @NotThreadSafe abstract class FrontendTransaction implements Identifiable { + private static final Logger LOG = LoggerFactory.getLogger(FrontendTransaction.class); + private final AbstractFrontendHistory history; private final TransactionIdentifier id; @@ -44,6 +48,8 @@ abstract class FrontendTransaction implements Identifiable> replaySequence(final long sequence) throws RequestException { // Fast path check: if the requested sequence is the next request, bail early if (expectedSequence == sequence) { @@ -108,9 +118,30 @@ abstract class FrontendTransaction implements Identifiable handleRequest(TransactionRequest request, - RequestEnvelope envelope, long now) throws RequestException; + // Request order has already been checked by caller and replaySequence() + @SuppressWarnings("checkstyle:IllegalCatch") + final @Nullable TransactionSuccess handleRequest(final TransactionRequest request, + final RequestEnvelope envelope, final long now) throws RequestException { + if (previousFailure != null) { + LOG.debug("{}: Rejecting request {} due to previous failure", persistenceId(), request, previousFailure); + throw previousFailure; + } + + try { + return doHandleRequest(request, envelope, now); + } catch (RuntimeException e) { + /* + * The request failed to process, we should not attempt to ever apply it again. Furthermore we cannot + * accept any further requests from this connection, simply because the transaction state is undefined. + */ + LOG.debug("{}: Request {} failed to process", persistenceId(), request, e); + previousFailure = new RuntimeRequestException("Request " + request + " failed to process", e); + throw previousFailure; + } + } + + abstract @Nullable TransactionSuccess doHandleRequest(TransactionRequest request, RequestEnvelope envelope, + long now) throws RequestException; private void recordResponse(final long sequence, final Object response) { if (replayQueue.isEmpty()) { -- 2.36.6