-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[Fix][Flaky-test] Fix testConsumeTxnMessage #16981
[Fix][Flaky-test] Fix testConsumeTxnMessage #16981
Conversation
Master #14109 ## Motivation The transaction commit is async, so the consumer can still receive message when the consumer rebuilds. ## Modification Add Awaitility.await().
@liangyepianzhou Do you see consumer reconnection logs? Yes, the change may be can resolve the flaky test, but it will also hide a potential BUG of message duplication. |
We should consider checking if the consumer is reconnected to the broker, if yes, the message duplication is expected. Otherwise, we should not receive duplicated messages. |
The consumer will reconnect and the message duplication is not expected. |
So, we should fix the test to make sure the consumer start to consume messages after confirm the last transaction has committed? |
Right, but we don't know when the last transaction was committed. |
Can we check through the TC stats? As I said, the change will hide the BUG that introduced the message duplication issues. If we don't have a way to check if the last transaction is committed, we should add it. |
* [Fix][Flaky-test] Fix testConsumeTxnMessage Master #14109 ## Motivation The transaction commit is async, so the consumer can still receive message when the consumer rebuilds. ## Modification Add Awaitility.await() for check-ing whether the ongoingTxns = 0.
* [Fix][Flaky-test] Fix testConsumeTxnMessage Master apache#14109 ## Motivation The transaction commit is async, so the consumer can still receive message when the consumer rebuilds. ## Modification Add Awaitility.await() for check-ing whether the ongoingTxns = 0.
* [Fix][Flaky-test] Fix testConsumeTxnMessage Master apache#14109 ## Motivation The transaction commit is async, so the consumer can still receive message when the consumer rebuilds. ## Modification Add Awaitility.await() for check-ing whether the ongoingTxns = 0. (cherry picked from commit c29503e) (cherry picked from commit 6db6679)
This PR depends on #15675 which is not included in branch-2.10. so remove |
This reverts commit dbf3b59.
Fixes #14109
Motivation
The transaction commit is async, so the consumer can still receive message when the consumer rebuilds.
Modification
Add Awaitility.await() for check-ing whether the ongoingTxns = 0.
Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Does this pull request potentially affect one of the following parts:
If
yes
was chosen, please highlight the changesDocumentation
Check the box below or label this PR directly.
Need to update docs?
doc-required
(Your PR needs to update docs and you will update later)
doc-not-needed
(Please explain why)
doc
(Your PR contains doc changes)
doc-complete
(Docs have been already added)