You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@toree.apache.org by kevin-bates <gi...@git.apache.org> on 2017/09/11 16:20:37 UTC
[GitHub] incubator-toree pull request #140: [TOREE-437] Establish alternate interrupt...
GitHub user kevin-bates opened a pull request:
https://github.com/apache/incubator-toree/pull/140
[TOREE-437] Establish alternate interrupt handler for background execution
Cell interrupts do not take place if Toree is running in the background (typically due to its parent being in the background). This PR adds support for the caller of Toree to specify an alternate interrupt signal via the environment variable `TOREE_ALTERNATE_SIGINT` - the value of which should be the string indicating the alternate signal name's suffix portion (e.g., "USR2" for `SIGUSR2`).
The common scenario would be to specify this env value in the kernel.json's `env:` stanza when necessary ...
```json
"display_name": "Apache Toree - Scala",
"env": {
"DEFAULT_INTERPRETER": "Scala",
"PYTHON_EXEC": "python",
"__TOREE_SPARK_OPTS__": "",
"__TOREE_OPTS__": "",
"TOREE_ALTERNATE_SIGINT": "USR2",
```
If the value of `TOREE_ALTERNATE_SIGINT` is not recognized as a valid signal name, a warning message will be logged, but the kernel's launch will proceed:
```
17/09/11 08:44:37 [WARN] o.a.t.Main$$anon$1 - Error occurred establishing alternate signal handler. Value of TOREE_ALTERNATE_SIGINT is probably bad: FOO. Error: Unknown signal: FOO
```
Fixes TOREE-437
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kevin-bates/incubator-toree alt-interrupt-signal
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-toree/pull/140.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #140
----
commit be78e5474b0b97a87a88ba35098af3aeb381edf4
Author: Kevin Bates <ke...@us.ibm.com>
Date: 2017-09-11T16:09:53Z
[TOREE-437] Establish alternate interrupt handler for background execution
----
---
[GitHub] incubator-toree pull request #140: [TOREE-437] Establish alternate interrupt...
Posted by lresende <gi...@git.apache.org>.
Github user lresende commented on a diff in the pull request:
https://github.com/apache/incubator-toree/pull/140#discussion_r138427625
--- Diff: kernel/src/main/scala/org/apache/toree/boot/layer/HookInitialization.scala ---
@@ -89,6 +90,29 @@ trait StandardHookInitialization extends HookInitialization {
}
}
})
+ // Define handler for alternate signal that will be fired in cases where
+ // the caller is in a background process in order to interrupt
+ // cell operations - since SIGINT doesn't propagate in those cases.
+ // Like INT above except we don't need to deal with shutdown in
+ // repeated situations.
+ val altSigint = System.getenv("TOREE_ALTERNATE_SIGINT")
+ if (altSigint != null) {
+ try {
+ Signal.handle(new Signal(altSigint), new SignalHandler() {
+
+ def handle(sig: Signal) = {
+ logger.info("Resetting code execution due to interrupt!")
+ interpreter.interrupt()
+
+ // TODO: Cancel group representing current code execution
+ //sparkContext.cancelJobGroup()
+ }
+ })
+ } catch {
+ case e:Exception => logger.warn("Error occurred establishing alternate signal handler. Value of " +
+ "TOREE_ALTERNATE_SIGINT is probably bad: " + altSigint + ". Error: " + e.getMessage )
--- End diff --
should it say 'bad' or some more specific message, or even no message and assume more details coming from the exception message?
---
[GitHub] incubator-toree pull request #140: [TOREE-437] Establish alternate interrupt...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-toree/pull/140
---
[GitHub] incubator-toree pull request #140: [TOREE-437] Establish alternate interrupt...
Posted by lresende <gi...@git.apache.org>.
Github user lresende commented on a diff in the pull request:
https://github.com/apache/incubator-toree/pull/140#discussion_r138427011
--- Diff: kernel/src/main/scala/org/apache/toree/boot/layer/HookInitialization.scala ---
@@ -69,8 +69,9 @@ trait StandardHookInitialization extends HookInitialization {
// TODO: Signals are not a good way to handle this since JVM only has the
// proprietary sun API that is not necessarily available on all platforms
Signal.handle(new Signal("INT"), new SignalHandler() {
- private val MaxSignalTime: Long = 3000 // 3 seconds
- var lastSignalReceived: Long = 0
+ private val MaxSignalTime: Long = 3000
+ // 3 seconds
+ var lastSignalReceived: Long = 0
--- End diff --
Can we remove the formatting changes above?
---