Repository: zeppelin Updated Branches: refs/heads/master cceeaef2a -> 2463731ed
[HOTFIX][ZEPPELIN-2037][ZEPPELIN-1832] Restart with several options include "per user/per note" and "scoped/isolated" ### What is this PR for? This is a second part of ZEPPELIN-2047. This issue relates to #2140 ### What type of PR is it? [Hot Fix] ### Todos * [x] - Per user with Isolated * [x] - Per note with Scoped * [x] - Per note with Isolated * [x] - Restart all interpreter when user click the restart button "Interpreter tab" ### What is the Jira issue? * https://issues.apache.org/jira/browse/ZEPPELIN-2037 * https://issues.apache.org/jira/browse/ZEPPELIN-1832 ### How should this be tested? N/A ### Screenshots (if appropriate) N/A ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: Jongyoul Lee <[email protected]> Closes #2149 from jongyoul/ZEPPELIN-2037-2-per-note and squashes the following commits: 8341348 [Jongyoul Lee] Changed "restart" in interpreter tab to restart all of interpreterGroups in that interpreterSetting bcccbb9 [Jongyoul Lee] Added test cases for "per note" as "isolated" 0d53d1d [Jongyoul Lee] Fixed to run "per note" as "scoped" 9d5b4b4 [Jongyoul Lee] Fixed to run "per user" as "isolated" Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/2463731e Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/2463731e Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/2463731e Branch: refs/heads/master Commit: 2463731ede1698a8235cb03edb30c5e39d4f7402 Parents: cceeaef Author: Jongyoul Lee <[email protected]> Authored: Fri Mar 17 23:46:43 2017 +0900 Committer: Jongyoul Lee <[email protected]> Committed: Sat Mar 18 13:24:31 2017 +0900 ---------------------------------------------------------------------- .../zeppelin/rest/InterpreterRestApi.java | 6 +- .../interpreter/InterpreterSetting.java | 14 +-- .../interpreter/InterpreterSettingManager.java | 28 ++---- .../interpreter/InterpreterFactoryTest.java | 4 +- .../interpreter/InterpreterSettingTest.java | 91 ++++++++++++++++++-- .../apache/zeppelin/notebook/NotebookTest.java | 8 +- 6 files changed, 112 insertions(+), 39 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2463731e/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java index 02b9931..a324e57 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java @@ -190,7 +190,11 @@ public class InterpreterRestApi { RestartInterpreterRequest request = gson.fromJson(message, RestartInterpreterRequest.class); String noteId = request == null ? null : request.getNoteId(); - interpreterSettingManager.restart(settingId, noteId, SecurityUtils.getPrincipal()); + if (null == noteId) { + interpreterSettingManager.close(setting); + } else { + interpreterSettingManager.restart(settingId, noteId, SecurityUtils.getPrincipal()); + } notebookServer.clearParagraphRuntimeInfo(setting); } catch (InterpreterException e) { http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2463731e/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 fd016e0..317efbd 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 @@ -244,12 +244,12 @@ public class InterpreterSetting { } } - void closeAndRemoveInterpreterGroupByUser(String user) { + void closeAndRemoveInterpreterGroup(String noteId, String user) { if (user.equals("anonymous")) { user = ""; } - String processKey = getInterpreterProcessKey(user, ""); - String sessionKey = getInterpreterSessionKey(user, ""); + String processKey = getInterpreterProcessKey(user, noteId); + String sessionKey = getInterpreterSessionKey(user, noteId); List<InterpreterGroup> groupToRemove = new LinkedList<>(); InterpreterGroup groupItem; for (String intpKey : new HashSet<>(interpreterGroupRef.keySet())) { @@ -274,9 +274,11 @@ public class InterpreterSetting { } void closeAndRemoveAllInterpreterGroups() { - HashSet<String> groupsToRemove = new HashSet<>(interpreterGroupRef.keySet()); - for (String key : groupsToRemove) { - closeAndRemoveInterpreterGroupByNoteId(key); + for (String processKey : new HashSet<>(interpreterGroupRef.keySet())) { + InterpreterGroup interpreterGroup = interpreterGroupRef.get(processKey); + for (String sessionKey : new HashSet<>(interpreterGroup.keySet())) { + interpreterGroup.close(interpreterGroupRef, processKey, sessionKey); + } } } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2463731e/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 147f279..585456f 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 @@ -935,24 +935,8 @@ public class InterpreterSettingManager { InterpreterSetting intpSetting = interpreterSettings.get(settingId); Preconditions.checkNotNull(intpSetting); - // restart interpreter setting in note page - if (noteIdIsExist(noteId) && intpSetting.getOption().isProcess()) { - intpSetting.closeAndRemoveInterpreterGroupByNoteId(noteId); - return; - } else { - // restart interpreter setting in interpreter setting page - restart(settingId, user); - } - - } - - private boolean noteIdIsExist(String noteId) { - return noteId == null ? false : true; - } - - public void restart(String id, String user) { synchronized (interpreterSettings) { - InterpreterSetting intpSetting = interpreterSettings.get(id); + intpSetting = interpreterSettings.get(settingId); // Check if dependency in specified path is changed // If it did, overwrite old dependency jar with new one if (intpSetting != null) { @@ -964,17 +948,17 @@ public class InterpreterSettingManager { if (user.equals("anonymous")) { intpSetting.closeAndRemoveAllInterpreterGroups(); } else { - intpSetting.closeAndRemoveInterpreterGroupByUser(user); + intpSetting.closeAndRemoveInterpreterGroup(noteId, user); } } else { - throw new InterpreterException("Interpreter setting id " + id + " not found"); + throw new InterpreterException("Interpreter setting id " + settingId + " not found"); } } } public void restart(String id) { - restart(id, "anonymous"); + restart(id, "", "anonymous"); } private void stopJobAllInterpreter(InterpreterSetting intpSetting) { @@ -1075,6 +1059,10 @@ public class InterpreterSettingManager { } } + public void close(InterpreterSetting interpreterSetting) { + interpreterSetting.closeAndRemoveAllInterpreterGroups(); + } + public void close() { List<Thread> closeThreads = new LinkedList<>(); synchronized (interpreterSettings) { http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2463731e/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java index 711f957..3d0fe1a 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java @@ -224,7 +224,7 @@ public class InterpreterFactoryTest { LazyOpenInterpreter interpreter2 = (LazyOpenInterpreter)interpreterGroup.get("user2").get(0); interpreter2.open(); - mock1Setting.closeAndRemoveInterpreterGroupByUser("user1"); + mock1Setting.closeAndRemoveInterpreterGroup("sharedProcess", "user1"); assertFalse(interpreter1.isOpen()); assertTrue(interpreter2.isOpen()); } @@ -270,7 +270,7 @@ public class InterpreterFactoryTest { LazyOpenInterpreter interpreter2 = (LazyOpenInterpreter)interpreterGroup2.get("shared_session").get(0); interpreter2.open(); - mock1Setting.closeAndRemoveInterpreterGroupByUser("user1"); + mock1Setting.closeAndRemoveInterpreterGroup("note1", "user1"); assertFalse(interpreter1.isOpen()); assertTrue(interpreter2.isOpen()); } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2463731e/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingTest.java index 7e40a1b..0008751 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingTest.java @@ -43,7 +43,7 @@ public class InterpreterSettingTest { assertEquals(1, interpreterSetting.getInterpreterGroup("user1", "note1").size()); - interpreterSetting.closeAndRemoveInterpreterGroupByUser("user2"); + interpreterSetting.closeAndRemoveInterpreterGroup("note1", "user2"); assertEquals(0, interpreterSetting.getAllInterpreterGroups().size()); } @@ -77,14 +77,14 @@ public class InterpreterSettingTest { assertEquals(2, interpreterSetting.getInterpreterGroup("user1", "note1").size()); assertEquals(2, interpreterSetting.getInterpreterGroup("user2", "note1").size()); - interpreterSetting.closeAndRemoveInterpreterGroupByUser("user1"); + interpreterSetting.closeAndRemoveInterpreterGroup("note1", "user1"); assertEquals(1, interpreterSetting.getInterpreterGroup("user2","note1").size()); // Check if non-existed key works or not - interpreterSetting.closeAndRemoveInterpreterGroupByUser("user1"); + interpreterSetting.closeAndRemoveInterpreterGroup("note1", "user1"); assertEquals(1, interpreterSetting.getInterpreterGroup("user2","note1").size()); - interpreterSetting.closeAndRemoveInterpreterGroupByUser("user2"); + interpreterSetting.closeAndRemoveInterpreterGroup("note1", "user2"); assertEquals(0, interpreterSetting.getAllInterpreterGroups().size()); } @@ -118,11 +118,90 @@ public class InterpreterSettingTest { assertEquals(1, interpreterSetting.getInterpreterGroup("user1", "note1").size()); assertEquals(1, interpreterSetting.getInterpreterGroup("user2", "note1").size()); - interpreterSetting.closeAndRemoveInterpreterGroupByUser("user1"); + interpreterSetting.closeAndRemoveInterpreterGroup("note1", "user1"); assertEquals(1, interpreterSetting.getInterpreterGroup("user2","note1").size()); assertEquals(1, interpreterSetting.getAllInterpreterGroups().size()); - interpreterSetting.closeAndRemoveInterpreterGroupByUser("user2"); + interpreterSetting.closeAndRemoveInterpreterGroup("note1", "user2"); + assertEquals(0, interpreterSetting.getAllInterpreterGroups().size()); + } + + @Test + public void perNoteScopedModeCloseAndRemoveInterpreterGroupTest() { + InterpreterOption interpreterOption = new InterpreterOption(); + interpreterOption.setPerNote(InterpreterOption.SCOPED); + InterpreterSetting interpreterSetting = new InterpreterSetting("", "", "", new ArrayList<InterpreterInfo>(), new Properties(), new ArrayList<Dependency>(), interpreterOption, "", null); + + interpreterSetting.setInterpreterGroupFactory(new InterpreterGroupFactory() { + @Override + public InterpreterGroup createInterpreterGroup(String interpreterGroupId, + InterpreterOption option) { + return new InterpreterGroup(interpreterGroupId); + } + }); + + Interpreter mockInterpreter1 = mock(RemoteInterpreter.class); + List<Interpreter> interpreterList1 = new ArrayList<>(); + interpreterList1.add(mockInterpreter1); + InterpreterGroup interpreterGroup = interpreterSetting.getInterpreterGroup("user1", "note1"); + interpreterGroup.put(interpreterSetting.getInterpreterSessionKey("user1", "note1"), interpreterList1); + + Interpreter mockInterpreter2 = mock(RemoteInterpreter.class); + List<Interpreter> interpreterList2 = new ArrayList<>(); + interpreterList2.add(mockInterpreter2); + interpreterGroup = interpreterSetting.getInterpreterGroup("user1", "note2"); + interpreterGroup.put(interpreterSetting.getInterpreterSessionKey("user1", "note2"), interpreterList2); + + assertEquals(1, interpreterSetting.getAllInterpreterGroups().size()); + assertEquals(2, interpreterSetting.getInterpreterGroup("user1", "note1").size()); + assertEquals(2, interpreterSetting.getInterpreterGroup("user1", "note2").size()); + + interpreterSetting.closeAndRemoveInterpreterGroup("note1", "user1"); + assertEquals(1, interpreterSetting.getInterpreterGroup("user1","note2").size()); + + // Check if non-existed key works or not + interpreterSetting.closeAndRemoveInterpreterGroup("note1", "user1"); + assertEquals(1, interpreterSetting.getInterpreterGroup("user1","note2").size()); + + interpreterSetting.closeAndRemoveInterpreterGroup("note2", "user1"); + assertEquals(0, interpreterSetting.getAllInterpreterGroups().size()); + } + + @Test + public void perNoteIsolatedModeCloseAndRemoveInterpreterGroupTest() { + InterpreterOption interpreterOption = new InterpreterOption(); + interpreterOption.setPerNote(InterpreterOption.ISOLATED); + InterpreterSetting interpreterSetting = new InterpreterSetting("", "", "", new ArrayList<InterpreterInfo>(), new Properties(), new ArrayList<Dependency>(), interpreterOption, "", null); + + interpreterSetting.setInterpreterGroupFactory(new InterpreterGroupFactory() { + @Override + public InterpreterGroup createInterpreterGroup(String interpreterGroupId, + InterpreterOption option) { + return new InterpreterGroup(interpreterGroupId); + } + }); + + Interpreter mockInterpreter1 = mock(RemoteInterpreter.class); + List<Interpreter> interpreterList1 = new ArrayList<>(); + interpreterList1.add(mockInterpreter1); + InterpreterGroup interpreterGroup = interpreterSetting.getInterpreterGroup("user1", "note1"); + interpreterGroup.put(interpreterSetting.getInterpreterSessionKey("user1", "note1"), interpreterList1); + + Interpreter mockInterpreter2 = mock(RemoteInterpreter.class); + List<Interpreter> interpreterList2 = new ArrayList<>(); + interpreterList2.add(mockInterpreter2); + interpreterGroup = interpreterSetting.getInterpreterGroup("user1", "note2"); + interpreterGroup.put(interpreterSetting.getInterpreterSessionKey("user1", "note2"), interpreterList2); + + assertEquals(2, interpreterSetting.getAllInterpreterGroups().size()); + assertEquals(1, interpreterSetting.getInterpreterGroup("user1", "note1").size()); + assertEquals(1, interpreterSetting.getInterpreterGroup("user1", "note2").size()); + + interpreterSetting.closeAndRemoveInterpreterGroup("note1", "user1"); + assertEquals(1, interpreterSetting.getInterpreterGroup("user1","note2").size()); + assertEquals(1, interpreterSetting.getAllInterpreterGroups().size()); + + interpreterSetting.closeAndRemoveInterpreterGroup("note2", "user1"); assertEquals(0, interpreterSetting.getAllInterpreterGroups().size()); } } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2463731e/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java index ae4501d..9b1a370 100644 --- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java +++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java @@ -925,8 +925,8 @@ public class NotebookTest implements JobListenerFactory{ // restart interpreter with scoped mode enabled for (InterpreterSetting setting : notebook.getInterpreterSettingManager().getInterpreterSettings(note1.getId())) { setting.getOption().setPerNote(InterpreterOption.SCOPED); - notebook.getInterpreterSettingManager().restart(setting.getId(), note1.getId()); - notebook.getInterpreterSettingManager().restart(setting.getId(), note2.getId()); + notebook.getInterpreterSettingManager().restart(setting.getId(), note1.getId(), anonymous.getUser()); + notebook.getInterpreterSettingManager().restart(setting.getId(), note2.getId(), anonymous.getUser()); } // run per note session enabled @@ -941,8 +941,8 @@ public class NotebookTest implements JobListenerFactory{ // restart interpreter with isolated mode enabled for (InterpreterSetting setting : notebook.getInterpreterSettingManager().getInterpreterSettings(note1.getId())) { setting.getOption().setPerNote(InterpreterOption.ISOLATED); - notebook.getInterpreterSettingManager().restart(setting.getId(), note1.getId()); - notebook.getInterpreterSettingManager().restart(setting.getId(), note2.getId()); + notebook.getInterpreterSettingManager().restart(setting.getId(), note1.getId(), anonymous.getUser()); + notebook.getInterpreterSettingManager().restart(setting.getId(), note2.getId(), anonymous.getUser()); } // run per note process enabled
