summaryrefslogtreecommitdiffstats
path: root/infra/data-impl/src
diff options
context:
space:
mode:
authorJan Srnicek <jsrnicek@cisco.com>2016-10-24 13:02:59 +0200
committerMaros Marsalek <mmarsale@cisco.com>2016-10-24 17:33:24 +0200
commitbb090e1254eacc95d7bd1dd45f6311967f81af86 (patch)
tree101df09cd4b8adad9d083a00442aecf90e9d9948 /infra/data-impl/src
parent81c7ae0e263515b5bf8acb59b06d61ba446b8f0b (diff)
HONEYCOMB-255 - Cutting identifiers to prevent failing of reverts
Mapping allready processes changes for reverting by InstanceIdentifier instead of using KeyedInstanceIdentifier(to prevent failing to identify handleable nodes) Modified logging to prevent double/triple logging of detailed cause of failed bulk update Reusing WriteContext for revert(removed try with resource to prevent closing of write context before revert) Change-Id: Ie939ebe443629f9cdad5b5b449aa8c5dac40ea67 Signed-off-by: Jan Srnicek <jsrnicek@cisco.com> Signed-off-by: Maros Marsalek <mmarsale@cisco.com>
Diffstat (limited to 'infra/data-impl/src')
-rw-r--r--infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegator.java31
-rw-r--r--infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/WriteTransaction.java2
-rw-r--r--infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorTest.java16
3 files changed, 38 insertions, 11 deletions
diff --git a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegator.java b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegator.java
index 213208064..3de9131c0 100644
--- a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegator.java
+++ b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegator.java
@@ -26,6 +26,7 @@ import com.google.common.collect.Multimap;
import com.google.common.util.concurrent.CheckedFuture;
import io.fd.honeycomb.data.DataModification;
import io.fd.honeycomb.data.ReadableDataManager;
+import io.fd.honeycomb.translate.MappingContext;
import io.fd.honeycomb.translate.TranslationException;
import io.fd.honeycomb.translate.util.RWUtils;
import io.fd.honeycomb.translate.util.TransactionMappingContext;
@@ -126,7 +127,8 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager
final WriterRegistry.DataObjectUpdates baUpdates = toBindingAware(modificationDiff.getUpdates());
LOG.debug("ConfigDataTree.modify() extracted updates={}", baUpdates);
- try (final WriteContext ctx = getTransactionWriteContext()) {
+ WriteContext ctx = getTransactionWriteContext();
+ try {
writerRegistry.update(baUpdates, ctx);
final CheckedFuture<Void, TransactionCommitFailedException> contextUpdateResult =
@@ -139,15 +141,18 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager
LOG.info("Trying to revert successful changes for current transaction");
try {
- e.revertChanges();
+ // attempt revert with fresh context, to allow write logic used context-binded data
+ e.revertChanges(getRevertTransactionContext(ctx.getMappingContext()));
LOG.info("Changes successfully reverted");
} catch (WriterRegistry.Reverter.RevertFailedException revertFailedException) {
// fail with failed revert
LOG.error("Failed to revert successful changes", revertFailedException);
throw revertFailedException;
}
-
- throw e; // fail with success revert
+ // fail with success revert
+ // not passing the cause,its logged above and it would be logged after transaction
+ // ended again(prevent double logging of same error
+ throw new WriterRegistry.Reverter.RevertSuccessException(e.getFailedIds());
} catch (TransactionCommitFailedException e) {
// TODO HONEYCOMB-162 revert should probably occur when context is not written successfully
final String msg = "Error while updating mapping context data";
@@ -156,9 +161,27 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager
} catch (TranslationException e) {
LOG.error("Error while processing data change (updates={})", baUpdates, e);
throw e;
+ } finally {
+ // Using finally instead of try-with-resources in order to leave ctx open for BulkUpdateException catch
+ // block. The context is needed there, but try-with-resources closes the resource before handling ex.
+ LOG.debug("Closing write context {}", ctx);
+ ctx.close();
}
}
+ /**
+ * Creates inverted transaction context for reverting of proceeded changes.
+ * Invert before/after transaction and reuse affected mapping context created by previous updates
+ * to access all data created by previous updates
+ * */
+ private TransactionWriteContext getRevertTransactionContext(final MappingContext affectedMappingContext){
+ // Before Tx == after partial update
+ final DOMDataReadOnlyTransaction beforeTx = ReadOnlyTransaction.create(this, EMPTY_OPERATIONAL);
+ // After Tx == before partial update
+ final DOMDataReadOnlyTransaction afterTx = ReadOnlyTransaction.create(untouchedModification, EMPTY_OPERATIONAL);
+ return new TransactionWriteContext(serializer, beforeTx, afterTx, affectedMappingContext);
+ }
+
private TransactionWriteContext getTransactionWriteContext() {
// Before Tx must use modification
final DOMDataReadOnlyTransaction beforeTx = ReadOnlyTransaction.create(untouchedModification, EMPTY_OPERATIONAL);
diff --git a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/WriteTransaction.java b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/WriteTransaction.java
index 2cbe1ec8b..93043ce07 100644
--- a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/WriteTransaction.java
+++ b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/WriteTransaction.java
@@ -142,7 +142,7 @@ final class WriteTransaction implements DOMDataWriteTransaction {
status = COMMITED;
} catch (DataValidationFailedException | TranslationException e) {
status = FAILED;
- LOG.error("Failed modify data tree", e);
+ LOG.debug("Submit failed", e);
return Futures.immediateFailedCheckedFuture(
new TransactionCommitFailedException("Failed to validate DataTreeModification", e));
}
diff --git a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorTest.java b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorTest.java
index 0a7c85b3b..322328508 100644
--- a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorTest.java
+++ b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegatorTest.java
@@ -49,6 +49,8 @@ import java.util.HashMap;
import java.util.Map;
import org.junit.Before;
import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
import org.mockito.Mock;
import org.opendaylight.controller.md.sal.binding.api.DataBroker;
import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException;
@@ -76,6 +78,9 @@ public class ModifiableDataTreeDelegatorTest {
@Mock
private org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction tx;
+ @Captor
+ private ArgumentCaptor<WriteContext> writeContextCaptor;
+
private ModifiableDataTreeManager configDataTree;
static final InstanceIdentifier<?> DEFAULT_ID = InstanceIdentifier.create(DataObject.class);
@@ -149,12 +154,11 @@ public class ModifiableDataTreeDelegatorTest {
dataModification.write(ModificationDiffTest.NESTED_LIST_ID, nestedList);
dataModification.validate();
dataModification.commit();
- fail("WriterRegistry.BulkUpdateException was expected");
- } catch (WriterRegistry.BulkUpdateException e) {
+ fail("WriterRegistry.RevertSuccessException was expected");
+ } catch (WriterRegistry.Reverter.RevertSuccessException e) {
verify(writer).update(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class));
assertThat(e.getFailedIds(), hasItem(DEFAULT_ID));
- verify(reverter).revert();
- assertEquals(failedOnUpdateException, e.getCause());
+ verify(reverter).revert(any(WriteContext.class));
}
}
@@ -171,7 +175,7 @@ public class ModifiableDataTreeDelegatorTest {
// Fail on revert:
final TranslationException failedOnRevertException = new TranslationException("revert failed");
doThrow(new WriterRegistry.Reverter.RevertFailedException(Collections.emptySet(), failedOnRevertException))
- .when(reverter).revert();
+ .when(reverter).revert(any(WriteContext.class));
try {
// Run the test
@@ -182,7 +186,7 @@ public class ModifiableDataTreeDelegatorTest {
fail("WriterRegistry.Reverter.RevertFailedException was expected");
} catch (WriterRegistry.Reverter.RevertFailedException e) {
verify(writer).update(any(WriterRegistry.DataObjectUpdates.class), any(WriteContext.class));
- verify(reverter).revert();
+ verify(reverter).revert(any(WriteContext.class));
assertEquals(failedOnRevertException, e.getCause());
}
}