You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2014/07/04 11:51:03 UTC

[21/45] git commit: BROOKLYN-13: Registers bundles when catalogue is loaded

BROOKLYN-13: Registers bundles when catalogue is loaded

load() method added to CatalogDo to register bundles with management context.

BasicBrooklynCatalog altered to not auto-load in another thread because it caused
a deadlock (management context locked, claims lock for catalogDo.load. BBC
constructor runs task in thread that claims catalogDo.load lock, then tries to claim
management context lock when getting the OsgiManager)


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/7fec791f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/7fec791f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/7fec791f

Branch: refs/heads/master
Commit: 7fec791f01b0efc6f29d53550fb433d030fae34e
Parents: 3a2f680
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Authored: Wed Jul 2 12:10:11 2014 +0100
Committer: Sam Corbett <sa...@cloudsoftcorp.com>
Committed: Wed Jul 2 12:10:11 2014 +0100

----------------------------------------------------------------------
 .../catalog/internal/BasicBrooklynCatalog.java  | 24 +++++-----
 .../catalog/internal/CatalogClasspathDo.java    | 48 +++++++++-----------
 .../brooklyn/catalog/internal/CatalogDo.java    | 26 ++++++++---
 .../internal/CatalogItemDtoAbstract.java        |  4 ++
 .../catalog/internal/CatalogLibrariesDo.java    | 35 ++++++++++++++
 .../catalog/internal/CatalogLibrariesDto.java   |  7 ++-
 .../brooklyn/management/ha/OsgiManager.java     | 10 ++--
 .../internal/AbstractManagementContext.java     |  2 +-
 .../internal/ManagementContextInternal.java     |  6 ++-
 .../src/main/java/brooklyn/util/osgi/Osgis.java | 15 ++++--
 10 files changed, 120 insertions(+), 57 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7fec791f/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java b/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
index 4db16bd..7f5f305 100644
--- a/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
+++ b/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
@@ -41,17 +41,8 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
     public BasicBrooklynCatalog(final ManagementContext mgmt, final CatalogDto dto) {
         this.mgmt = Preconditions.checkNotNull(mgmt, "managementContext");
         this.catalog = new CatalogDo(mgmt, dto);
-        
-        mgmt.getExecutionManager().submit(MutableMap.of("name", "loading catalog"), new Runnable() {
-            public void run() {
-                log.debug("Loading catalog for "+mgmt);
-                catalog.load(mgmt, null);
-                if (log.isDebugEnabled())
-                    log.debug("Loaded catalog for "+mgmt+": "+catalog+"; search classpath is "+catalog.getRootClassLoader());
-            }
-        });
     }
