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 05:21:14 GMT
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 <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/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 <jongyoul@gmail.com>
Authored: Fri Mar 17 23:46:43 2017 +0900
Committer: Jongyoul Lee <jongyoul@gmail.com>
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


Mime
View raw message