Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 418A9200C3C for ; Mon, 20 Mar 2017 03:45:40 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 3CFA6160B8E; Mon, 20 Mar 2017 02:45:40 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 3534D160B7D for ; Mon, 20 Mar 2017 03:45:39 +0100 (CET) Received: (qmail 28073 invoked by uid 500); 20 Mar 2017 02:45:38 -0000 Mailing-List: contact commits-help@zeppelin.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@zeppelin.apache.org Delivered-To: mailing list commits@zeppelin.apache.org Received: (qmail 28058 invoked by uid 99); 20 Mar 2017 02:45:37 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 20 Mar 2017 02:45:37 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id AF8E2DFE8F; Mon, 20 Mar 2017 02:45:37 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: jongyoul@apache.org To: commits@zeppelin.apache.org Message-Id: <811081500b784c1db100fd7e436500e0@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: zeppelin git commit: [HOTFIX] Handle removing interpreters while removing note Date: Mon, 20 Mar 2017 02:45:37 +0000 (UTC) archived-at: Mon, 20 Mar 2017 02:45:40 -0000 Repository: zeppelin Updated Branches: refs/heads/branch-0.7 8bb71cdf2 -> 8c180852c [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 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 (cherry picked from commit 632815c3582a5e91e9fdc8ee3c0473a0d87c268e) Signed-off-by: Jongyoul Lee Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/8c180852 Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/8c180852 Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/8c180852 Branch: refs/heads/branch-0.7 Commit: 8c180852c03497700cfc22dc093f83460f3f70f8 Parents: 8bb71cd Author: Jongyoul Lee Authored: Mon Mar 20 00:33:50 2017 +0900 Committer: Jongyoul Lee Committed: Mon Mar 20 11:45:31 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/8c180852/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 9f471f5..736ab7d 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 @@ -1014,6 +1014,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/8c180852/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/8c180852/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 474f98c..c399a95 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/8c180852/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(), new Properties(), new ArrayList(), interpreterOption, "", null); + + interpreterSetting.setInterpreterGroupFactory(new InterpreterGroupFactory() { + @Override + public InterpreterGroup createInterpreterGroup(String interpreterGroupId, + InterpreterOption option) { + return new InterpreterGroup(interpreterGroupId); + } + }); + + Interpreter mockInterpreter1 = mock(RemoteInterpreter.class); + List 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(), new Properties(), new ArrayList(), interpreterOption, "", null); + + interpreterSetting.setInterpreterGroupFactory(new InterpreterGroupFactory() { + @Override + public InterpreterGroup createInterpreterGroup(String interpreterGroupId, + InterpreterOption option) { + return new InterpreterGroup(interpreterGroupId); + } + }); + + Interpreter mockInterpreter1 = mock(RemoteInterpreter.class); + List 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(), new Properties(), new ArrayList(), interpreterOption, "", null); + + interpreterSetting.setInterpreterGroupFactory(new InterpreterGroupFactory() { + @Override + public InterpreterGroup createInterpreterGroup(String interpreterGroupId, + InterpreterOption option) { + return new InterpreterGroup(interpreterGroupId); + } + }); + + Interpreter mockInterpreter1 = mock(RemoteInterpreter.class); + List 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(), new Properties(), new ArrayList(), interpreterOption, "", null); + + interpreterSetting.setInterpreterGroupFactory(new InterpreterGroupFactory() { + @Override + public InterpreterGroup createInterpreterGroup(String interpreterGroupId, + InterpreterOption option) { + return new InterpreterGroup(interpreterGroupId); + } + }); + + Interpreter mockInterpreter1 = mock(RemoteInterpreter.class); + List 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()); + } }