You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by yaooqinn <gi...@git.apache.org> on 2017/08/28 09:33:42 UTC

[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should ...

GitHub user yaooqinn opened a pull request:

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

    [SPARK-21428][SQL][FOLLOWUP] Reused state should respect warehouse dir determined in SharedState

    ## What changes were proposed in this pull request?
    
    While running spark-sql, we will reuse cliSessionState, but the warehouse is determined later in SharedState. HiveClient should respect this config changing in this case.
    
    ## How was this patch tested?
    existing ut
    
    cc @cloud-fan @jiangxb1987 

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

    $ git pull https://github.com/yaooqinn/spark SPARK-21428-FOLLOWUP

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

    https://github.com/apache/spark/pull/19068.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 #19068
    
----
commit b9e9a19fdc36a2b65e46406f78e5d50d211a3e45
Author: Kent Yao <ya...@hotmail.com>
Date:   2017-08-28T09:25:17Z

    SPARK-21428-FOLLOWUP: CliSessionState should respect warehouse dir determined in SharedState

----


---
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 issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

    https://github.com/apache/spark/pull/19068
  
    Merged build finished. Test FAILed.


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

    https://github.com/apache/spark/pull/19068
  
    Merged build finished. Test FAILed.


---

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


[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should ...

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

    https://github.com/apache/spark/pull/19068#discussion_r135855867
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala ---
    @@ -81,11 +81,7 @@ private[hive] object SparkSQLCLIDriver extends Logging {
           System.exit(1)
         }
     
    -    val cliConf = new HiveConf(classOf[SessionState])
    -    // Override the location of the metastore since this is only used for local execution.
    -    HiveUtils.newTemporaryConfiguration(useInMemoryDerby = false).foreach {
    --- End diff --
    
    I don't think you have answered the question here - for example, the original code will update the `HiveConf.ConfVars.METASTORECONNECTURLKEY` property here, but you don't touch that after the change. I'm not confident to make that behavior change.


---
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 #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState shoul...

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

    https://github.com/apache/spark/pull/19068#discussion_r138543749
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala ---
    @@ -231,6 +231,42 @@ private[spark] object HiveUtils extends Logging {
         }.toMap
       }
     
    +  private[hive] def newHiveConfigurations(
    +      sparkConf: SparkConf = new SparkConf(loadDefaults = true),
    +      classLoader: ClassLoader = null)(
    +      hadoopConf: Configuration = SparkHadoopUtil.get.newConfiguration(sparkConf))(
    +      extraConfig: Map[String, String] = hiveClientConfigurations(hadoopConf)): HiveConf = {
    --- End diff --
    
    shouldn't the default value of `extraConfig` be Map.empty?


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

    https://github.com/apache/spark/pull/19068
  
    **[Test build #81909 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81909/testReport)** for PR 19068 at commit [`f2618b9`](https://github.com/apache/spark/commit/f2618b9bd8a8c5c6dcd50257f089e2412b30b252).


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    retest this please


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should respect...

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

    https://github.com/apache/spark/pull/19068
  
    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.
---

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


[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState shoul...

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

    https://github.com/apache/spark/pull/19068#discussion_r138564306
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala ---
    @@ -231,6 +231,42 @@ private[spark] object HiveUtils extends Logging {
         }.toMap
       }
     
    +  private[hive] def newHiveConfigurations(
    --- End diff --
    
    hiveClientConfigurations is used to change those time values in the form of "1s"/ "10min" to long values, we may give it a more proper name 


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

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


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

    https://github.com/apache/spark/pull/19068
  
    **[Test build #81909 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81909/testReport)** for PR 19068 at commit [`f2618b9`](https://github.com/apache/spark/commit/f2618b9bd8a8c5c6dcd50257f089e2412b30b252).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

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


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should respect...

Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    @gatorsmile Hi Sean,
    I am hitting this issue. Actually this seems like a regression as my script which was working before is no longer working. Here is my scenario.
    1) spark-sql
        create database testdb;
    2) exit spark-sql
    3) spark-sql
        use testdb;  => I get a database not found error.
    Can we please have this reviewed and merged if the changes are good. Thanks in advance.



---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should respect...

Posted by yaooqinn <gi...@git.apache.org>.
Github user yaooqinn commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    @dilipbiswal This is because the CliSessionState instance initialized in SparkCLISQLDriver pointing to a dummy metastore and reused later in the hive metastore client


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

    https://github.com/apache/spark/pull/19068
  
    **[Test build #81906 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81906/testReport)** for PR 19068 at commit [`b21fc72`](https://github.com/apache/spark/commit/b21fc7222c313eb570f788afb5b02a9a67c881a4).


---

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


[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState shoul...

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

    https://github.com/apache/spark/pull/19068#discussion_r138543230
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
    @@ -132,43 +134,26 @@ private[hive] class HiveClientImpl(
           // in hive jars, which will turn off isolation, if SessionSate.detachSession is
           // called to remove the current state after that, hive client created later will initialize
           // its own state by newState()
    -      Option(SessionState.get).getOrElse(newState())
    +      val ret = SessionState.get
    +      if (ret != null) {
    +        Option(hadoopConf.get(ConfVars.METASTOREWAREHOUSE.varname)).foreach { dir =>
    --- End diff --
    
    can you add some comments to explain this logic?


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

    https://github.com/apache/spark/pull/19068
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

Posted by yaooqinn <gi...@git.apache.org>.
Github user yaooqinn commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    retest this please


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

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


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

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


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

Posted by yaooqinn <gi...@git.apache.org>.
Github user yaooqinn commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    cc @cloud-fan @jiangxb1987 @gatorsmile pr title and descriptions updated


---

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


[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState shoul...

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

    https://github.com/apache/spark/pull/19068#discussion_r138625678
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala ---
    @@ -232,6 +232,54 @@ private[spark] object HiveUtils extends Logging {
       }
     
       /**
    +   * Generate an instance of [[HiveConf]] from [[SparkConf]]& hadoop [[Configuration]] &
    +   * formatted extra time configurations with an isolated classloader needed if isolationOn
    +   * for [[HiveClient]] construction
    +   * @param sparkConf a [[SparkConf]] object specifying Spark parameters
    +   * @param classLoader an isolated classloader needed if isolationOn for [[HiveClient]]
    +   *                    construction
    +   * @param hadoopConf a hadoop [[Configuration]] object, Optional if we want generated it from
    +   *                   the sparkConf
    +   * @param extraTimeConfs time configurations in the form of long values from the given hadoopConf
    --- End diff --
    
    OK



---

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


[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState shoul...

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

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


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

    https://github.com/apache/spark/pull/19068
  
    Merged build finished. Test FAILed.


---

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


[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState shoul...

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

    https://github.com/apache/spark/pull/19068#discussion_r138557772
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala ---
    @@ -231,6 +231,42 @@ private[spark] object HiveUtils extends Logging {
         }.toMap
       }
     
    +  private[hive] def newHiveConfigurations(
    +      sparkConf: SparkConf = new SparkConf(loadDefaults = true),
    +      classLoader: ClassLoader = null)(
    +      hadoopConf: Configuration = SparkHadoopUtil.get.newConfiguration(sparkConf))(
    +      extraConfig: Map[String, String] = hiveClientConfigurations(hadoopConf)): HiveConf = {
    --- End diff --
    
    This doesn't change the original logic,  check https://github.com/apache/spark/blob/master/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala#L283.


---

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


[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState shoul...

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

    https://github.com/apache/spark/pull/19068#discussion_r138818865
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala ---
    @@ -232,6 +232,54 @@ private[spark] object HiveUtils extends Logging {
       }
     
       /**
    +   * Generate an instance of [[HiveConf]] from [[SparkConf]]& hadoop [[Configuration]] &
    +   * other extra configs such as formatted extra time configurations with an isolated
    +   * classloader needed if isolationOn for [[HiveClient]] construction.
    +   * @param sparkConf a [[SparkConf]] object specifying Spark parameters.
    +   * @param classLoader an isolated classloader needed if isolationOn for [[HiveClient]]
    +   *                    construction.
    +   * @param hadoopConf a hadoop [[Configuration]] object, Optional if we want generated it from
    +   *                   the sparkConf.
    +   * @param extraConfig time configurations in the form of long values from the given hadoopConf
    --- End diff --
    
    `extraConfig` -> `extraConfigs`, and please update the description, it's not `time configurations`.


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

Posted by yaooqinn <gi...@git.apache.org>.
Github user yaooqinn commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    jenkins unreachable cc @cloud-fan 


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    retest this please


---

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


[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should ...

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

    https://github.com/apache/spark/pull/19068#discussion_r136766581
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala ---
    @@ -81,11 +81,7 @@ private[hive] object SparkSQLCLIDriver extends Logging {
           System.exit(1)
         }
     
    -    val cliConf = new HiveConf(classOf[SessionState])
    -    // Override the location of the metastore since this is only used for local execution.
    -    HiveUtils.newTemporaryConfiguration(useInMemoryDerby = false).foreach {
    --- End diff --
    
    @jiangxb1987 sorry for not @ you, please take a look 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 #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should ...

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

    https://github.com/apache/spark/pull/19068#discussion_r135937401
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala ---
    @@ -81,11 +81,7 @@ private[hive] object SparkSQLCLIDriver extends Logging {
           System.exit(1)
         }
     
    -    val cliConf = new HiveConf(classOf[SessionState])
    -    // Override the location of the metastore since this is only used for local execution.
    -    HiveUtils.newTemporaryConfiguration(useInMemoryDerby = false).foreach {
    --- End diff --
    
    Change this key to connect a dummy metastore instead of the real 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 issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

Posted by yaooqinn <gi...@git.apache.org>.
Github user yaooqinn commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    @cloud-fan i met linkage err before, and now i simplify the logic, could you trigger jenkins before reverting


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    > but the Hive configurations generated here just points to a dummy meta store
    
    Why did this works well before?


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

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


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    OK to test


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

    https://github.com/apache/spark/pull/19068
  
    **[Test build #81721 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81721/testReport)** for PR 19068 at commit [`9682eab`](https://github.com/apache/spark/commit/9682eabd4184340745e54b9eef8ac878ca942ba3).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

    https://github.com/apache/spark/pull/19068
  
    Merged build finished. Test FAILed.


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

    https://github.com/apache/spark/pull/19068
  
    **[Test build #81718 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81718/testReport)** for PR 19068 at commit [`267a1b2`](https://github.com/apache/spark/commit/267a1b2f5bb83b4f20810f704105c0d996b71e93).


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    @dilipbiswal does your script work after this PR?


---

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


[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState shoul...

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

    https://github.com/apache/spark/pull/19068#discussion_r138544218
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala ---
    @@ -231,6 +231,42 @@ private[spark] object HiveUtils extends Logging {
         }.toMap
       }
     
    +  private[hive] def newHiveConfigurations(
    --- End diff --
    
    can you add some comments for this method? Especially what's the difference between this one and `hiveClientConfigurations`.


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

    https://github.com/apache/spark/pull/19068
  
    **[Test build #81910 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81910/testReport)** for PR 19068 at commit [`c5c1c26`](https://github.com/apache/spark/commit/c5c1c2625d33dd08cbdde2e25041359bbbf50339).


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

    https://github.com/apache/spark/pull/19068
  
    **[Test build #81916 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81916/testReport)** for PR 19068 at commit [`c5c1c26`](https://github.com/apache/spark/commit/c5c1c2625d33dd08cbdde2e25041359bbbf50339).


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    @cloud-fan Yeah.. I have tried my script against this PR and it works fine. I am not familiar with the changes and don't know if it can have any side effects.  One thing that haven't had the time to find out is in my script ..
    ```
    1) spark-sql
        create database testdb;
    2) exit spark-sql
    3) spark-sql
        use testdb;  => I get a database not found error.
    ```
    How did the create database succeed i.e i didn't get any error ? If it did, then where did it create the database at ? Perhaps you know the answer :-)



---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    @cloud-fan it didn't trigger the test ?


---

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


[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState shoul...

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

    https://github.com/apache/spark/pull/19068#discussion_r138619511
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala ---
    @@ -232,6 +232,54 @@ private[spark] object HiveUtils extends Logging {
       }
     
       /**
    +   * Generate an instance of [[HiveConf]] from [[SparkConf]]& hadoop [[Configuration]] &
    +   * formatted extra time configurations with an isolated classloader needed if isolationOn
    +   * for [[HiveClient]] construction
    +   * @param sparkConf a [[SparkConf]] object specifying Spark parameters
    +   * @param classLoader an isolated classloader needed if isolationOn for [[HiveClient]]
    +   *                    construction
    +   * @param hadoopConf a hadoop [[Configuration]] object, Optional if we want generated it from
    +   *                   the sparkConf
    +   * @param extraTimeConfs time configurations in the form of long values from the given hadoopConf
    +   */
    +
    +  private[hive] def newHiveConfigurations(
    +      sparkConf: SparkConf = new SparkConf(loadDefaults = true),
    +      classLoader: ClassLoader = null)(
    +      hadoopConf: Configuration = SparkHadoopUtil.get.newConfiguration(sparkConf))(
    +      extraTimeConfs: Map[String, String] = formatTimeVarsForHiveClient(hadoopConf)): HiveConf = {
    --- End diff --
    
    How about we remove these default values and explicitly specify them in https://github.com/apache/spark/pull/19068/files#diff-f7aac41bf732c1ba1edbac436d331a55R84?


---

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


[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should ...

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

    https://github.com/apache/spark/pull/19068#discussion_r135940059
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala ---
    @@ -81,11 +81,7 @@ private[hive] object SparkSQLCLIDriver extends Logging {
           System.exit(1)
         }
     
    -    val cliConf = new HiveConf(classOf[SessionState])
    -    // Override the location of the metastore since this is only used for local execution.
    -    HiveUtils.newTemporaryConfiguration(useInMemoryDerby = false).foreach {
    --- End diff --
    
    METASTORECONNECTURLKEY connects derby by default, we may set this key in hive-site.xml to talk with metastore. hiveclient will use the state init here, I don't think we should beinit hive conf as the old way.


---
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 #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should ...

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

    https://github.com/apache/spark/pull/19068#discussion_r135494418
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala ---
    @@ -395,7 +395,7 @@ private[spark] object HiveUtils extends Logging {
             propMap.put(confvar.varname, confvar.getDefaultExpr())
           }
         }
    -    propMap.put(WAREHOUSE_PATH.key, localMetastore.toURI.toString)
    +    if (useInMemoryDerby) propMap.put(WAREHOUSE_PATH.key, localMetastore.toURI.toString)
    --- End diff --
    
    when is `useInMemoryDerby` false?


---
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 #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState shoul...

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

    https://github.com/apache/spark/pull/19068#discussion_r138557791
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
    @@ -132,43 +134,26 @@ private[hive] class HiveClientImpl(
           // in hive jars, which will turn off isolation, if SessionSate.detachSession is
           // called to remove the current state after that, hive client created later will initialize
           // its own state by newState()
    -      Option(SessionState.get).getOrElse(newState())
    +      val ret = SessionState.get
    +      if (ret != null) {
    +        Option(hadoopConf.get(ConfVars.METASTOREWAREHOUSE.varname)).foreach { dir =>
    --- End diff --
    
    ok, soon


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

    https://github.com/apache/spark/pull/19068
  
    Merged build finished. Test FAILed.


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    ok to test


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should respect...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    cc @cloud-fan @jiangxb1987 


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

    https://github.com/apache/spark/pull/19068
  
    **[Test build #81718 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81718/testReport)** for PR 19068 at commit [`267a1b2`](https://github.com/apache/spark/commit/267a1b2f5bb83b4f20810f704105c0d996b71e93).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    LGTM, merging to master!


---

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


[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should ...

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

    https://github.com/apache/spark/pull/19068#discussion_r135676677
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala ---
    @@ -81,11 +81,7 @@ private[hive] object SparkSQLCLIDriver extends Logging {
           System.exit(1)
         }
     
    -    val cliConf = new HiveConf(classOf[SessionState])
    -    // Override the location of the metastore since this is only used for local execution.
    -    HiveUtils.newTemporaryConfiguration(useInMemoryDerby = false).foreach {
    --- End diff --
    
    before #SPARK-21428, cliSessionState will be left behind; now we will reuse it. this func is used to organize execution hive configs, but now cliSessionState has to talk with metastore.


---
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 issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    can you run `./build/sbt "hive/test"` locally to make sure all hive tests pass?


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

    https://github.com/apache/spark/pull/19068
  
    **[Test build #81916 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81916/testReport)** for PR 19068 at commit [`c5c1c26`](https://github.com/apache/spark/commit/c5c1c2625d33dd08cbdde2e25041359bbbf50339).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState shoul...

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

    https://github.com/apache/spark/pull/19068#discussion_r138625628
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala ---
    @@ -232,6 +232,54 @@ private[spark] object HiveUtils extends Logging {
       }
     
       /**
    +   * Generate an instance of [[HiveConf]] from [[SparkConf]]& hadoop [[Configuration]] &
    +   * formatted extra time configurations with an isolated classloader needed if isolationOn
    +   * for [[HiveClient]] construction
    +   * @param sparkConf a [[SparkConf]] object specifying Spark parameters
    +   * @param classLoader an isolated classloader needed if isolationOn for [[HiveClient]]
    +   *                    construction
    +   * @param hadoopConf a hadoop [[Configuration]] object, Optional if we want generated it from
    +   *                   the sparkConf
    +   * @param extraTimeConfs time configurations in the form of long values from the given hadoopConf
    +   */
    +
    +  private[hive] def newHiveConfigurations(
    +      sparkConf: SparkConf = new SparkConf(loadDefaults = true),
    +      classLoader: ClassLoader = null)(
    +      hadoopConf: Configuration = SparkHadoopUtil.get.newConfiguration(sparkConf))(
    +      extraTimeConfs: Map[String, String] = formatTimeVarsForHiveClient(hadoopConf)): HiveConf = {
    --- End diff --
    
    OK



---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

Posted by yaooqinn <gi...@git.apache.org>.
Github user yaooqinn commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    @cloud-fan  The cliSessionState is meant to be reused but discarded for isolated hive client classloader couldn't get it through `SessionState.get()`, so hive client will generated a `SessionState` instance everytime while calling `HiveClient.newSession()`. In my foregoing pr, I made it reused but i didn't notice that it is just pointing to a dummy metatstore. This has to be fixed.


---

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


[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState shoul...

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

    https://github.com/apache/spark/pull/19068#discussion_r138556873
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala ---
    @@ -231,6 +231,42 @@ private[spark] object HiveUtils extends Logging {
         }.toMap
       }
     
    +  private[hive] def newHiveConfigurations(
    --- End diff --
    
    ok


---

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


[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState shoul...

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

    https://github.com/apache/spark/pull/19068#discussion_r138615099
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala ---
    @@ -232,6 +232,54 @@ private[spark] object HiveUtils extends Logging {
       }
     
       /**
    +   * Generate an instance of [[HiveConf]] from [[SparkConf]]& hadoop [[Configuration]] &
    +   * formatted extra time configurations with an isolated classloader needed if isolationOn
    +   * for [[HiveClient]] construction
    +   * @param sparkConf a [[SparkConf]] object specifying Spark parameters
    +   * @param classLoader an isolated classloader needed if isolationOn for [[HiveClient]]
    +   *                    construction
    +   * @param hadoopConf a hadoop [[Configuration]] object, Optional if we want generated it from
    +   *                   the sparkConf
    +   * @param extraTimeConfs time configurations in the form of long values from the given hadoopConf
    --- End diff --
    
    it's not only time configs, I think we'd better call it `config`, following `IsolatedClientLoader.config`


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

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


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

    https://github.com/apache/spark/pull/19068
  
    **[Test build #81721 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81721/testReport)** for PR 19068 at commit [`9682eab`](https://github.com/apache/spark/commit/9682eabd4184340745e54b9eef8ac878ca942ba3).


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

Posted by jinxing64 <gi...@git.apache.org>.
Github user jinxing64 commented on the issue:

    https://github.com/apache/spark/pull/19068
  
    @yaooqinn 
    This change works well for me, thanks for fix !
    After this change, hive client for execution(points to a dummy local metastore) will never be used when running sql in`spark-sql`, hive client points a true metastore. Right ?
    So why it is designed to have the hive client in `SparkSQLCLIDriver` points to a dummy local metastore before ? Is this change breaking some design?


---

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


[GitHub] spark issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

    https://github.com/apache/spark/pull/19068
  
    **[Test build #81910 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81910/testReport)** for PR 19068 at commit [`c5c1c26`](https://github.com/apache/spark/commit/c5c1c2625d33dd08cbdde2e25041359bbbf50339).
     * This patch **fails due to an unknown error code, -9**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should ...

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

    https://github.com/apache/spark/pull/19068#discussion_r135580204
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala ---
    @@ -81,11 +81,7 @@ private[hive] object SparkSQLCLIDriver extends Logging {
           System.exit(1)
         }
     
    -    val cliConf = new HiveConf(classOf[SessionState])
    -    // Override the location of the metastore since this is only used for local execution.
    -    HiveUtils.newTemporaryConfiguration(useInMemoryDerby = false).foreach {
    --- End diff --
    
    This function not only constructs the warehouse path, it also create many other configs, why is it safe to just ignore em? 


---
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 #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should ...

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

    https://github.com/apache/spark/pull/19068#discussion_r135536738
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala ---
    @@ -395,7 +395,7 @@ private[spark] object HiveUtils extends Logging {
             propMap.put(confvar.varname, confvar.getDefaultExpr())
           }
         }
    -    propMap.put(WAREHOUSE_PATH.key, localMetastore.toURI.toString)
    +    if (useInMemoryDerby) propMap.put(WAREHOUSE_PATH.key, localMetastore.toURI.toString)
    --- End diff --
    
    @cloud-fan updated. i notice that hiveConf initialized by executionHive way does not load hive-site.xml, so i changed it to meta Hive way


---
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 issue #19068: [SPARK-21428][SQL][FOLLOWUP]CliSessionState should point...

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

    https://github.com/apache/spark/pull/19068
  
    **[Test build #81906 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81906/testReport)** for PR 19068 at commit [`b21fc72`](https://github.com/apache/spark/commit/b21fc7222c313eb570f788afb5b02a9a67c881a4).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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