scylla/3.11.x: fix ShardAwarenessTest patches for Scylla 2025.1#155
Merged
Conversation
1d96f3d to
7ddaa9e
Compare
Collaborator
|
@nikagra , cicd is red |
Contributor
Author
|
@dkropachev I know. It is caused by the yesterday's |
6965248 to
7ddaa9e
Compare
7ddaa9e to
91fb779
Compare
Scylla 2025.1 introduced two changes that broke correctShardInTracingTest: 1. Coordinator-side trace events now run on shard 0 regardless of the data shard (introduced with the new scheduling-group tracing). The old code asserted that *every* trace event's thread ran on the expected shard, which fails for these coordinator events. 2. Thread names now carry a service-level suffix in the form 'shard N/sl:<level>' (e.g. 'shard 0/sl:default'). The old startsWith(shard) assertion fails on this suffix. Fix: narrow the thread-shard assertion to only the 'querying locally' event (the one that actually runs on the data shard), strip any '/sl:...' suffix before comparing, and promote the liveness guard from the no-op assertThat(anyLocal) to assertTrue(anyLocal). For 3.11.4.0 the existing NTS patch hunk is preserved; the new thread-name-fix hunk is prepended.
Both 3.11.4.0 and 3.11.5.15 patches had unconditionally dropped the 'release:' prefix from the CCM version argument. Without this prefix, CCM cannot download Scylla packages from S3 in GitHub Actions (where SCYLLA_UNIFIED_PACKAGE is not set), causing every test to fail with: ccm create ... -v 2026.1.6 --config-dir=... failed to execute Apply the same conditional logic introduced for 4.x patches in PR #150: use 'release:' prefix when SCYLLA_UNIFIED_PACKAGE is absent (GitHub Actions), bare version when it is present (Jenkins with local tarball).
Entries of the form '- ClassName #method' were misleading: YAML treats the '#method' part as an inline comment, so only 'ClassName' is parsed — the method hint was never machine-readable. PR #163 added a validator that raises ValueError on this format (aborts the entire test run). Fix: move the method hint to a separate '# (methodName)' comment line above each entry. The YAML value stays as a bare 'ClassName', the validator is satisfied, and the original class-level exclusion semantics are preserved.
91fb779 to
570ba08
Compare
dkropachev
approved these changes
Jun 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ShardAwarenessTest.correctShardInTracingTestfails on Scylla 2025.1 in thescylla/3.11.4.0andscylla/3.11.5.15matrix jobs with:See companion driver PR for full root-cause analysis: scylladb/java-driver#931
Scope
This PR covers 3.x driver versions only (
3.11.4.0,3.11.5.15). The4.xscylla driver versions are not changed —4.19.2.0requires Java 11+ and cannot run in the current matrix infrastructure (which hardcodes Java 8 inscripts/run_test.sh). That is tracked separately in issue #170 / draft PR #171.Changes
versions/scylla/3.11.4.0/patchReplaces the existing 1-hunk
ShardAwarenessTestdiff with a 2-hunk diff (b1867a0562→0d466c6b2a) that:@@ -82,12 +82,17 @@): narrows assertion to"querying locally"events only, strips/sl:...suffix, and fixes theassertThat(anyLocal)no-op →assertTrue(anyLocal).@@ -95,7 +100,7 @@): upgradesSimpleStrategytoNetworkTopologyStrategyand addsAND TABLETS = {'enabled': false}to disable tablets (line numbers shifted by +5 after hunk 1).Also updates the
CCMBridgegetScyllaVersion()hunk to conditionally apply therelease:prefix only whenSCYLLA_UNIFIED_PACKAGEis not set (matching the pattern from PR #150).versions/scylla/3.11.5.15/patchAdds a 2-hunk
ShardAwarenessTestdiff (ac36feb518→0d466c6b2a):@@ -82,12 +82,17 @@): same assertion fix as above.@@ -95,7 +100,7 @@): addsAND TABLETS = {'enabled': false}to the existingNetworkTopologyStrategykeyspace (NTS is already in the3.11.5.15tag).Also includes the same conditional
release:prefix fix forCCMBridge.versions/scylla/3.11.4.0/ignore.yamlandversions/scylla/3.11.5.15/ignore.yamlFix class-level ignore entries to use bare
- ClassNameformat (without inline#methodcomments) to satisfy the PR #163 validator.Testing
Primary goal —
scylla-2025.1java-driver-matrix-2025.1build #2 — GREEN on Scylla2025.1.15, all tested driver versions.correctShardInTracingTestpasses explicitly for both3.11.4.0and3.11.5.15.No regressions — master
java-driver-matrix-masterbuild #51 confirms:scylla/3.11.5.15scylla/3.11.4.0scylla/4.19.0.9datastax/4.19.3datastax/4.19.2correctShardInTracingTestshows SUCCESS explicitly in the log for both3.11.5.15and3.11.4.0.