From 7441c30c82b4007a09942839baca5c84de9e50b3 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Tue, 19 Apr 2022 22:53:18 +0200 Subject: [PATCH] Handle the case when newModification() fails When we are chaining DataTreeModification on top of each other, it is possible that we have an invalid modification, which fails to apply. In that case there is nothing we can do, as we cannot establish a baseline. Handle this case by allocating a specialized modification implementation, which allows close to no interactions. JIRA: CONTROLLER-2039 Change-Id: I9b55da7ccc89c9aeae641a049397d2120e292a9c Signed-off-by: Robert Varga --- .../actors/dds/AbstractProxyTransaction.java | 1 + .../dds/FailedDataTreeModification.java | 84 +++++++++++++++++++ .../FailedDataTreeModificationException.java | 21 +++++ .../actors/dds/LocalProxyTransaction.java | 37 ++++++-- .../dds/LocalReadWriteProxyTransaction.java | 32 ++++++- 5 files changed, 166 insertions(+), 9 deletions(-) create mode 100644 opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/FailedDataTreeModification.java create mode 100644 opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/FailedDataTreeModificationException.java 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 11027e4ce8..471e322a81 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 @@ -68,6 +68,7 @@ import org.slf4j.LoggerFactory; * * @author Robert Varga */ +// FIXME: sealed when we have JDK17+ abstract class AbstractProxyTransaction implements Identifiable { /** * Marker object used instead of read-type of requests, which are satisfied only once. This has a lower footprint diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/FailedDataTreeModification.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/FailedDataTreeModification.java new file mode 100644 index 0000000000..50cd0bacd0 --- /dev/null +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/FailedDataTreeModification.java @@ -0,0 +1,84 @@ +/* + * Copyright (c) 2022 PANTHEON.tech, 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.databroker.actors.dds; + +import static java.util.Objects.requireNonNull; + +import java.util.Optional; +import org.eclipse.jdt.annotation.NonNull; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; +import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; +import org.opendaylight.yangtools.yang.data.tree.api.CursorAwareDataTreeModification; +import org.opendaylight.yangtools.yang.data.tree.api.DataTreeModificationCursor; +import org.opendaylight.yangtools.yang.data.tree.api.DataTreeSnapshot; +import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext; +import org.opendaylight.yangtools.yang.model.spi.AbstractEffectiveModelContextProvider; + +/** + * A {@link CursorAwareDataTreeModification} which does not really do anything and throws an + * {@link FailedDataTreeModificationException} for most of its operations. Used in case we when + * {@link DataTreeSnapshot#newModification()} fails, see {@link LocalReadWriteProxyTransaction} for details. Surrounding + * code should guard against invocation of most of these methods. + */ +final class FailedDataTreeModification extends AbstractEffectiveModelContextProvider + implements CursorAwareDataTreeModification { + private final @NonNull Exception cause; + + FailedDataTreeModification(final EffectiveModelContext context, final Exception cause) { + super(context); + this.cause = requireNonNull(cause); + } + + @NonNull Exception cause() { + return cause; + } + + @Override + public void delete(final YangInstanceIdentifier path) { + throw ex(); + } + + @Override + public void merge(final YangInstanceIdentifier path, final NormalizedNode data) { + throw ex(); + } + + @Override + public void write(final YangInstanceIdentifier path, final NormalizedNode data) { + throw ex(); + } + + @Override + public void ready() { + // No-op + } + + @Override + public void applyToCursor(final DataTreeModificationCursor cursor) { + throw ex(); + } + + @Override + public Optional readNode(final YangInstanceIdentifier path) { + throw ex(); + } + + @Override + public CursorAwareDataTreeModification newModification() { + throw new UnsupportedOperationException(); + } + + @Override + public Optional openCursor(final YangInstanceIdentifier path) { + throw ex(); + } + + private @NonNull FailedDataTreeModificationException ex() { + return new FailedDataTreeModificationException(cause); + } +} diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/FailedDataTreeModificationException.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/FailedDataTreeModificationException.java new file mode 100644 index 0000000000..5f860a00d3 --- /dev/null +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/FailedDataTreeModificationException.java @@ -0,0 +1,21 @@ +/* + * Copyright (c) 2022 PANTHEON.tech, 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.databroker.actors.dds; + +import static java.util.Objects.requireNonNull; + +/** + * A box {@link RuntimeException} thrown by {@link FailedDataTreeModification} from its user-facing methods. + */ +final class FailedDataTreeModificationException extends RuntimeException { + private static final long serialVersionUID = 1L; + + FailedDataTreeModificationException(final Exception cause) { + super(null, requireNonNull(cause), false, false); + } +} diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalProxyTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalProxyTransaction.java index feddd5fc5a..93c8c9350e 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalProxyTransaction.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalProxyTransaction.java @@ -26,8 +26,10 @@ import org.opendaylight.controller.cluster.access.commands.ReadTransactionSucces import org.opendaylight.controller.cluster.access.commands.TransactionPurgeRequest; import org.opendaylight.controller.cluster.access.commands.TransactionRequest; import org.opendaylight.controller.cluster.access.concepts.Response; +import org.opendaylight.controller.cluster.access.concepts.RuntimeRequestException; import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier; import org.opendaylight.controller.cluster.datastore.util.AbstractDataTreeModificationCursor; +import org.opendaylight.mdsal.common.api.ReadFailedException; import org.opendaylight.yangtools.util.concurrent.FluentFutures; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; @@ -52,6 +54,7 @@ import org.slf4j.LoggerFactory; * * @author Robert Varga */ +// FIXME: sealed when we have JDK17+ abstract class LocalProxyTransaction extends AbstractProxyTransaction { private static final Logger LOG = LoggerFactory.getLogger(LocalProxyTransaction.class); @@ -77,12 +80,24 @@ abstract class LocalProxyTransaction extends AbstractProxyTransaction { @Override FluentFuture doExists(final YangInstanceIdentifier path) { - return FluentFutures.immediateBooleanFluentFuture(readOnlyView().readNode(path).isPresent()); + final boolean result; + try { + result = readOnlyView().readNode(path).isPresent(); + } catch (FailedDataTreeModificationException e) { + return FluentFutures.immediateFailedFluentFuture(ReadFailedException.MAPPER.apply(e)); + } + return FluentFutures.immediateBooleanFluentFuture(result); } @Override FluentFuture> doRead(final YangInstanceIdentifier path) { - return FluentFutures.immediateFluentFuture(readOnlyView().readNode(path)); + final Optional result; + try { + result = readOnlyView().readNode(path); + } catch (FailedDataTreeModificationException e) { + return FluentFutures.immediateFailedFluentFuture(ReadFailedException.MAPPER.apply(e)); + } + return FluentFutures.immediateFluentFuture(result); } @Override @@ -140,14 +155,24 @@ abstract class LocalProxyTransaction extends AbstractProxyTransaction { @NonNull Response handleExistsRequest(final @NonNull DataTreeSnapshot snapshot, final @NonNull ExistsTransactionRequest request) { - return new ExistsTransactionSuccess(request.getTarget(), request.getSequence(), - snapshot.readNode(request.getPath()).isPresent()); + try { + return new ExistsTransactionSuccess(request.getTarget(), request.getSequence(), + snapshot.readNode(request.getPath()).isPresent()); + } catch (FailedDataTreeModificationException e) { + return request.toRequestFailure(new RuntimeRequestException("Failed to access data", + ReadFailedException.MAPPER.apply(e))); + } } @NonNull Response handleReadRequest(final @NonNull DataTreeSnapshot snapshot, final @NonNull ReadTransactionRequest request) { - return new ReadTransactionSuccess(request.getTarget(), request.getSequence(), - snapshot.readNode(request.getPath())); + try { + return new ReadTransactionSuccess(request.getTarget(), request.getSequence(), + snapshot.readNode(request.getPath())); + } catch (FailedDataTreeModificationException e) { + return request.toRequestFailure(new RuntimeRequestException("Failed to access data", + ReadFailedException.MAPPER.apply(e))); + } } private boolean handleReadRequest(final TransactionRequest request, final Consumer> callback) { diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalReadWriteProxyTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalReadWriteProxyTransaction.java index a8a38a8497..7fcaf4e953 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalReadWriteProxyTransaction.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalReadWriteProxyTransaction.java @@ -91,10 +91,26 @@ final class LocalReadWriteProxyTransaction extends LocalProxyTransaction { */ private Exception recordedFailure; + @SuppressWarnings("checkstyle:IllegalCatch") LocalReadWriteProxyTransaction(final ProxyHistory parent, final TransactionIdentifier identifier, final DataTreeSnapshot snapshot) { super(parent, identifier, false); - modification = (CursorAwareDataTreeModification) snapshot.newModification(); + + if (snapshot instanceof FailedDataTreeModification) { + final var failed = (FailedDataTreeModification) snapshot; + recordedFailure = failed.cause(); + modification = failed; + } else { + CursorAwareDataTreeModification mod; + try { + mod = (CursorAwareDataTreeModification) snapshot.newModification(); + } catch (Exception e) { + LOG.debug("Failed to instantiate modification for {}", identifier, e); + recordedFailure = e; + mod = new FailedDataTreeModification(snapshot.getEffectiveModelContext(), e); + } + modification = mod; + } } LocalReadWriteProxyTransaction(final ProxyHistory parent, final TransactionIdentifier identifier) { @@ -406,8 +422,18 @@ final class LocalReadWriteProxyTransaction extends LocalProxyTransaction { // Rebase old modification on new data tree. final CursorAwareDataTreeModification mod = getModification(); - try (DataTreeModificationCursor cursor = mod.openCursor()) { - request.getModification().applyToCursor(cursor); + if (!(mod instanceof FailedDataTreeModification)) { + request.getDelayedFailure().ifPresentOrElse(failure -> { + if (recordedFailure == null) { + recordedFailure = failure; + } else { + recordedFailure.addSuppressed(failure); + } + }, () -> { + try (DataTreeModificationCursor cursor = mod.openCursor()) { + request.getModification().applyToCursor(cursor); + } + }); } if (markSealed()) { -- 2.36.6