You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Hao Hao (Code Review)" <ge...@cloudera.org> on 2017/11/17 02:33:54 UTC
[kudu-CR] [spark] add 'local-cluster' mode for unit test
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8585
Change subject: [spark] add 'local-cluster' mode for unit test
......................................................................
[spark] add 'local-cluster' mode for unit test
This patch adds 'local-cluster' mode for unit test, in which executors
are launched in separate JVMs. This mode is good for debuging issues,
such as serialization.
To enable this mode, set environment variable 'TEST_MODE' to
'local-cluster'. When using this mode, 'SPARK_HOME' has to be set and
point to a real Spark distribution. You can also config driver and
executor log4j configuration through variables 'EXE_LOG_PATH', and
'DRIVER_LOG_PATH'. Example setting is
'EXE_LOG_PATH=file:/spark/executor/log4j_executor.properties'.
Change-Id: I07b2daad6f6883eb7425fa7132208818bbe65788
---
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala
1 file changed, 33 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/85/8585/1
--
To view, visit http://gerrit.cloudera.org:8080/8585
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I07b2daad6f6883eb7425fa7132208818bbe65788
Gerrit-Change-Number: 8585
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
[kudu-CR] [spark] add 'local-cluster' mode for unit test
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8585 )
Change subject: [spark] add 'local-cluster' mode for unit test
......................................................................
Patch Set 1:
(3 comments)
> I pointed SPARK_HOME at the unpacked release tarball downloaded
> from spark.apache.org. Is the source tarball really necessary?
Looking at the spark source code, https://github.com/apache/spark/blob/master/launcher/src/main/java/org/apache/spark/launcher/AbstractCommandBuilder.java#L242, (where the failure actually come from), a built source code do seems necessary.
http://gerrit.cloudera.org:8080/#/c/8585/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala
File java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala:
http://gerrit.cloudera.org:8080/#/c/8585/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala@69
PS1, Line 69: testMode.toLowerCase
> Can be simplified to
I want to do a toLowerCase here, that is why I have the extra null checking. But do you
http://gerrit.cloudera.org:8080/#/c/8585/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala@78
PS1, Line 78: conf.setMaster("local[*]")
> I think it'd be better to throw an exception here so that it fails when TES
Not really, I agree with you we should throw an exception here after thinking more about it.
http://gerrit.cloudera.org:8080/#/c/8585/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala@80
PS1, Line 80: val executorLog: String = System.getenv("EXE_LOG_PATH")
> These configuration knobs are going to be hard to use, and I don't think th
Sure, but we have to add the class path configuration otherwise it will through runtime error.
--
To view, visit http://gerrit.cloudera.org:8080/8585
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I07b2daad6f6883eb7425fa7132208818bbe65788
Gerrit-Change-Number: 8585
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 22 Nov 2017 20:30:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] [spark] add 'local-cluster' mode for unit test
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8585 )
Change subject: [spark] add 'local-cluster' mode for unit test
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8585/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala
File java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala:
http://gerrit.cloudera.org:8080/#/c/8585/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala@69
PS1, Line 69: testMode.toLowerCase
> I want to do a toLowerCase here, that is why I have the extra null checking
sorry for the incomplete sentence. Complete it here: "But do you think a lower case support is necessary?"
--
To view, visit http://gerrit.cloudera.org:8080/8585
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I07b2daad6f6883eb7425fa7132208818bbe65788
Gerrit-Change-Number: 8585
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 22 Nov 2017 20:32:26 +0000
Gerrit-HasComments: Yes
[kudu-CR] [spark] add 'local-cluster' mode for unit test
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8585 )
Change subject: [spark] add 'local-cluster' mode for unit test
......................................................................
Patch Set 1:
I pointed SPARK_HOME at the unpacked release tarball downloaded from spark.apache.org. Is the source tarball really necessary?
--
To view, visit http://gerrit.cloudera.org:8080/8585
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I07b2daad6f6883eb7425fa7132208818bbe65788
Gerrit-Change-Number: 8585
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 21 Nov 2017 21:46:28 +0000
Gerrit-HasComments: No
[kudu-CR] [spark] add 'local-cluster' mode for unit test
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has abandoned this change. ( http://gerrit.cloudera.org:8080/8585 )
Change subject: [spark] add 'local-cluster' mode for unit test
......................................................................
Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/8585
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I07b2daad6f6883eb7425fa7132208818bbe65788
Gerrit-Change-Number: 8585
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [spark] add 'local-cluster' mode for unit test
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8585 )
Change subject: [spark] add 'local-cluster' mode for unit test
......................................................................
Patch Set 1:
> (5 comments)
>
> I wasn't able to get the tests to run on my system, I got a bunch
> of error messages like this:
>
> 12:22:13.383 [ERROR - ExecutorRunner for app-20171121122213-0000/0]
> (Logging.scala:91) Error running executor
> java.lang.IllegalStateException: Cannot find any build directories.
> at org.apache.spark.launcher.CommandBuilderUtils.checkState(CommandBuilderUtils.java:248)
> at org.apache.spark.launcher.AbstractCommandBuilder.getScalaVersion(AbstractCommandBuilder.java:241)
> at org.apache.spark.launcher.AbstractCommandBuilder.buildClassPath(AbstractCommandBuilder.java:195)
> at org.apache.spark.launcher.AbstractCommandBuilder.buildJavaCommand(AbstractCommandBuilder.java:118)
> at org.apache.spark.launcher.WorkerCommandBuilder.buildCommand(WorkerCommandBuilder.scala:39)
> at org.apache.spark.launcher.WorkerCommandBuilder.buildCommand(WorkerCommandBuilder.scala:47)
> at org.apache.spark.deploy.worker.CommandUtils$.buildCommandSeq(CommandUtils.scala:63)
> at org.apache.spark.deploy.worker.CommandUtils$.buildProcessBuilder(CommandUtils.scala:51)
> at org.apache.spark.deploy.worker.ExecutorRunner.org$apache$spark$deploy$worker$ExecutorRunner$$fetchAndRunExecutor(ExecutorRunner.scala:145)
> at org.apache.spark.deploy.worker.ExecutorRunner$$anon$1.run(ExecutorRunner.scala:73)
>
>
>
> Does that ring any bells?
Yes, it should be caused by either the "SPARK_HOME" is not properly set, or you did not happen to build the source code pointed by the "SPARK_HOME"? Sorry that I should also document the latter.
--
To view, visit http://gerrit.cloudera.org:8080/8585
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I07b2daad6f6883eb7425fa7132208818bbe65788
Gerrit-Change-Number: 8585
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 21 Nov 2017 21:22:31 +0000
Gerrit-HasComments: No
[kudu-CR] [spark] add 'local-cluster' mode for unit test
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8585 )
Change subject: [spark] add 'local-cluster' mode for unit test
......................................................................
Patch Set 1:
(5 comments)
I wasn't able to get the tests to run on my system, I got a bunch of error messages like this:
12:22:13.383 [ERROR - ExecutorRunner for app-20171121122213-0000/0] (Logging.scala:91) Error running executor
java.lang.IllegalStateException: Cannot find any build directories.
at org.apache.spark.launcher.CommandBuilderUtils.checkState(CommandBuilderUtils.java:248)
at org.apache.spark.launcher.AbstractCommandBuilder.getScalaVersion(AbstractCommandBuilder.java:241)
at org.apache.spark.launcher.AbstractCommandBuilder.buildClassPath(AbstractCommandBuilder.java:195)
at org.apache.spark.launcher.AbstractCommandBuilder.buildJavaCommand(AbstractCommandBuilder.java:118)
at org.apache.spark.launcher.WorkerCommandBuilder.buildCommand(WorkerCommandBuilder.scala:39)
at org.apache.spark.launcher.WorkerCommandBuilder.buildCommand(WorkerCommandBuilder.scala:47)
at org.apache.spark.deploy.worker.CommandUtils$.buildCommandSeq(CommandUtils.scala:63)
at org.apache.spark.deploy.worker.CommandUtils$.buildProcessBuilder(CommandUtils.scala:51)
at org.apache.spark.deploy.worker.ExecutorRunner.org$apache$spark$deploy$worker$ExecutorRunner$$fetchAndRunExecutor(ExecutorRunner.scala:145)
at org.apache.spark.deploy.worker.ExecutorRunner$$anon$1.run(ExecutorRunner.scala:73)
Does that ring any bells?
http://gerrit.cloudera.org:8080/#/c/8585/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala
File java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala:
http://gerrit.cloudera.org:8080/#/c/8585/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala@69
PS1, Line 69: testMode.toLowerCase
Can be simplified to
System.getenv("TEST_MODE") match {
http://gerrit.cloudera.org:8080/#/c/8585/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala@70
PS1, Line 70: // According to documentation on spark code, this mode: creates a Spark standalone
Might be useful to add a reference to SPARK-595.
http://gerrit.cloudera.org:8080/#/c/8585/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala@78
PS1, Line 78: conf.setMaster("local[*]")
I think it'd be better to throw an exception here so that it fails when TEST_MODE is set without SPARK_HOME. Is there a reason I'm missing to do a silent fallback?
http://gerrit.cloudera.org:8080/#/c/8585/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala@80
PS1, Line 80: val executorLog: String = System.getenv("EXE_LOG_PATH")
These configuration knobs are going to be hard to use, and I don't think they will ultimately be very useful. Can we skip adding them for now? We can always add them back later if they are necessary.
http://gerrit.cloudera.org:8080/#/c/8585/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala@90
PS1, Line 90: "local" | _
can be simplified to _
--
To view, visit http://gerrit.cloudera.org:8080/8585
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I07b2daad6f6883eb7425fa7132208818bbe65788
Gerrit-Change-Number: 8585
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 21 Nov 2017 21:13:42 +0000
Gerrit-HasComments: Yes