peter-toth commented on code in PR #58591:
URL: https://github.com/apache/spark/pull/58591#discussion_r3956116408
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/GroupPartitionsExec.scala:
##########
@@ -274,6 +307,34 @@ case class GroupPartitionsExec(
@transient private lazy val hasCoalescing: Boolean =
groupedPartitions.exists(_._2.size > 1)
+ // All values are computed on the driver by `grouping`, so they are reported
through
+ // `sendDriverMetrics` rather than task-side accumulators. Registration
reads constructor
+ // parameters only: pruning and replication are facts of the alignment path
and are registered
+ // only in the modes that produce them, while coalescing can happen in any
mode and reports 0
+ // when nothing merged.
+ @transient override lazy val metrics: Map[String, SQLMetric] = Map(
+ "numInputPartitions" -> SQLMetrics.createMetric(sparkContext, "number of
input partitions"),
+ "numPartitions" -> SQLMetrics.createMetric(sparkContext, "number of
partitions"),
+ "numEmptyPartitions" -> SQLMetrics.createMetric(sparkContext, "number of
empty partitions"),
+ "numCoalescedPartitions" ->
+ SQLMetrics.createMetric(sparkContext, "number of coalesced partitions"),
+ "maxPartitionsPerGroup" ->
+ SQLMetrics.createMetric(sparkContext, "max partitions per group")) ++ {
+ if (expectedPartitionKeys.isDefined && !distributePartitions) {
+ Map("numReplicatedPartitions" -> SQLMetrics.createMetric(sparkContext,
+ "number of replicated input partition reads"))
+ } else {
+ Map.empty[String, SQLMetric]
+ }
+ } ++ {
+ if (expectedPartitionKeys.isDefined) {
+ Map("numPrunedPartitions" ->
+ SQLMetrics.createMetric(sparkContext, "number of pruned input
partitions"))
Review Comment:
**Finding 3.** No `docs/web-ui.md` entry for the two accounting metrics.
(Separate point from the comment-wording thread just above.)
The five counting metrics read for themselves. These two do not. "Pruned"
means input partitions the alignment never references because the join proved
the key cannot produce output. "Replicated" means reads beyond the first pass,
so that `reads = input - pruned + replicated`. That takes three paragraphs in
the PR description, and none of it is reachable from the SQL tab.
The SQL metrics table at `docs/web-ui.md:347` is where that belongs. Two
rows would do:
```html
<tr><td> <code>number of pruned input partitions</code> </td><td> the number
of input partitions skipped because the join proved their partition key cannot
produce output </td><td> GroupPartitions </td></tr>
<tr><td> <code>number of replicated input partition reads</code> </td><td>
the number of extra reads of input partitions caused by replicating a key group
across the other side's partitions </td><td> GroupPartitions </td></tr>
```
`AQEShuffleRead`'s driver metrics are absent from that table too, so this is
a gap the table already has rather than something this PR diverges on.
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/GroupPartitionsExecSuite.scala:
##########
@@ -508,6 +523,281 @@ class GroupPartitionsExecSuite extends SharedSparkSession
{
assert(gpe.tryEnableSortedMerge().isEmpty)
}
}
+
+ test("SPARK-59310: basic counts without alignment") {
+ // Keys [1, 2, 1]: 3 input splits, key 1 coalesces partitions 0 and 2, 2
output partitions.
+ val child = ExecutableKeyedLeaf(KeyedPartitioning(Seq(exprA), Seq(row(1),
row(2), row(1))))
+ val gpe = GroupPartitionsExec(child)
+ gpe.execute()
+
+ assert(gpe.metrics("numInputPartitions").value === 3)
+ assert(gpe.metrics("numPartitions").value === 2)
+ assert(gpe.metrics("numEmptyPartitions").value === 0)
+ assert(gpe.metrics("numCoalescedPartitions").value === 1)
+ assert(gpe.metrics("maxPartitionsPerGroup").value === 2)
+ // Without expectedPartitionKeys there is no alignment, so its metrics
stay unregistered.
+ assert(!gpe.metrics.contains("numPrunedPartitions"))
+ assert(!gpe.metrics.contains("numReplicatedPartitions"))
+ }
+
+ test("SPARK-59310: zero coalesced without duplicate keys") {
+ val child = ExecutableKeyedLeaf(KeyedPartitioning(Seq(exprA), Seq(row(1),
row(2), row(3))))
+ val gpe = GroupPartitionsExec(child)
+ gpe.execute()
+
+ assert(gpe.metrics("numCoalescedPartitions").value === 0)
+ assert(gpe.metrics("numPartitions").value === 3)
+ assert(gpe.metrics("maxPartitionsPerGroup").value === 1)
+ }
+
+ test("SPARK-59310: distribute alignment pads and never replicates") {
+ // Child splits: key 1 -> [0], key 2 -> [1, 2]. Expected: key 1 x1, key 2
x3, key 3 x1.
+ // Key 2 spreads its 2 splits over 3 expected partitions (one empty pad)
and key 3 has no
+ // split (one more empty), so 5 output partitions with 2 empty and nothing
coalesced.
+ def keyOf(a: Int): InternalRowComparableWrapper =
+ InternalRowComparableWrapper(row(a), Seq(exprA))
+ val child = ExecutableKeyedLeaf(KeyedPartitioning(Seq(exprA), Seq(row(1),
row(2), row(2))))
+ val gpe = GroupPartitionsExec(child,
+ expectedPartitionKeys = Some(Seq(keyOf(1) -> 1, keyOf(2) -> 3, keyOf(3)
-> 1)),
+ distributePartitions = true)
+ gpe.execute()
+
+ assert(gpe.metrics("numInputPartitions").value === 3)
+ assert(gpe.metrics("numPartitions").value === 5)
+ assert(gpe.metrics("numEmptyPartitions").value === 2)
+ assert(gpe.metrics("numPrunedPartitions").value === 0)
+ assert(gpe.metrics("numCoalescedPartitions").value === 0, "distribute
never coalesces")
+ assert(!gpe.metrics.contains("numReplicatedPartitions"), "distribute never
replicates")
+ }
+
+ test("SPARK-59310: alignment prunes unmatched keys, pads missing ones") {
+ // The expected keys carry key 1 and a key-3 slot the child does not hold,
as an inner
+ // join's intersection combined with the other side's layout would. The 2
splits of key 2
+ // cannot produce join output and never enter the alignment; the missing
key 3 pads two
+ // empty output partitions, and being empty, replicates nothing despite
its 2 slots.
+ def keyOf(a: Int): InternalRowComparableWrapper =
+ InternalRowComparableWrapper(row(a), Seq(exprA))
+ val child = ExecutableKeyedLeaf(KeyedPartitioning(Seq(exprA), Seq(row(1),
row(2), row(2))))
+ val gpe = GroupPartitionsExec(child,
+ expectedPartitionKeys = Some(Seq(keyOf(1) -> 1, keyOf(3) -> 2)))
+ gpe.execute()
+
+ assert(gpe.metrics("numInputPartitions").value === 3)
+ assert(gpe.metrics("numPartitions").value === 3)
+ assert(gpe.metrics("numPrunedPartitions").value === 2)
+ assert(gpe.metrics("numEmptyPartitions").value === 2)
+ assert(gpe.metrics("numCoalescedPartitions").value === 0)
+ assert(gpe.metrics("maxPartitionsPerGroup").value === 1)
+ assert(gpe.metrics("numReplicatedPartitions").value === 0,
+ "empty groups replicate nothing, and the single-split key 1 has no copy")
+ }
+
+ test("SPARK-59310: replicate alignment counts the reads beyond the first") {
+ // The other join side expects 2 partitions for key 1, so this side's
splits for the key are
+ // replicated to both: the slot beyond the first re-reads both splits, 2
extra input
+ // partition reads. Each output partition also coalesces the 2 splits of
the key.
+ def keyOf(a: Int): InternalRowComparableWrapper =
+ InternalRowComparableWrapper(row(a), Seq(exprA))
+ val child = ExecutableKeyedLeaf(KeyedPartitioning(Seq(exprA), Seq(row(1),
row(1))))
+ val gpe = GroupPartitionsExec(child, expectedPartitionKeys =
Some(Seq(keyOf(1) -> 2)))
+ gpe.execute()
+
+ assert(gpe.metrics("numInputPartitions").value === 2)
+ assert(gpe.metrics("numPartitions").value === 2)
+ assert(gpe.metrics("numReplicatedPartitions").value === 2)
+ assert(gpe.metrics("numCoalescedPartitions").value === 2, "both copies
merge the 2 splits")
+ assert(gpe.metrics("maxPartitionsPerGroup").value === 2)
+ assert(gpe.metrics("numEmptyPartitions").value === 0)
+ assert(gpe.metrics("numPrunedPartitions").value === 0)
+ }
+
+ test("SPARK-59310: an inner join intersection prunes both sides") {
+ withSQLConf(
+ SQLConf.V2_BUCKETING_ENABLED.key -> "true",
+ SQLConf.V2_BUCKETING_PUSH_PART_VALUES_ENABLED.key -> "true",
+ SQLConf.V2_BUCKETING_PARTITION_FILTER_ENABLED.key -> "true",
+ SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withTable(s"testcat.ns.$items", s"testcat.ns.$purchases") {
+ createTable(items, itemsColumns, Array(identity("id")))
+ sql(s"INSERT INTO testcat.ns.$items VALUES " +
+ s"(1, 'aa', 40.0, cast('2020-01-01' as timestamp)), " +
+ s"(1, 'aa', 41.0, cast('2020-01-15' as timestamp)), " +
+ s"(2, 'bb', 10.0, cast('2020-01-01' as timestamp)), " +
+ s"(2, 'bb', 10.5, cast('2020-01-01' as timestamp)), " +
+ s"(3, 'cc', 15.5, cast('2020-02-01' as timestamp))")
+ createTable(purchases, purchasesColumns, Array(identity("item_id")))
+ sql(s"INSERT INTO testcat.ns.$purchases VALUES " +
+ s"(1, 42.0, cast('2020-01-01' as timestamp)), " +
+ s"(1, 44.0, cast('2020-01-15' as timestamp)), " +
+ s"(1, 45.0, cast('2020-01-15' as timestamp)), " +
+ s"(2, 11.0, cast('2020-01-01' as timestamp)), " +
+ s"(4, 19.5, cast('2020-02-01' as timestamp))")
+
+ // The sides hold different keys ({1,2,3} vs {1,2,4}), so grouping
alone cannot align
+ // them and the join pushes the inner intersection {1,2} down as the
expected keys.
+ // Each side then coalesces its duplicate-key splits (items merges two
groups of two,
+ // purchases one group of three) and prunes the one split of its
unmatched key
+ // (items key 3, purchases key 4); nothing is empty or replicated.
+ // Both AQE arms: the query has no shuffle, so the executed nodes and
their accumulator
+ // ids are the same with and without AQE, and the reporting chain must
work in both.
+ Seq(false, true).foreach { aqeEnabled =>
+ withSQLConf(SQLConf.ADAPTIVE_EXECUTION_ENABLED.key ->
aqeEnabled.toString) {
+ val df = sql(s"SELECT i.id FROM testcat.ns.$items i JOIN
testcat.ns.$purchases p " +
+ "ON i.id = p.item_id")
+ val previousExecutionIds = currentExecutionIds()
+ checkAnswer(df, Seq.fill(6)(Row(1L)) ++ Seq.fill(2)(Row(2L)))
+ val executionIds = currentExecutionIds().diff(previousExecutionIds)
+ assert(executionIds.size === 1)
+ val executionId = executionIds.head
+
+ // The metrics must survive the full reporting path: set on the
driver during
+ // doExecute, posted to the listener bus, and readable back from
the status store
+ // the SQL UI renders.
+ val metricValues = statusStore.executionMetrics(executionId)
+ val groupNodes =
+ statusStore.planGraph(executionId).nodes.filter(_.name ==
"GroupPartitions")
+ assert(groupNodes.size === 2, "one GroupPartitionsExec per join
side")
+ groupNodes.foreach { node =>
+ assert(metricValue(metricValues, node, "number of input
partitions") === "5")
+ assert(metricValue(metricValues, node, "number of partitions")
=== "2")
+ assert(metricValue(metricValues, node, "number of empty
partitions") === "0")
+ assert(metricValue(metricValues, node, "number of pruned input
partitions") === "1")
+ assert(metricValue(metricValues, node,
+ "number of replicated input partition reads") === "0",
+ "no expected key carries multiple splits, so nothing is
replicated")
+ }
+ assert(groupNodes.map(metricValue(metricValues, _, "number of
coalesced partitions"))
+ .sorted === Seq("1", "2"))
+ assert(groupNodes.map(metricValue(metricValues, _, "max partitions
per group"))
+ .sorted === Seq("2", "3"))
+ }
+ }
+ }
+ }
+ }
+
+ test("SPARK-59310: a disjoint inner join prunes both sides to empty end to
end") {
+ withSQLConf(
+ SQLConf.V2_BUCKETING_ENABLED.key -> "true",
+ SQLConf.V2_BUCKETING_PUSH_PART_VALUES_ENABLED.key -> "true",
+ SQLConf.V2_BUCKETING_PARTITION_FILTER_ENABLED.key -> "true",
+ SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1",
+ SQLConf.ADAPTIVE_EXECUTION_ENABLED.key -> "false") {
+ withTable(s"testcat.ns.$items", s"testcat.ns.$purchases") {
+ createTable(items, itemsColumns, Array(identity("id")))
+ sql(s"INSERT INTO testcat.ns.$items VALUES " +
+ s"(1, 'aa', 40.0, cast('2020-01-01' as timestamp)), " +
+ s"(2, 'bb', 10.0, cast('2020-01-01' as timestamp))")
+ createTable(purchases, purchasesColumns, Array(identity("item_id")))
+ sql(s"INSERT INTO testcat.ns.$purchases VALUES " +
+ s"(3, 42.0, cast('2020-01-01' as timestamp)), " +
+ s"(4, 44.0, cast('2020-01-15' as timestamp))")
+
+ // The key sets {1, 2} and {3, 4} are disjoint, so the inner join's
intersection is
+ // empty: the alignment emits no output partition on either side, each
side prunes both
+ // of its inputs, and doExecute takes the empty-RDD branch. The
metrics are sent before
+ // that branch, so the total pruning still reaches the store.
+ val df = sql(s"SELECT i.id FROM testcat.ns.$items i JOIN
testcat.ns.$purchases p " +
+ "ON i.id = p.item_id")
+ val previousExecutionIds = currentExecutionIds()
+ checkAnswer(df, Nil)
+ val executionIds = currentExecutionIds().diff(previousExecutionIds)
+ assert(executionIds.size === 1)
+ val executionId = executionIds.head
+
+ val metricValues = statusStore.executionMetrics(executionId)
+ val groupNodes =
+ statusStore.planGraph(executionId).nodes.filter(_.name ==
"GroupPartitions")
+ assert(groupNodes.size === 2, "one GroupPartitionsExec per join side")
+ groupNodes.foreach { node =>
+ assert(metricValue(metricValues, node, "number of input partitions")
=== "2")
+ assert(metricValue(metricValues, node, "number of partitions") ===
"0")
+ assert(metricValue(metricValues, node, "number of pruned input
partitions") === "2")
+ assert(metricValue(metricValues, node, "number of empty partitions")
=== "0")
+ assert(metricValue(metricValues, node, "number of coalesced
partitions") === "0")
+ assert(metricValue(metricValues, node, "max partitions per group")
=== "0")
+ assert(metricValue(metricValues, node,
+ "number of replicated input partition reads") === "0")
+ }
+ }
+ }
+ }
+
+ test("SPARK-59310: partial clustering replicates the smaller side") {
+ withSQLConf(
+ SQLConf.V2_BUCKETING_ENABLED.key -> "true",
+ SQLConf.V2_BUCKETING_PUSH_PART_VALUES_ENABLED.key -> "true",
+ SQLConf.V2_BUCKETING_PARTIALLY_CLUSTERED_DISTRIBUTION_ENABLED.key ->
"true",
+ SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1",
+ SQLConf.ADAPTIVE_EXECUTION_ENABLED.key -> "false") {
Review Comment:
**Finding 2.** The replicate path is only covered with AQE off.
This test and the disjoint one at line 685 pin `ADAPTIVE_EXECUTION_ENABLED`
to false, while the intersection test loops both arms. AQE is on by default, so
partial clustering -- the case this feature exists for -- is never exercised in
the configuration users run.
I flipped both `"false"`s to `"true"` on `358ca646ade` and all eight
SPARK-59310 tests passed. I also ran a variant where the join sits under a
`GROUP BY` on a non-partition column, so the `GroupPartitions` nodes land
inside a shuffle query stage rather than the result stage, and the metrics
still reached the status store.
So either drop the pin, or wrap this test in the same `Seq(false,
true).foreach` the intersection test uses.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]