From 08217531fbe76dbcc429c71d593894fc211e50aa Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Fri, 21 Mar 2014 13:30:45 +0100 Subject: [PATCH] Bug 499: Improved data change listener tree management Change-Id: I62843c3c4df32dd5a5849511d41352fa6822550b Signed-off-by: Tony Tkacik Signed-off-by: Robert Varga --- .../dom/broker/impl/DOMDataBrokerImpl.java | 18 +++-- .../store/impl/ChangeListenerNotifyTask.java | 1 - .../impl/DOMImmutableDataChangeEvent.java | 8 ++- .../store/impl/DataChangeEventResolver.java | 34 ++++++++-- .../dom/store/impl/InMemoryDOMDataStore.java | 13 ++-- .../md/sal/dom/store/impl/StoreUtils.java | 24 +++---- .../impl/tree/ListenerRegistrationNode.java | 67 +++++++++++++------ 7 files changed, 109 insertions(+), 56 deletions(-) diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMDataBrokerImpl.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMDataBrokerImpl.java index fcf8b40efe..fc87a91105 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMDataBrokerImpl.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMDataBrokerImpl.java @@ -120,7 +120,7 @@ public class DOMDataBrokerImpl implements DOMDataBroker, AutoCloseable { private ListenableFuture> submit( final WriteTransactionImpl transaction) { - LOG.debug("Tx: {} is submitted for execution.",transaction.getIdentifier()); + LOG.debug("Tx: {} is submitted for execution.", transaction.getIdentifier()); return executor.submit(new CommitCoordination(transaction)); } @@ -253,7 +253,8 @@ public class DOMDataBrokerImpl implements DOMDataBroker, AutoCloseable { } @Override - public void merge(final LogicalDatastoreType store, final InstanceIdentifier path, final NormalizedNode data) { + public void merge(final LogicalDatastoreType store, final InstanceIdentifier path, + final NormalizedNode data) { } } @@ -269,15 +270,18 @@ public class DOMDataBrokerImpl implements DOMDataBroker, AutoCloseable { @Override public RpcResult call() throws Exception { - Boolean canCommit = canCommit().get(); try { + Boolean canCommit = canCommit().get(); + if (canCommit) { try { preCommit().get(); try { commit().get(); - COORDINATOR_LOG.debug("Tx: {} Is commited.",transaction.getIdentifier()); - return Rpcs.getRpcResult(true, TransactionStatus.COMMITED, Collections.emptySet()); + COORDINATOR_LOG.debug("Tx: {} Is commited.", transaction.getIdentifier()); + return Rpcs.getRpcResult(true, TransactionStatus.COMMITED, + Collections. emptySet()); + } catch (InterruptedException | ExecutionException e) { COORDINATOR_LOG.error("Tx: {} Error during commit", transaction.getIdentifier(), e); } @@ -287,7 +291,7 @@ public class DOMDataBrokerImpl implements DOMDataBroker, AutoCloseable { transaction.getIdentifier(), e); } } else { - COORDINATOR_LOG.info("Tx: {} Did not pass canCommit phase."); + COORDINATOR_LOG.info("Tx: {} Did not pass canCommit phase.", transaction.getIdentifier()); abort().get(); } } catch (InterruptedException | ExecutionException e) { @@ -299,7 +303,7 @@ public class DOMDataBrokerImpl implements DOMDataBroker, AutoCloseable { } catch (InterruptedException | ExecutionException e) { COORDINATOR_LOG.error("Tx: {} Error during abort", transaction.getIdentifier(), e); } - return Rpcs.getRpcResult(false, TransactionStatus.FAILED, Collections.emptySet()); + return Rpcs.getRpcResult(false, TransactionStatus.FAILED, Collections. emptySet()); } public ListenableFuture preCommit() { diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ChangeListenerNotifyTask.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ChangeListenerNotifyTask.java index ff0fbf98cf..72ae24e007 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ChangeListenerNotifyTask.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ChangeListenerNotifyTask.java @@ -1,7 +1,6 @@ package org.opendaylight.controller.md.sal.dom.store.impl; import org.opendaylight.controller.md.sal.common.api.data.AsyncDataChangeEvent; -import org.opendaylight.controller.md.sal.dom.store.impl.tree.ListenerRegistrationNode.DataChangeListenerRegistration; import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.slf4j.Logger; diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMImmutableDataChangeEvent.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMImmutableDataChangeEvent.java index d0ebcf5d4c..3c6a3d60d8 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMImmutableDataChangeEvent.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMImmutableDataChangeEvent.java @@ -63,6 +63,12 @@ public final class DOMImmutableDataChangeEvent implements return removedPaths; } + @Override + public String toString() { + return "DOMImmutableDataChangeEvent [created=" + createdData.keySet() + ", updated=" + updatedData.keySet() + + ", removed=" + removedPaths + "]"; + } + public static class Builder { private NormalizedNode after; @@ -73,7 +79,6 @@ public final class DOMImmutableDataChangeEvent implements private final ImmutableMap.Builder> updated = ImmutableMap.builder(); private final ImmutableSet.Builder removed = ImmutableSet.builder(); - private Builder() { } @@ -122,4 +127,3 @@ public final class DOMImmutableDataChangeEvent implements } } - diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DataChangeEventResolver.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DataChangeEventResolver.java index c2faf86bce..a032c798c6 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DataChangeEventResolver.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DataChangeEventResolver.java @@ -4,15 +4,19 @@ import static org.opendaylight.controller.md.sal.dom.store.impl.DOMImmutableData import static org.opendaylight.controller.md.sal.dom.store.impl.StoreUtils.append; import static org.opendaylight.controller.md.sal.dom.store.impl.tree.TreeNodeUtils.getChild; +import java.util.ArrayList; +import java.util.Collection; + import org.opendaylight.controller.md.sal.common.api.data.AsyncDataBroker.DataChangeScope; import org.opendaylight.controller.md.sal.dom.store.impl.DOMImmutableDataChangeEvent.Builder; import org.opendaylight.controller.md.sal.dom.store.impl.tree.ListenerRegistrationNode; import org.opendaylight.controller.md.sal.dom.store.impl.tree.NodeModification; import org.opendaylight.controller.md.sal.dom.store.impl.tree.StoreMetadataNode; -import org.opendaylight.controller.md.sal.dom.store.impl.tree.ListenerRegistrationNode.DataChangeListenerRegistration; import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.PathArgument; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; @@ -20,6 +24,9 @@ import com.google.common.collect.ImmutableSet; public class DataChangeEventResolver { + + private static final Logger LOG = LoggerFactory.getLogger(DataChangeEventResolver.class); + private static final DOMImmutableDataChangeEvent NO_CHANGE = builder().build(); private InstanceIdentifier rootPath; private ListenerRegistrationNode listenerRoot; @@ -74,6 +81,7 @@ public class DataChangeEventResolver { } public Iterable resolve() { + LOG.trace("Resolving events for {}" ,modificationRoot); resolveAnyChangeEvent(rootPath, Optional.of(listenerRoot), modificationRoot, beforeRoot, afterRoot); return tasks.build(); } @@ -124,6 +132,7 @@ public class DataChangeEventResolver { InstanceIdentifier childPath = StoreUtils.append(path, childId); builder.merge(resolveCreateEvent(childPath, childListeners, child)); } + DOMImmutableDataChangeEvent event = builder.build(); if (listeners.isPresent()) { addNotifyTask(listeners.get().getListeners(), event); @@ -169,7 +178,7 @@ public class DataChangeEventResolver { switch (childMod.getModificationType()) { case WRITE: case DELETE: - one.merge(resolveAnyChangeEvent(childPath, childListen, childMod, childBefore, childBefore)); + one.merge(resolveAnyChangeEvent(childPath, childListen, childMod, childBefore, childAfter)); break; case SUBTREE_MODIFIED: subtree.merge(resolveSubtreeChangeEvent(childPath, childListen, childMod, childBefore.get(), @@ -194,16 +203,27 @@ public class DataChangeEventResolver { return builder().build(); } - private void addNotifyTask(final ListenerRegistrationNode listenerRegistrationNode, final DataChangeScope one, + private void addNotifyTask(final ListenerRegistrationNode listenerRegistrationNode, final DataChangeScope scope, final DOMImmutableDataChangeEvent event) { - - + Collection> potential = listenerRegistrationNode.getListeners(); + if(potential.isEmpty()) { + return; + } + ArrayList> toNotify = new ArrayList<>(potential.size()); + for(DataChangeListenerRegistration listener : potential) { + if(scope.equals(listener.getScope())) { + toNotify.add(listener); + } + } + addNotifyTask(toNotify, event); } - private void addNotifyTask(final Iterable> listeners, + private void addNotifyTask(final Collection> listeners, final DOMImmutableDataChangeEvent event) { - tasks .add(new ChangeListenerNotifyTask(ImmutableSet.copyOf(listeners),event)); + if(!listeners.isEmpty()) { + tasks.add(new ChangeListenerNotifyTask(ImmutableSet.copyOf(listeners),event)); + } } public static DataChangeEventResolver create() { diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java index 0944c2efae..0cd00cad7e 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java @@ -20,7 +20,6 @@ import org.opendaylight.controller.md.sal.dom.store.impl.tree.ListenerRegistrati import org.opendaylight.controller.md.sal.dom.store.impl.tree.ModificationType; import org.opendaylight.controller.md.sal.dom.store.impl.tree.NodeModification; import org.opendaylight.controller.md.sal.dom.store.impl.tree.StoreMetadataNode; -import org.opendaylight.controller.md.sal.dom.store.impl.tree.TreeNodeUtils; import org.opendaylight.controller.sal.core.spi.data.DOMStore; import org.opendaylight.controller.sal.core.spi.data.DOMStoreReadTransaction; import org.opendaylight.controller.sal.core.spi.data.DOMStoreReadWriteTransaction; @@ -98,13 +97,15 @@ public class InMemoryDOMDataStore implements DOMStore, Identifiable, Sch @Override public >> ListenerRegistration registerChangeListener( final InstanceIdentifier path, final L listener, final DataChangeScope scope) { - - Optional listenerNode = TreeNodeUtils.findNode(listenerTree, path); - checkState(listenerNode.isPresent()); + LOG.debug("{}: Registering data change listener {} for {}",name,listener,path); + ListenerRegistrationNode listenerNode = listenerTree; + for(PathArgument arg :path.getPath()) { + listenerNode = listenerNode.ensureChild(arg); + } synchronized (listener) { notifyInitialState(path, listener); } - return listenerNode.get().registerDataChangeListener(listener, scope); + return listenerNode.registerDataChangeListener(path,listener, scope); } private void notifyInitialState(final InstanceIdentifier path, @@ -138,6 +139,7 @@ public class InMemoryDOMDataStore implements DOMStore, Identifiable, Sch private synchronized void commit(final DataAndMetadataSnapshot currentSnapshot, final StoreMetadataNode newDataTree, final Iterable listenerTasks) { LOG.debug("Updating Store snaphot version: {} with version:{}",currentSnapshot.getMetadataTree().getSubtreeVersion(),newDataTree.getSubtreeVersion()); + if(LOG.isTraceEnabled()) { LOG.trace("Data Tree is {}",StoreUtils.toStringTree(newDataTree)); } @@ -323,7 +325,6 @@ public class InMemoryDOMDataStore implements DOMStore, Identifiable, Sch proposedSubtree = operationTree.apply(modification, Optional.of(metadataTree), increase(metadataTree.getSubtreeVersion())); - listenerTasks = DataChangeEventResolver.create() // .setRootPath(PUBLIC_ROOT_PATH) // .setBeforeRoot(Optional.of(metadataTree)) // diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/StoreUtils.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/StoreUtils.java index df58d62dd4..02c2a4fa06 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/StoreUtils.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/StoreUtils.java @@ -45,6 +45,11 @@ public final class StoreUtils { return new InitialDataChangeEvent(path, data.getData()); } + @SuppressWarnings({ "unchecked", "rawtypes" }) + public static Function, V> identifierExtractor() { + return (Function) EXTRACT_IDENTIFIER; + } + private static final class InitialDataChangeEvent implements AsyncDataChangeEvent> { @@ -88,30 +93,25 @@ public final class StoreUtils { } - @SuppressWarnings({ "unchecked", "rawtypes" }) - public static Function,V> identifierExtractor() { - return (Function) EXTRACT_IDENTIFIER; - } - public static Set toIdentifierSet(final Iterable> children) { - return FluentIterable.from(children).transform(StoreUtils.identifierExtractor()).toSet(); + return FluentIterable.from(children).transform(StoreUtils. identifierExtractor()).toSet(); } public static String toStringTree(final StoreMetadataNode metaNode) { StringBuilder builder = new StringBuilder(); - toStringTree(builder, metaNode,0); + toStringTree(builder, metaNode, 0); return builder.toString(); } - private static void toStringTree(final StringBuilder builder, final StoreMetadataNode metaNode,final int offset) { + private static void toStringTree(final StringBuilder builder, final StoreMetadataNode metaNode, final int offset) { String prefix = Strings.repeat(" ", offset); builder.append(prefix).append(toStringTree(metaNode.getIdentifier())); NormalizedNode dataNode = metaNode.getData(); - if(dataNode instanceof NormalizedNodeContainer) { + if (dataNode instanceof NormalizedNodeContainer) { builder.append(" {").append("\n"); - for(StoreMetadataNode child : metaNode.getChildren()) { - toStringTree(builder, child, offset+4); + for (StoreMetadataNode child : metaNode.getChildren()) { + toStringTree(builder, child, offset + 4); } builder.append(prefix).append("}"); } else { @@ -121,7 +121,7 @@ public final class StoreUtils { } private static String toStringTree(final PathArgument identifier) { - if( identifier instanceof NodeIdentifierWithPredicates) { + if (identifier instanceof NodeIdentifierWithPredicates) { StringBuilder builder = new StringBuilder(); builder.append(identifier.getNodeType().getLocalName()); builder.append(((NodeIdentifierWithPredicates) identifier).getKeyValues().values()); diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/ListenerRegistrationNode.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/ListenerRegistrationNode.java index ee49effd31..e48438d6fa 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/ListenerRegistrationNode.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/ListenerRegistrationNode.java @@ -1,5 +1,6 @@ package org.opendaylight.controller.md.sal.dom.store.impl.tree; +import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -12,10 +13,14 @@ import org.opendaylight.yangtools.concepts.ListenerRegistration; import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.PathArgument; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.google.common.base.Optional; -public class ListenerRegistrationNode implements StoreTreeNode,Identifiable { +public class ListenerRegistrationNode implements StoreTreeNode, Identifiable { + + private final Logger LOG = LoggerFactory.getLogger(ListenerRegistrationNode.class); private final ListenerRegistrationNode parent; private final Map children; @@ -23,10 +28,10 @@ public class ListenerRegistrationNode implements StoreTreeNode> listeners; private ListenerRegistrationNode(final PathArgument identifier) { - this(null,identifier); + this(null, identifier); } - private ListenerRegistrationNode(final ListenerRegistrationNode parent,final PathArgument identifier) { + private ListenerRegistrationNode(final ListenerRegistrationNode parent, final PathArgument identifier) { this.parent = parent; this.identifier = identifier; children = new HashMap<>(); @@ -42,23 +47,38 @@ public class ListenerRegistrationNode implements StoreTreeNode> getListeners() { - return listeners; + @SuppressWarnings({ "rawtypes", "unchecked" }) + public Collection> getListeners() { + return (Collection) listeners; } @Override public synchronized Optional getChild(final PathArgument child) { + return Optional.fromNullable(children.get(child)); + } + + public synchronized ListenerRegistrationNode ensureChild(final PathArgument child) { ListenerRegistrationNode potential = (children.get(child)); - if(potential == null) { + if (potential == null) { potential = new ListenerRegistrationNode(this, child); children.put(child, potential); } - return Optional.of(potential); + return potential; } - public >> ListenerRegistration registerDataChangeListener( + /** + * + * Registers listener on this node. + * + * @param path Full path on which listener is registered. + * @param listener Listener + * @param scope Scope of triggering event. + * @return + */ + public >> ListenerRegistration registerDataChangeListener(final InstanceIdentifier path, final L listener, final DataChangeScope scope) { - DataChangeListenerRegistration listenerReg = new DataChangeListenerRegistration(listener, scope,this); + + DataChangeListenerRegistration listenerReg = new DataChangeListenerRegistration(path,listener, scope, this); listeners.add(listenerReg); return listenerReg; } @@ -68,9 +88,8 @@ public class ListenerRegistrationNode implements StoreTreeNode>> extends AbstractObjectRegistration - implements ListenerRegistration { + public static class DataChangeListenerRegistration>> + extends AbstractObjectRegistration implements + org.opendaylight.controller.md.sal.dom.store.impl.DataChangeListenerRegistration { private final DataChangeScope scope; private ListenerRegistrationNode node; + private final InstanceIdentifier path; - public DataChangeListenerRegistration(final T listener, final DataChangeScope scope, final ListenerRegistrationNode node) { + public DataChangeListenerRegistration(final InstanceIdentifier path,final T listener, final DataChangeScope scope, + final ListenerRegistrationNode node) { super(listener); - + this.path = path; this.scope = scope; this.node = node; } - protected DataChangeScope getScope() { + @Override + public DataChangeScope getScope() { return scope; } @@ -117,5 +137,10 @@ public class ListenerRegistrationNode implements StoreTreeNode