Skip to content

Conversation

@sunchao
Copy link
Member

@sunchao sunchao commented May 4, 2021

What changes were proposed in this pull request?

Set IS_TESTING to true in BenchmarkBase, before running benchmarks.

Why are the changes needed?

Currently benchmark can be done via 2 ways: spark-submit, or SBT command. However in the former Spark will miss some properties such as IS_TESTING, which is necessary to turn on/off certain behavior like codegen (spark.sql.codegen.factoryMode). Therefore, the result could differ between the two. In addition, the benchmark GitHub workflow is using the spark-submit approach.

Does this PR introduce any user-facing change?

No

How was this patch tested?

N/A

@github-actions github-actions bot added the CORE label May 4, 2021
@sunchao
Copy link
Member Author

sunchao commented May 4, 2021

cc @dongjoon-hyun @viirya @HyukjinKwon found this while running benchmark through Github actions.

Copy link
Member

@viirya viirya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense to me.

@SparkQA
Copy link

SparkQA commented May 5, 2021

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

@SparkQA
Copy link

SparkQA commented May 5, 2021

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

@SparkQA
Copy link

SparkQA commented May 5, 2021

Test build #138143 has finished for PR 32440 at commit f7680a4.

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

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
cc @HyukjinKwon

@wangyum wangyum closed this in 4fe4b65 May 5, 2021
@wangyum
Copy link
Member

wangyum commented May 5, 2021

Merged to master.

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