[SPARK-25339][TEST] Refactor FilterPushdownBenchmark #22443

wangyum · 2018-09-17T17:08:05Z

What changes were proposed in this pull request?

Refactor FilterPushdownBenchmark use main method. we can use 3 ways to run this test now:

bin/spark-submit --class org.apache.spark.sql.execution.benchmark.FilterPushdownBenchmark spark-sql_2.11-2.5.0-SNAPSHOT-tests.jar
build/sbt "sql/test:runMain org.apache.spark.sql.execution.benchmark.FilterPushdownBenchmark"
SPARK_GENERATE_BENCHMARK_FILES=1 build/sbt "sql/test:runMain org.apache.spark.sql.execution.benchmark.FilterPushdownBenchmark"

The method 2 and the method 3 do not need to compile the spark-sql_*-tests.jar package. So these two methods are mainly for developers to quickly do benchmark.

How was this patch tested?

manual tests

wangyum · 2018-09-17T17:08:30Z

cc @dongjoon-hyun

dongjoon-hyun

Thank you for your swift PR, @wangyum ! I'll take a look tonight. BTW, could you update the title [TESTS] into [TEST] because [TEST] is simpler and more popular from the Spark log?

dongjoon-hyun · 2018-09-17T18:52:01Z

2 and 3 are possible and convenient as you mentioned. But, we are trying to avoid 2 and 3, aren't we?

2. build/sbt "sql/test:runMain org.apache.spark.sql.execution.benchmark.FilterPushdownBenchmark"
3. SPARK_GENERATE_BENCHMARK_FILES=1 build/sbt "sql/test:runMain org.apache.spark.sql.execution.benchmark.FilterPushdownBenchmark"

Please correct me if I'm wrong, @cloud-fan .

SparkQA · 2018-09-17T21:43:11Z

Test build #96148 has finished for PR 22443 at commit 6e7cfc8.

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

cloud-fan · 2018-09-18T04:06:18Z

sql/core/src/test/scala/org/apache/spark/sql/execution/benchmark/FilterPushdownBenchmark.scala

+ * To run this benchmark:
+ *  1. without sbt: bin/spark-submit --class <this class> <spark sql test jar>
+ *  2. build/sbt "sql/test:runMain <this class>"
+ *  3. generate result: SPARK_GENERATE_BENCHMARK_FILES=1 build/sbt "sql/test:runMain <this class>"


shall we print the benchmark result if SPARK_GENERATE_BENCHMARK_FILES is not set?

IIRC there is a special OutputStream that can print the output to console.

Yes, It can print the output to console if SPARK_GENERATE_BENCHMARK_FILES not set.

spark/core/src/main/scala/org/apache/spark/util/Benchmark.scala

Lines 59 to 63 in 4e8ac6e

val out = if (output.isDefined) {

new PrintStream(new TeeOutputStream(System.out, output.get))

} else {

System.out

}

nit: remove the one space indent before the numbered list

cloud-fan · 2018-09-18T07:17:55Z

@dongjoon-hyun we are trying to avoid the overhead of scalatest, not sbt. So this LGTM

dongjoon-hyun · 2018-09-18T09:47:55Z

Thank you for confirmation, @cloud-fan .

SparkQA · 2018-09-19T04:50:12Z

Test build #96213 has finished for PR 22443 at commit 075ef7a.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

wangyum · 2018-09-19T05:00:08Z

retest this please

SparkQA · 2018-09-19T07:05:02Z

Test build #96223 has finished for PR 22443 at commit 075ef7a.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

wangyum · 2018-09-19T07:09:20Z

Jenkins, retest this please.

SparkQA · 2018-09-19T12:06:08Z

Test build #96234 has finished for PR 22443 at commit 075ef7a.

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

cloud-fan · 2018-09-20T04:35:41Z

thanks, merging to master!

@dongjoon-hyun Can you create an umbrella JIRA for updating all the benchmark and take care of it? Thanks!

dongjoon-hyun · 2018-09-20T04:39:10Z

I see, @cloud-fan .

dongjoon-hyun · 2018-09-20T05:24:51Z

https://issues.apache.org/jira/browse/SPARK-25475 is created.

dongjoon-hyun · 2018-09-20T05:30:33Z

@gengliangwang . SPARK-25475 is created like the above, could you revise #22451 in order print the output as a separate file like this PR?

gengliangwang · 2018-09-20T06:09:14Z

@dongjoon-hyun No problem. I was waiting for this PR to be merged.

dongjoon-hyun · 2018-09-20T06:19:05Z

Thank you, @gengliangwang !

HyukjinKwon · 2018-09-28T08:43:45Z

core/src/main/scala/org/apache/spark/util/BenchmarkBase.scala

+      if (!file.exists()) {
+        file.createNewFile()
+      }
+      output = Some(new FileOutputStream(file))


nit: try-catch and close it.

