andygrove commented on code in PR #1485:
URL: https://github.com/apache/datafusion-comet/pull/1485#discussion_r1991975853
##########
common/src/main/scala/org/apache/comet/CometConf.scala:
##########
@@ -404,13 +404,15 @@ object CometConf extends ShimCometConf {
"Ensure that Comet shuffle memory overhead factor is a double greater
than 0")
.createWithDefault(1.0)
- val COMET_COLUMNAR_SHUFFLE_UNIFIED_MEMORY_ALLOCATOR_IN_TEST:
ConfigEntry[Boolean] =
- conf("spark.comet.columnar.shuffle.unifiedMemoryAllocatorTest")
- .doc("Whether to use Spark unified memory allocator for Comet columnar
shuffle in tests." +
- "If not configured, Comet will use a test-only memory allocator for
Comet columnar " +
- "shuffle when Spark test env detected. The test-ony allocator is
proposed to run with " +
- "Spark tests as these tests require on-heap memory configuration. " +
- "By default, this config is false.")
+ val COMET_COLUMNAR_SHUFFLE_BOUNDED_MEMORY_ALLOCATOR: ConfigEntry[Boolean] =
Review Comment:
I think that we should revert this config change. The original intent of the
config was to force use of the unified memory allocator in tests even when
running in on-heap mode. I still don't quite understand why we would want to do
this, but the current tests are passing with this approach.
--
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]
