Skip to content

Commit 0283077

Browse files
authored
Test: Validate memory limit for sort queries to extended test (apache#14142)
* External memory limit validation for sort * add bug tracker * cleanup * Update submodule * reviews * fix CI * move feature to module level
1 parent e9a77e0 commit 0283077

File tree

8 files changed

+475
-3
lines changed

8 files changed

+475
-3
lines changed

.github/workflows/extended.yml

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,42 @@ on:
3333
- main
3434

3535
jobs:
36+
# Check crate compiles and base cargo check passes
37+
linux-build-lib:
38+
name: linux build test
39+
runs-on: ubuntu-latest
40+
container:
41+
image: amd64/rust
42+
steps:
43+
- uses: actions/checkout@v4
44+
- name: Setup Rust toolchain
45+
uses: ./.github/actions/setup-builder
46+
with:
47+
rust-version: stable
48+
- name: Prepare cargo build
49+
run: cargo check --profile ci --all-targets
50+
51+
# Run extended tests (with feature 'extended_tests')
52+
linux-test-extended:
53+
name: cargo test (amd64)
54+
needs: linux-build-lib
55+
runs-on: ubuntu-latest
56+
container:
57+
image: amd64/rust
58+
steps:
59+
- uses: actions/checkout@v4
60+
with:
61+
submodules: true
62+
fetch-depth: 1
63+
- name: Setup Rust toolchain
64+
uses: ./.github/actions/setup-builder
65+
with:
66+
rust-version: stable
67+
- name: Run tests (excluding doctests)
68+
run: cargo test --profile ci --exclude datafusion-examples --exclude datafusion-benchmarks --workspace --lib --tests --bins --features avro,json,backtrace,extended_tests
69+
- name: Verify Working Directory Clean
70+
run: git diff --exit-code
71+
3672
# Check answers are correct when hash values collide
3773
hash-collisions:
3874
name: cargo test hash collisions (amd64)
@@ -51,7 +87,8 @@ jobs:
5187
- name: Run tests
5288
run: |
5389
cd datafusion
54-
cargo test --profile ci --exclude datafusion-examples --exclude datafusion-benchmarks --exclude datafusion-sqllogictest --workspace --lib --tests --features=force_hash_collisions,avro
90+
cargo test --profile ci --exclude datafusion-examples --exclude datafusion-benchmarks --exclude datafusion-sqllogictest --workspace --lib --tests --features=force_hash_collisions,avro,extended_tests
91+
5592
sqllogictest-sqlite:
5693
name: "Run sqllogictests with the sqlite test suite"
5794
runs-on: ubuntu-latest

datafusion/core/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ unicode_expressions = [
8080
"datafusion-sql/unicode_expressions",
8181
"datafusion-functions/unicode_expressions",
8282
]
83+
extended_tests = []
8384

8485
[dependencies]
8586
apache-avro = { version = "0.17", optional = true }
@@ -150,6 +151,7 @@ rand_distr = "0.4.3"
150151
regex = { workspace = true }
151152
rstest = { workspace = true }
152153
serde_json = { workspace = true }
154+
sysinfo = "0.33.1"
153155
test-utils = { path = "../../test-utils" }
154156
tokio = { workspace = true, features = ["rt-multi-thread", "parking_lot", "fs"] }
155157

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
//! Validates query's actual memory usage is consistent with the specified memory
19+
//! limit.
20+
21+
mod sort_mem_validation;
22+
mod utils;
Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,223 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
//! Memory limit validation tests for the sort queries
19+
//!
20+
//! These tests must run in separate processes to accurately measure memory usage.
21+
//! This file is organized as:
22+
//! - Test runners that spawn individual test processes
23+
//! - Test cases that contain the actual validation logic
24+
use std::{process::Command, str};
25+
26+
use log::info;
27+
28+
use crate::memory_limit::memory_limit_validation::utils;
29+
30+
// ===========================================================================
31+
// Test runners:
32+
// Runners are splitted into multiple tests to run in parallel
33+
// ===========================================================================
34+
35+
#[test]
36+
fn memory_limit_validation_runner_works_runner() {
37+
spawn_test_process("memory_limit_validation_runner_works");
38+
}
39+
40+
#[test]
41+
fn sort_no_mem_limit_runner() {
42+
spawn_test_process("sort_no_mem_limit");
43+
}
44+
45+
#[test]
46+
fn sort_with_mem_limit_1_runner() {
47+
spawn_test_process("sort_with_mem_limit_1");
48+
}
49+
50+
#[test]
51+
fn sort_with_mem_limit_2_runner() {
52+
spawn_test_process("sort_with_mem_limit_2");
53+
}
54+
55+
#[test]
56+
fn sort_with_mem_limit_3_runner() {
57+
spawn_test_process("sort_with_mem_limit_3");
58+
}
59+
60+
#[test]
61+
fn sort_with_mem_limit_2_cols_1_runner() {
62+
spawn_test_process("sort_with_mem_limit_2_cols_1");
63+
}
64+
65+
#[test]
66+
fn sort_with_mem_limit_2_cols_2_runner() {
67+
spawn_test_process("sort_with_mem_limit_2_cols_2");
68+
}
69+
70+
/// Helper function that executes a test in a separate process with the required environment
71+
/// variable set. Memory limit validation tasks need to measure memory resident set
72+
/// size (RSS), so they must run in a separate process.
73+
fn spawn_test_process(test: &str) {
74+
let test_path = format!(
75+
"memory_limit::memory_limit_validation::sort_mem_validation::{}",
76+
test
77+
);
78+
info!("Running test: {}", test_path);
79+
80+
// Run the test command
81+
let output = Command::new("cargo")
82+
.arg("test")
83+
.arg("--package")
84+
.arg("datafusion")
85+
.arg("--test")
86+
.arg("core_integration")
87+
.arg("--features")
88+
.arg("extended_tests")
89+
.arg("--")
90+
.arg(&test_path)
91+
.arg("--exact")
92+
.arg("--nocapture")
93+
.env("DATAFUSION_TEST_MEM_LIMIT_VALIDATION", "1")
94+
.output()
95+
.expect("Failed to execute test command");
96+
97+
// Convert output to strings
98+
let stdout = str::from_utf8(&output.stdout).unwrap_or("");
99+
let stderr = str::from_utf8(&output.stderr).unwrap_or("");
100+
101+
info!("{}", stdout);
102+
103+
assert!(
104+
output.status.success(),
105+
"Test '{}' failed with status: {}\nstdout:\n{}\nstderr:\n{}",
106+
test,
107+
output.status,
108+
stdout,
109+
stderr
110+
);
111+
}
112+
113+
// ===========================================================================
114+
// Test cases:
115+
// All following tests need to be run through their individual test wrapper.
116+
// When run directly, environment variable `DATAFUSION_TEST_MEM_LIMIT_VALIDATION`
117+
// is not set, test will return with a no-op.
118+
//
119+
// If some tests consistently fail, suppress by setting a larger expected memory
120+
// usage (e.g. 80_000_000 * 3 -> 80_000_000 * 4)
121+
// ===========================================================================
122+
123+
/// Test runner itself: if memory limit violated, test should fail.
124+
#[tokio::test]
125+
async fn memory_limit_validation_runner_works() {
126+
if std::env::var("DATAFUSION_TEST_MEM_LIMIT_VALIDATION").is_err() {
127+
println!("Skipping test because DATAFUSION_TEST_MEM_LIMIT_VALIDATION is not set");
128+
129+
return;
130+
}
131+
132+
let result = std::panic::catch_unwind(|| {
133+
tokio::runtime::Runtime::new().unwrap().block_on(async {
134+
utils::validate_query_with_memory_limits(
135+
20_000_000, // set an impossible limit: query requires at least 80MB
136+
None,
137+
"select * from generate_series(1,10000000) as t1(c1) order by c1",
138+
"select * from generate_series(1,1000000) as t1(c1) order by c1", // Baseline query with ~10% of data
139+
)
140+
.await;
141+
})
142+
});
143+
144+
assert!(
145+
result.is_err(),
146+
"Expected the query to panic due to memory limit"
147+
);
148+
}
149+
150+
#[tokio::test]
151+
async fn sort_no_mem_limit() {
152+
utils::validate_query_with_memory_limits(
153+
80_000_000 * 3,
154+
None,
155+
"select * from generate_series(1,10000000) as t1(c1) order by c1",
156+
"select * from generate_series(1,1000000) as t1(c1) order by c1", // Baseline query with ~10% of data
157+
)
158+
.await;
159+
}
160+
161+
#[tokio::test]
162+
async fn sort_with_mem_limit_1() {
163+
utils::validate_query_with_memory_limits(
164+
40_000_000 * 5,
165+
Some(40_000_000),
166+
"select * from generate_series(1,10000000) as t1(c1) order by c1",
167+
"select * from generate_series(1,1000000) as t1(c1) order by c1", // Baseline query with ~10% of data
168+
)
169+
.await;
170+
}
171+
172+
#[tokio::test]
173+
async fn sort_with_mem_limit_2() {
174+
utils::validate_query_with_memory_limits(
175+
80_000_000 * 3,
176+
Some(80_000_000),
177+
"select * from generate_series(1,10000000) as t1(c1) order by c1",
178+
"select * from generate_series(1,1000000) as t1(c1) order by c1", // Baseline query with ~10% of data
179+
)
180+
.await;
181+
}
182+
183+
#[tokio::test]
184+
async fn sort_with_mem_limit_3() {
185+
utils::validate_query_with_memory_limits(
186+
80_000_000 * 3,
187+
Some(80_000_000 * 10), // mem limit is large enough so that no spill happens
188+
"select * from generate_series(1,10000000) as t1(c1) order by c1",
189+
"select * from generate_series(1,1000000) as t1(c1) order by c1", // Baseline query with ~10% of data
190+
)
191+
.await;
192+
}
193+
194+
#[tokio::test]
195+
async fn sort_with_mem_limit_2_cols_1() {
196+
let memory_usage_in_theory = 80_000_000 * 2; // 2 columns
197+
let expected_max_mem_usage = memory_usage_in_theory * 4;
198+
utils::validate_query_with_memory_limits(
199+
expected_max_mem_usage,
200+
None,
201+
"select c1, c1 as c2 from generate_series(1,10000000) as t1(c1) order by c2 DESC, c1 ASC NULLS LAST",
202+
"select c1, c1 as c2 from generate_series(1,1000000) as t1(c1) order by c2 DESC, c1 ASC NULLS LAST", // Baseline query with ~10% of data
203+
)
204+
.await;
205+
}
206+
207+
// TODO: Query fails, fix it
208+
// Issue: https://github.com/apache/datafusion/issues/14143
209+
#[ignore]
210+
#[tokio::test]
211+
async fn sort_with_mem_limit_2_cols_2() {
212+
let memory_usage_in_theory = 80_000_000 * 2; // 2 columns
213+
let expected_max_mem_usage = memory_usage_in_theory * 3;
214+
let mem_limit = memory_usage_in_theory as f64 * 0.5;
215+
216+
utils::validate_query_with_memory_limits(
217+
expected_max_mem_usage,
218+
Some(mem_limit as i64),
219+
"select c1, c1 as c2 from generate_series(1,10000000) as t1(c1) order by c2 DESC, c1 ASC NULLS LAST",
220+
"select c1, c1 as c2 from generate_series(1,1000000) as t1(c1) order by c2 DESC, c1 ASC NULLS LAST", // Baseline query with ~10% of data
221+
)
222+
.await;
223+
}

0 commit comments

Comments
 (0)