You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by scwf <gi...@git.apache.org> on 2014/10/06 12:24:43 UTC
[GitHub] spark pull request: [SPARK-3809][SQL]fix HiveThriftServer2Suite to...
GitHub user scwf opened a pull request:
https://github.com/apache/spark/pull/2671
[SPARK-3809][SQL]fix HiveThriftServer2Suite to make it work correctly
Currently HiveThriftServer2Suite is a fake one, actually HiveThriftServer not even started there. Issues here:
1 Thriftserver not started. Testing will get this error ---
ERROR HiveThriftServer2Suite: Failed to start Hive Thrift server within 30 seconds
java.util.concurrent.TimeoutException: Futures timed out after [30 seconds]
2 Thriftserver not stoped. After test finished the process of thriftserver did not exit.
This patch fix this problems as follows:
1 Since thriftserver started as a daemon in https://github.com/apache/spark/pull/2509, output of the```start-thriftserver.sh``` has be redirect to a log file such as ```spark-kf-org.apache.spark.sql.hive.thriftserver.HiveThriftServer2-1-kf.out```, so to see whether this file contain "ThriftBinaryCLIService listening on" ` to assert server started successfully
2 Start server in ```beforeAll```
3 stop server in ```afterAll```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/scwf/spark fix-HiveThriftServer2Suite
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/2671.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 #2671
----
commit c39d0a5eabc1f505117e711c17a48b089b266483
Author: scwf <wa...@huawei.com>
Date: 2014-10-06T09:45:41Z
fix HiveThriftServer2Suite
commit 0081a508f147a2b7bd7065149b8d3da308ba3d37
Author: scwf <wa...@huawei.com>
Date: 2014-10-06T09:51:14Z
fix code format
----
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by scwf <gi...@git.apache.org>.
Github user scwf commented on the pull request:
https://github.com/apache/spark/pull/2671#issuecomment-58045037
Ok, since now the process output redirect to log file, we can also check with it to see where is the problem
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2671#issuecomment-58048497
[QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/273/consoleFull) for PR 2671 at commit [`48979f6`](https://github.com/apache/spark/commit/48979f6e6d3384b8f4b900d700d11db79e18284a).
* 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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/2671#issuecomment-57999047
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: [SPARK-3809][SQL]fix HiveThriftServer2Suite to...
Posted by scwf <gi...@git.apache.org>.
Github user scwf commented on the pull request:
https://github.com/apache/spark/pull/2671#issuecomment-58354129
we will fix it in #2675
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2671#issuecomment-58054018
[QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/273/consoleFull) for PR 2671 at commit [`48979f6`](https://github.com/apache/spark/commit/48979f6e6d3384b8f4b900d700d11db79e18284a).
* This patch **fails** unit tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` case e: Exception => logError("Source class " + classPath + " cannot be instantiated", e)`
* `case class CacheTableCommand(tableName: String, plan: Option[LogicalPlan], isLazy: Boolean)`
* `case class UncacheTableCommand(tableName: String) extends Command`
* `case class CacheTableCommand(`
* `case class UncacheTableCommand(tableName: String) extends LeafNode with Command `
* `case class DescribeCommand(child: SparkPlan, output: Seq[Attribute])(`
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/2671#issuecomment-58020543
ok to test
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2671#issuecomment-58029079
[QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/271/consoleFull) for PR 2671 at commit [`0081a50`](https://github.com/apache/spark/commit/0081a508f147a2b7bd7065149b8d3da308ba3d37).
* This patch **passes** unit tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/2671#discussion_r18459283
--- Diff: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suite.scala ---
@@ -123,14 +128,45 @@ class HiveThriftServer2Suite extends FunSuite with Logging {
|=========================================
""".stripMargin, cause)
} finally {
- warehousePath.delete()
- metastorePath.delete()
process.destroy()
}
}
+ override def afterAll() {
+ warehousePath.delete()
+ metastorePath.delete()
+ stopThriftserver
+ }
+
+ def stopThriftserver: Unit = {
+ val stopScript = "../../sbin/stop-thriftserver.sh".split("/").mkString(File.separator)
+ val builder = new ProcessBuilder(stopScript)
+ val process = builder.start()
+ new Thread("read stderr") {
+ override def run() {
+ for (line <- Source.fromInputStream(process.getErrorStream).getLines()) {
+ System.err.println(line)
+ }
+ }
+ }.start()
+ val output = new StringBuffer
+ val stdoutThread = new Thread("read stdout") {
+ override def run() {
+ for (line <- Source.fromInputStream(process.getInputStream).getLines()) {
+ output.append(line)
--- End diff --
`output` is never used. Maybe you intended to print it?
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by scwf <gi...@git.apache.org>.
Github user scwf commented on the pull request:
https://github.com/apache/spark/pull/2671#issuecomment-58003250
cc @liancheng
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/2671#issuecomment-58048461
Actually #2214 fixes several issues, you need all of them to make HiveThriftServer2Suite healthy. Please refer to the PR description 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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by scwf <gi...@git.apache.org>.
Github user scwf commented on the pull request:
https://github.com/apache/spark/pull/2671#issuecomment-58046480
@liancheng, can you retest this, i add some print to diagnose this
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/2671#discussion_r18459115
--- Diff: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suite.scala ---
@@ -70,37 +73,39 @@ class HiveThriftServer2Suite extends FunSuite with Logging {
val serverStarted = Promise[Unit]()
val buffer = new ArrayBuffer[String]()
+ val startString =
+ "starting org.apache.spark.sql.hive.thriftserver.HiveThriftServer2, logging to "
+ val maxTries = 30
def captureOutput(source: String)(line: String) {
buffer += s"$source> $line"
- if (line.contains("ThriftBinaryCLIService listening on")) {
- serverStarted.success(())
+ if (line.contains(startString)) {
+ val logFile = new File(line.substring(startString.length))
+ var tryNum = 0
+ // This is a hack to wait logFile ready
+ Thread.sleep(5000)
+ // logFile may have not finished, try every second
+ while (!logFile.exists() || (!fileToString(logFile).contains(
+ "ThriftBinaryCLIService listening on") && tryNum < maxTries)) {
+ Thread.sleep(1000)
+ }
+ if (fileToString(logFile).contains("ThriftBinaryCLIService listening on")) {
+ serverStarted.success(())
+ } else {
+ throw new TimeoutException()
+ }
}
}
-
val process = Process(command).run(
ProcessLogger(captureOutput("stdout"), captureOutput("stderr")))
Future {
val exitValue = process.exitValue()
- logInfo(s"Spark SQL Thrift server process exit value: $exitValue")
+ logInfo(s"Start Spark SQL Thrift server process exit value: $exitValue")
--- End diff --
Why "Start" here? When this line is executed, the server process has already ended.
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/2671#discussion_r18458935
--- Diff: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suite.scala ---
@@ -70,37 +73,39 @@ class HiveThriftServer2Suite extends FunSuite with Logging {
val serverStarted = Promise[Unit]()
val buffer = new ArrayBuffer[String]()
+ val startString =
+ "starting org.apache.spark.sql.hive.thriftserver.HiveThriftServer2, logging to "
+ val maxTries = 30
def captureOutput(source: String)(line: String) {
buffer += s"$source> $line"
- if (line.contains("ThriftBinaryCLIService listening on")) {
- serverStarted.success(())
+ if (line.contains(startString)) {
+ val logFile = new File(line.substring(startString.length))
+ var tryNum = 0
+ // This is a hack to wait logFile ready
+ Thread.sleep(5000)
+ // logFile may have not finished, try every second
+ while (!logFile.exists() || (!fileToString(logFile).contains(
+ "ThriftBinaryCLIService listening on") && tryNum < maxTries)) {
--- End diff --
`tryNum` is never increased.
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by scwf <gi...@git.apache.org>.
Github user scwf commented on the pull request:
https://github.com/apache/spark/pull/2671#issuecomment-58036125
Get it, i will check this
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/2671#discussion_r18459638
--- Diff: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suite.scala ---
@@ -123,14 +128,45 @@ class HiveThriftServer2Suite extends FunSuite with Logging {
|=========================================
""".stripMargin, cause)
} finally {
- warehousePath.delete()
- metastorePath.delete()
process.destroy()
}
}
+ override def afterAll() {
+ warehousePath.delete()
+ metastorePath.delete()
+ stopThriftserver
+ }
+
+ def stopThriftserver: Unit = {
+ val stopScript = "../../sbin/stop-thriftserver.sh".split("/").mkString(File.separator)
+ val builder = new ProcessBuilder(stopScript)
--- End diff --
Using Scala process API can be much simpler :)
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/2671#issuecomment-58033388
BTW, Jenkins passes because the exception re-thrown issue is not fixed in your PR :) You may check the full console output for sure https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/271/consoleFull
And mine still suffers the mysterious timeout. Keep digging...
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2671#issuecomment-58021025
[QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/271/consoleFull) for PR 2671 at commit [`0081a50`](https://github.com/apache/spark/commit/0081a508f147a2b7bd7065149b8d3da308ba3d37).
* 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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/2671#issuecomment-58044200
When trying #2214 several weeks ago, the Thrift server process simply couldn't start on Jenkins server, but everything's fine on my local machine.
However, the pull request builder had been refactored a lot by Josh. It seems that #2675 fails simply because my timeout was too tight for Jenkins. I'm trying to relax the timeout a bit.
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by scwf <gi...@git.apache.org>.
Github user scwf commented on the pull request:
https://github.com/apache/spark/pull/2671#issuecomment-58043321
It is very strange in my local maching it is ok. Hi @liancheng, can you get the log file in "stdout> starting org.apache.spark.sql.hive.thriftserver.HiveThriftServer2, logging to /home/jenkins/workspace/NewSparkPullRequestBuilder/sbin/../logs/spark-root-org.apache.spark.sql.hive.thriftserver.HiveThriftServer2-1-test02.amplab.out"
I think may be there is a already exist thriftserver process that leads server start failed on Jenkins, we need the log file to check.
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by scwf <gi...@git.apache.org>.
Github user scwf closed the pull request at:
https://github.com/apache/spark/pull/2671
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by scwf <gi...@git.apache.org>.
Github user scwf commented on the pull request:
https://github.com/apache/spark/pull/2671#issuecomment-58029661
Hi, @liancheng, thanks for you comments that is very useful.
1 Actually i have tried start a server for a every test case but the second one failed due to "org.apache.spark.sql.hive.thriftserver.HiveThriftServer2 running as process 82266. Stop it first." Now i get the reason from your patch "A Thread.sleep has to be introduced because the kill command used in stop-thriftserver.sh is not synchronous."
2 Use a tail process is ok, actually i think about it. Since the log file won't be a big file, so here i mike to use ```fileToString```
3 Yeah, here should remove log file
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/2671#issuecomment-58022609
@scwf Thanks, this is a good catch. However, I should mention that HiveThriftServer2Suite is known to be flaky before the Thrift server is made a daemon. I had opened #2214 to try to fix this issue, but unfortunately Jenkins fails because of unknown reason that I couldn't reproduce locally. After numerous unsuccessful tries, I haven't got time to get it done yet. Sorry for the trouble... The essential issue fixed in #2214 is that the exception caught in HiveThriftServer2Suite is not re-thrown in the `catch` clause. That's why it always passes no matter what exception is thrown.
Back to this PR, I have several comments:
1. Personally I don't prefer to start/stop the server process in `beforeAll`/`afterAll`. I'd like to make sure every test is executed against a Thrift server process with clean states.
1. The sleeps introduced in this PR can be eliminated by starting a `tail` process to watch the log file, and then monitor the output of the `tail` process. Since an empty log file does no harm, we can try to create a new empty log file to ensure the file exists before executing `tail`.
1. Log file should be removed after stopping the server process.
Since the Jenkins failure issue in #2214 is really tricky to fix and without fixing that, we couldn't make any change to `HiveThriftServer2Suite`, I'm going to open new PR to have another try to fix this issue together with those left in #2214.
---
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-3809][SQL]fix HiveThriftServer2Suite to...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/2671#issuecomment-58033821
The key point to use `tail` is to eliminate the `sleep` call rather than avoid `fileToString`.
---
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