Repository: zeppelin Updated Branches: refs/heads/branch-0.7 70ca8bdcb -> 8bbdc671c
[HOTFIX][ZEPPELIN-2037][ZEPPELIN-1832] Restart with several options include "per user/per note" and "scoped/isolated" This is a second part of ZEPPELIN-2047. This issue relates to #2140 [Hot Fix] * [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" * https://issues.apache.org/jira/browse/ZEPPELIN-2037 * https://issues.apache.org/jira/browse/ZEPPELIN-1832 N/A N/A * 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/8bbdc671 Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/8bbdc671 Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/8bbdc671 Branch: refs/heads/branch-0.7 Commit: 8bbdc671c3ac48ee47b212446a632672fd764358 Parents: 70ca8bd Author: Jongyoul Lee <[email protected]> Authored: Fri Mar 17 23:46:43 2017 +0900 Committer: Jongyoul Lee <[email protected]> Committed: Sat Mar 18 13:35:15 2017 +0900 ---------------------------------------------------------------------- .../zeppelin/rest/InterpreterRestApi.java | 9 +- .../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, 114 insertions(+), 40 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/8bbdc671/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 06da4fc..f45a454 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 @@ -179,18 +179,23 @@ public class InterpreterRestApi { @ZeppelinApi public Response restartSetting(String message, @PathParam("settingId") String settingId) { logger.info("Restart interpreterSetting {}, msg={}", settingId, message); + + InterpreterSetting setting = interpreterSettingManager.get(settingId); try { 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()); + } } catch (InterpreterException e) { logger.error("Exception in InterpreterRestApi while restartSetting ", e); return new JsonResponse<>(Status.NOT_FOUND, e.getMessage(), ExceptionUtils.getStackTrace(e)) .build(); } - InterpreterSetting setting = interpreterSettingManager.get(settingId); if (setting == null) { return new JsonResponse<>(Status.NOT_FOUND, "", settingId).build(); } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/8bbdc671/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 30a4967..17a4479 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 @@ -239,12 +239,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())) { @@ -269,9 +269,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/8bbdc671/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/8bbdc671/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 70c7a6b..d647337 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/8bbdc671/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/8bbdc671/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 5b394ee..1f292d8 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 @@ -951,8 +951,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 @@ -967,8 +967,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
