You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by zj...@apache.org on 2017/10/18 00:31:09 UTC
zeppelin git commit: ZEPPELIN-2998. Fix bug in restarting interpreter
in scoped mode
Repository: zeppelin
Updated Branches:
refs/heads/master 6b0f6e7a0 -> 24f81426c
ZEPPELIN-2998. Fix bug in restarting interpreter in scoped mode
### What is this PR for?
Fixed the bug mentioned in https://github.com/apache/zeppelin/pull/2554#discussion_r136703878
### What type of PR is it?
[Bug Fix]
### Todos
* [ ] - Task
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-2998
### How should this be tested?
* Unit test is added
### 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 #2626 from zjffdu/ZEPPELIN-2998 and squashes the following commits:
cc11fb6 [Jeff Zhang] ZEPPELIN-2998. Fix bug in restarting interpreter in scoped mode
Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/24f81426
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/24f81426
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/24f81426
Branch: refs/heads/master
Commit: 24f81426cb138d8191f59df8d69dd28867142cd8
Parents: 6b0f6e7
Author: Jeff Zhang <zj...@apache.org>
Authored: Mon Oct 16 20:50:07 2017 +0800
Committer: Jeff Zhang <zj...@apache.org>
Committed: Wed Oct 18 08:31:02 2017 +0800
----------------------------------------------------------------------
.../interpreter/InterpreterSetting.java | 13 ++--
.../interpreter/InterpreterSettingManager.java | 8 +-
.../interpreter/ManagedInterpreterGroup.java | 4 +-
.../InterpreterSettingManagerTest.java | 77 +++++++++++++++++++-
4 files changed, 83 insertions(+), 19 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/24f81426/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 a82d5bf..3b42752 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
@@ -205,10 +205,10 @@ public class InterpreterSetting {
return this;
}
-// public Builder setInterpreterRunner(InterpreterRunner runner) {
-// interpreterSetting.interpreterRunner = runner;
-// return this;
-// }
+ public Builder setInterpreterRunner(InterpreterRunner runner) {
+ interpreterSetting.interpreterRunner = runner;
+ return this;
+ }
public Builder setIntepreterSettingManager(
InterpreterSettingManager interpreterSettingManager) {
@@ -248,7 +248,6 @@ public class InterpreterSetting {
}
void postProcessing() {
-// createLauncher();
this.status = Status.READY;
}
@@ -370,7 +369,7 @@ public class InterpreterSetting {
try {
interpreterGroupWriteLock.lock();
if (!interpreterGroups.containsKey(groupId)) {
- LOGGER.info("Create InterpreterGroup with groupId {} for user {} and note {}",
+ LOGGER.info("Create InterpreterGroup with groupId: {} for user: {} and note: {}",
groupId, user, noteId);
ManagedInterpreterGroup intpGroup = createInterpreterGroup(groupId);
interpreterGroups.put(groupId, intpGroup);
@@ -653,7 +652,7 @@ public class InterpreterSetting {
return process;
}
- private List<Interpreter> getOrCreateSession(String user, String noteId) {
+ List<Interpreter> getOrCreateSession(String user, String noteId) {
ManagedInterpreterGroup interpreterGroup = getOrCreateInterpreterGroup(user, noteId);
Preconditions.checkNotNull(interpreterGroup, "No InterpreterGroup existed for user {}, " +
"noteId {}", user, noteId);
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/24f81426/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 f34195d..abaf634 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
@@ -782,13 +782,7 @@ public class InterpreterSettingManager {
//clean up metaInfos
intpSetting.setInfos(null);
copyDependenciesFromLocalPath(intpSetting);
-
- if (user.equals("anonymous")) {
- intpSetting.close();
- } else {
- intpSetting.closeInterpreters(user, noteId);
- }
-
+ intpSetting.closeInterpreters(user, noteId);
} else {
throw new InterpreterException("Interpreter setting id " + settingId + " not found");
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/24f81426/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ManagedInterpreterGroup.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ManagedInterpreterGroup.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ManagedInterpreterGroup.java
index ff9cb1c..96f0195 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ManagedInterpreterGroup.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ManagedInterpreterGroup.java
@@ -85,7 +85,7 @@ public class ManagedInterpreterGroup extends InterpreterGroup {
close(sessions.remove(sessionId));
//TODO(zjffdu) whether close InterpreterGroup if there's no session left in Zeppelin Server
if (sessions.isEmpty() && interpreterSetting != null) {
- LOGGER.info("Remove this InterpreterGroup {} as all the sessions are closed", id);
+ LOGGER.info("Remove this InterpreterGroup: {} as all the sessions are closed", id);
interpreterSetting.removeInterpreterGroup(id);
if (remoteInterpreterProcess != null) {
LOGGER.info("Kill RemoteIntetrpreterProcess");
@@ -133,7 +133,7 @@ public class ManagedInterpreterGroup extends InterpreterGroup {
for (Interpreter interpreter : interpreters) {
interpreter.setInterpreterGroup(this);
}
- LOGGER.info("Create Session {} in InterpreterGroup {} for user {}", sessionId, id, user);
+ LOGGER.info("Create Session: {} in InterpreterGroup: {} for user: {}", sessionId, id, user);
sessions.put(sessionId, interpreters);
return interpreters;
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/24f81426/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingManagerTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingManagerTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingManagerTest.java
index 605476f..19f16f5 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingManagerTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingManagerTest.java
@@ -34,6 +34,7 @@ import java.util.List;
import java.util.Map;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
@@ -181,9 +182,6 @@ public class InterpreterSettingManagerTest extends AbstractInterpreterTest {
// only close user1's session
interpreterSettingManager.restart(interpreterSetting.getId(), "note1", "user1");
assertEquals(2, interpreterGroup.getSessionNum());
- // close all the sessions
- interpreterSettingManager.restart(interpreterSetting.getId(), "note1", "anonymous");
- assertEquals(0, interpreterGroup.getSessionNum());
// remove interpreter setting
interpreterSettingManager.remove(interpreterSetting.getId());
@@ -281,6 +279,79 @@ public class InterpreterSettingManagerTest extends AbstractInterpreterTest {
Interpreter mock1Interpreter = interpreterFactory.getInterpreter("user1", "note1", "mock1");
editor = interpreterSettingManager.getEditorSetting(mock1Interpreter,"user1", "note1", "mock1");
assertEquals("text", editor.get("language"));
+ }
+
+ @Test
+ public void testRestartShared() throws InterpreterException {
+ InterpreterSetting interpreterSetting = interpreterSettingManager.getByName("test");
+ interpreterSetting.getOption().setPerUser("shared");
+ interpreterSetting.getOption().setPerNote("shared");
+
+ interpreterSetting.getOrCreateSession("user1", "note1");
+ interpreterSetting.getOrCreateInterpreterGroup("user2", "note2");
+ assertEquals(1, interpreterSetting.getAllInterpreterGroups().size());
+
+ interpreterSettingManager.restart(interpreterSetting.getId(), "user1", "note1");
+ assertEquals(0, interpreterSetting.getAllInterpreterGroups().size());
+ }
+
+ @Test
+ public void testRestartPerUserIsolated() throws InterpreterException {
+ InterpreterSetting interpreterSetting = interpreterSettingManager.getByName("test");
+ interpreterSetting.getOption().setPerUser("isolated");
+ interpreterSetting.getOption().setPerNote("shared");
+
+ interpreterSetting.getOrCreateSession("user1", "note1");
+ interpreterSetting.getOrCreateSession("user2", "note2");
+ assertEquals(2, interpreterSetting.getAllInterpreterGroups().size());
+
+ interpreterSettingManager.restart(interpreterSetting.getId(), "note1", "user1");
+ assertEquals(1, interpreterSetting.getAllInterpreterGroups().size());
+ }
+
+ @Test
+ public void testRestartPerNoteIsolated() throws InterpreterException {
+ InterpreterSetting interpreterSetting = interpreterSettingManager.getByName("test");
+ interpreterSetting.getOption().setPerUser("shared");
+ interpreterSetting.getOption().setPerNote("isolated");
+ interpreterSetting.getOrCreateSession("user1", "note1");
+ interpreterSetting.getOrCreateSession("user2", "note2");
+ assertEquals(2, interpreterSetting.getAllInterpreterGroups().size());
+
+ interpreterSettingManager.restart(interpreterSetting.getId(), "note1", "user1");
+ assertEquals(1, interpreterSetting.getAllInterpreterGroups().size());
+ }
+
+ @Test
+ public void testRestartPerUserScoped() throws InterpreterException {
+ InterpreterSetting interpreterSetting = interpreterSettingManager.getByName("test");
+ interpreterSetting.getOption().setPerUser("scoped");
+ interpreterSetting.getOption().setPerNote("shared");
+
+ interpreterSetting.getOrCreateSession("user1", "note1");
+ interpreterSetting.getOrCreateSession("user2", "note2");
+ assertEquals(1, interpreterSetting.getAllInterpreterGroups().size());
+ assertEquals(2, interpreterSetting.getAllInterpreterGroups().get(0).getSessionNum());
+
+ interpreterSettingManager.restart(interpreterSetting.getId(), "note1", "user1");
+ assertEquals(1, interpreterSetting.getAllInterpreterGroups().size());
+ assertEquals(1, interpreterSetting.getAllInterpreterGroups().get(0).getSessionNum());
+ }
+
+ @Test
+ public void testRestartPerNoteScoped() throws InterpreterException {
+ InterpreterSetting interpreterSetting = interpreterSettingManager.getByName("test");
+ interpreterSetting.getOption().setPerUser("shared");
+ interpreterSetting.getOption().setPerNote("scoped");
+
+ interpreterSetting.getOrCreateSession("user1", "note1");
+ interpreterSetting.getOrCreateSession("user2", "note2");
+ assertEquals(1, interpreterSetting.getAllInterpreterGroups().size());
+ assertEquals(2, interpreterSetting.getAllInterpreterGroups().get(0).getSessionNum());
+
+ interpreterSettingManager.restart(interpreterSetting.getId(), "note1", "user1");
+ assertEquals(1, interpreterSetting.getAllInterpreterGroups().size());
+ assertEquals(1, interpreterSetting.getAllInterpreterGroups().get(0).getSessionNum());
}
}