You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by adamosloizou <gi...@git.apache.org> on 2014/06/26 13:50:24 UTC
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
GitHub user adamosloizou opened a pull request:
https://github.com/apache/spark/pull/1230
SPARK-2159: Add support for stopping SparkContent on exit() repl command
#### Description
A patch that fixes issue [SPARK-2159](https://issues.apache.org/jira/browse/SPARK-2159) by offering the same stop behavior as when using Ctrl+D (EOF) but with the `exit` or `exit()` repl command.
#### Testing
Added test case in `ReplSuite`
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/adamosloizou/spark fix-stop-context-on-exit
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/1230.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 #1230
----
commit 9ff48509e2a726047e453db6f3a7f6291f38a9c9
Author: Adamos Loizou <ad...@tid.es>
Date: 2014-06-26T11:27:44Z
SPARK-2159: Add support for stopping SparkContent on exit or exit() repl command.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by adamosloizou <gi...@git.apache.org>.
Github user adamosloizou commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47525376
@ScrapCodes thanks for the feedback! I think the original issue was about offering a behaviour similar to the vanilla scala REPL. The `standardCommands` that you suggested are used with a `:` prepended such as `:quit`.
The idea was to offer the scala REPL `exit` functionality (notice the lack of `:`) while also stopping the Spark context.
I could add the quit too if you want but that is not part of the default scala REPL behavior:
```
scala> quit
<console>:8: error: not found: value quit
quit
^
scala> :quit
[success] Total time: 170 s, completed 30-Jun-2014 14:25:50
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by ScrapCodes <gi...@git.apache.org>.
Github user ScrapCodes commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-54045207
@adamosloizou Incase we agree on `:quit` is the correct way. Can you close this PR ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47372033
I think it's unlikely that people are redefining exit() in the shell or using "exit" as a variable name; but just for completeness, you can leave the shell by typing `:quit`.
(btw, if `exit()` is an alias to `System.exit()` or something, maybe registering a shutdown hook would suffice?)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by adamosloizou <gi...@git.apache.org>.
Github user adamosloizou commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-54045637
Fair enough. Closing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47283253
Merged build triggered.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47287480
All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16170/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by adamosloizou <gi...@git.apache.org>.
Github user adamosloizou commented on a diff in the pull request:
https://github.com/apache/spark/pull/1230#discussion_r14282766
--- Diff: repl/src/main/scala/org/apache/spark/repl/SparkILoop.scala ---
@@ -597,7 +597,13 @@ class SparkILoop(in0: Option[BufferedReader], protected val out: JPrintWriter,
if (!awaitInitialized()) return false
runThunks()
}
- if (line eq null) false // assume null means EOF
+ /* Stop loop if:
--- End diff --
Thanks for the nit. Fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by adamosloizou <gi...@git.apache.org>.
Github user adamosloizou commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47514571
@aarondav great idea! Was not aware of the init commands. It works like a charm, thanks! Updated PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by adamosloizou <gi...@git.apache.org>.
Github user adamosloizou closed the pull request at:
https://github.com/apache/spark/pull/1230
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/1230#discussion_r14269907
--- Diff: repl/src/main/scala/org/apache/spark/repl/SparkILoop.scala ---
@@ -597,7 +597,13 @@ class SparkILoop(in0: Option[BufferedReader], protected val out: JPrintWriter,
if (!awaitInitialized()) return false
runThunks()
}
- if (line eq null) false // assume null means EOF
+ /* Stop loop if:
--- End diff --
super nit: start multi-line comment on the next line.
(Also, not sure what's the Spark convention here; but internally we always use "//" for multi-line comments, so that it's easy to comment out long blocks of code including any comments.)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47283273
Merged build started.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47286934
Will this keep working with your patch? (Tested with spark-shell.)
scala> val exit = "foo"
exit: String = foo
scala> exit
res0: String = foo
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by adamosloizou <gi...@git.apache.org>.
Github user adamosloizou commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47748924
FYI I believe `:quit` is considered the correct way in scala 2.11. You can still do something like `exit` but now it forces the user to go for `sys.exit`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47216732
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47287479
Merged build finished. All automated tests passed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by adamosloizou <gi...@git.apache.org>.
Github user adamosloizou commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47508367
@vanzin The shutdown hook was my 1st attempt (it would also take care of Ctrl-C), but I eventually went for the current solution because the hook wouldn't exit gracefully. It bombs with a big exception:
```
uncaught exception during compilation: java.lang.AssertionError
Exception in thread "shutdownHook1" java.lang.AssertionError: assertion failed: Tried to find '$line9' in '/var/folders/yz/cmjy6vj1787fq0kb202qbms40000gp/T/spark-a0256dbe-b4b0-4fa4-b718-b47e8392d84c' but it is not a directory
at scala.reflect.io.AbstractFile.subdirectoryNamed(AbstractFile.scala:254)
at scala.tools.nsc.backend.jvm.BytecodeWriters$class.getFile(BytecodeWriters.scala:31)
at scala.tools.nsc.backend.jvm.BytecodeWriters$class.scala$tools$nsc$backend$jvm$BytecodeWriters$$getFile(BytecodeWriters.scala:37)
at scala.tools.nsc.backend.jvm.BytecodeWriters$ClassBytecodeWriter$class.writeClass(BytecodeWriters.scala:89)
at scala.tools.nsc.backend.jvm.GenASM$AsmPhase$$anon$4.writeClass(GenASM.scala:67)
at scala.tools.nsc.backend.jvm.GenASM$JBuilder.writeIfNotTooBig(GenASM.scala:459)
at scala.tools.nsc.backend.jvm.GenASM$JPlainBuilder.genClass(GenASM.scala:1413)
at scala.tools.nsc.backend.jvm.GenASM$AsmPhase.run(GenASM.scala:120)
at scala.tools.nsc.Global$Run.compileUnitsInternal(Global.scala:1583)
at scala.tools.nsc.Global$Run.compileUnits(Global.scala:1557)
at scala.tools.nsc.Global$Run.compileSources(Global.scala:1553)
at org.apache.spark.repl.SparkIMain.compileSourcesKeepingRun(SparkIMain.scala:468)
at org.apache.spark.repl.SparkIMain$ReadEvalPrint.compileAndSaveRun(SparkIMain.scala:859)
at org.apache.spark.repl.SparkIMain$ReadEvalPrint.compile(SparkIMain.scala:815)
at org.apache.spark.repl.SparkIMain$Request.compile$lzycompute(SparkIMain.scala:1009)
at org.apache.spark.repl.SparkIMain$Request.compile(SparkIMain.scala:1004)
at org.apache.spark.repl.SparkIMain.interpret(SparkIMain.scala:644)
at org.apache.spark.repl.SparkIMain.interpret(SparkIMain.scala:609)
at org.apache.spark.repl.SparkILoop.reallyInterpret$1(SparkILoop.scala:801)
at org.apache.spark.repl.SparkILoop.interpretStartingWith(SparkILoop.scala:846)
at org.apache.spark.repl.SparkILoop.command(SparkILoop.scala:758)
at org.apache.spark.repl.SparkILoop$$anonfun$sparkCleanUp$1.apply(SparkILoop.scala:163)
at org.apache.spark.repl.SparkILoop$$anonfun$sparkCleanUp$1.apply(SparkILoop.scala:163)
at org.apache.spark.repl.SparkIMain.beQuietDuring(SparkIMain.scala:263)
at org.apache.spark.repl.SparkILoop.sparkCleanUp(SparkILoop.scala:162)
at org.apache.spark.repl.SparkILoop.closeInterpreter(SparkILoop.scala:169)
at org.apache.spark.repl.SparkILoop$$anonfun$process$1$$anonfun$apply$mcZ$sp$7.apply$mcV$sp(SparkILoop.scala:942)
at scala.sys.ShutdownHookThread$$anon$1.run(ShutdownHookThread.scala:33)
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by ScrapCodes <gi...@git.apache.org>.
Github user ScrapCodes commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47708185
Yeah I should have said `exit` command and not functionality.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47579685
So you're saying there's no way out of the scala 2.11 repl? That's hardcore.
On Mon, Jun 30, 2014 at 11:49 AM, Prashant Sharma <no...@github.com>
wrote:
> Ahh I think I understand your intention now. We can have this but there is
> one thing I had like to add is this exit functionality is deprecated and is
> not available for scala 2.11.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/spark/pull/1230#issuecomment-47570546>.
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by adamosloizou <gi...@git.apache.org>.
Github user adamosloizou commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47319385
@vanzin great catch! Unfortunately, it will not work with this patch as it captures the `exit` before it passes it down to the evaluation section:
```
scala> val exit = 1
exit: Int = 1
scala> exit
Stopping spark context.
```
From a quick look, it seems to be non-trivial to intercept the `exit` evaluation at a lower level.
The patch seems to only subvert single line evals of `exit`:
```
scala> :paste
// Entering paste mode (ctrl-D to finish)
val exit = 1
exit
// Exiting paste mode, now interpreting.
exit: Int = 1
res0: Int = 1
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by ScrapCodes <gi...@git.apache.org>.
Github user ScrapCodes commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47570546
Ahh I think I understand your intention now. We can have this but there is one thing I had like to add is this exit functionality is deprecated and is not available for scala 2.11.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47395740
Would it be infeasible to just define the method "exit" for the user? Like here:
https://github.com/apache/spark/blob/master/repl/src/main/scala/org/apache/spark/repl/SparkILoopInit.scala#L122
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47282894
@scrapcodes - want to take a quick look at this? Jenkins, test this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by ScrapCodes <gi...@git.apache.org>.
Github user ScrapCodes commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47520651
Hey !, This is okay. You might wanna do it for quit too ?
My actual doubt was that why can't we add it here https://github.com/apache/spark/blob/master/repl/src/main/scala/org/apache/spark/repl/SparkILoop.scala#L286, Haven't tried it myself, but feels like the last argument can call closeInterpreter ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2159: Add support for stopping SparkCont...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:
https://github.com/apache/spark/pull/1230#issuecomment-47580323
I think he means `exit` is gone; you still have `:quit` (or so I hope).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---