Skip to content

[fix][test]Fix flaky test DispatcherBlockConsumerTest.testBrokerSubscriptionRecovery #18228

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

Merged

Conversation

poorbarcode
Copy link
Contributor

@poorbarcode poorbarcode commented Oct 28, 2022

Fixes #17011

Motivation

  1. In this test, make a batch of messages is acked, and make the remaining messages are redelivered, then try to receive the redelivered messages again. However, the sequence num is used for cmd-ack. But if the order of the messages changes, the acked messages will not be the expected messages, which makes the test unstable.

截屏2022-09-02 12 15 36

  1. There is a bug that caused repeated consumption, PR [fix][broker]fix Repeated messages of shared dispatcher #18227 has fixed it.

Changes

Use the message content to determine whether the message should ack.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Oct 28, 2022
@poorbarcode poorbarcode changed the title [fix][flaky-test]DispatcherBlockConsumerTest. testBrokerSubscriptionRecovery [fix][flaky-test]DispatcherBlockConsumerTest.testBrokerSubscriptionRecovery Oct 28, 2022
@poorbarcode poorbarcode changed the title [fix][flaky-test]DispatcherBlockConsumerTest.testBrokerSubscriptionRecovery [fix][test]fix flaky test: DispatcherBlockConsumerTest.testBrokerSubscriptionRecovery Oct 28, 2022
@codelipenghui codelipenghui added this to the 2.12.0 milestone Oct 28, 2022
@codelipenghui codelipenghui reopened this Oct 28, 2022
@Technoboy- Technoboy- changed the title [fix][test]fix flaky test: DispatcherBlockConsumerTest.testBrokerSubscriptionRecovery [fix][test]Fix flaky test DispatcherBlockConsumerTest.testBrokerSubscriptionRecovery Oct 28, 2022
@codecov-commenter
Copy link

codecov-commenter commented Oct 28, 2022

Codecov Report

Merging #18228 (7c325f6) into master (6c65ca0) will increase coverage by 3.62%.
The diff coverage is 34.49%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #18228      +/-   ##
============================================
+ Coverage     34.91%   38.54%   +3.62%     
- Complexity     5707     8209    +2502     
============================================
  Files           607      683      +76     
  Lines         53396    67290   +13894     
  Branches       5712     7218    +1506     
============================================
+ Hits          18644    25934    +7290     
- Misses        32119    38396    +6277     
- Partials       2633     2960     +327     
Flag Coverage Δ
unittests 38.54% <34.49%> (+3.62%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...org/apache/bookkeeper/mledger/LedgerOffloader.java 0.00% <ø> (ø)
...che/bookkeeper/mledger/LedgerOffloaderFactory.java 0.00% <ø> (ø)
...pache/bookkeeper/mledger/LedgerOffloaderStats.java 0.00% <ø> (ø)
...ookkeeper/mledger/LedgerOffloaderStatsDisable.java 0.00% <ø> (ø)
...a/org/apache/bookkeeper/mledger/ManagedCursor.java 85.71% <ø> (ø)
...che/bookkeeper/mledger/ManagedLedgerException.java 41.17% <ø> (ø)
...bookkeeper/mledger/ManagedLedgerFactoryConfig.java 86.66% <ø> (ø)
...g/apache/bookkeeper/mledger/ManagedLedgerInfo.java 22.22% <ø> (ø)
...he/bookkeeper/mledger/OffloadedLedgerMetadata.java 0.00% <ø> (ø)
...ava/org/apache/bookkeeper/mledger/ScanOutcome.java 0.00% <ø> (ø)
... and 723 more

@Technoboy- Technoboy- merged commit b7b9239 into apache:master Oct 28, 2022
@poorbarcode poorbarcode deleted the flaky/testBrokerSubscriptionRecovery branch October 30, 2022 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs ready-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flaky-test: PersistentStreamingDispatcherBlockConsumerTest.testBrokerSubscriptionRecovery
5 participants