Skip to content

Conversation

@sarutak
Copy link
Member

@sarutak sarutak commented Aug 14, 2021

What changes were proposed in this pull request?

This PR fixes an issue that UISeleniumSuite in sql/hive-thriftserver doesn't work even though the ignored test is enabled due to the following two reasons.

(1) The suite waits for thriftserver starting up by reading a startup message from stdin but the expected message is never read.
(2) The URL and CSS selector for test are wrong.

To resolve (1), this PR adopt the way that HiveThriftServer2Suite does.

Files.write(
"""log4j.rootCategory=INFO, console
|log4j.appender.console=org.apache.log4j.ConsoleAppender
|log4j.appender.console.target=System.err
|log4j.appender.console.layout=org.apache.log4j.PatternLayout
|log4j.appender.console.layout.ConversionPattern=%d{yy/MM/dd HH:mm:ss} %p %c{1}: %m%n
""".stripMargin,
new File(s"$tempLog4jConf/log4j.properties"),
StandardCharsets.UTF_8)
tempLog4jConf
}
s"""$startScript
| --master local
| --hiveconf ${ConfVars.METASTORECONNECTURLKEY}=$metastoreJdbcUri
| --hiveconf ${ConfVars.METASTOREWAREHOUSE}=$warehousePath
| --hiveconf ${ConfVars.HIVE_SERVER2_THRIFT_BIND_HOST}=localhost
| --hiveconf ${ConfVars.HIVE_SERVER2_TRANSPORT_MODE}=$mode
| --hiveconf ${ConfVars.HIVE_SERVER2_LOGGING_OPERATION_LOG_LOCATION}=$operationLogPath
| --hiveconf ${ConfVars.LOCALSCRATCHDIR}=$lScratchDir
| --hiveconf $portConf=0
| --driver-class-path $driverClassPath
| --driver-java-options -Dlog4j.debug
| --conf spark.ui.enabled=false
| ${extraConf.mkString("\n")}
""".stripMargin.split("\\s+").toSeq

This PR also enables the ignored test. Let's disable it again in the following PR if it's still flaky.

Why are the changes needed?

Bug fix.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

CIs.

@github-actions github-actions bot added the SQL label Aug 14, 2021
@SparkQA
Copy link

SparkQA commented Aug 14, 2021

Test build #142465 has finished for PR 33741 at commit 78bed9f.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Aug 14, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46972/

@SparkQA
Copy link

SparkQA commented Aug 14, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46972/

@codecov-commenter

This comment has been minimized.

@HyukjinKwon
Copy link
Member

cc @gengliangwang FYI

@sarutak
Copy link
Member Author

sarutak commented Aug 18, 2021

cc: @dongjoon-hyun too.

@gengliangwang
Copy link
Member

@sarutak sorry for the late reply. It seems that the test is ignored. Shall we enable it in this PR?

@sarutak
Copy link
Member Author

sarutak commented Aug 18, 2021

@gengliangwang Thank you for the reply. The reason the test was ignored seems flakiness (#6345).
But it's 6 years ago and now it's difficult to know how flaky the test was.
How about enabling the ignored test and disable again if it's still flaky?

@gengliangwang
Copy link
Member

If we are not going to enable it, it doesn't make sense to fix it...
Let's see how the CI test behaves. I tried on my local setup and the test passes.

@SparkQA
Copy link

SparkQA commented Aug 18, 2021

Test build #142618 has finished for PR 33741 at commit 7ffac06.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Aug 18, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47119/

@SparkQA
Copy link

SparkQA commented Aug 18, 2021

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47119/

@AmplabJenkins
Copy link

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/47119/

@gengliangwang
Copy link
Member

Thanks, merging to master

sarutak added a commit that referenced this pull request Sep 2, 2021
…ation in UI by config

### What changes were proposed in this pull request?

This PR adds a test for SPARK-36400 (#33743).

### Why are the changes needed?

SPARK-36512 (#33741) was fixed so we can add this test now.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

New test.

Closes #33885 from sarutak/add-reduction-test.

Authored-by: Kousuke Saruta <sarutak@oss.nttdata.com>
Signed-off-by: Kousuke Saruta <sarutak@oss.nttdata.com>
asiunov pushed a commit to ascend-io/spark that referenced this pull request Aug 25, 2022
### What changes were proposed in this pull request?

This PR fixes an issue that `UISeleniumSuite` in `sql/hive-thriftserver` doesn't work even though the ignored test is enabled due to the following two reasons.

(1) The suite waits for thriftserver starting up by reading a startup message from stdin but the expected message is never read.
(2) The URL and CSS selector for test are wrong.

To resolve (1), this PR adopt the way that `HiveThriftServer2Suite` does.
https://github.com/apache/spark/blob/3f8ec0dae4ddfd7ee55370dad5d44d03a9f10387/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suites.scala#L1222-L1248

This PR also enables the ignored test. Let's disable it again in the following PR if it's still flaky.

### Why are the changes needed?

Bug fix.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

CIs.

Closes apache#33741 from sarutak/fix-thrift-uiseleniumsuite.

Authored-by: Kousuke Saruta <sarutak@oss.nttdata.com>
Signed-off-by: Gengliang Wang <gengliang@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants