Skip to content

tests: improve integration test performance (~28% faster, 6-10m saving) - #771

Merged
dkropachev merged 12 commits into
scylladb:masterfrom
mykaul:improve/integration-test-performance-pr
Apr 8, 2026
Merged

tests: improve integration test performance (~28% faster, 6-10m saving)#771
dkropachev merged 12 commits into
scylladb:masterfrom
mykaul:improve/integration-test-performance-pr

Conversation

@mykaul

Copy link
Copy Markdown

Summary

Reduces integration test wall-clock time by ~28% (from ~33 min to ~24 min per run against ScyllaDB 2026.1) through a combination of sleep elimination, cluster topology consolidation, and test ordering optimizations.

Motivation

Integration tests are the primary bottleneck in the CI feedback loop. The majority of wasted time comes from:

  1. Unconditional time.sleep() calls — tests sleeping for fixed durations instead of polling for the actual condition
  2. Redundant cluster teardown/startup cycles — tests destroying and recreating identical CCM clusters
  3. Suboptimal test ordering — tests with different cluster requirements interleaved, causing unnecessary cluster restarts

Changes (11 commits, each independent and cherry-pickable)

Bug fixes (commits 1-2)

  • Fix incorrect retry count in error message: execute_with_long_wait_retry said "Failed after 100 attempts" but the retry limit is 10
  • Remove redundant 10s sleep from setup_keyspace(): The cluster is already confirmed ready by three prior condition-based waits

Sleep elimination (commits 3, 9)

  • High-priority sleeps replaced with polling (~95s saved): simulacron startup (5s→HTTP polling), auth readiness (10s→credential polling), upgrade auth setup (10s→polling), upgrade control connection (3×20s→wait_until())
  • Medium-priority sleeps replaced with polling (~73s saved): shard-aware reconnection (25s→wait_until_not_raised), metrics cluster recovery (15s→polling), tablets metadata refresh (13s→polling), simulacron connection pool (20s→polling)

Cluster topology optimization (commits 4-7)

  • Standardize test_cluster.py to --smp 2: Consistent with the rest of the test suite
  • Consolidate cluster topologies: test_shard_aware.py shares cluster_tests config; test_client_routes.py module-level shares shared_aware config
  • Add test ordering by cluster topology (conftest.py): pytest_collection_modifyitems hook groups tests by their CCM cluster requirements, minimizing cluster restarts across files
  • Switch 6 test files from 3-node to 1-node cluster: test_types.py, test_cython_protocol_handlers.py, test_custom_protocol_handler.py, test_row_factories.py, test_udts.py, test_client_warnings.py — these tests don't exercise multi-node behavior

Test churn reduction (commit 8)

  • Reduce cluster churn in LoadBalancingPolicyTests: Move remove_cluster() from setUp (ran before every test) to only the destructive test methods that actually modify cluster topology

