andygrove commented on code in PR #2805:
URL: 
https://github.com/apache/datafusion-ballista/pull/2805#discussion_r4505789717


##########
ballista/core/src/config.rs:
##########
@@ -560,11 +560,12 @@ static CONFIG_ENTRIES: LazyLock<HashMap<String, 
ConfigEntry>> = LazyLock::new(||
         .with_doc_default("(empty)"),
         ConfigEntry::new(
             BALLISTA_SHUFFLE_COMPRESSION_CODEC.to_string(),
-            "Compression codec specification \
-            used in the shuffle process. Possible values: \
-            none, lz4, zstd. Defaults to lz4 to preserve current 
behaviour".to_string(),
+            "Compression codec used for shuffle data written to disk and \
+            sent between executors. Possible values: none, lz4, zstd. \
+            zstd writes shuffle files about half the size of lz4 for little \
+            extra CPU, which matters most when shuffles cross the 
network".to_string(),
             DataType::Utf8,
-            Some("lz4".to_string()),
+            Some("zstd".to_string()),

Review Comment:
   Good call. I compared them with `shuffle_bench` on 60M lineitem rows into 
200 partitions: level 1 gave 2.55 GB, level 3 gave 2.50 GB, and level 1 wrote 
7-10% faster. So level 1 keeps almost all of the size win. 4980ebb68 sets level 
1 explicitly for all shuffle writes, including the pipelined Flight path, and I 
added the numbers to the description.



##########
ballista/core/src/config.rs:
##########
@@ -560,11 +560,12 @@ static CONFIG_ENTRIES: LazyLock<HashMap<String, 
ConfigEntry>> = LazyLock::new(||
         .with_doc_default("(empty)"),
         ConfigEntry::new(
             BALLISTA_SHUFFLE_COMPRESSION_CODEC.to_string(),
-            "Compression codec specification \
-            used in the shuffle process. Possible values: \
-            none, lz4, zstd. Defaults to lz4 to preserve current 
behaviour".to_string(),
+            "Compression codec used for shuffle data written to disk and \
+            sent between executors. Possible values: none, lz4, zstd. \
+            zstd writes shuffle files about half the size of lz4 for little \
+            extra CPU, which matters most when shuffles cross the 
network".to_string(),

Review Comment:
   Yes, that was Comet with its default lz4, so it was comparing codecs and not 
systems. I reworded the description to say so.



-- 
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]

Reply via email to