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

improvement(latency_calculator): allow use it in longevity tests #9628

Conversation

soyacz
Copy link
Contributor

@soyacz soyacz commented Dec 30, 2024

latency_calculator_decorator requires workload to be set. Up to now it was taken from the test name. This change allows to set it by workload_name test parameter, so it can be used in all kind of tests.

Added latency verification for all 'individual nemesis' cases (proposal, anyway need to test with something).

closes: #9627

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)

Copy link
Contributor

@juliayakovlev juliayakovlev left a comment

Choose a reason for hiding this comment

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

@soyacz @fruch
Should we calculate latency for prepare stage in longevity?

def run_prepare_write_cmd(self):

Maybe we need go over tests and check where it is important to have

`latency_calculator_decorator` requires workload to be set. Up to now it
was taken from the test name. This change allows to set it by
`workload_name` test parameter, so it can be used in all kind of tests.

Added latency verification for all 'individual nemesis' cases (proposal,
anyway need to test with something).

closes: scylladb#9627
@soyacz soyacz force-pushed the make-latency-calculator-decorator-to-work-with-longevities branch from 7de539a to 1c62e8d Compare December 31, 2024 07:43
@soyacz soyacz marked this pull request as ready for review December 31, 2024 07:43
@fruch
Copy link
Contributor

fruch commented Dec 31, 2024

@soyacz @fruch Should we calculate latency for prepare stage in longevity?

def run_prepare_write_cmd(self):

Maybe we need go over tests and check where it is important to have

@soyacz was just testing, to see we can collect metric into argus also in longevity, as needed.

we are not gonna collect all metric from all longevity (at least not now)

@soyacz
Copy link
Contributor Author

soyacz commented Dec 31, 2024

@soyacz @fruch Should we calculate latency for prepare stage in longevity?

def run_prepare_write_cmd(self):

Maybe we need go over tests and check where it is important to have

@soyacz was just testing, to see we can collect metric into argus also in longevity, as needed.

we are not gonna collect all metric from all longevity (at least not now)

Yes, I tried it only, decision to add to all longevities is separate one.

I think we could consider it individual nemesis cases - we could see the trends and possibly find regressions with it. But requires more effort as latencies are above 10ms causing test to fail, so we would need to make more decisions (whether increase error thresholds or reduce load/throttle it).

Copy link
Contributor

@fruch fruch 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 2bfc791 into scylladb:master Dec 31, 2024
7 checks passed
@fruch fruch added the backport/none Backport is not required label Dec 31, 2024
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 promoted-to-master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make latency_calculator_decorator be usable in longevity tests
4 participants