You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by mo...@apache.org on 2017/02/23 02:31:30 UTC
zeppelin git commit: [ZEPPELIN-2103] Unnecessary read to Helium
registry
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 <mo...@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 <mo...@apache.org>
Authored: Mon Feb 13 19:59:30 2017 +0900
Committer: Lee moon soo <mo...@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());
+ }
}