erenavsarogullari commented on code in PR #20375:
URL: https://github.com/apache/datafusion/pull/20375#discussion_r2876107961
##########
datafusion/execution/src/disk_manager.rs:
##########
@@ -420,7 +420,8 @@ impl RefCountedTempFile {
let global_disk_usage =
self.disk_manager.used_disk_space.load(Ordering::Relaxed);
if global_disk_usage > self.disk_manager.max_temp_directory_size {
return resources_err!(
- "The used disk space during the spilling process has exceeded
the allowable limit of {}. Try increasing the `max_temp_directory_size` in the
disk manager configuration.",
+ "The used disk space during the spilling process has exceeded
the allowable limit of {}. \
+ Please try increasing the config:
`datafusion.runtime.max_temp_directory_size`.",
Review Comment:
Does `centralizing` mean to be defined as `constant` (e.g: under
`runtime_env.rs`)? If so, AFAIS, DF configs do not have constant reference for
config names and i think it can be useful to avoid config name duplication and
drift risk as you mentioned.
Also, can `version` property per `ConfigEntry` be useful by showing when the
config is added (e.g: `53.0.0`)? Spark uses `version` property per config as
[reference](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala#L247).
##########
datafusion/core/tests/memory_limit/mod.rs:
##########
@@ -602,10 +602,10 @@ async fn test_disk_spill_limit_reached() -> Result<()> {
.await
.unwrap();
- let err = df.collect().await.unwrap_err();
- assert_contains!(
- err.to_string(),
- "The used disk space during the spilling process has exceeded the
allowable limit"
+ let error_message = df.collect().await.unwrap_err().to_string();
+ assert!(
+ error_message.contains("The used disk space during the spilling
process has exceeded the allowable limit") &&
+
error_message.contains("datafusion.runtime.max_temp_directory_size"),
);
Review Comment:
Yes, expected error message substrings can be iterated and addressed.
--
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]
