Skip to content

fix(nemesis): Fix wrong nemesis usage#13287

Open
pehala wants to merge 4 commits intoscylladb:masterfrom
pehala:fix_wrong_nemesis_usage
Open

fix(nemesis): Fix wrong nemesis usage#13287
pehala wants to merge 4 commits intoscylladb:masterfrom
pehala:fix_wrong_nemesis_usage

Conversation

@pehala
Copy link
Contributor

@pehala pehala commented Jan 21, 2026

Nemesis execution is not intuitive and the actual execution mechanism does not execute code inside disrupt method in nemesis classes, it is only used for pattern matching and then the same method is executed with default arguments.
This PR fixes it and as a consequence we will now test more nemesis, so needs to carefully tested.

ManagerBackup nemesis are in COMPLEX_NEMESIS and as such are not executed by Sisyphus, so for them this PR is not critical but it improves them regardless and brings them closer to how nemesis should look like

  • Simplifies manager backup nemesis logic
    • Split into two disrupt methods
    • Remove single use methods
  • Fix ClusterRollingRestartRandomOrder by adding new disrupt method

Fixes #13039

Testing

PR pre-checks (self review)

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

@pehala pehala requested a review from yarongilor January 21, 2026 09:33
@pehala pehala force-pushed the fix_wrong_nemesis_usage branch from 738cc79 to 0f84590 Compare January 21, 2026 09:33
@pehala pehala added the backport/none Backport is not required label Jan 21, 2026
@pehala pehala changed the title Fix wrong nemesis usage fix(nemesis): Fix wrong nemesis usage Jan 21, 2026
Copy link
Contributor

@yarongilor yarongilor left a comment

Choose a reason for hiding this comment

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

the manager backup nemesis works fine in performance test in:
scylla-enterprise-perf-manager-native-backup-nemesis.jenkinsfile
It is not used by Sisyphus, no need to modify it.

@pehala
Copy link
Contributor Author

pehala commented Jan 21, 2026

the manager backup nemesis works fine in performance test in:
scylla-enterprise-perf-manager-native-backup-nemesis.jenkinsfile
It is not used by Sisyphus, no need to modify it.

Why it is not used by Sisyphus? It has class, it is in the yamls, so it should be used

@pehala
Copy link
Contributor Author

pehala commented Jan 21, 2026

Why it is not used by Sisyphus? It has class, it is in the yamls, so it should be used

Discussed offline:
Yaron is correct, it is in COMPLEX_NEMESIS and as such is not run. This patch brings it closer to running as proper nemesis, more PRs are needed but this is still improvement and a pushes it in a good direction. Followups will enable it so it runs regularly in supported environments

@fruch
Copy link
Contributor

fruch commented Jan 21, 2026

@pehala @yarongilor why this nemesis are not part of Sisyphus ? where is the reason documented ?

@pehala
Copy link
Contributor Author

pehala commented Jan 21, 2026

@pehala @yarongilor why this nemesis are not part of Sisyphus ? where is the reason documented ?

Due to bucket and object storage requirements that are region specific. It is not documented and I was confused by it as well, I can add documentation in this PR

I also created task to enable them and we will tackle it in followup

yarongilor
yarongilor previously approved these changes Jan 22, 2026
Copy link
Contributor

@yarongilor yarongilor left a comment

Choose a reason for hiding this comment

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

LGTM, if nemeses tested and passed.

@pehala pehala marked this pull request as ready for review January 23, 2026 08:21
@pehala pehala requested a review from a team January 23, 2026 09:16
@scylladbbot
Copy link

@pehala new branch branch-2026.1 was added, please add backport label if needed

@pehala pehala force-pushed the fix_wrong_nemesis_usage branch from 75f1b48 to a17c294 Compare January 25, 2026 09:58
@pehala pehala added backport/2026.1 and removed backport/none Backport is not required labels Jan 25, 2026
@fruch
Copy link
Contributor

fruch commented Jan 27, 2026

@pehala
would recommend rebasing, and fixing the precommit issues (new labels for nemesis from other PR)

@pehala pehala force-pushed the fix_wrong_nemesis_usage branch from a17c294 to e729287 Compare January 28, 2026 09:34
@pehala pehala added backport/none Backport is not required and removed backport/2026.1 labels Jan 29, 2026
@pehala pehala marked this pull request as draft January 29, 2026 13:34
Removed arguments so they are now run as expected.
Simplify method for backup
It was never executed in the expected way
Add disrupt_rolling_restart_cluster_random to effectively reenable it
…veBackup

Explains why they are not part of Sisyphus
@pehala pehala force-pushed the fix_wrong_nemesis_usage branch from e729287 to 33313b6 Compare January 30, 2026 12:35
@pehala pehala marked this pull request as ready for review January 30, 2026 13:24
@pehala
Copy link
Contributor Author

pehala commented Jan 30, 2026

v2:

  • Rebased
  • Rerun test in the cover letter to verify

@pehala pehala requested a review from yarongilor January 30, 2026 13:25
Was not executed as part of Sisyphus and served no real purpose
@pehala pehala force-pushed the fix_wrong_nemesis_usage branch from 33313b6 to 331b367 Compare January 30, 2026 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/none Backport is not required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Resolve Nemesis Classes with arguments

4 participants