Conversation
|
amahussein
left a comment
There was a problem hiding this comment.
The design is the conservative one. prune is an allowlist ending in case _ => None, so an unknown node falls back rather than opting in, and the boundary pass replaces the sampling job createRangeBounds already ran over the full input rather than adding a third read. Worth saying plainly that a bad boundary costs skew and spill here, not wrong results.
Nothing in CI can detect a wrong partition assignment, which is exactly what a boundary bug produces; details inline. And Fixes #15444 does not hold: that reproducer has no clustering key, and optimized write here partitions round-robin or hash, never by range, so getPartitioner's range case is not on its path. Please drop it or say which part you address.
other questions related to the description
- Which stage did the 107x come from, and was it one run per arm or a median of several? The table is headed "Stage metrics", so it reads as the boundary stage rather than end-to-end. Both numbers are interesting; they are different claims.
- Which plan produced it? The plugin's OSS z-order and liquid-clustering acceleration (
ZOrderRules.partExprRule) reuses therangeBoundsDelta's CPURangePartitioneralready computed and never builds aGpuRangePartitioningexchange, so that path is untouched by this change. A one-line plan fragment showingGpuColumnarExchange gpurangepartitioning(...)would settle which path the measurement covers, and would also tell the next reader which workloads benefit. - What does a narrow table do? 585 columns with a one-column clustering key is the best case. When the key covers most columns,
buildstill succeeds and the boundary pass re-reads at nearly full width, adding a plan node and a per-batch projection. I see no mechanism for a meaningful regression, but the description should say where the benefit goes to zero.
|
|
||
| @transient lazy val inputBatchRDD: RDD[ColumnarBatch] = child.executeColumnar() | ||
|
|
||
| @transient private lazy val rangeBoundaryPlan: Option[GpuRangeBoundaryExec] = |
There was a problem hiding this comment.
Non-blocking: unconditional for every GPU range exchange the pruner accepts, with no off switch, so a field regression needs a patched release to mitigate.
Suggest an internal boolean defaulting to true, shaped like spark.rapids.sql.shuffledHashJoin.optimizeShuffle. It also lets a test select between the two paths, which the suite comment needs.
There was a problem hiding this comment.
Fixed. I added the default on internal setting spark.rapids.sql.rangePartitioning.sampleKeysOnly. Setting it to false restores the original full width GPU sampling path, and the disabled path is covered by a correctness test.
The 107× result is boundary-collection time, comparing one baseline run with one optimized run. It is not a median or an end-to-end result. I updated the description to make that explicit. |
The measured plan contained GpuColumnarExchange gpurangepartitioning( ASC, ASC, 58), so it exercised GpuRangePartitioner.createRangeBounds. I added this fragment and clarified that paths reusing Delta’s CPU-computed bounds are not affected. |
Agreed. I clarified that the benefit approaches zero when the input is narrow or the ordering keys require most columns. I also added a default-on internal kill switch so the original full-width path can be restored if necessary. |
Signed-off-by: Rahul Prabhu <raprabhu@nvidia.com>
7096c04 to
81b6adc
Compare
Signed-off-by: Rahul Prabhu <raprabhu@nvidia.com>
|
Thanks @sdrp713 |
There was a problem hiding this comment.
Thanks, this addresses everything from the last round.
The boundary test now asserts the range invariant itself, which is the property a mis-mapped projection or a wrongly bound sorter would break, and the fiddly parts are right: empty partitions dropped before comparison, and <= rather than <. The conf is .internal() and reads through the same idiom the file already uses, so no generated docs are owed.
One blocking item, and it is a title edit: please add [databricks]. The diff touches no ...db/ path, so DBR premerge will not run on its own, and this is the only part of the support matrix with no evidence behind it. It is reachable there, since the DBR scan metas convert to the same shared GpuFileSourceScanExec, and the exchange's DBR subclasses inherit the new subqueries override.
Premerge still has not run on this head.
That settles which path the numbers cover, thanks. One inconsistency: the fragment is two keys and 58 ranges, but the Performance section says 1,000 ranges and a table clustered by a column. With the table declared as one run per arm, the plan is what makes it checkable, so worth saying which run it came from. |
gerashegalov
left a comment
There was a problem hiding this comment.
Could we add coverage for the two successful pruning paths that are currently untested?
- A deterministic computed range key through GpuProjectExec, checking that the scan retains all key dependencies and that the resulting partitions satisfy the range-ordering invariant.
- A partition-column-only range key, checking the empty data schema, pruned partition schema, row preservation, and the same ordering invariant.
|
build |
Signed-off-by: Rahul Prabhu <raprabhu@nvidia.com>
|
build |
|
builds are failing due to unrelated cudf dependency |
Fixes #15903
Description
Problem
GPU range-boundary collection currently samples the full input row. For wide tables, this means decoding and retaining hundreds of payload columns even though boundary selection only needs the range key dependencies. This can cause excessive GPU memory pressure, spill, and semaphore wait.
Change
Build a narrow GPU plan for
GpuRangePartitioner.createRangeBoundscontaining only:The sampling algorithm, weighting, ordering, and boundary selection are unchanged. The subsequent range shuffle still processes the complete rows.
Pruning is allowlist based. Unsupported or nondeterministic plans use the original full width GPU path. The optimization can also be disabled with:
spark.rapids.sql.rangePartitioning.sampleKeysOnly=falseDifferent valid samples may produce different boundaries and therefore different skew or spill, but do not change result rows.
This applies to plans containing a GPU range exchange, such as:
GpuColumnarExchange gpurangepartitioning(<rpKey1> ASC NULLS FIRST, <rpKey2> ASC NULLS FIRST, 58), REPARTITION_BY_NUMIt does not affect paths that reuse range bounds already computed by Delta through
ZOrderRules.partExprRule. The benefit approaches zero for narrow inputs or when range key dependencies include most columns.Performance Results
The optimization was evaluated using a Delta Lake liquid-clustering workload consisting of:
OPTIMIZE FULLoperation on a table clustered by a column.The measured physical plan contained this GPU range exchange:
GpuColumnarExchange gpurangepartitioning(<rpKey1> ASC NULLS FIRST, <rpKey2> ASC NULLS FIRST, 58), REPARTITION_BY_NUMThe two
rpKeyexpressions are generated ordering keys, and the exchange creates 58 range partitions.The input contained:
The following configuration was used for both runs:
g4dn.8xlargeGPU executorsspark.sql.files.maxPartitionBytes=2gThe following is a single-run comparison of the GPU range-boundary sampling stage executed by
GpuRangePartitioner.createRangeBounds. These are not end-to-end results or medians.Checklists
Documentation
Testing
(Please provide the names of the existing tests in the PR description.)
Performance