kazuyukitanimura commented on code in PR #6943:
URL: https://github.com/apache/datafusion-comet/pull/6943#discussion_r4265128193
##########
docs/source/contributor-guide/native_shuffle.md:
##########
Review Comment:
Could you add offsets_as_lengths.rs to this table? It now defines part of
the block format (buffer roles, the byte-plane layout and LENGTHS_TAG_FLAG), so
someone reading the "Rust Side" table to find the format code should see it
next to shuffle_block_writer.rs and ipc.rs.
##########
spark/src/test/scala/org/apache/comet/exec/CometNativeShuffleSuite.scala:
##########
@@ -1563,6 +1563,30 @@ class CometNativeShuffleSuite extends CometTestBase with
AdaptiveSparkPlanHelper
}
}
+ test("native shuffle stores offsets as lengths only with zstd") {
Review Comment:
One thing I noticed is that every other Comet suite and the Spark SQL run
use the default lz4, so these two tests are the only place a flagged block goes
end to end through the writer, the spill, the fetch and the decode. Could the
data include nulls, empty strings, a binary column and a struct with a string
field? Could the answer check also run once with spilling on, for example the
columnar suite's default spill threshold and a small
spark.comet.shuffle.native.maxBufferBytes for native? The byte-count comparison
can keep the higher threshold. Alternatively, a one-off local run of
CometNativeShuffleSuite and CometShuffleSuite with
spark.comet.shuffle.compression.codec=zstd, with the result posted here, would
give similar confidence. The PR description mentions you ran something like
that before narrowing this to zstd.
--
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]
