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