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

rptest/scale_tests: compute expected time for non-ts workload #16704

Merged

Conversation

nvartolomei
Copy link
Contributor

@nvartolomei nvartolomei commented Feb 24, 2024

This restores the default timeout for non-ts workload as it was pre my change. We have the bandwidth setting for the single producer/consumer but we fail to achieve it with random reads.

Fixes #16696

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.3.x
  • v23.2.x
  • v23.1.x

Release Notes

  • none

@nvartolomei nvartolomei force-pushed the nv/scale-mpt-fix-non-tiered-storage branch from 31cb4d0 to 169f6af Compare February 24, 2024 04:38
@nvartolomei
Copy link
Contributor Author

/cdt
tests/rptest/scale_tests/many_partitions_test.py

@nvartolomei nvartolomei force-pushed the nv/scale-mpt-fix-non-tiered-storage branch from 169f6af to d0fe469 Compare February 26, 2024 10:04
@nvartolomei
Copy link
Contributor Author

/cdt
tests/rptest/scale_tests/many_partitions_test.py

@nvartolomei
Copy link
Contributor Author

Test failures are unrelated. Some CI infra instability and builds/ducktape runs are irrelevant for this runs beside cdt-aws-ci.

@@ -587,12 +587,22 @@ def _write_and_random_read(self, scale: ScaleParameters, topic_names):
self.logger.info(
"Write+randread stress test complete, verifying sequentially")

max_msgs = None
expect_transmit_time = 600 # empirically derived
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah my comment got swallowed, though this still LGTM.

I would've expected this to be set to None if trying to revert the behavior to before https://github.com/redpanda-data/redpanda/pull/15287/files#diff-1918605fe4f7ee419c1af283695cfb29917fbad61e10faafa40623c051a5e0c4L590

That said, looking around, I'm not sure where None is handled gracefully, and 600 seems much better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. I'd leave it as-is as explicit value here is a tad more readable.

@nvartolomei nvartolomei merged commit 8f64df2 into redpanda-data:dev Feb 27, 2024
17 of 19 checks passed
@dotnwat
Copy link
Member

dotnwat commented Feb 28, 2024

Thanks @nvartolomei !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI Failure (key symptom) in ManyPartitionsTest.test_many_partitions_compacted
3 participants