You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by WangTaoTheTonic <gi...@git.apache.org> on 2014/09/23 18:52:10 UTC

[GitHub] spark pull request: [SPARK-3658]Take thrift server as a daemon

GitHub user WangTaoTheTonic opened a pull request:

    https://github.com/apache/spark/pull/2509

    [SPARK-3658]Take thrift server as a daemon

    https://issues.apache.org/jira/browse/SPARK-3658
    
    And keep the `CLASS_NOT_FOUND_EXIT_STATUS` and exit message in `SparkSubmit.scala`.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/WangTaoTheTonic/spark thriftserver

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/2509.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 #2509
    
----
commit 598e21eb4ced955e1064edbdc101d1fd77e52e79
Author: WangTao <ba...@aliyun.com>
Date:   2014-09-23T16:49:04Z

    take thrift server as a daemon

----


---
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-3658][SQL]Take thrift server as a daemo...

Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-57082932
  
    @liancheng I tried `export` and it worked. Thanks for the suggestion.
    Also modified permission of `stop-thriftserver.sh`.


---
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-3658][SQL]Take thrift server as a daemo...

Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-57549839
  
    Thanks! I've merged this to master.


---
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-3658]Take thrift server as a daemon

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-56566592
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20710/


---
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-3658][SQL]Take thrift server as a daemo...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-57119163
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/20958/consoleFull) for   PR 2509 at commit [`5dcaab2`](https://github.com/apache/spark/commit/5dcaab2d4ef6c279872aa65e62c1be5456858c6c).
     * This patch **passes** unit tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `          println(s"Failed to load main class $childMainClass.")`



---
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-3658][SQL]Take thrift server as a daemo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2509#discussion_r18122679
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -320,6 +320,10 @@ object SparkSubmit {
         } catch {
           case e: ClassNotFoundException =>
             e.printStackTrace(printStream)
    +        if (childMainClass.contains("thriftserver")) {
    +          println(s"Failed to load main class $childMainClass.")
    +          println("You need to build Spark with -Phive.")
    +        }
    --- End diff --
    
    After some thoughts, I think it's OK to add this checking here. Mainly because it avoids using the exit code checking trick, which may cause potential exit code conflict.


---
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-3658][SQL]Take thrift server as a daemo...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-57083074
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20937/


---
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-3658][SQL]Take thrift server as a daemo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2509#discussion_r18122666
  
    --- Diff: sbin/stop-thriftserver.sh ---
    @@ -0,0 +1,25 @@
    +#!/usr/bin/env bash
    --- End diff --
    
    This file should be executable, please `chmod +x`.


