zeppelin-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zjf...@apache.org
Subject zeppelin git commit: ZEPPELIN-3435. Interpreter timeout lifecycle leads to interpreter process orphans
Date Tue, 01 May 2018 03:28:13 GMT
Repository: zeppelin
Updated Branches:
  refs/heads/branch-0.8 71e9ca2dc -> 20384bfba


ZEPPELIN-3435. Interpreter timeout lifecycle leads to interpreter process orphans

### What is this PR for?
This issue happens when LifecycleManager try to close InterpreterGroup when it is in the middle
of starting.
This PR change the interface of LifecycleManager, and only add InterpreterGroup to LifecycleManager
only when its interpreter process is started.

### What type of PR is it?
[Bug Fix]

### Todos
* [ ] - Task

### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-3435

### How should this be tested?
* CI pass

### 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: Jeff Zhang <zjffdu@apache.org>

Closes #2951 from zjffdu/ZEPPELIN-3435 and squashes the following commits:

8b2fde1 [Jeff Zhang] ZEPPELIN-3435. Interpreter timeout lifecycle leads to interpreter process
orphans

(cherry picked from commit 97086be4ec8e1d9a506303753b886a3a2177578a)
Signed-off-by: Jeff Zhang <zjffdu@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/20384bfb
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/20384bfb
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/20384bfb

Branch: refs/heads/branch-0.8
Commit: 20384bfbab4490dac26a82dd45cd369660fa64d0
Parents: 71e9ca2
Author: Jeff Zhang <zjffdu@apache.org>
Authored: Sat Apr 28 08:20:45 2018 +0800
Committer: Jeff Zhang <zjffdu@apache.org>
Committed: Tue May 1 11:28:08 2018 +0800

----------------------------------------------------------------------
 .../org/apache/zeppelin/interpreter/LifecycleManager.java |  5 +----
 .../zeppelin/interpreter/ManagedInterpreterGroup.java     |  3 +--
 .../interpreter/lifecycle/NullLifecycleManager.java       |  8 +-------
 .../interpreter/lifecycle/TimeoutLifecycleManager.java    | 10 +++-------
 .../lifecycle/TimeoutLifecycleManagerTest.java            |  9 +++++++--
 5 files changed, 13 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/20384bfb/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/LifecycleManager.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/LifecycleManager.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/LifecycleManager.java
