ump45nose commented on code in PR #7104:
URL: https://github.com/apache/datafusion-comet/pull/7104#discussion_r4326156002


##########
native/jni-bridge/src/errors.rs:
##########
@@ -808,18 +808,54 @@ fn generic_exception(error: &CometError, backtrace: 
Option<&str>) -> JvmExceptio
 /// message as a stack trace when available.
 fn natural_exception(error: &CometError, backtrace: Option<&str>) -> 
JvmException<'static> {
     let exception = error.to_exception();
+    let msg = with_comet_config_hint(exception.msg);
     let message = match backtrace {
-        Some(backtrace_string) => {
-            to_stacktrace_string(exception.msg, 
backtrace_string.to_string()).unwrap()
-        }
-        None => exception.msg,
+        Some(backtrace_string) => to_stacktrace_string(msg, 
backtrace_string.to_string()).unwrap(),
+        None => msg,
     };
     JvmException::New {
         class: exception.class,
         message,
     }
 }
 
+/// The config key DataFusion's spill-limit error tells users to raise. Comet 
sets the limit
+/// from `spark.comet.maxTempDirectorySize` instead, and DataFusion raises the 
error as a plain
+/// `std::io::Error`, so the key in the message text is the only thing to 
match on.
+const DATAFUSION_SPILL_LIMIT_KEY: &str = 
"`datafusion.runtime.max_temp_directory_size`";
+
+const SPILL_LIMIT_HINT: &str = "Comet does not use `datafusion.runtime.*` 
settings: \
+     the spill limit is `spark.comet.maxTempDirectorySize`. It applies to each 
Comet native plan \
+     separately, so an executor can use several times that much disk.";
+
+/// The first config key DataFusion's external-sort OOM error tells users to 
change. Comet never
+/// uses it, and the sort is identified only by the `Context` string around 
`ResourcesExhausted`,
+/// so this quoted text is what to match on.
+const DATAFUSION_SORT_MEMORY_KEY: &str = "'datafusion.runtime.memory_limit'";
+
+const SORT_MEMORY_HINT: &str = "Comet does not use 
`datafusion.runtime.memory_limit` or \

Review Comment:
   Thanks. I dropped the three `SORT_MEMORY_HINT.contains` checks and kept the 
default `sort_spill_reservation_bytes` assertion in 0c8c753, and updated the PR 
description to match the new hint.
   



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