ulysses-you commented on PR #58591: URL: https://github.com/apache/spark/pull/58591#issuecomment-5582684128
Thanks @dongjoon-hyun and @peter-toth for the thorough reviews — all items addressed in a9a920e489a, and the PR description has been updated accordingly. Summary of the changes: - Narrowed the replicated-reads metric registration to expected keys that actually carry multiple slots, renamed its key to `numReplicatedPartitionReads`, and made the metrics map the single source of truth for what `sendDriverMetrics` posts (`set` skips unregistered names). - Kept the direct `sendDriverMetrics()` calls in both `doExecute` and `doExecuteColumnar`: `SparkPlan.execute()` / `executeColumnar()` already memoize per instance through `LazyTry`, so the post happens at most once. - Dropped the redundant `numInputPartitions` field from `PartitionGrouping`; `sendDriverMetrics` reads `child.outputPartitioning.numPartitions`. - Moved the three e2e metric tests to `KeyGroupedPartitioningSuite` (carries `@ExtendedSQLTest`), each running with AQE on and off; `GroupPartitionsExecSuite` is back to `SharedSparkSession` with unit tests only, and `ExecutableKeyedLeaf` is top level. - Documented `number of pruned input partitions`, `number of replicated input partition reads` and `max partitions per group` in `docs/web-ui.md`. - Kept `assert(numSplits > 0)`; the reason is in the thread reply. -- 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]
