Skip to content

feature(c-s): c-s v4 on master and correct driver version reporting #13254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
CodeLieutenant wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feature(c-s): c-s v4 on master and correct driver version reporting #13254

CodeLieutenant wants to merge 1 commit into from

Conversation

@CodeLieutenant
Copy link
Contributor

@CodeLieutenant CodeLieutenant commented Jan 19, 2026 •
edited
Loading

This PR upgrades to the latest c-s version with a patch to Java v4 driver (the timeout issue when creating keyspaces). Also supports correctly the Java V4 driver to Argus when the driver is selected.

Testing

PR pre-checks (self review)

  • I added the relevant backport labels
  • I didn't leave commented-out/debugging code

Reminders

  • Add New configuration option and document them (in sdcm/sct_config.py)
  • Add unit tests to cover my changes (under unit-test/ folder)
  • Update the Readme/doc folder relevant to this change (if needed)

@CodeLieutenant CodeLieutenant requested a review from soyacz January 19, 2026 11:23
@CodeLieutenant CodeLieutenant self-assigned this Jan 19, 2026
@CodeLieutenant CodeLieutenant added backport/none Backport is not required test-integration Enable running the integration tests suite labels Jan 19, 2026
@CodeLieutenant
Copy link
Contributor Author

CodeLieutenant commented Jan 19, 2026

I think this should be backported, as it contains fixes needed by other branches, but this PR cannot be unless all other branches switch to v4 driver by default.

@fruch @soyacz Should I extract this change from v3 to v4 in another PR so that this can be backported?

@CodeLieutenant CodeLieutenant force-pushed the feat--cassandra-stress-v4 branch from 62fb558 to 0058a1f Compare January 19, 2026 11:38
@soyacz
Copy link
Contributor

soyacz commented Jan 19, 2026

I think this should be backported, as it contains fixes needed by other branches, but this PR cannot be unless all other branches switch to v4 driver by default.

@fruch @soyacz Should I extract this change from v3 to v4 in another PR so that this can be backported?

I didn't backport yet the previous PR with c-s update as I saw we didn't backport previous version. @fruch do we backport c-s to older branches (perf-v17 still must be backported)?

soyacz
soyacz reviewed Jan 19, 2026
View reviewed changes
@CodeLieutenant CodeLieutenant force-pushed the feat--cassandra-stress-v4 branch from 0058a1f to 0164e2a Compare January 19, 2026 12:07
pehala
pehala reviewed Jan 20, 2026
View reviewed changes
sdcm/reporting/tooling_reporter.py
Comment on lines +122 to +123
if self.is_driver_4x and (driver_version := result.get("scylla-java-driver-4x")):
self.additional_data = f"java-driver-4x: {driver_version}"
Copy link
Contributor

@pehala pehala Jan 20, 2026

Choose a reason for hiding this comment

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

When can scylla-java-driver-4 be None?

Copy link
Contributor

@fruch fruch Jan 20, 2026

Choose a reason for hiding this comment

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

with older version of c-s

Copy link
Contributor Author

@CodeLieutenant CodeLieutenant Jan 20, 2026

Choose a reason for hiding this comment

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

Should I remove this check then, since it will always use new version from now on?

@fruch
Copy link
Contributor

fruch commented Jan 20, 2026

I think this should be backported, as it contains fixes needed by other branches, but this PR cannot be unless all other branches switch to v4 driver by default.
@fruch @soyacz Should I extract this change from v3 to v4 in another PR so that this can be backported?

I didn't backport yet the previous PR with c-s update as I saw we didn't backport previous version. @fruch do we backport c-s to older branches (perf-v17 still must be backported)?

it's not important for releases since we aren't gonna use it in the releases.
let's target on v17 for the those 4.x related changes

fruch
fruch reviewed Jan 20, 2026
View reviewed changes
@fruch
Copy link
Contributor

fruch commented Jan 20, 2026

@CodeLieutenant the most important requirement here
a short run of a test using 4.x and one 3.x, so we can review the outcome, and prove it's working as expected

@CodeLieutenant CodeLieutenant force-pushed the feat--cassandra-stress-v4 branch from 0164e2a to a707f07 Compare January 20, 2026 17:21
@CodeLieutenant CodeLieutenant force-pushed the feat--cassandra-stress-v4 branch from a707f07 to 7dd2431 Compare January 20, 2026 17:53
@CodeLieutenant
Copy link
Contributor Author

CodeLieutenant commented Jan 20, 2026

@fruch @soyacz @pehala Integration tests failing have nothing to do with c-s and this PR

@CodeLieutenant
Copy link
Contributor Author

CodeLieutenant commented Jan 20, 2026

@soyacz @pehala @fruch A little help here

I've run the same job as for v4 driver, but this
one used cql-stress for some unknown reason to me.

The only difference I've out is ENV SCT_C_S_DRIVER_VERSION=3. Is this a new behavior in master, to run cal-stress instead of c-s

Screenshot 2026-01-20 at 19 00 27

@CodeLieutenant CodeLieutenant marked this pull request as ready for review January 20, 2026 18:55
@soyacz
Copy link
Contributor

soyacz commented Jan 21, 2026

@soyacz @pehala @fruch A little help here

I've run the same job as for v4 driver, but this one used cql-stress for some unknown reason to me.

The only difference I've out is ENV SCT_C_S_DRIVER_VERSION=3. Is this a new behavior in master, to run cal-stress instead of c-s

the reason is that test config you used is test-cases/longevity/longevity-100gb-4h-cql-stress.yaml

@CodeLieutenant
Copy link
Contributor Author

CodeLieutenant commented Jan 21, 2026

@soyacz @pehala @fruch A little help here

I've run the same job as for v4 driver, but this one used cql-stress for some unknown reason to me.

The only difference I've out is ENV SCT_C_S_DRIVER_VERSION=3. Is this a new behavior in master, to run cal-stress instead of c-s

the reason is that test config you used is test-cases/longevity/longevity-100gb-4h-cql-stress.yaml

But the same test worked for java v4, that strange here, I'll update the config and rerun

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fruch fruch Awaiting requested review from fruch fruch is a code owner

@pehala pehala Awaiting requested review from pehala

@soyacz soyacz Awaiting requested review from soyacz soyacz is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees

@CodeLieutenant CodeLieutenant

Labels

backport/none Backport is not required test-integration Enable running the integration tests suite

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@CodeLieutenant @soyacz @fruch @pehala