wiedld commented on code in PR #11665:
URL: https://github.com/apache/datafusion/pull/11665#discussion_r1692420852


##########
datafusion/execution/src/memory_pool/pool.rs:
##########
@@ -311,4 +423,56 @@ mod tests {
         let err = r4.try_grow(30).unwrap_err().strip_backtrace();
         assert_eq!(err, "Resources exhausted: Failed to allocate additional 30 
bytes for s4 with 0 bytes already allocated - maximum available is 20");
     }
+
+    #[test]
+    fn test_tracked_consumers_pool() {
+        let pool: Arc<dyn MemoryPool> = Arc::new(TrackConsumersPool::new(
+            GreedyMemoryPool::new(100),
+            NonZeroUsize::new(3).unwrap(),
+        ));
+
+        // Test: see error message when no consumers recorded yet
+        let mut r0 = MemoryConsumer::new("r0").register(&pool);
+        let expected = "Failed to allocate additional 150 bytes for r0 with 0 
bytes already allocated - maximum available is 100. The top memory consumers 
(across reservations) are: r0 consumed 0 bytes";

Review Comment:
   With the [proposed 
change](https://github.com/apache/datafusion/pull/11665/files#r1692419610) in a 
follow PR, the final error message would read:
   
   `Failed to allocate additional 150 bytes for r0 with 0 bytes already 
allocated for this reservation - 100 bytes remain available for the total pool. 
The top memory consumers (across reservations) are: r0 consumed 0 bytes`



##########
datafusion/execution/src/memory_pool/pool.rs:
##########
@@ -231,6 +235,10 @@ impl MemoryPool for FairSpillPool {
     }
 }
 
+/// Constructs a resources error based upon the individual 
[`MemoryReservation`].
+///
+/// The error references the total bytes affiliated with the reservation's
+/// [`MemoryConsumer`], and not the total within the collective [`MemoryPool`].
 #[inline(always)]
 fn insufficient_capacity_err(

Review Comment:
   In a follow up PR, I would like to iterate on this error message.
   
   Take the original/current:
   `Failed to allocate additional {} bytes for {} with {} bytes already 
allocated - maximum available is {}`
   
   And change into something like:
   `Failed to allocate additional {} bytes for {} with {} bytes already 
allocated for this reservation - {} bytes remain available for the total pool`



-- 
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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to