zeppelin-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From m...@apache.org
Subject zeppelin git commit: [ZEPPELIN-2103] Unnecessary read to Helium registry
Date Thu, 23 Feb 2017 02:31:30 GMT
Repository: zeppelin
Updated Branches:
  refs/heads/master 99b90effd -> ac1e73c50


[ZEPPELIN-2103] Unnecessary read to Helium registry

### What is this PR for?
Every `Helium.getAllPackageInfo()` call read helium package information from all registry
configured (both local registry, online registry by default).
Problem is, `Helium.getAllPackageInfo()` is used inside of many other methods. like `Helium.suggestApp()`,
`Helium.enable()`, `Helium.disable()`, `Helium.recreateBundle()`, `Helium.getPackageInfo()`.
So local/remote registry is unnecessarily accessed more than it should do.

### What type of PR is it?
Bug Fix

### Todos
* [x] - Hold the result and reuse it
* [x] - Unit test

### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-2103

### How should this be tested?
Unittest included

### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no

Author: Lee moon soo <moon@apache.org>

Closes #2015 from Leemoonsoo/ZEPPELIN-2103 and squashes the following commits:

57d19f7 [Lee moon soo] Hold package info and reuse unless refresh flag set to true


Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/ac1e73c5
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/ac1e73c5
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/ac1e73c5

Branch: refs/heads/master
Commit: ac1e73c5053c3c764cd3e82047368b1988d9d911
Parents: 99b90ef
Author: Lee moon soo <moon@apache.org>
Authored: Mon Feb 13 19:59:30 2017 +0900
Committer: Lee moon soo <moon@apache.org>
Committed: Thu Feb 23 11:31:14 2017 +0900

----------------------------------------------------------------------
 .../java/org/apache/zeppelin/helium/Helium.java | 79 +++++++++++++-------
 .../org/apache/zeppelin/helium/HeliumTest.java  | 39 ++++++++++
 2 files changed, 90 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/ac1e73c5/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/Helium.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/Helium.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/Helium.java
