From: Tomas Olvecky Date: Wed, 23 Oct 2013 16:27:26 +0000 (+0200) Subject: Implement consistency barrier in yang-store-impl. X-Git-Tag: jenkins-controller-bulk-release-prepare-only-2-1~582 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=f6c67b9b23b31b59bc2ca351365b890377db90fa Implement consistency barrier in yang-store-impl. Fix possible exceptions occurring on startup that are caused by OSGi container notifying about added bundles in wrong order. Change-Id: I3dae0447a775b6fd03c9d02ac0471bad82e0f2fb Signed-off-by: Tomas Olvecky --- diff --git a/opendaylight/config/yang-store-impl/src/main/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTracker.java b/opendaylight/config/yang-store-impl/src/main/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTracker.java deleted file mode 100644 index ee2864878d..0000000000 --- a/opendaylight/config/yang-store-impl/src/main/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTracker.java +++ /dev/null @@ -1,199 +0,0 @@ -/* - * Copyright (c) 2013 Cisco Systems, Inc. and others. All rights reserved. - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Public License v1.0 which accompanies this distribution, - * and is available at http://www.eclipse.org/legal/epl-v10.html - */ -package org.opendaylight.controller.config.yang.store.impl; - -import java.io.IOException; -import java.io.InputStream; -import java.net.URL; -import java.util.*; - -import org.opendaylight.controller.config.yang.store.api.YangStoreException; -import org.opendaylight.controller.config.yang.store.api.YangStoreListenerRegistration; -import org.opendaylight.controller.config.yang.store.api.YangStoreService; -import org.opendaylight.controller.config.yang.store.api.YangStoreSnapshot; -import org.opendaylight.controller.config.yang.store.spi.YangStoreListener; -import org.osgi.framework.Bundle; -import org.osgi.framework.BundleContext; -import org.osgi.framework.BundleEvent; -import org.osgi.util.tracker.BundleTracker; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Function; -import com.google.common.base.Optional; -import com.google.common.base.Preconditions; -import com.google.common.collect.Collections2; -import com.google.common.collect.HashMultimap; -import com.google.common.collect.Multimap; -import com.google.common.collect.Sets; - -public class ExtenderYangTracker extends BundleTracker implements - YangStoreService { - - private static final Logger logger = LoggerFactory - .getLogger(ExtenderYangTracker.class); - - private final Multimap bundlesToYangURLs = HashMultimap - .create(); - private final YangStoreCache cache = new YangStoreCache(); - private final MbeParser mbeParser; - private final List listeners = new ArrayList<>(); - - public ExtenderYangTracker(BundleContext context) { - this(context, new MbeParser()); - - } - - @VisibleForTesting - ExtenderYangTracker(BundleContext context, MbeParser mbeParser) { - super(context, Bundle.ACTIVE, null); - this.mbeParser = mbeParser; - logger.trace("Registered as extender with context {}", context); - } - - @Override - public Object addingBundle(Bundle bundle, BundleEvent event) { - - // Ignore system bundle: - // system bundle might have config-api on classpath && - // config-api contains yang files => - // system bundle might contain yang files from that bundle - if (bundle.getBundleId() == 0) - return bundle; - - Enumeration yangURLs = bundle.findEntries("META-INF/yang", "*.yang", false); - if (yangURLs != null) { - synchronized (this) { - while (yangURLs.hasMoreElements()) { - URL url = yangURLs.nextElement(); - logger.debug("Bundle {} found yang file {}", bundle, url); - bundlesToYangURLs.put(bundle, url); - } - Collection urls = bundlesToYangURLs.get(bundle); - notifyListeners(urls, true); - } - } - return bundle; - } - - private void notifyListeners(Collection urls, boolean adding) { - if (urls.size() > 0) { - RuntimeException potential = new RuntimeException("Error while notifying listeners"); - for (YangStoreListener listener : listeners) { - try { - if (adding) { - listener.onAddedYangURL(urls); - } else { - listener.onRemovedYangURL(urls); - } - } catch(RuntimeException e) { - potential.addSuppressed(e); - } - } - if (potential.getSuppressed().length > 0) { - throw potential; - } - } - } - - @Override - public synchronized void removedBundle(Bundle bundle, BundleEvent event, Object object) { - Collection urls = bundlesToYangURLs.removeAll(bundle); - if (urls.size() > 0) { - logger.debug("Removed following yang URLs {} because of removed bundle {}", urls, bundle); - notifyListeners(urls, false); - } - } - - @Override - public synchronized YangStoreSnapshot getYangStoreSnapshot() - throws YangStoreException { - Optional yangStoreOpt = cache - .getCachedYangStore(bundlesToYangURLs); - if (yangStoreOpt.isPresent()) { - logger.debug("Returning cached yang store {}", yangStoreOpt.get()); - return yangStoreOpt.get(); - } - - try { - YangStoreSnapshot yangStoreSnapshot = mbeParser - .parseYangFiles(fromUrlsToInputStreams()); - logger.debug( - "{} module entries parsed successfully from {} yang files", - yangStoreSnapshot.countModuleMXBeanEntries(), - bundlesToYangURLs.values().size()); - cache.cacheYangStore(bundlesToYangURLs, yangStoreSnapshot); - - return yangStoreSnapshot; - } catch (RuntimeException e) { - logger.warn( - "Unable to parse yang files, yang files that were picked up so far: {}", - bundlesToYangURLs, e); - throw new YangStoreException("Unable to parse yang files", e); - } - } - - private Collection fromUrlsToInputStreams() { - return Collections2.transform(bundlesToYangURLs.values(), - new Function() { - - @Override - public InputStream apply(URL url) { - try { - return url.openStream(); - } catch (IOException e) { - logger.warn("Unable to open stream from {}", url); - throw new IllegalStateException( - "Unable to open stream from " + url, e); - } - } - }); - } - - @Override - public synchronized YangStoreListenerRegistration registerListener(final YangStoreListener listener) { - listeners.add(listener); - return new YangStoreListenerRegistration() { - @Override - public void close() { - listeners.remove(listener); - } - }; - } - - private static final class YangStoreCache { - - Set cachedUrls; - YangStoreSnapshot cachedYangStoreSnapshot; - - Optional getCachedYangStore( - Multimap bundlesToYangURLs) { - Set urls = setFromMultimapValues(bundlesToYangURLs); - if (cachedUrls != null && cachedUrls.equals(urls)) { - Preconditions.checkState(cachedYangStoreSnapshot != null); - return Optional.of(cachedYangStoreSnapshot); - } - return Optional.absent(); - } - - private static Set setFromMultimapValues( - Multimap bundlesToYangURLs) { - Set urls = Sets.newHashSet(bundlesToYangURLs.values()); - Preconditions.checkState(bundlesToYangURLs.size() == urls.size()); - return urls; - } - - void cacheYangStore(Multimap urls, - YangStoreSnapshot yangStoreSnapshot) { - this.cachedUrls = setFromMultimapValues(urls); - this.cachedYangStoreSnapshot = yangStoreSnapshot; - } - - } -} diff --git a/opendaylight/config/yang-store-impl/src/main/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTrackerCustomizer.java b/opendaylight/config/yang-store-impl/src/main/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTrackerCustomizer.java new file mode 100644 index 0000000000..07ffff6029 --- /dev/null +++ b/opendaylight/config/yang-store-impl/src/main/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTrackerCustomizer.java @@ -0,0 +1,249 @@ +/* + * Copyright (c) 2013 Cisco Systems, Inc. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.controller.config.yang.store.impl; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Function; +import com.google.common.base.Optional; +import com.google.common.base.Preconditions; +import com.google.common.collect.*; +import org.opendaylight.controller.config.yang.store.api.YangStoreException; +import org.opendaylight.controller.config.yang.store.api.YangStoreListenerRegistration; +import org.opendaylight.controller.config.yang.store.api.YangStoreService; +import org.opendaylight.controller.config.yang.store.api.YangStoreSnapshot; +import org.opendaylight.controller.config.yang.store.spi.YangStoreListener; +import org.osgi.framework.Bundle; +import org.osgi.framework.BundleEvent; +import org.osgi.util.tracker.BundleTrackerCustomizer; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.io.InputStream; +import java.net.URL; +import java.util.*; + +/** + * Note on consistency: + * When this bundle is activated after other bundles containing yang files, the resolving order + * is not preserved. We thus maintain two maps, one containing consistent snapshot, other inconsistent. The + * container should eventually send all events and thus making the inconsistent map redundant. + */ +public class ExtenderYangTrackerCustomizer implements BundleTrackerCustomizer, YangStoreService { + + private static final Logger logger = LoggerFactory + .getLogger(ExtenderYangTrackerCustomizer.class); + + private final Multimap consistentBundlesToYangURLs = HashMultimap.create(); + + /* + Map of currently problematic yang files that should get fixed eventually after all events are received. + */ + private final Multimap inconsistentBundlesToYangURLs = HashMultimap.create(); + + private final YangStoreCache cache = new YangStoreCache(); + private final MbeParser mbeParser; + private final List listeners = new ArrayList<>(); + + public ExtenderYangTrackerCustomizer() { + this(new MbeParser()); + + } + + @VisibleForTesting + ExtenderYangTrackerCustomizer(MbeParser mbeParser) { + this.mbeParser = mbeParser; + } + + @Override + public Object addingBundle(Bundle bundle, BundleEvent event) { + + // Ignore system bundle: + // system bundle might have config-api on classpath && + // config-api contains yang files => + // system bundle might contain yang files from that bundle + if (bundle.getBundleId() == 0) + return bundle; + + Enumeration enumeration = bundle.findEntries("META-INF/yang", "*.yang", false); + if (enumeration != null && enumeration.hasMoreElements()) { + synchronized (this) { + List addedURLs = new ArrayList<>(); + while (enumeration.hasMoreElements()) { + URL url = enumeration.nextElement(); + addedURLs.add(url); + } + logger.trace("Bundle {} has event {}, bundle state {}, URLs {}", bundle, event, bundle.getState(), addedURLs); + // test that yang store is consistent + Multimap proposedNewState = HashMultimap.create(consistentBundlesToYangURLs); + proposedNewState.putAll(inconsistentBundlesToYangURLs); + proposedNewState.putAll(bundle, addedURLs); + boolean adding = true; + if (tryToUpdateState(addedURLs, proposedNewState, adding) == false) { + inconsistentBundlesToYangURLs.putAll(bundle, addedURLs); + } + } + } + return bundle; + } + + private synchronized boolean tryToUpdateState(Collection changedURLs, Multimap proposedNewState, boolean adding) { + Preconditions.checkArgument(changedURLs.size() > 0, "No change can occur when no URLs are changed"); + try(YangStoreSnapshot snapshot = createSnapshot(mbeParser, proposedNewState)) { + // consistent state + // merge into + consistentBundlesToYangURLs.clear(); + consistentBundlesToYangURLs.putAll(proposedNewState); + inconsistentBundlesToYangURLs.clear(); + // update cache + updateCache(snapshot); + logger.info("Yang store updated to new consistent state"); + logger.trace("Yang store updated to new consistent state containing {}", consistentBundlesToYangURLs); + + notifyListeners(changedURLs, adding); + return true; + } catch(YangStoreException e) { + // inconsistent state + logger.debug("Yang store is falling back on last consistent state containing {}, inconsistent yang files {}, reason {}", + consistentBundlesToYangURLs, inconsistentBundlesToYangURLs, e.toString()); + return false; + } + } + + private void updateCache(YangStoreSnapshot snapshot) { + cache.cacheYangStore(consistentBundlesToYangURLs, snapshot); + } + + @Override + public void modifiedBundle(Bundle bundle, BundleEvent event, Object object) { + logger.debug("Modified bundle {} {} {}", bundle, event, object); + } + + /** + * Notifiers get only notified when consistent snapshot has changed. + */ + private void notifyListeners(Collection changedURLs, boolean adding) { + Preconditions.checkArgument(changedURLs.size() > 0, "Cannot notify when no URLs changed"); + if (changedURLs.size() > 0) { + RuntimeException potential = new RuntimeException("Error while notifying listeners"); + for (YangStoreListener listener : listeners) { + try { + if (adding) { + listener.onAddedYangURL(changedURLs); + } else { + listener.onRemovedYangURL(changedURLs); + } + } catch(RuntimeException e) { + potential.addSuppressed(e); + } + } + if (potential.getSuppressed().length > 0) { + throw potential; + } + } + } + + + /** + * If removing YANG files makes yang store inconsistent, method {@link #getYangStoreSnapshot()} + * will throw exception. There is no rollback. + */ + @Override + public synchronized void removedBundle(Bundle bundle, BundleEvent event, Object object) { + inconsistentBundlesToYangURLs.removeAll(bundle); + Collection consistentURLsToBeRemoved = consistentBundlesToYangURLs.removeAll(bundle); + + if (consistentURLsToBeRemoved.isEmpty()){ + return; // no change + } + boolean adding = false; + notifyListeners(consistentURLsToBeRemoved, adding); + } + + @Override + public synchronized YangStoreSnapshot getYangStoreSnapshot() + throws YangStoreException { + Optional yangStoreOpt = cache.getCachedYangStore(consistentBundlesToYangURLs); + if (yangStoreOpt.isPresent()) { + logger.trace("Returning cached yang store {}", yangStoreOpt.get()); + return yangStoreOpt.get(); + } + YangStoreSnapshot snapshot = createSnapshot(mbeParser, consistentBundlesToYangURLs); + updateCache(snapshot); + return snapshot; + } + + private static YangStoreSnapshot createSnapshot(MbeParser mbeParser, Multimap multimap) throws YangStoreException { + try { + YangStoreSnapshot yangStoreSnapshot = mbeParser.parseYangFiles(fromUrlsToInputStreams(multimap)); + logger.trace("{} module entries parsed successfully from {} yang files", + yangStoreSnapshot.countModuleMXBeanEntries(), multimap.values().size()); + return yangStoreSnapshot; + } catch (RuntimeException e) { + throw new YangStoreException("Unable to parse yang files from following URLs: " + multimap, e); + } + } + + private static Collection fromUrlsToInputStreams(Multimap multimap) { + return Collections2.transform(multimap.values(), + new Function() { + + @Override + public InputStream apply(URL url) { + try { + return url.openStream(); + } catch (IOException e) { + logger.warn("Unable to open stream from {}", url); + throw new IllegalStateException( + "Unable to open stream from " + url, e); + } + } + }); + } + + @Override + public synchronized YangStoreListenerRegistration registerListener(final YangStoreListener listener) { + listeners.add(listener); + return new YangStoreListenerRegistration() { + @Override + public void close() { + listeners.remove(listener); + } + }; + } + + private static final class YangStoreCache { + + Set cachedUrls; + YangStoreSnapshot cachedYangStoreSnapshot; + + Optional getCachedYangStore( + Multimap bundlesToYangURLs) { + Set urls = setFromMultimapValues(bundlesToYangURLs); + if (cachedUrls != null && cachedUrls.equals(urls)) { + Preconditions.checkState(cachedYangStoreSnapshot != null); + return Optional.of(cachedYangStoreSnapshot); + } + return Optional.absent(); + } + + private static Set setFromMultimapValues( + Multimap bundlesToYangURLs) { + Set urls = Sets.newHashSet(bundlesToYangURLs.values()); + Preconditions.checkState(bundlesToYangURLs.size() == urls.size()); + return urls; + } + + void cacheYangStore(Multimap urls, + YangStoreSnapshot yangStoreSnapshot) { + this.cachedUrls = setFromMultimapValues(urls); + this.cachedYangStoreSnapshot = yangStoreSnapshot; + } + + } +} diff --git a/opendaylight/config/yang-store-impl/src/main/java/org/opendaylight/controller/config/yang/store/impl/YangStoreActivator.java b/opendaylight/config/yang-store-impl/src/main/java/org/opendaylight/controller/config/yang/store/impl/YangStoreActivator.java index 2331fd15a8..a358e5f7c1 100644 --- a/opendaylight/config/yang-store-impl/src/main/java/org/opendaylight/controller/config/yang/store/impl/YangStoreActivator.java +++ b/opendaylight/config/yang-store-impl/src/main/java/org/opendaylight/controller/config/yang/store/impl/YangStoreActivator.java @@ -13,41 +13,44 @@ import java.util.Hashtable; import org.opendaylight.controller.config.yang.store.api.YangStoreService; import org.osgi.framework.BundleActivator; import org.osgi.framework.BundleContext; +import org.osgi.framework.BundleEvent; import org.osgi.framework.ServiceRegistration; +import org.osgi.util.tracker.BundleTracker; import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class YangStoreActivator implements BundleActivator { - private ExtenderYangTracker extender; + private BundleTracker bundleTracker; private ServiceRegistration registration; private static final Logger logger = LoggerFactory .getLogger(YangStoreActivator.class); @Override public void start(BundleContext context) throws Exception { - extender = new ExtenderYangTracker(context); - extender.open(); + ExtenderYangTrackerCustomizer customizerAndService = new ExtenderYangTrackerCustomizer(); + bundleTracker = new BundleTracker(context, BundleEvent.RESOLVED | BundleEvent.UNRESOLVED, customizerAndService); + bundleTracker.open(); Dictionary properties = new Hashtable<>(); registration = context.registerService(YangStoreService.class, - extender, properties); + customizerAndService, properties); } @Override public void stop(BundleContext context) throws Exception { try { - extender.close(); + bundleTracker.close(); } catch (Exception e) { - logger.warn("Exception while closing extender", e); + logger.warn("Exception while closing bundleTracker", e); } - - if (registration != null) + if (registration != null) { try { registration.unregister(); } catch (Exception e) { logger.warn("Exception while unregistring yang store service", e); } + } } } diff --git a/opendaylight/config/yang-store-impl/src/test/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTrackerTest.java b/opendaylight/config/yang-store-impl/src/test/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTrackerCustomizerTest.java similarity index 87% rename from opendaylight/config/yang-store-impl/src/test/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTrackerTest.java rename to opendaylight/config/yang-store-impl/src/test/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTrackerCustomizerTest.java index e40d7348a0..f04dcc4abf 100644 --- a/opendaylight/config/yang-store-impl/src/test/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTrackerTest.java +++ b/opendaylight/config/yang-store-impl/src/test/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTrackerCustomizerTest.java @@ -9,11 +9,7 @@ package org.opendaylight.controller.config.yang.store.impl; import static org.junit.Assert.assertEquals; import static org.mockito.Matchers.anyCollectionOf; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.*; import java.io.InputStream; import java.net.MalformedURLException; @@ -32,11 +28,10 @@ import org.osgi.framework.BundleContext; import com.google.common.collect.Lists; -public class ExtenderYangTrackerTest { +public class ExtenderYangTrackerCustomizerTest { - @Mock - private BundleContext context; - private ExtenderYangTracker tested; + + private ExtenderYangTrackerCustomizer tested; @Mock private MbeParser parser; @Mock @@ -45,12 +40,13 @@ public class ExtenderYangTrackerTest { @Before public void setUp() throws YangStoreException { MockitoAnnotations.initMocks(this); - doReturn("context").when(context).toString(); - tested = new ExtenderYangTracker(context, parser); + + tested = new ExtenderYangTrackerCustomizer(parser); doReturn(yangStoreSnapshot).when(parser).parseYangFiles( anyCollectionOf(InputStream.class)); doReturn(22).when(yangStoreSnapshot).countModuleMXBeanEntries(); doReturn("mock yang store").when(yangStoreSnapshot).toString(); + doNothing().when(yangStoreSnapshot).close(); } @Test @@ -74,9 +70,11 @@ public class ExtenderYangTrackerTest { bundle = getMockedBundle(10, false); tested.addingBundle(bundle, null); - tested.getYangStoreSnapshot(); + for(int i = 0; i< 10; i++){ + tested.getYangStoreSnapshot(); + } - verify(parser, times(3)).parseYangFiles( + verify(parser, times(5)).parseYangFiles( anyCollectionOf(InputStream.class)); returnedStore = tested.getYangStoreSnapshot(); @@ -90,6 +88,7 @@ public class ExtenderYangTrackerTest { private Bundle getMockedBundle(int sizeOfUrls, boolean system) throws MalformedURLException { Bundle mock = mock(Bundle.class); + doReturn(32).when(mock).getState();//mock just for logging List urls = Lists.newArrayList(); for (int i = 0; i < sizeOfUrls; i++) {