---
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-3658]Take thrift server as a daemon

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-56553241
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/20710/consoleFull) for   PR 2509 at commit [`598e21e`](https://github.com/apache/spark/commit/598e21eb4ced955e1064edbdc101d1fd77e52e79).
     * This patch merges cleanly.


---
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-3658][SQL]Take thrift server as a daemo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2509#discussion_r18126747
  
    --- Diff: sbin/spark-daemon.sh ---
    @@ -142,8 +142,12 @@ case $startStop in
     
         spark_rotate_log "$log"
         echo starting $command, logging to $log
    -    cd "$SPARK_PREFIX"
    -    nohup nice -n $SPARK_NICENESS "$SPARK_PREFIX"/bin/spark-class $command "$@" >> "$log" 2>&1 < /dev/null &
    +    if [ $option == spark-submit ]; then
    +      nohup nice -n $SPARK_NICENESS "$SPARK_PREFIX"/bin/spark-submit --class $command \
    --- End diff --
    
    Hm, that's fair. I'd use `export` in `sbin/start-thriftserver.sh` to fix this issue (exported environment variables are accessible in bash subprocesses):
    
    ```bash
    export SUBMIT_USAGE_FUNCTION=usage
    exec "$FWDIR"/sbin/spark-daemon.sh spark-submit $CLASS 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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3658][SQL]Take thrift server as a daemo...

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-57115385
  
    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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3658][SQL]Take thrift server as a daemo...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-57089022
  
    **[Tests timed out](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/176/consoleFull)** after     a configured wait of `120m`.


---
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-3658]Take thrift server as a daemon

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-56566579
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/20710/consoleFull) for   PR 2509 at commit [`598e21e`](https://github.com/apache/spark/commit/598e21eb4ced955e1064edbdc101d1fd77e52e79).
     * This patch **passes** unit tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `          println(s"Failed to load main class $childMainClass.")`
      * `                logInfo("Interrupting user class to stop.")`



---
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-3658][SQL]Take thrift server as a daemo...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/2509


---
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-3658][SQL]Take thrift server as a daemo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-57085803
  
    After updating my local repo, I found that `stop-thriftserver.sh` is still not executable. Make sure to `git add` this file after `chmod +x`. This is the only pending issue from my perspective.


---
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-3658][SQL]Take thrift server as a daemo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2509#discussion_r18018268
  
    --- Diff: sbin/spark-daemon.sh ---
    @@ -142,8 +142,12 @@ case $startStop in
     
         spark_rotate_log "$log"
         echo starting $command, logging to $log
    -    cd "$SPARK_PREFIX"
    -    nohup nice -n $SPARK_NICENESS "$SPARK_PREFIX"/bin/spark-class $command "$@" >> "$log" 2>&1 < /dev/null &
    +    if [ $command == "org.apache.spark.sql.hive.thriftserver.HiveThriftServer2" ]; then
    --- End diff --
    
    The key difference between `HiveThriftServer2` and other daemon processes is that `HiveThriftServer2` should be started with `spark-submit`. Instead of making `HiveThriftServer2` a special case here, it would be better to add `spark-submit` support for `spark-daemon.sh`. For example, using a special command named "spark-submit", and put the class name as the first arguments.


---
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-3658][SQL]Take thrift server as a daemo...

Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-56818354
  
    @liancheng  As you said, I put "spark-submit" as an option to achieve generalization and use source(dot)  instead of `exec` to make `SUBMISSION_OPTS` and `APPLICATION_OPTS` still work in `spark-daemon.sh`.
    
    As for the ClassNotFound error message, we could either do like this or just delete it?
    
    please check.thanks



---
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-3658][SQL]Take thrift server as a daemo...

Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2509#discussion_r18123634
  
    --- Diff: sbin/spark-daemon.sh ---
    @@ -142,8 +142,12 @@ case $startStop in
     
         spark_rotate_log "$log"
         echo starting $command, logging to $log
    -    cd "$SPARK_PREFIX"
    -    nohup nice -n $SPARK_NICENESS "$SPARK_PREFIX"/bin/spark-class $command "$@" >> "$log" 2>&1 < /dev/null &
    +    if [ $option == spark-submit ]; then
    +      nohup nice -n $SPARK_NICENESS "$SPARK_PREFIX"/bin/spark-submit --class $command \
    --- End diff --
    
    Yeah you are right, but if we put `gatherSparkSubmitOpts` before `spark-submit` there comes a trouble that as `gatherSparkSubmitOpts` takes an argument `SUBMIT_USAGE_FUNCTION` which is different in those scripts who invoke `gatherSparkSubmitOpts`(now we have three: `start-thriftserver.sh`, `spark-sql` and `pyspark`).
    Instead of passing different `SUBMIT_USAGE_FUNCTION` and sourcing `bin/utils.sh` inside of  `spark-daemon.sh`, maybe it's better to assign values to `SUBMISSION_OPTS` and `APPLICATION_OPTS` outside of it.
    There exists another option that we would not passing `SUBMIT_USAGE_FUNCTION` to  `gatherSparkSubmitOpts` which leads to less user-friendliness.


---
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-3658][SQL]Take thrift server as a daemo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2509#discussion_r18018270
  
    --- Diff: sbin/spark-daemon.sh ---
    @@ -142,8 +142,12 @@ case $startStop in
     
         spark_rotate_log "$log"
         echo starting $command, logging to $log
    -    cd "$SPARK_PREFIX"
    -    nohup nice -n $SPARK_NICENESS "$SPARK_PREFIX"/bin/spark-class $command "$@" >> "$log" 2>&1 < /dev/null &
    +    if [ $command == "org.apache.spark.sql.hive.thriftserver.HiveThriftServer2" ]; then
    +      nohup nice -n $SPARK_NICENESS "$SPARK_PREFIX"/bin/spark-submit --class $command \
    +        "${SUBMISSION_OPTS[@]}" spark-internal "${APPLICATION_OPTS[@]}" >> "$log" 2>&1 < /dev/null &
    --- End diff --
    
    I don't think `SUBMISSION_OPTS` and `APPLICATION_OPTS` are properly defined here. Function `gatherSparkSubmitOpts` should be called here to define these two variables. You may refer to `sbin/start-thriftserver.sh` for details.


---
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-3658][SQL]Take thrift server as a daemo...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-56825846
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/20807/consoleFull) for   PR 2509 at commit [`8ad9f95`](https://github.com/apache/spark/commit/8ad9f95f66105154a28d10999cc53cb6f3e48004).
     * This patch **passes** unit tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `          println(s"Failed to load main class $childMainClass.")`



---
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-3658][SQL]Take thrift server as a daemo...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-56825854
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20807/


---
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-3658][SQL]Take thrift server as a daemo...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-57085586
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/176/consoleFull) for   PR 2509 at commit [`5dcaab2`](https://github.com/apache/spark/commit/5dcaab2d4ef6c279872aa65e62c1be5456858c6c).
     * This patch merges cleanly.


---
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-3658][SQL]Take thrift server as a daemo...

Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-57097696
  
    eh...I cloned the repository on another laptop and found it's executable, as shown in top-left corner of https://github.com/WangTaoTheTonic/spark/blob/thriftserver/sbin/stop-thriftserver.sh.
    
    Could you verify this again?


---
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-3658][SQL]Take thrift server as a daemo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2509#discussion_r18126738
  
    --- Diff: sbin/spark-daemon.sh ---
    @@ -142,8 +142,12 @@ case $startStop in
     
         spark_rotate_log "$log"
         echo starting $command, logging to $log
    -    cd "$SPARK_PREFIX"
    -    nohup nice -n $SPARK_NICENESS "$SPARK_PREFIX"/bin/spark-class $command "$@" >> "$log" 2>&1 < /dev/null &
    +    if [ $option == spark-submit ]; then
    +      nohup nice -n $SPARK_NICENESS "$SPARK_PREFIX"/bin/spark-submit --class $command \
    --- End diff --
    
    Hm, that's fair. I'd use `export` in `sbin/start-thriftserver.sh` before `exec` to fix this issue (exported environment variables can be accessed by bash subprocesses):
    
    ```bash
    export SUBMIT_USAGE_FUNCTION=usage
    exec "$FWDIR"/bin/spark-submit --class $CLASS "${SUBMISSION_OPTS[@]}" spark-internal "${APPLICATION_OPTS[@]}"
    ```


---
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-3658][SQL]Take thrift server as a daemo...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-57115522
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/20958/consoleFull) for   PR 2509 at commit [`5dcaab2`](https://github.com/apache/spark/commit/5dcaab2d4ef6c279872aa65e62c1be5456858c6c).
     * This patch merges cleanly.


---
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-3658][SQL]Take thrift server as a daemo...

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-56779230
  
    @liancheng - can you take a look at this one?


---
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-3658][SQL]Take thrift server as a daemo...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-57119166
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20958/


---
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-3658][SQL]Take thrift server as a daemo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-57047019
  
    Thanks for working on this! I tested this PR locally and it works fine. But there are still some minor issues pending to be resolved, please refer to the comments for details.


---
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-3658][SQL]Take thrift server as a daemo...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-56814057
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/20807/consoleFull) for   PR 2509 at commit [`8ad9f95`](https://github.com/apache/spark/commit/8ad9f95f66105154a28d10999cc53cb6f3e48004).
     * This patch merges cleanly.


---
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-3658][SQL]Take thrift server as a daemo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-56784684
  
    Generally this is a good idea. But it would be better to make `spark-daemon.sh` more general, rather than making `HiveThriftServer2` a special case.


---
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-3658][SQL]Take thrift server as a daemo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2509#discussion_r18122728
  
    --- Diff: sbin/spark-daemon.sh ---
    @@ -142,8 +142,12 @@ case $startStop in
     
         spark_rotate_log "$log"
         echo starting $command, logging to $log
    -    cd "$SPARK_PREFIX"
    -    nohup nice -n $SPARK_NICENESS "$SPARK_PREFIX"/bin/spark-class $command "$@" >> "$log" 2>&1 < /dev/null &
    +    if [ $option == spark-submit ]; then
    +      nohup nice -n $SPARK_NICENESS "$SPARK_PREFIX"/bin/spark-submit --class $command \
    --- End diff --
    
    We need to call `gatherSparkSubmitOpts "$@"` before this line to generate `SUBMISSION_OPTS` and `APPLICATION_OPTS`. I don't think we should source `spark-daemon.sh` in `start-thriftserver.sh` because
    
    1. `gatherSparkSubmitOpts` is tightly coupled with `spark-submit`. In fact we should always call `gatherSparkSubmitOpts` before calling `spark-submit`. Put them together is more preferable.
    2. When adding another daemon component that uses `spark-submit` in the future, we don't want to duplicate the `gatherSparkSubmitOpts` call in the new start script.


---
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-3658][SQL]Take thrift server as a daemo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2509#discussion_r18018267
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala ---
    @@ -320,6 +320,10 @@ object SparkSubmit {
         } catch {
           case e: ClassNotFoundException =>
             e.printStackTrace(printStream)
    +        if (childMainClass.contains("thriftserver")) {
    +          println(s"Failed to load main class $childMainClass.")
    +          println("You need to build Spark with -Phive.")
    +        }
    --- End diff --
    
    I feel complicated to add this logic here... `SparkSubmit` should not be coupled with Thrift server, but it seems that this is the only legitimate place.


---
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-3658][SQL]Take thrift server as a daemo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:

    https://github.com/apache/spark/pull/2509#issuecomment-57108375
  
    Ah, sorry, my fault. Then this LGTM, thanks!


---
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