index e2e1b49..918e9aa 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/Helium.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/helium/Helium.java
@@ -48,6 +48,8 @@ public class Helium {
   private final HeliumBundleFactory bundleFactory;
   private final HeliumApplicationFactory applicationFactory;
 
+  Map<String, List<HeliumPackageSearchResult>> allPackages;
+
   public Helium(
       String heliumConfPath,
       String registryPaths,
@@ -142,7 +144,7 @@ public class Helium {
   }
 
   private void clearNotExistsPackages() {
-    Map<String, List<HeliumPackageSearchResult>> all = getAllPackageInfo();
+    Map<String, List<HeliumPackageSearchResult>> all = getAllPackageInfo(false);
 
     // clear visualization display order
     List<String> packageOrder = heliumConf.getBundleDisplayOrder();
@@ -164,43 +166,64 @@ public class Helium {
   }
 
   public Map<String, List<HeliumPackageSearchResult>> getAllPackageInfo() {
+    return getAllPackageInfo(true);
+  }
+
+  public Map<String, List<HeliumPackageSearchResult>> getAllPackageInfo(boolean
refresh) {
     Map<String, String> enabledPackageInfo = heliumConf.getEnabledPackages();
 
-    Map<String, List<HeliumPackageSearchResult>> map = new HashMap<>();
     synchronized (registry) {
-      for (HeliumRegistry r : registry) {
-        try {
-          for (HeliumPackage pkg : r.getAll()) {
-            String name = pkg.getName();
-            String artifact = enabledPackageInfo.get(name);
-            boolean enabled = (artifact != null && artifact.equals(pkg.getArtifact()));
-
-            if (!map.containsKey(name)) {
-              map.put(name, new LinkedList<HeliumPackageSearchResult>());
+      if (refresh || allPackages == null) {
+        allPackages = new HashMap<>();
+        for (HeliumRegistry r : registry) {
+          try {
+            for (HeliumPackage pkg : r.getAll()) {
+              String name = pkg.getName();
+              String artifact = enabledPackageInfo.get(name);
+              boolean enabled = (artifact != null && artifact.equals(pkg.getArtifact()));
+
+              if (!allPackages.containsKey(name)) {
+                allPackages.put(name, new LinkedList<HeliumPackageSearchResult>());
+              }
+              allPackages.get(name).add(new HeliumPackageSearchResult(r.name(), pkg, enabled));
             }
-            map.get(name).add(new HeliumPackageSearchResult(r.name(), pkg, enabled));
+          } catch (IOException e) {
+            logger.error(e.getMessage(), e);
           }
-        } catch (IOException e) {
-          logger.error(e.getMessage(), e);
         }
-      }
-    }
+      } else {
 
-    // sort version (artifact)
-    for (String name : map.keySet()) {
-      List<HeliumPackageSearchResult> packages = map.get(name);
-      Collections.sort(packages, new Comparator<HeliumPackageSearchResult>() {
-        @Override
-        public int compare(HeliumPackageSearchResult o1, HeliumPackageSearchResult o2) {
-          return o2.getPkg().getArtifact().compareTo(o1.getPkg().getArtifact());
+        for (String name : allPackages.keySet()) {
+          List<HeliumPackageSearchResult> pkgs = allPackages.get(name);
+          String artifact = enabledPackageInfo.get(name);
+          LinkedList<HeliumPackageSearchResult> newResults =
+              new LinkedList<HeliumPackageSearchResult>();
+
+          for (HeliumPackageSearchResult pkg : pkgs) {
+            boolean enabled = (artifact != null && artifact.equals(pkg.getPkg().getArtifact()));
+            newResults.add(new HeliumPackageSearchResult(pkg.getRegistry(), pkg.getPkg(),
enabled));
+          }
+
+          allPackages.put(name, newResults);
         }
-      });
+      }
+
+      // sort version (artifact)
+      for (String name : allPackages.keySet()) {
+        List<HeliumPackageSearchResult> packages = allPackages.get(name);
+        Collections.sort(packages, new Comparator<HeliumPackageSearchResult>() {
+          @Override
+          public int compare(HeliumPackageSearchResult o1, HeliumPackageSearchResult o2)
{
+            return o2.getPkg().getArtifact().compareTo(o1.getPkg().getArtifact());
+          }
+        });
+      }
+      return allPackages;
     }
-    return map;
   }
 
   public HeliumPackageSearchResult getPackageInfo(String name, String artifact) {
-    Map<String, List<HeliumPackageSearchResult>> infos = getAllPackageInfo();
+    Map<String, List<HeliumPackageSearchResult>> infos = getAllPackageInfo(false);
     List<HeliumPackageSearchResult> packages = infos.get(name);
     if (artifact == null) {
       return packages.get(0);
@@ -276,7 +299,7 @@ public class Helium {
       allResources = ResourcePoolUtils.getAllResources();
     }
 
-    for (List<HeliumPackageSearchResult> pkgs : getAllPackageInfo().values()) {
+    for (List<HeliumPackageSearchResult> pkgs : getAllPackageInfo(false).values())
{
       for (HeliumPackageSearchResult pkg : pkgs) {
         if (pkg.getPkg().getType() == HeliumType.APPLICATION && pkg.isEnabled())
{
           ResourceSet resources = ApplicationLoader.findRequiredResourceSet(
@@ -304,7 +327,7 @@ public class Helium {
    * @return ordered list of enabled buildBundle package
    */
   public List<HeliumPackage> getBundlePackagesToBundle() {
-    Map<String, List<HeliumPackageSearchResult>> allPackages = getAllPackageInfo();
+    Map<String, List<HeliumPackageSearchResult>> allPackages = getAllPackageInfo(false);
     List<String> visOrder = heliumConf.getBundleDisplayOrder();
 
     List<HeliumPackage> orderedBundlePackages = new LinkedList<>();

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/ac1e73c5/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumTest.java
index 1607c2c..9301c18 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumTest.java
@@ -100,4 +100,43 @@ public class HeliumTest {
     // then
     assertEquals(2, helium.getAllPackageInfo().size());
   }
+
+
+  @Test
+  public void testRefresh() throws IOException, URISyntaxException, TaskRunnerException {
+    File heliumConf = new File(tmpDir, "helium.conf");
+    Helium helium = new Helium(
+        heliumConf.getAbsolutePath(), localRegistryPath.getAbsolutePath(), null, null, null);
+    HeliumTestRegistry registry1 = new HeliumTestRegistry("r1", "r1");
+    helium.addRegistry(registry1);
+
+    // when
+    registry1.add(new HeliumPackage(
+        HeliumType.APPLICATION,
+        "name1",
+        "desc1",
+        "artifact1",
+        "className1",
+        new String[][]{},
+        "",
+        ""));
+
+    // then
+    assertEquals(1, helium.getAllPackageInfo(false).size());
+
+    // when
+    registry1.add(new HeliumPackage(
+        HeliumType.APPLICATION,
+        "name2",
+        "desc2",
+        "artifact2",
+        "className2",
+        new String[][]{},
+        "",
+        ""));
+
+    // then
+    assertEquals(1, helium.getAllPackageInfo(false).size());
+    assertEquals(2, helium.getAllPackageInfo(true).size());
+  }
 }


Mime
View raw message