Repository: zeppelin Updated Branches: refs/heads/master 0f81b6d61 -> 23c5cac82
ZEPPELIN-3029. Cannot delete an interpreter ### What is this PR for? Only copy interpreterSetting from interpreterSettingTemplates when zeppelin is started the first time (interpreter.json doesn't exist) ### What type of PR is it? [Bug Fix] ### Todos * [ ] - Task ### What is the Jira issue? * https://issues.apache.org/jira/browse/ZEPPELIN-3029 ### How should this be tested? * Manually verified. Delete an interpreter, and then restart zeppelin to check. ### Screenshots (if appropriate) ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: Jeff Zhang <zjf...@apache.org> Closes #2648 from zjffdu/ZEPPELIN-3029 and squashes the following commits: 255e607 [Jeff Zhang] Address comments 4865aba [Jeff Zhang] ZEPPELIN-3029. Cannot delete an interpreter Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/23c5cac8 Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/23c5cac8 Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/23c5cac8 Branch: refs/heads/master Commit: 23c5cac8245aea16bfc0568cf2e5c2ae6dfd0e6d Parents: 0f81b6d Author: Jeff Zhang <zjf...@apache.org> Authored: Wed Nov 8 19:39:16 2017 +0800 Committer: Jeff Zhang <zjf...@apache.org> Committed: Fri Nov 10 13:53:09 2017 +0800 ---------------------------------------------------------------------- .../interpreter/ClassloaderInterpreter.java | 261 ------------------- .../interpreter/InterpreterSetting.java | 22 +- .../interpreter/InterpreterSettingManager.java | 50 ++-- .../zeppelin/interpreter/MiniHadoopCluster.java | 1 + .../src/test/resources/conf/interpreter.json | 54 ++++ 5 files changed, 91 insertions(+), 297 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/23c5cac8/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/ClassloaderInterpreter.java ---------------------------------------------------------------------- diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/ClassloaderInterpreter.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/ClassloaderInterpreter.java deleted file mode 100644 index f8afa45..0000000 --- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/ClassloaderInterpreter.java +++ /dev/null @@ -1,261 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.zeppelin.interpreter; - -import java.net.URL; -import java.util.List; -import java.util.Properties; - -import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion; -import org.apache.zeppelin.scheduler.Scheduler; - -/** - * Add to the classpath interpreters. - * - */ -public class ClassloaderInterpreter - extends Interpreter - implements WrappedInterpreter { - - private ClassLoader cl; - private Interpreter intp; - - public ClassloaderInterpreter(Interpreter intp, ClassLoader cl) { - super(new Properties()); - this.cl = cl; - this.intp = intp; - } - - @Override - public Interpreter getInnerInterpreter() { - return intp; - } - - public ClassLoader getClassloader() { - return cl; - } - - @Override - public InterpreterResult interpret(String st, InterpreterContext context) - throws InterpreterException { - ClassLoader oldcl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(cl); - try { - return intp.interpret(st, context); - } catch (InterpreterException e) { - throw e; - } catch (Exception e) { - throw new InterpreterException(e); - } finally { - cl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(oldcl); - } - } - - - @Override - public void open() throws InterpreterException { - ClassLoader oldcl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(cl); - try { - intp.open(); - } catch (Exception e) { - throw new InterpreterException(e); - } finally { - cl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(oldcl); - } - } - - @Override - public void close() throws InterpreterException { - ClassLoader oldcl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(cl); - try { - intp.close(); - } catch (Exception e) { - throw new InterpreterException(e); - } finally { - cl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(oldcl); - } - } - - @Override - public void cancel(InterpreterContext context) throws InterpreterException { - ClassLoader oldcl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(cl); - try { - intp.cancel(context); - } catch (Exception e) { - throw new InterpreterException(e); - } finally { - cl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(oldcl); - } - } - - @Override - public FormType getFormType() throws InterpreterException { - ClassLoader oldcl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(cl); - try { - return intp.getFormType(); - } finally { - cl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(oldcl); - } - } - - @Override - public int getProgress(InterpreterContext context) throws InterpreterException { - ClassLoader oldcl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(cl); - try { - return intp.getProgress(context); - } catch (Exception e) { - throw new InterpreterException(e); - } finally { - cl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(oldcl); - } - } - - @Override - public Scheduler getScheduler() { - ClassLoader oldcl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(cl); - try { - return intp.getScheduler(); - } finally { - cl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(oldcl); - } - } - - @Override - public List<InterpreterCompletion> completion(String buf, int cursor, - InterpreterContext interpreterContext) throws InterpreterException { - ClassLoader oldcl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(cl); - try { - List completion = intp.completion(buf, cursor, interpreterContext); - return completion; - } finally { - cl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(oldcl); - } - } - - - @Override - public String getClassName() { - ClassLoader oldcl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(cl); - try { - return intp.getClassName(); - } finally { - cl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(oldcl); - } - } - - @Override - public void setInterpreterGroup(InterpreterGroup interpreterGroup) { - ClassLoader oldcl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(cl); - try { - intp.setInterpreterGroup(interpreterGroup); - } finally { - cl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(oldcl); - } - } - - @Override - public InterpreterGroup getInterpreterGroup() { - ClassLoader oldcl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(cl); - try { - return intp.getInterpreterGroup(); - } finally { - cl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(oldcl); - } - } - - @Override - public void setClassloaderUrls(URL [] urls) { - ClassLoader oldcl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(cl); - try { - intp.setClassloaderUrls(urls); - } finally { - cl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(oldcl); - } - } - - @Override - public URL [] getClassloaderUrls() { - ClassLoader oldcl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(cl); - try { - return intp.getClassloaderUrls(); - } finally { - cl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(oldcl); - } - } - - @Override - public void setProperties(Properties properties) { - ClassLoader oldcl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(cl); - try { - intp.setProperties(properties); - } finally { - cl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(oldcl); - } - } - - @Override - public Properties getProperties() { - ClassLoader oldcl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(cl); - try { - return intp.getProperties(); - } finally { - cl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(oldcl); - } - } - - @Override - public String getProperty(String key) { - ClassLoader oldcl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(cl); - try { - return intp.getProperty(key); - } finally { - cl = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(oldcl); - } - } -} http://git-wip-us.apache.org/repos/asf/zeppelin/blob/23c5cac8/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java index 0596cc5..944672c 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java @@ -309,30 +309,37 @@ public class InterpreterSetting { return interpreterSettingManager; } - public void setAngularObjectRegistryListener(AngularObjectRegistryListener + public InterpreterSetting setAngularObjectRegistryListener(AngularObjectRegistryListener angularObjectRegistryListener) { this.angularObjectRegistryListener = angularObjectRegistryListener; + return this; } - public void setAppEventListener(ApplicationEventListener appEventListener) { + public InterpreterSetting setAppEventListener(ApplicationEventListener appEventListener) { this.appEventListener = appEventListener; + return this; } - public void setRemoteInterpreterProcessListener(RemoteInterpreterProcessListener + public InterpreterSetting setRemoteInterpreterProcessListener(RemoteInterpreterProcessListener remoteInterpreterProcessListener) { this.remoteInterpreterProcessListener = remoteInterpreterProcessListener; + return this; } - public void setDependencyResolver(DependencyResolver dependencyResolver) { + public InterpreterSetting setDependencyResolver(DependencyResolver dependencyResolver) { this.dependencyResolver = dependencyResolver; + return this; } - public void setInterpreterSettingManager(InterpreterSettingManager interpreterSettingManager) { + public InterpreterSetting setInterpreterSettingManager( + InterpreterSettingManager interpreterSettingManager) { this.interpreterSettingManager = interpreterSettingManager; + return this; } - public void setLifecycleManager(LifecycleManager lifecycleManager) { + public InterpreterSetting setLifecycleManager(LifecycleManager lifecycleManager) { this.lifecycleManager = lifecycleManager; + return this; } public LifecycleManager getLifecycleManager() { @@ -511,8 +518,9 @@ public class InterpreterSetting { return conf; } - public void setConf(ZeppelinConfiguration conf) { + public InterpreterSetting setConf(ZeppelinConfiguration conf) { this.conf = conf; + return this; } public List<Dependency> getDependencies() { http://git-wip-us.apache.org/repos/asf/zeppelin/blob/23c5cac8/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java index fb86954..0b7efd5 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java @@ -165,6 +165,18 @@ public class InterpreterSettingManager { init(); } + + private void initInterpreterSetting(InterpreterSetting interpreterSetting) { + interpreterSetting.setConf(conf) + .setInterpreterSettingManager(this) + .setAngularObjectRegistryListener(angularObjectRegistryListener) + .setRemoteInterpreterProcessListener(remoteInterpreterProcessListener) + .setAppEventListener(appEventListener) + .setDependencyResolver(dependencyResolver) + .setLifecycleManager(lifecycleManager) + .postProcessing(); + } + /** * Load interpreter setting from interpreter-setting.json */ @@ -172,6 +184,11 @@ public class InterpreterSettingManager { if (!Files.exists(interpreterSettingPath)) { // nothing to read LOGGER.warn("Interpreter Setting file {} doesn't exist", interpreterSettingPath); + for (InterpreterSetting interpreterSettingTemplate : interpreterSettingTemplates.values()) { + InterpreterSetting interpreterSetting = new InterpreterSetting(interpreterSettingTemplate); + initInterpreterSetting(interpreterSetting); + interpreterSettings.put(interpreterSetting.getId(), interpreterSetting); + } return; } @@ -179,17 +196,10 @@ public class InterpreterSettingManager { InterpreterInfoSaving infoSaving = InterpreterInfoSaving.loadFromFile(interpreterSettingPath); //TODO(zjffdu) still ugly (should move all to InterpreterInfoSaving) for (InterpreterSetting savedInterpreterSetting : infoSaving.interpreterSettings.values()) { - savedInterpreterSetting.setConf(conf); - savedInterpreterSetting.setInterpreterSettingManager(this); - savedInterpreterSetting.setAngularObjectRegistryListener(angularObjectRegistryListener); - savedInterpreterSetting.setRemoteInterpreterProcessListener( - remoteInterpreterProcessListener); - savedInterpreterSetting.setAppEventListener(appEventListener); - savedInterpreterSetting.setDependencyResolver(dependencyResolver); - savedInterpreterSetting.setLifecycleManager(lifecycleManager); savedInterpreterSetting.setProperties(InterpreterSetting.convertInterpreterProperties( savedInterpreterSetting.getProperties() )); + initInterpreterSetting(savedInterpreterSetting); InterpreterSetting interpreterSettingTemplate = interpreterSettingTemplates.get(savedInterpreterSetting.getGroup()); @@ -377,14 +387,7 @@ public class InterpreterSettingManager { interpreterSettingTemplate); InterpreterSetting interpreterSetting = new InterpreterSetting(interpreterSettingTemplate); - interpreterSetting.setAngularObjectRegistryListener(angularObjectRegistryListener); - interpreterSetting.setRemoteInterpreterProcessListener(remoteInterpreterProcessListener); - interpreterSetting.setAppEventListener(appEventListener); - interpreterSetting.setDependencyResolver(dependencyResolver); - interpreterSetting.setInterpreterSettingManager(this); - interpreterSetting.setLifecycleManager(lifecycleManager); - interpreterSetting.postProcessing(); - interpreterSettings.put(interpreterSetting.getId(), interpreterSetting); + initInterpreterSetting(interpreterSetting); } @VisibleForTesting @@ -640,13 +643,7 @@ public class InterpreterSettingManager { setting.appendDependencies(dependencies); setting.setInterpreterOption(option); setting.setProperties(p); - setting.setAppEventListener(appEventListener); - setting.setRemoteInterpreterProcessListener(remoteInterpreterProcessListener); - setting.setDependencyResolver(dependencyResolver); - setting.setAngularObjectRegistryListener(angularObjectRegistryListener); - setting.setLifecycleManager(lifecycleManager); - setting.setInterpreterSettingManager(this); - setting.postProcessing(); + initInterpreterSetting(setting); interpreterSettings.put(setting.getId(), setting); saveToFile(); return setting; @@ -655,12 +652,7 @@ public class InterpreterSettingManager { @VisibleForTesting public void addInterpreterSetting(InterpreterSetting interpreterSetting) { interpreterSettingTemplates.put(interpreterSetting.getName(), interpreterSetting); - interpreterSetting.setAppEventListener(appEventListener); - interpreterSetting.setDependencyResolver(dependencyResolver); - interpreterSetting.setLifecycleManager(lifecycleManager); - interpreterSetting.setAngularObjectRegistryListener(angularObjectRegistryListener); - interpreterSetting.setRemoteInterpreterProcessListener(remoteInterpreterProcessListener); - interpreterSetting.setInterpreterSettingManager(this); + initInterpreterSetting(interpreterSetting); interpreterSettings.put(interpreterSetting.getId(), interpreterSetting); } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/23c5cac8/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/MiniHadoopCluster.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/MiniHadoopCluster.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/MiniHadoopCluster.java index 619d01a..b0799ae 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/MiniHadoopCluster.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/MiniHadoopCluster.java @@ -44,6 +44,7 @@ public class MiniHadoopCluster { // start MiniYarnCluster YarnConfiguration baseConfig = new YarnConfiguration(hadoopConf); + baseConfig.set("yarn.nodemanager.disk-health-checker.max-disk-utilization-per-disk-percentage", "95"); this.yarnCluster = new MiniYARNCluster(getClass().getName(), 2, 1, 1); yarnCluster.init(baseConfig); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/23c5cac8/zeppelin-zengine/src/test/resources/conf/interpreter.json ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/test/resources/conf/interpreter.json b/zeppelin-zengine/src/test/resources/conf/interpreter.json index 45e1d60..0c5e829 100644 --- a/zeppelin-zengine/src/test/resources/conf/interpreter.json +++ b/zeppelin-zengine/src/test/resources/conf/interpreter.json @@ -66,6 +66,60 @@ "users": [], "isUserImpersonate": false } + }, + + "2C4BJDRRZ" : { + "group": "mock1", + "name": "mock1", + "className": "org.apache.zeppelin.interpreter.mock.MockInterpreter1", + "properties": { + }, + "option": { + "remote": true, + "port": -1, + "perNote": "shared", + "perUser": "shared", + "isExistingProcess": false, + "setPermission": false, + "users": [], + "isUserImpersonate": false + } + }, + + "2C3PTPMUH" : { + "group": "mock2", + "name": "mock2", + "className": "org.apache.zeppelin.interpreter.mock.MockInterpreter2", + "properties": { + }, + "option": { + "remote": true, + "port": -1, + "perNote": "shared", + "perUser": "isolated", + "isExistingProcess": false, + "setPermission": false, + "users": [], + "isUserImpersonate": false + } + }, + + "2C5DCRVGM" : { + "group": "mock_resource_pool", + "name": "mock_resource_pool", + "className": "org.apache.zeppelin.interpreter.remote.mock.MockInterpreterResourcePool", + "properties": { + }, + "option": { + "remote": true, + "port": -1, + "perNote": "shared", + "perUser": "shared", + "isExistingProcess": false, + "setPermission": false, + "users": [], + "isUserImpersonate": false + } } }, "interpreterBindings": {