You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/06/03 23:12:28 UTC
[GitHub] [spark] ekoifman opened a new pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomSh…
ekoifman opened a new pull request #32776:
URL: https://github.com/apache/spark/pull/32776
…uffleReader in AQE
<!--
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://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error message, please read the guideline first:
https://spark.apache.org/error-message-guidelines.html
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
CustomShuffleReaderExec reports "number of skewed partitions" and "number of skewed partition splits".
It would be useful to also report "number of partitions to coalesce" and "number of coalesced partitions" and include this in string rendering of the SparkPlan node so that it looks like this
### 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.
-->
Improves usability
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
Yes, it now provides more information about `CustomShuffleReaderExec` operator behavior in the UI and logged version of the plan.
### 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.
-->
Existing tests
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-858315767
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44140/
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-881074922
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45613/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-858294433
ok to test
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to AQEShuffleRead in AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r677343975
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/AQEShuffleReadExec.scala
##########
@@ -183,6 +194,17 @@ case class AQEShuffleReadExec private(
} else {
Map.empty
}
+ } ++ {
+ if (isLocalRead) {
+ Map.empty
+ } else {
+ if (hasCoalescedPartition) {
Review comment:
reply to https://github.com/apache/spark/pull/32776#discussion_r676750494
If this is a local reader, then `hasCoalescedPartition` can never be true, right? I still think the `if (isLocalRead)` above is not needed.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-885351226
**[Test build #141526 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141526/testReport)** for PR 32776 at commit [`83a4082`](https://github.com/apache/spark/commit/83a40829101facba700463dfcb1d2c803d7fc157).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to AQEShuffleRead in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-887691853
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46226/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-885430071
**[Test build #141526 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141526/testReport)** for PR 32776 at commit [`83a4082`](https://github.com/apache/spark/commit/83a40829101facba700463dfcb1d2c803d7fc157).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-881059716
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45613/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-887092114
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141655/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r648845848
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/CustomShuffleReaderExec.scala
##########
@@ -76,19 +76,76 @@ case class CustomShuffleReaderExec private(
val desc = if (isLocalReader) {
"local"
} else if (hasCoalescedPartition && hasSkewedPartition) {
- "coalesced and skewed"
+ s"$coalescedDetail and $skewedDetail"
Review comment:
AFAIK plan node string never contains metrics before this PR.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to AQEShuffleRead in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-887844115
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141713/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to AQEShuffleRead in AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r677662671
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/ExplainSuite.scala
##########
@@ -572,6 +572,7 @@ class ExplainSuiteAE extends ExplainSuiteHelper with EnableAdaptiveExecutionSuit
"""
|(11) AQEShuffleRead
|Input [5]: [k#x, count#xL, sum#xL, sum#x, count#xL]
+ |Arguments: coalesced
Review comment:
just for curiosity: which code change leads to 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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-886983675
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46171/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-881125496
**[Test build #141100 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141100/testReport)** for PR 32776 at commit [`5360f51`](https://github.com/apache/spark/commit/5360f5127a69fd1468f3bb17f3b822e1edeffdad).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ekoifman commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
ekoifman commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r670878574
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/CustomShuffleReaderExec.scala
##########
@@ -76,19 +76,76 @@ case class CustomShuffleReaderExec private(
val desc = if (isLocalReader) {
"local"
} else if (hasCoalescedPartition && hasSkewedPartition) {
- "coalesced and skewed"
+ s"$coalescedDetail and $skewedDetail"
} else if (hasCoalescedPartition) {
- "coalesced"
+ coalescedDetail
} else if (hasSkewedPartition) {
- "skewed"
+ skewedDetail
} else {
""
}
Iterator(desc)
}
+ private def isCoalesced(spec: ShufflePartitionSpec) = coalesceRange(spec) > 1
+ /**
+ * How many partitions were coalesced; 0 if not [[CoalescedPartitionSpec]]
+ */
+ private def coalesceRange(spec: ShufflePartitionSpec) = spec match {
+ case s: CoalescedPartitionSpec => s.endReducerIndex - s.startReducerIndex
+ case _ => 0
+ }
+
+ /* This is left as documentation
+ * Is it worth reporting this? For example, if we have
+ * MapOutputStatistics 0,0,0,72,0
+ * MapOutputStatistics 0,0,0,138,138
+ * with target partition size 10, we'll have
+ * CoalescedPartitionSpec(3,4) & CoalescedPartitionSpec(4,5)
+ * So pre-shuffle partitions 0,1,2 are dropped
+ * Another example, (target size 10)
+ * MapOutputStatistics 0,3,0,2,7
+ * MapOutputStatistics 0,2,0,2,7
+ * Results in CoalescedPartitionSpec(1,4) & CoalescedPartitionSpec(4,5)
+ * So pre-shuffle partition 2 is included
+ * We could figure out dropped partitions but doesn't seem that useful.
+ */
+ private def numDroppedPartitions = 0
+
+ private def numCoalescedPartitions = partitionSpecs.count(isCoalesced)
+
+ /**
+ * partitions that will be combined with others (as opposed to taken as is, spilt, dropped)
+ */
+ private def numPartitionsToCoalesce = partitionSpecs.filter(isCoalesced)
+ .foldLeft(0)((c, s) => c + coalesceRange(s))
+
+ /**
+ * total splits of all skewed partitions
+ */
+ private def skewedPartitionSplits = partitionSpecs.collect {
+ case p: PartialReducerPartitionSpec => p
+ }
- def hasCoalescedPartition: Boolean =
- partitionSpecs.exists(_.isInstanceOf[CoalescedPartitionSpec])
Review comment:
this was done via https://github.com/apache/spark/pull/32872
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ekoifman commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
ekoifman commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r649340441
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/CustomShuffleReaderExec.scala
##########
@@ -171,6 +238,19 @@ case class CustomShuffleReaderExec private(
} else {
Map.empty
}
+ } ++ {
+ if (isLocalReader) {
+ Map.empty
+ } else {
+ if (hasCoalescedPartition) {
+ Map("numCoalescedPartitions" ->
+ SQLMetrics.createMetric(sparkContext, "number of coalesced partitions"),
+ "numPartitionsToCoalesce" ->
Review comment:
I added the coalesce metrics in the same way skew-related metrics were done.
Are you suggesting that all of them move to the `ShuffleExchangeExec` below?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ekoifman commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to AQEShuffleRead in AQE
Posted by GitBox <gi...@apache.org>.
ekoifman commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r677510453
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/AQEShuffleReadExec.scala
##########
@@ -183,6 +194,17 @@ case class AQEShuffleReadExec private(
} else {
Map.empty
}
+ } ++ {
+ if (isLocalRead) {
+ Map.empty
+ } else {
+ if (hasCoalescedPartition) {
Review comment:
yes, you're right
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ekoifman commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
ekoifman commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r649314100
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/CustomShuffleReaderExec.scala
##########
@@ -76,19 +76,76 @@ case class CustomShuffleReaderExec private(
val desc = if (isLocalReader) {
"local"
} else if (hasCoalescedPartition && hasSkewedPartition) {
- "coalesced and skewed"
+ s"$coalescedDetail and $skewedDetail"
Review comment:
The reason I put this additional info here is that it's easy to see what AQE did in large (100s of operators) plans in the logged version of the plan. It puts that info in the right context - the 1st image in the PR description shows what it looks like.
If the additional info was not exposed as SQL metrics but simply computed just to put in the node description - would it still be wrong to put it there?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-887081282
**[Test build #141655 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141655/testReport)** for PR 32776 at commit [`18f865b`](https://github.com/apache/spark/commit/18f865bdfcf67b7f8c330257aae214f3773857fc).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-881033429
**[Test build #141100 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141100/testReport)** for PR 32776 at commit [`5360f51`](https://github.com/apache/spark/commit/5360f5127a69fd1468f3bb17f3b822e1edeffdad).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-858463477
**[Test build #139613 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139613/testReport)** for PR 32776 at commit [`aee1392`](https://github.com/apache/spark/commit/aee1392720815f332e8fb993b4672bb03fe4ccb1).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-885383124
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46044/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to AQEShuffleRead in AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-888029450
thanks, merging to master/3.2!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ulysses-you commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r648834767
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/CustomShuffleReaderExec.scala
##########
@@ -76,19 +76,76 @@ case class CustomShuffleReaderExec private(
val desc = if (isLocalReader) {
"local"
} else if (hasCoalescedPartition && hasSkewedPartition) {
- "coalesced and skewed"
+ s"$coalescedDetail and $skewedDetail"
} else if (hasCoalescedPartition) {
- "coalesced"
+ coalescedDetail
} else if (hasSkewedPartition) {
- "skewed"
+ skewedDetail
} else {
""
}
Iterator(desc)
}
+ private def isCoalesced(spec: ShufflePartitionSpec) = coalesceRange(spec) > 1
+ /**
+ * How many partitions were coalesced; 0 if not [[CoalescedPartitionSpec]]
+ */
+ private def coalesceRange(spec: ShufflePartitionSpec) = spec match {
+ case s: CoalescedPartitionSpec => s.endReducerIndex - s.startReducerIndex
+ case _ => 0
+ }
+
+ /* This is left as documentation
+ * Is it worth reporting this? For example, if we have
+ * MapOutputStatistics 0,0,0,72,0
+ * MapOutputStatistics 0,0,0,138,138
+ * with target partition size 10, we'll have
+ * CoalescedPartitionSpec(3,4) & CoalescedPartitionSpec(4,5)
+ * So pre-shuffle partitions 0,1,2 are dropped
+ * Another example, (target size 10)
+ * MapOutputStatistics 0,3,0,2,7
+ * MapOutputStatistics 0,2,0,2,7
+ * Results in CoalescedPartitionSpec(1,4) & CoalescedPartitionSpec(4,5)
+ * So pre-shuffle partition 2 is included
+ * We could figure out dropped partitions but doesn't seem that useful.
+ */
+ private def numDroppedPartitions = 0
+
+ private def numCoalescedPartitions = partitionSpecs.count(isCoalesced)
+
+ /**
+ * partitions that will be combined with others (as opposed to taken as is, spilt, dropped)
+ */
+ private def numPartitionsToCoalesce = partitionSpecs.filter(isCoalesced)
+ .foldLeft(0)((c, s) => c + coalesceRange(s))
+
+ /**
+ * total splits of all skewed partitions
+ */
+ private def skewedPartitionSplits = partitionSpecs.collect {
+ case p: PartialReducerPartitionSpec => p
+ }
- def hasCoalescedPartition: Boolean =
- partitionSpecs.exists(_.isInstanceOf[CoalescedPartitionSpec])
Review comment:
this change worth a new PR, can you create a new one for it ? 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ekoifman commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
ekoifman commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-886902809
rebased on master after SPARK-36217
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r648844909
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/CustomShuffleReaderExec.scala
##########
@@ -76,19 +76,76 @@ case class CustomShuffleReaderExec private(
val desc = if (isLocalReader) {
"local"
} else if (hasCoalescedPartition && hasSkewedPartition) {
- "coalesced and skewed"
+ s"$coalescedDetail and $skewedDetail"
Review comment:
It makes sense to add more metrics but it doesn't make sense to include metrics in the plan node string.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-885373169
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46044/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ekoifman commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
ekoifman commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r676750494
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/CustomShuffleReaderExec.scala
##########
@@ -182,6 +193,17 @@ case class CustomShuffleReaderExec private(
} else {
Map.empty
}
+ } ++ {
+ if (isLocalReader) {
Review comment:
I think so. For local reader all `CustomShuffleReaderExec.partitionSpecs` should be `PartialMapperPartitionSpec`
so `hasCoalescedPartitions` is false, so `numCoalescedPartitions` metric would always be `0`. It doesn't seem to add value to report it.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to AQEShuffleRead in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-887691853
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46226/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-886998600
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46171/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-881133229
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141100/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ekoifman commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
ekoifman commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r670878574
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/CustomShuffleReaderExec.scala
##########
@@ -76,19 +76,76 @@ case class CustomShuffleReaderExec private(
val desc = if (isLocalReader) {
"local"
} else if (hasCoalescedPartition && hasSkewedPartition) {
- "coalesced and skewed"
+ s"$coalescedDetail and $skewedDetail"
} else if (hasCoalescedPartition) {
- "coalesced"
+ coalescedDetail
} else if (hasSkewedPartition) {
- "skewed"
+ skewedDetail
} else {
""
}
Iterator(desc)
}
+ private def isCoalesced(spec: ShufflePartitionSpec) = coalesceRange(spec) > 1
+ /**
+ * How many partitions were coalesced; 0 if not [[CoalescedPartitionSpec]]
+ */
+ private def coalesceRange(spec: ShufflePartitionSpec) = spec match {
+ case s: CoalescedPartitionSpec => s.endReducerIndex - s.startReducerIndex
+ case _ => 0
+ }
+
+ /* This is left as documentation
+ * Is it worth reporting this? For example, if we have
+ * MapOutputStatistics 0,0,0,72,0
+ * MapOutputStatistics 0,0,0,138,138
+ * with target partition size 10, we'll have
+ * CoalescedPartitionSpec(3,4) & CoalescedPartitionSpec(4,5)
+ * So pre-shuffle partitions 0,1,2 are dropped
+ * Another example, (target size 10)
+ * MapOutputStatistics 0,3,0,2,7
+ * MapOutputStatistics 0,2,0,2,7
+ * Results in CoalescedPartitionSpec(1,4) & CoalescedPartitionSpec(4,5)
+ * So pre-shuffle partition 2 is included
+ * We could figure out dropped partitions but doesn't seem that useful.
+ */
+ private def numDroppedPartitions = 0
+
+ private def numCoalescedPartitions = partitionSpecs.count(isCoalesced)
+
+ /**
+ * partitions that will be combined with others (as opposed to taken as is, spilt, dropped)
+ */
+ private def numPartitionsToCoalesce = partitionSpecs.filter(isCoalesced)
+ .foldLeft(0)((c, s) => c + coalesceRange(s))
+
+ /**
+ * total splits of all skewed partitions
+ */
+ private def skewedPartitionSplits = partitionSpecs.collect {
+ case p: PartialReducerPartitionSpec => p
+ }
- def hasCoalescedPartition: Boolean =
- partitionSpecs.exists(_.isInstanceOf[CoalescedPartitionSpec])
Review comment:
first bullet was done via https://github.com/apache/spark/pull/32872
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-881073269
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45613/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to AQEShuffleRead in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-887672769
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46226/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-881074922
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45613/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-885363742
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46044/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to AQEShuffleRead in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-887649856
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46226/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ekoifman commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to AQEShuffleRead in AQE
Posted by GitBox <gi...@apache.org>.
ekoifman commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r677660104
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/AQEShuffleReadExec.scala
##########
@@ -183,6 +194,17 @@ case class AQEShuffleReadExec private(
} else {
Map.empty
}
+ } ++ {
+ if (isLocalRead) {
+ Map.empty
+ } else {
+ if (hasCoalescedPartition) {
Review comment:
updated
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ekoifman commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
ekoifman commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r670878987
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/CustomShuffleReaderExec.scala
##########
@@ -76,19 +76,76 @@ case class CustomShuffleReaderExec private(
val desc = if (isLocalReader) {
"local"
} else if (hasCoalescedPartition && hasSkewedPartition) {
- "coalesced and skewed"
+ s"$coalescedDetail and $skewedDetail"
Review comment:
remved `stringArgs` changes
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-883446292
Since skewed partitions and coalesced partitions can appear together in one custom shuffle reader, I think it's reasonable to add the `numCoalescedPartitions` metric.
`numPartitionsToCoalesce` is a bit overkill to me. If we can know the original number of partitions of the shuffle (ShuffleExchangeExec can report it), we can get a sense of the custom shuffle reader by looking at num skewed partitions/splits and num coalesced partitions.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ekoifman commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
ekoifman commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-856890486
@cloud-fan could you please take a look
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to AQEShuffleRead in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-887844115
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141713/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to AQEShuffleRead in AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #32776:
URL: https://github.com/apache/spark/pull/32776
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ekoifman commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
ekoifman commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r648838734
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/CustomShuffleReaderExec.scala
##########
@@ -76,19 +76,76 @@ case class CustomShuffleReaderExec private(
val desc = if (isLocalReader) {
"local"
} else if (hasCoalescedPartition && hasSkewedPartition) {
- "coalesced and skewed"
+ s"$coalescedDetail and $skewedDetail"
} else if (hasCoalescedPartition) {
- "coalesced"
+ coalescedDetail
} else if (hasSkewedPartition) {
- "skewed"
+ skewedDetail
} else {
""
}
Iterator(desc)
}
+ private def isCoalesced(spec: ShufflePartitionSpec) = coalesceRange(spec) > 1
+ /**
+ * How many partitions were coalesced; 0 if not [[CoalescedPartitionSpec]]
+ */
+ private def coalesceRange(spec: ShufflePartitionSpec) = spec match {
+ case s: CoalescedPartitionSpec => s.endReducerIndex - s.startReducerIndex
+ case _ => 0
+ }
+
+ /* This is left as documentation
+ * Is it worth reporting this? For example, if we have
+ * MapOutputStatistics 0,0,0,72,0
+ * MapOutputStatistics 0,0,0,138,138
+ * with target partition size 10, we'll have
+ * CoalescedPartitionSpec(3,4) & CoalescedPartitionSpec(4,5)
+ * So pre-shuffle partitions 0,1,2 are dropped
+ * Another example, (target size 10)
+ * MapOutputStatistics 0,3,0,2,7
+ * MapOutputStatistics 0,2,0,2,7
+ * Results in CoalescedPartitionSpec(1,4) & CoalescedPartitionSpec(4,5)
+ * So pre-shuffle partition 2 is included
+ * We could figure out dropped partitions but doesn't seem that useful.
+ */
+ private def numDroppedPartitions = 0
+
+ private def numCoalescedPartitions = partitionSpecs.count(isCoalesced)
+
+ /**
+ * partitions that will be combined with others (as opposed to taken as is, spilt, dropped)
+ */
+ private def numPartitionsToCoalesce = partitionSpecs.filter(isCoalesced)
+ .foldLeft(0)((c, s) => c + coalesceRange(s))
+
+ /**
+ * total splits of all skewed partitions
+ */
+ private def skewedPartitionSplits = partitionSpecs.collect {
+ case p: PartialReducerPartitionSpec => p
+ }
- def hasCoalescedPartition: Boolean =
- partitionSpecs.exists(_.isInstanceOf[CoalescedPartitionSpec])
Review comment:
I don't understand this comment. This change is a critical part of this PR
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-858466250
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139613/
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-854245118
Can one of the admins verify this patch?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-886962850
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/46171/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-858294837
**[Test build #139613 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139613/testReport)** for PR 32776 at commit [`aee1392`](https://github.com/apache/spark/commit/aee1392720815f332e8fb993b4672bb03fe4ccb1).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-885442604
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141526/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-858335819
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44140/
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r649352199
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/CustomShuffleReaderExec.scala
##########
@@ -76,19 +76,76 @@ case class CustomShuffleReaderExec private(
val desc = if (isLocalReader) {
"local"
} else if (hasCoalescedPartition && hasSkewedPartition) {
- "coalesced and skewed"
+ s"$coalescedDetail and $skewedDetail"
Review comment:
No other plan node does this: metrics are only available through UI or programing APIs (e.g. `QueryExecutionListener`).
I can't find a strong reason to start doing it.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-886924741
**[Test build #141655 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141655/testReport)** for PR 32776 at commit [`18f865b`](https://github.com/apache/spark/commit/18f865bdfcf67b7f8c330257aae214f3773857fc).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomSh…
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-854245118
Can one of the admins verify this patch?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-881033429
**[Test build #141100 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141100/testReport)** for PR 32776 at commit [`5360f51`](https://github.com/apache/spark/commit/5360f5127a69fd1468f3bb17f3b822e1edeffdad).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r648846235
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/CustomShuffleReaderExec.scala
##########
@@ -76,19 +76,76 @@ case class CustomShuffleReaderExec private(
val desc = if (isLocalReader) {
"local"
} else if (hasCoalescedPartition && hasSkewedPartition) {
- "coalesced and skewed"
+ s"$coalescedDetail and $skewedDetail"
} else if (hasCoalescedPartition) {
- "coalesced"
+ coalescedDetail
} else if (hasSkewedPartition) {
- "skewed"
+ skewedDetail
} else {
""
}
Iterator(desc)
}
+ private def isCoalesced(spec: ShufflePartitionSpec) = coalesceRange(spec) > 1
+ /**
+ * How many partitions were coalesced; 0 if not [[CoalescedPartitionSpec]]
+ */
+ private def coalesceRange(spec: ShufflePartitionSpec) = spec match {
+ case s: CoalescedPartitionSpec => s.endReducerIndex - s.startReducerIndex
+ case _ => 0
+ }
+
+ /* This is left as documentation
+ * Is it worth reporting this? For example, if we have
+ * MapOutputStatistics 0,0,0,72,0
+ * MapOutputStatistics 0,0,0,138,138
+ * with target partition size 10, we'll have
+ * CoalescedPartitionSpec(3,4) & CoalescedPartitionSpec(4,5)
+ * So pre-shuffle partitions 0,1,2 are dropped
+ * Another example, (target size 10)
+ * MapOutputStatistics 0,3,0,2,7
+ * MapOutputStatistics 0,2,0,2,7
+ * Results in CoalescedPartitionSpec(1,4) & CoalescedPartitionSpec(4,5)
+ * So pre-shuffle partition 2 is included
+ * We could figure out dropped partitions but doesn't seem that useful.
Review comment:
I don't think it's useful to report this metrics.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-858294837
**[Test build #139613 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139613/testReport)** for PR 32776 at commit [`aee1392`](https://github.com/apache/spark/commit/aee1392720815f332e8fb993b4672bb03fe4ccb1).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-858347806
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/44140/
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ekoifman commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
ekoifman commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-883564783
That makes sense.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ekoifman commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
ekoifman commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-881035108
@cloud-fan
I rebased after https://github.com/apache/spark/pull/32872 and https://github.com/apache/spark/pull/33342
and removed the changes to `CustomShuffleReaderExec.stringArgs` that you objected to.
So now this just adds 2 new metrics `numCoalescedPartitions` and `numPartitionsToCoalesce` to compliment the existing `numSkewedSplits` metrics.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r676519068
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/CustomShuffleReaderExec.scala
##########
@@ -182,6 +193,17 @@ case class CustomShuffleReaderExec private(
} else {
Map.empty
}
+ } ++ {
+ if (isLocalReader) {
Review comment:
do we need this if condition here?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to AQEShuffleRead in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-887612350
**[Test build #141713 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141713/testReport)** for PR 32776 at commit [`7f51402`](https://github.com/apache/spark/commit/7f5140228c5f4f5876dd6653d90472fc92bdb5f4).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-885383124
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46044/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-858466250
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139613/
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ekoifman commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to AQEShuffleRead in AQE
Posted by GitBox <gi...@apache.org>.
ekoifman commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r677673122
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/ExplainSuite.scala
##########
@@ -572,6 +572,7 @@ class ExplainSuiteAE extends ExplainSuiteHelper with EnableAdaptiveExecutionSuit
"""
|(11) AQEShuffleRead
|Input [5]: [k#x, count#xL, sum#xL, sum#x, count#xL]
+ |Arguments: coalesced
Review comment:
There isn't one in this PR. This line was part of the actual output before. The test checks if the actual plan contains
```
"""
|(11) AQEShuffleRead
|Input [5]: [k#x, count#xL, sum#xL, sum#x, count#xL]
|Arguments: coalesced
|""".stripMargin,
```
which is true in both versions.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-885351226
**[Test build #141526 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141526/testReport)** for PR 32776 at commit [`83a4082`](https://github.com/apache/spark/commit/83a40829101facba700463dfcb1d2c803d7fc157).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-886998600
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/46171/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to AQEShuffleRead in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-887823729
**[Test build #141713 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141713/testReport)** for PR 32776 at commit [`7f51402`](https://github.com/apache/spark/commit/7f5140228c5f4f5876dd6653d90472fc92bdb5f4).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-887092114
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141655/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomSh…
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-854245118
Can one of the admins verify this patch?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-886924741
**[Test build #141655 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141655/testReport)** for PR 32776 at commit [`18f865b`](https://github.com/apache/spark/commit/18f865bdfcf67b7f8c330257aae214f3773857fc).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-858347806
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/44140/
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to AQEShuffleRead in AQE
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-887612350
**[Test build #141713 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/141713/testReport)** for PR 32776 at commit [`7f51402`](https://github.com/apache/spark/commit/7f5140228c5f4f5876dd6653d90472fc92bdb5f4).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32776:
URL: https://github.com/apache/spark/pull/32776#discussion_r648845537
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/CustomShuffleReaderExec.scala
##########
@@ -171,6 +238,19 @@ case class CustomShuffleReaderExec private(
} else {
Map.empty
}
+ } ++ {
+ if (isLocalReader) {
+ Map.empty
+ } else {
+ if (hasCoalescedPartition) {
+ Map("numCoalescedPartitions" ->
+ SQLMetrics.createMetric(sparkContext, "number of coalesced partitions"),
+ "numPartitionsToCoalesce" ->
Review comment:
There is always a shuffle node below `CustomShuffleReader`. I think it makes more sense to let the shuffle node to report the metrics of the number of partitions/reducers.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-885442604
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141526/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32776: [SPARK-35639][SQL] Add metrics about coalesced partitions to CustomShuffleReader in AQE
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32776:
URL: https://github.com/apache/spark/pull/32776#issuecomment-881133229
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/141100/
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org