From 7708d30b155169f6226b09973d6ff457214ec75e Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Mon, 3 Apr 2023 21:10:24 +0200 Subject: [PATCH] Centralize DataTree operations Centralize taking the snapshot and creating a modifications to the extent possible. Change-Id: Ifd2632f583230046686c97cd1b07fefd06266451 Signed-off-by: Robert Varga --- .../ReadOnlyShardDataTreeTransaction.java | 2 +- .../ReadWriteShardDataTreeTransaction.java | 8 ++-- .../cluster/datastore/ShardDataTree.java | 41 +++++++++++-------- .../ShardDataTreeTransactionChain.java | 10 +++-- .../datastore/StandaloneFrontendHistory.java | 6 +-- 5 files changed, 36 insertions(+), 31 deletions(-) diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ReadOnlyShardDataTreeTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ReadOnlyShardDataTreeTransaction.java index 0906995fca..28042ecc3d 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ReadOnlyShardDataTreeTransaction.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ReadOnlyShardDataTreeTransaction.java @@ -12,7 +12,7 @@ import org.opendaylight.yangtools.yang.data.tree.api.DataTreeSnapshot; final class ReadOnlyShardDataTreeTransaction extends AbstractShardDataTreeTransaction { ReadOnlyShardDataTreeTransaction(final ShardDataTreeTransactionParent parent, final TransactionIdentifier id, - final DataTreeSnapshot snapshot) { + final DataTreeSnapshot snapshot) { super(parent, id, snapshot); } } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ReadWriteShardDataTreeTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ReadWriteShardDataTreeTransaction.java index 7d0cbb0cd6..b55d24ac8b 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ReadWriteShardDataTreeTransaction.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ReadWriteShardDataTreeTransaction.java @@ -7,21 +7,21 @@ */ package org.opendaylight.controller.cluster.datastore; -import com.google.common.base.Preconditions; +import static com.google.common.base.Preconditions.checkState; + import java.util.Optional; import java.util.SortedSet; import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier; import org.opendaylight.yangtools.yang.data.tree.api.DataTreeModification; public final class ReadWriteShardDataTreeTransaction extends AbstractShardDataTreeTransaction { - ReadWriteShardDataTreeTransaction(final ShardDataTreeTransactionParent parent, final TransactionIdentifier id, - final DataTreeModification modification) { + final DataTreeModification modification) { super(parent, id, modification); } ShardDataTreeCohort ready(final Optional> participatingShardNames) { - Preconditions.checkState(close(), "Transaction is already closed"); + checkState(close(), "Transaction is already closed"); return getParent().finishTransaction(this, participatingShardNames); } } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java index 7ad721fc90..c4bbe5b411 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java @@ -237,7 +237,7 @@ public class ShardDataTree extends ShardDataTreeTransactionParent { * @return A state snapshot */ @NonNull ShardDataTreeSnapshot takeStateSnapshot() { - final NormalizedNode rootNode = dataTree.takeSnapshot().readNode(YangInstanceIdentifier.empty()).get(); + final NormalizedNode rootNode = takeSnapshot().readNode(YangInstanceIdentifier.empty()).get(); final Builder>, ShardDataTreeSnapshotMetadata> metaBuilder = ImmutableMap.builder(); @@ -279,7 +279,7 @@ public class ShardDataTree extends ShardDataTreeTransactionParent { } } - final DataTreeModification unwrapped = dataTree.takeSnapshot().newModification(); + final DataTreeModification unwrapped = newModification(); final DataTreeModification mod = wrapper.apply(unwrapped); // delete everything first mod.delete(YangInstanceIdentifier.empty()); @@ -335,7 +335,7 @@ public class ShardDataTree extends ShardDataTreeTransactionParent { @SuppressWarnings("checkstyle:IllegalCatch") private void applyRecoveryCandidate(final CommitTransactionPayload payload) throws IOException { final Entry entry = payload.acquireCandidate(); - final DataTreeModification unwrapped = dataTree.takeSnapshot().newModification(); + final DataTreeModification unwrapped = newModification(); final PruningDataTreeModification mod = createPruningModification(unwrapped, NormalizedNodeStreamVersion.MAGNESIUM.compareTo(entry.getValue().getVersion()) > 0); @@ -401,7 +401,7 @@ public class ShardDataTree extends ShardDataTreeTransactionParent { final TransactionIdentifier identifier = entry.getKey(); LOG.debug("{}: Applying foreign transaction {}", logContext, identifier); - final DataTreeModification mod = dataTree.takeSnapshot().newModification(); + final DataTreeModification mod = newModification(); // TODO: check version here, which will enable us to perform forward-compatibility transformations DataTreeCandidates.applyToModification(mod, entry.getValue().getCandidate()); mod.ready(); @@ -618,24 +618,29 @@ public class ShardDataTree extends ShardDataTreeTransactionParent { return chain; } - final ReadOnlyShardDataTreeTransaction newReadOnlyTransaction(final TransactionIdentifier txId) { + final @NonNull ReadOnlyShardDataTreeTransaction newReadOnlyTransaction(final TransactionIdentifier txId) { shard.getShardMBean().incrementReadOnlyTransactionCount(); final var historyId = txId.getHistoryId(); - if (historyId.getHistoryId() == 0) { - return new ReadOnlyShardDataTreeTransaction(this, txId, dataTree.takeSnapshot()); - } - return ensureTransactionChain(historyId, null).newReadOnlyTransaction(txId); + return historyId.getHistoryId() == 0 ? newStandaloneReadOnlyTransaction(txId) + : ensureTransactionChain(historyId, null).newReadOnlyTransaction(txId); + } + + final @NonNull ReadOnlyShardDataTreeTransaction newStandaloneReadOnlyTransaction(final TransactionIdentifier txId) { + return new ReadOnlyShardDataTreeTransaction(this, txId, takeSnapshot()); } - final ReadWriteShardDataTreeTransaction newReadWriteTransaction(final TransactionIdentifier txId) { + final @NonNull ReadWriteShardDataTreeTransaction newReadWriteTransaction(final TransactionIdentifier txId) { shard.getShardMBean().incrementReadWriteTransactionCount(); final var historyId = txId.getHistoryId(); - if (historyId.getHistoryId() == 0) { - return new ReadWriteShardDataTreeTransaction(this, txId, dataTree.takeSnapshot().newModification()); - } - return ensureTransactionChain(historyId, null).newReadWriteTransaction(txId); + return historyId.getHistoryId() == 0 ? newStandaloneReadWriteTransaction(txId) + : ensureTransactionChain(historyId, null).newReadWriteTransaction(txId); + } + + final @NonNull ReadWriteShardDataTreeTransaction newStandaloneReadWriteTransaction( + final TransactionIdentifier txId) { + return new ReadWriteShardDataTreeTransaction(this, txId, newModification()); } @VisibleForTesting @@ -728,8 +733,8 @@ public class ShardDataTree extends ShardDataTreeTransactionParent { } final Optional readCurrentData() { - return dataTree.takeSnapshot().readNode(YangInstanceIdentifier.empty()) - .map(state -> DataTreeCandidates.fromNormalizedNode(YangInstanceIdentifier.empty(), state)); + return readNode(YangInstanceIdentifier.empty()) + .map(state -> DataTreeCandidates.fromNormalizedNode(YangInstanceIdentifier.empty(), state)); } final void registerTreeChangeListener(final YangInstanceIdentifier path, final DOMDataTreeChangeListener listener, @@ -775,7 +780,7 @@ public class ShardDataTree extends ShardDataTreeTransactionParent { @VisibleForTesting public final Optional readNode(final YangInstanceIdentifier path) { - return dataTree.takeSnapshot().readNode(path); + return takeSnapshot().readNode(path); } final DataTreeSnapshot takeSnapshot() { @@ -784,7 +789,7 @@ public class ShardDataTree extends ShardDataTreeTransactionParent { @VisibleForTesting final DataTreeModification newModification() { - return dataTree.takeSnapshot().newModification(); + return takeSnapshot().newModification(); } final Collection getAndClearPendingTransactions() { diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionChain.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionChain.java index 72f700c744..6be13ae129 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionChain.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTransactionChain.java @@ -13,6 +13,7 @@ import static java.util.Objects.requireNonNull; import com.google.common.base.MoreObjects; import java.util.Optional; import java.util.SortedSet; +import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.controller.cluster.access.concepts.LocalHistoryIdentifier; import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier; import org.opendaylight.yangtools.concepts.Identifiable; @@ -53,19 +54,20 @@ final class ShardDataTreeTransactionChain extends ShardDataTreeTransactionParent return previousTx.getSnapshot(); } - ReadOnlyShardDataTreeTransaction newReadOnlyTransaction(final TransactionIdentifier txId) { + @NonNull ReadOnlyShardDataTreeTransaction newReadOnlyTransaction(final TransactionIdentifier txId) { final DataTreeSnapshot snapshot = getSnapshot(); LOG.debug("Allocated read-only transaction {} snapshot {}", txId, snapshot); return new ReadOnlyShardDataTreeTransaction(this, txId, snapshot); } - ReadWriteShardDataTreeTransaction newReadWriteTransaction(final TransactionIdentifier txId) { + @NonNull ReadWriteShardDataTreeTransaction newReadWriteTransaction(final TransactionIdentifier txId) { final DataTreeSnapshot snapshot = getSnapshot(); LOG.debug("Allocated read-write transaction {} snapshot {}", txId, snapshot); - openTransaction = new ReadWriteShardDataTreeTransaction(this, txId, snapshot.newModification()); - return openTransaction; + final var ret = new ReadWriteShardDataTreeTransaction(this, txId, snapshot.newModification()); + openTransaction = ret; + return ret; } void close() { diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/StandaloneFrontendHistory.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/StandaloneFrontendHistory.java index df13cd1368..57c680da2a 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/StandaloneFrontendHistory.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/StandaloneFrontendHistory.java @@ -60,14 +60,12 @@ final class StandaloneFrontendHistory extends AbstractFrontendHistory { @Override FrontendTransaction createOpenSnapshot(final TransactionIdentifier id) { - return FrontendReadOnlyTransaction.create(this, - new ReadOnlyShardDataTreeTransaction(tree, id, tree.takeSnapshot())); + return FrontendReadOnlyTransaction.create(this, tree.newStandaloneReadOnlyTransaction(id)); } @Override FrontendTransaction createOpenTransaction(final TransactionIdentifier id) { - return FrontendReadWriteTransaction.createOpen(this, - new ReadWriteShardDataTreeTransaction(tree, id, tree.takeSnapshot().newModification())); + return FrontendReadWriteTransaction.createOpen(this, tree.newStandaloneReadWriteTransaction(id)); } @Override -- 2.36.6