You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "ychris78 (via GitHub)" <gi...@apache.org> on 2026/01/30 09:23:44 UTC
[PR] Allow specifying the executionMode when executing the tpcds benchmark [kyuubi]
ychris78 opened a new pull request, #7318:
URL: https://github.com/apache/kyuubi/pull/7318
<!--
Thanks for sending a pull request!
Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/master/contributing/code/index.html
2. If the PR is related to an issue in https://github.com/apache/kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug, and what versions are affected.
-->
The underlying system supports specifying the executionMode when running the TPCDS benchmark, but this option is not exposed to users through parameters.
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
Manual test
### Was this patch authored or co-authored using generative AI tooling?
<!--
If a generative AI tooling has been used in the process of authoring this patch, please include
phrase 'Generated-by: ' followed by the name of the tool and its version.
If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Allow specifying the executionMode when executing the tpcds benchmark [kyuubi]
Posted by "ychris78 (via GitHub)" <gi...@apache.org>.
ychris78 commented on code in PR #7318:
URL: https://github.com/apache/kyuubi/pull/7318#discussion_r2745658267
##########
dev/kyuubi-tpcds/src/main/scala/org/apache/kyuubi/tpcds/benchmark/TPCDS_2_4_Queries.scala:
##########
@@ -132,18 +128,8 @@ trait TPCDS_2_4_Queries extends Benchmark {
"q99",
"ss_max")
- val tpcds2_4Queries: Seq[Query] = queryNames.map { queryName =>
- val in = getClass.getClassLoader.getResourceAsStream(s"tpcds_2_4/$queryName.sql")
- val queryContent: String = Source.fromInputStream(in)(Codec.UTF8).mkString
- in.close()
-
- Query(
- queryName + "-v2.4",
- queryContent,
- description = "TPC-DS 2.4 Query",
- executionMode = CollectResults)
- }
+ val tpcds2_4Queries: Seq[Query]
- val tpcds2_4QueriesMap: Map[String, Query] =
+ lazy val tpcds2_4QueriesMap: Map[String, Query] =
Review Comment:
Unused code, removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Allow specifying the executionMode when executing the tpcds benchmark [kyuubi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #7318:
URL: https://github.com/apache/kyuubi/pull/7318#issuecomment-3823645206
## [Codecov](https://app.codecov.io/gh/apache/kyuubi/pull/7318?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 0.00%. Comparing base ([`dfd9b49`](https://app.codecov.io/gh/apache/kyuubi/commit/dfd9b4935378c79d3c1f99e344273ff71e21765b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`f61162a`](https://app.codecov.io/gh/apache/kyuubi/commit/f61162a4a5cb3f564b976196abe57d9ec1fc5cd1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #7318 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 698 698
Lines 43654 43654
Branches 5896 5896
======================================
Misses 43654 43654
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/kyuubi/pull/7318?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Allow specifying the executionMode when executing the tpcds benchmark [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 closed pull request #7318: Allow specifying the executionMode when executing the tpcds benchmark
URL: https://github.com/apache/kyuubi/pull/7318
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Allow specifying the executionMode when executing the tpcds benchmark [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #7318:
URL: https://github.com/apache/kyuubi/pull/7318#issuecomment-3838593921
thanks, merged to master
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Allow specifying the executionMode when executing the tpcds benchmark [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #7318:
URL: https://github.com/apache/kyuubi/pull/7318#discussion_r2745419543
##########
dev/kyuubi-tpcds/README.md:
##########
@@ -48,15 +48,16 @@ $SPARK_HOME/bin/spark-submit \
Support options:
-| key | default | description |
-|-------------|------------------------|-------------------------------------------------------------------------------|
-| db | none(required) | the TPC-DS database |
-| benchmark | tpcds-v2.4-benchmark | the name of application |
-| iterations | 3 | the number of iterations to run |
-| breakdown | false | whether to record breakdown results of an execution |
-| results-dir | /spark/sql/performance | dir to store benchmark results, e.g. hdfs://hdfs-nn:9870/pref |
-| include | none(optional) | name of the queries to run, use comma to split multiple names, e.g. q1,q2 |
-| exclude | none(optional) | name of the queries to exclude, use comma to split multiple names, e.g. q2,q4 |
+| key | default | description |
+|-------------|------------------------|---------------------------------------------------------------------------------------------------------------------|
+| db | none(required) | the TPC-DS database |
+| benchmark | tpcds-v2.4-benchmark | the name of application |
+| iterations | 3 | the number of iterations to run |
+| breakdown | false | whether to record breakdown results of an execution |
+| results-dir | /spark/sql/performance | dir to store benchmark results, e.g. hdfs://hdfs-nn:9870/pref |
+| include | none(optional) | name of the queries to run, use comma to split multiple names, e.g. q1,q2 |
+| exclude | none(optional) | name of the queries to exclude, use comma to split multiple names, e.g. q2,q4 |
+| execution-mode | collect | how a given Spark benchmark should be run, only the following four modes are supported: collect,foreach,saveToParquet,hash |
Review Comment:
`dev/reformat` may fix this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Allow specifying the executionMode when executing the tpcds benchmark [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #7318:
URL: https://github.com/apache/kyuubi/pull/7318#discussion_r2745418390
##########
dev/kyuubi-tpcds/src/main/scala/org/apache/kyuubi/tpcds/benchmark/TPCDS_2_4_Queries.scala:
##########
@@ -132,18 +128,8 @@ trait TPCDS_2_4_Queries extends Benchmark {
"q99",
"ss_max")
- val tpcds2_4Queries: Seq[Query] = queryNames.map { queryName =>
- val in = getClass.getClassLoader.getResourceAsStream(s"tpcds_2_4/$queryName.sql")
- val queryContent: String = Source.fromInputStream(in)(Codec.UTF8).mkString
- in.close()
-
- Query(
- queryName + "-v2.4",
- queryContent,
- description = "TPC-DS 2.4 Query",
- executionMode = CollectResults)
- }
+ val tpcds2_4Queries: Seq[Query]
- val tpcds2_4QueriesMap: Map[String, Query] =
+ lazy val tpcds2_4QueriesMap: Map[String, Query] =
Review Comment:
what's the benefit, and where is the var used?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Allow specifying the executionMode when executing the tpcds benchmark [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #7318:
URL: https://github.com/apache/kyuubi/pull/7318#discussion_r2745424306
##########
dev/kyuubi-tpcds/src/main/scala/org/apache/kyuubi/tpcds/benchmark/RunBenchmark.scala:
##########
@@ -73,6 +74,9 @@ object RunBenchmark {
c.copy(exclude = x.split(",").map(_.trim).filter(_.nonEmpty).toSet)
}
.text("name of the queries to exclude, use comma to split multiple names, e.g. q2,q4")
+ opt[String]('m', "execution-mode")
Review Comment:
short option is misleading, let's keep only the long version
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Allow specifying the executionMode when executing the tpcds benchmark [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #7318:
URL: https://github.com/apache/kyuubi/pull/7318#discussion_r2745421955
##########
dev/kyuubi-tpcds/src/main/scala/org/apache/kyuubi/tpcds/benchmark/TPCDS.scala:
##########
@@ -17,12 +17,40 @@
package org.apache.kyuubi.tpcds.benchmark
+import scala.io.{Codec, Source}
+
import org.apache.spark.sql.SparkSession
+import org.apache.kyuubi.tpcds.benchmark.ExecutionMode._
+
/**
* TPC-DS benchmark's dataset.
*/
class TPCDS(@transient sparkSession: SparkSession)
extends Benchmark(sparkSession)
with TPCDS_2_4_Queries
- with Serializable {}
+ with Serializable {
+
+ override val tpcds2_4Queries: Seq[Query] = queryNames.map { queryName =>
+ val in = getClass.getClassLoader.getResourceAsStream(s"tpcds_2_4/$queryName.sql")
+ val queryContent: String = Source.fromInputStream(in)(Codec.UTF8).mkString
+ in.close()
Review Comment:
though this is copied, could you please rewrite it with `try ... finally` ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Allow specifying the executionMode when executing the tpcds benchmark [kyuubi]
Posted by "ychris78 (via GitHub)" <gi...@apache.org>.
ychris78 commented on PR #7318:
URL: https://github.com/apache/kyuubi/pull/7318#issuecomment-3838208783
@turboFei Could you help review this PR? thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Allow specifying the executionMode when executing the tpcds benchmark [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #7318:
URL: https://github.com/apache/kyuubi/pull/7318#discussion_r2745789063
##########
dev/kyuubi-tpcds/src/main/scala/org/apache/kyuubi/tpcds/benchmark/TPCDS.scala:
##########
@@ -17,12 +17,44 @@
package org.apache.kyuubi.tpcds.benchmark
+import scala.io.{Codec, Source}
+
import org.apache.spark.sql.SparkSession
+import org.apache.kyuubi.tpcds.benchmark.ExecutionMode._
+
/**
* TPC-DS benchmark's dataset.
*/
class TPCDS(@transient sparkSession: SparkSession)
extends Benchmark(sparkSession)
with TPCDS_2_4_Queries
- with Serializable {}
+ with Serializable {
+
+ override val tpcds2_4Queries: Seq[Query] = queryNames.map { queryName =>
+ val in = getClass.getClassLoader.getResourceAsStream(s"tpcds_2_4/$queryName.sql")
+ var queryContent: String = ""
+ try {
+ queryContent = Source.fromInputStream(in)(Codec.UTF8).mkString
Review Comment:
```suggestion
val queryContent = try {
Source.fromInputStream(in)(Codec.UTF8).mkString
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Allow specifying the executionMode when executing the tpcds benchmark [kyuubi]
Posted by "ychris78 (via GitHub)" <gi...@apache.org>.
ychris78 commented on code in PR #7318:
URL: https://github.com/apache/kyuubi/pull/7318#discussion_r2745659194
##########
dev/kyuubi-tpcds/README.md:
##########
@@ -48,15 +48,16 @@ $SPARK_HOME/bin/spark-submit \
Support options:
-| key | default | description |
-|-------------|------------------------|-------------------------------------------------------------------------------|
-| db | none(required) | the TPC-DS database |
-| benchmark | tpcds-v2.4-benchmark | the name of application |
-| iterations | 3 | the number of iterations to run |
-| breakdown | false | whether to record breakdown results of an execution |
-| results-dir | /spark/sql/performance | dir to store benchmark results, e.g. hdfs://hdfs-nn:9870/pref |
-| include | none(optional) | name of the queries to run, use comma to split multiple names, e.g. q1,q2 |
-| exclude | none(optional) | name of the queries to exclude, use comma to split multiple names, e.g. q2,q4 |
+| key | default | description |
+|-------------|------------------------|---------------------------------------------------------------------------------------------------------------------|
+| db | none(required) | the TPC-DS database |
+| benchmark | tpcds-v2.4-benchmark | the name of application |
+| iterations | 3 | the number of iterations to run |
+| breakdown | false | whether to record breakdown results of an execution |
+| results-dir | /spark/sql/performance | dir to store benchmark results, e.g. hdfs://hdfs-nn:9870/pref |
+| include | none(optional) | name of the queries to run, use comma to split multiple names, e.g. q1,q2 |
+| exclude | none(optional) | name of the queries to exclude, use comma to split multiple names, e.g. q2,q4 |
+| execution-mode | collect | how a given Spark benchmark should be run, only the following four modes are supported: collect,foreach,saveToParquet,hash |
Review Comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Allow specifying the executionMode when executing the tpcds benchmark [kyuubi]
Posted by "ychris78 (via GitHub)" <gi...@apache.org>.
ychris78 commented on code in PR #7318:
URL: https://github.com/apache/kyuubi/pull/7318#discussion_r2745660073
##########
dev/kyuubi-tpcds/src/main/scala/org/apache/kyuubi/tpcds/benchmark/TPCDS.scala:
##########
@@ -17,12 +17,40 @@
package org.apache.kyuubi.tpcds.benchmark
+import scala.io.{Codec, Source}
+
import org.apache.spark.sql.SparkSession
+import org.apache.kyuubi.tpcds.benchmark.ExecutionMode._
+
/**
* TPC-DS benchmark's dataset.
*/
class TPCDS(@transient sparkSession: SparkSession)
extends Benchmark(sparkSession)
with TPCDS_2_4_Queries
- with Serializable {}
+ with Serializable {
+
+ override val tpcds2_4Queries: Seq[Query] = queryNames.map { queryName =>
+ val in = getClass.getClassLoader.getResourceAsStream(s"tpcds_2_4/$queryName.sql")
+ val queryContent: String = Source.fromInputStream(in)(Codec.UTF8).mkString
+ in.close()
Review Comment:
done
##########
dev/kyuubi-tpcds/src/main/scala/org/apache/kyuubi/tpcds/benchmark/RunBenchmark.scala:
##########
@@ -73,6 +74,9 @@ object RunBenchmark {
c.copy(exclude = x.split(",").map(_.trim).filter(_.nonEmpty).toSet)
}
.text("name of the queries to exclude, use comma to split multiple names, e.g. q2,q4")
+ opt[String]('m', "execution-mode")
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org