[GitHub] [zeppelin] liuxunorg commented on a change in pull request #3342: [ZEPPELIN-4031] Fixed Unable to detect that the interpreter process was killed manually
liuxunorg commented on a change in pull request #3342: [ZEPPELIN-4031] Fixed
Unable to detect that the interpreter process was killed manually
URL: https://github.com/apache/zeppelin/pull/3342#discussion_r269836270
##
File path:
zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreter.java
##
@@ -199,13 +199,31 @@ public Void call(Client client) throws Exception {
}
}
+ // Detecting an invalid interpreter process,
+ // Clean up session, Recreate the interpreter process
+ private void resurrectionInvalidIntpProcess() throws InterpreterException {
+if (interpreterProcess != null && !interpreterProcess.isRunning() &&
isOpened) {
+ LOGGER.info("Check whether the InterpreterProcess has been shutdown.");
+ // clean invalid session and dirty data of interpreterSetting
+ ManagedInterpreterGroup intpGroup = getInterpreterGroup();
+ intpGroup.close();
+ // recreate RemoteInterpreterProcess
+ isOpened = false;
+ isCreated = false;
+ open();
+}
+ }
+
@Override
public InterpreterResult interpret(final String st, final InterpreterContext
context)
throws InterpreterException {
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("st:\n{}", st);
}
+// Detecting an invalid interpreter process
+resurrectionInvalidIntpProcess();
Review comment:
Every time the user executes a note, Called
`RemoteInterpreter.java::interpret(final String st, final InterpreterContext
context)` function,
Check if the remote interpreter process is valid, If the remote interpreter
is invalid,
In the `RemoteInterpreter.java`, It is very easy to perform session
recycling for the interpreter process,
And recreate the interpreter. Conforms to zeppelin's original process of
creating an interpreter.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zeppelin] liuxunorg commented on a change in pull request #3342: [ZEPPELIN-4031] Fixed Unable to detect that the interpreter process was killed manually
liuxunorg commented on a change in pull request #3342: [ZEPPELIN-4031] Fixed
Unable to detect that the interpreter process was killed manually
URL: https://github.com/apache/zeppelin/pull/3342#discussion_r268967854
##
File path:
zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ManagedInterpreterGroup.java
##
@@ -141,17 +165,27 @@ private void close(Collection interpreters)
{
private void closeInterpreter(Interpreter interpreter) {
Scheduler scheduler = interpreter.getScheduler();
-for (final Job job : scheduler.getAllJobs()) {
- job.abort();
- job.setStatus(Job.Status.ABORT);
- LOGGER.info("Job " + job.getJobName() + " aborted ");
-}
+// Need to abort the task being executed
+// when actively shutting down the remote interpreter
+if (false == remoteIntpProcessIsShutdown()) {
Review comment:
### 1. If interpreter process is normal, isRunning() must equal true
`isRunning()` is not a flag, It's a function call.
`interpreterProcess.isRunning()` Will call
`RemoteInterpreterUtils.java::checkIfRemoteEndpointAccessible()` , Check if the
remote interpreter is available through the socket.
### 2. Avoid invalid calls
`Job.abort()` Will go to the remote to call the interpreter,
`Interpreter.cancel(getInterpreterContext(null));` function. If the interpreter
is no longer connected, The `interpreter.cancel()` function cannot be called
correctly.
But if you find that the remote interpreter is not available in
`job.abort()`, Will pass again
`RemoteInterpreter.java::getOrCreateInterpreterProcess()`, Trying to create
an interpreter process, This will fail due to a 30 second timeout.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zeppelin] liuxunorg commented on a change in pull request #3342: [ZEPPELIN-4031] Fixed Unable to detect that the interpreter process was killed manually
liuxunorg commented on a change in pull request #3342: [ZEPPELIN-4031] Fixed
Unable to detect that the interpreter process was killed manually
URL: https://github.com/apache/zeppelin/pull/3342#discussion_r268965044
##
File path:
zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/ManagedInterpreterGroup.java
##
@@ -58,10 +58,19 @@ public InterpreterSetting getInterpreterSetting() {
public synchronized RemoteInterpreterProcess
getOrCreateInterpreterProcess(String userName,
Properties properties)
throws IOException {
+if (remoteIntpProcessIsShutdown()) {
+ LOGGER.info("Check whether the InterpreterProcess has been shutdown.");
+ // clean invalid session and dirty data of interpreterSetting
+ close();
Review comment:
`remoteIntpProcessIsShutdown()==true`, Found that the remote interpreter
process is unavailable, clean invalid session by close() function.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zeppelin] liuxunorg commented on a change in pull request #3342: [ZEPPELIN-4031] Fixed Unable to detect that the interpreter process was killed manually
liuxunorg commented on a change in pull request #3342: [ZEPPELIN-4031] Fixed
Unable to detect that the interpreter process was killed manually
URL: https://github.com/apache/zeppelin/pull/3342#discussion_r268963065
##
File path:
zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreter.java
##
@@ -103,7 +103,7 @@ public String getSessionId() {
}
public synchronized RemoteInterpreterProcess getOrCreateInterpreterProcess()
throws IOException {
-if (this.interpreterProcess != null) {
+if (this.interpreterProcess != null && interpreterProcess.isRunning()) {
return this.interpreterProcess;
}
ManagedInterpreterGroup intpGroup = getInterpreterGroup();
Review comment:
The original code is not perfect.
Because `RemoteInterpreter.java::getOrCreateInterpreterProcess()` is called,
an available RemoteInterpreter process must be returned.
`interpreterProcess.isRunning()` Will call
`RemoteInterpreterUtils.java::checkIfRemoteEndpointAccessible()` Check if the
remote interpreter is available through the socket.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