-    
+
     public boolean blockIfNotLoaded(Duration timeout) {
         try {
             return getCatalog().blockIfNotLoaded(timeout);
@@ -89,7 +80,18 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
     public ClassLoader getRootClassLoader() {
         return catalog.getRootClassLoader();
     }
-    
+
+    /**
+     * Loads this catalog
+     */
+    public void load() {
+        log.debug("Loading catalog for " + mgmt);
+        getCatalog().load(mgmt, null);
+        if (log.isDebugEnabled()) {
+            log.debug("Loaded catalog for " + mgmt + ": " + catalog + "; search classpath is " + catalog.getRootClassLoader());
+        }
+    }
+
     @SuppressWarnings("unchecked")
     @Override
     public <T> Class<? extends T> loadClass(CatalogItem<T> item) {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7fec791f/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java b/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
index ffe6e9c..de5a7fb 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
@@ -87,10 +87,11 @@ public class CatalogClasspathDo {
     /** causes all scanning-based classpaths to scan the classpaths
     * (but does _not_ load all JARs) */
     synchronized void load() {
-        if (classpath==null) return;
+        if (classpath == null || isLoaded) return;
 
-        if (classpath.getEntries()==null) urls = new URL[0];
-        else {
+        if (classpath.getEntries() == null) {
+            urls = new URL[0];
+        } else {
             urls = new URL[classpath.getEntries().size()];
             for (int i=0; i<urls.length; i++) {
                 try {
@@ -114,34 +115,29 @@ public class CatalogClasspathDo {
         ReflectionScanner scanner = null;
         if (!catalog.isLocal()) {
             log.warn("Scanning not supported for remote catalogs; ignoring scan request in "+catalog);
-        } else if (classpath.getEntries()==null || classpath.getEntries().isEmpty()) {
-            // no entries; check if we are local, and if so scan the default classpath
-            if (!catalog.isLocal()) {
-                log.warn("Scanning not supported for remote catalogs; ignoring scan request in "+catalog);
-            } else {                
-                // scan default classpath:
-                ClassLoader baseCL = null;
-                Iterable<URL> baseCP = null;
-                if (catalog.mgmt instanceof ManagementContextInternal) {
-                    baseCL = ((ManagementContextInternal)catalog.mgmt).getBaseClassLoader();
+        } else if (classpath.getEntries() == null || classpath.getEntries().isEmpty()) {
+            // scan default classpath:
+            ClassLoader baseCL = null;
+            Iterable<URL> baseCP = null;
+            if (catalog.mgmt instanceof ManagementContextInternal) {
+                baseCL = ((ManagementContextInternal)catalog.mgmt).getBaseClassLoader();
+                baseCP = ((ManagementContextInternal)catalog.mgmt).getBaseClassPathForScanning();
+            }
+            scanner = new ReflectionScanner(baseCP, prefix, baseCL, catalog.getRootClassLoader());
+            if (scanner.getSubTypesOf(Entity.class).isEmpty()) {
+                try {
+                    ((ManagementContextInternal)catalog.mgmt).setBaseClassPathForScanning(ClasspathHelper.forJavaClassPath());
+                    log.debug("Catalog scan of default classloader returned nothing; reverting to java.class.path");
                     baseCP = ((ManagementContextInternal)catalog.mgmt).getBaseClassPathForScanning();
-                }
-                scanner = new ReflectionScanner(baseCP, prefix, baseCL, catalog.getRootClassLoader());
-                if (scanner.getSubTypesOf(Entity.class).isEmpty()) {
-                    try {
-                        ((ManagementContextInternal)catalog.mgmt).setBaseClassPathForScanning(ClasspathHelper.forJavaClassPath());
-                        log.debug("Catalog scan of default classloader returned nothing; reverting to java.class.path");
-                        baseCP = ((ManagementContextInternal)catalog.mgmt).getBaseClassPathForScanning();
-                        scanner = new ReflectionScanner(baseCP, prefix, baseCL, catalog.getRootClassLoader());
-                    } catch (Exception e) {
-                        log.info("Catalog scan is empty, and unable to use java.class.path (base classpath is "+baseCP+")");
-                        Exceptions.propagateIfFatal(e);
-                    }
+                    scanner = new ReflectionScanner(baseCP, prefix, baseCL, catalog.getRootClassLoader());
+                } catch (Exception e) {
+                    log.info("Catalog scan is empty, and unable to use java.class.path (base classpath is "+baseCP+")");
+                    Exceptions.propagateIfFatal(e);
                 }
             }
         } else {
             // scan specified jars:
-            scanner = new ReflectionScanner(urls==null || urls.length==0 ? null : Arrays.asList(urls), prefix, getLocalClassLoader()); 
+            scanner = new ReflectionScanner(urls==null || urls.length==0 ? null : Arrays.asList(urls), prefix, getLocalClassLoader());
         }
         
         if (scanner!=null) {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7fec791f/core/src/main/java/brooklyn/catalog/internal/CatalogDo.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogDo.java b/core/src/main/java/brooklyn/catalog/internal/CatalogDo.java
index c478e59..b8759a4 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogDo.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogDo.java
@@ -5,7 +5,6 @@ import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.TimeoutException;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -20,6 +19,7 @@ import brooklyn.util.time.CountdownTimer;
 import brooklyn.util.time.Duration;
 
 import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
 
 public class CatalogDo {
 
@@ -61,7 +61,7 @@ public class CatalogDo {
         getCache();
         return this;
     }
-    
+
     protected synchronized void loadThisCatalog(ManagementContext mgmt, CatalogDo parent) {
         if (isLoaded()) return;
         if (this.parent!=null && !this.parent.equals(parent))
@@ -70,8 +70,17 @@ public class CatalogDo {
             log.warn("Catalog "+this+" being initialised with different mgmt "+mgmt+" when already managed by "+this.mgmt, new Throwable("source of reparented "+this));
         this.parent = parent;
         this.mgmt = mgmt;
+        dto.populateFromUrl();
+        loadCatalogClasspath();
+        loadCatalogItems();
+        isLoaded = true;
+        synchronized (this) {
+            notifyAll();
+        }
+    }
+
+    private void loadCatalogClasspath() {
         try {
-            dto.populateFromUrl();
             classpath = new CatalogClasspathDo(this);
             classpath.load();
         } catch (Exception e) {
@@ -79,9 +88,14 @@ public class CatalogDo {
             log.error("Unable to load catalog "+this+" (ignoring): "+e);
             log.info("Trace for failure to load "+this+": "+e, e);
         }
-        isLoaded = true;
-        synchronized (this) {
-            notifyAll();
+    }
+
+    private void loadCatalogItems() {
+        List<CatalogLibrariesDo> loadedLibraries = Lists.newLinkedList();
+        for (CatalogItemDtoAbstract entry : dto.entries) {
+            CatalogLibrariesDo library = new CatalogLibrariesDo(entry.getLibrariesDto());
+            library.load(mgmt);
+            loadedLibraries.add(library);
         }
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7fec791f/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java b/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java
index dcee2fa..d834911 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java
@@ -42,6 +42,10 @@ public abstract class CatalogItemDtoAbstract<T> implements CatalogItem<T> {
     @Nonnull
     @Override
     public CatalogItemLibraries getLibraries() {
+        return getLibrariesDto();
+    }
+
+    public CatalogLibrariesDto getLibrariesDto() {
         return libraries;
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7fec791f/core/src/main/java/brooklyn/catalog/internal/CatalogLibrariesDo.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogLibrariesDo.java b/core/src/main/java/brooklyn/catalog/internal/CatalogLibrariesDo.java
index 04a8f80..86295a1 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogLibrariesDo.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogLibrariesDo.java
@@ -2,14 +2,27 @@ package brooklyn.catalog.internal;
 
 import java.util.List;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Stopwatch;
 
 import brooklyn.catalog.CatalogItem;
+import brooklyn.management.ManagementContext;
+import brooklyn.management.ha.OsgiManager;
+import brooklyn.management.internal.ManagementContextInternal;
+import brooklyn.util.guava.Maybe;
+import brooklyn.util.text.Strings;
+import brooklyn.util.time.Time;
 
 public class CatalogLibrariesDo implements CatalogItem.CatalogItemLibraries {
 
+    private static final Logger LOG = LoggerFactory.getLogger(CatalogLibrariesDo.class);
     private final CatalogLibrariesDto librariesDto;
 
+
     public CatalogLibrariesDo(CatalogLibrariesDto librariesDto) {
         this.librariesDto = Preconditions.checkNotNull(librariesDto, "librariesDto");
     }
@@ -19,4 +32,26 @@ public class CatalogLibrariesDo implements CatalogItem.CatalogItemLibraries {
         return librariesDto.getBundles();
     }
 
+    /**
+     * Registers all bundles with the management context's OSGi framework.
+     */
+    void load(ManagementContext managementContext) {
+        ManagementContextInternal mgmt = (ManagementContextInternal) managementContext;
+        Maybe<OsgiManager> osgi = mgmt.getOsgiManager();
+        List<String> bundles = getBundles();
+        if (osgi.isAbsent()) {
+            LOG.warn("{} not loading bundles in {} because osgi manager is unavailable. Bundles: {}",
+                    new Object[]{this, managementContext, Joiner.on(", ").join(bundles)});
+            return;
+        } else if (LOG.isDebugEnabled()) {
+            LOG.debug("{} loading bundles in {}: {}",
+                    new Object[]{this, managementContext, Joiner.on(", ").join(bundles)});
+        }
+        Stopwatch timer = Stopwatch.createStarted();
+        for (String bundleUrl : bundles) {
+            osgi.get().registerBundle(bundleUrl);
+        }
+        LOG.debug("Catalog registered {} bundles in {}", bundles.size(), Time.makeTimeStringRounded(timer));
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7fec791f/core/src/main/java/brooklyn/catalog/internal/CatalogLibrariesDto.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogLibrariesDto.java b/core/src/main/java/brooklyn/catalog/internal/CatalogLibrariesDto.java
index 2a49cfd..3c05387 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogLibrariesDto.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogLibrariesDto.java
@@ -3,18 +3,21 @@ package brooklyn.catalog.internal;
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
 
+import com.google.common.collect.ImmutableList;
+
 import brooklyn.catalog.CatalogItem;
 
 public class CatalogLibrariesDto implements CatalogItem.CatalogItemLibraries {
 
-    List<String> bundles = new CopyOnWriteArrayList<String>();
+    private List<String> bundles = new CopyOnWriteArrayList<String>();
 
     public void addBundle(String url) {
         bundles.add(url);
     }
 
+    /** @return An immutable copy of the bundle URLs referenced by this object */
     public List<String> getBundles() {
-        return bundles;
+        return ImmutableList.copyOf(bundles);
     }
 
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7fec791f/core/src/main/java/brooklyn/management/ha/OsgiManager.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/ha/OsgiManager.java b/core/src/main/java/brooklyn/management/ha/OsgiManager.java
index d519af5..a968660 100644
--- a/core/src/main/java/brooklyn/management/ha/OsgiManager.java
+++ b/core/src/main/java/brooklyn/management/ha/OsgiManager.java
@@ -49,12 +49,14 @@ public class OsgiManager {
         framework = null;
     }
 
-    // TODO: throws BundleException appropriate?
-    public void registerBundle(String bundleUrl) throws BundleException {
-        Osgis.install(framework, bundleUrl);
+    public void registerBundle(String bundleUrl) {
+        try {
+            Osgis.install(framework, bundleUrl);
+        } catch (BundleException e) {
+            throw Throwables.propagate(e);
+        }
     }
 
-    // TODO: Handle .get failing
     public <T> Maybe<Class<T>> tryResolveClass(String bundleUrl, String type) {
         try {
             Maybe<Bundle> bundle = Osgis.getBundle(framework, bundleUrl);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7fec791f/core/src/main/java/brooklyn/management/internal/AbstractManagementContext.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/internal/AbstractManagementContext.java b/core/src/main/java/brooklyn/management/internal/AbstractManagementContext.java
index bb9db2e..ae94534 100644
--- a/core/src/main/java/brooklyn/management/internal/AbstractManagementContext.java
+++ b/core/src/main/java/brooklyn/management/internal/AbstractManagementContext.java
@@ -334,7 +334,7 @@ public abstract class AbstractManagementContext implements ManagementContextInte
             if (log.isDebugEnabled())
                 log.debug("Loaded default (local classpath) catalog: "+catalog);
         }
-        catalog.getCatalog().load(this, null);
+        catalog.load();
         
         this.catalog = catalog;
     }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7fec791f/core/src/main/java/brooklyn/management/internal/ManagementContextInternal.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/internal/ManagementContextInternal.java b/core/src/main/java/brooklyn/management/internal/ManagementContextInternal.java
index 4707a7f..64c5b5e 100644
--- a/core/src/main/java/brooklyn/management/internal/ManagementContextInternal.java
+++ b/core/src/main/java/brooklyn/management/internal/ManagementContextInternal.java
@@ -60,8 +60,10 @@ public interface ManagementContextInternal extends ManagementContext {
 
     UsageManager getUsageManager();
     
-    /** returns OSGi manager, if available; may be absent if OSGi not supported, e.g. in test contexts
-     * (but major contexts will support this) */
+    /**
+     * @return The OSGi manager, if available; may be absent if OSGi is not supported,
+     * e.g. in test contexts (but will be supported in all major contexts).
+     */
     Maybe<OsgiManager> getOsgiManager();
 
     InternalEntityFactory getEntityFactory();

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7fec791f/core/src/main/java/brooklyn/util/osgi/Osgis.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/util/osgi/Osgis.java b/core/src/main/java/brooklyn/util/osgi/Osgis.java
index 4c6e351..05fac8e 100644
--- a/core/src/main/java/brooklyn/util/osgi/Osgis.java
+++ b/core/src/main/java/brooklyn/util/osgi/Osgis.java
@@ -91,7 +91,7 @@ public class Osgis {
     
     /*
      * loading framework factory and starting framework based on:
-     * http://felix.apache.org/documentation/subprojects/apache-felix-framework/apache-felix-framework-launching-and-embedding.html :
+     * http://felix.apache.org/documentation/subprojects/apache-felix-framework/apache-felix-framework-launching-and-embedding.html
      */
     
     public static FrameworkFactory newFrameworkFactory() {
@@ -138,13 +138,18 @@ public class Osgis {
         return framework;
     }
 
-    /** install a bundle from the given URL, doing a check if already installed, and
-     * using the {@link ResourceUtils} loader for this project (brooklyn core) */
+    /**
+     * Installs a bundle from the given URL, doing a check if already installed, and
+     * using the {@link ResourceUtils} loader for this project (brooklyn core)
+     */
     public static Bundle install(Framework framework, String url) throws BundleException {
         Bundle bundle = framework.getBundleContext().getBundle(url);
-        if (bundle != null) return bundle;
-        
+        if (bundle != null) {
+            return bundle;
+        }
+
         // use our URL resolution so we get classpath items
+        LOG.debug("Installing bundle into {} from url: {}", framework, url);
         InputStream stream = ResourceUtils.create(Osgis.class).getResourceFromUrl(url);
         return framework.getBundleContext().installBundle(url, stream);
     }