From 5503731d866d318e9d5a2183608092a9d332dfe6 Mon Sep 17 00:00:00 2001 From: Jan Srnicek Date: Mon, 23 Oct 2017 10:57:13 +0200 Subject: HONEYCOMB-405 - Revert fix for indirect updates If indirect update(delete+create) fails in a way, that delete passed, but update part failed, delete part must be reverted Moves reverter creation to MDTG and test cases related too it to ModifiableDataTreeDelegatorRevertTest Fixes tracking of allready processed changes by tracking them from perspective of processModifications() method Introduces UpdateFailedException as replacement for BulkUpdateException(now thrown also for single updates) Separates ReverterImpl from FlatWriterRegistry and ads unit tests Change-Id: If0066d0716d9476be89b1d99985b6745becac15e Signed-off-by: Jan Srnicek --- .../data/impl/ModifiableDataTreeDelegator.java | 50 ++++++++++++++-------- 1 file changed, 33 insertions(+), 17 deletions(-) (limited to 'infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ModifiableDataTreeDelegator.java') 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 66dcbe5d9..e76c5b6e5 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 @@ -33,8 +33,12 @@ import io.fd.honeycomb.translate.util.TransactionMappingContext; import io.fd.honeycomb.translate.util.write.TransactionWriteContext; import io.fd.honeycomb.translate.write.DataObjectUpdate; import io.fd.honeycomb.translate.write.WriteContext; +import io.fd.honeycomb.translate.write.registry.UpdateFailedException; import io.fd.honeycomb.translate.write.registry.WriterRegistry; +import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException; @@ -136,33 +140,37 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager ((TransactionMappingContext) ctx.getMappingContext()).submit(); // Blocking on context data update contextUpdateResult.checkedGet(); - - } catch (WriterRegistry.BulkUpdateException e) { + } catch (UpdateFailedException e) { + // TODO - HONEYCOMB-411 LOG.warn("Failed to apply all changes", e); - LOG.info("Trying to revert successful changes for current transaction"); - - try { - // 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(comitted) changes, failure occurred for: {}. State might be corrupted.", - revertFailedException.getFailedUpdate(), revertFailedException); - throw revertFailedException; + final List processed = e.getProcessed(); + if (processed.isEmpty()) { + // nothing was processed, which means either very first operation failed, or it was single operation + // update. In both cases, no revert is needed + LOG.info("Nothing to revert"); + throw e; + } else { + LOG.info("Trying to revert successful changes for current transaction"); + try { + // attempt revert with fresh context, to allow write logic used context-binded data + new Reverter(processed, writerRegistry) + .revert(getRevertTransactionContext(ctx.getMappingContext())); + LOG.info("Changes successfully reverted"); + } catch (Reverter.RevertFailedException revertFailedException) { + // fail with failed revert + LOG.error("Failed to revert successful(comitted) changes", revertFailedException); + throw revertFailedException; + } } // 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.getUnrevertedSubtrees()); + throw new Reverter.RevertSuccessException(getNonProcessedNodes(baUpdates, processed)); } 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"; LOG.error(msg, e); throw new TranslationException(msg, e); - } 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. @@ -200,6 +208,14 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager } } + private static Set> getNonProcessedNodes(final WriterRegistry.DataObjectUpdates allUpdates, + final List alreadyProcessed) { + return allUpdates.getAllModifications().stream() + .filter(update -> !alreadyProcessed.contains(update)) + .map(DataObjectUpdate::getId) + .collect(Collectors.toSet()); + } + @VisibleForTesting static WriterRegistry.DataObjectUpdates toBindingAware( final WriterRegistry registry, -- cgit 1.2.3-korg