viirya commented on code in PR #6943:
URL: https://github.com/apache/datafusion-comet/pull/6943#discussion_r4264131932
##########
spark/src/main/scala/org/apache/comet/CometConf.scala:
##########
@@ -823,6 +823,17 @@ object CometConf extends ShimCometConf {
.intConf
.createWithDefault(1)
+ val COMET_SHUFFLE_COMPRESSION_OFFSETS_AS_LENGTHS: ConfigEntry[Boolean] =
+ conf("spark.comet.shuffle.compression.offsetsAsLengths.enabled")
+ .category(CATEGORY_SHUFFLE)
+ .doc(
+ "Whether Comet shuffle stores the offsets of string, binary, array and
map columns as " +
Review Comment:
Same scope question as in the tuning guide. Since this string becomes the
user-facing description in `configs.md`, could it mention that Celeborn shuffle
and JVM shuffle blocks with dictionary-encoded columns are not affected?
##########
docs/source/user-guide/latest/tuning/shuffle.md:
##########
@@ -115,6 +115,12 @@ both Comet's native shuffle and the JVM columnar shuffle
writer. Supported value
useful when shuffle I/O or network bandwidth is the bottleneck. When ZSTD is
selected, the level is
controlled by `spark.comet.shuffle.compression.zstd.level` (default `1`).
+With zstd, Comet also stores the offsets of string, binary, array and map
columns as lengths before
Review Comment:
This reads as if every zstd shuffle block gets lengths, but two common cases
don't. Celeborn blocks are written as before. On the JVM columnar path, any
column that `spark.comet.shuffle.jvm.preferDictionary.ratio` turns into a
dictionary sends the whole block through `StreamWriter`, so the other string
columns in that block keep their offsets too. Low-cardinality strings, where
the benchmark shows the largest win, are exactly the columns that get
dictionary-encoded there. Could this say it applies to native shuffle and to
JVM shuffle blocks without dictionary-encoded columns, and not to Celeborn?
--
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]
