feat: RR partition with vectorized distribution - #5449
Conversation
|
@andygrove @sunchao need your brain on the below: Although the performance is great, we need to think if this PR addresses task retry correctly, namely: Spark requires round-robin shuffle to be deterministic under task retry. If a retried map Spark's round-robin assignment is positional, not content-based var position = new XORShiftRandom(partitionId).nextInt(numPartitions)
(_: InternalRow) => { position += 1; position } // then HashPartitioner does the modA counter seeded per map task, bumped per row. Placement depends purely on a row's index
Why determinism is requiredA map task can be retried after downstream reducers have already consumed the original How this PR provides determinism
The difference is where the order guarantee comes from:
So the PR provides determinism by assumption, not by construction. It takes Spark's What it does not have is Spark's enforcement. Spark does not trust its upstream, it sorts. |
|
I agree with the retry concern, and I think the performance improvement makes this worth pursuing. One extra detail is that the batch boundaries need to stay the same too. Even with identical row order, Another option is to count rows across incoming batches and send each fixed-size group to the next partition. That would make the assignment independent of how the reader divides rows into batches. We would still need repeatable row order. There’s also a separate issue with the starting partition: the counter receives zero for every task, rather than the Spark input partition ID. If each task produces one batch, everything goes to partition 0. Passing the actual input partition ID through should fix that. Before enabling this more broadly, maybe we should add a test that fails and retries a task after some output has already been consumed, then checks for missing or duplicated rows. I’d also check partition sizes and whole-job runtime, so we know the faster writes aren’t offset by uneven work downstream. |


Which issue does this PR close?
Experiment for #5397
Motivation
Comet implements Spark's round-robin shuffle (
df.repartition(n)) as hash partitioning overevery column of every row.
create_murmur3_hashesrecurses into everyStructchild per row,and the row-level scatter forces
interleave_record_batchto walk every column and nestedchild again on flush. On a wide nested schema (the motivating workload had ~194 nested
columns) this dominates the shuffle write.
Round-robin has no content-based placement requirement. The only reason to hash row content
is determinism under retry, and when the upstream operator emits the same batches in the same
order under retry (Comet's Parquet scan and other order-preserving operators), batch identity
is already a deterministic key.
Changes
CometPartitioning::RoundRobin(usize, RoundRobinStrategy)replacesRoundRobin(usize, usize).Existing behaviour becomes
RoundRobinStrategy::HashAll { max_hash_columns }, still default.RoundRobinStrategy::WholeBatchassigns eachRecordBatchwhole to one partition viaround_robin_batch_seq, seeded with the input partition id so concurrent mappers do not alltarget partition 0 first.
partition_startsis built directly andpartition_row_indicesisleft unmaterialized;
buffer_partitioned_batch_may_spilltakesOption<&[u32]>and treatsNoneas an identity mapping.PartitionedBatchIterator::nextclones the source batch instead of interleaving when a chunkcovers one whole source batch in order. Generic, so
HashAllbenefits opportunistically.spark.comet.shuffle.native.partitioning.roundrobin.batchGranular(defaultfalse),plumbed through
CometNativeShuffleWriterand thebatch_granularprotobuf field intoPhysicalPlanner::create_partitioning.partition_ids,partition_row_indices, ~64 KB/task) no longer allocated forSinglePartitionorWholeBatch.Trade-offs
Distribution is even at batch granularity, not row granularity: fewer batches than partitions
leaves partitions empty, and unequal batch sizes give unequal partitions. Retry safety holds
only under order-preserving upstreams. Hence default
false, documented on bothRoundRobinStrategyand theCometConfentry.HashAllsemantics are unchanged.Testing
test_round_robin_batch_granular_retry_deterministicasserts byte-identical data and indexfiles across two runs over a nested-struct schema, with no rows dropped.
test_round_robin_batch_granular_whole_batch_per_partitionasserts every IPC block holds awhole input batch. Benches in
native/shuffle/benches/shuffle_writer.rscompare bothstrategies end-to-end on a 40-column nested schema, in a partitioning-only microbench, and
over a real parquet clickstream dataset.
MultiPartitionShuffleRepartitioner,ShufflePartitioner,PartitionWriter,LocalPartitionWriter, andShufflePartitionerMetricsarepuband re-exported#[doc(hidden)]for the benches.Flow
flowchart TB subgraph BEFORE["Before: RoundRobin(n, max_hash_columns)"] direction TB A1["input RecordBatch"] --> A2["create_murmur3_hashes<br/>per row, recurses into every Struct child"] A2 --> A3["partition_ids[i] = pmod(hash, n)<br/>+ partition_row_indices"] A3 --> A4["rows scattered across all n partitions"] A4 --> A5["interleave_record_batch<br/>walks every column + nested child"] A5 --> A6["ShuffleBlockWriter"] end subgraph AFTER["After: RoundRobin(n, WholeBatch)"] direction TB B1["input RecordBatch"] --> B2["target = round_robin_batch_seq % n"] B2 --> B3["partition_starts direct<br/>partition_row_indices = None"] B3 --> B4{"chunk = whole source batch in order?"} B4 -- yes --> B5["clone source batch, no interleave"] B4 -- no --> B6["interleave_record_batch"] B5 --> B7["ShuffleBlockWriter"] B6 --> B7 end BEFORE ~~~ AFTERRemaining per-batch work: one modulo, two
resizecalls, and aRecordBatchclone (Arcbumps, not a data copy).