HyukjinKwon · 2018-09-28T08:44:23Z

core/src/main/scala/org/apache/spark/util/BenchmarkBase.scala

+      if (!file.exists()) {
+        file.createNewFile()
+      }
+      output = Some(new FileOutputStream(file))


nit: Option(new FileOutputStream(file)) and remove if (o != null) { below.

Please address them when you fix some codes around here next time.

Thanks @HyukjinKwon. Will fix it next time.

wangyum added 2 commits September 18, 2018 00:51

Refactor FilterPushdownBenchmark

7b50eb5

Revert numRows

6e7cfc8

dongjoon-hyun reviewed Sep 17, 2018

View reviewed changes

wangyum changed the title ~~[SPARK-25339][TESTS] Refactor FilterPushdownBenchmark~~ [SPARK-25339][TEST] Refactor FilterPushdownBenchmark Sep 18, 2018

cloud-fan reviewed Sep 18, 2018

View reviewed changes

dongjoon-hyun mentioned this pull request Sep 18, 2018

[SPARK-24777][SQL] Add write benchmark for AVRO #22451

Closed

Remove the one space indent before the numbered list

075ef7a

asfgit closed this in 0e31a6f Sep 20, 2018

HyukjinKwon reviewed Sep 28, 2018

View reviewed changes

wangyum mentioned this pull request Oct 6, 2018

[SPARK-25492][TEST] Refactor WideSchemaBenchmark to use main method #22501

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-25339][TEST] Refactor FilterPushdownBenchmark #22443

[SPARK-25339][TEST] Refactor FilterPushdownBenchmark #22443

wangyum commented Sep 17, 2018

wangyum commented Sep 17, 2018

dongjoon-hyun left a comment

dongjoon-hyun commented Sep 17, 2018

SparkQA commented Sep 17, 2018

cloud-fan Sep 18, 2018

cloud-fan Sep 18, 2018

wangyum Sep 18, 2018

gengliangwang Sep 18, 2018

wangyum Sep 19, 2018

cloud-fan commented Sep 18, 2018

dongjoon-hyun commented Sep 18, 2018

SparkQA commented Sep 19, 2018

wangyum commented Sep 19, 2018

SparkQA commented Sep 19, 2018

wangyum commented Sep 19, 2018

SparkQA commented Sep 19, 2018

cloud-fan commented Sep 20, 2018

dongjoon-hyun commented Sep 20, 2018

dongjoon-hyun commented Sep 20, 2018

dongjoon-hyun commented Sep 20, 2018

gengliangwang commented Sep 20, 2018

dongjoon-hyun commented Sep 20, 2018

HyukjinKwon Sep 28, 2018

HyukjinKwon Sep 28, 2018

HyukjinKwon Sep 28, 2018 •

edited

wangyum Sep 28, 2018 •

edited

	val out = if (output.isDefined) {
	new PrintStream(new TeeOutputStream(System.out, output.get))
	} else {
	System.out
	}

[SPARK-25339][TEST] Refactor FilterPushdownBenchmark #22443

[SPARK-25339][TEST] Refactor FilterPushdownBenchmark #22443

Conversation

wangyum commented Sep 17, 2018

What changes were proposed in this pull request?

How was this patch tested?

wangyum commented Sep 17, 2018

dongjoon-hyun left a comment

Choose a reason for hiding this comment

dongjoon-hyun commented Sep 17, 2018

SparkQA commented Sep 17, 2018

cloud-fan Sep 18, 2018

Choose a reason for hiding this comment

cloud-fan Sep 18, 2018

Choose a reason for hiding this comment

wangyum Sep 18, 2018

Choose a reason for hiding this comment

gengliangwang Sep 18, 2018

Choose a reason for hiding this comment

wangyum Sep 19, 2018

Choose a reason for hiding this comment

cloud-fan commented Sep 18, 2018

dongjoon-hyun commented Sep 18, 2018

SparkQA commented Sep 19, 2018

wangyum commented Sep 19, 2018

SparkQA commented Sep 19, 2018

wangyum commented Sep 19, 2018

SparkQA commented Sep 19, 2018

cloud-fan commented Sep 20, 2018

dongjoon-hyun commented Sep 20, 2018

dongjoon-hyun commented Sep 20, 2018

dongjoon-hyun commented Sep 20, 2018

gengliangwang commented Sep 20, 2018

dongjoon-hyun commented Sep 20, 2018

HyukjinKwon Sep 28, 2018

Choose a reason for hiding this comment

HyukjinKwon Sep 28, 2018

Choose a reason for hiding this comment

HyukjinKwon Sep 28, 2018 • edited

Choose a reason for hiding this comment

wangyum Sep 28, 2018 • edited

Choose a reason for hiding this comment

HyukjinKwon Sep 28, 2018 •

edited

wangyum Sep 28, 2018 •

edited