[CORE-14250] Ducktape: Reduce cost of datalake/schema_evolution_test.py#28466
Conversation
77a40f6 to
ca16950
Compare
|
/ci-repeat 1 |
ca16950 to
7baf4ed
Compare
|
/ci-repeat 1 |
1 similar comment
|
/ci-repeat 1 |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors datalake/schema_evolution_test.py to improve test efficiency by reducing redundant setup and teardown operations. Instead of parameterizing test functions across schema languages and test cases, the tests now iterate through these parameters within a single test run.
Key changes:
- Consolidates test cases (
LEGAL_TEST_CASESandILLEGAL_TEST_CASESmerged intoTEST_CASES) - Reduces test matrix by eliminating full cartesian product of query engines and catalogs
- Combines previously separate tests (
test_legal_schema_evolution,test_illegal_schema_evolution,test_old_schema_writer) into singletest_schema_evolution
CI test resultstest results on build#76095
test results on build#76172
test results on build#76181
test results on build#76231
|
|
/ci-repeat 1 |
|
/ci-repeat 1 |
Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>
Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>
66c2885 to
3072ed4
Compare
|
Yeah I guess the improvement looks poor (< 2x runtime improvement) compared to the test case reduction (30x !!), because tests got much longer: before all tests were in the ~1:20 range, now one of the remaining 6 is 19 minutes long. Still, what matters is total node time, and looking at the detailed results, that was reduced by a factor of ~5x (the 19 minute run had poor utilization since it was running alone): so that's the overall improvement I would expect here, which is great! |
|
@travisdowns yeah that tracks. I think that's about the best we can do without reducing coverage, which might be fine really but maybe a question for another day.
Sweet, wasn't clear how to calculate that. Where can I find it? |
|
/ci-repeat 1 |
nvartolomei
left a comment
There was a problem hiding this comment.
couple of nits but lgtm overall
| def all_cases(self) -> dict[str, EvolutionTestCase]: | ||
| return TEST_CASES | ||
|
|
||
| def cases_by_modes( |
There was a problem hiding this comment.
nit: had to double check what the function does because it's a class method. I'd move all these to be free-standing functions.
There was a problem hiding this comment.
another reason i double checked is that it reads as if it filters cases by mode (argument) where instead it is cases_grouped_by_mode
nit category too
We don't actually need a cartesian product here, just to make sure that we cover every query engine and catalog type at least once. Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>
Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>
75ab0da
3072ed4 to
75ab0da
Compare
|
/backport v25.3.x |
|
/backport v25.2.x |
|
/backport v25.1.x |
|
/backport v24.3.x |
|
Failed to create a backport PR to v25.2.x branch. I tried: |
|
Failed to create a backport PR to v25.1.x branch. I tried: |
|
Failed to create a backport PR to v24.3.x branch. I tried: |
PR refactors
datalake/schema_evolution_test.pyto cut down on wasted work setting up and tearing down datalake services for every test case. The general idea is to remove test case & schema language parameterization from test functions, instead iterating through them manually, within a single test run.Also changed the way we combine query engines and catalogs. Previously we would parameterize with a cartesian product of query engines & catalog types, resulting in 6 (at the time of this writing) combinations. My claim is that we only really care about running with each QE & catalog at least once, so we can instead generate N combinations thereof where
N = max(n_qe, n_catalog). The effect of this is less pronounced for docker builds, which run exclusively against s3 (minio) due to azurite issues.Other adjustments we could make:
The reports below show a reduction in test time for schema_evolution_test.py running in isolation in release mode w/ otherwise default settings, though ducktape parallelism probably has an effect here.
New build
flatten case & schema type
https://buildkite.com/redpanda/redpanda/builds/76066#019a74ff-2497-4da5-b044-58700827e2c5
also combine old schema and base evo tests
https://buildkite.com/redpanda/redpanda/builds/76080#019a7588-0de9-42c1-ba29-d445e28a7462
and then cut the number of query engine / catalog combos in half
https://buildkite.com/redpanda/redpanda/builds/76092#019a76f6-a2dc-47c8-bc52-370bbfdbdf97
Old build
https://buildkite.com/redpanda/redpanda/builds/76067#019a7500-9b42-4fbd-a01f-7eb5d9d1d978
Backports Required
Release Notes