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 2016/12/15 02:52:46 UTC
zeppelin git commit: ZEPPELIN-1773. Remove method destroy() in
Interpreter
Repository: zeppelin
Updated Branches:
refs/heads/master 7ef0554af -> c6c67d7b1
ZEPPELIN-1773. Remove method destroy() in Interpreter
### What is this PR for?
IMHO `destroy` is a little functional duplicated with `close`. And it seems currently only cassandra use destroy and it just calls `close()` https://github.com/apache/zeppelin/blob/master/cassandra/src/main/java/org/apache/zeppelin/cassandra/CassandraInterpreter.java#L231. So IMO, we could delete destroy method, also we can delete the destroy method in `InterpreterGroup`. Besides, I move the terminate remoteInterpreterProcess in `close`.
### What type of PR is it?
[Improvement]
### Todos
* [ ] - Task
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-1773
### How should this be tested?
Just remove something so the existing test pass should be sufficient.
### 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 <zj...@apache.org>
Closes #1740 from zjffdu/ZEPPELIN-1773 and squashes the following commits:
4fd2e38 [Jeff Zhang] ZEPPELIN-1773. Remove method destroy() in Interpreter
Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/c6c67d7b
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/c6c67d7b
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/c6c67d7b
Branch: refs/heads/master
Commit: c6c67d7b112eceb36846e03207cb9c37519f1730
Parents: 7ef0554
Author: Jeff Zhang <zj...@apache.org>
Authored: Fri Dec 9 10:12:51 2016 +0800
Committer: Lee moon soo <mo...@apache.org>
Committed: Wed Dec 14 18:52:39 2016 -0800
----------------------------------------------------------------------
.../cassandra/CassandraInterpreter.java | 6 --
.../zeppelin/flink/FlinkInterpreterTest.java | 1 -
.../zeppelin/interpreter/Interpreter.java | 7 --
.../zeppelin/interpreter/InterpreterGroup.java | 89 ++++----------------
.../remote/RemoteInterpreterServer.java | 1 -
.../remote/RemoteAngularObjectTest.java | 1 -
.../RemoteInterpreterOutputTestStream.java | 1 -
.../remote/RemoteInterpreterTest.java | 1 -
.../resource/DistributedResourcePoolTest.java | 2 -
.../interpreter/InterpreterFactory.java | 1 -
.../interpreter/InterpreterSetting.java | 1 -
11 files changed, 17 insertions(+), 94 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c6c67d7b/cassandra/src/main/java/org/apache/zeppelin/cassandra/CassandraInterpreter.java
----------------------------------------------------------------------
diff --git a/cassandra/src/main/java/org/apache/zeppelin/cassandra/CassandraInterpreter.java b/cassandra/src/main/java/org/apache/zeppelin/cassandra/CassandraInterpreter.java
index 47d8e3a..a4984ad 100644
--- a/cassandra/src/main/java/org/apache/zeppelin/cassandra/CassandraInterpreter.java
+++ b/cassandra/src/main/java/org/apache/zeppelin/cassandra/CassandraInterpreter.java
@@ -226,10 +226,4 @@ public class CassandraInterpreter extends Interpreter {
.createOrGetParallelScheduler(CassandraInterpreter.class.getName() + this.hashCode(),
parseInt(getProperty(CASSANDRA_INTERPRETER_PARALLELISM)));
}
-
- @Override
- public void destroy() {
- super.destroy();
- this.close();
- }
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c6c67d7b/flink/src/test/java/org/apache/zeppelin/flink/FlinkInterpreterTest.java
----------------------------------------------------------------------
diff --git a/flink/src/test/java/org/apache/zeppelin/flink/FlinkInterpreterTest.java b/flink/src/test/java/org/apache/zeppelin/flink/FlinkInterpreterTest.java
index 72b59eb..d443508 100644
--- a/flink/src/test/java/org/apache/zeppelin/flink/FlinkInterpreterTest.java
+++ b/flink/src/test/java/org/apache/zeppelin/flink/FlinkInterpreterTest.java
@@ -46,7 +46,6 @@ public class FlinkInterpreterTest {
@AfterClass
public static void tearDown() {
flink.close();
- flink.destroy();
}
@Test
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c6c67d7b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java
index cc57cb0..4045fc9 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java
@@ -131,13 +131,6 @@ public abstract class Interpreter {
return SchedulerFactory.singleton().createOrGetFIFOScheduler("interpreter_" + this.hashCode());
}
- /**
- * Called when interpreter is no longer used.
- */
- @ZeppelinApi
- public void destroy() {
- }
-
public static Logger logger = LoggerFactory.getLogger(Interpreter.class);
private InterpreterGroup interpreterGroup;
private URL [] classloaderUrls;
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c6c67d7b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterGroup.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterGroup.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterGroup.java
index 11dd2b3..b9b27a8 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterGroup.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterGroup.java
@@ -138,8 +138,6 @@ public class InterpreterGroup extends ConcurrentHashMap<String, List<Interpreter
this.remoteInterpreterProcess = remoteInterpreterProcess;
}
-
-
/**
* Close all interpreter instances in this group
*/
@@ -150,6 +148,15 @@ public class InterpreterGroup extends ConcurrentHashMap<String, List<Interpreter
intpToClose.addAll(intpGroupForSession);
}
close(intpToClose);
+
+ // make sure remote interpreter process terminates
+ if (remoteInterpreterProcess != null) {
+ while (remoteInterpreterProcess.referenceCount() > 0) {
+ remoteInterpreterProcess.dereference();
+ }
+ remoteInterpreterProcess = null;
+ }
+ allInterpreterGroups.remove(id);
}
/**
@@ -160,6 +167,14 @@ public class InterpreterGroup extends ConcurrentHashMap<String, List<Interpreter
LOGGER.info("Close interpreter group " + getId() + " for session: " + sessionId);
List<Interpreter> intpForSession = this.get(sessionId);
close(intpForSession);
+
+ if (remoteInterpreterProcess != null) {
+ remoteInterpreterProcess.dereference();
+ if (remoteInterpreterProcess.referenceCount() <= 0) {
+ remoteInterpreterProcess = null;
+ allInterpreterGroups.remove(id);
+ }
+ }
}
private void close(Collection<Interpreter> intpToClose) {
@@ -193,76 +208,6 @@ public class InterpreterGroup extends ConcurrentHashMap<String, List<Interpreter
}
}
- /**
- * Destroy all interpreter instances in this group for the session
- * @param sessionId
- */
- public void destroy(String sessionId) {
- LOGGER.info("Destroy interpreter group " + getId() + " for session " + sessionId);
- List<Interpreter> intpForSession = this.get(sessionId);
- destroy(intpForSession);
-
- if (remoteInterpreterProcess != null) {
- remoteInterpreterProcess.dereference();
- if (remoteInterpreterProcess.referenceCount() <= 0) {
- remoteInterpreterProcess = null;
- allInterpreterGroups.remove(id);
- }
- }
- }
-
-
- /**
- * Destroy all interpreter instances in this group
- */
- public void destroy() {
- LOGGER.info("Destroy interpreter group " + getId());
- List<Interpreter> intpToDestroy = new LinkedList<>();
- for (List<Interpreter> intpGroupForSession : this.values()) {
- intpToDestroy.addAll(intpGroupForSession);
- }
- destroy(intpToDestroy);
-
- // make sure remote interpreter process terminates
- if (remoteInterpreterProcess != null) {
- while (remoteInterpreterProcess.referenceCount() > 0) {
- remoteInterpreterProcess.dereference();
- }
- remoteInterpreterProcess = null;
- }
-
- allInterpreterGroups.remove(id);
- }
-
- private void destroy(Collection<Interpreter> intpToDestroy) {
- if (intpToDestroy == null) {
- return;
- }
-
- List<Thread> destroyThreads = new LinkedList<>();
-
- for (final Interpreter intp : intpToDestroy) {
- Thread t = new Thread() {
- public void run() {
- intp.destroy();
- }
- };
-
- t.start();
- destroyThreads.add(t);
- }
-
- for (Thread t : destroyThreads) {
- try {
- t.join();
- } catch (InterruptedException e) {
- LOGGER.error("Can't close interpreter", e);
- }
- }
- }
-
-
-
public void setResourcePool(ResourcePool resourcePool) {
this.resourcePool = resourcePool;
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c6c67d7b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
index de91f71..b167256 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java
@@ -106,7 +106,6 @@ public class RemoteInterpreterServer
eventClient.waitForEventQueueBecomesEmpty();
if (interpreterGroup != null) {
interpreterGroup.close();
- interpreterGroup.destroy();
}
server.stop();
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c6c67d7b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteAngularObjectTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteAngularObjectTest.java b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteAngularObjectTest.java
index d4acbfd..f7404e3 100644
--- a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteAngularObjectTest.java
+++ b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteAngularObjectTest.java
@@ -103,7 +103,6 @@ public class RemoteAngularObjectTest implements AngularObjectRegistryListener {
public void tearDown() throws Exception {
intp.close();
intpGroup.close();
- intpGroup.destroy();
}
@Test
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c6c67d7b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterOutputTestStream.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterOutputTestStream.java b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterOutputTestStream.java
index 0ac11c2..e3dc6b4 100644
--- a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterOutputTestStream.java
+++ b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterOutputTestStream.java
@@ -58,7 +58,6 @@ public class RemoteInterpreterOutputTestStream implements RemoteInterpreterProce
@After
public void tearDown() throws Exception {
intpGroup.close();
- intpGroup.destroy();
}
private RemoteInterpreter createMockInterpreter() {
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c6c67d7b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterTest.java b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterTest.java
index d9eae3e..2a834b1 100644
--- a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterTest.java
+++ b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterTest.java
@@ -73,7 +73,6 @@ public class RemoteInterpreterTest {
@After
public void tearDown() throws Exception {
intpGroup.close();
- intpGroup.destroy();
}
private RemoteInterpreter createMockInterpreterA(Properties p) {
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c6c67d7b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/resource/DistributedResourcePoolTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/resource/DistributedResourcePoolTest.java b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/resource/DistributedResourcePoolTest.java
index 76f46ec..8095043 100644
--- a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/resource/DistributedResourcePoolTest.java
+++ b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/resource/DistributedResourcePoolTest.java
@@ -134,11 +134,9 @@ public class DistributedResourcePoolTest {
eventPoller1.shutdown();
intp1.close();
intpGroup1.close();
- intpGroup1.destroy();
eventPoller2.shutdown();
intp2.close();
intpGroup2.close();
- intpGroup2.destroy();
}
@Test
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c6c67d7b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java
index 3465e75..c620b35 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterFactory.java
@@ -745,7 +745,6 @@ public class InterpreterFactory implements InterpreterGroupFactory {
InterpreterGroup interpreterGroup = interpreterSetting.getInterpreterGroup(user, noteId);
String key = getInterpreterSessionKey(user, noteId, interpreterSetting);
interpreterGroup.close(key);
- interpreterGroup.destroy(key);
synchronized (interpreterGroup) {
interpreterGroup.remove(key);
interpreterGroup.notifyAll(); // notify createInterpreterForNote()
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c6c67d7b/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 3011f69..1f3ddc5 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
@@ -178,7 +178,6 @@ public class InterpreterSetting {
if (groupToRemove != null) {
groupToRemove.close();
- groupToRemove.destroy();
}
}