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 <zjf...@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 <zjf...@apache.org> Authored: Mon Oct 16 20:50:07 2017 +0800 Committer: Jeff Zhang <zjf...@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()); } }
