zeppelin-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jongy...@apache.org
Subject zeppelin git commit: [HOTFIX][ZEPPELIN-2037][ZEPPELIN-1832] Restart with several options include "per user/per note" and "scoped/isolated"
Date Sat, 18 Mar 2017 04:24:35 GMT
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 <jongyoul@gmail.com>

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 <jongyoul@gmail.com>
Authored: Fri Mar 17 23:46:43 2017 +0900
Committer: Jongyoul Lee <jongyoul@apache.org>
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


Mime
View raw message