Repository: zeppelin Updated Branches: refs/heads/master 4a9377db9 -> 632815c35
[HOTFIX] Handle removing interpreters while removing note ### What is this PR for? Removing some interpreters while removing note. In case of "Per note", "Isolated" interpreter process should be removed if a note be removed. In case of "Scoped" of "Per note", that session should be removed and if there's no more session in that process, the process should be removed, too. ### What type of PR is it? [Hot Fix] ### Todos * [x] - Handle removing interpreters when removing note ### What is the Jira issue? N/A ### How should this be tested? 1. set python interpreter as "per note/scoped" 1. run ```%python print(1)``` in a note 1. remove that note 1. check if python process exists with grep ### 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: Jongyoul Lee <jongy...@gmail.com> Closes #2159 from jongyoul/hotfix/remove-note-remove-interpreter-instances and squashes the following commits: cb3de97 [Jongyoul Lee] Added method to handle removing interpreters while removing note Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/632815c3 Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/632815c3 Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/632815c3 Branch: refs/heads/master Commit: 632815c3582a5e91e9fdc8ee3c0473a0d87c268e Parents: 4a9377d Author: Jongyoul Lee <jongy...@gmail.com> Authored: Mon Mar 20 00:33:50 2017 +0900 Committer: Jongyoul Lee <jongy...@apache.org> Committed: Mon Mar 20 11:45:19 2017 +0900 ---------------------------------------------------------------------- .../apache/zeppelin/socket/NotebookServer.java | 1 + .../interpreter/InterpreterSettingManager.java | 15 +-- .../org/apache/zeppelin/notebook/Notebook.java | 7 ++ .../interpreter/InterpreterSettingTest.java | 120 +++++++++++++++++++ 4 files changed, 129 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/632815c3/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java index 1a6f0be..23d80a4 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java @@ -1082,6 +1082,7 @@ public class NotebookServer extends WebSocketServlet if (note != null && !note.isTrash()){ fromMessage.put("name", Folder.TRASH_FOLDER_ID + "/" + note.getName()); renameNote(conn, userAndRoles, notebook, fromMessage, "move"); + notebook.moveNoteToTrash(note.getId()); } } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/632815c3/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 585456f..1832564 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 @@ -799,20 +799,7 @@ public class InterpreterSettingManager { public void removeInterpretersForNote(InterpreterSetting interpreterSetting, String user, String noteId) { - InterpreterOption option = interpreterSetting.getOption(); - if (option.isProcess()) { - interpreterSetting.closeAndRemoveInterpreterGroupByNoteId(noteId); - } else if (option.isSession()) { - InterpreterGroup interpreterGroup = interpreterSetting.getInterpreterGroup(user, noteId); - String key = getInterpreterSessionKey(user, noteId, interpreterSetting); - interpreterGroup.close(key); - synchronized (interpreterGroup) { - interpreterGroup.remove(key); - interpreterGroup.notifyAll(); // notify createInterpreterForNote() - } - logger.info("Interpreter instance {} for note {} is removed", interpreterSetting.getName(), - noteId); - } + interpreterSetting.closeAndRemoveInterpreterGroup(noteId, ""); } public String getInterpreterSessionKey(String user, String noteId, InterpreterSetting setting) { http://git-wip-us.apache.org/repos/asf/zeppelin/blob/632815c3/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java index dfef6f4..e7f7e49 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java @@ -322,6 +322,13 @@ public class Notebook implements NoteEventListener { } } + public void moveNoteToTrash(String noteId) { + for (InterpreterSetting interpreterSetting : interpreterSettingManager + .getInterpreterSettings(noteId)) { + interpreterSettingManager.removeInterpretersForNote(interpreterSetting, "", noteId); + } + } + public void removeNote(String id, AuthenticationInfo subject) { Preconditions.checkNotNull(subject, "AuthenticationInfo should not be null"); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/632815c3/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 0008751..1aab757 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 @@ -204,4 +204,124 @@ public class InterpreterSettingTest { interpreterSetting.closeAndRemoveInterpreterGroup("note2", "user1"); assertEquals(0, interpreterSetting.getAllInterpreterGroups().size()); } + + @Test + public void perNoteScopedModeRemoveInterpreterGroupWhenNoteIsRemoved() { + 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); + + assertEquals(1, interpreterSetting.getAllInterpreterGroups().size()); + assertEquals(1, interpreterSetting.getInterpreterGroup("user1", "note1").size()); + + // This method will be called when remove note + interpreterSetting.closeAndRemoveInterpreterGroup("note1",""); + assertEquals(0, interpreterSetting.getAllInterpreterGroups().size()); + // Be careful that getInterpreterGroup makes interpreterGroup if it doesn't exist + assertEquals(0, interpreterSetting.getInterpreterGroup("user1","note1").size()); + } + + @Test + public void perNoteIsolatedModeRemoveInterpreterGroupWhenNoteIsRemoved() { + 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); + + assertEquals(1, interpreterSetting.getAllInterpreterGroups().size()); + assertEquals(1, interpreterSetting.getInterpreterGroup("user1", "note1").size()); + + // This method will be called when remove note + interpreterSetting.closeAndRemoveInterpreterGroup("note1",""); + assertEquals(0, interpreterSetting.getAllInterpreterGroups().size()); + // Be careful that getInterpreterGroup makes interpreterGroup if it doesn't exist + assertEquals(0, interpreterSetting.getInterpreterGroup("user1","note1").size()); + } + + @Test + public void perUserScopedModeNeverRemoveInterpreterGroupWhenNoteIsRemoved() { + InterpreterOption interpreterOption = new InterpreterOption(); + interpreterOption.setPerUser(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); + + assertEquals(1, interpreterSetting.getAllInterpreterGroups().size()); + assertEquals(1, interpreterSetting.getInterpreterGroup("user1", "note1").size()); + + // This method will be called when remove note + interpreterSetting.closeAndRemoveInterpreterGroup("note1",""); + assertEquals(1, interpreterSetting.getAllInterpreterGroups().size()); + // Be careful that getInterpreterGroup makes interpreterGroup if it doesn't exist + assertEquals(1, interpreterSetting.getInterpreterGroup("user1","note1").size()); + } + + @Test + public void perUserIsolatedModeNeverRemoveInterpreterGroupWhenNoteIsRemoved() { + InterpreterOption interpreterOption = new InterpreterOption(); + interpreterOption.setPerUser(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); + + assertEquals(1, interpreterSetting.getAllInterpreterGroups().size()); + assertEquals(1, interpreterSetting.getInterpreterGroup("user1", "note1").size()); + + // This method will be called when remove note + interpreterSetting.closeAndRemoveInterpreterGroup("note1",""); + assertEquals(1, interpreterSetting.getAllInterpreterGroups().size()); + // Be careful that getInterpreterGroup makes interpreterGroup if it doesn't exist + assertEquals(1, interpreterSetting.getInterpreterGroup("user1","note1").size()); + } }
