Skip to content
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.

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

feature(nemesis): introduce lock for target selection #7016

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

fruch
Copy link
Contributor

@fruch fruch commented Dec 26, 2023

since we run into multiple cases on parallel nemesis that there were multiple nemesis using the same node

we are introducing a lock over the selection of target nodes so we won't be able to pick it multiple times

Fixes: #6553

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 relevent to this change (if needed)

@fruch fruch added test-provision-aws Run provision test on AWS test-provision-gce Run provision test on GCE labels Dec 26, 2023
@fruch fruch force-pushed the add_nemesis_target_lock branch from c413483 to 7954f29 Compare December 26, 2023 22:27
@fruch fruch marked this pull request as draft December 26, 2023 22:27
@fruch fruch force-pushed the add_nemesis_target_lock branch 2 times, most recently from 18ed779 to 9e72716 Compare December 27, 2023 15:56
@fruch fruch marked this pull request as ready for review December 28, 2023 21:38
@fruch fruch force-pushed the add_nemesis_target_lock branch 2 times, most recently from dfd5080 to 097ec3b Compare December 31, 2023 09:32
@fruch fruch removed test-provision-aws Run provision test on AWS test-provision-gce Run provision test on GCE labels Jan 1, 2024
Copy link
Contributor

@vponomaryov vponomaryov left a comment

Choose a reason for hiding this comment

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

Please, remove commits that were merged as part of another your PR: https://github.com/scylladb/scylla-cluster-tests/pull/7024/commits

@fruch fruch force-pushed the add_nemesis_target_lock branch from 097ec3b to 4b25a0a Compare January 18, 2024 08:54
@fruch fruch requested a review from vponomaryov January 18, 2024 08:55
@fruch
Copy link
Contributor Author

fruch commented Jan 18, 2024

Please, remove commits that were merged as part of another your PR: https://github.com/scylladb/scylla-cluster-tests/pull/7024/commits

removed, and solved conflicts

sdcm/cluster.py Show resolved Hide resolved
sdcm/nemesis.py Show resolved Hide resolved
@fruch fruch force-pushed the add_nemesis_target_lock branch from 4b25a0a to 8358b9e Compare January 30, 2024 10:26
sdcm/cluster.py Outdated Show resolved Hide resolved
Copy link
Contributor

@vponomaryov vponomaryov left a comment

Choose a reason for hiding this comment

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

Need to fix the breakage with undefined target_node.

since we run into multiple cases on parallel nemesis
that there were multiple nemesis using the same node

we are introducing a lock over the selection of target nodes
so we won't be able to pick it multiple times

Fixes: scylladb#6553
@fruch fruch force-pushed the add_nemesis_target_lock branch from 8358b9e to 32e8c89 Compare February 4, 2024 10:25
@fruch fruch requested a review from vponomaryov February 4, 2024 10:25
Copy link
Contributor

@vponomaryov vponomaryov left a comment

Choose a reason for hiding this comment

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

LGTM

@fruch fruch merged commit 2a551db into scylladb:master Feb 13, 2024
5 checks passed
fruch added a commit to fruch/scylla-cluster-tests that referenced this pull request Feb 21, 2024
in PR scylladb#7016 there was a change that drop this part from the code
without it, we can get into case that nemesis which are calling this
function directly, might leave some nodes mark with `running_nemesis`
while no cleaup code can figure it out, and would unmark only the
current target selected.

Fixes: scylladb#7220
vponomaryov pushed a commit that referenced this pull request Feb 21, 2024
in PR #7016 there was a change that drop this part from the code
without it, we can get into case that nemesis which are calling this
function directly, might leave some nodes mark with `running_nemesis`
while no cleaup code can figure it out, and would unmark only the
current target selected.

Fixes: #7220
@fruch fruch added backport/2023.1 Need to backport to 2023.1 backport/2024.1 Need backport to 2024.1 promoted-to-master labels Aug 28, 2024
mergify bot pushed a commit that referenced this pull request Aug 28, 2024
in PR #7016 there was a change that drop this part from the code
without it, we can get into case that nemesis which are calling this
function directly, might leave some nodes mark with `running_nemesis`
while no cleaup code can figure it out, and would unmark only the
current target selected.

Fixes: #7220
(cherry picked from commit dbb58dc)
mergify bot pushed a commit that referenced this pull request Aug 28, 2024
in PR #7016 there was a change that drop this part from the code
without it, we can get into case that nemesis which are calling this
function directly, might leave some nodes mark with `running_nemesis`
while no cleaup code can figure it out, and would unmark only the
current target selected.

Fixes: #7220
(cherry picked from commit dbb58dc)
fruch added a commit that referenced this pull request Aug 28, 2024
in PR #7016 there was a change that drop this part from the code
without it, we can get into case that nemesis which are calling this
function directly, might leave some nodes mark with `running_nemesis`
while no cleaup code can figure it out, and would unmark only the
current target selected.

Fixes: #7220
(cherry picked from commit dbb58dc)
fruch added a commit that referenced this pull request Aug 28, 2024
in PR #7016 there was a change that drop this part from the code
without it, we can get into case that nemesis which are calling this
function directly, might leave some nodes mark with `running_nemesis`
while no cleaup code can figure it out, and would unmark only the
current target selected.

Fixes: #7220
(cherry picked from commit dbb58dc)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/2023.1 Need to backport to 2023.1 backport/2024.1 Need backport to 2024.1 promoted-to-master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 nemesis: NonDisruptive and Disruptive running on same node.
2 participants