index fc2a7bd..f36cb0d 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/LifecycleManager.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/LifecycleManager.java
@@ -24,10 +24,7 @@ package org.apache.zeppelin.interpreter;
  */
 public interface LifecycleManager {
 
-  void onInterpreterGroupCreated(ManagedInterpreterGroup interpreterGroup);
-
-  void onInterpreterSessionCreated(ManagedInterpreterGroup interpreterGroup,
-                                   String sessionId);
+  void onInterpreterProcessStarted(ManagedInterpreterGroup interpreterGroup);
 
   void onInterpreterUse(ManagedInterpreterGroup interpreterGroup,
                         String sessionId);

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/20384bfb/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ManagedInterpreterGroup.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ManagedInterpreterGroup.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ManagedInterpreterGroup.java
index e19c9ca..ecbaf16 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ManagedInterpreterGroup.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ManagedInterpreterGroup.java
@@ -48,7 +48,6 @@ public class ManagedInterpreterGroup extends InterpreterGroup {
   ManagedInterpreterGroup(String id, InterpreterSetting interpreterSetting) {
     super(id);
     this.interpreterSetting = interpreterSetting;
-    interpreterSetting.getLifecycleManager().onInterpreterGroupCreated(this);
   }
 
   public InterpreterSetting getInterpreterSetting() {
@@ -63,6 +62,7 @@ public class ManagedInterpreterGroup extends InterpreterGroup {
       remoteInterpreterProcess = interpreterSetting.createInterpreterProcess(id, userName,
           properties);
       remoteInterpreterProcess.start(userName);
+      interpreterSetting.getLifecycleManager().onInterpreterProcessStarted(this);
       remoteInterpreterProcess.getRemoteInterpreterEventPoller()
           .setInterpreterProcess(remoteInterpreterProcess);
       remoteInterpreterProcess.getRemoteInterpreterEventPoller().setInterpreterGroup(this);
@@ -156,7 +156,6 @@ public class ManagedInterpreterGroup extends InterpreterGroup {
         interpreter.setInterpreterGroup(this);
       }
       LOGGER.info("Create Session: {} in InterpreterGroup: {} for user: {}", sessionId, id,
user);
-      interpreterSetting.getLifecycleManager().onInterpreterSessionCreated(this, sessionId);
       sessions.put(sessionId, interpreters);
       return interpreters;
     }

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/20384bfb/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/lifecycle/NullLifecycleManager.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/lifecycle/NullLifecycleManager.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/lifecycle/NullLifecycleManager.java
index ce633c6..5a62d22 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/lifecycle/NullLifecycleManager.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/lifecycle/NullLifecycleManager.java
@@ -32,13 +32,7 @@ public class NullLifecycleManager implements LifecycleManager {
   }
 
   @Override
-  public void onInterpreterGroupCreated(ManagedInterpreterGroup interpreterGroup) {
-
-  }
-
-  @Override
-  public void onInterpreterSessionCreated(ManagedInterpreterGroup interpreterGroup,
-                                          String sessionId) {
+  public void onInterpreterProcessStarted(ManagedInterpreterGroup interpreterGroup) {
 
   }
 

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/20384bfb/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/lifecycle/TimeoutLifecycleManager.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/lifecycle/TimeoutLifecycleManager.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/lifecycle/TimeoutLifecycleManager.java
index 7042060..90f3f55 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/lifecycle/TimeoutLifecycleManager.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/lifecycle/TimeoutLifecycleManager.java
@@ -58,18 +58,14 @@ public class TimeoutLifecycleManager implements LifecycleManager {
   }
 
   @Override
-  public void onInterpreterGroupCreated(ManagedInterpreterGroup interpreterGroup) {
+  public void onInterpreterProcessStarted(ManagedInterpreterGroup interpreterGroup) {
+    LOGGER.info("Process of InterpreterGroup {} is started", interpreterGroup.getId());
     interpreterGroups.put(interpreterGroup, System.currentTimeMillis());
   }
 
   @Override
-  public void onInterpreterSessionCreated(ManagedInterpreterGroup interpreterGroup,
-                                          String sessionId) {
-
-  }
-
-  @Override
   public void onInterpreterUse(ManagedInterpreterGroup interpreterGroup, String sessionId)
{
+    LOGGER.debug("InterpreterGroup {} is used in session {}", interpreterGroup.getId(), sessionId);
     interpreterGroups.put(interpreterGroup, System.currentTimeMillis());
   }
 }

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/20384bfb/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/lifecycle/TimeoutLifecycleManagerTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/lifecycle/TimeoutLifecycleManagerTest.java
b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/lifecycle/TimeoutLifecycleManagerTest.java
index 329cb7a..1041502 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/lifecycle/TimeoutLifecycleManagerTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/lifecycle/TimeoutLifecycleManagerTest.java
@@ -54,13 +54,18 @@ public class TimeoutLifecycleManagerTest extends AbstractInterpreterTest
{
     interpreterSettingManager.setInterpreterBinding("user1", "note1", interpreterSettingManager.getSettingIds());
     assertTrue(interpreterFactory.getInterpreter("user1", "note1", "test.echo") instanceof
RemoteInterpreter);
     RemoteInterpreter remoteInterpreter = (RemoteInterpreter) interpreterFactory.getInterpreter("user1",
"note1", "test.echo");
+    assertFalse(remoteInterpreter.isOpened());
+    InterpreterSetting interpreterSetting = interpreterSettingManager.getInterpreterSettingByName("test");
+    assertEquals(1, interpreterSetting.getAllInterpreterGroups().size());
+    Thread.sleep(15*1000);
+    // InterpreterGroup is not removed after 15 seconds, as TimeoutLifecycleManager only
manage it after it is started
+    assertEquals(1, interpreterSetting.getAllInterpreterGroups().size());
+
     InterpreterContext context = new InterpreterContext("noteId", "paragraphId", "repl",
         "title", "text", AuthenticationInfo.ANONYMOUS, new HashMap<String, Object>(),
new GUI(),
         new GUI(), null, null, new ArrayList<InterpreterContextRunner>(), null);
     remoteInterpreter.interpret("hello world", context);
     assertTrue(remoteInterpreter.isOpened());
-    InterpreterSetting interpreterSetting = interpreterSettingManager.getInterpreterSettingByName("test");
-    assertEquals(1, interpreterSetting.getAllInterpreterGroups().size());
 
     Thread.sleep(15 * 1000);
     // interpreterGroup is timeout, so is removed.


Mime
View raw message