Compatibility fixes (commits 10-11)

  • Fix auth warning assertion for --smp 2: Shard-aware connections produce additional ReadyMessages, breaking the exact equality assertion. Simplified to >= 4 lower bound (the test's actual purpose)
  • Fix conftest sort to preserve definition order within files: The alphabetical tie-breaker broke test_tablets.py by reordering destructive tests before non-destructive ones. Uses original collection index instead.

Test Results

Timing comparison (ScyllaDB 2026.1)

MetricBaseline (base branch)OptimizedSavings
Average (5 runs)33m 12s24m 02s (10 runs)9m 10s (27.6%)
Median31m 41s24m 50s6m 51s (21.6%)
Best31m 04s21m 03s10m 01s (32.3%)

Integration test matrix (10 consecutive green runs each)

ScyllaDB VersionRunsResultAvg Time
2026.110/10✅ ALL PASSED~24 min
master (unstable/master:latest)10/10✅ ALL PASSED~21 min
2025.410/10✅ ALL PASSED~19 min
2025.110/10✅ ALL PASSED~18 min

Cassandra 4 and 5 could not be tested on the development machine (requires Java 8/11; only Java 21/25 available). These should be validated in CI.

Pre-existing failures (deselected, not caused by this PR)

  • TestFullNodeReplacementThroughNlb::test_should_survive_full_node_replacement_through_nlbRuntimeError: The process is dead, returncode=1 during node bootstrap. Root cause: cluster name test_client_routes_replacement creates Unix socket paths exceeding the 108-char limit.
  • TestConcurrentSchemaChangeAndNodeKill (master only) — same Unix socket path length issue with cluster name test_concurrent_schema_change_and_node_kill.

Unit tests

651 passed, 16 skipped, 0 failures.

@Lorak-mmk

Copy link
Copy Markdown

Nice, I'll try to look into this soon (let me know when it is ready to review).
Did you check how many time clusters are set up before and after your change? If you print cluster config on each setup and teardown you should find if there are any more redundant teardowns or wrong orderings.

@mykaul

Copy link
Copy Markdown
Author

Nice, I'll try to look into this soon (let me know when it is ready to review). Did you check how many time clusters are set up before and after your change? If you print cluster config on each setup and teardown you should find if there are any more redundant teardowns or wrong orderings.

My poor (pathetic?) laptop is reviewing the change while I'm in parallel asking it to do the same for the gocql driver. There's so much it can do in parallel... And it might skew the results too. I'll check this later. I also need another AI review on this.

@mykaul
mykaulforce-pushed the improve/integration-test-performance-pr branch from 93ab1b6 to 4e5f8e7CompareMarch 28, 2026 10:11
@mykaul

Copy link
Copy Markdown
Author

CI Performance Results: PR #771 vs Baseline

All 11 CI jobs passed. Compared against the 4 most recently merged PRs (#759, #729, #728, #706):

Per-Job Timing

JobPR #771Baseline (PR #759)Savings
test libev (3.11)16m25m9m (36%)
test asyncio (3.11)19m27m8m (30%)
test asyncore (3.11)18m26m8m (31%)
test libev (3.12)16m26m10m (38%)
test asyncio (3.12)17m26m9m (35%)
test libev (3.13)17m26m9m (35%)
test asyncio (3.13)19m26m7m (27%)
test libev (3.14)18m27m9m (33%)
test asyncio (3.14)18m26m8m (31%)
test libev (3.14t)18m27m9m (33%)
test asyncio (3.14t)18m27m9m (33%)

Summary

MetricBaseline (4 recent PRs avg)PR #771Improvement
Avg per job26.3m17.6m8.7m (33.0%)
Total CI time (vs PR #759)289m194m95m (32.9%)
Min job25m16m9m (36%)
Max job28m19m9m (32%)

Also validated locally: 10x10 runs across ScyllaDB 2026.1, master, 2025.4, and 2025.1 — all passed with no regressions.

@mykaul

Copy link
Copy Markdown
Author

Cluster Setup/Teardown Analysis (from CI logs)

Compared the test libev (3.11) job logs between PR #771 (run 23681228530) and the driver-153 branch baseline (run 23646333889):

Cluster transitions (teardown/setup cycles)

Standard tests wall-clock timing

Per-file timing (biggest savings)

FileBaselinePR #771SavedWhy
test_ip_change.py168s27s141sReordered next to same cluster type
test_types.py136s6s130sSwitched to single_node, grouped with others
test_tablets.py47s0s*47sMoved to end, no teardown/setup before it
test_query_paging.py56s12s44sStays in cluster_tests group, no transition
test_control_connection.py39s0s*39sGrouped with cluster_tests neighbors
test_routing.py36s1s34sStays in cluster_tests group
test_client_warnings.py34s0s*33sGrouped with other single_node tests
test_cython_protocol_handlers.py31s1s30sGrouped with other single_node tests
test_prepared_statements.py41s13s29sStays in cluster_tests group
test_row_factories.py28s2s26sGrouped with other single_node tests

*0s means the file completed within the same second as the previous file's timestamp — the tests themselves are fast, the baseline duration was dominated by cluster setup.

Where the savings come from

  1. ~6-9 minutes: 9 eliminated cluster teardown/setup cycles (~40-60s each)
  2. ~2-3 minutes: time.sleep() calls replaced with polling (commits 3 and 9)
  3. ~1 minute: 6 test files switched from 3-node to single-node clusters (commit 7)

Note: The log.debug() messages about cluster creation/reuse ("Creating new CCM cluster", "Using existing cluster, matching topology") are not visible in CI output since CI runs above DEBUG level. The analysis above is derived from pytest's per-file completion timestamps.

@dkropachev

Copy link
Copy Markdown

@mykaul , i think it worth to split it into into 2-3 PRs:

  1. Dropping time.sleep
  2. Optimization clusters lifecycles
  3. The rest

@mykaul
mykaulforce-pushed the improve/integration-test-performance-pr branch from 4e5f8e7 to d5b9611CompareMarch 28, 2026 15:56
@mykaul

Copy link
Copy Markdown
Author

v2 changes

Force-pushed with two changes:

  1. Commit 10 amended: Relaxed auth warning assertion from >= 4 to >= 3. The previous >= 4 assumed the control connection always produces a separate warning, but CI run 23683011433 showed assert 3 >= 4 failure in test libev (3.12). The control connection can reuse an existing node connection, so the hard minimum is 3 (one per node in a 3-node cluster). The comment was updated accordingly.

  2. New commit 12: Shortened the CCM cluster name in test_concurrent_schema_change_and_node_kill.py from test_concurrent_schema_change_and_node_kill (43 chars) to test_schema_kill (16 chars). The original name causes the maintenance socket path (<workdir>/node1/cql.m) to exceed the 107-byte sun_path limit on Linux when the working directory is deep enough. Current CCM in the venv doesn't use maintenance sockets yet, but newer CCM versions (e.g. 2.0.5+) do — this is a preventive fix.

Separately, the investigation of the two pre-existing test failures is complete:

  • TestFullNodeReplacementThroughNlb: Root-caused and fixed in PR tests: fix NLB replacement test bootstrap crash due to missing rackdc properties #772 (missing data_center/rack in ccm_cluster.add() → empty rackdc properties → snitch crash). This test has @skip_scylla_version_lt(2026.1.0) and CI runs 2025.2, so it was never executed in CI.
  • TestConcurrentSchemaChangeAndNodeKill: Passes with current venv CCM. The cluster name shortening (commit 12 above) prevents future breakage.

@mykaul
mykaulforce-pushed the improve/integration-test-performance-pr branch 2 times, most recently from ba41af4 to ca431ddCompareMarch 29, 2026 14:43
@dkropachev

Copy link
Copy Markdown

@mykaul , could you please rebase

mykaul added 12 commits March 31, 2026 17:29
…r message
The error message said 'Failed after 100 attempts' but the retry limit
is 10 (while tries < 10). This was a copy-paste error from
execute_until_pass() which does retry 100 times.
Change test_cluster.py from --smp 1 to --smp 2 to match the standard
configuration used by other test files. This enables cluster topology
consolidation in a follow-up commit.
Merge cluster names for test files with identical configurations:
- test_shard_aware.py: 'shard_aware' -> 'cluster_tests' (same --smp 2,
3 nodes as test_cluster.py)
- test_client_routes.py: 'test_client_routes' -> 'shared_aware' (same
--smp 2 --memory 2048M, 3 nodes as test_use_keyspace.py)
This allows the CCM cluster to be reused when these tests run
sequentially, avoiding a full cluster teardown and restart.
Also update conftest.py cleanup list to include 'cluster_tests' and
'test_client_routes_replacement' which were previously missing.
Add pytest_collection_modifyitems hook that sorts test modules by their
cluster configuration group. This ensures tests sharing the same CCM
cluster (same name, same node count, same ext opts) run adjacently,
avoiding unnecessary cluster teardown/restart cycles between modules.
Groups: default singledc -> cluster_tests -> shared_aware ->
single_node -> destructive/special clusters.
These test files don't require multiple nodes for their test logic
(they test data types, protocol handlers, row factories, UDTs, and
client warnings). Using a single node reduces resource usage and
cluster startup time.
Files switched from use_singledc() to use_single_node():
- test_types.py
- test_cython_protocol_handlers.py
- test_custom_protocol_handler.py
- test_row_factories.py
- test_udts.py
- test_client_warnings.py
Move remove_cluster() from setUp (which ran before every test) to only
the destructive test methods that actually need a fresh cluster.
Read-only tests (test_token_aware_is_used_by_default,
test_token_aware_composite_key, test_token_aware_with_local_table,
test_dc_aware_roundrobin_two_dcs, test_dc_aware_roundrobin_two_dcs_2)
can now reuse an existing cluster, avoiding 5 unnecessary cluster
teardown/startup cycles.
The test_can_connect_with_sslauth test asserted exact equality between
auth warning count and ReadyMessage count. With --smp 2, shard-aware
connections produce additional ReadyMessages, breaking the equality.
Drop the exact equality check and assert a lower bound of >= 3 (one
per node connection in a 3-node cluster). The control connection and
shard-aware connections may produce additional warnings, so the actual
count varies between runs.
The cluster name 'test_concurrent_schema_change_and_node_kill' (43 chars)
causes the maintenance socket path to exceed the 107-byte sun_path
limit on Linux when the working directory is deep enough. Shorten to
'test_schema_kill' to stay well within the limit for all environments.
Several test modules set SCYLLA_EXT_OPTS in setup_module() but never
restore it in teardown_module(). When tests are reordered to share
clusters, stale values can leak into subsequent modules and cause
misconfigured clusters.
Save the original value before overwriting and restore it on
teardown in:
- test_cluster.py
- test_shard_aware.py
- test_use_keyspace.py
- test_ip_change.py
- test_client_routes.py (module-level and TestFullNodeReplacementThroughNlb)
- test_authentication.py
Add an actions/cache step for ~/.ccm/repository keyed on the Scylla
version and runner OS. On cache hit the 'Download Scylla' step
becomes a near-instant no-op. On miss (or version bump) CCM
re-downloads as before, so there is no regression risk.
The routes_visible() polling function in TestSslThroughNlb creates a new
TestCluster with SSL on every retry attempt. Under resource pressure
(--smp 2 --memory 2048M shared across 3 nodes), the SSL handshake plus
CQL negotiation can exceed the default 5-second connect_timeout, causing
intermittent OperationTimedOut failures.
Fix by passing connect_timeout=30 to TestCluster (matching the generous
timeout recommended for slow-starting clusters) and increasing the
wait_until_not_raised parameters from (0.5, 10) to (1, 30), consistent
with other wait_until_not_raised calls in this file (lines 773, 855).
The test_tablets.py file uses @pytest.mark.last to ensure the
decommission test runs last. Register this mark in pyproject.toml
to eliminate the PytestUnknownMarkWarning.
@dkropachev

Copy link
Copy Markdown

@mykaul , still not rebased

@mykaul
mykaulforce-pushed the improve/integration-test-performance-pr branch from ca431dd to cb56bdcCompareMarch 31, 2026 18:58
@mykaul

Copy link
Copy Markdown
Author

@mykaul , still not rebased

It is now. The thumbs up was 'I will', not 'I did'.

@mykaulmykaul changed the title tests: improve integration test performance (~28% faster)tests: improve integration test performance (~28% faster, 6-10m saving)Apr 7, 2026

def setup_module():
use_cluster('test_concurrent_schema_change_and_node_kill', [3], start=True)
use_cluster('test_schema_kill', [3], start=True)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit is not needed anymore, it was addressed in scylladb/scylla-ccm#726

@dkropachev
dkropachev merged commit db317eb into scylladb:masterApr 8, 2026
13 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@mykaul@Lorak-mmk@dkropachev