Translate uint values for old streams 42/87042/10
authorRobert Varga <robert.varga@pantheon.tech>
Sun, 19 Jan 2020 12:11:28 +0000 (13:11 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Mon, 20 Jan 2020 11:03:58 +0000 (12:03 +0100)
When we encounter a commit that was produced before Mg, we know
uints are upcasted and hence want to convert values. Same goes
for recovery snapshots.

JIRA: CONTROLLER-1923
Change-Id: Id22830abaf5adc2d9ca7f5d0233409a971cf0c01
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinator.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/persisted/CommitTransactionPayload.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/persisted/DataTreeCandidateInputOutput.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/persisted/ShardSnapshotState.java
opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeTest.java

index bbcc42c..a097b68 100644 (file)
@@ -59,10 +59,12 @@ import org.opendaylight.controller.cluster.datastore.persisted.CommitTransaction
 import org.opendaylight.controller.cluster.datastore.persisted.CreateLocalHistoryPayload;
 import org.opendaylight.controller.cluster.datastore.persisted.DataTreeCandidateInputOutput.DataTreeCandidateWithVersion;
 import org.opendaylight.controller.cluster.datastore.persisted.MetadataShardDataTreeSnapshot;
+import org.opendaylight.controller.cluster.datastore.persisted.PayloadVersion;
 import org.opendaylight.controller.cluster.datastore.persisted.PurgeLocalHistoryPayload;
 import org.opendaylight.controller.cluster.datastore.persisted.PurgeTransactionPayload;
 import org.opendaylight.controller.cluster.datastore.persisted.ShardDataTreeSnapshot;
 import org.opendaylight.controller.cluster.datastore.persisted.ShardDataTreeSnapshotMetadata;
+import org.opendaylight.controller.cluster.datastore.persisted.ShardSnapshotState;
 import org.opendaylight.controller.cluster.datastore.utils.DataTreeModificationOutput;
 import org.opendaylight.controller.cluster.datastore.utils.PruningDataTreeModification;
 import org.opendaylight.controller.cluster.raft.protobuff.client.messages.Payload;
@@ -84,6 +86,7 @@ import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeSnapshot;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeTip;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailedException;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.TreeType;
+import org.opendaylight.yangtools.yang.data.codec.binfmt.NormalizedNodeStreamVersion;
 import org.opendaylight.yangtools.yang.data.impl.schema.tree.InMemoryDataTreeFactory;
 import org.opendaylight.yangtools.yang.data.util.DataSchemaContextTree;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
@@ -295,15 +298,10 @@ public class ShardDataTree extends ShardDataTreeTransactionParent {
      * @throws DataValidationFailedException when the snapshot fails to apply
      */
     void applySnapshot(final @NonNull ShardDataTreeSnapshot snapshot) throws DataValidationFailedException {
+        // TODO: we should be taking ShardSnapshotState here and performing forward-compatibility translation
         applySnapshot(snapshot, UnaryOperator.identity());
     }
 
-    private PruningDataTreeModification wrapWithPruning(final DataTreeModification delegate) {
-        return new PruningDataTreeModification(delegate, dataTree,
-            // TODO: we should be able to reuse the pruner, provided we are not reentrant
-            ReusableNormalizedNodePruner.forDataSchemaContext(dataSchemaContext));
-    }
-
     /**
      * Apply a snapshot coming from recovery. This method does not assume the SchemaContexts match and performs data
      * pruning in an attempt to adjust the state to our current SchemaContext.
@@ -311,16 +309,31 @@ public class ShardDataTree extends ShardDataTreeTransactionParent {
      * @param snapshot Snapshot that needs to be applied
      * @throws DataValidationFailedException when the snapshot fails to apply
      */
-    void applyRecoverySnapshot(final @NonNull ShardDataTreeSnapshot snapshot) throws DataValidationFailedException {
-        applySnapshot(snapshot, this::wrapWithPruning);
+    void applyRecoverySnapshot(final @NonNull ShardSnapshotState snapshot) throws DataValidationFailedException {
+        // TODO: we should be able to reuse the pruner, provided we are not reentrant
+        ReusableNormalizedNodePruner pruner = ReusableNormalizedNodePruner.forDataSchemaContext(dataSchemaContext);
+        if (snapshot.needsMigration()) {
+            pruner = pruner.withUintAdaption();
+        }
+
+        // For lambda below
+        final ReusableNormalizedNodePruner finalPruner = pruner;
+        applySnapshot(snapshot.getSnapshot(),
+            delegate -> new PruningDataTreeModification(delegate, dataTree, finalPruner));
     }
 
     @SuppressWarnings("checkstyle:IllegalCatch")
     private void applyRecoveryCandidate(final CommitTransactionPayload payload) throws IOException {
         final Entry<TransactionIdentifier, DataTreeCandidateWithVersion> entry = payload.getCandidate();
         final DataTreeModification unwrapped = dataTree.takeSnapshot().newModification();
-        // FIXME: CONTROLLER-1923: examine version first
-        final PruningDataTreeModification mod = wrapWithPruning(unwrapped);
+
+        // TODO: we should be able to reuse the pruner, provided we are not reentrant
+        ReusableNormalizedNodePruner pruner = ReusableNormalizedNodePruner.forDataSchemaContext(dataSchemaContext);
+        if (NormalizedNodeStreamVersion.MAGNESIUM.compareTo(entry.getValue().getVersion()) > 0) {
+            pruner = pruner.withUintAdaption();
+        }
+
+        final PruningDataTreeModification mod = new PruningDataTreeModification(unwrapped, dataTree, pruner);
         DataTreeCandidates.applyToModification(mod, entry.getValue().getCandidate());
         mod.ready();
 
@@ -1027,7 +1040,7 @@ public class ShardDataTree extends ShardDataTreeTransactionParent {
         final TransactionIdentifier txId = cohort.getIdentifier();
         final Payload payload;
         try {
-            payload = CommitTransactionPayload.create(txId, candidate,
+            payload = CommitTransactionPayload.create(txId, candidate, PayloadVersion.current(),
                     shard.getDatastoreContext().getInitialPayloadSerializedBufferCapacity());
         } catch (IOException e) {
             LOG.error("{}: Failed to encode transaction {} candidate {}", logContext, txId, candidate, e);
index aeaad48..a6c56a5 100644 (file)
@@ -122,14 +122,15 @@ abstract class ShardRecoveryCoordinator implements RaftActorRecoveryCohort {
     public void applyRecoverySnapshot(final Snapshot.State snapshotState) {
         if (!(snapshotState instanceof ShardSnapshotState)) {
             log.debug("{}: applyRecoverySnapshot ignoring snapshot: {}", shardName, snapshotState);
+            return;
         }
 
         log.debug("{}: Applying recovered snapshot", shardName);
-
-        ShardDataTreeSnapshot shardSnapshot = ((ShardSnapshotState)snapshotState).getSnapshot();
+        final ShardSnapshotState shardSnapshotState = (ShardSnapshotState)snapshotState;
         try {
-            store.applyRecoverySnapshot(shardSnapshot);
+            store.applyRecoverySnapshot(shardSnapshotState);
         } catch (Exception e) {
+            final ShardDataTreeSnapshot shardSnapshot = shardSnapshotState.getSnapshot();
             final File f = writeRoot("snapshot", shardSnapshot.getRootNode().orElse(null));
             throw new IllegalStateException(String.format(
                     "%s: Failed to apply recovery snapshot %s. Node data was written to file %s",
index fed3483..af19d14 100644 (file)
@@ -26,6 +26,7 @@ import java.io.Serializable;
 import java.io.StreamCorruptedException;
 import java.util.AbstractMap.SimpleImmutableEntry;
 import java.util.Map.Entry;
+import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier;
 import org.opendaylight.controller.cluster.datastore.persisted.DataTreeCandidateInputOutput.DataTreeCandidateWithVersion;
 import org.opendaylight.controller.cluster.raft.protobuff.client.messages.IdentifiablePayload;
@@ -54,13 +55,13 @@ public abstract class CommitTransactionPayload extends IdentifiablePayload<Trans
 
     }
 
-    public static CommitTransactionPayload create(final TransactionIdentifier transactionId,
-            final DataTreeCandidate candidate, final int initialSerializedBufferCapacity) throws IOException {
-
+    public static @NonNull CommitTransactionPayload create(final TransactionIdentifier transactionId,
+            final DataTreeCandidate candidate, final PayloadVersion version, final int initialSerializedBufferCapacity)
+                    throws IOException {
         final ChunkedOutputStream cos = new ChunkedOutputStream(initialSerializedBufferCapacity);
         try (DataOutputStream dos = new DataOutputStream(cos)) {
             transactionId.writeTo(dos);
-            DataTreeCandidateInputOutput.writeDataTreeCandidate(dos, candidate);
+            DataTreeCandidateInputOutput.writeDataTreeCandidate(dos, version, candidate);
         }
 
         final Variant<byte[], ChunkedByteArray> source = cos.toVariant();
@@ -69,12 +70,18 @@ public abstract class CommitTransactionPayload extends IdentifiablePayload<Trans
     }
 
     @VisibleForTesting
-    public static CommitTransactionPayload create(final TransactionIdentifier transactionId,
+    public static @NonNull CommitTransactionPayload create(final TransactionIdentifier transactionId,
+            final DataTreeCandidate candidate, final PayloadVersion version) throws IOException {
+        return create(transactionId, candidate, version, 512);
+    }
+
+    @VisibleForTesting
+    public static @NonNull CommitTransactionPayload create(final TransactionIdentifier transactionId,
             final DataTreeCandidate candidate) throws IOException {
-        return create(transactionId, candidate, 512);
+        return create(transactionId, candidate, PayloadVersion.current());
     }
 
-    public Entry<TransactionIdentifier, DataTreeCandidateWithVersion> getCandidate() throws IOException {
+    public @NonNull Entry<TransactionIdentifier, DataTreeCandidateWithVersion> getCandidate() throws IOException {
         Entry<TransactionIdentifier, DataTreeCandidateWithVersion> localCandidate = candidate;
         if (localCandidate == null) {
             synchronized (this) {
@@ -87,7 +94,7 @@ public abstract class CommitTransactionPayload extends IdentifiablePayload<Trans
         return localCandidate;
     }
 
-    public final Entry<TransactionIdentifier, DataTreeCandidateWithVersion> getCandidate(
+    public final @NonNull Entry<TransactionIdentifier, DataTreeCandidateWithVersion> getCandidate(
             final ReusableStreamReceiver receiver) throws IOException {
         final DataInput in = newDataInput();
         return new SimpleImmutableEntry<>(TransactionIdentifier.readFrom(in),
index b78bb56..ee829e8 100644 (file)
@@ -10,6 +10,7 @@ package org.opendaylight.controller.cluster.datastore.persisted;
 import static java.util.Objects.requireNonNull;
 
 import com.google.common.annotations.Beta;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import java.io.DataInput;
 import java.io.DataOutput;
@@ -201,9 +202,10 @@ public final class DataTreeCandidateInputOutput {
         }
     }
 
-    public static void writeDataTreeCandidate(final DataOutput out, final DataTreeCandidate candidate)
-            throws IOException {
-        try (NormalizedNodeDataOutput writer = PayloadVersion.current().getStreamVersion().newDataOutput(out)) {
+    @VisibleForTesting
+    public static void writeDataTreeCandidate(final DataOutput out, final PayloadVersion version,
+            final DataTreeCandidate candidate) throws IOException {
+        try (NormalizedNodeDataOutput writer = version.getStreamVersion().newDataOutput(out)) {
             writer.writeYangInstanceIdentifier(candidate.getRootPath());
 
             final DataTreeCandidateNode node = candidate.getRootNode();
@@ -236,6 +238,11 @@ public final class DataTreeCandidateInputOutput {
         }
     }
 
+    public static void writeDataTreeCandidate(final DataOutput out, final DataTreeCandidate candidate)
+            throws IOException {
+        writeDataTreeCandidate(out, PayloadVersion.current(), candidate);
+    }
+
     private static void throwUnhandledNodeType(final DataTreeCandidateNode node) {
         throw new IllegalArgumentException("Unhandled node type " + node.getModificationType());
     }
index a8c7393..a294584 100644 (file)
@@ -9,6 +9,7 @@ package org.opendaylight.controller.cluster.datastore.persisted;
 
 import static java.util.Objects.requireNonNull;
 
+import com.google.common.annotations.VisibleForTesting;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.Externalizable;
 import java.io.IOException;
@@ -62,7 +63,8 @@ public class ShardSnapshotState implements Snapshot.State {
     private final @NonNull ShardDataTreeSnapshot snapshot;
     private final boolean migrated;
 
-    ShardSnapshotState(final @NonNull ShardDataTreeSnapshot snapshot, final boolean migrated) {
+    @VisibleForTesting
+    public ShardSnapshotState(final @NonNull ShardDataTreeSnapshot snapshot, final boolean migrated) {
         this.snapshot = requireNonNull(snapshot);
         this.migrated = migrated;
     }
index 2b274ad..43dd465 100644 (file)
@@ -35,6 +35,8 @@ import static org.opendaylight.controller.cluster.datastore.ShardDataTreeMocking
 import com.google.common.base.Ticker;
 import com.google.common.primitives.UnsignedLong;
 import com.google.common.util.concurrent.FutureCallback;
+import java.io.IOException;
+import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
@@ -49,20 +51,29 @@ import org.mockito.InOrder;
 import org.mockito.Mockito;
 import org.opendaylight.controller.cluster.datastore.jmx.mbeans.shard.ShardStats;
 import org.opendaylight.controller.cluster.datastore.persisted.CommitTransactionPayload;
+import org.opendaylight.controller.cluster.datastore.persisted.MetadataShardDataTreeSnapshot;
+import org.opendaylight.controller.cluster.datastore.persisted.PayloadVersion;
+import org.opendaylight.controller.cluster.datastore.persisted.ShardSnapshotState;
 import org.opendaylight.controller.md.cluster.datastore.model.CarsModel;
 import org.opendaylight.controller.md.cluster.datastore.model.PeopleModel;
 import org.opendaylight.controller.md.cluster.datastore.model.SchemaContextHelper;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeChangeListener;
 import org.opendaylight.yangtools.yang.common.Uint64;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates;
+import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
 import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidates;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeSnapshot;
+import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailedException;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.TreeType;
+import org.opendaylight.yangtools.yang.data.impl.schema.Builders;
+import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
 
 public class ShardDataTreeTest extends AbstractTest {
@@ -476,6 +487,58 @@ public class ShardDataTreeTest extends AbstractTest {
         assertEquals("People node", peopleNode, optional.get());
     }
 
+    @Test
+    public void testUintCommitPayload() throws IOException {
+        shardDataTree.applyRecoveryPayload(CommitTransactionPayload.create(nextTransactionId(),
+            DataTreeCandidates.fromNormalizedNode(YangInstanceIdentifier.empty(), bigIntegerRoot()),
+            PayloadVersion.SODIUM_SR1));
+
+        assertCarsUint64();
+    }
+
+    @Test
+    public void testUintSnapshot() throws IOException, DataValidationFailedException {
+        shardDataTree.applyRecoverySnapshot(new ShardSnapshotState(new MetadataShardDataTreeSnapshot(bigIntegerRoot()),
+            true));
+
+        assertCarsUint64();
+    }
+
+    private void assertCarsUint64() {
+        final DataTreeSnapshot snapshot = shardDataTree.newReadOnlyTransaction(nextTransactionId()).getSnapshot();
+        final NormalizedNode<?, ?> cars = snapshot.readNode(CarsModel.CAR_LIST_PATH).get();
+
+        assertEquals(Builders.mapBuilder()
+            .withNodeIdentifier(new NodeIdentifier(CarsModel.CAR_QNAME))
+            .withChild(Builders.mapEntryBuilder()
+                .withNodeIdentifier(NodeIdentifierWithPredicates.of(CarsModel.CAR_QNAME,
+                    CarsModel.CAR_NAME_QNAME, "foo"))
+                .withChild(ImmutableNodes.leafNode(CarsModel.CAR_NAME_QNAME, "foo"))
+                // Note: Uint64
+                .withChild(ImmutableNodes.leafNode(CarsModel.CAR_PRICE_QNAME, Uint64.ONE))
+                .build())
+            .build(), cars);
+    }
+
+    private static ContainerNode bigIntegerRoot() {
+        return Builders.containerBuilder()
+                .withNodeIdentifier(new NodeIdentifier(SchemaContext.NAME))
+                .withChild(Builders.containerBuilder()
+                    .withNodeIdentifier(new NodeIdentifier(CarsModel.CARS_QNAME))
+                    .withChild(Builders.mapBuilder()
+                        .withNodeIdentifier(new NodeIdentifier(CarsModel.CAR_QNAME))
+                        .withChild(Builders.mapEntryBuilder()
+                            .withNodeIdentifier(NodeIdentifierWithPredicates.of(CarsModel.CAR_QNAME,
+                                CarsModel.CAR_NAME_QNAME, "foo"))
+                            .withChild(ImmutableNodes.leafNode(CarsModel.CAR_NAME_QNAME, "foo"))
+                            // Note: old BigInteger
+                            .withChild(ImmutableNodes.leafNode(CarsModel.CAR_PRICE_QNAME, BigInteger.ONE))
+                            .build())
+                        .build())
+                    .build())
+                .build();
+    }
+
     private ShardDataTreeCohort newShardDataTreeCohort(final DataTreeOperation operation) {
         final ReadWriteShardDataTreeTransaction transaction =
                 shardDataTree.newReadWriteTransaction(nextTransactionId());

©2013 OpenDaylight, A Linux Foundation Collaborative Project. All Rights Reserved.
OpenDaylight is a registered trademark of The OpenDaylight Project, Inc.
Linux Foundation and OpenDaylight are registered trademarks of the Linux Foundation.
Linux is a registered trademark of Linus Torvalds.