Skip to content

Commit 1c8ec9b

Browse files
adriangbclaude
andcommitted
fix: dfbench respects DATAFUSION_RUNTIME_MEMORY_LIMIT env var
When no `--memory-limit` CLI flag is provided, `runtime_env_builder()` now falls back to the `DATAFUSION_RUNTIME_MEMORY_LIMIT` environment variable. This makes the memory pool configuration consistent with how other `DATAFUSION_*` env vars are handled via `SessionConfig::from_env()`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 6713439 commit 1c8ec9b

File tree

1 file changed

+42
-1
lines changed

1 file changed

+42
-1
lines changed

benchmarks/src/util/options.rs

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,15 @@ impl CommonOpt {
9191
pub fn runtime_env_builder(&self) -> Result<RuntimeEnvBuilder> {
9292
let mut rt_builder = RuntimeEnvBuilder::new();
9393
const NUM_TRACKED_CONSUMERS: usize = 5;
94-
if let Some(memory_limit) = self.memory_limit {
94+
// Use CLI --memory-limit if provided, otherwise fall back to
95+
// DATAFUSION_RUNTIME_MEMORY_LIMIT env var
96+
let memory_limit = self.memory_limit.or_else(|| {
97+
std::env::var("DATAFUSION_RUNTIME_MEMORY_LIMIT")
98+
.ok()
99+
.and_then(|val| parse_memory_limit(&val).ok())
100+
});
101+
102+
if let Some(memory_limit) = memory_limit {
95103
let pool: Arc<dyn MemoryPool> = match self.mem_pool_type.as_str() {
96104
"fair" => Arc::new(TrackConsumersPool::new(
97105
FairSpillPool::new(memory_limit),
@@ -138,6 +146,39 @@ fn parse_memory_limit(limit: &str) -> Result<usize, String> {
138146
mod tests {
139147
use super::*;
140148

149+
#[test]
150+
fn test_runtime_env_builder_reads_env_var() {
151+
// Set the env var and verify runtime_env_builder picks it up
152+
// when no CLI --memory-limit is provided
153+
let opt = CommonOpt {
154+
iterations: 3,
155+
partitions: None,
156+
batch_size: None,
157+
mem_pool_type: "fair".to_string(),
158+
memory_limit: None,
159+
sort_spill_reservation_bytes: None,
160+
debug: false,
161+
};
162+
163+
// With env var set, builder should succeed and have a memory pool
164+
// SAFETY: This test is single-threaded and the env var is restored after use
165+
unsafe {
166+
std::env::set_var("DATAFUSION_RUNTIME_MEMORY_LIMIT", "2G");
167+
}
168+
let builder = opt.runtime_env_builder().unwrap();
169+
let runtime = builder.build().unwrap();
170+
unsafe {
171+
std::env::remove_var("DATAFUSION_RUNTIME_MEMORY_LIMIT");
172+
}
173+
// A 2G memory pool should be present — verify it reports the correct limit
174+
match runtime.memory_pool.memory_limit() {
175+
datafusion::execution::memory_pool::MemoryLimit::Finite(limit) => {
176+
assert_eq!(limit, 2 * 1024 * 1024 * 1024);
177+
}
178+
_ => panic!("Expected Finite memory limit"),
179+
}
180+
}
181+
141182
#[test]
142183
fn test_parse_memory_limit_all() {
143184
// Test valid inputs

0 commit comments

Comments
 (0)