Repository: zeppelin Updated Branches: refs/heads/branch-0.6 d30751ada -> dce37cdab
[ZEPPELIN-1234] Fix issue related to indefinite waiting of zeppelin server when existing remote interpreter is restarted ### What is this PR for? This PR fix the issue related to indefinite waiting of zeppelin server when existing remote interpreter is restarted ### What type of PR is it? Bug fix ### Todos * [ ] - NA ### What is the Jira issue? * https://issues.apache.org/jira/browse/ZEPPELIN-1234 ### How should this be tested? -Create a remote interpreter,start and connect it as mentioned in doc http://zeppelin.apache.org/docs/0.6.0/manual/interpreters.html#connecting-to-the-existing-remote-interpreter - Try to restart the interpreter ### 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: Sachin <[email protected]> Closes #1234 from SachinJanani/branch-0.6 and squashes the following commits: aee03a0 [Sachin] Incorporated review comments by adding Test 8688548 [Sachin] [ZEPPELIN-1234] Fix for bug ZEPPELIN-1234 Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/dce37cda Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/dce37cda Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/dce37cda Branch: refs/heads/branch-0.6 Commit: dce37cdab97e507e0f4cf0c65f146f6c81550eea Parents: d30751a Author: Sachin <[email protected]> Authored: Fri Jul 29 16:50:22 2016 +0530 Committer: Jongyoul Lee <[email protected]> Committed: Thu Aug 4 00:39:58 2016 +0900 ---------------------------------------------------------------------- .../remote/RemoteInterpreterProcess.java | 2 +- .../remote/RemoteInterpreterProcessTest.java | 49 ++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/dce37cda/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterProcess.java ---------------------------------------------------------------------- diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterProcess.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterProcess.java index b4579bc..7cad9e5 100644 --- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterProcess.java +++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterProcess.java @@ -256,7 +256,7 @@ public class RemoteInterpreterProcess implements ExecuteResultHandler { } } - if (isRunning()) { + if (isRunning() && !isInterpreterAlreadyExecuting) { logger.info("kill interpreter process"); watchdog.destroyProcess(); } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/dce37cda/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterProcessTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterProcessTest.java b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterProcessTest.java index 77571a9..e4a80c4 100644 --- a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterProcessTest.java +++ b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterProcessTest.java @@ -135,4 +135,53 @@ public class RemoteInterpreterProcessTest { assertEquals(2, rip.reference(intpGroup)); assertEquals(true, rip.isRunning()); } + + @Test + public void testExistingInterpreterDereference() throws TException, InterruptedException { + // Using Mocked RemoteInterpreterServer to reproduce the issue. + CustomRemoteInterpreterServer server = new CustomRemoteInterpreterServer(3680); + server.start(); + long startTime = System.currentTimeMillis(); + /* + * If RemoteInterpreterServer didn't start within 30 seconds than this test may fail which might + * be due to issue in RemoteInterpreterServer + */ + while (System.currentTimeMillis() - startTime < 30 * 1000) { + if (server.isRunning()) { + break; + } else { + Thread.sleep(200); + } + } + Properties properties = new Properties(); + properties.setProperty(Constants.ZEPPELIN_INTERPRETER_PORT, "3680"); + properties.setProperty(Constants.ZEPPELIN_INTERPRETER_HOST, "localhost"); + InterpreterGroup intpGroup = mock(InterpreterGroup.class); + when(intpGroup.getProperty()).thenReturn(properties); + when(intpGroup.containsKey(Constants.EXISTING_PROCESS)).thenReturn(true); + RemoteInterpreterProcess rip = new RemoteInterpreterProcess(INTERPRETER_SCRIPT, "nonexists", + "fakeRepo", new HashMap<String, String>(), 1000, null); + assertFalse(rip.isRunning()); + assertEquals(0, rip.referenceCount()); + assertEquals(1, rip.reference(intpGroup)); + // Calling reference once again to depict multiple intrepreters in a group + assertEquals(2, rip.reference(intpGroup)); + assertEquals(true, rip.isRunning()); + rip.dereference(); + rip.dereference(); + } + + + class CustomRemoteInterpreterServer extends RemoteInterpreterServer { + public CustomRemoteInterpreterServer(int port) throws TTransportException { + super(port); + } + + @Override + public void shutdown() throws TException { + // Keeping this method intentionally empty to depict that server is not stopped + } + + } + }
