-
Notifications
You must be signed in to change notification settings - Fork 14.8k
KAFKA-19429: Deflake streams_smoke_test #20019
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
Conversation
It looks like we are checking for properties that are not guaranteed under at_least_once, for example, exact counting (not allowing for overcounting). This change relaxes the validation constraint to only check that we we counted _at least_ N messages, and our sums come out as _at least_ the expected sum.
|
PTAL @aliehsaeedii |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Relax the smoke test validation to only require at least the expected counts and sums under at-least-once semantics (non-EOS), while preserving exact validation for EOS.
- Introduce a
validationPredicateto switch between exact (equals) and at-least (>=) checks - Refactor all
verifyandverifyTAggcalls to accept the new predicate - Add
lessEqualshelper for the at-least-once comparison logic
Comments suppressed due to low confidence (3)
streams/src/test/java/org/apache/kafka/streams/tests/SmokeTestDriver.java:517
- [nitpick] Consider renaming
validationPredicateto something likecomparisonPredicateorvalueComparatorto clarify that it encapsulates the comparison operation between expected and actual values.
final BiPredicate<Number, Number> validationPredicate;
streams/src/test/java/org/apache/kafka/streams/tests/SmokeTestDriver.java:665
- Add unit tests for the new at-least-once (
lessEquals) predicate and the EOS vs non-EOS validation paths in bothverifyTAggandverifyto ensure the new logic behaves as expected.
final BiPredicate<Number, Number> validationPredicate,
streams/src/test/java/org/apache/kafka/streams/tests/SmokeTestDriver.java
Outdated
Show resolved
Hide resolved
streams/src/test/java/org/apache/kafka/streams/tests/SmokeTestDriver.java
Show resolved
Hide resolved
streams/src/test/java/org/apache/kafka/streams/tests/SmokeTestDriver.java
Outdated
Show resolved
Hide resolved
…Driver.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…Driver.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
It looks like we are checking for properties that are not guaranteed under at_least_once, for example, exact counting (not allowing for overcounting). This change relaxes the validation constraint to only check that we counted _at least_ N messages, and our sums come out as _at least_ the expected sum. Reviewers: Matthias J. Sax <matthias@confluent.io>
It looks like we are checking for properties that are not guaranteed under at_least_once, for example, exact counting (not allowing for overcounting). This change relaxes the validation constraint to only check that we counted _at least_ N messages, and our sums come out as _at least_ the expected sum. Reviewers: Matthias J. Sax <matthias@confluent.io>
It looks like we are checking for properties that are not guaranteed under at_least_once, for example, exact counting (not allowing for overcounting). This change relaxes the validation constraint to only check that we counted _at least_ N messages, and our sums come out as _at least_ the expected sum. Reviewers: Matthias J. Sax <matthias@confluent.io>
It looks like we are checking for properties that are not guaranteed under at_least_once, for example, exact counting (not allowing for overcounting). This change relaxes the validation constraint to only check that we counted _at least_ N messages, and our sums come out as _at least_ the expected sum. Reviewers: Matthias J. Sax <matthias@confluent.io>
It looks like we are checking for properties that are not guaranteed
under at_least_once, for example, exact counting (not allowing for
overcounting).
This change relaxes the validation constraint to only check that we
counted at least N messages, and our sums come out as at least the
expected sum.
Reviewers: Matthias J. Sax matthias@confluent.io