From bb090e1254eacc95d7bd1dd45f6311967f81af86 Mon Sep 17 00:00:00 2001 From: Jan Srnicek Date: Mon, 24 Oct 2016 13:02:59 +0200 Subject: 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 Signed-off-by: Maros Marsalek --- .../util/write/TransactionWriteContext.java | 3 --- .../util/write/registry/FlatWriterRegistry.java | 26 ++++++++++++---------- 2 files changed, 14 insertions(+), 15 deletions(-) (limited to 'infra/translate-utils/src/main/java') diff --git a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/TransactionWriteContext.java b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/TransactionWriteContext.java index 0128ee4a8..dd397a605 100644 --- a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/TransactionWriteContext.java +++ b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/TransactionWriteContext.java @@ -108,9 +108,6 @@ public final class TransactionWriteContext implements WriteContext { return mappingContext; } - /** - * Does not close the transactions. - */ @Override public void close() { ctx.close(); diff --git a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistry.java b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistry.java index ab80eb4ac..e5b829d6a 100644 --- a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistry.java +++ b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/write/registry/FlatWriterRegistry.java @@ -17,6 +17,8 @@ package io.fd.honeycomb.translate.util.write.registry; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; +import static io.fd.honeycomb.translate.util.RWUtils.makeIidWildcarded; import com.google.common.base.Optional; import com.google.common.collect.HashMultimap; @@ -26,9 +28,9 @@ import com.google.common.collect.Lists; import com.google.common.collect.Multimap; import com.google.common.collect.Sets; import io.fd.honeycomb.translate.TranslationException; -import io.fd.honeycomb.translate.write.WriteContext; import io.fd.honeycomb.translate.util.RWUtils; import io.fd.honeycomb.translate.write.DataObjectUpdate; +import io.fd.honeycomb.translate.write.WriteContext; import io.fd.honeycomb.translate.write.WriteFailedException; import io.fd.honeycomb.translate.write.Writer; import io.fd.honeycomb.translate.write.registry.WriterRegistry; @@ -211,11 +213,11 @@ final class FlatWriterRegistry implements WriterRegistry { LOG.trace("Update successful for type: {}", writerType); LOG.debug("Update successful for: {}", singleUpdate); } catch (Exception e) { - LOG.error("Error while processing data change of: {} (updates={})", writerType, writersData, e); + // do not log this exception here,its logged in ModifiableDataTreeDelegator final Reverter reverter = attemptRevert - ? new ReverterImpl(processedNodes, updates, writersOrder, ctx) - : () -> {}; // NOOP reverter + ? new ReverterImpl(processedNodes, updates, writersOrder) + : (final WriteContext writeContext) -> {};//NOOP reverter // Find out which changes left unprocessed final Set> unprocessedChanges = updates.values().stream() @@ -258,30 +260,29 @@ final class FlatWriterRegistry implements WriterRegistry { private final Collection> processedNodes; private final Multimap, ? extends DataObjectUpdate> updates; private final Set> revertDeleteOrder; - private final WriteContext ctx; ReverterImpl(final Collection> processedNodes, final Multimap, ? extends DataObjectUpdate> updates, - final Set> writersOrderOriginal, - final WriteContext ctx) { + final Set> writersOrderOriginal) { this.processedNodes = processedNodes; this.updates = updates; // Use opposite ordering when executing revert this.revertDeleteOrder = writersOrderOriginal == FlatWriterRegistry.this.writersOrder ? FlatWriterRegistry.this.writersOrderReversed : FlatWriterRegistry.this.writersOrder; - this.ctx = ctx; } @Override - public void revert() throws RevertFailedException { + public void revert(@Nonnull final WriteContext writeContext) throws RevertFailedException { + checkNotNull(writeContext, "Cannot revert changes for null context"); + Multimap, DataObjectUpdate> updatesToRevert = filterAndRevertProcessed(updates, processedNodes); LOG.info("Attempting revert for changes: {}", updatesToRevert); try { // Perform reversed bulk update without revert attempt - bulkUpdate(updatesToRevert, ctx, true, revertDeleteOrder); + bulkUpdate(updatesToRevert, writeContext, true, revertDeleteOrder); LOG.info("Revert successful"); } catch (BulkUpdateException e) { LOG.error("Revert failed", e); @@ -298,11 +299,12 @@ final class FlatWriterRegistry implements WriterRegistry { final Collection> processedNodes) { final Multimap, DataObjectUpdate> filtered = HashMultimap.create(); for (InstanceIdentifier processedNode : processedNodes) { - final InstanceIdentifier wildcardedIid = RWUtils.makeIidWildcarded(processedNode); + final InstanceIdentifier wildcardedIid = makeIidWildcarded(processedNode); if (updates.containsKey(wildcardedIid)) { updates.get(wildcardedIid).stream() .filter(dataObjectUpdate -> processedNode.contains(dataObjectUpdate.getId())) - .forEach(dataObjectUpdate -> filtered.put(processedNode, dataObjectUpdate.reverse())); + // putting under unkeyed identifier, to prevent failing of checkAllTypesCanBeHandled + .forEach(dataObjectUpdate -> filtered.put(wildcardedIid, dataObjectUpdate.reverse())); } } return filtered; -- cgit 1.2.3-korg