alamb commented on a change in pull request #1668: URL: https://github.com/apache/arrow-datafusion/pull/1668#discussion_r791020352
########## File path: datafusion/src/execution/context.rs ########## @@ -1245,6 +1286,31 @@ mod tests { use tempfile::TempDir; use test::*; + #[tokio::test] + async fn shared_memory_and_disk_manager() { + // Demonstrate the ability to share DiskManager and + // MemoryManager between two different executions. + let ctx1 = ExecutionContext::new(); + + // configure with same memory / disk manager + let memory_manager = ctx1.runtime_env().memory_manager.clone(); + let disk_manager = ctx1.runtime_env().disk_manager.clone(); + let config = ExecutionConfig::new() + .with_existing_memory_manager(memory_manager.clone()) + .with_existing_disk_manager(disk_manager.clone()); + + let ctx2 = ExecutionContext::with_config(config); + + assert!(std::ptr::eq(Arc::as_ptr(&memory_manager), Arc::as_ptr(&ctx1.runtime_env().memory_manager))); Review comment: Here is the tests for the primary usecase we have in IOx -- sharing `DiskManager` and `MemoryManager`s across plans ########## File path: datafusion/src/execution/memory_manager.rs ########## @@ -463,26 +546,53 @@ mod tests { runtime.register_consumer(&(requester1.clone() as Arc<dyn MemoryConsumer>)); // first requester entered, should be able to use any of the remaining 80 - requester1.do_with_mem(40).await?; - requester1.do_with_mem(10).await?; + requester1.do_with_mem(40).await.unwrap(); + requester1.do_with_mem(10).await.unwrap(); assert_eq!(requester1.get_spills(), 0); assert_eq!(requester1.mem_used(), 50); assert_eq!(*runtime.memory_manager.requesters_total.lock().unwrap(), 50); let requester2 = Arc::new(DummyRequester::new(0, runtime.clone())); runtime.register_consumer(&(requester2.clone() as Arc<dyn MemoryConsumer>)); - requester2.do_with_mem(20).await?; - requester2.do_with_mem(30).await?; + requester2.do_with_mem(20).await.unwrap(); + requester2.do_with_mem(30).await.unwrap(); assert_eq!(requester2.get_spills(), 1); assert_eq!(requester2.mem_used(), 30); - requester1.do_with_mem(10).await?; + requester1.do_with_mem(10).await.unwrap(); assert_eq!(requester1.get_spills(), 1); assert_eq!(requester1.mem_used(), 10); assert_eq!(*runtime.memory_manager.requesters_total.lock().unwrap(), 40); + } - Ok(()) + + #[tokio::test] + #[should_panic(expected = "invalid max_memory. Expected greater than 0, got 0")] Review comment: previously such config errors would `panic` -- 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...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org