Bug 509: Fixed incorrect merging of Data Store Writes / Events 17/6217/2
authorTony Tkacik <ttkacik@cisco.com>
Tue, 15 Apr 2014 15:48:56 +0000 (17:48 +0200)
committerTony Tkacik <ttkacik@cisco.com>
Tue, 15 Apr 2014 16:28:40 +0000 (18:28 +0200)
ForwardedBackwardsCompatibleDataBroker:

In old Data API only way to replace element was to use
remove/put combination, which changed with new Async DOM Broker
APIs where put is actually add/replace so previous delete is not
neccessary.

BackwardsCompatibleTransaction keeps track of such remove/put
operations and propagates put-only to new Async Data Transaction.

DOMImmutableDataChangeEvent:

Builder for DOMImmutableDataChange event used ImmutableMap builder
which throws IllegalArgumentException when same key/value combo
is overriden, which made impossible to merge some events. Switched
Builder implementation to use combination of HashMap and
ImmutableMap.copyOf() in order to prevent this
IllegalArgumentException to fail commit process.

Change-Id: I935bd4f5499c26190c52adcaee1df6335640bfaf
Signed-off-by: Tony Tkacik <ttkacik@cisco.com>
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/ForwardedBackwardsCompatibleDataBroker.java
opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMImmutableDataChangeEvent.java

index ee76073..ab0dc6e 100644 (file)
@@ -201,6 +201,10 @@ public class ForwardedBackwardsCompatibleDataBroker extends AbstractForwardedDat
         private final Map<InstanceIdentifier<? extends DataObject>, DataObject> original = new HashMap<>();
         private TransactionStatus status = TransactionStatus.NEW;
 
+        private final Set<InstanceIdentifier<? extends DataObject>> posponedRemovedOperational = new HashSet<>();
+        private final Set<InstanceIdentifier<? extends DataObject>> posponedRemovedConfiguration = new HashSet<>();
+
+
         @Override
         public final TransactionStatus getStatus() {
             return status;
@@ -214,12 +218,13 @@ public class ForwardedBackwardsCompatibleDataBroker extends AbstractForwardedDat
 
         @Override
         public void putOperationalData(final InstanceIdentifier<? extends DataObject> path, final DataObject data) {
-
+            posponedRemovedOperational.remove(path);
             doPutWithEnsureParents(getDelegate(), LogicalDatastoreType.OPERATIONAL, path, data);
         }
 
         @Override
         public void putConfigurationData(final InstanceIdentifier<? extends DataObject> path, final DataObject data) {
+            posponedRemovedConfiguration.remove(path);
             DataObject originalObj = readConfigurationData(path);
             if (originalObj != null) {
                 original.put(path, originalObj);
@@ -233,13 +238,12 @@ public class ForwardedBackwardsCompatibleDataBroker extends AbstractForwardedDat
 
         @Override
         public void removeOperationalData(final InstanceIdentifier<? extends DataObject> path) {
-            doDelete(getDelegate(), LogicalDatastoreType.OPERATIONAL, path);
-
+            posponedRemovedOperational.add(path);
         }
 
         @Override
         public void removeConfigurationData(final InstanceIdentifier<? extends DataObject> path) {
-            doDelete(getDelegate(), LogicalDatastoreType.CONFIGURATION, path);
+            posponedRemovedConfiguration.add(path);
         }
 
         @Override
@@ -307,25 +311,34 @@ public class ForwardedBackwardsCompatibleDataBroker extends AbstractForwardedDat
             return getDelegate().getIdentifier();
         }
 
-        private void changeStatus(TransactionStatus status) {
+        private void changeStatus(final TransactionStatus status) {
             LOG.trace("Transaction {} changed status to {}", getIdentifier(), status);
             this.status = status;
         }
 
         @Override
         public ListenableFuture<RpcResult<TransactionStatus>> commit() {
+
+            for(InstanceIdentifier<? extends DataObject> path : posponedRemovedConfiguration) {
+                doDelete(getDelegate(), LogicalDatastoreType.CONFIGURATION, path);
+            }
+
+            for(InstanceIdentifier<? extends DataObject> path : posponedRemovedOperational) {
+                doDelete(getDelegate(), LogicalDatastoreType.OPERATIONAL, path);
+            }
+
             final ListenableFuture<RpcResult<TransactionStatus>> f = ForwardedBackwardsCompatibleDataBroker.this.commit(this);
 
             changeStatus(TransactionStatus.SUBMITED);
 
             Futures.addCallback(f, new FutureCallback<RpcResult<TransactionStatus>>() {
                 @Override
-                public void onSuccess(RpcResult<TransactionStatus> result) {
+                public void onSuccess(final RpcResult<TransactionStatus> result) {
                     changeStatus(result.getResult());
                 }
 
                 @Override
-                public void onFailure(Throwable t) {
+                public void onFailure(final Throwable t) {
                     LOG.error("Transaction {} failed to complete", getIdentifier(), t);
                     changeStatus(TransactionStatus.FAILED);
                 }
index 86f08de..3dfca40 100644 (file)
@@ -1,5 +1,8 @@
 package org.opendaylight.controller.md.sal.dom.store.impl;
 
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
@@ -10,8 +13,6 @@ import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.PathArgument;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
 
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
 
 public final class DOMImmutableDataChangeEvent implements
         AsyncDataChangeEvent<InstanceIdentifier, NormalizedNode<?, ?>> {
@@ -33,10 +34,10 @@ public final class DOMImmutableDataChangeEvent implements
     private DOMImmutableDataChangeEvent(final Builder change) {
         original = change.before;
         updated = change.after;
-        originalData = change.original.build();
-        createdData = change.created.build();
-        updatedData = change.updated.build();
-        removedPaths = change.removed.build();
+        originalData = Collections.unmodifiableMap(change.original);
+        createdData = Collections.unmodifiableMap(change.created);
+        updatedData = Collections.unmodifiableMap(change.updated);
+        removedPaths = Collections.unmodifiableSet(change.removed);
         scope = change.scope;
     }
 
@@ -125,10 +126,10 @@ public final class DOMImmutableDataChangeEvent implements
         private NormalizedNode<?, ?> after;
         private NormalizedNode<?, ?> before;
 
-        private final ImmutableMap.Builder<InstanceIdentifier, NormalizedNode<?, ?>> original = ImmutableMap.builder();
-        private final ImmutableMap.Builder<InstanceIdentifier, NormalizedNode<?, ?>> created = ImmutableMap.builder();
-        private final ImmutableMap.Builder<InstanceIdentifier, NormalizedNode<?, ?>> updated = ImmutableMap.builder();
-        private final ImmutableSet.Builder<InstanceIdentifier> removed = ImmutableSet.builder();
+        private final Map<InstanceIdentifier, NormalizedNode<?, ?>> original = new HashMap<>();
+        private final Map<InstanceIdentifier, NormalizedNode<?, ?>> created = new HashMap<>();
+        private final Map<InstanceIdentifier, NormalizedNode<?, ?>> updated = new HashMap<>();
+        private final Set<InstanceIdentifier> removed = new HashSet<>();
 
         private Builder(final DataChangeScope scope) {
             Preconditions.checkNotNull(scope, "Data change scope should not be null.");

©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.